Skip to content

Commit 00a5469

Browse files
committed
Fix non-interactive deployment failure when secrets exist in Secret Manager
PR firebase#9335 added a check that fails deployments in non-interactive mode when secrets are required. However, it didn't verify whether those secrets already exist in Secret Manager, causing deployments to fail even when all secrets were properly configured. This change queries Secret Manager before throwing the error to check if each required secret exists. Only truly missing secrets will cause the deployment to fail. Fixes firebase#9368
1 parent 299aa83 commit 00a5469

File tree

3 files changed

+125
-12
lines changed

3 files changed

+125
-12
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Fixed non-interactive deployment failure when secrets are already configured in Secret Manager

src/deploy/functions/params.spec.ts

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import * as sinon from "sinon";
33

44
import * as prompt from "../../prompt";
55
import * as params from "./params";
6+
import * as secretManager from "../../gcp/secretManager";
7+
import { FirebaseError } from "../../error";
68

79
const expect = chai.expect;
810
const fakeConfig = {
@@ -298,4 +300,97 @@ describe("resolveParams", () => {
298300
input.resolves("22");
299301
await expect(params.resolveParams(paramsToResolve, fakeConfig, {})).to.eventually.be.rejected;
300302
});
303+
304+
describe("non-interactive mode with secrets", () => {
305+
let getSecretMetadataStub: sinon.SinonStub;
306+
307+
beforeEach(() => {
308+
getSecretMetadataStub = sinon.stub(secretManager, "getSecretMetadata");
309+
});
310+
311+
afterEach(() => {
312+
getSecretMetadataStub.restore();
313+
});
314+
315+
it("should succeed when secrets already exist in Secret Manager", async () => {
316+
const paramsToResolve: params.Param[] = [
317+
{
318+
name: "MY_SECRET",
319+
type: "secret",
320+
},
321+
];
322+
getSecretMetadataStub.resolves({
323+
secret: { name: "MY_SECRET" },
324+
secretVersion: { name: "MY_SECRET/versions/1", state: "ENABLED" },
325+
});
326+
327+
await expect(params.resolveParams(paramsToResolve, fakeConfig, {}, { nonInteractive: true }))
328+
.to.eventually.be.fulfilled;
329+
});
330+
331+
it("should throw error when secrets don't exist in non-interactive mode", async () => {
332+
const paramsToResolve: params.Param[] = [
333+
{
334+
name: "MISSING_SECRET",
335+
type: "secret",
336+
},
337+
];
338+
getSecretMetadataStub.resolves({});
339+
340+
await expect(
341+
params.resolveParams(paramsToResolve, fakeConfig, {}, { nonInteractive: true }),
342+
).to.eventually.be.rejectedWith(
343+
FirebaseError,
344+
/In non-interactive mode but have no value for the following secrets: MISSING_SECRET/,
345+
);
346+
});
347+
348+
it("should only report missing secrets, not existing ones in non-interactive mode", async () => {
349+
const paramsToResolve: params.Param[] = [
350+
{
351+
name: "EXISTING_SECRET",
352+
type: "secret",
353+
},
354+
{
355+
name: "MISSING_SECRET",
356+
type: "secret",
357+
},
358+
];
359+
getSecretMetadataStub.callsFake((projectId: string, secretName: string) => {
360+
if (secretName === "EXISTING_SECRET") {
361+
return Promise.resolve({
362+
secret: { name: "EXISTING_SECRET" },
363+
secretVersion: { name: "EXISTING_SECRET/versions/1", state: "ENABLED" },
364+
});
365+
}
366+
return Promise.resolve({});
367+
});
368+
369+
try {
370+
await params.resolveParams(paramsToResolve, fakeConfig, {}, { nonInteractive: true });
371+
expect.fail("Should have thrown an error");
372+
} catch (err: any) {
373+
expect(err.message).to.include("MISSING_SECRET");
374+
expect(err.message).to.not.include("EXISTING_SECRET");
375+
}
376+
});
377+
378+
it("should include format flag in error for JSON secrets", async () => {
379+
const paramsToResolve: params.Param[] = [
380+
{
381+
name: "JSON_SECRET",
382+
type: "secret",
383+
format: "json",
384+
},
385+
];
386+
getSecretMetadataStub.resolves({});
387+
388+
try {
389+
await params.resolveParams(paramsToResolve, fakeConfig, {}, { nonInteractive: true });
390+
expect.fail("Should have thrown an error");
391+
} catch (err: any) {
392+
expect(err.message).to.include("--format=json --data-file <file.json>");
393+
}
394+
});
395+
});
301396
});

src/deploy/functions/params.ts

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -397,18 +397,35 @@ export async function resolveParams(
397397

398398
// Check for missing secrets in non-interactive mode
399399
if (nonInteractive && needSecret.length > 0) {
400-
const secretNames = needSecret.map((p) => p.name).join(", ");
401-
const commands = needSecret
402-
.map(
403-
(p) =>
404-
`\tfirebase functions:secrets:set ${p.name}${(p as SecretParam).format === "json" ? " --format=json --data-file <file.json>" : ""}`,
405-
)
406-
.join("\n");
407-
throw new FirebaseError(
408-
`In non-interactive mode but have no value for the following secrets: ${secretNames}\n\n` +
409-
"Set these secrets before deploying:\n" +
410-
commands,
411-
);
400+
// First check which secrets actually don't exist in Secret Manager
401+
const missingSecrets: SecretParam[] = [];
402+
for (const param of needSecret) {
403+
const secretParam = param as SecretParam;
404+
const metadata = await secretManager.getSecretMetadata(
405+
firebaseConfig.projectId,
406+
secretParam.name,
407+
"latest",
408+
);
409+
if (!metadata.secret) {
410+
missingSecrets.push(secretParam);
411+
}
412+
}
413+
414+
// Only throw an error if there are truly missing secrets
415+
if (missingSecrets.length > 0) {
416+
const secretNames = missingSecrets.map((p) => p.name).join(", ");
417+
const commands = missingSecrets
418+
.map(
419+
(p) =>
420+
`\tfirebase functions:secrets:set ${p.name}${p.format === "json" ? " --format=json --data-file <file.json>" : ""}`,
421+
)
422+
.join("\n");
423+
throw new FirebaseError(
424+
`In non-interactive mode but have no value for the following secrets: ${secretNames}\n\n` +
425+
"Set these secrets before deploying:\n" +
426+
commands,
427+
);
428+
}
412429
}
413430

414431
// The functions emulator will handle secrets

0 commit comments

Comments
 (0)