Skip to content

fix(auth): avoid redundant retrieveSubjectToken call in AwsCredentials with service account impersonation - #14447

Draft
lsirac wants to merge 1 commit into
mainfrom
fix/aws-credentials-redundant-subject-token
Draft

lsirac wants to merge 1 commit into
mainfrom
fix/aws-credentials-redundant-subject-token

Conversation

@lsirac

@lsirac lsirac commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

When service account impersonation is configured on AwsCredentials, refreshAccessToken() previously called retrieveSubjectToken() on the outer credential before exchangeExternalCredentialForAccessToken() delegated to impersonatedCredentials.refreshAccessToken(). This queried the AWS metadata server or AwsSecurityCredentialsSupplier twice and discarded the first signed GetCallerIdentity subject token.

AwsCredentials.refreshAccessToken() now checks getServiceAccountImpersonationUrl() != null and delegates directly to this.impersonatedCredentials.refreshAccessToken(), reusing the cached impersonatedCredentials instance across refreshes.

Testing

  • Unit tests: AwsCredentialsTest and ExternalAccountCredentialsTest (91 tests passing)
  • Live end-to-end manual testing with real AWS EC2 IMDSv2 temporary security credentials (byoid-test IAM role in AWS account 077071391996, us-east-2) against byoid-pool/providers/aws-pid2 on sts.googleapis.com, iamcredentials.googleapis.com, oauth2.googleapis.com/tokeninfo, and pubsub.googleapis.com: https://paste.googleplex.com/4757180109160448

…s when service account impersonation is configured

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

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.

Code Review

This pull request updates AwsCredentials to reuse cached impersonated credentials during access token refreshes, preventing redundant AWS metadata queries when the source STS token is still valid. The test suite has been updated to assert correct request counts and verify caching behavior. A review comment suggests simplifying the implementation in AwsCredentials by delegating directly to super.refreshAccessToken() instead of duplicating the lazy initialization and delegation logic.

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.

1 participant