Syncd SA changes

cicd-mcp-server/cloudbuild/cloudbuild.go

@@ -135,7 +135,6 @@ func setPermissionsForCloudBuildSA(ctx context.Context, projectID, serviceAccoun
 	gcbSARoles := []string{
 		"roles/artifactregistry.writer",
 		"roles/cloudbuild.builds.editor",
-		"roles/cloudbuild.workerpools.use",
 		"roles/developerconnect.tokenAccessor",
 		"roles/logging.logWriter",
 		"roles/run.developer",
@@ -155,7 +154,7 @@ func setPermissionsForCloudBuildSA(ctx context.Context, projectID, serviceAccoun
 	}
 
 	// 2. Roles for the Developer Connect Service Agent (Project Level)
-	dcP4sa := fmt.Sprintf("serviceAccount:service-%d@gcp-sa-developerconnect.iam.gserviceaccount.com", projectNumber)
+	dcP4sa := fmt.Sprintf("serviceAccount:service-%d@gcp-sa-devconnect.iam.gserviceaccount.com", projectNumber)
 	_, err = iamClient.AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), "roles/secretmanager.admin", dcP4sa)
 	if err != nil {
 		return "", fmt.Errorf("unable to add secretmanager.admin role to Developer Connect P4SA %s: %w", dcP4sa, err)
@@ -174,14 +173,21 @@ func setPermissionsForCloudBuildSA(ctx context.Context, projectID, serviceAccoun
 		}
 	}
 
-	// 4. Role for the Cloud Build Service Agent on the Cloud Run SA (Default Compute SA)
+	// 4. Grant Cloud Build Service Agent permissions to impersonate the Cloud Run SA (Default Compute SA)
 	defaultComputeSA := fmt.Sprintf("serviceAccount:%d-compute@developer.gserviceaccount.com", projectNumber)
 	resource := fmt.Sprintf("projects/%s/serviceAccounts/%s", projectID, strings.TrimPrefix(defaultComputeSA, "serviceAccount:"))
-	_, err = iamClient.AddIAMRoleBinding(ctx, resource, "roles/iam.serviceAccountUser", gcbP4sa)
+	_, err = iamClient.AddServiceAccountRoleBinding(ctx, resource, "roles/iam.serviceAccountUser", gcbP4sa)
 	if err != nil {
 		return "", fmt.Errorf("unable to add iam.serviceAccountUser role to Cloud Build P4SA %s on SA %s err: %w", gcbP4sa, defaultComputeSA, err)
 	}
 
+	// 5. Grant Cloud Build Service Agent permissions to impersonate the SA passed in (SA that is going to run the build)
+	r := fmt.Sprintf("projects/%s/serviceAccounts/%s", projectID, strings.TrimPrefix(resolvedSA, "serviceAccount:"))
+	_, err = iamClient.AddServiceAccountRoleBinding(ctx, r, "roles/iam.serviceAccountUser", gcbP4sa)
+	if err != nil {
+		return "", fmt.Errorf("unable to add iam.serviceAccountUser role to Cloud Build P4SA %s on SA %s err: %w", gcbP4sa, r, err)
+	}
+
 	return resolvedSA, nil
 }
 

cicd-mcp-server/cloudbuild/cloudbuild_test.go

@@ -42,21 +42,21 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
 	gcbSARoles := []string{
 		"roles/artifactregistry.writer",
 		"roles/cloudbuild.builds.editor",
-		"roles/cloudbuild.workerpools.use",
 		"roles/developerconnect.tokenAccessor",
 		"roles/logging.logWriter",
 		"roles/run.developer",
 		"roles/serviceusage.serviceUsageConsumer",
 		"roles/storage.admin",
 	}
-	dcP4sa := fmt.Sprintf("serviceAccount:service-%d@gcp-sa-developerconnect.iam.gserviceaccount.com", projectNumber)
+	dcP4sa := fmt.Sprintf("serviceAccount:service-%d@gcp-sa-devconnect.iam.gserviceaccount.com", projectNumber)
 	gcbP4sa := fmt.Sprintf("serviceAccount:service-%d@gcp-sa-cloudbuild.iam.gserviceaccount.com", projectNumber)
 	gcbP4saRoles := []string{
 		"roles/cloudbuild.serviceAgent",
 		"roles/developerconnect.tokenAccessor",
 	}
 	defaultComputeSA := fmt.Sprintf("%d-compute@developer.gserviceaccount.com", projectNumber)
 	saResource := fmt.Sprintf("projects/%s/serviceAccounts/%s", projectID, defaultComputeSA)
