diff --git a/src/apphosting/backend.spec.ts b/src/apphosting/backend.spec.ts index eba297efd97..54ff08f0bac 100644 --- a/src/apphosting/backend.spec.ts +++ b/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); + }); + }); }); diff --git a/src/apphosting/backend.ts b/src/apphosting/backend.ts index 8a5aa579901..42491062399 100644 --- a/src/apphosting/backend.ts +++ b/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 { + 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.`); +} diff --git a/src/apphosting/rollout.spec.ts b/src/apphosting/rollout.spec.ts index 8b943d1bfb5..3646c80c07e 100644 --- a/src/apphosting/rollout.spec.ts +++ b/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,7 +153,7 @@ 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; @@ -168,8 +161,7 @@ describe("apphosting rollouts", () => { }); 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,7 +170,7 @@ 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; @@ -186,8 +178,7 @@ describe("apphosting rollouts", () => { }); 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/); }); }); diff --git a/src/apphosting/rollout.ts b/src/apphosting/rollout.ts index 32b2bcb8d4e..b27cd02c344 100644 --- a/src/apphosting/rollout.ts +++ b/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 = { 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 { - 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, diff --git a/src/commands/apphosting-rollouts-create.ts b/src/commands/apphosting-rollouts-create.ts index ab03b13464b..a5761351dc8 100644 --- a/src/commands/apphosting-rollouts-create.ts +++ b/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 ") .description("create a rollout using a build for an App Hosting backend") - .option("-l, --location ", "specify the region of the backend") .option( "-b, --git-branch ", "repository branch to deploy (mutually exclusive with -g)", @@ -18,10 +16,6 @@ export const command = new Command("apphosting:rollouts:create ") .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 ") ); } - await createRollout(backendId, projectId, location, branch, commit, options.force); + await createRollout(backendId, projectId, branch, commit, options.force); });