Skip to content

Commit bcadcef

Browse files
committed
Fix IAM permissions for Cloud Build SA
1 parent 19fe16b commit bcadcef

2 files changed

Lines changed: 46 additions & 4 deletions

File tree

cicd-mcp-server/cloudbuild/cloudbuild.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,21 +122,42 @@ func addCreateTriggerTool(server *mcp.Server, cbClient cloudbuildclient.CloudBui
122122
mcp.AddTool(server, &mcp.Tool{Name: "create_build_trigger", Description: "Creates a new Cloud Build trigger."}, createTriggerToolFunc)
123123
}
124124

125-
// setPermissionsForSA resolves the SA and grants it a role.
125+
// setPermissionsForCloudBuildSA resolves the SA and grants it a role.
126126
func setPermissionsForCloudBuildSA(ctx context.Context, projectID, serviceAccount string, rmClient resourcemanagerclient.ResourcemanagerClient, iamClient iamclient.IAMClient) (string, error) {
127127
resolvedSA := serviceAccount
128128

129129
// If the serviceAccount prefix is not there, add it.
130130
if !strings.HasPrefix(resolvedSA, "serviceAccount:") {
131131
resolvedSA = fmt.Sprintf("serviceAccount:%s", resolvedSA)
132132
}
133-
roles := []string{"roles/developerconnect.tokenAccessor"}
133+
roles := []string{
134+
"roles/logging.logWriter",
135+
"roles/artifactregistry.writer",
136+
"roles/developerconnect.tokenAccessor",
137+
"roles/storage.admin",
138+
"roles/secretmanager.admin",
139+
"roles/run.admin",
140+
"roles/iam.serviceAccountUser",
141+
"roles/cloudbuild.builds.builder",
142+
}
134143
for _, r := range roles {
135144
_, err := iamClient.AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), r, resolvedSA)
136145
if err != nil {
137146
return "", fmt.Errorf("unable to add role %s to member %s on resource %s err: %w", r, resolvedSA, fmt.Sprintf("projects/%s", projectID), err)
138147
}
139148
}
149+
150+
// Grant Secret Manager Admin to Developer Connect Service Agent (P4SA)
151+
projectNumber, err := rmClient.ToProjectNumber(ctx, projectID)
152+
if err != nil {
153+
return "", fmt.Errorf("unable to resolve project id to number: %w", err)
154+
}
155+
p4sa := fmt.Sprintf("serviceAccount:service-%d@gcp-sa-developerconnect.iam.gserviceaccount.com", projectNumber)
156+
_, err = iamClient.AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), "roles/secretmanager.admin", p4sa)
157+
if err != nil {
158+
return "", fmt.Errorf("unable to add secretmanager.admin role to Developer Connect P4SA %s: %w", p4sa, err)
159+
}
160+
140161
return resolvedSA, nil
141162
}
142163

cicd-mcp-server/cloudbuild/cloudbuild_test.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,37 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
3838
serviceAccount := "serviceAccount:test-sa@example.com"
3939
serviceAccountWOPrefix := "test-sa@example.com"
4040

41+
roles := []string{
42+
"roles/logging.logWriter",
43+
"roles/artifactregistry.writer",
44+
"roles/developerconnect.tokenAccessor",
45+
"roles/storage.admin",
46+
"roles/secretmanager.admin",
47+
"roles/run.admin",
48+
"roles/iam.serviceAccountUser",
49+
"roles/cloudbuild.builds.builder",
50+
}
51+
projectNumber := int64(12345)
52+
p4sa := fmt.Sprintf("serviceAccount:service-%d@gcp-sa-developerconnect.iam.gserviceaccount.com", projectNumber)
53+
4154
t.Run("with service account", func(t *testing.T) {
42-
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), "roles/developerconnect.tokenAccessor", serviceAccount).Return(nil, nil)
55+
for _, r := range roles {
56+
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), r, serviceAccount).Return(nil, nil)
57+
}
58+
mockRMClient.EXPECT().ToProjectNumber(ctx, projectID).Return(projectNumber, nil)
59+
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), "roles/secretmanager.admin", p4sa).Return(nil, nil)
4360

4461
resolvedSA, err := setPermissionsForCloudBuildSA(ctx, projectID, serviceAccount, mockRMClient, mockIAMClient)
4562
assert.NoError(t, err)
4663
assert.Equal(t, serviceAccount, resolvedSA)
4764
})
4865

4966
t.Run("with service account, no prefix", func(t *testing.T) {
50-
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), "roles/developerconnect.tokenAccessor", serviceAccount).Return(nil, nil)
67+
for _, r := range roles {
68+
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), r, serviceAccount).Return(nil, nil)
69+
}
70+
mockRMClient.EXPECT().ToProjectNumber(ctx, projectID).Return(projectNumber, nil)
71+
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), "roles/secretmanager.admin", p4sa).Return(nil, nil)
5172

5273
resolvedSA, err := setPermissionsForCloudBuildSA(ctx, projectID, serviceAccountWOPrefix, mockRMClient, mockIAMClient)
5374
assert.NoError(t, err)

0 commit comments

Comments
 (0)