Use the same secret label on create and update (#5704)

* always use the `firebase-managed` label
* add a test

CHANGELOG.md

@@ -0,0 +1 @@
+- Fix an issue where Secret Manager secrets were tagged incorrectly (#5704).

src/deploy/functions/params.ts

@@ -7,6 +7,7 @@ import * as secretManager from "../../gcp/secretManager";
 import { listBuckets } from "../../gcp/storage";
 import { isCelExpression, resolveExpression } from "./cel";
 import { FirebaseConfig } from "./args";
+import { labels as secretLabels } from "../../functions/secrets";
 
 // A convinience type containing options for Prompt's select
 interface ListItem {
@@ -457,8 +458,7 @@ async function handleSecret(secretParam: SecretParam, projectId: string) {
         secretParam.name
       }. Enter a value for ${secretParam.label || secretParam.name}:`,
     });
-    const secretLabel: Record<string, string> = { "firebase-hosting-managed": "yes" };
-    await secretManager.createSecret(projectId, secretParam.name, secretLabel);
+    await secretManager.createSecret(projectId, secretParam.name, secretLabels());
     await secretManager.addVersion(projectId, secretParam.name, secretValue);
     return secretValue;
   } else if (!metadata.secretVersion) {

src/functions/secrets.ts

@@ -23,7 +23,7 @@ import { logger } from "../logger";
 import { functionsOrigin } from "../api";
 import { assertExhaustive } from "../functional";
 
-const FIREBASE_MANGED = "firebase-managed";
+const FIREBASE_MANAGED = "firebase-managed";
 
 type ProjectInfo = {
   projectId: string;
@@ -34,15 +34,15 @@ type ProjectInfo = {
  * Returns true if secret is managed by Firebase.
  */
 export function isFirebaseManaged(secret: Secret): boolean {
-  return Object.keys(secret.labels || []).includes(FIREBASE_MANGED);
+  return Object.keys(secret.labels || []).includes(FIREBASE_MANAGED);
 }
 
 /**
  * Return labels to mark secret as managed by Firebase.
  * @internal
  */
 export function labels(): Record<string, string> {
-  return { [FIREBASE_MANGED]: "true" };
+  return { [FIREBASE_MANAGED]: "true" };
 }
 
 function toUpperSnakeCase(key: string): string {
@@ -173,7 +173,7 @@ export async function pruneSecrets(
   const prunedSecrets: Set<string> = new Set();
 
   // Collect all Firebase managed secret versions
-  const haveSecrets = await listSecrets(projectId, `labels.${FIREBASE_MANGED}=true`);
+  const haveSecrets = await listSecrets(projectId, `labels.${FIREBASE_MANAGED}=true`);
   for (const secret of haveSecrets) {
     const versions = await listSecretVersions(projectId, secret.name, `NOT state: DESTROYED`);
     for (const version of versions) {

src/test/functions/secrets.spec.ts

@@ -124,6 +124,17 @@ describe("functions/secret", () => {
       expect(promptStub).to.have.been.calledOnce;
     });
 
+    it("does not prompt user to have Firebase manage the secret if already managed by Firebase", async () => {
+      getStub.resolves({ ...secret, labels: secrets.labels() });
+      patchStub.resolves(secret);
+
+      await expect(
+        secrets.ensureSecret("project-id", "MY_SECRET", options)
+      ).to.eventually.deep.equal(secret);
+      expect(warnStub).not.to.have.been.calledOnce;
+      expect(promptStub).not.to.have.been.calledOnce;
+    });
+
     it("creates a new secret if it doesn't exists", async () => {
       getStub.rejects({ status: 404 });
       createStub.resolves(secret);