Skip location prompt if backend is unambiguous for apphosting:secrets:grantaccess

src/apphosting/index.ts

@@ -191,19 +191,19 @@
 async function promptNewBackendId(
   projectId: string,
   location: string,
   prompt: any,
 ): Promise<string> {
   while (true) {
     const backendId = await promptOnce(prompt);
     try {
       await apphosting.getBackend(projectId, location, backendId);
     } catch (err: any) {
       if (err.status === 404) {
         return backendId;
       }
       throw new FirebaseError(
         `Failed to check if backend with id ${backendId} already exists in ${location}`,
         { original: err },
       );
     }
     logWarning(`Backend with id ${backendId} already exists in ${location}`);
@@ -259,9 +259,9 @@
       "Default service account used to run builds and deploys for Firebase App Hosting",
       "Firebase App Hosting compute service account",
     );
   } catch (err: any) {
     // 409 Already Exists errors can safely be ignored.
     if (err.status !== 409) {
       throw err;
     }
   }
@@ -410,7 +410,7 @@
  */
 export async function promptLocation(
   projectId: string,
-  prompt = "Please select a location:",
+  prompt: string = "Please select a location:",
 ): Promise<string> {
   const allowedLocations = (await apphosting.listLocations(projectId)).map((loc) => loc.locationId);
 
@@ -422,3 +422,42 @@
     choices: allowedLocations,
   })) as string;
 }
+
+/**
+ * Fetches a backend from the server. If there are multiple backends with that name (ie multi-regional backends),
+ * prompts the user to disambiguate.
+ */
+export async function getBackendForAmbiguousLocation(
+  projectId: string,
+  backendId: string,
+  locationDisambugationPrompt: string,
+): Promise<apphosting.Backend> {
+  let { unreachable, backends } = await apphosting.listBackends(projectId, "-");
+  if (unreachable && unreachable.length !== 0) {
+    logWarning(
+      `The following locations are currently unreachable: ${unreachable}.\n` +
+        "If your backend is in one of these regions, please try again later.",
+    );
+  }
+  backends = backends.filter(
+    (backend) => apphosting.parseBackendName(backend.name).id === backendId,
+  );
+  if (backends.length === 0) {
+    throw new FirebaseError(`No backend named "${backendId}" found.`);
+  }
+  if (backends.length === 1) {
+    return backends[0];
+  }
+
+  const backendsByLocation = new Map<string, apphosting.Backend>();
+  backends.forEach((backend) =>
+    backendsByLocation.set(apphosting.parseBackendName(backend.name).location, backend),
+  );
+  const location = await promptOnce({
+    name: "location",
+    type: "list",
+    message: locationDisambugationPrompt,
+    choices: [...backendsByLocation.keys()],
+  });
+  return backendsByLocation.get(location)!;
+}

src/commands/apphosting-backends-delete.ts

@@ -6,36 +6,31 @@ import { promptOnce } from "../prompt";
 import * as utils from "../utils";
 import * as apphosting from "../gcp/apphosting";
 import { printBackendsTable } from "./apphosting-backends-list";
-import { deleteBackendAndPoll, promptLocation } from "../apphosting";
+import { deleteBackendAndPoll, getBackendForAmbiguousLocation } from "../apphosting";
+import * as ora from "ora";
 
 export const command = new Command("apphosting:backends:delete <backend>")
   .description("delete a Firebase App Hosting backend")
