Skip to content
Merged
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
7 changes: 7 additions & 0 deletions src/gcp/cloudbuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,10 @@ export async function deleteRepository(
const res = await client.delete<Operation>(name);
return res.body;
}

/**
* Returns email associated with the Cloud Build Service Agent.
*/
export function serviceAgentEmail(projectNumber: string): string {
return `service-${projectNumber}@gcp-sa-cloudbuild.iam.gserviceaccount.com`;
}
38 changes: 35 additions & 3 deletions src/init/features/frameworks/repo.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import * as clc from "colorette";

import * as gcb from "../../../gcp/cloudbuild";
import * as rm from "../../../gcp/resourceManager";
import * as poller from "../../../operation-poller";
import * as utils from "../../../utils";
import { cloudbuildOrigin } from "../../../api";
import { FirebaseError } from "../../../error";
import { logger } from "../../../logger";
import { promptOnce } from "../../../prompt";
import { getProjectNumber } from "../../../getProjectNumber";

export interface ConnectionNameParts {
projectId: string;
Expand Down Expand Up @@ -81,6 +83,10 @@ export async function linkGitHubRepository(
logger.info(clc.bold(`\n${clc.yellow("===")} Connect a GitHub repository`));
const existingConns = await listFrameworksConnections(projectId);
if (existingConns.length < 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This may not be the best sentinel for whether the SA is set up properly. Users can always go into their IAM policy and remove the role grant. The more futureproof way of doing this is to read their IAM policy and verify the service account is still provisioned correctly.

I'm happy to leave it as is for M2 but we should probably note this in a line comment or track in a bug

Copy link
Contributor Author

@taeold taeold Dec 6, 2023

Choose a reason for hiding this comment

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

This may not be the best sentinel for whether the SA is set up properly. Users can always go into their IAM policy and remove the role grant.

AFAIK, this is the only code path right now where we need the IAM grant on the GCB service account. If we are reusing an existing connection, there is no need to make the IAM grant. It's only when setting up the connection for the very first time when we need the IAM grant.

A sub-goal here was to make sure we don't ask for granting this IAM policy unless it's necessary since it's yet another friction for onboarding.

const grantSuccess = await promptSecretManagerAdminGrant(projectId);
if (!grantSuccess) {
throw new FirebaseError("Insufficient IAM permissions to create a new connection to GitHub");
}
let oauthConn = await getOrCreateConnection(projectId, location, FRAMEWORKS_OAUTH_CONN_NAME);
while (oauthConn.installationState.stage === "PENDING_USER_OAUTH") {
oauthConn = await promptConnectionAuth(oauthConn);
Expand Down Expand Up @@ -156,14 +162,40 @@ async function promptRepositoryUri(
return { remoteUri, connection: remoteUriToConnection[remoteUri] };
}

async function promptSecretManagerAdminGrant(projectId: string): Promise<Boolean> {
const projectNumber = await getProjectNumber({ projectId });
const cbsaEmail = gcb.serviceAgentEmail(projectNumber);
logger.info(
"To create a new GitHub connection, Secret Manager Admin role (roles/secretmanager.admin) is required on the Cloud Build Service Agent."
);
const grant = await promptOnce({
type: "confirm",
message: "Grant the required role to the Cloud Build Service Agent?",
});
if (!grant) {
logger.info(
"You, or your project administrator, should run the following command to grant the required role:\n\n" +
`\tgcloud projects add-iam-policy-binding ${projectId} \\\n` +
`\t --member="serviceAccount:${cbsaEmail} \\\n` +
`\t --role="roles/secretmanager.admin\n`
);
return false;
}
await rm.addServiceAccountToRoles(projectId, cbsaEmail, ["roles/secretmanager.admin"], true);
logger.info("Successfully granted the required role to the Cloud Build Service Agent!");
return true;
}

async function promptConnectionAuth(conn: gcb.Connection): Promise<gcb.Connection> {
logger.info("You must authorize the Cloud Build GitHub app.");
logger.info();
logger.info("First, sign in to GitHub and authorize Cloud Build GitHub app:");
const cleanup = await utils.openInBrowserPopup(
logger.info("Sign in to GitHub and authorize Cloud Build GitHub app:");
const { url, cleanup } = await utils.openInBrowserPopup(
conn.installationState.actionUri,
"Authorize the GitHub app"
);
logger.info(`\t${url}`);
logger.info();
await promptOnce({
type: "input",
message: "Press Enter once you have authorized the app",
Expand Down Expand Up @@ -215,7 +247,7 @@ export async function getOrCreateConnection(
try {
conn = await gcb.getConnection(projectId, location, connectionId);
} catch (err: unknown) {
if ((err as FirebaseError).status === 404) {
if ((err as any).status === 404) {
conn = await createConnection(projectId, location, connectionId, githubConfig);
} else {
throw err;
Expand Down
13 changes: 9 additions & 4 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,10 @@ export async function openInBrowser(url: string): Promise<void> {
/**
* Like openInBrowser but opens the url in a popup.
*/
export async function openInBrowserPopup(url: string, buttonText: string): Promise<() => void> {
export async function openInBrowserPopup(
url: string,
buttonText: string
): Promise<{ url: string; cleanup: () => void }> {
const popupPage = fs
.readFileSync(path.join(__dirname, "../templates/popup.html"), { encoding: "utf-8" })
.replace("${url}", url)
Expand All @@ -787,10 +790,12 @@ export async function openInBrowserPopup(url: string, buttonText: string): Promi
server.listen(port);

const popupPageUri = `http://localhost:${port}`;
logger.info(popupPageUri);
await openInBrowser(popupPageUri);

return () => {
server.close();
return {
url: popupPageUri,
cleanup: () => {
server.close();
},
};
}