Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 77 additions & 3 deletions src/deploy/functions/backend.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe("Backend", () => {
const RUN_SERVICE: run.Service = {
name: "projects/project/locations/region/services/id",
labels: {
"goog-managed-by": "cloud-functions",
"goog-managed-by": "cloudfunctions",
"goog-cloudfunctions-runtime": "nodejs16",
"firebase-functions-codebase": "default",
},
Expand Down Expand Up @@ -128,6 +128,17 @@ describe("Backend", () => {
expect(backend.functionName(ENDPOINT)).to.equal(
"projects/project/locations/region/functions/id",
);
expect(backend.eventarcTriggerIdForFunction(ENDPOINT)).to.equal("firebase-id-region");
expect(
backend.eventarcTriggerIdForFunction({ ...ENDPOINT, region: "us-central1" }),
).not.to.equal(backend.eventarcTriggerIdForFunction({ ...ENDPOINT, region: "europe-west1" }));
expect(
backend.eventarcTriggerIdForFunction({
...ENDPOINT,
id: "a".repeat(60),
region: "us-central1",
}),
).to.have.length(63);
});

it("merge", () => {
Expand Down Expand Up @@ -332,21 +343,60 @@ describe("Backend", () => {
},
labels: {
"deployment-tool": "cli-firebase",
"goog-managed-by": "cloud-functions",
"goog-managed-by": "cloudfunctions",
"goog-cloudfunctions-runtime": "nodejs16",
"firebase-functions-codebase": "default",
},
secretEnvironmentVariables: [],
ingressSettings: "ALLOW_ALL" as const,
timeoutSeconds: 60,
serviceAccount: null,
runServiceId: "id",
};
delete wantEndpoint.state;

expect(have).to.deep.equal(backend.of(wantEndpoint));
expect(listAllFunctionsV2).to.not.have.been.called;
});

it("should read existing functions from Cloud Run when Dart functions are enabled", async () => {
isEnabled.withArgs("dartfunctions").returns(true);
listAllFunctions.onFirstCall().resolves({
functions: [],
unreachable: [],
});
listAllFunctionsV2.onFirstCall().resolves({
functions: [HAVE_CLOUD_FUNCTION_V2],
unreachable: [],
});
const directRunService: run.Service = {
...RUN_SERVICE,
name: "projects/project/locations/region/services/dart-function",
labels: {
...RUN_SERVICE.labels,
"goog-managed-by": "firebase-functions",
"goog-cloudfunctions-runtime": "dart3",
},
annotations: {
"firebase-functions-metadata": JSON.stringify({
functionId: "dart-function",
}),
},
};
listServices.onFirstCall().resolves([RUN_SERVICE, directRunService]);

const context = newContext();
context.projectId = "project";
const have = await backend.existingBackend(context);

expect(listServices).to.have.been.calledOnceWith("project");
expect(listAllFunctionsV2).to.have.been.calledOnceWith("project");
expect(backend.allEndpoints(have).map((endpoint) => endpoint.platform)).to.have.members([
"gcfv2",
"run",
]);
});

it("should handle Cloud Run list errors gracefully when experiment is enabled", async () => {
isEnabled.withArgs("functionsrunapionly").returns(true);
listAllFunctions.onFirstCall().resolves({
Expand Down Expand Up @@ -387,14 +437,15 @@ describe("Backend", () => {
},
labels: {
"deployment-tool": "cli-firebase",
"goog-managed-by": "cloud-functions",
"goog-managed-by": "cloudfunctions",
"goog-cloudfunctions-runtime": "nodejs16",
"firebase-functions-codebase": "default",
},
secretEnvironmentVariables: [],
ingressSettings: "ALLOW_ALL" as const,
timeoutSeconds: 60,
serviceAccount: null,
runServiceId: "id",
};
delete wantEndpoint.state;

Expand Down Expand Up @@ -500,6 +551,29 @@ describe("Backend", () => {
);
});

it("should throw when Cloud Run discovery fails for a Dart deployment", async () => {
isEnabled.withArgs("dartfunctions").returns(true);
listAllFunctions.onFirstCall().resolves({
functions: [],
unreachable: [],
});
listAllFunctionsV2.onFirstCall().resolves({
functions: [],
unreachable: [],
});
listServices.rejects(new Error("Cloud Run API unavailable"));
const want = backend.of({
...ENDPOINT,
platform: "run",
httpsTrigger: {},
});

await expect(backend.checkAvailability(newContext(), want)).to.eventually.be.rejectedWith(
FirebaseError,
/Existing Dart functions cannot be reconciled safely/,
);
});

it("Should only warn when deploying GCFv1 and GCFv2 is unavailable.", async () => {
listAllFunctions.onFirstCall().resolves({
functions: [],
Expand Down
31 changes: 28 additions & 3 deletions src/deploy/functions/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { assertExhaustive, flattenArray } from "../../functional";
import { logger } from "../../logger";
import * as experiments from "../../experiments";
import * as crypto from "crypto";

/** Retry settings for a ScheduleSpec. */
export interface ScheduleRetryConfig {
Expand Down Expand Up @@ -195,7 +196,7 @@
return allMemoryOptions.includes(mem as MemoryOptions);
}

export function isValidEgressSetting(egress: unknown): egress is VpcEgressSettings {

Check warning on line 199 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Missing JSDoc comment
return egress === "PRIVATE_RANGES_ONLY" || egress === "ALL_TRAFFIC";
}

Expand Down Expand Up @@ -465,7 +466,7 @@
}

/**

Check warning on line 469 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Expected JSDoc block to be aligned
* A helper utility to create an empty backend.
* Tests that verify the behavior of one possible resource in a Backend can use
* this method to avoid compiler errors when new fields are added to Backend.
Expand Down Expand Up @@ -581,6 +582,17 @@
return `firebase-schedule-${cloudFunction.id}-${cloudFunction.region}`;
}

/** Returns a region-qualified Eventarc trigger ID within the 63-character API limit. */
export function eventarcTriggerIdForFunction(cloudFunction: TargetIds): string {
const normalizedId = cloudFunction.id.toLowerCase().replace(/_/g, "-");
const fullId = `firebase-${normalizedId}-${cloudFunction.region}`;
if (fullId.length <= 63) {
return fullId;
}
const hash = crypto.createHash("sha256").update(fullId).digest("hex").slice(0, 8);
return `${fullId.slice(0, 54)}-${hash}`;
Comment on lines +592 to +593

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If fullId.slice(0, 54) ends with a hyphen, appending -${hash} will result in consecutive hyphens (e.g., --hash). While Eventarc trigger IDs can technically contain consecutive hyphens, it is cleaner and more robust to trim any trailing hyphens from the sliced prefix before appending the hash.

  const hash = crypto.createHash(\"sha256\").update(fullId).digest(\"hex\").slice(0, 8);\n  const prefix = fullId.slice(0, 54).replace(/-+$/, \"\");\n  return prefix + \"-\" + hash;

}

/**
* A caching accessor of the existing backend.
* The method explicitly loads Cloud Functions from their API but implicitly deduces
Expand Down Expand Up @@ -619,14 +631,17 @@
}
unreachableRegions.gcfV1 = gcfV1Results.unreachable;

const addEndpoint = (endpoint: Endpoint): void => {
existingBackend.endpoints[endpoint.region] = existingBackend.endpoints[endpoint.region] || {};
existingBackend.endpoints[endpoint.region][endpoint.id] = endpoint;
};

if (experiments.isEnabled("functionsrunapionly")) {
await loadCloudRunServices(ctx, existingBackend, unreachableRegions, false);
} else {
const gcfV2Results = await gcfV2.listAllFunctions(ctx.projectId);
for (const apiFunction of gcfV2Results.functions) {
const endpoint = gcfV2.endpointFromFunction(apiFunction);
existingBackend.endpoints[endpoint.region] = existingBackend.endpoints[endpoint.region] || {};
existingBackend.endpoints[endpoint.region][endpoint.id] = endpoint;
addEndpoint(gcfV2.endpointFromFunction(apiFunction));
}
unreachableRegions.gcfV2 = gcfV2Results.unreachable;

Expand All @@ -643,7 +658,7 @@
/**
* Loads Cloud Run services into the existing backend.
* @param ctx Context from the Command library, used for caching.
* @param existingBackend The existing backend to load Cloud Run services into.

Check warning on line 661 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Missing @param "unreachableRegions.run"
* @param unreachableRegions Object to track unreachable regions.
* @param onlyMissing If true, only loads missing Cloud Run services.
*/
Expand All @@ -663,8 +678,8 @@
existingBackend.endpoints[endpoint.region][endpoint.id] = endpoint;
}
}
} catch (err: any) {

Check warning on line 681 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unexpected any. Specify a different type
logger.debug(`Error loading Cloud Run services: ${err.message}`);

Check warning on line 682 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe member access .message on an `any` value

Check warning on line 682 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Invalid type "any" of template literal expression
unreachableRegions.run = ["unknown"];
}
}
Expand All @@ -681,9 +696,12 @@
await existingBackend(context);
const gcfV1Regions = new Set();
const gcfV2Regions = new Set();
let hasRunEndpoints = false;
for (const ep of allEndpoints(want)) {
if (ep.platform === "gcfv1") {
gcfV1Regions.add(ep.region);
} else if (ep.platform === "run") {
hasRunEndpoints = true;
} else {
gcfV2Regions.add(ep.region);
}
Expand Down Expand Up @@ -711,6 +729,13 @@
);
}

if (hasRunEndpoints && context.unreachableRegions?.run.length) {
throw new FirebaseError(
"Cloud Run services could not be listed for this project. " +
"Existing Dart functions cannot be reconciled safely. Ensure the Cloud Run API is enabled and that you have permission to list services, then try again.",
);
}

if (context.unreachableRegions?.gcfV1.length) {
utils.logLabeledWarning(
"functions",
Expand Down Expand Up @@ -850,7 +875,7 @@
logger.info(
`Function name ${httpsFunc.id} is too long to have a deterministic Cloud Run URI. Printing the non-deterministic URI instead.`,
);
return httpsFunc.uri!;

Check warning on line 878 in src/deploy/functions/backend.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Forbidden non-null assertion
}
return `https://${serviceName}-${projectNumber}.${httpsFunc.region}.run.app`;
}
82 changes: 82 additions & 0 deletions src/deploy/functions/checkIam.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import * as checkIam from "./checkIam";
import * as storage from "../../gcp/storage";
import * as rm from "../../gcp/resourceManager";
import * as iam from "../../gcp/iam";
import * as backend from "./backend";

const projectId = "my-project";
Expand Down Expand Up @@ -75,6 +76,26 @@
});
});

describe("obtainEventarcDeliveryAgentBindings", () => {
it("grants delivery roles to explicit service accounts", () => {
const serviceAccount = "eventarc@my-project.iam.gserviceaccount.com";

const bindings = checkIam.obtainEventarcDeliveryAgentBindings([serviceAccount]);
expect(bindings).to.deep.equal([
{
role: checkIam.RUN_INVOKER_ROLE,
members: [`serviceAccount:${serviceAccount}`],
},
{
role: checkIam.EVENTARC_EVENT_RECEIVER_ROLE,
members: [`serviceAccount:${serviceAccount}`],
},
]);
bindings[0].members.push("serviceAccount:another@example.com");
expect(bindings[1].members).not.to.include("serviceAccount:another@example.com");
});
});

describe("ensureServiceAgentRoles", () => {
it("should return early if we do not have new services", async () => {
const v1EventFn: backend.Endpoint = {
Expand Down Expand Up @@ -119,6 +140,67 @@
expect(setIamStub).to.not.have.been.called;
});

it("should ensure delivery roles for new direct Run event functions", async () => {
const serviceAccount = "eventarc@";
const expandedServiceAccount = "eventarc@my-project.iam.gserviceaccount.com";
const runEventFn: backend.Endpoint = {
id: "dartFirestore",
entryPoint: "dartFirestore",
platform: "run",
eventTrigger: {
eventType: "google.cloud.firestore.document.v1.written",
retry: false,
serviceAccount,
},
...SPEC,
project: projectId,
};
getIamStub.resolves({
etag: "etag",
version: 3,
bindings: [BINDING],
});
setIamStub.resolves();

await checkIam.ensureServiceAgentRoles(
projectId,
projectNumber,
backend.of(runEventFn),
backend.empty(),
);

const updatedPolicy = setIamStub.firstCall.args[1];

Check warning on line 172 in src/deploy/functions/checkIam.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe assignment of an `any` value
const eventReceiverBinding = updatedPolicy.bindings.find(

Check warning on line 173 in src/deploy/functions/checkIam.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe member access .bindings on an `any` value

Check warning on line 173 in src/deploy/functions/checkIam.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe assignment of an `any` value
(binding: iam.Binding) => binding.role === checkIam.EVENTARC_EVENT_RECEIVER_ROLE,
);
expect(eventReceiverBinding?.members).to.include(`serviceAccount:${expandedServiceAccount}`);
});

it("should not read IAM for unchanged direct Run event functions", async () => {
const runEventFn: backend.Endpoint = {
id: "dartFirestore",
entryPoint: "dartFirestore",
platform: "run",
eventTrigger: {
eventType: "google.cloud.firestore.document.v1.written",
retry: false,
serviceAccount: "eventarc@",
},
...SPEC,
project: projectId,
};

await checkIam.ensureServiceAgentRoles(
projectId,
projectNumber,
backend.of(runEventFn),
backend.of(runEventFn),
);

expect(getIamStub).to.not.have.been.called;
expect(setIamStub).to.not.have.been.called;
});

it("should return early if we fail to get the IAM policy", async () => {
storageStub.resolves(STORAGE_RES);
getIamStub.rejects("Failed to get the IAM policy");
Expand Down
Loading
Loading