Fix non-interactive deployment failure when secrets exist in Secret Manager

CHANGELOG.md

@@ -0,0 +1 @@
+- Fixed non-interactive deployment failure when secrets are already configured in Secret Manager

src/deploy/functions/params.spec.ts

@@ -3,6 +3,8 @@ import * as sinon from "sinon";
 
 import * as prompt from "../../prompt";
 import * as params from "./params";
+import * as secretManager from "../../gcp/secretManager";
+import { FirebaseError } from "../../error";
 
 const expect = chai.expect;
 const fakeConfig = {
@@ -298,4 +300,97 @@ describe("resolveParams", () => {
     input.resolves("22");
     await expect(params.resolveParams(paramsToResolve, fakeConfig, {})).to.eventually.be.rejected;
   });
+
+  describe("non-interactive mode with secrets", () => {
+    let getSecretMetadataStub: sinon.SinonStub;
+
+    beforeEach(() => {
+      getSecretMetadataStub = sinon.stub(secretManager, "getSecretMetadata");
+    });
+
+    afterEach(() => {
+      getSecretMetadataStub.restore();
+    });
+
+    it("should succeed when secrets already exist in Secret Manager", async () => {
+      const paramsToResolve: params.Param[] = [
+        {
+          name: "MY_SECRET",
+          type: "secret",
+        },
+      ];
+      getSecretMetadataStub.resolves({
+        secret: { name: "MY_SECRET" },
+        secretVersion: { name: "MY_SECRET/versions/1", state: "ENABLED" },
+      });
+
+      await expect(params.resolveParams(paramsToResolve, fakeConfig, {}, { nonInteractive: true }))
+        .to.eventually.be.fulfilled;
+    });
+
+    it("should throw error when secrets don't exist in non-interactive mode", async () => {
+      const paramsToResolve: params.Param[] = [
+        {
+          name: "MISSING_SECRET",
+          type: "secret",
+        },
+      ];
+      getSecretMetadataStub.resolves({});
+
+      await expect(
+        params.resolveParams(paramsToResolve, fakeConfig, {}, { nonInteractive: true }),
+      ).to.eventually.be.rejectedWith(
+        FirebaseError,
+        /In non-interactive mode but have no value for the following secrets: MISSING_SECRET/,
+      );
+    });
+
+    it("should only report missing secrets, not existing ones in non-interactive mode", async () => {
+      const paramsToResolve: params.Param[] = [
+        {
+          name: "EXISTING_SECRET",
+          type: "secret",
+        },
+        {
+          name: "MISSING_SECRET",
+          type: "secret",
+        },
+      ];
+      getSecretMetadataStub.callsFake((projectId: string, secretName: string) => {
+        if (secretName === "EXISTING_SECRET") {
+          return Promise.resolve({
+            secret: { name: "EXISTING_SECRET" },
+            secretVersion: { name: "EXISTING_SECRET/versions/1", state: "ENABLED" },
+          });
+        }
+        return Promise.resolve({});
+      });
+
+      try {
+        await params.resolveParams(paramsToResolve, fakeConfig, {}, { nonInteractive: true });
+        expect.fail("Should have thrown an error");
+      } catch (err: any) {
+        expect(err.message).to.include("MISSING_SECRET");
+        expect(err.message).to.not.include("EXISTING_SECRET");
+      }
+    });
+
+    it("should include format flag in error for JSON secrets", async () => {
+      const paramsToResolve: params.Param[] = [
+        {
+          name: "JSON_SECRET",
+          type: "secret",
+          format: "json",
+        },
+      ];
+      getSecretMetadataStub.resolves({});
+
+      try {
+        await params.resolveParams(paramsToResolve, fakeConfig, {}, { nonInteractive: true });
+        expect.fail("Should have thrown an error");
+      } catch (err: any) {
+        expect(err.message).to.include("--format=json --data-file <file.json>");
+      }
+    });
+  });
 });

src/deploy/functions/params.ts

@@ -397,18 +397,33 @@ export async function resolveParams(
 
   // Check for missing secrets in non-interactive mode
   if (nonInteractive && needSecret.length > 0) {
-    const secretNames = needSecret.map((p) => p.name).join(", ");
-    const commands = needSecret
-      .map(
-        (p) =>
-          `\tfirebase functions:secrets:set ${p.name}${(p as SecretParam).format === "json" ? " --format=json --data-file <file.json>" : ""}`,
-      )
-      .join("\n");
-    throw new FirebaseError(
-      `In non-interactive mode but have no value for the following secrets: ${secretNames}\n\n` +
-        "Set these secrets before deploying:\n" +
-        commands,
-    );
+    // Check all secrets in parallel for better performance
+    const metadataChecks = needSecret.map((param) => {
+      const secretParam = param as SecretParam;
+      return secretManager.getSecretMetadata(firebaseConfig.projectId, secretParam.name, "latest");
+    });
+    const metadataResults = await Promise.all(metadataChecks);
+
+    // Filter for secrets that don't exist
+    const missingSecrets: SecretParam[] = needSecret.filter((param, index) => {
+      return !metadataResults[index].secret;
+    }) as SecretParam[];
+
+    // Only throw an error if there are truly missing secrets
+    if (missingSecrets.length > 0) {
+      const secretNames = missingSecrets.map((p) => p.name).join(", ");
+      const commands = missingSecrets
+        .map(
+          (p) =>
+            `\tfirebase functions:secrets:set ${p.name}${p.format === "json" ? " --format=json --data-file <file.json>" : ""}`,
+        )
+        .join("\n");
+      throw new FirebaseError(
+        `In non-interactive mode but have no value for the following secrets: ${secretNames}\n\n` +
+          "Set these secrets before deploying:\n" +
+          commands,
+      );
+    }
   }
 
   // The functions emulator will handle secrets