feat(auth): add AWS security credentials supplier support#5595
feat(auth): add AWS security credentials supplier support#5595Linkgoron wants to merge 1 commit intogoogleapis:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces support for AWS Workload Identity Federation by adding the AwsSecurityCredentialsSupplier trait and the AwsExternalAccountBuilder. It refactors the existing AWS credential resolution logic to support custom suppliers and includes new tests for these features. Review feedback suggests deriving Debug for the new builder to maintain consistency with repository style guides and recommends addressing an inconsistency in ProgrammaticBuilder regarding universe_domain handling.
| assert_eq!( | ||
| config.service_account_impersonation_url, | ||
| Some("https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/test-principal:generateAccessToken".to_string()) | ||
| ); |
There was a problem hiding this comment.
This test codifies an inconsistency where ProgrammaticBuilder ignores the universe_domain when generating the service account impersonation URL. While AwsExternalAccountBuilder correctly respects the universe domain (as seen in the test at line 2137), ProgrammaticBuilder::with_target_principal hardcodes googleapis.com. To fix this, ProgrammaticBuilder should be updated to use the deferred resolution logic now available in ExternalAccountConfigBuilder::with_target_principal. Note that per repository rules, a feature does not have to be fully implemented in a single pull request if the remaining work is documented in the PR description or as a TODO comment in the code.
References
- A feature does not have to be fully implemented in a single pull request if the remaining work is documented in the PR description or as a TODO comment in the code, allowing for continuous improvement.
There was a problem hiding this comment.
This is true, the problem here is that I didn't want to change existing behavior, just add my enhancement. if it's fine that this PR also fixes this bug - I can fix this. Or is the issue just the existence of the test?
Add a programmatic AWS external account builder that accepts caller-provided AWS region and security credentials. This lets applications delegate AWS credential resolution to the AWS SDK while google-cloud-auth builds the AWS subject token and performs the Google token exchange. Also add supplier-path tests and censor sensitive AWS credential fields in Debug output.
493ed55 to
f11f06e
Compare
This PR adds a programmatic AWS external account builder that accepts caller-provided AWS region and security credentials. This lets applications delegate AWS credential resolution to the AWS SDK while google-cloud-auth builds the AWS subject token and performs the Google token exchange.
This, hopefully, adds similar behavior to the ones that already exist in the Go, Python or Node SDKs.
Also add supplier-path tests and censor sensitive AWS credential fields in Debug output.