diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f707441b4e..e69de29bb2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +0,0 @@ -- Release Firestore Emulator version 1.19.5 which adds support for import and export in Datastore Mode (#7020). -- Fix non static check for not-found route in Next.js 14.2 (#7012) -- Fix Next.js path issue on Windows (#7031) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 0063d819d31..68cf1493a11 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,12 +1,12 @@ { "name": "firebase-tools", - "version": "13.7.3", + "version": "13.7.5", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "firebase-tools", - "version": "13.7.3", + "version": "13.7.5", "license": "MIT", "dependencies": { "@google-cloud/cloud-sql-connector": "^1.2.3", diff --git a/package.json b/package.json index da6953345e7..d5741cb0677 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "firebase-tools", - "version": "13.7.3", + "version": "13.7.5", "description": "Command-Line Interface for Firebase", "main": "./lib/index.js", "bin": { diff --git a/src/api.ts b/src/api.ts index 9d60bfb5dfd..31913262fa8 100755 --- a/src/api.ts +++ b/src/api.ts @@ -24,6 +24,12 @@ export const cloudMonitoringOrigin = () => utils.envOverride("CLOUD_MONITORING_URL", "https://monitoring.googleapis.com"); export const containerRegistryDomain = () => utils.envOverride("CONTAINER_REGISTRY_DOMAIN", "gcr.io"); + +export const developerConnectOrigin = () => + utils.envOverride("DEVELOPERCONNECT_URL", "https://developerconnect.googleapis.com"); +export const developerConnectP4SADomain = () => + utils.envOverride("DEVELOPERCONNECT_P4SA_DOMAIN", "gcp-sa-devconnect.iam.gserviceaccount.com"); + export const artifactRegistryDomain = () => utils.envOverride("ARTIFACT_REGISTRY_DOMAIN", "https://artifactregistry.googleapis.com"); export const appDistributionOrigin = () => @@ -31,6 +37,14 @@ export const appDistributionOrigin = () => "FIREBASE_APP_DISTRIBUTION_URL", "https://firebaseappdistribution.googleapis.com", ); +export const apphostingOrigin = () => + utils.envOverride("FIREBASE_APPHOSTING_URL", "https://firebaseapphosting.googleapis.com"); +export const apphostingP4SADomain = () => + utils.envOverride( + "FIREBASE_APPHOSTING_P4SA_DOMAIN", + "gcp-sa-firebaseapphosting.iam.gserviceaccount.com", + ); + export const authOrigin = () => utils.envOverride("FIREBASE_AUTH_URL", "https://accounts.google.com"); export const consoleOrigin = () => @@ -71,15 +85,6 @@ export const functionsDefaultRegion = () => export const cloudbuildOrigin = () => utils.envOverride("FIREBASE_CLOUDBUILD_URL", "https://cloudbuild.googleapis.com"); -export const developerConnectOrigin = () => - utils.envOverride("FIREBASE_DEVELOPERCONNECT_URL", "https://developerconnect.googleapis.com"); - -export const developerConnectP4SAOrigin = () => - utils.envOverride( - "FIREBASE_DEVELOPERCONNECT_P4SA_URL", - "gcp-sa-devconnect.iam.gserviceaccount.com", - ); - export const cloudschedulerOrigin = () => utils.envOverride("FIREBASE_CLOUDSCHEDULER_URL", "https://cloudscheduler.googleapis.com"); export const cloudTasksOrigin = () => @@ -128,8 +133,7 @@ export const cloudRunApiOrigin = () => utils.envOverride("CLOUD_RUN_API_URL", "https://run.googleapis.com"); export const serviceUsageOrigin = () => utils.envOverride("FIREBASE_SERVICE_USAGE_URL", "https://serviceusage.googleapis.com"); -export const apphostingOrigin = () => - utils.envOverride("APPHOSTING_URL", "https://firebaseapphosting.googleapis.com"); + export const githubOrigin = () => utils.envOverride("GITHUB_URL", "https://github.com"); export const githubApiOrigin = () => utils.envOverride("GITHUB_API_URL", "https://api.github.com"); export const secretManagerOrigin = () => diff --git a/src/apphosting/githubConnections.ts b/src/apphosting/githubConnections.ts index 2809aafed8f..83d851e6b00 100644 --- a/src/apphosting/githubConnections.ts +++ b/src/apphosting/githubConnections.ts @@ -135,7 +135,7 @@ export async function linkGitHubRepository( const repo = await getOrCreateRepository(projectId, location, connectionId, repoCloneUri); utils.logSuccess(`Successfully linked GitHub repository at remote URI`); - utils.logSuccess(`\t${repo.cloneUri}`); + utils.logSuccess(`\t${repo.cloneUri}\n`); return repo; } diff --git a/src/apphosting/index.ts b/src/apphosting/index.ts index bb6a975dc69..dfa04ffd023 100644 --- a/src/apphosting/index.ts +++ b/src/apphosting/index.ts @@ -8,7 +8,9 @@ import { artifactRegistryDomain, cloudRunApiOrigin, cloudbuildOrigin, + consoleOrigin, developerConnectOrigin, + iamOrigin, secretManagerOrigin, } from "../api"; import { Backend, BackendOutputOnlyFields, API_VERSION, Build, Rollout } from "../gcp/apphosting"; @@ -23,6 +25,8 @@ import * as deploymentTool from "../deploymentTool"; import { DeepOmit } from "../metaprogramming"; import * as apps from "./app"; import { GitRepositoryLink } from "../gcp/devConnect"; +import * as ora from "ora"; + const DEFAULT_COMPUTE_SERVICE_ACCOUNT_NAME = "firebase-app-hosting-compute"; const apphostingPollerOptions: Omit = { @@ -48,7 +52,13 @@ export async function doSetup( ensure(projectId, secretManagerOrigin(), "apphosting", true), ensure(projectId, cloudRunApiOrigin(), "apphosting", true), ensure(projectId, artifactRegistryDomain(), "apphosting", true), + ensure(projectId, iamOrigin(), "apphosting", true), ]); + logBullet("First we need a few details to create your backend.\n"); + + // 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. + await ensureAppHostingComputeServiceAccount(projectId, serviceAccount); const allowedLocations = (await apphosting.listLocations(projectId)).map((loc) => loc.locationId); if (location) { @@ -59,8 +69,6 @@ export async function doSetup( } } - logBullet("First we need a few details to create your backend.\n"); - location = location || (await promptLocation(projectId, "Select a location to host your backend:\n")); logSuccess(`Location set to ${location}.\n`); @@ -90,6 +98,7 @@ export async function doSetup( message: "Specify your app's root directory relative to your repository", }); + const createBackendSpinner = ora("Creating your new backend...").start(); const backend = await createBackend( projectId, location, @@ -99,6 +108,7 @@ export async function doSetup( webApp?.id, rootDir, ); + createBackendSpinner.succeed(`Successfully created backend:\n\t${backend.name}\n`); // TODO: Once tag patterns are implemented, prompt which method the user // prefers. We could reduce the number of questions asked by letting people @@ -120,11 +130,16 @@ export async function doSetup( }); if (!confirmRollout) { - logSuccess(`Successfully created backend:\n\t${backend.name}`); logSuccess(`Your backend will be deployed at:\n\thttps://${backend.uri}`); return; } + logBullet( + `You may also track this rollout at:\n\t${consoleOrigin()}/project/${projectId}/apphosting`, + ); + const createRolloutSpinner = ora( + "Starting a new rollout... This make take a few minutes. It's safe to exit now.", + ).start(); await orchestrateRollout(projectId, location, backendId, { source: { codebase: { @@ -132,9 +147,42 @@ export async function doSetup( }, }, }); + createRolloutSpinner.succeed(`Your backend is now deployed at:\n\thttps://${backend.uri}`); +} - logSuccess(`Successfully created backend:\n\t${backend.name}`); - logSuccess(`Your backend is now deployed at:\n\thttps://${backend.uri}`); +/** + * Ensures the service account is present the user has permissions to use it by + * checking the `iam.serviceAccounts.actAs` permission. If the permissions + * check fails, this returns an error. If the permission check fails with a + * "not found" error, this attempts to provision the service account. + */ +export async function ensureAppHostingComputeServiceAccount( + projectId: string, + serviceAccount: string | null, +): Promise { + const sa = serviceAccount || defaultComputeServiceAccountEmail(projectId); + const name = `projects/${projectId}/serviceAccounts/${sa}`; + try { + await iam.testResourceIamPermissions( + iamOrigin(), + "v1", + name, + ["iam.serviceAccounts.actAs"], + `projects/${projectId}`, + ); + } catch (err: unknown) { + if (!(err instanceof FirebaseError)) { + throw err; + } + if (err.status === 404) { + await provisionDefaultComputeServiceAccount(projectId); + } else if (err.status === 403) { + throw new FirebaseError( + `Failed to create backend due to missing delegation permissions for ${sa}. Make sure you have the iam.serviceAccounts.actAs permission.`, + { original: err }, + ); + } + } } /** @@ -191,9 +239,6 @@ export async function createBackend( appId: webAppId, }; - // TODO: remove computeServiceAccount when the backend supports the field. - delete backendReqBody.serviceAccount; - async function createBackendAndPoll(): Promise { const op = await apphosting.createBackend(projectId, location, backendReqBody, backendId); return await poller.pollOperation({ @@ -203,23 +248,7 @@ export async function createBackend( }); } - try { - return await createBackendAndPoll(); - } catch (err: any) { - if (err.status === 403) { - if (err.message.includes(defaultServiceAccount)) { - // Create the default service account if it doesn't exist and try again. - await provisionDefaultComputeServiceAccount(projectId); - return await createBackendAndPoll(); - } else if (serviceAccount && err.message.includes(serviceAccount)) { - throw new FirebaseError( - `Failed to create backend due to missing delegation permissions for ${serviceAccount}. Make sure you have the iam.serviceAccounts.actAs permission.`, - { children: [err] }, - ); - } - } - throw err; - } + return await createBackendAndPoll(); } async function provisionDefaultComputeServiceAccount(projectId: string): Promise { @@ -227,8 +256,8 @@ async function provisionDefaultComputeServiceAccount(projectId: string): Promise await iam.createServiceAccount( projectId, DEFAULT_COMPUTE_SERVICE_ACCOUNT_NAME, - "Firebase App Hosting compute service account", "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. @@ -240,12 +269,9 @@ async function provisionDefaultComputeServiceAccount(projectId: string): Promise projectId, defaultComputeServiceAccountEmail(projectId), [ - // TODO: Update to roles/firebaseapphosting.computeRunner when it is available. - "roles/firebaseapphosting.viewer", - "roles/artifactregistry.createOnPushWriter", - "roles/logging.logWriter", - "roles/storage.objectAdmin", + "roles/firebaseapphosting.computeRunner", "roles/firebase.sdkAdminServiceAgent", + "roles/developerconnect.readTokenAccessor", ], /* skipAccountLookup= */ true, ); @@ -279,6 +305,10 @@ export async function setDefaultTrafficPolicy( }); } +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + /** * Creates a new build and rollout and polls both to completion. */ @@ -288,7 +318,7 @@ export async function orchestrateRollout( backendId: string, buildInput: DeepOmit, ): Promise<{ rollout: Rollout; build: Build }> { - logBullet("Starting a new rollout... this may take a few minutes."); + await delay(45 * 1000); const buildId = await apphosting.getNextRolloutId(projectId, location, backendId, 1); const buildOp = await apphosting.createBuild(projectId, location, backendId, buildId, buildInput); @@ -343,7 +373,6 @@ export async function orchestrateRollout( }); const [rollout, build] = await Promise.all([rolloutPoll, buildPoll]); - logSuccess("Rollout completed."); if (build.state !== "READY") { if (!build.buildLogsUri) { diff --git a/src/apphosting/secrets/index.ts b/src/apphosting/secrets/index.ts index 6ce7d2a8a98..8fd96e5cf0c 100644 --- a/src/apphosting/secrets/index.ts +++ b/src/apphosting/secrets/index.ts @@ -62,9 +62,11 @@ export function serviceAccountsForBackend( */ export async function grantSecretAccess( projectId: string, + projectNumber: string, secretName: string, accounts: MultiServiceAccounts, ): Promise { + const p4saEmail = apphosting.serviceAgentEmail(projectNumber); const newBindings: iam.Binding[] = [ { role: "roles/secretmanager.secretAccessor", @@ -78,6 +80,11 @@ export async function grantSecretAccess( role: "roles/secretmanager.viewer", members: accounts.buildServiceAccounts.map((sa) => `serviceAccount:${sa}`), }, + // The App Hosting service agent needs the version manager role for automated garbage collection. + { + role: "roles/secretmanager.secretVersionManager", + members: [`serviceAccount:${p4saEmail}`], + }, ]; let existingBindings; diff --git a/src/commands/apphosting-backends-create.ts b/src/commands/apphosting-backends-create.ts index bb006181154..6472ffc29e7 100644 --- a/src/commands/apphosting-backends-create.ts +++ b/src/commands/apphosting-backends-create.ts @@ -19,8 +19,8 @@ export const command = new Command("apphosting:backends:create") ) .option( "-w, --with-dev-connect", - "use the Developer Connect flow insetad of Cloud Build Repositories (testing)", - false, + "use the Developer Connect flow instead of Cloud Build Repositories (testing)", + true, ) .before(ensureApiEnabled) .before(requireInteractive) diff --git a/src/commands/apphosting-secrets-grantaccess.ts b/src/commands/apphosting-secrets-grantaccess.ts index d0618d7d7c6..5645378c081 100644 --- a/src/commands/apphosting-secrets-grantaccess.ts +++ b/src/commands/apphosting-secrets-grantaccess.ts @@ -50,5 +50,5 @@ export const command = new Command("apphosting:secrets:grantaccess " const backend = await apphosting.getBackend(projectId, location, backendId); const accounts = secrets.toMulti(secrets.serviceAccountsForBackend(projectNumber, backend)); - await secrets.grantSecretAccess(projectId, secretName, accounts); + await secrets.grantSecretAccess(projectId, projectNumber, secretName, accounts); }); diff --git a/src/commands/apphosting-secrets-set.ts b/src/commands/apphosting-secrets-set.ts index cee2a1c8fa3..390181afbb6 100644 --- a/src/commands/apphosting-secrets-set.ts +++ b/src/commands/apphosting-secrets-set.ts @@ -71,7 +71,7 @@ export const command = new Command("apphosting:secrets:set ") // TODO: For existing secrets, enter the grantSecretAccess dialog only when the necessary permissions don't exist. } else { - await secrets.grantSecretAccess(projectId, secretName, accounts); + await secrets.grantSecretAccess(projectId, projectNumber, secretName, accounts); } await config.maybeAddSecretToYaml(secretName); diff --git a/src/emulator/downloadableEmulators.ts b/src/emulator/downloadableEmulators.ts index db1613dc635..cefec4bed02 100755 --- a/src/emulator/downloadableEmulators.ts +++ b/src/emulator/downloadableEmulators.ts @@ -222,6 +222,7 @@ const Commands: { [s in DownloadableEmulators]: DownloadableEmulatorCommand } = "single_project_mode", ], joinArgs: false, + shell: false, }, firestore: { binary: "java", @@ -245,6 +246,7 @@ const Commands: { [s in DownloadableEmulators]: DownloadableEmulatorCommand } = // "single_project_mode_error", ], joinArgs: false, + shell: false, }, storage: { // This is for the Storage Emulator rules runtime, which is started @@ -260,18 +262,21 @@ const Commands: { [s in DownloadableEmulators]: DownloadableEmulatorCommand } = ], optionalArgs: [], joinArgs: false, + shell: false, }, pubsub: { binary: getExecPath(Emulators.PUBSUB)!, args: [], optionalArgs: ["port", "host"], joinArgs: true, + shell: true, }, ui: { binary: "node", args: [getExecPath(Emulators.UI)], optionalArgs: [], joinArgs: false, + shell: false, }, dataconnect: { binary: getExecPath(Emulators.DATACONNECT), @@ -353,6 +358,7 @@ export function _getCommand( args: cmdLineArgs, optionalArgs: baseCmd.optionalArgs, joinArgs: baseCmd.joinArgs, + shell: baseCmd.shell, }; } @@ -407,7 +413,7 @@ async function _runBinary( const logger = EmulatorLogger.forEmulator(emulator.name); emulator.stdout = fs.createWriteStream(getLogFileName(emulator.name)); try { - emulator.instance = childProcess.spawn(command.binary, command.args, { + const opts: childProcess.SpawnOptions = { env: { ...process.env, ...extraEnv }, // `detached` must be true as else a SIGINT (Ctrl-c) will stop the child process before we can handle a // graceful shutdown and call `downloadableEmulators.stop(...)` ourselves. @@ -415,7 +421,11 @@ async function _runBinary( // related to this issue: https://github.com/grpc/grpc-java/pull/6512 detached: true, stdio: ["inherit", "pipe", "pipe"], - }); + }; + if (command.shell && utils.IS_WINDOWS) { + opts.shell = true; + } + emulator.instance = childProcess.spawn(command.binary, command.args, opts); } catch (e: any) { if (e.code === "EACCES") { // Known issue when WSL users don't have java diff --git a/src/emulator/types.ts b/src/emulator/types.ts index 8ebb4ef9de6..74dd055ec18 100644 --- a/src/emulator/types.ts +++ b/src/emulator/types.ts @@ -151,6 +151,7 @@ export interface DownloadableEmulatorCommand { args: string[]; optionalArgs: string[]; joinArgs: boolean; + shell: boolean; } export interface EmulatorDownloadOptions { diff --git a/src/functions/python.ts b/src/functions/python.ts index 82bad6eb5d1..6baa355b0a0 100644 --- a/src/functions/python.ts +++ b/src/functions/python.ts @@ -2,6 +2,7 @@ import * as path from "path"; import * as spawn from "cross-spawn"; import * as cp from "child_process"; import { logger } from "../logger"; +import { IS_WINDOWS } from "../utils"; /** * Default directory for python virtual environment. @@ -12,12 +13,11 @@ export const DEFAULT_VENV_DIR = "venv"; * Get command for running Python virtual environment for given platform. */ export function virtualEnvCmd(cwd: string, venvDir: string): { command: string; args: string[] } { - const activateScriptPath = - process.platform === "win32" ? ["Scripts", "activate.bat"] : ["bin", "activate"]; + const activateScriptPath = IS_WINDOWS ? ["Scripts", "activate.bat"] : ["bin", "activate"]; const venvActivate = `"${path.join(cwd, venvDir, ...activateScriptPath)}"`; return { - command: process.platform === "win32" ? venvActivate : ".", - args: [process.platform === "win32" ? "" : venvActivate], + command: IS_WINDOWS ? venvActivate : ".", + args: [IS_WINDOWS ? "" : venvActivate], }; } diff --git a/src/gcp/apphosting.ts b/src/gcp/apphosting.ts index c25ba16c23c..f005d47df2d 100644 --- a/src/gcp/apphosting.ts +++ b/src/gcp/apphosting.ts @@ -1,7 +1,7 @@ import * as proto from "../gcp/proto"; import { Client } from "../apiv2"; import { needProjectId } from "../projectUtils"; -import { apphostingOrigin } from "../api"; +import { apphostingOrigin, apphostingP4SADomain } from "../api"; import { ensure } from "../ensureApiEnabled"; import * as deploymentTool from "../deploymentTool"; import { FirebaseError } from "../error"; @@ -264,6 +264,15 @@ export interface ListBackendsResponse { unreachable: string[]; } +const P4SA_DOMAIN = apphostingP4SADomain(); + +/** + * Returns the App Hosting service agent. + */ +export function serviceAgentEmail(projectNumber: string): string { + return `service-${projectNumber}@${P4SA_DOMAIN}`; +} + /** * Creates a new Backend in a given project and location. */ diff --git a/src/gcp/devConnect.ts b/src/gcp/devConnect.ts index ee60438b74c..a69d63cd3bc 100644 --- a/src/gcp/devConnect.ts +++ b/src/gcp/devConnect.ts @@ -1,5 +1,5 @@ import { Client } from "../apiv2"; -import { developerConnectOrigin, developerConnectP4SAOrigin } from "../api"; +import { developerConnectOrigin, developerConnectP4SADomain } from "../api"; import { generateServiceIdentityAndPoll } from "./serviceusage"; const PAGE_SIZE_MAX = 1000; @@ -275,7 +275,7 @@ export async function getGitRepositoryLink( * Returns email associated with the Developer Connect Service Agent */ export function serviceAgentEmail(projectNumber: string): string { - return `service-${projectNumber}@${developerConnectP4SAOrigin()}`; + return `service-${projectNumber}@${developerConnectP4SADomain()}`; } /** diff --git a/src/gcp/secretManager.ts b/src/gcp/secretManager.ts index 7388793c758..32b419ce471 100644 --- a/src/gcp/secretManager.ts +++ b/src/gcp/secretManager.ts @@ -145,7 +145,7 @@ export async function getSecretMetadata( const secretInfo: any = {}; try { secretInfo.secret = await getSecret(projectId, secretName); - secretInfo.secretVersion = getSecretVersion(projectId, secretName, version); + secretInfo.secretVersion = await getSecretVersion(projectId, secretName, version); } catch (err: any) { // Throw anything other than the expected 404 errors. if (err.status !== 404) { diff --git a/src/test/apphosting/index.spec.ts b/src/test/apphosting/index.spec.ts index 040f8b0b12a..eb310243036 100644 --- a/src/test/apphosting/index.spec.ts +++ b/src/test/apphosting/index.spec.ts @@ -11,12 +11,12 @@ import { deleteBackendAndPoll, promptLocation, setDefaultTrafficPolicy, + ensureAppHostingComputeServiceAccount, } from "../../apphosting/index"; import * as deploymentTool from "../../deploymentTool"; import { FirebaseError } from "../../error"; -describe("operationsConverter", () => { - const sandbox: sinon.SinonSandbox = sinon.createSandbox(); +describe("apphosting setup functions", () => { const projectId = "projectId"; const location = "us-central1"; const backendId = "backendId"; @@ -29,37 +29,39 @@ describe("operationsConverter", () => { let listLocationsStub: sinon.SinonStub; let createServiceAccountStub: sinon.SinonStub; let addServiceAccountToRolesStub: sinon.SinonStub; + let testResourceIamPermissionsStub: sinon.SinonStub; beforeEach(() => { - promptOnceStub = sandbox.stub(prompt, "promptOnce").throws("Unexpected promptOnce call"); - pollOperationStub = sandbox - .stub(poller, "pollOperation") - .throws("Unexpected pollOperation call"); - createBackendStub = sandbox + promptOnceStub = sinon.stub(prompt, "promptOnce").throws("Unexpected promptOnce call"); + pollOperationStub = sinon.stub(poller, "pollOperation").throws("Unexpected pollOperation call"); + createBackendStub = sinon .stub(apphosting, "createBackend") .throws("Unexpected createBackend call"); - deleteBackendStub = sandbox + deleteBackendStub = sinon .stub(apphosting, "deleteBackend") .throws("Unexpected deleteBackend call"); - updateTrafficStub = sandbox + updateTrafficStub = sinon .stub(apphosting, "updateTraffic") .throws("Unexpected updateTraffic call"); - listLocationsStub = sandbox + listLocationsStub = sinon .stub(apphosting, "listLocations") .throws("Unexpected listLocations call"); - createServiceAccountStub = sandbox + createServiceAccountStub = sinon .stub(iam, "createServiceAccount") .throws("Unexpected createServiceAccount call"); - addServiceAccountToRolesStub = sandbox + addServiceAccountToRolesStub = sinon .stub(resourceManager, "addServiceAccountToRoles") .throws("Unexpected addServiceAccountToRoles call"); + testResourceIamPermissionsStub = sinon + .stub(iam, "testResourceIamPermissions") + .throws("Unexpected testResourceIamPermissions call"); }); afterEach(() => { - sandbox.verifyAndRestore(); + sinon.verifyAndRestore(); }); - describe("onboardBackend", () => { + describe("createBackend", () => { const webAppId = "webAppId"; const op = { @@ -102,73 +104,12 @@ describe("operationsConverter", () => { rootDirectory: "/", }, labels: deploymentTool.labels(), + serviceAccount: "custom-service-account", appId: webAppId, }; expect(createBackendStub).to.be.calledWith(projectId, location, backendInput); }); - it("should provision the default compute service account", async () => { - createBackendStub.resolves(op); - pollOperationStub - // Initial CreateBackend operation should throw a permission denied to trigger service account creation. - .onFirstCall() - .throws( - new FirebaseError( - `missing actAs permission on firebase-app-hosting-compute@${projectId}.iam.gserviceaccount.com`, - { status: 403 }, - ), - ) - .onSecondCall() - .resolves(completeBackend); - createServiceAccountStub.resolves({}); - addServiceAccountToRolesStub.resolves({}); - - await createBackend( - projectId, - location, - backendId, - cloudBuildConnRepo, - /* serviceAccount= */ null, - webAppId, - ); - - // CreateBackend should be called twice; once initially and once after the service account was created - expect(createBackendStub).to.be.calledTwice; - expect(createServiceAccountStub).to.be.calledOnce; - expect(addServiceAccountToRolesStub).to.be.calledOnce; - }); - - it("does not try to provision a custom service account", () => { - createBackendStub.resolves(op); - pollOperationStub - // Initial CreateBackend operation should throw a permission denied to - // potentially trigger service account creation. - .onFirstCall() - .throws( - new FirebaseError("missing actAs permission on my-service-account", { status: 403 }), - ) - .onSecondCall() - .resolves(completeBackend); - - expect( - createBackend( - projectId, - location, - backendId, - cloudBuildConnRepo, - /* serviceAccount= */ "my-service-account", - webAppId, - ), - ).to.be.rejectedWith( - FirebaseError, - "Failed to create backend due to missing delegation permissions for my-service-account. Make sure you have the iam.serviceAccounts.actAs permission.", - ); - - expect(createBackendStub).to.be.calledOnce; - expect(createServiceAccountStub).to.not.have.been.called; - expect(addServiceAccountToRolesStub).to.not.have.been.called; - }); - it("should set default rollout policy to 100% all at once", async () => { const completeTraffic: apphosting.Traffic = { name: `projects/${projectId}/locations/${location}/backends/${backendId}/traffic`, @@ -198,7 +139,66 @@ describe("operationsConverter", () => { }); }); - describe("delete backend", () => { + describe("ensureAppHostingComputeServiceAccount", () => { + const serviceAccount = "hello@example.com"; + + it("should succeed if the user has permissions for the service account", async () => { + testResourceIamPermissionsStub.resolves(); + + await expect(ensureAppHostingComputeServiceAccount(projectId, serviceAccount)).to.be + .fulfilled; + + expect(testResourceIamPermissionsStub).to.be.calledOnce; + expect(createServiceAccountStub).to.not.be.called; + expect(addServiceAccountToRolesStub).to.not.be.called; + }); + + it("should succeed if the user can create the service account when it does not exist", async () => { + testResourceIamPermissionsStub.rejects( + new FirebaseError("Permission denied", { status: 404 }), + ); + createServiceAccountStub.resolves(); + addServiceAccountToRolesStub.resolves(); + + await expect(ensureAppHostingComputeServiceAccount(projectId, serviceAccount)).to.be + .fulfilled; + + expect(testResourceIamPermissionsStub).to.be.calledOnce; + expect(createServiceAccountStub).to.be.calledOnce; + expect(addServiceAccountToRolesStub).to.be.calledOnce; + }); + + it("should throw an error if the user does not have permissions", async () => { + testResourceIamPermissionsStub.rejects( + new FirebaseError("Permission denied", { status: 403 }), + ); + + await expect( + ensureAppHostingComputeServiceAccount(projectId, serviceAccount), + ).to.be.rejectedWith(/Failed to create backend due to missing delegation permissions/); + + expect(testResourceIamPermissionsStub).to.be.calledOnce; + expect(createServiceAccountStub).to.not.be.called; + expect(addServiceAccountToRolesStub).to.not.be.called; + }); + + it("should throw the error if the user cannot create the service account", async () => { + testResourceIamPermissionsStub.rejects( + new FirebaseError("Permission denied", { status: 404 }), + ); + createServiceAccountStub.rejects(new FirebaseError("failed to create SA")); + + await expect( + ensureAppHostingComputeServiceAccount(projectId, serviceAccount), + ).to.be.rejectedWith("failed to create SA"); + + expect(testResourceIamPermissionsStub).to.be.calledOnce; + expect(createServiceAccountStub).to.be.calledOnce; + expect(addServiceAccountToRolesStub).to.not.be.called; + }); + }); + + describe("deleteBackendAndPoll", () => { it("should delete a backend", async () => { const op = { name: `projects/${projectId}/locations/${location}/backends/${backendId}`, @@ -213,7 +213,7 @@ describe("operationsConverter", () => { }); }); - describe("prompt location", () => { + describe("promptLocation", () => { const supportedLocations = [{ name: "us-central1", locationId: "us-central1" }]; beforeEach(() => { diff --git a/src/test/apphosting/secrets/index.spec.ts b/src/test/apphosting/secrets/index.spec.ts index 174a3bdd413..67dde8393d9 100644 --- a/src/test/apphosting/secrets/index.spec.ts +++ b/src/test/apphosting/secrets/index.spec.ts @@ -226,7 +226,7 @@ describe("secrets", () => { gcsm.getIamPolicy.resolves(existingPolicy); gcsm.setIamPolicy.resolves(); - await secrets.grantSecretAccess(secret.projectId, secret.name, { + await secrets.grantSecretAccess(secret.projectId, "12345", secret.name, { buildServiceAccounts: ["buildSA"], runServiceAccounts: ["computeSA"], }); @@ -244,6 +244,12 @@ describe("secrets", () => { role: "roles/secretmanager.viewer", members: ["serviceAccount:buildSA"], }, + { + role: "roles/secretmanager.secretVersionManager", + members: [ + "serviceAccount:service-12345@gcp-sa-firebaseapphosting.iam.gserviceaccount.com", + ], + }, ]; expect(gcsm.getIamPolicy).to.be.calledWithMatch(secret);