Fix IAM client to use seperate method for add IAM service account permissions (#131)

Author: yeshwanth1993

cicd-mcp-server/cloudbuild/cloudbuild.go

@@ -154,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)
@@ -176,14 +176,14 @@ func setPermissionsForCloudBuildSA(ctx context.Context, projectID, serviceAccoun
 	// 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.AddIAMRoleBinding(ctx, r, "roles/iam.serviceAccountUser", gcbP4sa)
+	_, 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)
 	}

cicd-mcp-server/cloudbuild/cloudbuild_test.go

@@ -48,7 +48,7 @@ func TestSetPermissionsForCloudBuildSA(t *testing.T) {
 		"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",
@@ -67,8 +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().AddIAMRoleBinding(ctx, userSAResource, "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)
@@ -84,8 +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().AddIAMRoleBinding(ctx, userSAResource, "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)

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
+}