diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..02c5458034c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Resolves undefined regions earlier, during the build to backend resolution phase (#10471) diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index df2d3950d6b..efbde154987 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -257,8 +257,8 @@ export type Endpoint = Triggered & { // Defaults to the compute service account when a function is first created as a GCF gen 2 function. serviceAccount?: ServiceAccount | Expression | null; - // defaults to ["us-central1"], overridable in firebase-tools with - // process.env.FIREBASE_FUNCTIONS_DEFAULT_REGION + // Defaults to REGION_TBD. The deployment region is resolved dynamically at deploy-time + // based on event trigger sources or matching existing functions, falling back to "us-central1". region?: ListField; // The Cloud project associated with this endpoint. diff --git a/src/deploy/functions/prepare.spec.ts b/src/deploy/functions/prepare.spec.ts index d99f1648f36..9bfd658002f 100644 --- a/src/deploy/functions/prepare.spec.ts +++ b/src/deploy/functions/prepare.spec.ts @@ -186,57 +186,7 @@ describe("prepare", () => { }); }); - describe("matchRegionsForExisting", () => { - it("does nothing if no endpoints in REGION_TBD", () => { - const want = backend.of(ENDPOINT); - const have = backend.empty(); - prepare.matchRegionsForExisting(want, have); - expect(want).to.deep.equal(backend.of(ENDPOINT)); - }); - - it("infers region from have backend if unique", () => { - const wantE = { ...ENDPOINT, region: build.REGION_TBD }; - const want = backend.of(wantE); - - // Choosing a region that is neither an old or new default - // for the test - const haveE = { ...ENDPOINT, region: "europe-west1" }; - const have = backend.of(haveE); - - prepare.matchRegionsForExisting(want, have); - - expect(want.endpoints["europe-west1"]?.["id"]).to.exist; - expect(want.endpoints["europe-west1"]?.["id"].region).to.equal("europe-west1"); - expect(want.endpoints[build.REGION_TBD]).to.not.exist; - }); - - it("leaves in REGION_TBD if not found in have", () => { - const wantE = { ...ENDPOINT, region: build.REGION_TBD }; - const want = backend.of(wantE); - const have = backend.empty(); - - prepare.matchRegionsForExisting(want, have); - - expect(want.endpoints[build.REGION_TBD]?.["id"]).to.exist; - expect(want.endpoints["us-central1"]).to.not.exist; - }); - - it("throws error if ambiguous", () => { - const wantE = { ...ENDPOINT, region: build.REGION_TBD }; - const want = backend.of(wantE); - - const haveE1 = { ...ENDPOINT, id: "id", region: "us-east1" }; - const haveE2 = { ...ENDPOINT, id: "id", region: "us-west1" }; - const have = backend.of(haveE1, haveE2); - - expect(() => prepare.matchRegionsForExisting(want, have)).to.throw( - FirebaseError, - /Cannot resolve default region for function id. It exists in multiple regions. The region must be specified to continue./, - ); - }); - }); - - describe("resolveDefaultRegions", () => { + describe("resolveDefaultRegionsForBuild", () => { let sandbox: sinon.SinonSandbox; let getDatabaseStub: sinon.SinonStub; let getBucketStub: sinon.SinonStub; @@ -253,74 +203,122 @@ describe("prepare", () => { sandbox.restore(); }); - it("does nothing if no endpoints in REGION_TBD", async () => { - const want = backend.empty(); + it("does nothing if no endpoints or in REGION_TBD", async () => { + const want = build.empty(); const have = backend.empty(); - await prepare.resolveDefaultRegions(want, have); + await prepare.resolveDefaultRegionsForBuild(want, have); expect(want.endpoints).to.deep.equal({}); }); it("infers region from have backend if unique", async () => { - const wantE = { ...ENDPOINT, region: build.REGION_TBD }; - const want = backend.of(wantE); + const want = build.of({ + id: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + httpsTrigger: {}, + region: [build.REGION_TBD], + }, + }); const haveE = { ...ENDPOINT, region: "us-east1" }; const have = backend.of(haveE); - await prepare.resolveDefaultRegions(want, have); + await prepare.resolveDefaultRegionsForBuild(want, have); - expect(want.endpoints["us-east1"]?.["id"]).to.exist; - expect(want.endpoints["us-east1"]?.["id"].region).to.equal("us-east1"); - expect(want.endpoints[build.REGION_TBD]).to.not.exist; + expect(want.endpoints["id"].region).to.deep.equal(["us-east1"]); + }); + + it("resolves region to us-east1 and correctly formats VPC connector path with us-east1", async () => { + const want = build.of({ + id: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + httpsTrigger: {}, + region: [build.REGION_TBD], + vpc: { + connector: "my-connector", + }, + }, + }); + + const haveE = { ...ENDPOINT, id: "id", region: "us-east1" }; + const have = backend.of(haveE); + + await prepare.resolveDefaultRegionsForBuild(want, have); + expect(want.endpoints["id"].region).to.deep.equal(["us-east1"]); + + const backendResult = build.toBackend(want, {}); + const endpointDef = backendResult.endpoints["us-east1"]?.["id"]; + expect(endpointDef).to.not.be.undefined; + expect(endpointDef?.vpc?.connector).to.equal( + "projects/project/locations/us-east1/connectors/my-connector", + ); }); it("throws error if ambiguous", async () => { - const wantE = { ...ENDPOINT, region: build.REGION_TBD }; - const want = backend.of(wantE); + const want = build.of({ + id: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + httpsTrigger: {}, + region: [build.REGION_TBD], + }, + }); const haveE1 = { ...ENDPOINT, id: "id", region: "us-east1" }; const haveE2 = { ...ENDPOINT, id: "id", region: "us-west1" }; const have = backend.of(haveE1, haveE2); - await expect(prepare.resolveDefaultRegions(want, have)).to.be.rejectedWith( + await expect(prepare.resolveDefaultRegionsForBuild(want, have)).to.be.rejectedWith( FirebaseError, /Cannot resolve default region for function id. It exists in multiple regions. The region must be specified to continue./, ); }); it("resolves us-east1 for global resource blocking triggers", async () => { - const wantE: backend.Endpoint = { - ...ENDPOINT_BASE, - id: "beforeCreate", - region: build.REGION_TBD, - blockingTrigger: { - eventType: "providers/cloud.auth/eventTypes/user.beforeCreate", + const want = build.of({ + beforeCreate: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + blockingTrigger: { + eventType: "providers/cloud.auth/eventTypes/user.beforeCreate", + }, }, - }; - const want = backend.of(wantE); + }); const have = backend.empty(); - await prepare.resolveDefaultRegions(want, have); + await prepare.resolveDefaultRegionsForBuild(want, have); - expect(want.endpoints["us-east1"]?.["beforeCreate"]).to.exist; + expect(want.endpoints["beforeCreate"].region).to.deep.equal(["us-east1"]); }); it("resolves us-east1 for global event triggers", async () => { - const wantE: backend.Endpoint = { - ...ENDPOINT_BASE, - id: "onPublish", - region: build.REGION_TBD, - eventTrigger: { - eventType: "google.cloud.pubsub.topic.v1.messagePublished", - retry: false, + const want = build.of({ + onPublish: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + eventTrigger: { + eventType: "google.cloud.pubsub.topic.v1.messagePublished", + retry: false, + }, + region: [build.REGION_TBD], }, - }; - const want = backend.of(wantE); + }); const have = backend.empty(); - await prepare.resolveDefaultRegions(want, have); + await prepare.resolveDefaultRegionsForBuild(want, have); - expect(want.endpoints["us-east1"]?.["onPublish"]).to.exist; + expect(want.endpoints["onPublish"].region).to.deep.equal(["us-east1"]); }); describe("Firestore event triggers", () => { @@ -333,24 +331,27 @@ describe("prepare", () => { testCases.forEach(({ dbLocation, expectedRegion }) => { it(`should resolve ${expectedRegion} when database location is ${dbLocation}`, async () => { - const wantE: backend.Endpoint = { - ...ENDPOINT_BASE, - id: "onDocumentCreate", - region: build.REGION_TBD, - eventTrigger: { - eventType: "google.cloud.firestore.document.v1.created", - eventFilters: { database: "(default)" }, - retry: false, + const want = build.of({ + onDocumentCreate: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + eventTrigger: { + eventType: "google.cloud.firestore.document.v1.created", + eventFilters: { database: "(default)" }, + retry: false, + }, + region: [build.REGION_TBD], }, - }; - const want = backend.of(wantE); + }); const have = backend.empty(); getDatabaseStub.resolves({ locationId: dbLocation }); - await prepare.resolveDefaultRegions(want, have); + await prepare.resolveDefaultRegionsForBuild(want, have); - expect(want.endpoints[expectedRegion]?.["onDocumentCreate"]).to.exist; + expect(want.endpoints["onDocumentCreate"].region).to.deep.equal([expectedRegion]); }); }); }); @@ -365,112 +366,126 @@ describe("prepare", () => { testCases.forEach(({ bucketLocation, expectedRegion }) => { it(`should resolve ${expectedRegion} when bucket location is ${bucketLocation}`, async () => { - const wantE: backend.Endpoint = { - ...ENDPOINT_BASE, - id: "onArchive", - region: build.REGION_TBD, - eventTrigger: { - eventType: "google.cloud.storage.object.v1.archived", - eventFilters: { bucket: "my-bucket" }, - retry: false, + const want = build.of({ + onArchive: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + eventTrigger: { + eventType: "google.cloud.storage.object.v1.archived", + eventFilters: { bucket: "my-bucket" }, + retry: false, + }, + region: [build.REGION_TBD], }, - }; - const want = backend.of(wantE); + }); const have = backend.empty(); getBucketStub.resolves({ location: bucketLocation }); - await prepare.resolveDefaultRegions(want, have); + await prepare.resolveDefaultRegionsForBuild(want, have); - expect(want.endpoints[expectedRegion]?.["onArchive"]).to.exist; + expect(want.endpoints["onArchive"].region).to.deep.equal([expectedRegion]); }); }); }); it("resolves region for Database event triggers based on instance location", async () => { - const wantE: backend.Endpoint = { - ...ENDPOINT_BASE, - id: "onWrite", - region: build.REGION_TBD, - eventTrigger: { - eventType: "google.firebase.database.ref.v1.written", - eventFilters: { instance: "my-instance" }, - retry: false, + const want = build.of({ + onWrite: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + eventTrigger: { + eventType: "google.firebase.database.ref.v1.written", + eventFilters: { instance: "my-instance" }, + retry: false, + }, + region: [build.REGION_TBD], }, - }; - const want = backend.of(wantE); + }); const have = backend.empty(); getDatabaseInstanceDetailsStub.resolves({ location: "europe-west1" }); - await prepare.resolveDefaultRegions(want, have); + await prepare.resolveDefaultRegionsForBuild(want, have); - expect(want.endpoints["europe-west1"]?.["onWrite"]).to.exist; + expect(want.endpoints["onWrite"].region).to.deep.equal(["europe-west1"]); }); it("resolves region for DataConnect event triggers based on service location", async () => { - const wantE: backend.Endpoint = { - ...ENDPOINT_BASE, - id: "onMutationExecuted", - region: build.REGION_TBD, - eventTrigger: { - eventType: "google.firebase.dataconnect.connector.v1.mutationExecuted", - eventFilters: { - service: "projects/project/locations/europe-west1/services/my-service", + const want = build.of({ + onMutationExecuted: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + eventTrigger: { + eventType: "google.firebase.dataconnect.connector.v1.mutationExecuted", + eventFilters: { + service: "projects/project/locations/europe-west1/services/my-service", + }, + retry: false, }, - retry: false, + region: [build.REGION_TBD], }, - }; - const want = backend.of(wantE); + }); const have = backend.empty(); - await prepare.resolveDefaultRegions(want, have); + await prepare.resolveDefaultRegionsForBuild(want, have); - expect(want.endpoints["europe-west1"]?.["onMutationExecuted"]).to.exist; + expect(want.endpoints["onMutationExecuted"].region).to.deep.equal(["europe-west1"]); }); it("resolves region for DataConnect event triggers based on connector location", async () => { - const wantE: backend.Endpoint = { - ...ENDPOINT_BASE, - id: "onMutationExecutedConnector", - region: build.REGION_TBD, - eventTrigger: { - eventType: "google.firebase.dataconnect.connector.v1.mutationExecuted", - eventFilters: { - connector: - "projects/project/locations/europe-west2/services/my-service/connectors/my-connector", + const want = build.of({ + onMutationExecutedConnector: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + eventTrigger: { + eventType: "google.firebase.dataconnect.connector.v1.mutationExecuted", + eventFilters: { + connector: + "projects/project/locations/europe-west2/services/my-service/connectors/my-connector", + }, + retry: false, }, - retry: false, + region: [build.REGION_TBD], }, - }; - const want = backend.of(wantE); + }); const have = backend.empty(); - await prepare.resolveDefaultRegions(want, have); + await prepare.resolveDefaultRegionsForBuild(want, have); - expect(want.endpoints["europe-west2"]?.["onMutationExecutedConnector"]).to.exist; + expect(want.endpoints["onMutationExecutedConnector"].region).to.deep.equal(["europe-west2"]); }); it("does not infer region from have backend if it belongs to a different codebase", async () => { - const wantE = { ...ENDPOINT, region: build.REGION_TBD, codebase: "codebaseA" }; - const want = backend.of(wantE); + const want = build.of({ + id: { + platform: "gcfv2", + entryPoint: "entry", + project: "project", + runtime: latest("nodejs"), + httpsTrigger: {}, + region: [build.REGION_TBD], + }, + }); - // An existing endpoint with the same ID but in codebaseB - const haveE = { ...ENDPOINT, region: "europe-west1", codebase: "codebaseB" }; + const haveE = { ...ENDPOINT, id: "id", region: "europe-west1", codebase: "codebaseB" }; const have = backend.of(haveE); - // Mimic the Phase 4 change: Filter down to relevant endpoints const relevantEndpoints = backend .allEndpoints(have) - .filter((e) => e.codebase === wantE.codebase || e.codebase === undefined); + .filter((e) => e.codebase === "codebaseA" || e.codebase === undefined); - // Since codebaseB's existing function is filtered out, it won't match. - // It should instead correctly fall back to "us-central1". - await prepare.resolveDefaultRegions(want, backend.of(...relevantEndpoints)); + await prepare.resolveDefaultRegionsForBuild(want, backend.of(...relevantEndpoints)); - expect(want.endpoints["us-central1"]?.["id"]).to.exist; - expect(want.endpoints["us-central1"]?.["id"].region).to.equal("us-central1"); - expect(want.endpoints[build.REGION_TBD]).to.not.exist; + expect(want.endpoints["id"].region).to.deep.equal(["us-central1"]); }); }); diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index ad21e308c77..05675ac25ec 100644 --- a/src/deploy/functions/prepare.ts +++ b/src/deploy/functions/prepare.ts @@ -124,6 +124,8 @@ export async function prepare( context.filters, ); + const existingBackend = await backend.existingBackend(context); + // == Phase 1.5 Prepare extensions found in codebases if any if (Object.values(wantBuilds).some((b) => b.extensions)) { const extContext: ExtContext = {}; @@ -149,6 +151,11 @@ export async function prepare( const userEnvs = functionsEnv.loadUserEnvs(userEnvOpt); const envs = { ...userEnvs, ...firebaseEnvs }; + const relevantEndpoints = backend + .allEndpoints(existingBackend) + .filter((e) => e.codebase === codebase || e.codebase === undefined); + await resolveDefaultRegionsForBuild(wantBuild, backend.of(...relevantEndpoints)); + const { backend: wantBackend, envs: resolvedEnvs } = await build.resolveBackend({ build: wantBuild, firebaseConfig, @@ -282,13 +289,7 @@ export async function prepare( // Resolve default regions for backends we want before grouping endpoints by codebase. // This way, endpoints aren't incorrectly grouped together under the REGION_TBD region if the // region is unresolved for multiple codebases. - const existingBackend = await backend.existingBackend(context); - for (const [codebase, wantBackend] of Object.entries(wantBackends)) { - const relevantEndpoints = backend - .allEndpoints(existingBackend) - .filter((e) => e.codebase === codebase || e.codebase === undefined); - await resolveDefaultRegions(wantBackend, backend.of(...relevantEndpoints)); - } + const haveBackends = groupEndpointsByCodebase( wantBackends, backend.allEndpoints(existingBackend), @@ -349,83 +350,52 @@ export async function prepare( applyBackendHashToBackends(wantBackends, context); } -function moveEndpointToRegion( - backend: backend.Backend, - endpoint: backend.Endpoint, - region: string, -) { - endpoint.region = region; - backend.endpoints[region] = backend.endpoints[region] || {}; - backend.endpoints[region][endpoint.id] = endpoint; - delete backend.endpoints[build.REGION_TBD][endpoint.id]; - if (Object.keys(backend.endpoints[build.REGION_TBD]).length === 0) { - delete backend.endpoints[build.REGION_TBD]; - } -} - /** - * Verifies that we don't have a peculiar edge case where we cannot know what region a default endpoint was in. - * This is only possible in insane edge cases (esp since you can only have multi-region functions for HTTPS and - * regional AI Logic functions) where a customer HAD specified multiple regions in a function and then deleted - * the regions annotation entirely and we don't know which to delete and which to keep. + * Resolves default regions for endpoints in a Build before it is converted to a Backend. + * This allows the VPC connector string to be built with the correct region in build.ts. */ -export function matchRegionsForExisting(want: backend.Backend, have: backend.Backend): void { - for (const [id, wantE] of Object.entries(want.endpoints[build.REGION_TBD] || {})) { - let matching: backend.Endpoint | undefined; - for (const region of Object.keys(have.endpoints)) { - if (region === build.REGION_TBD) { - continue; +export async function resolveDefaultRegionsForBuild( + buildObj: build.Build, + have: backend.Backend, +): Promise { + for (const [id, endpoint] of Object.entries(buildObj.endpoints)) { + if (!endpoint.region?.length || endpoint.region.includes(build.REGION_TBD)) { + let resolvedRegion = DEFAULT_FUNCTION_REGION; + + // Match existing endpoints. + let matching: backend.Endpoint | undefined; + for (const region of Object.keys(have.endpoints)) { + if (have.endpoints[region][id]) { + if (matching) { + throw new FirebaseError( + `Cannot resolve default region for function ${id}. It exists in multiple regions. The region must be specified to continue.`, + ); + } + matching = have.endpoints[region][id]; + } } - if (have.endpoints[region][id]) { - if (matching) { - throw new FirebaseError( - `Cannot resolve default region for function ${id}. It exists in multiple regions. The region must be specified to continue.`, + + if (matching) { + resolvedRegion = matching.region; + } else { + // Match triggers. + try { + const fullEndpoint = { ...endpoint, id } as any; + if (build.isBlockingTriggered(endpoint)) { + resolvedRegion = resolveRegionForBlockingTrigger(fullEndpoint); + } else if (build.isEventTriggered(endpoint)) { + resolvedRegion = await resolveRegionForEventTrigger(fullEndpoint); + } + } catch (err: any) { + logger.debug( + `Failed to resolve region for endpoint ${id}. Defaulting to ${DEFAULT_FUNCTION_REGION}.`, + getErrStack(err), ); } - matching = have.endpoints[region][id]; } - } - if (!matching) { - continue; + endpoint.region = [resolvedRegion]; } - - moveEndpointToRegion(want, wantE, matching.region); - } -} - -/** - * Resolves regions for endpoints that were not specified in the build. - * This is an improvement from old logic where everything was hard-coded to us-central1. Now, - * we can move defaults to adjust for regional capacity or automaically match the function - * to its event source allowing region to be specified less often. - */ -// N.B. This is async because it will eventually look up backend info -export async function resolveDefaultRegions( - want: backend.Backend, - have: backend.Backend, -): Promise { - matchRegionsForExisting(want, have); - - const endpoints = Object.values(want.endpoints[build.REGION_TBD] || {}); - - for (const endpoint of endpoints) { - let resolvedRegion = "us-central1"; - - try { - if (backend.isBlockingTriggered(endpoint)) { - resolvedRegion = resolveRegionForBlockingTrigger(endpoint); - } else if (backend.isEventTriggered(endpoint)) { - resolvedRegion = await resolveRegionForEventTrigger(endpoint); - } - } catch (err: any) { - logger.debug( - `Failed to resolve region for endpoint ${endpoint.id}. Defaulting to us-central1.`, - getErrStack(err), - ); - } - - moveEndpointToRegion(want, endpoint, resolvedRegion); } }