diff --git a/cicd-mcp-server/cloudbuild/cloudbuild.go b/cicd-mcp-server/cloudbuild/cloudbuild.go index 1993b1c..d721517 100644 --- a/cicd-mcp-server/cloudbuild/cloudbuild.go +++ b/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) } diff --git a/cicd-mcp-server/cloudbuild/cloudbuild_test.go b/cicd-mcp-server/cloudbuild/cloudbuild_test.go index b176302..bbdac22 100644 --- a/cicd-mcp-server/cloudbuild/cloudbuild_test.go +++ b/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) diff --git a/cicd-mcp-server/iam/client/iamclient.go b/cicd-mcp-server/iam/client/iamclient.go index a8e24b7..9681ba3 100644 --- a/cicd-mcp-server/iam/client/iamclient.go +++ b/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 +} \ No newline at end of file diff --git a/cicd-mcp-server/iam/client/mocks/mock_iamclient.go b/cicd-mcp-server/iam/client/mocks/mock_iamclient.go index 08d0339..eb06d24 100644 --- a/cicd-mcp-server/iam/client/mocks/mock_iamclient.go +++ b/cicd-mcp-server/iam/client/mocks/mock_iamclient.go @@ -52,6 +52,21 @@ func (mr *MockIAMClientMockRecorder) AddIAMRoleBinding(ctx, resourceID, role, me return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddIAMRoleBinding", reflect.TypeOf((*MockIAMClient)(nil).AddIAMRoleBinding), ctx, resourceID, role, member) } +// AddServiceAccountRoleBinding mocks base method. +func (m *MockIAMClient) AddServiceAccountRoleBinding(ctx context.Context, resourceID, role, member string) (*iam.Policy, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddServiceAccountRoleBinding", ctx, resourceID, role, member) + ret0, _ := ret[0].(*iam.Policy) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AddServiceAccountRoleBinding indicates an expected call of AddServiceAccountRoleBinding. +func (mr *MockIAMClientMockRecorder) AddServiceAccountRoleBinding(ctx, resourceID, role, member interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddServiceAccountRoleBinding", reflect.TypeOf((*MockIAMClient)(nil).AddServiceAccountRoleBinding), ctx, resourceID, role, member) +} + // CreateServiceAccount mocks base method. func (m *MockIAMClient) CreateServiceAccount(ctx context.Context, projectID, displayName, accountID string) (*iam.ServiceAccount, error) { m.ctrl.T.Helper()