Skip to content

Commit 6950829

Browse files
authoredMar 26, 2024··
Fix "could not assert Secret Manager permissions" Cloud Build error (#6904)
* fix secret manager admin permissions required bug for cloudbuild connections * only ensureSecretManagerAdminGrant when creating an oauth token --------- Co-authored-by: Mathusan Selvarajah <mathusan@google.com>
1 parent 4a17ca7 commit 6950829

File tree

2 files changed

+45
-3
lines changed

2 files changed

+45
-3
lines changed
 

‎src/init/features/apphosting/repo.ts

+15-3
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ export async function linkGitHubRepository(
9696
const existingConns = await listAppHostingConnections(projectId);
9797

9898
if (existingConns.length === 0) {
99-
await ensureSecretManagerAdminGrant(projectId);
10099
existingConns.push(
101100
await createFullyInstalledConnection(projectId, location, generateConnectionId(), oauthConn),
102101
);
@@ -170,11 +169,24 @@ async function createFullyInstalledConnection(
170169
* Gets or creates the sentinel GitHub connection resource that contains our Firebase-wide GitHub Oauth token.
171170
* This Oauth token can be used to create other connections without reprompting the user to grant access.
172171
*/
173-
async function getOrCreateOauthConnection(
172+
export async function getOrCreateOauthConnection(
174173
projectId: string,
175174
location: string,
176175
): Promise<gcb.Connection> {
177-
let conn = await getOrCreateConnection(projectId, location, APPHOSTING_OAUTH_CONN_NAME);
176+
let conn: gcb.Connection;
177+
try {
178+
conn = await gcb.getConnection(projectId, location, APPHOSTING_OAUTH_CONN_NAME);
179+
} catch (err: unknown) {
180+
if ((err as any).status === 404) {
181+
// Cloud build P4SA requires the secret manager admin role.
182+
// This is required when creating an initial connection which is the Oauth connection in our case.
183+
await ensureSecretManagerAdminGrant(projectId);
184+
conn = await createConnection(projectId, location, APPHOSTING_OAUTH_CONN_NAME);
185+
} else {
186+
throw err;
187+
}
188+
}
189+
178190
while (conn.installationState.stage === "PENDING_USER_OAUTH") {
179191
utils.logBullet("You must authorize the Cloud Build GitHub app.");
180192
utils.logBullet("Sign in to GitHub and authorize Cloud Build GitHub app:");

‎src/test/init/apphosting/repo.spec.ts

+30
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ import * as sinon from "sinon";
22
import { expect } from "chai";
33

44
import * as gcb from "../../../gcp/cloudbuild";
5+
import * as rm from "../../../gcp/resourceManager";
56
import * as prompt from "../../../prompt";
67
import * as poller from "../../../operation-poller";
78
import * as repo from "../../../init/features/apphosting/repo";
89
import * as utils from "../../../utils";
10+
import * as srcUtils from "../../../../src/getProjectNumber";
911
import { FirebaseError } from "../../../error";
1012

1113
const projectId = "projectId";
@@ -53,8 +55,11 @@ describe("composer", () => {
5355
let getConnectionStub: sinon.SinonStub;
5456
let getRepositoryStub: sinon.SinonStub;
5557
let createConnectionStub: sinon.SinonStub;
58+
let serviceAccountHasRolesStub: sinon.SinonStub;
5659
let createRepositoryStub: sinon.SinonStub;
5760
let fetchLinkableRepositoriesStub: sinon.SinonStub;
61+
let getProjectNumberStub: sinon.SinonStub;
62+
let openInBrowserPopupStub: sinon.SinonStub;
5863

5964
beforeEach(() => {
6065
promptOnceStub = sandbox.stub(prompt, "promptOnce").throws("Unexpected promptOnce call");
@@ -70,13 +75,20 @@ describe("composer", () => {
7075
createConnectionStub = sandbox
7176
.stub(gcb, "createConnection")
7277
.throws("Unexpected createConnection call");
78+
serviceAccountHasRolesStub = sandbox.stub(rm, "serviceAccountHasRoles").resolves(true);
7379
createRepositoryStub = sandbox
7480
.stub(gcb, "createRepository")
7581
.throws("Unexpected createRepository call");
7682
fetchLinkableRepositoriesStub = sandbox
7783
.stub(gcb, "fetchLinkableRepositories")
7884
.throws("Unexpected fetchLinkableRepositories call");
7985
sandbox.stub(utils, "openInBrowser").resolves();
86+
openInBrowserPopupStub = sandbox
87+
.stub(utils, "openInBrowserPopup")
88+
.throws("Unexpected openInBrowserPopup call");
89+
getProjectNumberStub = sandbox
90+
.stub(srcUtils, "getProjectNumber")
91+
.throws("Unexpected getProjectNumber call");
8092
});
8193

8294
afterEach(() => {
@@ -139,6 +151,24 @@ describe("composer", () => {
139151
expect(createConnectionStub).to.be.calledWith(projectId, location, connectionId);
140152
});
141153

154+
it("checks if secret manager admin role is granted for cloud build P4SA when creating an oauth connection", async () => {
155+
getConnectionStub.onFirstCall().rejects(new FirebaseError("error", { status: 404 }));
156+
getConnectionStub.onSecondCall().resolves(completeConn);
157+
createConnectionStub.resolves(op);
158+
pollOperationStub.resolves(pendingConn);
159+
promptOnceStub.resolves("any key");
160+
getProjectNumberStub.onFirstCall().resolves(projectId);
161+
openInBrowserPopupStub.resolves({ url: "", cleanup: sandbox.stub() });
162+
163+
await repo.getOrCreateOauthConnection(projectId, location);
164+
expect(serviceAccountHasRolesStub).to.be.calledWith(
165+
projectId,
166+
`service-${projectId}@gcp-sa-cloudbuild.iam.gserviceaccount.com`,
167+
["roles/secretmanager.admin"],
168+
true,
169+
);
170+
});
171+
142172
it("creates repository if it doesn't exist", async () => {
143173
getConnectionStub.resolves(completeConn);
144174
fetchLinkableRepositoriesStub.resolves(repos);

0 commit comments

Comments
 (0)
Please sign in to comment.