Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove --location from apphosting:backends:create and prompt for primary region #8208

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 34 additions & 18 deletions src/apphosting/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
// SSL.
const maybeNodeError = err as { cause: { code: string }; code: string };
if (
/HANDSHAKE_FAILURE/.test(maybeNodeError?.cause?.code) ||

Check warning on line 51 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Use `String#includes()` method with a string instead
"EPROTO" === maybeNodeError?.code
) {
return false;
Expand All @@ -73,7 +73,6 @@
export async function doSetup(
projectId: string,
webAppName: string | null,
location: string | null,
serviceAccount: string | null,
): Promise<void> {
await Promise.all([
Expand All @@ -89,21 +88,11 @@
// possible to reduce the likelihood that the subsequent Cloud Build fails. See b/336862200.
await ensureAppHostingComputeServiceAccount(projectId, serviceAccount);

const allowedLocations = (await apphosting.listLocations(projectId)).map((loc) => loc.locationId);
if (location) {
if (!allowedLocations.includes(location)) {
throw new FirebaseError(
`Invalid location ${location}. Valid choices are ${allowedLocations.join(", ")}`,
);
}
}

location =
location || (await promptLocation(projectId, "Select a location to host your backend:\n"));
const primaryRegion = await promptPrimaryRegion(projectId, "Select a primary region to host your backend:\n");

Check failure on line 91 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Replace `projectId,·"Select·a·primary·region·to·host·your·backend:\n"` with `⏎····projectId,⏎····"Select·a·primary·region·to·host·your·backend:\n",⏎··`

Check failure on line 91 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / unit (18)

Replace `projectId,·"Select·a·primary·region·to·host·your·backend:\n"` with `⏎····projectId,⏎····"Select·a·primary·region·to·host·your·backend:\n",⏎··`

Check failure on line 91 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / unit (22)

Replace `projectId,·"Select·a·primary·region·to·host·your·backend:\n"` with `⏎····projectId,⏎····"Select·a·primary·region·to·host·your·backend:\n",⏎··`

Check failure on line 91 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / unit (18)

Replace `projectId,·"Select·a·primary·region·to·host·your·backend:\n"` with `⏎····projectId,⏎····"Select·a·primary·region·to·host·your·backend:\n",⏎··`

const gitRepositoryLink: GitRepositoryLink = await githubConnections.linkGitHubRepository(
projectId,
location,
primaryRegion,
);

const rootDir = await promptOnce({
Expand All @@ -120,7 +109,7 @@
logSuccess(`Repo linked successfully!\n`);

logBullet(`${clc.yellow("===")} Set up your backend`);
const backendId = await promptNewBackendId(projectId, location, {
const backendId = await promptNewBackendId(projectId, primaryRegion, {
name: "backendId",
type: "input",
default: "my-web-app",
Expand All @@ -136,7 +125,7 @@
const createBackendSpinner = ora("Creating your new backend...").start();
const backend = await createBackend(
projectId,
location,
primaryRegion,
backendId,
gitRepositoryLink,
serviceAccount,
Expand All @@ -145,7 +134,7 @@
);
createBackendSpinner.succeed(`Successfully created backend!\n\t${backend.name}\n`);

await setDefaultTrafficPolicy(projectId, location, backendId, branch);
await setDefaultTrafficPolicy(projectId, primaryRegion, backendId, branch);

const confirmRollout = await promptOnce({
type: "confirm",
Expand All @@ -168,6 +157,8 @@
const createRolloutSpinner = ora(
"Starting a new rollout; this may take a few minutes. It's safe to exit now.",
).start();
// TODO: remove this renaming when orchestrateRolloutArgs have been updated.
const location = primaryRegion

Check failure on line 161 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Insert `;`

Check failure on line 161 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / unit (18)

Insert `;`

Check failure on line 161 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / unit (22)

Insert `;`

Check failure on line 161 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / unit (18)

Insert `;`
await orchestrateRollout({
projectId,
location,
Expand Down Expand Up @@ -263,10 +254,10 @@
async function promptNewBackendId(
projectId: string,
location: string,
prompt: any,

Check warning on line 257 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
): Promise<string> {
while (true) {

Check warning on line 259 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected constant condition
const backendId = await promptOnce(prompt);

Check warning on line 260 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe argument of type `any` assigned to a parameter of type `QuestionsThatReturnAString<Answers>`
try {
await apphosting.getBackend(projectId, location, backendId);
} catch (err: unknown) {
Expand Down Expand Up @@ -387,12 +378,37 @@
});
}

/**
* Prompts the user for a location. If there's only a single valid location, skips the prompt and returns that location.
*/
export async function promptPrimaryRegion(
projectId: string,
prompt = "Please select a primary region:",
): Promise<string> {
const allowedRegions = (await apphosting.listLocations(projectId)).map((loc) => loc.locationId);
if (allowedRegions.length === 1) {
return allowedRegions[0];
}

const primaryRegion = (await promptOnce({
name: "primaryRegion",
type: "list",
default: DEFAULT_LOCATION,
message: prompt,
choices: allowedRegions,
})) as string;

logSuccess(`Primary Region set to ${primaryRegion}.\n`);

return primaryRegion;
}

/**
* Prompts the user for a location. If there's only a single valid location, skips the prompt and returns that location.
*/
export async function promptLocation(
projectId: string,
prompt = "Please select a location:",
prompt = "Please select a primary region:",
): Promise<string> {
const allowedLocations = (await apphosting.listLocations(projectId)).map((loc) => loc.locationId);
if (allowedLocations.length === 1) {
Expand All @@ -407,7 +423,7 @@
choices: allowedLocations,
})) as string;

logSuccess(`Location set to ${location}.\n`);
logSuccess(`Primary Region set to ${location}.\n`);

return location;
}
Expand Down Expand Up @@ -443,7 +459,7 @@
let { unreachable, backends } = await apphosting.listBackends(projectId, "-");
if (unreachable && unreachable.length !== 0) {
logWarning(
`The following locations are currently unreachable: ${unreachable}.\n` +

Check warning on line 462 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Invalid type "string[]" of template literal expression
"If your backend is in one of these regions, please try again later.",
);
}
Expand All @@ -466,11 +482,11 @@
backends.forEach((backend) =>
backendsByLocation.set(apphosting.parseBackendName(backend.name).location, backend),
);
const location = await promptOnce({

Check warning on line 485 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
name: "location",
type: "list",
message: locationDisambugationPrompt,
choices: [...backendsByLocation.keys()],
});
return backendsByLocation.get(location)!;

Check warning on line 491 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Forbidden non-null assertion

Check warning on line 491 in src/apphosting/backend.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe argument of type `any` assigned to a parameter of type `string`
}
3 changes: 0 additions & 3 deletions src/commands/apphosting-backends-create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"-a, --app <webAppId>",
"specify an existing Firebase web app's ID to associate your App Hosting backend with",
)
.option("-l, --location <location>", "specify the location of the backend", "")
.option(
"-s, --service-account <serviceAccount>",
"specify the service account used to run the server",
Expand All @@ -25,13 +24,11 @@
.action(async (options: Options) => {
const projectId = needProjectId(options);
const webAppId = options.app;
const location = options.location;
const serviceAccount = options.serviceAccount;

await doSetup(

Check failure on line 29 in src/commands/apphosting-backends-create.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Replace `⏎······projectId,⏎······webAppId·as·string·|·null,⏎······serviceAccount·as·string·|·null,⏎····` with `projectId,·webAppId·as·string·|·null,·serviceAccount·as·string·|·null`

Check failure on line 29 in src/commands/apphosting-backends-create.ts

View workflow job for this annotation

GitHub Actions / unit (18)

Replace `⏎······projectId,⏎······webAppId·as·string·|·null,⏎······serviceAccount·as·string·|·null,⏎····` with `projectId,·webAppId·as·string·|·null,·serviceAccount·as·string·|·null`

Check failure on line 29 in src/commands/apphosting-backends-create.ts

View workflow job for this annotation

GitHub Actions / unit (22)

Replace `⏎······projectId,⏎······webAppId·as·string·|·null,⏎······serviceAccount·as·string·|·null,⏎····` with `projectId,·webAppId·as·string·|·null,·serviceAccount·as·string·|·null`

Check failure on line 29 in src/commands/apphosting-backends-create.ts

View workflow job for this annotation

GitHub Actions / unit (18)

Replace `⏎······projectId,⏎······webAppId·as·string·|·null,⏎······serviceAccount·as·string·|·null,⏎····` with `projectId,·webAppId·as·string·|·null,·serviceAccount·as·string·|·null`
projectId,
webAppId as string | null,
location as string | null,
serviceAccount as string | null,
);
});
2 changes: 1 addition & 1 deletion src/commands/apphosting-backends-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Options } from "../options";
import * as apphosting from "../gcp/apphosting";
import * as Table from "cli-table3";

const TABLE_HEAD = ["Backend", "Repository", "URL", "Location", "Updated Date"];
const TABLE_HEAD = ["Backend", "Repository", "URL", "Primary Region", "Updated Date"];

export const command = new Command("apphosting:backends:list")
.description("list Firebase App Hosting backends")
Expand Down
Loading