-  .option("-l, --location <location>", "specify the location of the backend", "")
+  .option("-l, --location <location>", "specify the location of the backend", "-")
   .withForce()
   .before(apphosting.ensureApiEnabled)
   .action(async (backendId: string, options: Options) => {
     const projectId = needProjectId(options);
     let location = options.location as string;
-
-    location =
-      location ||
-      (await promptLocation(
+    let backend: apphosting.Backend;
+    if (location === "-" || location === "") {
+      backend = await getBackendForAmbiguousLocation(
         projectId,
+        backendId,
         "Please select the location of the backend you'd like to delete:",
-      ));
-
-    let backend: apphosting.Backend;
-    try {
-      backend = await apphosting.getBackend(projectId, location, backendId);
-    } catch (err: any) {
-      throw new FirebaseError(`No backends found with given parameters. Command aborted.`, {
-        original: err,
-      });
+      );
+      location = apphosting.parseBackendName(backend.name).location;
+    } else {
+      backend = await getBackendForLocation(projectId, location, backendId);
     }
 
-    utils.logWarning("You are about to permanently delete the backend:");
-    const backends: apphosting.Backend[] = [backend];
-    printBackendsTable(backends);
+    utils.logWarning("You are about to permanently delete this backend:");
+    printBackendsTable([backend]);
 
     const confirmDeletion = await promptOnce(
       {
@@ -47,18 +42,29 @@ export const command = new Command("apphosting:backends:delete <backend>")
       options,
     );
     if (!confirmDeletion) {
-      throw new FirebaseError("Deletion Aborted");
+      return;
     }
 
+    const spinner = ora("Deleting backend...").start();
     try {
       await deleteBackendAndPoll(projectId, location, backendId);
-      utils.logSuccess(`Successfully deleted the backend: ${backendId}`);
+      spinner.succeed(`Successfully deleted the backend: ${backendId}`);
     } catch (err: any) {
-      throw new FirebaseError(
-        `Failed to delete backend: ${backendId}. Please check the parameters you have provided.`,
-        { original: err },
-      );
+      spinner.stop();
+      throw new FirebaseError(`Failed to delete backend: ${backendId}.`, { original: err });
     }
-
-    return backend;
   });
+
+async function getBackendForLocation(
+  projectId: string,
+  location: string,
+  backendId: string,
+): Promise<apphosting.Backend> {
+  try {
+    return await apphosting.getBackend(projectId, location, backendId);
+  } catch (err: any) {
+    throw new FirebaseError(`No backend named "${backendId}" found in ${location}.`, {
+      original: err,
+    });
+  }
+}

src/commands/apphosting-backends-list.ts

@@ -42,14 +42,13 @@ export function printBackendsTable(backends: apphosting.Backend[]): void {
   });
 
   for (const backend of backends) {
-    // sample backend.name value: "projects/<project-name>/locations/us-central1/backends/<backend-id>"
-    const [backendLocation, , backendId] = backend.name.split("/").slice(3, 6);
+    const { location, id } = apphosting.parseBackendName(backend.name);
     table.push([
-      backendId,
+      id,
       // sample repository value: "projects/<project-name>/locations/us-central1/connections/<connection-id>/repositories/<repository-name>"
       backend.codebase?.repository?.split("/").pop() ?? "",
       backend.uri.startsWith("https:") ? backend.uri : "https://" + backend.uri,
-      backendLocation,
+      location,
       datetimeString(new Date(backend.updateTime)),
     ]);
   }

src/commands/apphosting-secrets-grantaccess.ts

@@ -7,11 +7,11 @@ import * as secretManager from "../gcp/secretManager";
 import { requirePermissions } from "../requirePermissions";
 import * as apphosting from "../gcp/apphosting";
 import * as secrets from "../apphosting/secrets";
-import { promptLocation } from "../apphosting";
+import { getBackendForAmbiguousLocation } from "../apphosting";
 
 export const command = new Command("apphosting:secrets:grantaccess <secretName>")
   .description("grant service accounts permissions to the provided secret")
-  .option("-l, --location <location>", "backend location")
+  .option("-l, --location <location>", "backend location", "-")
   .option("-b, --backend <backend>", "backend name")
   .before(requireAuth)
   .before(secretManager.ensureApi)
@@ -34,20 +34,24 @@ export const command = new Command("apphosting:secrets:grantaccess <secretName>"
       );
     }
 
-    let location = options.location as string;
-
-    location =
-      location || (await promptLocation(projectId, "Please select the location of your backend"));
-
-    // TODO: consider showing dialog if --backend is missing
-
     const exists = await secretManager.secretExists(projectId, secretName);
     if (!exists) {
       throw new FirebaseError(`Cannot find secret ${secretName}`);
     }
 
     const backendId = options.backend as string;
-    const backend = await apphosting.getBackend(projectId, location, backendId);
+    const location = options.location as string;
+    let backend: apphosting.Backend;
+    if (location === "" || location === "-") {
+      backend = await getBackendForAmbiguousLocation(
+        projectId,
+        backendId,
+        "Please select the location of your backend:",
+      );
+    } else {
+      backend = await apphosting.getBackend(projectId, location, backendId);
+    }
+
     const accounts = secrets.toMulti(secrets.serviceAccountsForBackend(projectNumber, backend));
 
     await secrets.grantSecretAccess(projectId, projectNumber, secretName, accounts);

