Remove --location from apphosting:rollouts:create and error when more than one backend is found. (#8271)

Author: annajowang

src/apphosting/backend.spec.ts

@@ -13,6 +13,7 @@ import {
   setDefaultTrafficPolicy,
   ensureAppHostingComputeServiceAccount,
   getBackendForAmbiguousLocation,
+  getBackend,
 } from "./backend";
 import * as deploymentTool from "../deploymentTool";
 import { FirebaseError } from "../error";
@@ -327,4 +328,56 @@ describe("apphosting setup functions", () => {
       });
     });
   });
+
+  describe("getBackend", () => {
+    const backendChickenAsia = {
+      name: `projects/${projectId}/locations/asia-east1/backends/chicken`,
+      labels: {},
+      createTime: "0",
+      updateTime: "1",
+      uri: "https://placeholder.com",
+    };
+
+    const backendChickenEurope = {
+      name: `projects/${projectId}/locations/europe-west4/backends/chicken`,
+      labels: {},
+      createTime: "0",
+      updateTime: "1",
+      uri: "https://placeholder.com",
+    };
+
+    const backendCow = {
+      name: `projects/${projectId}/locations/us-central1/backends/cow`,
+      labels: {},
+      createTime: "0",
+      updateTime: "1",
+      uri: "https://placeholder.com",
+    };
+
+    const allBackends = [backendChickenAsia, backendChickenEurope, backendCow];
+
+    it("throws if more than one backend is found", async () => {
+      listBackendsStub.resolves({ backends: allBackends });
+
+      await expect(getBackend(projectId, "chicken")).to.be.rejectedWith(
+        "You have multiple backends with the same chicken ID. " +
+          "This is not allowed until we can support more locations. " +
+          "Please delete and recreate any backends that share an ID with another backend.",
+      );
+    });
+
+    it("throws if no backend is found", async () => {
+      listBackendsStub.resolves({ backends: allBackends });
+
+      await expect(getBackend(projectId, "farmer")).to.be.rejectedWith(
+        "No backend named farmer found.",
+      );
+    });
+
+    it("returns backend", async () => {
+      listBackendsStub.resolves({ backends: allBackends });
+
+      await expect(getBackend(projectId, "cow")).to.eventually.equal(backendCow);
+    });
+  });
 });

src/apphosting/backend.ts

@@ -443,7 +443,7 @@ export async function getBackendForAmbiguousLocation(
   let { unreachable, backends } = await apphosting.listBackends(projectId, "-");
   if (unreachable && unreachable.length !== 0) {
     logWarning(
-      `The following locations are currently unreachable: ${unreachable}.\n` +
+      `The following locations are currently unreachable: ${unreachable.join(", ")}.\n` +
         "If your backend is in one of these regions, please try again later.",
     );
   }
@@ -474,3 +474,33 @@ export async function getBackendForAmbiguousLocation(
   });
   return backendsByLocation.get(location)!;
 }
+
+/**
+ * Fetches a backend from the server. If there are multiple backends with the name, it will throw an error
+ * telling the user that there are other backends with the same name that need to be deleted.
+ */
+export async function getBackend(
+  projectId: string,
+  backendId: string,
+): Promise<apphosting.Backend> {
+  let { unreachable, backends } = await apphosting.listBackends(projectId, "-");
+  backends = backends.filter(
+    (backend) => apphosting.parseBackendName(backend.name).id === backendId,
+  );
+  if (backends.length > 1) {
+    throw new FirebaseError(
+      `You have multiple backends with the same ${backendId} ID. This is not allowed until we can support more locations. ` +
+        "Please delete and recreate any backends that share an ID with another backend.",
+    );
+  }
+  if (backends.length === 1) {
+    return backends[0];
+  }
+  if (unreachable && unreachable.length !== 0) {
+    logWarning(
+      `Backends with the following primary regions are unreachable: ${unreachable.join(", ")}.\n` +
+        "If your backend is in one of these regions, please try again later.",
+    );
+  }
+  throw new FirebaseError(`No backend named ${backendId} found.`);
+}

src/apphosting/rollout.spec.ts

