[Draft] fix: preserve existing flag across triggers

scripts/functions-deploy-tests/tests.ts

@@ -9,6 +9,7 @@
 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";
 
@@ -40,7 +41,7 @@
   v2ScheduleOpts: functionsv2.scheduler.ScheduleOptions;
 }
 
 async function setOpts(opts: Opts) {
   let stmt = "";
   for (const [name, opt] of Object.entries(opts)) {
     if (opt) {
@@ -52,10 +53,10 @@
 
 async function listFns(runId: string): Promise<Record<string, Endpoint>> {
   const result = await cli.exec("functions:list", FIREBASE_PROJECT, ["--json"], __dirname, false);
   const output = JSON.parse(result.stdout);
 
   const eps: Record<string, Endpoint> = {};
   for (const ep of output.result as Endpoint[]) {
     const id = ep.id.replace(`${runId}-`, "");
     if (ep.id !== id) {
       // By default, functions list does not attempt to fully hydrate configuration options for task queue and schedule
@@ -66,7 +67,7 @@
       }
       if ("scheduleTrigger" in ep) {
         const jobName = scheduler.jobNameForEndpoint(ep, "us-central1");
         const job = (await scheduler.getJob(jobName)).body as scheduler.Job;
         if (job.retryConfig) {
           const cfg = job.retryConfig;
           ep.scheduleTrigger.retryConfig = {
@@ -129,8 +130,8 @@
   after(async () => {
     try {
       await fs.unlink(path.join(FUNCTIONS_DIR, "index.js"));
     } catch (e: any) {
       if (e?.code === "ENOENT") {
         return;
       }
       throw e;
@@ -352,6 +353,77 @@
     }
   });
 
+  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 () => {

src/deploy/functions/backend.ts

@@ -195,7 +195,7 @@
   return allMemoryOptions.includes(mem as MemoryOptions);
 }
 
 export function isValidEgressSetting(egress: unknown): egress is VpcEgressSettings {
   return egress === "PRIVATE_RANGES_ONLY" || egress === "ALL_TRAFFIC";
 }
 
@@ -316,6 +316,7 @@
   } | null;
   ingressSettings?: IngressSettings | null;
   serviceAccount?: string | null;
+  preserveExternalChanges?: boolean | null;
 }
 
 export const AllFunctionsPlatforms = ["gcfv1", "gcfv2", "run"] as const;

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: {

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<boolean>;
 
+  // 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<boolean>;
+
   // Defaults to "gcfv2".
   platform?: "gcfv1" | "gcfv2" | "run";
 
@@ -551,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) {