test(functions): failing repro tests for declarative security managed SA bugs

src/deploy/functions/prepare.spec.ts

@@ -142,11 +142,11 @@
         .to.be.rejectedWith(FirebaseError)
         .then((error) => {
           // Should always list latest runtimes
           expect(error.message).to.include(latest("nodejs"));
           expect(error.message).to.include(latest("python"));
 
           // Should never list a decommissioned runtime
           expect(error.message).to.not.include("nodejs6");
         });
     });
 
@@ -728,7 +728,7 @@
         ...ENDPOINT_BASE,
         httpsTrigger: {},
       };
       const have: backend.Endpoint = JSON.parse(JSON.stringify(want));
       have.timeoutSeconds = 120;
 
       prepare.inferDetailsFromExisting(backend.of(want), backend.of(have), /* usedDotEnv= */ false);
@@ -985,7 +985,7 @@
       await prepare.warnIfNewGenkitFunctionIsMissingSecrets(
         backend.empty(),
         backend.of(nonGenkitEndpoint),
         {} as any,
       );
       expect(confirm).to.not.be.called;
     });
@@ -994,7 +994,7 @@
       await prepare.warnIfNewGenkitFunctionIsMissingSecrets(
         backend.empty(),
         backend.of(genkitEndpointWithSecrets),
         {} as any,
       );
       expect(confirm).to.not.be.called;
     });
@@ -1003,7 +1003,7 @@
       await prepare.warnIfNewGenkitFunctionIsMissingSecrets(
         backend.of(genkitEndpointWithoutSecrets),
         backend.of(genkitEndpointWithoutSecrets),
         {} as any,
       );
       expect(confirm).to.not.be.called;
     });
@@ -1200,12 +1200,15 @@
 
   describe("discoverSecurityDetails", () => {
     let testIamPermissionsStub: sinon.SinonStub;
+    let generateManagedSANameStub: sinon.SinonStub;
 
     beforeEach(() => {
       testIamPermissionsStub = sinon
         .stub(iam, "testIamPermissions")
         .resolves({ passed: true } as any);
-      sinon.stub(iam, "generateManagedServiceAccountName").resolves("firebase-fn-123");
+      generateManagedSANameStub = sinon
+        .stub(iam, "generateManagedServiceAccountName")
+        .resolves("firebase-fn-123");
       sinon.stub(resourcemanager, "getServiceAccountRoles").resolves([]);
     });
 
@@ -1229,6 +1232,44 @@
       expect(e.labels?.["firebase-declarative-security-etag"]).to.equal(result.newEtag);
     });
 
+    it("reuses an existing managed service account in the project when no deployed functions reference it (BUG: currently generates a new one)", async () => {
+      // Scenario: a previous deploy created the managed SA and granted it roles, but
+      // function creation failed, so `have` is empty. The project still contains
+      // the managed SA, discoverable via IAM lookup.
+      const preexistingSA = "firebase-fn-1234567890@project.iam.gserviceaccount.com";
+      const getServiceAccountStub = sinon.stub(iam, "getServiceAccount").resolves({
+        name: `projects/project/serviceAccounts/${preexistingSA}`,
+        projectId: "project",
+        uniqueId: "12345",
+        email: preexistingSA,
+        displayName: "Firebase Functions managed service account",
+        etag: "etag",
+        description: "",
+        oauth2ClientId: "",
+        disabled: false,
+      });
+
+      const e: backend.Endpoint = {
+        ...ENDPOINT,
+      };
+      const want = backend.of(e);
+      want.requiredRoles = ["roles/viewer"];
+      const have = backend.empty();
+
+      const result = await prepare.discoverSecurityDetails("default", want, have, "project");
+
+      // Desired behavior: reuse the SA that already exists in the project instead
+      // of generating a brand new random name (which orphans the previous SA and
+      // its project-level role grants on every failed deploy).
+      expect(
+        getServiceAccountStub.called || !generateManagedSANameStub.called,
+        "expected discoverSecurityDetails to look up existing managed service accounts " +
+          "in the project instead of unconditionally generating a new random SA name",
+      ).to.be.true;
+      expect(result.managedSA).to.equal(preexistingSA);
+      expect(e.serviceAccount).to.equal(preexistingSA);
+    });
+
     it("should reset endpoints to default service account when unenrolling (opting out)", async () => {
       const e: backend.Endpoint = {
         ...ENDPOINT,

src/deploy/functions/release/fabricator.spec.ts

@@ -670,6 +670,31 @@ describe("Fabricator", () => {
       expect(gcfv2.deleteFunction).to.have.been.called;
     });
 
+    it("retries function creation when a freshly created service account has not yet propagated (BUG: currently fails immediately)", async () => {
+      // GCF returns HTTP 404 when the deploy references a service account that
+      // was created moments ago and has not propagated through IAM yet.
+      const saNotFoundErr = new Error(
+        "Service account projects/-/serviceAccounts/firebase-fn-123@test-project.iam.gserviceaccount.com was not found. Please verify that the caller has iam.serviceAccounts.actAs permission on the service account.",
+      );
+      (saNotFoundErr as any).status = 404;
+      gcfv2.createFunction.onFirstCall().rejects(saNotFoundErr);
+      gcfv2.createFunction.resolves({ name: "op", done: false });
+      poller.pollOperation.resolves({ serviceConfig: { service: "service" } });
+      run.setInvokerCreate.resolves();
+
+      // Use the same executor type as a real deploy (release/index.ts) so that
+      // retry-code based mitigations are exercised too, with fast backoff.
+      const queueFab = new fabricator.Fabricator({
+        ...ctorArgs,
+        functionExecutor: new executor.QueueExecutor({ retries: 3, backoff: 1, maxBackoff: 10 }),
+      });
+
+      const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" });
+      await queueFab.createV2Function(ep, new scraper.SourceTokenScraper());
+
+      expect(gcfv2.createFunction).to.have.been.calledTwice;
+    });
+
     it("throws on set invoker failure", async () => {
       gcfv2.createFunction.resolves({ name: "op", done: false });
       poller.pollOperation.resolves({ serviceConfig: { service: "service" } });
@@ -2071,6 +2096,26 @@ describe("Fabricator", () => {
       );
     });
 
+    it("waits for a newly created service account to be visible before returning from grantNewRoles (BUG: no propagation check)", async () => {
+      // IAM service account creation is eventually consistent. grantNewRoles
+      // should poll getServiceAccount until the new SA is visible before the
+      // deploy proceeds to create functions that actAs it.
+      const getServiceAccountStub = sinon.stub(iam, "getServiceAccount").resolves({
+        name: "projects/test-project/serviceAccounts/firebase-fn-123@my-proj.iam.gserviceaccount.com",
+        email: "firebase-fn-123@my-proj.iam.gserviceaccount.com",
+      } as any);
+      const plan: planner.CodebasePlan = {
+        regionalChangesets: {},
+        serviceAccountToCreate: "firebase-fn-123@my-proj.iam.gserviceaccount.com",
+        managedServiceAccount: "firebase-fn-123@my-proj.iam.gserviceaccount.com",
+      };
+
+      await fab.grantNewRoles(plan, "default");
+
+      expect(createServiceAccountStub).to.have.been.called;
+      expect(getServiceAccountStub).to.have.been.called;
+    });
+
     it("should remove roles or delete SA in removeOldRoles", async () => {
       const plan: planner.CodebasePlan = {
         regionalChangesets: {},