@@ -22,8 +22,7 @@ describe("apphosting rollouts", () => {
   const gitRepoLinkId = `${user}-${repo}`;
   const buildAndRolloutId = "build-2024-10-01-001";
 
-  let getBackendForLocationStub: sinon.SinonStub;
-  let getBackendForAmbiguousLocationStub: sinon.SinonStub;
+  let getBackend: sinon.SinonStub;
   let getRepoDetailsFromBackendStub: sinon.SinonStub;
   let listAllBranchesStub: sinon.SinonStub;
   let getGitHubBranchStub: sinon.SinonStub;
@@ -36,12 +35,7 @@ describe("apphosting rollouts", () => {
   let sleepStub: sinon.SinonStub;
 
   beforeEach(() => {
-    getBackendForLocationStub = sinon
-      .stub(backend, "getBackendForLocation")
-      .throws("unexpected getBackendForLocation call");
-    getBackendForAmbiguousLocationStub = sinon
-      .stub(backend, "getBackendForAmbiguousLocation")
-      .throws("unexpected getBackendForAmbiguousLocation call");
+    getBackend = sinon.stub(backend, "getBackend").throws("unexpected getBackend call");
     getRepoDetailsFromBackendStub = sinon
       .stub(devConnect, "getRepoDetailsFromBackend")
       .throws("unexpected getRepoDetailsFromBackend call");
@@ -149,8 +143,7 @@ describe("apphosting rollouts", () => {
 
     describe("createRollout", () => {
       it("should create a new rollout from user-specified branch", async () => {
-        getBackendForLocationStub.resolves(backend);
-        getBackendForAmbiguousLocationStub.resolves(backend);
+        getBackend.resolves(backend);
         getRepoDetailsFromBackendStub.resolves(repoLinkDetails);
         listAllBranchesStub.resolves(branches);
         getGitHubBranchStub.resolves(branchInfo);
@@ -160,16 +153,15 @@ describe("apphosting rollouts", () => {
         pollOperationStub.onFirstCall().resolves(rollout);
         pollOperationStub.onSecondCall().resolves(build);
 
-        await createRollout(backendId, projectId, location, branchId, undefined, true);
+        await createRollout(backendId, projectId, branchId, undefined, true);
 
         expect(createBuildStub).to.be.called;
         expect(createRolloutStub).to.be.called;
         expect(pollOperationStub).to.be.called;
       });
 
       it("should create a new rollout from user-specified commit", async () => {
-        getBackendForLocationStub.resolves(backend);
-        getBackendForAmbiguousLocationStub.resolves(backend);
+        getBackend.resolves(backend);
         getRepoDetailsFromBackendStub.resolves(repoLinkDetails);
         getGitHubCommitStub.resolves(commitInfo);
         getNextRolloutIdStub.resolves(buildAndRolloutId);
@@ -178,16 +170,15 @@ describe("apphosting rollouts", () => {
         pollOperationStub.onFirstCall().resolves(rollout);
         pollOperationStub.onSecondCall().resolves(build);
 
-        await createRollout(backendId, projectId, location, undefined, commitSha, true);
+        await createRollout(backendId, projectId, undefined, commitSha, true);
 
         expect(createBuildStub).to.be.called;
         expect(createRolloutStub).to.be.called;
         expect(pollOperationStub).to.be.called;
       });
 
       it("should prompt user for a branch if branch or commit ID is not specified", async () => {
-        getBackendForLocationStub.resolves(backend);
-        getBackendForAmbiguousLocationStub.resolves(backend);
+        getBackend.resolves(backend);
         getRepoDetailsFromBackendStub.resolves(repoLinkDetails);
         promptGitHubBranchStub.resolves(branchId);
         getGitHubBranchStub.resolves(branchInfo);
@@ -197,7 +188,7 @@ describe("apphosting rollouts", () => {
         pollOperationStub.onFirstCall().resolves(rollout);
         pollOperationStub.onSecondCall().resolves(build);
 
-        await createRollout(backendId, projectId, location, undefined, undefined, false);
+        await createRollout(backendId, projectId, undefined, undefined, false);
 
         expect(promptGitHubBranchStub).to.be.called;
         expect(createBuildStub).to.be.called;
@@ -206,33 +197,31 @@ describe("apphosting rollouts", () => {
       });
 
       it("should throw an error if GitHub branch is not found", async () => {
-        getBackendForLocationStub.resolves(backend);
-        getBackendForAmbiguousLocationStub.resolves(backend);
+        getBackend.resolves(backend);
         getRepoDetailsFromBackendStub.resolves(repoLinkDetails);
         listAllBranchesStub.resolves(branches);
 
         await expect(
-          createRollout(backendId, projectId, location, "invalid-branch", undefined, true),
+          createRollout(backendId, projectId, "invalid-branch", undefined, true),
         ).to.be.rejectedWith(/Unrecognized git branch/);
       });
 
       it("should throw an error if GitHub commit is not found", async () => {
-        getBackendForLocationStub.resolves(backend);
-        getBackendForAmbiguousLocationStub.resolves(backend);
+        getBackend.resolves(backend);
         getRepoDetailsFromBackendStub.resolves(repoLinkDetails);
         getGitHubCommitStub.rejects(new FirebaseError("error", { status: 422 }));
 
         await expect(
-          createRollout(backendId, projectId, location, undefined, commitSha, true),
+          createRollout(backendId, projectId, undefined, commitSha, true),
         ).to.be.rejectedWith(/Unrecognized git commit/);
       });
 
       it("should throw an error if --force flag is specified but --git-branch and --git-commit are missing", async () => {
-        getBackendForLocationStub.resolves(backend);
+        getBackend.resolves(backend);
         getRepoDetailsFromBackendStub.resolves(repoLinkDetails);
 
         await expect(
-          createRollout(backendId, projectId, location, undefined, undefined, true),
+          createRollout(backendId, projectId, undefined, undefined, true),
         ).to.be.rejectedWith(/Failed to create rollout with --force option/);
       });
     });

src/apphosting/rollout.ts

@@ -13,7 +13,7 @@ import * as poller from "../operation-poller";
 import { logBullet, sleep } from "../utils";
 import { apphostingOrigin, consoleOrigin } from "../api";
 import { DeepOmit } from "../metaprogramming";
-import { getBackendForAmbiguousLocation, getBackendForLocation } from "./backend";
+import { getBackend } from "./backend";
 
 const apphostingPollerOptions: Omit<poller.OperationPollerOptions, "operationResourceName"> = {
   apiOrigin: apphostingOrigin(),
@@ -31,29 +31,19 @@ const GIT_COMMIT_SHA_REGEX = /^(?:[0-9a-f]{40}|[0-9a-f]{7})$/;
 export async function createRollout(
   backendId: string,
   projectId: string,
-  location: string,
   branch?: string,
   commit?: string,
   force?: boolean,
 ): Promise<void> {
-  let backend: apphosting.Backend;
-  if (location === "-" || location === "") {
-    backend = await getBackendForAmbiguousLocation(
-      projectId,
-      backendId,
-      "Please select the location of the backend you'd like to roll out:",
-      force,
-    );
-    location = apphosting.parseBackendName(backend.name).location;
-  } else {
-    backend = await getBackendForLocation(projectId, location, backendId);
-  }
+  const backend = await getBackend(projectId, backendId);
 
   if (!backend.codebase.repository) {
     throw new FirebaseError(
       `Backend ${backendId} is misconfigured due to missing a connected repository. You can delete and recreate your backend using 'firebase apphosting:backends:delete' and 'firebase apphosting:backends:create'.`,
     );
   }
+
+  const { location } = apphosting.parseBackendName(backend.name);
   const { repoLink, owner, repo, readToken } = await getRepoDetailsFromBackend(
     projectId,
     location,

src/commands/apphosting-rollouts-create.ts

@@ -4,11 +4,9 @@ import { Options } from "../options";
 import { needProjectId } from "../projectUtils";
 import { FirebaseError } from "../error";
 import { createRollout } from "../apphosting/rollout";
-import { logWarning } from "../utils";
 
 export const command = new Command("apphosting:rollouts:create <backendId>")
   .description("create a rollout using a build for an App Hosting backend")
-  .option("-l, --location <location>", "specify the region of the backend")
   .option(
     "-b, --git-branch <gitBranch>",
     "repository branch to deploy (mutually exclusive with -g)",
@@ -18,10 +16,6 @@ export const command = new Command("apphosting:rollouts:create <backendId>")
   .before(apphosting.ensureApiEnabled)
   .action(async (backendId: string, options: Options) => {
     const projectId = needProjectId(options);
-    if (options.location !== undefined) {
-      logWarning("--location is being removed in the next major release.");
-    }
-    const location = (options.location as string) ?? "-";
 
     const branch = options.gitBranch as string | undefined;
     const commit = options.gitCommit as string | undefined;
@@ -31,5 +25,5 @@ export const command = new Command("apphosting:rollouts:create <backendId>")
       );
     }
 
-    await createRollout(backendId, projectId, location, branch, commit, options.force);
+    await createRollout(backendId, projectId, branch, commit, options.force);
   });