-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat: Custom Credential Supplier Documentation #10203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Custom Credential Supplier Documentation #10203
Conversation
|
Here is the summary of changes. You are about to add 4 region tags.
This comment is generated by snippet-bot.
|
Summary of ChangesHello @vverman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the authentication capabilities of the Google Cloud SDKs by introducing documentation and functional code samples for Custom Credential Suppliers. This feature allows developers to seamlessly integrate third-party identity providers, such as Okta and AWS, with Google Cloud Workload Identity Federation, which is crucial for hybrid or multi-cloud environments where standard authentication methods may not apply. The changes provide clear guidance and practical examples for setting up and utilizing these custom authentication flows. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this 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 introduces valuable documentation and sample code for using custom credential suppliers with AWS and Okta, which is a great addition for developers in multi-cloud or hybrid environments. The code is well-structured and the documentation is clear. I've provided a few suggestions to enhance thread-safety in the custom supplier implementations and to replace a deprecated method with a modern, safer alternative. Overall, this is an excellent contribution.
| public String getRegion(ExternalAccountSupplierContext context) { | ||
| if (this.region == null) { | ||
| Region awsRegion = new DefaultAwsRegionProviderChain().getRegion(); | ||
| if (awsRegion == null) { | ||
| throw new IllegalStateException( | ||
| "Unable to resolve AWS region. Ensure AWS_REGION is set or configured."); | ||
| } | ||
| this.region = awsRegion.id(); | ||
| } | ||
| return this.region; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getRegion method performs lazy initialization of the region field, but it's not thread-safe. If multiple threads call this method concurrently when region is null, they could all attempt to resolve the region, which is inefficient. Since GoogleCredentials objects can be shared across threads, their components should be thread-safe.
To ensure thread safety, you can add the synchronized keyword to the method signature.
| public String getRegion(ExternalAccountSupplierContext context) { | |
| if (this.region == null) { | |
| Region awsRegion = new DefaultAwsRegionProviderChain().getRegion(); | |
| if (awsRegion == null) { | |
| throw new IllegalStateException( | |
| "Unable to resolve AWS region. Ensure AWS_REGION is set or configured."); | |
| } | |
| this.region = awsRegion.id(); | |
| } | |
| return this.region; | |
| } | |
| @Override | |
| public synchronized String getRegion(ExternalAccountSupplierContext context) { | |
| if (this.region == null) { | |
| Region awsRegion = new DefaultAwsRegionProviderChain().getRegion(); | |
| if (awsRegion == null) { | |
| throw new IllegalStateException( | |
| "Unable to resolve AWS region. Ensure AWS_REGION is set or configured."); | |
| } | |
| this.region = awsRegion.id(); | |
| } | |
| return this.region; | |
| } |
| public String getSubjectToken(ExternalAccountSupplierContext context) throws IOException { | ||
| // Check if the current token is still valid (with a 60-second buffer). | ||
| boolean isTokenValid = | ||
| this.accessToken != null | ||
| && this.expiryTime != null | ||
| && Instant.now().isBefore(this.expiryTime.minusSeconds(TOKEN_REFRESH_BUFFER_SECONDS)); | ||
|
|
||
| if (isTokenValid) { | ||
| return this.accessToken; | ||
| } | ||
|
|
||
| fetchOktaAccessToken(); | ||
| return this.accessToken; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getSubjectToken method caches and refreshes the Okta access token, but it's not thread-safe. If multiple threads call this method concurrently when the token is expired, they will all attempt to fetch a new token by calling fetchOktaAccessToken(). This creates a race condition where multiple tokens could be fetched unnecessarily.
Since GoogleCredentials objects are often shared across threads, their underlying suppliers should be thread-safe. You can easily fix this by making the method synchronized.
| public String getSubjectToken(ExternalAccountSupplierContext context) throws IOException { | |
| // Check if the current token is still valid (with a 60-second buffer). | |
| boolean isTokenValid = | |
| this.accessToken != null | |
| && this.expiryTime != null | |
| && Instant.now().isBefore(this.expiryTime.minusSeconds(TOKEN_REFRESH_BUFFER_SECONDS)); | |
| if (isTokenValid) { | |
| return this.accessToken; | |
| } | |
| fetchOktaAccessToken(); | |
| return this.accessToken; | |
| } | |
| @Override | |
| public synchronized String getSubjectToken(ExternalAccountSupplierContext context) throws IOException { | |
| // Check if the current token is still valid (with a 60-second buffer). | |
| boolean isTokenValid = | |
| this.accessToken != null | |
| && this.expiryTime != null | |
| && Instant.now().isBefore(this.expiryTime.minusSeconds(TOKEN_REFRESH_BUFFER_SECONDS)); | |
| if (isTokenValid) { | |
| return this.accessToken; | |
| } | |
| fetchOktaAccessToken(); | |
| return this.accessToken; | |
| } |
| try (DataOutputStream out = new DataOutputStream(conn.getOutputStream())) { | ||
| // Scopes define the permissions the access token will have. | ||
| // Update "gcp.test.read" to match your Okta configuration. | ||
| String params = "grant_type=client_credentials&scope=gcp.test.read"; | ||
| out.writeBytes(params); | ||
| out.flush(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method DataOutputStream.writeBytes() is deprecated and should be avoided. It does not properly convert characters to bytes and can lead to character encoding issues, especially with non-ASCII characters.
While it might work for the current params string, it's better to use a modern approach that explicitly handles character encoding. You can get the byte array from the string using a specific charset (like StandardCharsets.UTF_8) and write it to a raw OutputStream.
| try (DataOutputStream out = new DataOutputStream(conn.getOutputStream())) { | |
| // Scopes define the permissions the access token will have. | |
| // Update "gcp.test.read" to match your Okta configuration. | |
| String params = "grant_type=client_credentials&scope=gcp.test.read"; | |
| out.writeBytes(params); | |
| out.flush(); | |
| } | |
| try (java.io.OutputStream out = conn.getOutputStream()) { | |
| // Scopes define the permissions the access token will have. | |
| // Update "gcp.test.read" to match your Okta configuration. | |
| String params = "grant_type=client_credentials&scope=gcp.test.read"; | |
| out.write(params.getBytes(StandardCharsets.UTF_8)); | |
| out.flush(); | |
| } |
Description
Adding documentation for Custom Credential Suppliers.
Custom Credential Suppliers enable developers to securely integrate third-party authentication directly into the Google Cloud SDKs. Custom Credential Suppliers are primarily used to handle authentication in non-standard cloud environments.
The design and scopes for this are documented under this design doc
Checklist
pom.xmlparent set to latestshared-configurationmvn clean verifyrequiredmvn -P lint checkstyle:checkrequiredmvn -P lint clean compile pmd:cpd-check spotbugs:checkadvisory onlyThese tests will safely skip if the env variables aren't provided.
For the aws custom credential supplier, we need: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_REGION, GCP_WORKLOAD_AUDIENCE, GCS_BUCKET_NAME. Please refer to the auth/README.md under custom credential suppliers for AWS.
For the okta custom credential supplier, we need: OKTA_DOMAIN, OKTA_CLIENT_ID, OKTA_CLIENT_SECRET, GCP_WORKLOAD_AUDIENCE, GCS_BUCKET_NAME. Please refer to the auth/README.md under custom credential suppliers for Okta.