Skip to content
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

fix: timing of stale token refreshes on ComputeEngine #749

Merged
merged 8 commits into from Sep 30, 2021

Conversation

igorbernstein2
Copy link
Contributor

@igorbernstein2 igorbernstein2 commented Sep 28, 2021

ComputeEngine metadata server has its own token caching mechanism that will return a cached token
until the last 5 minutes of its expiration. This has a negative interaction with stale token refreshes because
stale token refresh kicks in T-6mins until T-5mins. This will cause every stale refresh to return the same
stale token.

This PR updates the timing for ComputeEngineCredentials to start a stale refresh at T-4mins and consider
the token expired at T-3 mins. The implementation is a bit noisy because it includes a change OAuth2Credentials
to make the thresholds configureable and now that we targeting java8, I migrated to using java8 time data types.

ComputeEngine metadata server has its own token caching mechanism that will return a cached token until the last 5 minutes of its expiration. This has a negative interaction with stale token refreshes because stale token refresh kicks in T-6mins until T-5mins. This will cause every stale refresh to return the same stale token.

This PR updates the timing for ComputeEngineCredentials to start a stale refresh at T-4mins and consider the token expired at T-3 mins. The implementation is a bit noisy because it includes a change OAuth2Credentials to make the thresholds configureable and now that we targeting java8, I migrated to using java8 time data types
@igorbernstein2 igorbernstein2 requested a review from a team as a code owner September 28, 2021 19:19
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 28, 2021
Copy link
Member

@TimurSadykov TimurSadykov left a comment

Choose a reason for hiding this comment

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

LGTM

@igorbernstein2 igorbernstein2 merged commit c813d55 into googleapis:main Sep 30, 2021
@igorbernstein2 igorbernstein2 deleted the refresh-time branch October 1, 2021 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants