diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..b5fc357fb88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Fixed non-interactive deployment failure when secrets are already configured in Secret Manager diff --git a/src/deploy/functions/params.spec.ts b/src/deploy/functions/params.spec.ts index ca7ee49b792..dcc7675091b 100644 --- a/src/deploy/functions/params.spec.ts +++ b/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 "); + } + }); + }); }); diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 31a4a80c3e8..133f3fbe86b 100644 --- a/src/deploy/functions/params.ts +++ b/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 " : ""}`, - ) - .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 " : ""}`, + ) + .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