diff --git a/src/apphosting/index.ts b/src/apphosting/index.ts index 2268f9f404f..961348549d1 100644 --- a/src/apphosting/index.ts +++ b/src/apphosting/index.ts @@ -9,6 +9,7 @@ import { cloudRunApiOrigin, cloudbuildOrigin, developerConnectOrigin, + iamOrigin, secretManagerOrigin, } from "../api"; import { Backend, BackendOutputOnlyFields, API_VERSION, Build, Rollout } from "../gcp/apphosting"; @@ -48,6 +49,7 @@ export async function doSetup( ensure(projectId, secretManagerOrigin(), "apphosting", true), ensure(projectId, cloudRunApiOrigin(), "apphosting", true), ensure(projectId, artifactRegistryDomain(), "apphosting", true), + ensure(projectId, iamOrigin(), "apphosting", true), ]); const allowedLocations = (await apphosting.listLocations(projectId)).map((loc) => loc.locationId); @@ -90,6 +92,8 @@ export async function doSetup( message: "Specify your app's root directory relative to your repository", }); + await ensureAppHostingComputeServiceAccount(projectId, serviceAccount); + const backend = await createBackend( projectId, location, @@ -137,6 +141,41 @@ export async function doSetup( 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 }, + ); + } + } +} + /** * Prompts the user for a backend id and verifies that it doesn't match a pre-existing backend. */ @@ -200,23 +239,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 { diff --git a/src/test/apphosting/index.spec.ts b/src/test/apphosting/index.spec.ts index c47204b8027..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 = { @@ -108,68 +110,6 @@ describe("operationsConverter", () => { 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`, @@ -199,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}`, @@ -214,7 +213,7 @@ describe("operationsConverter", () => { }); }); - describe("prompt location", () => { + describe("promptLocation", () => { const supportedLocations = [{ name: "us-central1", locationId: "us-central1" }]; beforeEach(() => {