Prompt correctly for secrets defined in codebases with namespaced prefixes

src/deploy/functions/build.spec.ts

@@ -728,8 +728,12 @@ describe("applyPrefix", () => {
     expect(Object.keys(testBuild.endpoints).sort()).to.deep.equal(["func1", "func2"]);
   });
 
-  it("should prefix secret names in secretEnvironmentVariables", () => {
+  it("should prefix secret names in secretEnvironmentVariables and update param references to that secret", () => {
     const testBuild: build.Build = {
+      params: [
+        { type: "secret", name: "API_KEY" },
+        { type: "secret", name: "DB_PASSWORD" },
+      ],
       endpoints: {
         func1: {
           region: "us-central1",
@@ -755,7 +759,6 @@ describe("applyPrefix", () => {
           ],
         },
       },
-      params: [],
       requiredAPIs: [],
     };
 
@@ -772,6 +775,10 @@ describe("applyPrefix", () => {
     expect(testBuild.endpoints["staging-func2"].secretEnvironmentVariables).to.deep.equal([
       { key: "SERVICE_TOKEN", secret: "staging-service-secret", projectId: "test-project" },
     ]);
+    expect(testBuild.params).to.deep.equal([
+      { type: "secret", name: "API_KEY", resourceId: "STAGING_API_KEY" },
+      { type: "secret", name: "DB_PASSWORD", resourceId: "STAGING_DB_PASSWORD" },
+    ]);
   });
 
   it("throws if combined function id exceeds 63 characters", () => {

src/deploy/functions/build.ts

@@ -8,6 +8,7 @@
 import { Runtime } from "./runtimes/supported";
 import { ExprParseError } from "./cel";
 import { defineSecret } from "firebase-functions/params";
+import { toUpperSnakeCase } from "../../functions/secrets";
 
 export const REGION_TBD = "REGION_TBD";
 export const SECRET_REF_PREFIX = "FIREBASE_SECRET_REF_";
@@ -354,15 +355,15 @@
   envs: Record<string, params.ParamValue>;
   secretRefs: Record<string, string>;
 }> {
-  const { paramValues: paramValues, secretRefs: secretRefs } = await params.resolveParams(
-    opts.build.params,
-    opts.firebaseConfig,
-    envWithTypes(opts.build.params, opts.userEnvs),
-    opts.codebase,
-    opts.nonInteractive,
-    opts.force,
-    opts.isEmulator,
-  );
+  const { paramValues: paramValues, secretRefs: secretRefs } = await params.resolveParams({
+    params: opts.build.params,
+    firebaseConfig: opts.firebaseConfig,
+    userEnvs: envWithTypes(opts.build.params, opts.userEnvs),
+    codebase: opts.codebase,
+    nonInteractive: opts.nonInteractive,
+    force: opts.force,
+    isEmulator: opts.isEmulator,
+  });
 
   return { backend: toBackend(opts.build, paramValues), envs: paramValues, secretRefs: secretRefs };
 }
@@ -516,7 +517,7 @@
       // List param, we try resolving a String param instead.
       try {
         regions = params.resolveList(bdEndpoint.region, paramValues);
       } catch (err: any) {
         if (err instanceof ExprParseError) {
           regions = [params.resolveString(bdEndpoint.region, paramValues)];
         } else {
@@ -740,6 +741,14 @@
  * Prefixes all endpoint IDs and secret names in a build with a given prefix.
  * This ensures that functions and their associated secrets from different codebases
  * remain isolated and don't conflict when deployed to the same project.
+ *
+ * Secret params in a build are rewritten to point to resource names respecting
+ * the same prefixing, so that the interactive secret creation flow still works
+ * and the non-interactive message prints the correct secret to create.
+ *
+ * When deploying a function which already has secret bindings in its .env files,
+ * applyEnvSecretBindings will run after this and overwrite both the updated
+ * secret params and SecretEnvVars to reflect deployed reality.
  */
 export function applyPrefix(build: Build, prefix: string): void {
   if (!prefix) {
@@ -767,12 +776,19 @@
     if (endpoint.secretEnvironmentVariables) {
       endpoint.secretEnvironmentVariables = endpoint.secretEnvironmentVariables.map((secret) => ({
         ...secret,
-        secret: `${prefix}-${secret.secret}`,
+        secret: toUpperSnakeCase(`${prefix}-${secret.secret}`),
       }));
     }
   }
   build.endpoints = newEndpoints;
 
+  for (const param of build.params) {
+    if (param.type !== "secret") {
+      continue;
+    }
+    param.resourceId = toUpperSnakeCase(`${prefix}-${param.resourceId || param.name}`);
+  }
+
   if (build.lifecycleHooks) {
     for (const hook of Object.values(build.lifecycleHooks)) {
       if ("task" in hook) {
@@ -806,7 +822,7 @@
  * /version can be omitted and will cause the secret to resolve to whatever the latest version was at time of deploy.
  *
  * For each binding imported from the .env file,
- * 1) TODO: Check if a conflicting SecretParam with the same name exists. If so, override the param so that the prompting flow will look in the right place when deciding whether or not to create a new Secret.
+ * 1) Check if a conflicting SecretParam with the same name exists. If so, override the param so that the prompting flow will look in the right place when deciding whether or not to create a new Secret.
  * 2) Upsert the binding directly into the Build's SecretEnvVars, which will cause it to be actually available in process.ENV
  */
 export function applyEnvSecretBindings(

src/deploy/functions/params.spec.ts

@@ -100,7 +100,14 @@
     const paramsToResolve: params.Param[] = [];
     const userEnv: Record<string, params.ParamValue> = {};
     expect(
-      (await params.resolveParams(paramsToResolve, fakeConfig, userEnv, "default")).paramValues,
+      (
+        await params.resolveParams({
+          params: paramsToResolve,
+          firebaseConfig: fakeConfig,
+          userEnvs: userEnv,
+          codebase: "default",
+        })
+      ).paramValues,
     ).to.deep.equal(expectedInternalParams);
   });
 
@@ -121,7 +128,14 @@
       baz: new params.ParamValue("true", false, { string: false, number: false, boolean: true }),
     };
     expect(
-      (await params.resolveParams(paramsToResolve, fakeConfig, userEnv, "default")).paramValues,
+      (
+        await params.resolveParams({
+          params: paramsToResolve,
+          firebaseConfig: fakeConfig,
+          userEnvs: userEnv,
+          codebase: "default",
+        })
+      ).paramValues,
     ).to.deep.equal(
       Object.assign(
         {
@@ -148,7 +162,14 @@
       }),
     };
     expect(
-      (await params.resolveParams(paramsToResolve, fakeConfig, userEnv, "default")).paramValues,
+      (
+        await params.resolveParams({
+          params: paramsToResolve,
+          firebaseConfig: fakeConfig,
+          userEnvs: userEnv,
+          codebase: "default",
+        })
+      ).paramValues,
     ).to.deep.equal({
       DATABASE_URL: new params.ParamValue(fakeConfig.databaseURL, true, {
         string: true,
@@ -178,12 +199,12 @@
     const userEnv: Record<string, params.ParamValue> = {};
     expect(
       (
-        await params.resolveParams(
-          paramsToResolve,
-          { locationId: "", projectId: "foo", storageBucket: "", databaseURL: "" },
-          userEnv,
-          "default",
-        )
+        await params.resolveParams({
+          params: paramsToResolve,
+          firebaseConfig: { locationId: "", projectId: "foo", storageBucket: "", databaseURL: "" },
+          userEnvs: userEnv,
+          codebase: "default",
+        })
       ).paramValues,
     ).to.deep.equal({
       GCLOUD_PROJECT: expectedInternalParams.GCLOUD_PROJECT,
@@ -202,7 +223,14 @@
     ];
     input.resolves("bar");
     expect(
-      (await params.resolveParams(paramsToResolve, fakeConfig, {}, "default")).paramValues,
+      (
+        await params.resolveParams({
+          params: paramsToResolve,
+          firebaseConfig: fakeConfig,
+          userEnvs: {},
+          codebase: "default",
+        })
+      ).paramValues,
     ).to.deep.equal(
       Object.assign(
         {
@@ -229,8 +257,13 @@
       },
     ];
     input.resolves("baz");
-    await params.resolveParams(paramsToResolve, fakeConfig, {}, "default");
+    await params.resolveParams({
+      params: paramsToResolve,
+      firebaseConfig: fakeConfig,
+      userEnvs: {},
+      codebase: "default",
+    });
     expect(input.getCall(1).args[0].default).to.eq("baz");
   });
 
   it("can resolve a CEL expression containing only identities", async () => {
@@ -249,8 +282,13 @@
       },
     ];
     input.resolves("baz");
-    await params.resolveParams(paramsToResolve, fakeConfig, {}, "default");
+    await params.resolveParams({
+      params: paramsToResolve,
+      firebaseConfig: fakeConfig,
+      userEnvs: {},
+      codebase: "default",
+    });
     expect(input.getCall(1).args[0].default).to.eq("baz/quox");
   });
 
   it("can resolve a CEL expression depending on the internal params", async () => {
@@ -275,10 +313,15 @@
       },
     ];
     input.resolves("baz");
-    await params.resolveParams(paramsToResolve, fakeConfig, {}, "default");
+    await params.resolveParams({
+      params: paramsToResolve,
+      firebaseConfig: fakeConfig,
+      userEnvs: {},
+      codebase: "default",
+    });
     expect(input.getCall(0).args[0].default).to.eq("https://foo.firebaseio.com/quox");
     expect(input.getCall(1).args[0].default).to.eq("projectID: foo");
     expect(input.getCall(2).args[0].default).to.eq(
       "http://foo.appspot.com.storage.googleapis.com/",
     );
   });
@@ -293,8 +336,14 @@
       },
     ];
     input.resolves("");
-    await expect(params.resolveParams(paramsToResolve, fakeConfig, {}, "default")).to.eventually.be
-      .rejected;
+    await expect(
+      params.resolveParams({
+        params: paramsToResolve,
+        firebaseConfig: fakeConfig,
+        userEnvs: {},
+        codebase: "default",
+      }),
+    ).to.eventually.be.rejected;
   });
 
   it("errors when the default is a CEL expression that resolves to the wrong type", async () => {
@@ -313,19 +362,32 @@
       },
     ];
     input.resolves("22");
-    await expect(params.resolveParams(paramsToResolve, fakeConfig, {}, "default")).to.eventually.be
-      .rejected;
+    await expect(
+      params.resolveParams({
+        params: paramsToResolve,
+        firebaseConfig: fakeConfig,
+        userEnvs: {},
+        codebase: "default",
+      }),
+    ).to.eventually.be.rejected;
   });
 
   it("does not throw in non-interactive mode if secret exists in cloud", async () => {
     const paramsToResolve: params.Param[] = [{ name: "MY_SECRET", type: "secret" }];
     const getSecretMetadataStub = sinon.stub(secretManager, "getSecretMetadata").resolves({
       secret: { name: "MY_SECRET", projectId: "foo", labels: {}, replication: {} },
       secretVersion: { versionId: "1", state: "ENABLED", secret: {} as any },
     });
 
-    await expect(params.resolveParams(paramsToResolve, fakeConfig, {}, "default", true)).to.be
-      .fulfilled;
+    await expect(
+      params.resolveParams({
+        params: paramsToResolve,
+        firebaseConfig: fakeConfig,
+        userEnvs: {},
+        codebase: "default",
+        nonInteractive: true,
+      }),
+    ).to.be.fulfilled;
 
     getSecretMetadataStub.restore();
   });
@@ -337,7 +399,13 @@
     });
 
     await expect(
-      params.resolveParams(paramsToResolve, fakeConfig, {}, "default", true),
+      params.resolveParams({
+        params: paramsToResolve,
+        firebaseConfig: fakeConfig,
+        userEnvs: {},
+        codebase: "default",
+        nonInteractive: true,
+      }),
     ).to.be.rejectedWith(FirebaseError, /In non-interactive mode but have no value for the secret/);
 
     getSecretMetadataStub.restore();
