Secrets set now handles test vs production secrets (#8359)

apphosting:secrets:set now asks if the secret is test-only and can update apphosting.emulator.yaml instead for these cases (as well as granting individuals or groups access).

1. When adding a new secret, prompts if this secret is for local development only.
2. If only local development, asks for a list of users or groups to grant access.
3. If local only, tries to add the secret to apphosting.emulator.yaml instead of apphosting.yaml
4. If local only, tries stripping any "test-" prefix off of suggested key names to help with user flows where people will want something like stripe-key and test-stripe-key to both point to STRIPE_KEY in apphosting.yaml and apphosting.emulator.yaml respectively
5. Fixes a slight bug where we found a project root with X file and try to insert the variable in Y file (e.g. apphosting.local.yaml exists, but we're adding to apphosting.yaml)
6. Prints guidance on how to grant access to a secret in further commands

Author: inlined

CHANGELOG.md

@@ -1,2 +1,3 @@
 - Fix bug in Auth emulator where accounts:lookup is case-sensitive for emails (#8344)
 - firebase apphosting:secrets:grantAccess can now grant access to emails and can grant multiple secrets at once (#8357)
+- firebase apphosting:secrets:set now has flows to help with test secrets (#8359)

src/apphosting/config.spec.ts

@@ -22,37 +22,37 @@ describe("config", () => {
     });
 
     it("finds apphosting.yaml at cwd", () => {
-      fs.fileExistsSync.withArgs("/cwd/apphosting.yaml").returns(true);
-      expect(config.discoverBackendRoot("/cwd")).equals("/cwd");
+      fs.listFiles.withArgs("/parent/cwd").returns(["apphosting.yaml"]);
+      expect(config.discoverBackendRoot("/parent/cwd")).equals("/parent/cwd");
     });
 
     it("finds apphosting.yaml in a parent directory", () => {
-      fs.fileExistsSync.withArgs("/parent/cwd/apphosting.yaml").returns(false);
-      fs.fileExistsSync.withArgs("/parent/cwd/firebase.json").returns(false);
-      fs.fileExistsSync.withArgs("/parent/apphosting.yaml").returns(true);
+      fs.listFiles.withArgs("/parent/cwd").returns(["random_file.txt"]);
+      fs.listFiles.withArgs("/parent").returns(["apphosting.yaml"]);
 
       expect(config.discoverBackendRoot("/parent/cwd")).equals("/parent");
     });
 
     it("returns null if it finds firebase.json without finding apphosting.yaml", () => {
-      fs.fileExistsSync.withArgs("/parent/cwd/apphosting.yaml").returns(false);
-      fs.fileExistsSync.withArgs("/parent/cwd/firebase.json").returns(false);
-      fs.fileExistsSync.withArgs("/parent/apphosting.yaml").returns(false);
-      fs.fileExistsSync.withArgs("/parent/firebase.json").returns(true);
+      fs.listFiles.withArgs("/parent/cwd").returns([]);
+      fs.listFiles.withArgs("/parent").returns(["firebase.json"]);
 
       expect(config.discoverBackendRoot("/parent/cwd")).equals(null);
     });
 
     it("returns if it reaches the fs root", () => {
-      fs.fileExistsSync.withArgs("/parent/cwd/apphosting.yaml").returns(false);
-      fs.fileExistsSync.withArgs("/parent/cwd/firebase.json").returns(false);
-      fs.fileExistsSync.withArgs("/parent/apphosting.yaml").returns(false);
-      fs.fileExistsSync.withArgs("/parent/firebase.json").returns(false);
-      fs.fileExistsSync.withArgs("/apphosting.yaml").returns(false);
-      fs.fileExistsSync.withArgs("/firebase.json").returns(false);
+      fs.listFiles.withArgs("/parent/cwd").returns([]);
+      fs.listFiles.withArgs("/parent").returns(["random_file.txt"]);
+      fs.listFiles.withArgs("/").returns([]);
 
       expect(config.discoverBackendRoot("/parent/cwd")).equals(null);
     });
+
+    it("discovers backend root from any apphosting yaml file", () => {
+      fs.listFiles.withArgs("/parent/cwd").returns(["apphosting.staging.yaml"]);
+
+      expect(config.discoverBackendRoot("/parent/cwd")).equals("/parent/cwd");
+    });
   });
 
   describe("get/setEnv", () => {

src/apphosting/config.ts

@@ -7,7 +7,7 @@ import { NodeType } from "yaml/dist/nodes/Node";
 import * as prompt from "../prompt";
 import * as dialogs from "./secrets/dialogs";
 import { AppHostingYamlConfig } from "./yaml";
-import { FirebaseError } from "../error";
+import { FirebaseError, getError } from "../error";
 import { promptForAppHostingYaml } from "./utils";
 import { fetchSecrets } from "./secrets";
 import { logger } from "../logger";
@@ -66,9 +66,14 @@ const EXPORTABLE_CONFIG = [SECRET_CONFIG];
 export function discoverBackendRoot(cwd: string): string | null {
   let dir = cwd;
 
-  while (!fs.fileExistsSync(resolve(dir, APPHOSTING_BASE_YAML_FILE))) {
+  while (true) {
+    const files = fs.listFiles(dir);
+    if (files.some((file) => APPHOSTING_YAML_FILE_REGEX.test(file))) {
+      return dir;
+    }
+
     // We've hit project root
-    if (fs.fileExistsSync(resolve(dir, "firebase.json"))) {
+    if (files.includes("firebase.json")) {
       return null;
     }
 
@@ -79,8 +84,6 @@ export function discoverBackendRoot(cwd: string): string | null {
     }
     dir = parent;
   }
-
-  return dir;
 }
 
 /**
@@ -93,9 +96,23 @@ export function listAppHostingFilesInPath(path: string): string[] {
     .map((file) => join(path, file));
 }
 
-/** Load apphosting.yaml */
+/**
+ * Load an apphosting yaml file if it exists.
+ * Throws if the file exists but is malformed.
+ * Returns an empty document if the file does not exist.
+ */
 export function load(yamlPath: string): yaml.Document {
-  const raw = fs.readFile(yamlPath);
+  let raw: string;
+  try {
+    raw = fs.readFile(yamlPath);
+  } catch (err: any) {
+    if (err.code !== "ENOENT") {
+      throw new FirebaseError(`Unexpected error trying to load ${yamlPath}`, {
+        original: getError(err),
+      });
+    }
+    return new yaml.Document();
+  }
   return yaml.parseDocument(raw);
 }
 
@@ -140,13 +157,16 @@ export function upsertEnv(document: yaml.Document, env: Env): void {
 }
 
 /**
- * Given a secret name, guides the user whether they want to add that secret to apphosting.yaml.
- * If an apphosting.yaml exists and includes the secret already is used as a variable name, exist early.
- * If apphosting.yaml does not exist, offers to create it.
+ * Given a secret name, guides the user whether they want to add that secret to the specified apphosting yaml file.
+ * If an the file exists and includes the secret already is used as a variable name, exist early.
+ * If the file does not exist, offers to create it.
  * If env does not exist, offers to add it.
  * If secretName is not a valid env var name, prompts for an env var name.
  */
-export async function maybeAddSecretToYaml(secretName: string): Promise<void> {
+export async function maybeAddSecretToYaml(
+  secretName: string,
+  fileName: string = APPHOSTING_BASE_YAML_FILE,
+): Promise<void> {
   // We must go through the exports object for stubbing to work in tests.
   const dynamicDispatch = exports as {
     discoverBackendRoot: typeof discoverBackendRoot;
@@ -160,7 +180,7 @@ export async function maybeAddSecretToYaml(secretName: string): Promise<void> {
   let path: string | undefined;
   let projectYaml: yaml.Document;
   if (backendRoot) {
-    path = join(backendRoot, APPHOSTING_BASE_YAML_FILE);
+    path = join(backendRoot, fileName);
     projectYaml = dynamicDispatch.load(path);
   } else {
     projectYaml = new yaml.Document();
@@ -170,21 +190,23 @@ export async function maybeAddSecretToYaml(secretName: string): Promise<void> {
     return;
   }
   const addToYaml = await prompt.confirm({
-    message: "Would you like to add this secret to apphosting.yaml?",
+    message: `Would you like to add this secret to ${fileName}?`,
     default: true,
   });
   if (!addToYaml) {
     return;
   }
   if (!path) {
     path = await prompt.promptOnce({
-      message:
-        "It looks like you don't have an apphosting.yaml yet. Where would you like to store it?",
+      message: `It looks like you don't have an ${fileName} yet. Where would you like to store it?`,
       default: process.cwd(),
     });
-    path = join(path, APPHOSTING_BASE_YAML_FILE);
+    path = join(path, fileName);
   }
-  const envName = await dialogs.envVarForSecret(secretName);
+  const envName = await dialogs.envVarForSecret(
+    secretName,
+    /* trimTestPrefix= */ fileName === APPHOSTING_EMULATORS_YAML_FILE,
+  );
   dynamicDispatch.upsertEnv(projectYaml, {
     variable: envName,
     secret: secretName,

src/apphosting/secrets/dialogs.spec.ts

@@ -465,5 +465,17 @@ describe("dialogs", () => {
         /Key X_GOOGLE_SECRET starts with a reserved prefix/,
       );
     });
+
+    it("can trim test prefixes", async () => {
+      prompt.promptOnce.resolves("SECRET");
+
+      await expect(
+        dialogs.envVarForSecret("test-secret", /* trimTestPrefix=*/ true),
+      ).to.eventually.equal("SECRET");
+      expect(prompt.promptOnce).to.have.been.calledWithMatch({
+        message: "What environment variable name would you like to use?",
+        default: "SECRET",
+      });
+    });
   });
 });

src/apphosting/secrets/dialogs.ts

@@ -207,8 +207,14 @@ function toUpperSnakeCase(key: string): string {
     .toUpperCase();
 }
 
-export async function envVarForSecret(secret: string): Promise<string> {
-  const upper = toUpperSnakeCase(secret);
+export async function envVarForSecret(
+  secret: string,
+  trimTestPrefix: boolean = false,
+): Promise<string> {
+  let upper = toUpperSnakeCase(secret);
+  if (trimTestPrefix && upper.startsWith("TEST_")) {
+    upper = upper.substring("TEST_".length);
+  }
   if (upper === secret) {
     try {
       env.validateKey(secret);

src/commands/apphosting-secrets-set.ts

@@ -11,6 +11,7 @@ import * as secrets from "../apphosting/secrets";
 import * as dialogs from "../apphosting/secrets/dialogs";
 import * as config from "../apphosting/config";
 import * as utils from "../utils";
+import * as prompt from "../prompt";
 
 export const command = new Command("apphosting:secrets:set <secretName>")
   .description("create or update a secret for use in Firebase App Hosting")
@@ -61,6 +62,28 @@ export const command = new Command("apphosting:secrets:set <secretName>")
       return;
     }
 
+    const type = await prompt.promptOnce({
+      type: "list",
+      name: "type",
+      message: "Is this secret for production or only local testing?",
+      choices: [
+        { name: "Production", value: "production" },
+        { name: "Local testing only", value: "local" },
+      ],
+    });
+
+    if (type === "local") {
+      const emailList = await prompt.promptOnce({
+        type: "input",
+        name: "emails",
+        message:
+          "Please enter a comma separated list of user or groups who should have access to this secret:",
+      });
+      await secrets.grantEmailsSecretAccess(projectId, [secretName], emailList.split(","));
+      await config.maybeAddSecretToYaml(secretName, config.APPHOSTING_EMULATORS_YAML_FILE);
+      return;
+    }
+
     const accounts = await dialogs.selectBackendServiceAccounts(projectNumber, projectId, options);
 
     // If we're not granting permissions, there's no point in adding to YAML either.
@@ -74,5 +97,11 @@ export const command = new Command("apphosting:secrets:set <secretName>")
       await secrets.grantSecretAccess(projectId, projectNumber, secretName, accounts);
     }
 
-    await config.maybeAddSecretToYaml(secretName);
+    await config.maybeAddSecretToYaml(secretName, config.APPHOSTING_BASE_YAML_FILE);
+    utils.logBullet(
+      "To grant additional users access to this secret run " +
+        clc.bold(`firebase apphosting:secrets:grantaccess ${secretName} --email [email list]`) +
+        ".\nTo grant additional backends access to this secret run " +
+        clc.bold(`firebase apphosting:secrets:grantaccess ${secretName} --backend [backend ID]`),
+    );
   });