Skip to content

Commit 07b8efb

Browse files
Fix CLoud Build SA (#130)
1 parent 3581477 commit 07b8efb

3 files changed

Lines changed: 37 additions & 7 deletions

File tree

cicd-mcp-server/cloudbuild/cloudbuild.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ func setPermissionsForCloudBuildSA(ctx context.Context, projectID, serviceAccoun
135135
gcbSARoles := []string{
136136
"roles/artifactregistry.writer",
137137
"roles/cloudbuild.builds.editor",
138-
"roles/cloudbuild.workerpools.use",
139138
"roles/developerconnect.tokenAccessor",
140139
"roles/logging.logWriter",
141140
"roles/run.developer",
@@ -174,14 +173,21 @@ func setPermissionsForCloudBuildSA(ctx context.Context, projectID, serviceAccoun
174173
}
175174
}
176175

177-
// 4. Role for the Cloud Build Service Agent on the Cloud Run SA (Default Compute SA)
176+
// 4. Grant Cloud Build Service Agent permissions to impersonate the Cloud Run SA (Default Compute SA)
178177
defaultComputeSA := fmt.Sprintf("serviceAccount:%d-compute@developer.gserviceaccount.com", projectNumber)
179178
resource := fmt.Sprintf("projects/%s/serviceAccounts/%s", projectID, strings.TrimPrefix(defaultComputeSA, "serviceAccount:"))
180179
_, err = iamClient.AddIAMRoleBinding(ctx, resource, "roles/iam.serviceAccountUser", gcbP4sa)
181180
if err != nil {
182181
return "", fmt.Errorf("unable to add iam.serviceAccountUser role to Cloud Build P4SA %s on SA %s err: %w", gcbP4sa, defaultComputeSA, err)
183182
}
184183

184+
// 5. Grant Cloud Build Service Agent permissions to impersonate the SA passed in (SA that is going to run the build)
185+
r := fmt.Sprintf("projects/%s/serviceAccounts/%s", projectID, strings.TrimPrefix(resolvedSA, "serviceAccount:"))
186+
_, err = iamClient.AddIAMRoleBinding(ctx, r, "roles/iam.serviceAccountUser", gcbP4sa)
187+
if err != nil {
188+
return "", fmt.Errorf("unable to add iam.serviceAccountUser role to Cloud Build P4SA %s on SA %s err: %w", gcbP4sa, r, err)
189+
}
190+
185191
return resolvedSA, nil
186192
}
187193

cicd-mcp-server/cloudbuild/cloudbuild_test.go

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
4242
gcbSARoles := []string{
4343
"roles/artifactregistry.writer",
4444
"roles/cloudbuild.builds.editor",
45-
"roles/cloudbuild.workerpools.use",
4645
"roles/developerconnect.tokenAccessor",
4746
"roles/logging.logWriter",
4847
"roles/run.developer",
@@ -57,6 +56,7 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
5756
}
5857
defaultComputeSA := fmt.Sprintf("%d-compute@developer.gserviceaccount.com", projectNumber)
5958
saResource := fmt.Sprintf("projects/%s/serviceAccounts/%s", projectID, defaultComputeSA)
59+
userSAResource := fmt.Sprintf("projects/%s/serviceAccounts/%s", projectID, "test-sa@example.com")
6060

6161
t.Run("with service account", func(t *testing.T) {
6262
for _, r := range gcbSARoles {
@@ -68,6 +68,7 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
6868
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), r, gcbP4sa).Return(nil, nil)
6969
}
7070
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, saResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
71+
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, userSAResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
7172

7273
resolvedSA, err := setPermissionsForCloudBuildSA(ctx, projectID, serviceAccount, mockRMClient, mockIAMClient)
7374
assert.NoError(t, err)
@@ -84,6 +85,7 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
8485
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), r, gcbP4sa).Return(nil, nil)
8586
}
8687
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, saResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
88+
mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, userSAResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
8789

8890
resolvedSA, err := setPermissionsForCloudBuildSA(ctx, projectID, serviceAccountWOPrefix, mockRMClient, mockIAMClient)
8991
assert.NoError(t, err)
@@ -118,3 +120,27 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
118120
assert.Error(t, err)
119121
})
120122
}
123+
124+
func TestIsValidServiceAccount(t *testing.T) {
125+
tests := []struct {
126+
name string
127+
sa string
128+
want bool
129+
}{
130+
{"valid sa", "serviceAccount:test-sa@project.iam.gserviceaccount.com", true},
131+
{"valid sa with dashes", "serviceAccount:my-sa-123@my-project-456.iam.gserviceaccount.com", true},
132+
{"missing prefix", "test-sa@project.iam.gserviceaccount.com", false},
133+
{"wrong prefix", "user:test-sa@project.iam.gserviceaccount.com", false},
134+
{"missing domain", "serviceAccount:test-sa@project", false},
135+
{"wrong domain", "serviceAccount:test-sa@project.com", false},
136+
{"empty string", "", false},
137+
}
138+
139+
for _, tt := range tests {
140+
t.Run(tt.name, func(t *testing.T) {
141+
if got := IsValidServiceAccount(tt.sa); got != tt.want {
142+
t.Errorf("IsValidServiceAccount() = %v, want %v", got, tt.want)
143+
}
144+
})
145+
}
146+
}

skills/google-cicd-pipeline-design/references/how_to_create_cloudbuild_trigger.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,8 @@ Cloud Build triggers connect to source code via Developer Connect. The entire co
4444

4545
Cloud Build triggers should **always** use a dedicated, user-managed service account instead of the default Compute Engine service account to follow the principle of least privilege. **CRITICAL: NEVER use the default Compute Engine service account (typically formatted as `[PROJECT_NUMBER]-compute@developer.gserviceaccount.com`), even if you observe existing triggers in the project using it.**
4646

47-
1. **Check for Service Account**: Check if a dedicated service account for Cloud Build (e.g., `cloud-build-runner@<PROJECT_ID>.iam.gserviceaccount.com`) already exists.
48-
2. **Create Service Account (if needed)**: If no dedicated service account exists, create one using `gcloud iam service-accounts create`.
49-
3. **Required Roles (Ensured Automatically)**: The `create_build_trigger` MCP tool will **automatically ensure** the necessary roles (e.g., `roles/logging.logWriter`, `roles/artifactregistry.writer`, `roles/developerconnect.tokenAccessor`, `roles/run.developer`, `roles/storage.admin`, `roles/serviceusage.serviceUsageConsumer`, `roles/cloudbuild.builds.editor`, `roles/cloudbuild.workerpools.use`, and specific IAM delegation on the default Compute SA) are granted to the relevant service accounts. You **do not** need to grant these permissions manually.
50-
47+
1. **Create Service Account**: Create one using `gcloud iam service-accounts create`.
48+
2. **Required Roles (Ensured Automatically)**: Using the `create_build_trigger` MCP tool will **automatically ensure** the necessary roles are granted to the given service account. You **do not** need to grant these permissions manually.
5149
---
5250

5351
## Final Step: Creating the Trigger

0 commit comments

Comments
 (0)