@@ -347,7 +415,13 @@
     const paramsToResolve: params.Param[] = [{ name: "MY_SECRET", type: "secret" }];
     const getSecretMetadataSpy = sinon.spy(secretManager, "getSecretMetadata");
 
-    await params.resolveParams(paramsToResolve, fakeConfig, {}, "default", false, false, true);
+    await params.resolveParams({
+      params: paramsToResolve,
+      firebaseConfig: fakeConfig,
+      userEnvs: {},
+      codebase: "default",
+      isEmulator: true,
+    });
     expect(getSecretMetadataSpy.called).to.be.false;
 
     getSecretMetadataSpy.restore();
@@ -362,7 +436,12 @@
       },
     ];
     input.resolves("bar");
-    await params.resolveParams(paramsToResolve, fakeConfig, {}, "my-codebase");
+    await params.resolveParams({
+      params: paramsToResolve,
+      firebaseConfig: fakeConfig,
+      userEnvs: {},
+      codebase: "my-codebase",
+    });
     expect(
       loggerInfoStub.calledWith(sinon.match(/Prompting for parameters for codebase.*my-codebase/)),
     ).to.be.true;

src/deploy/functions/params.ts

@@ -290,7 +290,7 @@
     return pv;
   }
 
   setDelimiter(delimiter: string) {
     this.delimiter = delimiter;
   }
 
