Skip to content

Fix IAM client to use seperate method for add IAM service account per…#131

Merged
haroonc merged 1 commit intomainfrom
fix-iam
Apr 12, 2026
Merged

Fix IAM client to use seperate method for add IAM service account per…#131
haroonc merged 1 commit intomainfrom
fix-iam

Conversation

@yeshwanth1993
Copy link
Copy Markdown
Contributor

…missions

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

@yeshwanth1993 yeshwanth1993 requested a review from haroonc April 12, 2026 16:35
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Developer Connect service account identifier and introduces a specialized AddServiceAccountRoleBinding method to the IAM client for managing service-account-level permissions. The Cloud Build logic was updated to utilize this new method for service account impersonation. Feedback was provided to correct the indentation from spaces to tabs in the new implementation and to add a missing trailing newline to the file.

Comment on lines +153 to +194
// 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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The implementation of AddServiceAccountRoleBinding uses spaces for indentation instead of tabs. In Go, tabs are the standard for indentation. Additionally, the file is missing a newline at the end.

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

@haroonc haroonc merged commit 137dbeb into main Apr 12, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants