From 8b77ef660c082601b322c4bc7fb8614c4426049a Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Thu, 23 Apr 2026 16:09:21 +0100 Subject: [PATCH 1/5] fix(iam): add mergeInvokerBinding + preserve conditional bindings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces `mergeInvokerBinding` in iam.ts — a focused helper that rebuilds the bindings array around a single unconditional binding for the given role, optionally unioning its members with the declared set. `setInvokerUpdate` in both run.ts and cloudfunctions.ts now accepts an options arg `{mergeExistingMembers?: boolean}`. When true, externally added members on the existing unconditional invoker binding are preserved — the building block needed to honor `preserveExternalChanges` at the fabricator layer (wired up in a later commit). As a side fix, conditional bindings on the invoker role (those with a `condition` field) are now preserved across updates. Previously the filter `.filter(b => b.role !== invokerRole)` dropped every binding with that role, silently losing any out-of-band conditional access policy. Both `run.ts` and `cloudfunctions.ts` had the same bug. Default behavior (options absent, or `mergeExistingMembers: false`) is unchanged — the replace semantics that existed before still apply. --- src/gcp/cloudfunctions.spec.ts | 102 +++++++++++++++++++++++++ src/gcp/cloudfunctions.ts | 41 +++++++--- src/gcp/iam.spec.ts | 81 ++++++++++++++++++++ src/gcp/iam.ts | 36 +++++++++ src/gcp/run.spec.ts | 136 +++++++++++++++++++++++++++++++-- src/gcp/run.ts | 40 +++++++--- 6 files changed, 407 insertions(+), 29 deletions(-) diff --git a/src/gcp/cloudfunctions.spec.ts b/src/gcp/cloudfunctions.spec.ts index 2e202aa99ae..0ed9f490ff3 100644 --- a/src/gcp/cloudfunctions.spec.ts +++ b/src/gcp/cloudfunctions.spec.ts @@ -756,5 +756,107 @@ describe("cloudfunctions", () => { ]), ).to.not.be.rejected; }); + + it("merges existing invoker members when mergeExistingMembers is true", async () => { + nock(functionsOrigin()) + .get("/v1/function:getIamPolicy") + .reply(200, { + bindings: [ + { role: "roles/cloudfunctions.invoker", members: ["user:kept@example.com"] }, + { role: "random-role", members: ["user:other"] }, + ], + etag: "abc", + version: 3, + }); + let posted: any; + nock(functionsOrigin()) + .post("/v1/function:setIamPolicy", (body) => { + posted = body; + return true; + }) + .reply(200, {}); + + await expect( + cloudfunctions.setInvokerUpdate("project", "function", ["public"], { + mergeExistingMembers: true, + }), + ).to.not.be.rejected; + + const invoker = posted.policy.bindings.find( + (b: any) => b.role === "roles/cloudfunctions.invoker" && !b.condition, + ); + expect(invoker.members.sort()).to.deep.equal( + ["allUsers", "user:kept@example.com"].sort(), + ); + expect(posted.policy.bindings).to.deep.include({ + role: "random-role", + members: ["user:other"], + }); + }); + + it("replaces members when mergeExistingMembers is false", async () => { + nock(functionsOrigin()) + .get("/v1/function:getIamPolicy") + .reply(200, { + bindings: [ + { role: "roles/cloudfunctions.invoker", members: ["user:stale@example.com"] }, + ], + etag: "abc", + version: 3, + }); + let posted: any; + nock(functionsOrigin()) + .post("/v1/function:setIamPolicy", (body) => { + posted = body; + return true; + }) + .reply(200, {}); + + await expect( + cloudfunctions.setInvokerUpdate("project", "function", ["public"], { + mergeExistingMembers: false, + }), + ).to.not.be.rejected; + + const invoker = posted.policy.bindings.find( + (b: any) => b.role === "roles/cloudfunctions.invoker" && !b.condition, + ); + expect(invoker.members).to.deep.equal(["allUsers"]); + }); + + it("preserves conditional invoker bindings when updating", async () => { + const conditional = { + role: "roles/cloudfunctions.invoker", + members: ["serviceAccount:gated@project.iam.gserviceaccount.com"], + condition: { expression: "request.time < timestamp('2030-01-01T00:00:00Z')" }, + }; + nock(functionsOrigin()) + .get("/v1/function:getIamPolicy") + .reply(200, { + bindings: [ + conditional, + { role: "roles/cloudfunctions.invoker", members: ["user:old@example.com"] }, + ], + etag: "abc", + version: 3, + }); + let posted: any; + nock(functionsOrigin()) + .post("/v1/function:setIamPolicy", (body) => { + posted = body; + return true; + }) + .reply(200, {}); + + await expect( + cloudfunctions.setInvokerUpdate("project", "function", ["public"]), + ).to.not.be.rejected; + + expect(posted.policy.bindings).to.deep.include(conditional); + const unconditional = posted.policy.bindings.find( + (b: any) => b.role === "roles/cloudfunctions.invoker" && !b.condition, + ); + expect(unconditional.members).to.deep.equal(["allUsers"]); + }); }); }); diff --git a/src/gcp/cloudfunctions.ts b/src/gcp/cloudfunctions.ts index ff766ef5189..c7598bfab79 100644 --- a/src/gcp/cloudfunctions.ts +++ b/src/gcp/cloudfunctions.ts @@ -336,8 +336,20 @@ export async function setInvokerCreate( } /** - * Gets the current IAM policy on function update, - * overrides the current invoker role with the supplied invoker members + * Options for {@link setInvokerUpdate}. + * `mergeExistingMembers`: when true, preserve any externally-added members on + * the existing unconditional invoker binding instead of replacing them. + */ +export interface SetInvokerUpdateOptions { + mergeExistingMembers?: boolean; +} + +/** + * Gets the current IAM policy on function update and updates the invoker role + * with the supplied invoker members. When `options.mergeExistingMembers` is + * true the new members are unioned with any existing unconditional members on + * the invoker binding (used to honor `preserveExternalChanges`). Conditional + * bindings on the same role are always preserved untouched. * @param projectId id of the project * @param fnName function name * @param invoker an array of invoker strings @@ -347,6 +359,7 @@ export async function setInvokerUpdate( projectId: string, fnName: string, invoker: string[], + options?: SetInvokerUpdateOptions, ): Promise { if (invoker.length === 0) { throw new FirebaseError("Invoker cannot be an empty array"); @@ -354,21 +367,27 @@ export async function setInvokerUpdate( const invokerMembers = proto.getInvokerMembers(invoker, projectId); const invokerRole = "roles/cloudfunctions.invoker"; const currentPolicy = await getIamPolicy(fnName); - const currentInvokerBinding = currentPolicy.bindings?.find( - (binding) => binding.role === invokerRole, + const currentUnconditional = currentPolicy.bindings?.find( + (binding) => binding.role === invokerRole && !binding.condition, ); + const desiredMembers = + options?.mergeExistingMembers && currentUnconditional + ? Array.from(new Set([...currentUnconditional.members, ...invokerMembers])) + : invokerMembers; if ( - currentInvokerBinding && - JSON.stringify(currentInvokerBinding.members.sort()) === JSON.stringify(invokerMembers.sort()) + currentUnconditional && + JSON.stringify([...currentUnconditional.members].sort()) === + JSON.stringify([...desiredMembers].sort()) ) { return; } - const bindings = (currentPolicy.bindings || []).filter((binding) => binding.role !== invokerRole); - bindings.push({ - role: invokerRole, - members: invokerMembers, - }); + const bindings = iam.mergeInvokerBinding( + currentPolicy.bindings || [], + invokerRole, + invokerMembers, + { merge: !!options?.mergeExistingMembers }, + ); const policy: iam.Policy = { bindings: bindings, diff --git a/src/gcp/iam.spec.ts b/src/gcp/iam.spec.ts index 6539d5b588e..8f2d3b65faf 100644 --- a/src/gcp/iam.spec.ts +++ b/src/gcp/iam.spec.ts @@ -56,6 +56,87 @@ describe("iam", () => { }); }); + describe("mergeInvokerBinding", () => { + const role = "roles/run.invoker"; + + it("appends an unconditional invoker binding when none exists", () => { + const bindings: iam.Binding[] = [{ role: "other", members: ["user:foo"] }]; + const result = iam.mergeInvokerBinding(bindings, role, ["allUsers"], { + merge: true, + }); + expect(result).to.deep.equal([ + { role: "other", members: ["user:foo"] }, + { role, members: ["allUsers"] }, + ]); + }); + + it("unions existing unconditional members when merge is true", () => { + const bindings: iam.Binding[] = [ + { role, members: ["user:kept@example.com"] }, + { role: "other", members: ["user:foo"] }, + ]; + const result = iam.mergeInvokerBinding(bindings, role, ["allUsers"], { + merge: true, + }); + const invoker = result.find((b) => b.role === role && !b.condition); + expect(invoker!.members.sort()).to.deep.equal( + ["allUsers", "user:kept@example.com"].sort(), + ); + }); + + it("replaces unconditional members when merge is false", () => { + const bindings: iam.Binding[] = [ + { role, members: ["user:stale@example.com"] }, + ]; + const result = iam.mergeInvokerBinding(bindings, role, ["allUsers"], { + merge: false, + }); + const invoker = result.find((b) => b.role === role && !b.condition); + expect(invoker!.members).to.deep.equal(["allUsers"]); + }); + + it("leaves conditional bindings on the same role untouched in both modes", () => { + const conditional: iam.Binding = { + role, + members: ["serviceAccount:gated@project.iam.gserviceaccount.com"], + condition: { expression: "resource.name.startsWith('foo')" }, + }; + const bindings: iam.Binding[] = [ + conditional, + { role, members: ["user:old@example.com"] }, + ]; + + const merged = iam.mergeInvokerBinding(bindings, role, ["allUsers"], { + merge: true, + }); + expect(merged).to.deep.include(conditional); + const mergedInvoker = merged.find((b) => b.role === role && !b.condition); + expect(mergedInvoker!.members.sort()).to.deep.equal( + ["allUsers", "user:old@example.com"].sort(), + ); + + const replaced = iam.mergeInvokerBinding(bindings, role, ["allUsers"], { + merge: false, + }); + expect(replaced).to.deep.include(conditional); + const replacedInvoker = replaced.find((b) => b.role === role && !b.condition); + expect(replacedInvoker!.members).to.deep.equal(["allUsers"]); + }); + + it("deduplicates when declared members already exist", () => { + const bindings: iam.Binding[] = [ + { role, members: ["allUsers", "user:other@example.com"] }, + ]; + const result = iam.mergeInvokerBinding(bindings, role, ["allUsers"], { + merge: true, + }); + const invoker = result.find((b) => b.role === role && !b.condition); + expect(invoker!.members.sort()).to.deep.equal( + ["allUsers", "user:other@example.com"].sort(), + ); + }); + }); + describe("testIamPermissions", () => { const tests: { desc: string; diff --git a/src/gcp/iam.ts b/src/gcp/iam.ts index 7a51af5bde3..26c49f19ecd 100644 --- a/src/gcp/iam.ts +++ b/src/gcp/iam.ts @@ -246,6 +246,42 @@ export function mergeBindings(policy: Policy, requiredBindings: Binding[]): bool return updated; } +/** + * Reconstructs `bindings` with a single unconditional binding for `role` + * whose members come from the declared `members` list. When `options.merge` + * is true, the resulting members are the union of the declared members and + * any members already present on an existing unconditional binding for that + * role — this is how we preserve externally-added invokers when a user has + * opted into `preserveExternalChanges`. + * + * Conditional bindings (those with a `condition` field) for the same role + * are left untouched in either mode — Firebase never writes conditional + * invoker bindings, so if one is present it was added out-of-band and must + * be preserved. + */ +export function mergeInvokerBinding( + bindings: Binding[], + role: string, + members: string[], + options: { merge: boolean }, +): Binding[] { + const result: Binding[] = []; + let existingUnconditional: Binding | undefined; + for (const b of bindings) { + if (b.role === role && !b.condition) { + existingUnconditional = b; + continue; + } + result.push(b); + } + const finalMembers = + options.merge && existingUnconditional + ? Array.from(new Set([...existingUnconditional.members, ...members])) + : [...members]; + result.push({ role, members: finalMembers }); + return result; +} + /** Utility to print the required binding commands */ export function printManualIamConfig( requiredBindings: Binding[], diff --git a/src/gcp/run.spec.ts b/src/gcp/run.spec.ts index 3661401fcdc..a9b990beb8a 100644 --- a/src/gcp/run.spec.ts +++ b/src/gcp/run.spec.ts @@ -136,7 +136,7 @@ describe("run", () => { apiGetStub.onFirstCall().throws("Error calling get api."); await expect( - run.setInvokerUpdate("project", "service", ["public"], client), + run.setInvokerUpdate("project", "service", ["public"], undefined, client), ).to.be.rejectedWith("Failed to get the IAM Policy on the Service service"); expect(apiGetStub).to.be.called; @@ -147,7 +147,7 @@ describe("run", () => { apiPostStub.throws("Error calling set api."); await expect( - run.setInvokerUpdate("project", "service", ["public"], client), + run.setInvokerUpdate("project", "service", ["public"], undefined, client), ).to.be.rejectedWith("Failed to set the IAM Policy on the Service service"); expect(apiGetStub).to.be.calledOnce; expect(apiPostStub).to.be.calledOnce; @@ -170,8 +170,9 @@ describe("run", () => { return Promise.resolve(); }); - await expect(run.setInvokerUpdate("project", "service", ["public"], client)).to.not.be - .rejected; + await expect( + run.setInvokerUpdate("project", "service", ["public"], undefined, client), + ).to.not.be.rejected; expect(apiGetStub).to.be.calledOnce; expect(apiPostStub).to.be.calledOnce; }); @@ -200,8 +201,9 @@ describe("run", () => { return Promise.resolve(); }); - await expect(run.setInvokerUpdate("project", "service", ["private"], client)).to.not.be - .rejected; + await expect( + run.setInvokerUpdate("project", "service", ["private"], undefined, client), + ).to.not.be.rejected; expect(apiGetStub).to.be.calledOnce; expect(apiPostStub).to.be.calledOnce; }); @@ -209,8 +211,11 @@ describe("run", () => { it("should set the policy with a set of invokers with active policies", async () => { apiGetStub.onFirstCall().resolves({ body: {} }); apiPostStub.onFirstCall().callsFake((path: string, json: any) => { - json.policy.bindings[0].members.sort(); - expect(json.policy.bindings[0].members).to.deep.eq([ + const invoker = json.policy.bindings.find( + (b: any) => b.role === "roles/run.invoker" && !b.condition, + ); + invoker.members.sort(); + expect(invoker.members).to.deep.eq([ "serviceAccount:service-account1@project.iam.gserviceaccount.com", "serviceAccount:service-account2@project.iam.gserviceaccount.com", "serviceAccount:service-account3@project.iam.gserviceaccount.com", @@ -228,6 +233,7 @@ describe("run", () => { "service-account2@project.iam.gserviceaccount.com", "service-account3@", ], + undefined, client, ), ).to.not.be.rejected; @@ -262,12 +268,126 @@ describe("run", () => { "service-account3@", "service-account1@", ], + undefined, client, ), ).to.not.be.rejected; expect(apiGetStub).to.be.calledOnce; expect(apiPostStub).to.not.be.called; }); + + it("merges existing invoker members when mergeExistingMembers is true", async () => { + apiGetStub.onFirstCall().resolves({ + body: { + bindings: [ + { + role: "roles/run.invoker", + members: ["user:kept@example.com"], + }, + { role: "random-role", members: ["user:other"] }, + ], + etag: "abc", + version: 3, + }, + }); + apiPostStub.onFirstCall().callsFake((path: string, json: any) => { + const invoker = json.policy.bindings.find( + (b: any) => b.role === "roles/run.invoker" && !b.condition, + ); + expect(invoker.members.sort()).to.deep.equal( + ["allUsers", "user:kept@example.com"].sort(), + ); + expect(json.policy.bindings).to.deep.include({ + role: "random-role", + members: ["user:other"], + }); + expect(json.policy.etag).to.equal("abc"); + return Promise.resolve(); + }); + + await expect( + run.setInvokerUpdate( + "project", + "service", + ["public"], + { mergeExistingMembers: true }, + client, + ), + ).to.not.be.rejected; + expect(apiGetStub).to.be.calledOnce; + expect(apiPostStub).to.be.calledOnce; + }); + + it("replaces members (no merge) when mergeExistingMembers is false", async () => { + apiGetStub.onFirstCall().resolves({ + body: { + bindings: [ + { + role: "roles/run.invoker", + members: ["user:stale@example.com"], + }, + ], + etag: "abc", + version: 3, + }, + }); + apiPostStub.onFirstCall().callsFake((path: string, json: any) => { + const invoker = json.policy.bindings.find( + (b: any) => b.role === "roles/run.invoker" && !b.condition, + ); + expect(invoker.members).to.deep.equal(["allUsers"]); + return Promise.resolve(); + }); + + await expect( + run.setInvokerUpdate( + "project", + "service", + ["public"], + { mergeExistingMembers: false }, + client, + ), + ).to.not.be.rejected; + expect(apiGetStub).to.be.calledOnce; + expect(apiPostStub).to.be.calledOnce; + }); + + it("preserves conditional invoker bindings when updating", async () => { + const conditional = { + role: "roles/run.invoker", + members: ["serviceAccount:gated@project.iam.gserviceaccount.com"], + condition: { expression: "request.time < timestamp('2030-01-01T00:00:00Z')" }, + }; + apiGetStub.onFirstCall().resolves({ + body: { + bindings: [ + conditional, + { + role: "roles/run.invoker", + members: ["user:old@example.com"], + }, + ], + etag: "abc", + version: 3, + }, + }); + apiPostStub.onFirstCall().callsFake((path: string, json: any) => { + // conditional binding survives + expect(json.policy.bindings).to.deep.include(conditional); + // unconditional binding was replaced (default behavior) + const unconditional = json.policy.bindings.find( + (b: any) => b.role === "roles/run.invoker" && !b.condition, + ); + expect(unconditional.members).to.deep.equal(["allUsers"]); + return Promise.resolve(); + }); + + await expect( + run.setInvokerUpdate("project", "service", ["public"], undefined, client), + ).to.not.be.rejected; + expect(apiGetStub).to.be.calledOnce; + expect(apiPostStub).to.be.calledOnce; + }); }); }); describe("updateService", () => { diff --git a/src/gcp/run.ts b/src/gcp/run.ts index 837e436f946..6c22d6c8112 100644 --- a/src/gcp/run.ts +++ b/src/gcp/run.ts @@ -304,7 +304,20 @@ export async function setInvokerCreate( } /** - * Gets the current IAM policy for the run service and overrides the invoker role with the supplied invoker members + * Options for {@link setInvokerUpdate}. + * `mergeExistingMembers`: when true, preserve any externally-added members on + * the existing unconditional invoker binding instead of replacing them. + */ +export interface SetInvokerUpdateOptions { + mergeExistingMembers?: boolean; +} + +/** + * Gets the current IAM policy for the run service and updates the invoker role + * with the supplied invoker members. When `options.mergeExistingMembers` is + * true the new members are unioned with any existing unconditional members on + * the invoker binding (used to honor `preserveExternalChanges`). Conditional + * bindings on the same role are always preserved untouched. * @param projectId id of the project * @param serviceName cloud run service * @param invoker an array of invoker strings @@ -314,6 +327,7 @@ export async function setInvokerUpdate( projectId: string, serviceName: string, invoker: string[], + options?: SetInvokerUpdateOptions, httpClient: Client = client, // for unit testing ) { if (invoker.length === 0) { @@ -322,21 +336,27 @@ export async function setInvokerUpdate( const invokerMembers = proto.getInvokerMembers(invoker, projectId); const invokerRole = "roles/run.invoker"; const currentPolicy = await getIamPolicy(serviceName, httpClient); - const currentInvokerBinding = currentPolicy.bindings?.find( - (binding) => binding.role === invokerRole, + const currentUnconditional = currentPolicy.bindings?.find( + (binding) => binding.role === invokerRole && !binding.condition, ); + const desiredMembers = + options?.mergeExistingMembers && currentUnconditional + ? Array.from(new Set([...currentUnconditional.members, ...invokerMembers])) + : invokerMembers; if ( - currentInvokerBinding && - JSON.stringify(currentInvokerBinding.members.sort()) === JSON.stringify(invokerMembers.sort()) + currentUnconditional && + JSON.stringify([...currentUnconditional.members].sort()) === + JSON.stringify([...desiredMembers].sort()) ) { return; } - const bindings = (currentPolicy.bindings || []).filter((binding) => binding.role !== invokerRole); - bindings.push({ - role: invokerRole, - members: invokerMembers, - }); + const bindings = iam.mergeInvokerBinding( + currentPolicy.bindings || [], + invokerRole, + invokerMembers, + { merge: !!options?.mergeExistingMembers }, + ); const policy: iam.Policy = { bindings: bindings, From aca03f79aac54f2eebaea56ab4e4d983b3923c18 Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Thu, 23 Apr 2026 16:11:05 +0100 Subject: [PATCH 2/5] feat(runtimes): carry preserveExternalChanges through manifest decode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `preserveExternalChanges?: Field` to the v1alpha1 manifest wire format, the build endpoint type, and the manifest validator. Mirrors the existing `omit` plumbing (same Field semantics, same copyIfPresent pattern). Without this, the flag survives in the user's SDK config and in firebase-config.json but gets dropped at decode time — which is why it silently does nothing today for IAM invoker preservation. No behavior change yet: the flag is carried through decode, but nothing reads it. Wiring it into the backend endpoint and the fabricator follows. --- src/deploy/functions/build.ts | 5 ++ .../runtimes/discovery/v1alpha1.spec.ts | 58 +++++++++++++++++++ .../functions/runtimes/discovery/v1alpha1.ts | 3 + 3 files changed, 66 insertions(+) diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index df2d3950d6b..67c2c2d6576 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -246,6 +246,11 @@ export type Endpoint = Triggered & { // Defaults to false. If true, the function will be ignored during the deploy process. omit?: Field; + // Defaults to false. If true, deploys honor externally-modified Cloud Run / Cloud + // Functions IAM invoker bindings (and potentially other preserve-aware knobs) instead + // of overwriting them with the Firebase-managed default on every deploy. + preserveExternalChanges?: Field; + // Defaults to "gcfv2". platform?: "gcfv1" | "gcfv2" | "run"; diff --git a/src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts b/src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts index e401d79c534..94336860bf6 100644 --- a/src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts +++ b/src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts @@ -1079,6 +1079,64 @@ describe("buildFromV1Alpha", () => { expect(parsed).to.deep.equal(build.of({ id: expectedBuild })); }); + it("copies preserveExternalChanges literal into the build endpoint", () => { + const yaml: v1alpha1.WireManifest = { + specVersion: "v1alpha1", + endpoints: { + id: { + ...MIN_WIRE_ENDPOINT, + httpsTrigger: {}, + preserveExternalChanges: true, + }, + }, + }; + + const parsed = v1alpha1.buildFromV1Alpha1(yaml, PROJECT, REGION, RUNTIME); + const expected: build.Endpoint = { + ...DEFAULTED_ENDPOINT, + httpsTrigger: {}, + preserveExternalChanges: true, + }; + expect(parsed).to.deep.equal(build.of({ id: expected })); + }); + + it("copies preserveExternalChanges as a CEL Field into the build endpoint", () => { + const yaml: v1alpha1.WireManifest = { + specVersion: "v1alpha1", + endpoints: { + id: { + ...MIN_WIRE_ENDPOINT, + httpsTrigger: {}, + preserveExternalChanges: "{{ params.PRESERVE }}", + }, + }, + }; + + const parsed = v1alpha1.buildFromV1Alpha1(yaml, PROJECT, REGION, RUNTIME); + const expected: build.Endpoint = { + ...DEFAULTED_ENDPOINT, + httpsTrigger: {}, + preserveExternalChanges: "{{ params.PRESERVE }}", + }; + expect(parsed).to.deep.equal(build.of({ id: expected })); + }); + + it("leaves preserveExternalChanges undefined when absent from the manifest", () => { + const yaml: v1alpha1.WireManifest = { + specVersion: "v1alpha1", + endpoints: { + id: { + ...MIN_WIRE_ENDPOINT, + httpsTrigger: {}, + }, + }, + }; + + const parsed = v1alpha1.buildFromV1Alpha1(yaml, PROJECT, REGION, RUNTIME); + const endpoint = parsed.endpoints.id; + expect(endpoint.preserveExternalChanges).to.equal(undefined); + }); + it("handles multiple regions", () => { const yaml: v1alpha1.WireManifest = { specVersion: "v1alpha1", diff --git a/src/deploy/functions/runtimes/discovery/v1alpha1.ts b/src/deploy/functions/runtimes/discovery/v1alpha1.ts index d84282e30d0..56a4df74931 100644 --- a/src/deploy/functions/runtimes/discovery/v1alpha1.ts +++ b/src/deploy/functions/runtimes/discovery/v1alpha1.ts @@ -47,6 +47,7 @@ export type WireEndpoint = build.Triggered & Partial & Partial<{ scheduleTrigger: WireScheduleTrigger }> & { omit?: build.Field; + preserveExternalChanges?: build.Field; labels?: Record | null; environmentVariables?: Record | null; availableMemoryMb?: build.MemoryOption | build.Expression | null; @@ -154,6 +155,7 @@ function assertBuildEndpoint(ep: WireEndpoint, id: string): void { platform: (platform) => build.AllFunctionsPlatforms.includes(platform), entryPoint: "string", omit: "Field?", + preserveExternalChanges: "Field?", availableMemoryMb: (mem) => mem === null || isCEL(mem) || backend.isValidMemoryOption(mem), maxInstances: "Field?", minInstances: "Field?", @@ -460,6 +462,7 @@ function parseEndpointForBuild( parsed, ep, "omit", + "preserveExternalChanges", "availableMemoryMb", "cpu", "maxInstances", From 265dcc591f306f883a7f8a3d1dacba180335a2e4 Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Thu, 23 Apr 2026 16:13:05 +0100 Subject: [PATCH 3/5] feat(deploy): resolve preserveExternalChanges into the backend endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `preserveExternalChanges?: boolean | null` to backend.ServiceConfiguration so the backend endpoint carries the user's intent into fabricator. `toBackend` resolves the build-layer Field via the existing Resolver pattern, swallowing resolution errors and defaulting to `false` — matching how `omit` is treated. Users who never set the flag keep `undefined`, so the existing replace-on-deploy behavior is unchanged by this commit. Still no behavior change at the IAM layer: the value is available to the fabricator but not yet read there. That's the next commit. --- src/deploy/functions/backend.ts | 1 + src/deploy/functions/build.spec.ts | 52 ++++++++++++++++++++++++++++++ src/deploy/functions/build.ts | 12 +++++++ 3 files changed, 65 insertions(+) diff --git a/src/deploy/functions/backend.ts b/src/deploy/functions/backend.ts index d32ad73e747..d8c89c5aa93 100644 --- a/src/deploy/functions/backend.ts +++ b/src/deploy/functions/backend.ts @@ -316,6 +316,7 @@ export interface ServiceConfiguration { } | null; ingressSettings?: IngressSettings | null; serviceAccount?: string | null; + preserveExternalChanges?: boolean | null; } export const AllFunctionsPlatforms = ["gcfv1", "gcfv2", "run"] as const; diff --git a/src/deploy/functions/build.spec.ts b/src/deploy/functions/build.spec.ts index 641daf1c47e..9a547c57246 100644 --- a/src/deploy/functions/build.spec.ts +++ b/src/deploy/functions/build.spec.ts @@ -259,6 +259,58 @@ describe("toBackend", () => { } }); + it("resolves literal preserveExternalChanges onto the backend endpoint", () => { + const desiredBuild: build.Build = build.of({ + func: { + platform: "gcfv2", + region: ["us-central1"], + project: "project", + runtime: "nodejs16", + entryPoint: "func", + preserveExternalChanges: true, + httpsTrigger: {}, + }, + }); + const backend = build.toBackend(desiredBuild, {}); + const endpointDef = Object.values(backend.endpoints)[0]; + expect(endpointDef!.func.preserveExternalChanges).to.equal(true); + }); + + it("resolves a CEL preserveExternalChanges via a param onto the backend endpoint", () => { + const desiredBuild: build.Build = build.of({ + func: { + platform: "gcfv2", + region: ["us-central1"], + project: "project", + runtime: "nodejs16", + entryPoint: "func", + preserveExternalChanges: "{{ params.PRESERVE }}", + httpsTrigger: {}, + }, + }); + const backend = build.toBackend(desiredBuild, { + PRESERVE: new ParamValue("false", false, { boolean: true }), + }); + const endpointDef = Object.values(backend.endpoints)[0]; + expect(endpointDef!.func.preserveExternalChanges).to.equal(false); + }); + + it("leaves preserveExternalChanges undefined on the backend endpoint when absent", () => { + const desiredBuild: build.Build = build.of({ + func: { + platform: "gcfv2", + region: ["us-central1"], + project: "project", + runtime: "nodejs16", + entryPoint: "func", + httpsTrigger: {}, + }, + }); + const backend = build.toBackend(desiredBuild, {}); + const endpointDef = Object.values(backend.endpoints)[0]; + expect(endpointDef!.func.preserveExternalChanges).to.equal(undefined); + }); + it("enforces enum correctness for VPC egress settings", () => { const desiredBuild: build.Build = build.of({ func: { diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index 67c2c2d6576..58b745c3207 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -556,6 +556,18 @@ export function toBackend( "cpu", nullsafeVisitor((cpu) => (cpu === "gcf_gen1" ? cpu : r.resolveInt(cpu))), ); + proto.convertIfPresent( + bkEndpoint, + bdEndpoint, + "preserveExternalChanges", + nullsafeVisitor((v) => { + try { + return r.resolveBoolean(v); + } catch { + return false; + } + }), + ); if (bdEndpoint.vpc) { bkEndpoint.vpc = {}; if (typeof bdEndpoint.vpc.connector !== "undefined" && bdEndpoint.vpc.connector !== null) { From 55dfcaa57512ec752178b7012ebe9e53df904cf6 Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Thu, 23 Apr 2026 16:19:49 +0100 Subject: [PATCH 4/5] feat(fabricator): honor preserveExternalChanges at every invoker call site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every `setInvoker*` site now branches on `endpoint.preserveExternalChanges`: - When true: route through `setInvokerUpdate` with `{ mergeExistingMembers: true }`, so externally-added invoker members are preserved across deploys. - When false/absent: keep today's replace semantics so clean-up via redeploy still works for users who haven't opted in. Covers all 13 call sites across v1 + v2 × create + update × all trigger types (HTTPS, callable, task queue, blocking, scheduled, dataConnect, run-native httpsTrigger). The v1 create path introduces a small `setV1Invoker` local helper and v2 introduces `setV2Invoker` to avoid repeating the ternary seven times per function. Fixes firebase/firebase-tools#6549. Subsumes firebase/firebase-tools#10411 — that PR made merge unconditional for scheduled v2, which would have regressed the documented `preserveExternalChanges: false` default for every other user. This commit gates the behavior on the user's declared intent and covers every trigger type, not just scheduled. --- .../functions/release/fabricator.spec.ts | 245 ++++++++++++++++++ src/deploy/functions/release/fabricator.ts | 45 +++- 2 files changed, 278 insertions(+), 12 deletions(-) diff --git a/src/deploy/functions/release/fabricator.spec.ts b/src/deploy/functions/release/fabricator.spec.ts index 1608c7c0608..afbdf425b7b 100644 --- a/src/deploy/functions/release/fabricator.spec.ts +++ b/src/deploy/functions/release/fabricator.spec.ts @@ -1862,4 +1862,249 @@ describe("Fabricator", () => { expect(runv2.deleteService).to.have.been.called; }); }); + + describe("preserveExternalChanges wiring", () => { + // When preserveExternalChanges is true on the endpoint, invoker writes + // must route through setInvokerUpdate with {mergeExistingMembers: true} + // so externally-added invoker members survive redeploys. When false or + // absent, the original setInvokerCreate (replace) path is preserved — + // so users who never opted in keep today's cleanup semantics. + + describe("createV2Function", () => { + it("HTTPS: flag on routes to setInvokerUpdate with merge", async () => { + gcfv2.createFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerUpdate.resolves(); + const ep = endpoint( + { httpsTrigger: {} }, + { platform: "gcfv2", preserveExternalChanges: true }, + ); + + await fab.createV2Function(ep, new scraper.SourceTokenScraper()); + + expect(run.setInvokerCreate).to.not.have.been.called; + expect(run.setInvokerUpdate).to.have.been.calledWith( + ep.project, + "service", + ["public"], + { mergeExistingMembers: true }, + ); + }); + + it("HTTPS: flag off keeps setInvokerCreate", async () => { + gcfv2.createFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerCreate.resolves(); + const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" }); + + await fab.createV2Function(ep, new scraper.SourceTokenScraper()); + + expect(run.setInvokerUpdate).to.not.have.been.called; + expect(run.setInvokerCreate).to.have.been.calledWith(ep.project, "service", ["public"]); + }); + + it("callable: flag on routes to setInvokerUpdate with merge", async () => { + gcfv2.createFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerUpdate.resolves(); + const ep = endpoint( + { callableTrigger: {} }, + { platform: "gcfv2", preserveExternalChanges: true }, + ); + + await fab.createV2Function(ep, new scraper.SourceTokenScraper()); + + expect(run.setInvokerUpdate).to.have.been.calledWith( + ep.project, + "service", + ["public"], + { mergeExistingMembers: true }, + ); + }); + + it("task queue: flag on routes to setInvokerUpdate with merge", async () => { + gcfv2.createFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerUpdate.resolves(); + const ep = endpoint( + { taskQueueTrigger: { invoker: ["custom@"] } }, + { platform: "gcfv2", preserveExternalChanges: true }, + ); + + await fab.createV2Function(ep, new scraper.SourceTokenScraper()); + + expect(run.setInvokerUpdate).to.have.been.calledWith( + ep.project, + "service", + ["custom@"], + { mergeExistingMembers: true }, + ); + }); + + it("blocking (auth): flag on routes to setInvokerUpdate with merge", async () => { + gcfv2.createFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerUpdate.resolves(); + const ep = endpoint( + { + blockingTrigger: { + eventType: "providers/cloud.auth/eventTypes/user.beforeCreate", + }, + }, + { platform: "gcfv2", preserveExternalChanges: true }, + ); + + await fab.createV2Function(ep, new scraper.SourceTokenScraper()); + + expect(run.setInvokerUpdate).to.have.been.calledWith( + ep.project, + "service", + ["public"], + { mergeExistingMembers: true }, + ); + }); + + it("scheduled: flag on routes to setInvokerUpdate with merge", async () => { + gcfv2.createFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerUpdate.resolves(); + const ep = endpoint( + { scheduleTrigger: { schedule: "every 5 minutes" } }, + { platform: "gcfv2", serviceAccount: "sa@", preserveExternalChanges: true }, + ); + + await fab.createV2Function(ep, new scraper.SourceTokenScraper()); + + expect(run.setInvokerUpdate).to.have.been.calledWith( + ep.project, + "service", + ["sa@"], + { mergeExistingMembers: true }, + ); + }); + + it("dataConnect: flag on routes to setInvokerUpdate with merge", async () => { + gcfv2.createFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerUpdate.resolves(); + const ep = endpoint( + { dataConnectGraphqlTrigger: { invoker: ["custom@"] } }, + { platform: "gcfv2", preserveExternalChanges: true }, + ); + + await fab.createV2Function(ep, new scraper.SourceTokenScraper()); + + expect(run.setInvokerUpdate).to.have.been.called; + const args = run.setInvokerUpdate.firstCall.args; + expect(args[3]).to.deep.equal({ mergeExistingMembers: true }); + }); + }); + + describe("updateV2Function", () => { + // N.B. updateV2Function only calls setInvokerUpdate when the endpoint + // has an explicit invoker (null or a list). A bare `httpsTrigger: {}` + // leaves invoker undefined and skips the IAM update entirely. + it("passes mergeExistingMembers:true when flag is on", async () => { + gcfv2.updateFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerUpdate.resolves(); + const ep = endpoint( + { httpsTrigger: { invoker: ["custom@"] } }, + { platform: "gcfv2", preserveExternalChanges: true }, + ); + + await fab.updateV2Function(ep, new scraper.SourceTokenScraper()); + + expect(run.setInvokerUpdate).to.have.been.calledWith( + ep.project, + "service", + ["custom@"], + { mergeExistingMembers: true }, + ); + }); + + it("passes mergeExistingMembers:false when flag is off", async () => { + gcfv2.updateFunction.resolves({ name: "op", done: false }); + poller.pollOperation.resolves({ serviceConfig: { service: "service" } }); + run.setInvokerUpdate.resolves(); + const ep = endpoint( + { httpsTrigger: { invoker: ["custom@"] } }, + { platform: "gcfv2" }, + ); + + await fab.updateV2Function(ep, new scraper.SourceTokenScraper()); + + expect(run.setInvokerUpdate).to.have.been.calledWith( + ep.project, + "service", + ["custom@"], + { mergeExistingMembers: false }, + ); + }); + }); + + describe("createV1Function", () => { + it("HTTPS: flag on routes to gcf.setInvokerUpdate with merge", async () => { + gcf.createFunction.resolves({ name: "op", type: "create", done: false }); + poller.pollOperation.resolves({ httpsTrigger: { url: "url" } }); + gcf.setInvokerUpdate.resolves(); + const ep = endpoint({ httpsTrigger: {} }, { preserveExternalChanges: true }); + + await fab.createV1Function(ep, new scraper.SourceTokenScraper()); + + expect(gcf.setInvokerCreate).to.not.have.been.called; + expect(gcf.setInvokerUpdate).to.have.been.calledWith( + ep.project, + backend.functionName(ep), + ["public"], + { mergeExistingMembers: true }, + ); + }); + }); + + describe("updateV1Function", () => { + it("HTTPS: flag on passes mergeExistingMembers:true", async () => { + gcf.updateFunction.resolves({ name: "op", type: "update", done: false }); + poller.pollOperation.resolves({}); + gcf.setInvokerUpdate.resolves(); + const ep = endpoint( + { httpsTrigger: { invoker: ["custom@"] } }, + { preserveExternalChanges: true }, + ); + + await fab.updateV1Function(ep, new scraper.SourceTokenScraper()); + + expect(gcf.setInvokerUpdate).to.have.been.calledWith( + ep.project, + backend.functionName(ep), + ["custom@"], + { mergeExistingMembers: true }, + ); + }); + }); + + describe("run-native setInvoker (createRunFunction)", () => { + it("flag on routes to setInvokerUpdate with merge", async () => { + runv2.createService.resolves({ uri: "https://service", name: "service" } as any); + run.setInvokerUpdate.resolves(); + const ep = endpoint( + { httpsTrigger: {} }, + { + platform: "run", + baseImageUri: "gcr.io/base", + preserveExternalChanges: true, + }, + ); + + await fab.createRunFunction(ep); + + expect(run.setInvokerUpdate).to.have.been.calledWith( + ep.project, + `projects/${ep.project}/locations/${ep.region}/services/${ep.id}`, + ["public"], + { mergeExistingMembers: true }, + ); + }); + }); + }); }); diff --git a/src/deploy/functions/release/fabricator.ts b/src/deploy/functions/release/fabricator.ts index 33919b61ece..2c15148c68d 100644 --- a/src/deploy/functions/release/fabricator.ts +++ b/src/deploy/functions/release/fabricator.ts @@ -304,12 +304,18 @@ export class Fabricator { .catch(rethrowAs(endpoint, "create")); endpoint.uri = resultFunction?.httpsTrigger?.url; + const setV1Invoker = (members: string[]): Promise => + endpoint.preserveExternalChanges + ? gcf.setInvokerUpdate(endpoint.project, backend.functionName(endpoint), members, { + mergeExistingMembers: true, + }) + : gcf.setInvokerCreate(endpoint.project, backend.functionName(endpoint), members); if (backend.isHttpsTriggered(endpoint)) { const invoker = endpoint.httpsTrigger.invoker || ["public"]; if (!invoker.includes("private")) { await this.executor .run(async () => { - await gcf.setInvokerCreate(endpoint.project, backend.functionName(endpoint), invoker); + await setV1Invoker(invoker); }) .catch(rethrowAs(endpoint, "set invoker")); } @@ -317,7 +323,7 @@ export class Fabricator { // Callable functions should always be public await this.executor .run(async () => { - await gcf.setInvokerCreate(endpoint.project, backend.functionName(endpoint), ["public"]); + await setV1Invoker(["public"]); }) .catch(rethrowAs(endpoint, "set invoker")); } else if (backend.isTaskQueueTriggered(endpoint)) { @@ -327,7 +333,7 @@ export class Fabricator { if (invoker && !invoker.includes("private")) { await this.executor .run(async () => { - await gcf.setInvokerCreate(endpoint.project, backend.functionName(endpoint), invoker); + await setV1Invoker(invoker); }) .catch(rethrowAs(endpoint, "set invoker")); } @@ -338,7 +344,7 @@ export class Fabricator { // Auth Blocking functions should always be public await this.executor .run(async () => { - await gcf.setInvokerCreate(endpoint.project, backend.functionName(endpoint), ["public"]); + await setV1Invoker(["public"]); }) .catch(rethrowAs(endpoint, "set invoker")); } @@ -459,11 +465,17 @@ export class Fabricator { ); return; } + const setV2Invoker = (members: string[]): Promise => + endpoint.preserveExternalChanges + ? run.setInvokerUpdate(endpoint.project, serviceName, members, { + mergeExistingMembers: true, + }) + : run.setInvokerCreate(endpoint.project, serviceName, members); if (backend.isHttpsTriggered(endpoint)) { const invoker = endpoint.httpsTrigger.invoker || ["public"]; if (!invoker.includes("private")) { await this.executor - .run(() => run.setInvokerCreate(endpoint.project, serviceName, invoker)) + .run(() => setV2Invoker(invoker)) .catch(rethrowAs(endpoint, "set invoker")); } } else if (backend.isDataConnectGraphqlTriggered(endpoint)) { @@ -472,13 +484,13 @@ export class Fabricator { invoker.push(getDataConnectP4SA(this.projectNumber)); if (!invoker.includes("private")) { await this.executor - .run(() => run.setInvokerCreate(endpoint.project, serviceName, invoker)) + .run(() => setV2Invoker(invoker)) .catch(rethrowAs(endpoint, "set invoker")); } } else if (backend.isCallableTriggered(endpoint)) { // Callable functions should always be public await this.executor - .run(() => run.setInvokerCreate(endpoint.project, serviceName, ["public"])) + .run(() => setV2Invoker(["public"])) .catch(rethrowAs(endpoint, "set invoker")); } else if (backend.isTaskQueueTriggered(endpoint)) { // Like HTTPS triggers, taskQueueTriggers have an invoker, but unlike HTTPS they don't default @@ -487,7 +499,7 @@ export class Fabricator { if (invoker && !invoker.includes("private")) { await this.executor .run(async () => { - await run.setInvokerCreate(endpoint.project, serviceName, invoker); + await setV2Invoker(invoker); }) .catch(rethrowAs(endpoint, "set invoker")); } @@ -497,14 +509,14 @@ export class Fabricator { ) { // Auth Blocking functions should always be public await this.executor - .run(() => run.setInvokerCreate(endpoint.project, serviceName, ["public"])) + .run(() => setV2Invoker(["public"])) .catch(rethrowAs(endpoint, "set invoker")); } else if (backend.isScheduleTriggered(endpoint)) { const invoker = endpoint.serviceAccount ? [endpoint.serviceAccount] : [await gce.getDefaultServiceAccount(this.projectNumber)]; await this.executor - .run(() => run.setInvokerCreate(endpoint.project, serviceName, invoker)) + .run(() => setV2Invoker(invoker)) .catch(rethrowAs(endpoint, "set invoker")); } } @@ -544,7 +556,11 @@ export class Fabricator { } if (invoker) { await this.executor - .run(() => gcf.setInvokerUpdate(endpoint.project, backend.functionName(endpoint), invoker!)) + .run(() => + gcf.setInvokerUpdate(endpoint.project, backend.functionName(endpoint), invoker!, { + mergeExistingMembers: !!endpoint.preserveExternalChanges, + }), + ) .catch(rethrowAs(endpoint, "set invoker")); } } @@ -624,7 +640,11 @@ export class Fabricator { if (invoker) { await this.executor - .run(() => run.setInvokerUpdate(endpoint.project, serviceName, invoker!)) + .run(() => + run.setInvokerUpdate(endpoint.project, serviceName, invoker!, { + mergeExistingMembers: !!endpoint.preserveExternalChanges, + }), + ) .catch(rethrowAs(endpoint, "set invoker")); } } @@ -750,6 +770,7 @@ export class Fabricator { endpoint.project, `projects/${endpoint.project}/locations/${endpoint.region}/services/${endpoint.runServiceId}`, invoker, + { mergeExistingMembers: !!endpoint.preserveExternalChanges }, ), ) .catch(rethrowAs(endpoint, "set invoker")); From 41b25456c13dec5f911429d51a07117ac16c53e5 Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Thu, 23 Apr 2026 16:21:52 +0100 Subject: [PATCH 5/5] test(functions-deploy): e2e preserveExternalChanges invoker preservation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an end-to-end test that: 1. Deploys with preserveExternalChanges=true 2. Adds an external `user:` member to the v2scheduled function's Cloud Run `roles/run.invoker` binding via the real IAM API 3. Redeploys (flag still on) — asserts the member is still present 4. Redeploys with flag off — asserts the member is removed This is the scenario from issue #6549 end-to-end. Requires a real GCP test project to run (`npm run test:functions-deploy`); unit + fabricator tests from earlier commits cover the decoding/gating paths for CI speed. --- scripts/functions-deploy-tests/tests.ts | 72 +++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/scripts/functions-deploy-tests/tests.ts b/scripts/functions-deploy-tests/tests.ts index 2e40949fd26..3ce43b7d033 100644 --- a/scripts/functions-deploy-tests/tests.ts +++ b/scripts/functions-deploy-tests/tests.ts @@ -9,6 +9,7 @@ import * as cli from "./cli"; import * as proto from "../../src/gcp/proto"; import * as tasks from "../../src/gcp/cloudtasks"; import * as scheduler from "../../src/gcp/cloudscheduler"; +import * as run from "../../src/gcp/run"; import { Endpoint } from "../../src/deploy/functions/backend"; import { requireAuth } from "../../src/requireAuth"; @@ -352,6 +353,77 @@ describe("firebase deploy", function (this) { } }); + it("preserves externally-added Cloud Run invoker members when preserveExternalChanges is true", async () => { + // Regression test for firebase/firebase-tools#6549. + // Covers the scheduled-v2 path which is what the original issue reports. + const TEST_INVOKER = `user:test-preserve-${RUN_ID}@example.com`; + + // 1. Deploy with the flag set. + const optsOn: Opts = { + v1Opts: { preserveExternalChanges: true }, + v2Opts: { preserveExternalChanges: true }, + v1TqOpts: {}, + v2TqOpts: {}, + v1IdpOpts: {}, + v2IdpOpts: {}, + v1ScheduleOpts: {}, + v2ScheduleOpts: { schedule: "every 30 minutes" }, + }; + const firstDeploy = await setOptsAndDeploy(optsOn); + expect(firstDeploy.stdout, "first deploy").to.match(/Deploy complete!/); + + const endpoints = await listFns(RUN_ID); + const scheduled = Object.values(endpoints).find( + (e) => e.platform === "gcfv2" && "scheduleTrigger" in e, + ); + expect(scheduled, "v2scheduled endpoint").to.not.be.undefined; + + // 2. Add an external invoker member directly to the Cloud Run service. + const runServiceName = `projects/${FIREBASE_PROJECT}/locations/${scheduled!.region}/services/${scheduled!.runServiceId}`; + const beforePolicy = await run.getIamPolicy(runServiceName); + const preservedBinding = beforePolicy.bindings?.find( + (b) => b.role === "roles/run.invoker" && !b.condition, + ); + const preservedMembers = [...(preservedBinding?.members || []), TEST_INVOKER]; + await run.setIamPolicy(runServiceName, { + bindings: [ + ...(beforePolicy.bindings || []).filter((b) => b.role !== "roles/run.invoker" || b.condition), + { role: "roles/run.invoker", members: preservedMembers }, + ], + etag: beforePolicy.etag || "", + version: 3, + }); + + // 3. Redeploy with the flag still set — the external invoker must survive. + const secondDeploy = await setOptsAndDeploy(optsOn); + expect(secondDeploy.stdout, "second deploy").to.match(/Deploy complete!|No changes detected/); + + const afterOnPolicy = await run.getIamPolicy(runServiceName); + const afterOnBinding = afterOnPolicy.bindings?.find( + (b) => b.role === "roles/run.invoker" && !b.condition, + ); + expect(afterOnBinding?.members, "invoker members after preserve-on redeploy").to.include( + TEST_INVOKER, + ); + + // 4. Flip the flag off and redeploy — the external invoker should now be removed. + const optsOff: Opts = { + ...optsOn, + v1Opts: { preserveExternalChanges: false }, + v2Opts: { preserveExternalChanges: false }, + }; + const thirdDeploy = await setOptsAndDeploy(optsOff); + expect(thirdDeploy.stdout, "third deploy").to.match(/Deploy complete!|No changes detected/); + + const afterOffPolicy = await run.getIamPolicy(runServiceName); + const afterOffBinding = afterOffPolicy.bindings?.find( + (b) => b.role === "roles/run.invoker" && !b.condition, + ); + expect(afterOffBinding?.members, "invoker members after preserve-off redeploy").to.not.include( + TEST_INVOKER, + ); + }); + // BUGBUG: Setting options to null SHOULD restore their values to default, but this isn't correctly implemented in // the CLI. it.skip("restores default values when unspecified and preserveExternalChanges is not set", async () => {