Skip to content

Provision App Hosting compute service account during init flow #8580

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.

Sign up for GitHub

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

Merged
blidd-google merged 6 commits into from
May 15, 2025
Merged

Provision App Hosting compute service account during init flow #8580

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
93ec712
provision compute SA in fah init flow
blidd-google May 14, 2025
2460769
changelog
blidd-google May 14, 2025
2dc8702
Merge branch 'master' into bl-fah-init-revised
blidd-google May 14, 2025
0242d9f
remove SA role comment
blidd-google May 14, 2025
e456d81
Merge branch 'master' into bl-fah-init-revised
blidd-google May 15, 2025
0337933
Merge branch 'master' into bl-fah-init-revised
blidd-google May 15, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
- Fixed issue where `firebase init hosting:github` didn't correctly parse the repo input. (#8536)
- Add GCP API client functions to support App Hosting deploy from source feature. (#8545)
- Fix an issue where updating a Cloud Function that retires would add incorrect fields to the updateMask. (#8560)
- Provision App Hosting compute service account during init flow. (#8580)
23 changes: 15 additions & 8 deletions src/apphosting/backend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,7 @@ export async function doSetup(
webAppName: string | null,
serviceAccount: string | null,
): Promise<void> {
await Promise.all([
ensure(projectId, developerConnectOrigin(), "apphosting", true),
ensure(projectId, cloudbuildOrigin(), "apphosting", true),
ensure(projectId, secretManagerOrigin(), "apphosting", true),
ensure(projectId, cloudRunApiOrigin(), "apphosting", true),
ensure(projectId, artifactRegistryDomain(), "apphosting", true),
ensure(projectId, iamOrigin(), "apphosting", true),
]);
await ensureRequiredApisEnabled(projectId);

// Hack: Because IAM can take ~45 seconds to propagate, we provision the service account as soon as
// possible to reduce the likelihood that the subsequent Cloud Build fails. See b/336862200.
Expand Down Expand Up @@ -179,6 +172,20 @@ export async function doSetup(
logSuccess(`Your backend is now deployed at:\n\thttps://${backend.uri}`);
}

/**
* Check that all GCP APIs required for App Hosting are enabled.
*/
export async function ensureRequiredApisEnabled(projectId: string): Promise<void> {
await Promise.all([
ensure(projectId, developerConnectOrigin(), "apphosting", true),
ensure(projectId, cloudbuildOrigin(), "apphosting", true),
ensure(projectId, secretManagerOrigin(), "apphosting", true),
ensure(projectId, cloudRunApiOrigin(), "apphosting", true),
ensure(projectId, artifactRegistryDomain(), "apphosting", true),
ensure(projectId, iamOrigin(), "apphosting", true),
]);
}

/**
* Set up a new App Hosting-type Developer Connect GitRepoLink, optionally with a specific connection ID
*/
Expand Down
2 changes: 1 addition & 1 deletion src/commands/init.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ let choices: {
},
{
value: "apphosting",
name: "App Hosting: Configure an apphosting.yaml file for App Hosting",
name: "App Hosting: Enable web app deployments with App Hosting",
Copy link
Copy Markdown
Member

@kmcnellis kmcnellis May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong to me? are we enabling deployments, or making a backend? I think the flow as-written doesn't link to github?

Copy link
Copy Markdown
Contributor Author

@blidd-google blidd-google May 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @jpreid7396 my understanding is that the description is meant to be generic enough to cover set up for deploy from source, but also any other features we might want to include as part of this flow. I'll defer to Julia here about the language specifics

checked: false,
hidden: false,
},
Expand Down
29 changes: 26 additions & 3 deletions src/init/features/apphosting.ts
View file
Open in desktop
Copy link
Copy Markdown
Member

@kmcnellis kmcnellis May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In

      projectId,
      location,
      backendId,
      /* serviceAccount= */ null,
      /* repository= */ undefined,
      webApp?.id,
    );

It's a real trap right now to make a backend without attaching a git repo (or doing deploy from source), since AFAIK we don't have a command to add one later

Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,25 @@ import * as clc from "colorette";
import { existsSync } from "fs";
import * as ora from "ora";
import * as path from "path";
import { Setup } from "..";
import { webApps } from "../../apphosting/app";
import {
createBackend,
ensureAppHostingComputeServiceAccount,
ensureRequiredApisEnabled,
promptExistingBackend,
promptLocation,
promptNewBackendId,
} from "../../apphosting/backend";
import { Config } from "../../config";
import { FirebaseError } from "../../error";
import { AppHostingSingle } from "../../firebaseConfig";
import { ensureApiEnabled } from "../../gcp/apphosting";
import { isBillingEnabled } from "../../gcp/cloudbilling";
import { input, select } from "../../prompt";
import { readTemplateSync } from "../../templates";
import * as utils from "../../utils";
import { logBullet } from "../../utils";
import { input, select } from "../../prompt";
import { Setup } from "..";
import { isBillingEnabled } from "../../gcp/cloudbilling";

const APPHOSTING_YAML_TEMPLATE = readTemplateSync("init/apphosting/apphosting.yaml");

Expand All @@ -31,6 +34,26 @@ export async function doSetup(setup: Setup, config: Config): Promise<void> {
"Firebase App Hosting requires billing to be enabled on your project. Please enable billing by following the steps at https://cloud.google.com/billing/docs/how-to/modify-project",
);
}
await ensureApiEnabled({ projectId });
await ensureRequiredApisEnabled(projectId);
// N.B. Deploying a backend from source requires the App Hosting compute service
Copy link
Copy Markdown
Member

@kmcnellis kmcnellis May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we actually attaching the the storage.objectViewer role right now?
Also, even if we were doing that, this comment is about the storage.objectViewer role, but its on the block of code that actually provisions the SA, and we'd need to do that even if the use isn't using deploy from source

Copy link
Copy Markdown
Contributor Author

@blidd-google blidd-google May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the storage.objectViewer role change isn't relevant until the follow-up deploy from source PR #8516. Removing the comment.

// account to have the storage.objectViewer IAM role.
//
// We don't want to update the IAM permissions right before attempting to deploy,
// since IAM propagation delay will likely cause the first one to fail. However,
// `firebase init apphosting` is a prerequisite to the `firebase deploy` command,
// so we check and add the role here to give the IAM changes time to propagate.
try {
await ensureAppHostingComputeServiceAccount(projectId, /* serviceAccount= */ "");
} catch (err) {
if ((err as FirebaseError).status === 400) {
utils.logWarning(
"Your App Hosting compute service account is still being provisioned. Please try again in a few moments.",
);
}
throw err;
}

utils.logBullet(
"This command links your local project to Firebase App Hosting. You will be able to deploy your web app with `firebase deploy` after setup.",
);
Expand Down
7 changes: 5 additions & 2 deletions src/init/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface SetupInfo {

interface Feature {
name: string;
displayName?: string;
// OLD WAY: A single setup function to ask questions and actuate the setup.
doSetup?: (setup: Setup, config: Config, options: Options) => Promise<unknown>;

Expand Down Expand Up @@ -64,7 +65,7 @@ const featuresList: Feature[] = [
{ name: "remoteconfig", doSetup: features.remoteconfig },
{ name: "hosting:github", doSetup: features.hostingGithub },
{ name: "genkit", doSetup: features.genkit },
{ name: "apphosting", doSetup: features.apphosting },
{ name: "apphosting", displayName: "App Hosting", doSetup: features.apphosting },
];

const featureMap = new Map(featuresList.map((feature) => [feature.name, feature]));
Expand All @@ -73,7 +74,9 @@ export async function init(setup: Setup, config: any, options: any): Promise<any
const nextFeature = setup.features?.shift();
if (nextFeature) {
const f = lookupFeature(nextFeature);
logger.info(clc.bold(`\n${clc.white("===")} ${capitalize(nextFeature)} Setup`));
logger.info(
clc.bold(`\n${clc.white("===")} ${f.displayName || capitalize(nextFeature)} Setup`),
);

if (f.doSetup) {
await f.doSetup(setup, config, options);
Expand Down
Loading