@@ -317,7 +317,7 @@
     if (this.rawValue.includes("[")) {
       // Convert quotes to apostrophes
       const unquoted = this.rawValue.replace(/'/g, '"');
       return JSON.parse(unquoted);
     }
 
     // Continue to handle something like "a,b,c"
@@ -383,6 +383,16 @@
   assertExhaustive(param);
 }
 
+interface ResolveParamOpts {
+  params: Param[];
+  firebaseConfig: FirebaseConfig;
+  userEnvs: Record<string, ParamValue>;
+  codebase: string;
+  nonInteractive?: boolean;
+  force?: boolean;
+  isEmulator?: boolean;
+}
+
 /**
  * A param defined by the SDK may resolve to:
  * - a reference to a secret in Cloud Secret Manager, which we validate the existence of and prompt for if missing
@@ -395,14 +405,17 @@
  *   - after prompting, the resolved value of the param is written to the most specific .env file available
  */
 export async function resolveParams(
-  params: Param[],
-  firebaseConfig: FirebaseConfig,
-  userEnvs: Record<string, ParamValue>,
-  codebase: string,
-  nonInteractive?: boolean,
-  force?: boolean,
-  isEmulator = false,
+  opts: ResolveParamOpts,
 ): Promise<{ paramValues: Record<string, ParamValue>; secretRefs: Record<string, string> }> {
+  const {
+    params,
+    firebaseConfig,
+    userEnvs,
+    codebase,
+    nonInteractive = false,
+    force = false,
+    isEmulator = false,
+  } = opts;
   const paramValues: Record<string, ParamValue> = populateDefaultParams(firebaseConfig);
   const secretRefs: Record<string, string> = {};