-
Notifications
You must be signed in to change notification settings - Fork 972
Fix StsWebIdentityTokenFileCredentialsProvider not respecting prefetchTime and staleTime configuration #6650
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: master
Are you sure you want to change the base?
Fix StsWebIdentityTokenFileCredentialsProvider not respecting prefetchTime and staleTime configuration #6650
Conversation
| credentialsProviderLocal = | ||
|
|
||
| StsAssumeRoleWithWebIdentityCredentialsProvider.Builder internalBuilder = | ||
| StsAssumeRoleWithWebIdentityCredentialsProvider.builder() | ||
| .stsClient(builder.stsClient) | ||
| .refreshRequest(supplier) | ||
| .build(); | ||
| .refreshRequest(supplier); | ||
|
|
||
| internalBuilder.staleTime(this.staleTime()) | ||
| .prefetchTime(this.prefetchTime()) | ||
| .asyncCredentialUpdateEnabled(this.asyncCredentialUpdateEnabled()); | ||
|
|
||
| credentialsProviderLocal = internalBuilder.build(); |
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.
Any reason we need internalBuilder? Can we set the fields directly?
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.
Hmm no reason, yeah modified it to set the fields directly.
| @@ -131,6 +131,13 @@ public Duration prefetchTime() { | |||
| return prefetchTime; | |||
| } | |||
|
|
|||
| /** | |||
| * Whether the provider should fetch credentials asynchronously in the background. | |||
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.
Can we specify the default value is false here too?
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.
Sure, updated.
|



Motivation and Context
Fixes GitHub issue #6185 where StsWebIdentityTokenFileCredentialsProvider ignores custom prefetchTime and staleTime configurations.
Modifications
staleTime,prefetchTime,asyncCredentialUpdateEnabled) to internalStsAssumeRoleWithWebIdentityCredentialsProvidersuper(constructor, provider)asyncCredentialUpdateEnabled()getter methodTesting
Added unit tests to validate custom configurations controls actual refresh behavior.
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License