src/gcp/apphosting.ts

@@ -273,6 +273,17 @@ export function serviceAgentEmail(projectNumber: string): string {
   return `service-${projectNumber}@${P4SA_DOMAIN}`;
 }
 
+/** Splits a backend resource name into its parts. */
+export function parseBackendName(backendName: string): {
+  projectName: string;
+  location: string;
+  id: string;
+} {
+  // sample value: "projects/<project-name>/locations/us-central1/backends/<backend-id>"
+  const [, projectName, , location, , id] = backendName.split("/");
+  return { projectName, location, id };
+}
+
 /**
  * Creates a new Backend in a given project and location.
  */

src/test/apphosting/index.spec.ts

@@ -12,6 +12,7 @@ import {
   promptLocation,
   setDefaultTrafficPolicy,
   ensureAppHostingComputeServiceAccount,
+  getBackendForAmbiguousLocation,
 } from "../../apphosting/index";
 import * as deploymentTool from "../../deploymentTool";
 import { FirebaseError } from "../../error";
@@ -24,6 +25,7 @@ describe("apphosting setup functions", () => {
   let promptOnceStub: sinon.SinonStub;
   let pollOperationStub: sinon.SinonStub;
   let createBackendStub: sinon.SinonStub;
+  let listBackendsStub: sinon.SinonStub;
   let deleteBackendStub: sinon.SinonStub;
   let updateTrafficStub: sinon.SinonStub;
   let listLocationsStub: sinon.SinonStub;
@@ -37,6 +39,9 @@ describe("apphosting setup functions", () => {
     createBackendStub = sinon
       .stub(apphosting, "createBackend")
       .throws("Unexpected createBackend call");
+    listBackendsStub = sinon
+      .stub(apphosting, "listBackends")
+      .throws("Unexpected listBackends call");
     deleteBackendStub = sinon
       .stub(apphosting, "deleteBackend")
       .throws("Unexpected deleteBackend call");
@@ -222,7 +227,7 @@ describe("apphosting setup functions", () => {
     });
 
     it("returns a location selection", async () => {
-      const location = await promptLocation(projectId);
+      const location = await promptLocation(projectId, /* prompt= */ "");
       expect(location).to.be.eq("us-central1");
     });
 
@@ -250,4 +255,66 @@ describe("apphosting setup functions", () => {
       });
     });
   });
+
+  describe("getBackendForAmbiguousLocation", () => {
+    const backendFoo = {
+      name: `projects/${projectId}/locations/${location}/backends/foo`,
+      labels: {},
+      createTime: "0",
+      updateTime: "1",
+      uri: "https://placeholder.com",
+    };
+
+    const backendFooOtherRegion = {
+      name: `projects/${projectId}/locations/otherRegion/backends/foo`,
+      labels: {},
+      createTime: "0",
+      updateTime: "1",
+      uri: "https://placeholder.com",
+    };
+
+    const backendBar = {
+      name: `projects/${projectId}/locations/${location}/backends/bar`,
+      labels: {},
+      createTime: "0",
+      updateTime: "1",
+      uri: "https://placeholder.com",
+    };
+
+    it("throws if there are no matching backends", async () => {
+      listBackendsStub.resolves({ backends: [] });
+
+      await expect(
+        getBackendForAmbiguousLocation(projectId, "baz", /* prompt= */ ""),
+      ).to.be.rejectedWith(/No backend named "baz" found./);
+    });
+
+    it("returns unambiguous backend", async () => {
+      listBackendsStub.resolves({ backends: [backendFoo, backendBar] });
+
+      await expect(
+        getBackendForAmbiguousLocation(projectId, "foo", /* prompt= */ ""),
+      ).to.eventually.equal(backendFoo);
+    });
+
+    it("prompts for location if backend is ambiguous", async () => {
+      listBackendsStub.resolves({ backends: [backendFoo, backendFooOtherRegion, backendBar] });
+      promptOnceStub.resolves(location);
+
+      await expect(
+        getBackendForAmbiguousLocation(
+          projectId,
+          "foo",
+          "Please select the location of the backend you'd like to delete:",
+        ),
+      ).to.eventually.equal(backendFoo);
+
+      expect(promptOnceStub).to.be.calledWith({
+        name: "location",
+        type: "list",
+        message: "Please select the location of the backend you'd like to delete:",
+        choices: [location, "otherRegion"],
+      });
+    });
+  });
 });