+	userSAResource := fmt.Sprintf("projects/%s/serviceAccounts/%s", projectID, "test-sa@example.com")
 
 	t.Run("with service account", func(t *testing.T) {
 		for _, r := range gcbSARoles {
@@ -67,7 +67,8 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
 		for _, r := range gcbP4saRoles {
 			mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), r, gcbP4sa).Return(nil, nil)
 		}
-		mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, saResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
+		mockIAMClient.EXPECT().AddServiceAccountRoleBinding(ctx, saResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
+		mockIAMClient.EXPECT().AddServiceAccountRoleBinding(ctx, userSAResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
 
 		resolvedSA, err := setPermissionsForCloudBuildSA(ctx, projectID, serviceAccount, mockRMClient, mockIAMClient)
 		assert.NoError(t, err)
@@ -83,7 +84,8 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
 		for _, r := range gcbP4saRoles {
 			mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, fmt.Sprintf("projects/%s", projectID), r, gcbP4sa).Return(nil, nil)
 		}
-		mockIAMClient.EXPECT().AddIAMRoleBinding(ctx, saResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
+		mockIAMClient.EXPECT().AddServiceAccountRoleBinding(ctx, saResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
+		mockIAMClient.EXPECT().AddServiceAccountRoleBinding(ctx, userSAResource, "roles/iam.serviceAccountUser", gcbP4sa).Return(nil, nil)
 
 		resolvedSA, err := setPermissionsForCloudBuildSA(ctx, projectID, serviceAccountWOPrefix, mockRMClient, mockIAMClient)
 		assert.NoError(t, err)
@@ -118,3 +120,27 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
 		assert.Error(t, err)
 	})
 }
+
+func TestIsValidServiceAccount(t *testing.T) {
+	tests := []struct {
+		name string
+		sa   string
+		want bool
+	}{
+		{"valid sa", "serviceAccount:test-sa@project.iam.gserviceaccount.com", true},
+		{"valid sa with dashes", "serviceAccount:my-sa-123@my-project-456.iam.gserviceaccount.com", true},
+		{"missing prefix", "test-sa@project.iam.gserviceaccount.com", false},
+		{"wrong prefix", "user:test-sa@project.iam.gserviceaccount.com", false},
+		{"missing domain", "serviceAccount:test-sa@project", false},
+		{"wrong domain", "serviceAccount:test-sa@project.com", false},
+		{"empty string", "", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := IsValidServiceAccount(tt.sa); got != tt.want {
+				t.Errorf("IsValidServiceAccount() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

cicd-mcp-server/iam/client/iamclient.go

@@ -35,10 +35,11 @@ type RoleBindingList struct {
 
 // Client is an interface for interacting with the IAM API.
 type IAMClient interface {
-	CreateServiceAccount(ctx context.Context, projectID, displayName, accountID string) (*iamv1.ServiceAccount, error)
-	AddIAMRoleBinding(ctx context.Context, resourceID, role, member string) (*cloudresourcemanagerv1.Policy, error)
-	ListServiceAccounts(ctx context.Context, projectID string) (*ServiceAccountList, error)
-	GetIAMRoleBinding(ctx context.Context, projectID, serviceAccountEmail string) (*RoleBindingList, error)
+    CreateServiceAccount(ctx context.Context, projectID, displayName, accountID string) (*iamv1.ServiceAccount, error)
+    AddIAMRoleBinding(ctx context.Context, resourceID, role, member string) (*cloudresourcemanagerv1.Policy, error) // Project-level
+    AddServiceAccountRoleBinding(ctx context.Context, resourceID, role, member string) (*iamv1.Policy, error) // SA-level
+    ListServiceAccounts(ctx context.Context, projectID string) (*ServiceAccountList, error)
+    GetIAMRoleBinding(ctx context.Context, projectID, serviceAccountEmail string) (*RoleBindingList, error)
 }
 
 // clientImpl is a client for interacting with the IAM API.
@@ -148,3 +149,46 @@ func (c *IAMClientImpl) GetIAMRoleBinding(ctx context.Context, projectID, servic
 
 	return &RoleBindingList{Items: roles}, nil
 }
+
+// AddServiceAccountRoleBinding adds a member to a role on a specific SA and returns the updated Policy.
+func (c *IAMClientImpl) AddServiceAccountRoleBinding(ctx context.Context, resourceID, role, member string) (*iamv1.Policy, error) {
+    // 1. Get the current policy (includes the ETag)
+    policy, err := c.iamService.Projects.ServiceAccounts.GetIamPolicy(resourceID).Context(ctx).Do()
+    if err != nil {
+        return nil, fmt.Errorf("failed to get iam policy: %w", err)
+    }
+
+    // 2. Modify the policy
+    updated := false
+    for _, binding := range policy.Bindings {
+        if binding.Role == role {
+            for _, m := range binding.Members {
+                if m == member {
+                    return policy, nil // Already exists, exit early to save an API call
+                }
+            }
+            binding.Members = append(binding.Members, member)
+            updated = true
+            break
+        }
+    }
+
+    if !updated {
+        policy.Bindings = append(policy.Bindings, &iamv1.Binding{
+            Role:    role,
+            Members: []string{member},
+        })
+    }
+
+    // 3. Set the policy (The ETag in 'policy' ensures we don't overwrite concurrent changes)
+    request := &iamv1.SetIamPolicyRequest{
+        Policy: policy,
+    }
+
+    updatedPolicy, err := c.iamService.Projects.ServiceAccounts.SetIamPolicy(resourceID, request).Context(ctx).Do()
+    if err != nil {
+        return nil, fmt.Errorf("failed to set iam policy: %w", err)
+    }
+
+    return updatedPolicy, nil
+}

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

@@ -44,10 +44,8 @@ Cloud Build triggers connect to source code via Developer Connect. The entire co
 
 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.**
 
-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.
-2.  **Create Service Account (if needed)**: If no dedicated service account exists, create one using `gcloud iam service-accounts create`.
-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.
-
+1.  **Create Service Account**: Create one using `gcloud iam service-accounts create`.
+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.
 ---
 
 ## Final Step: Creating the Trigger