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 #2271: Refresh token every minute #4264

Merged
merged 4 commits into from
Jul 22, 2022

Conversation

Vlatombe
Copy link
Contributor

@Vlatombe Vlatombe commented Jul 8, 2022

Description

Should address #2271.
Platforms such as Amazon EKS since 1.21 rely on BoundServiceAccountTokenVolume to have time-limited access tokens. It is expected from kubernetes client implementations to refresh the configuration periodically to get a new token.

From kubernetes 1.21 release notes

Clients should reload the token from disk periodically (once per minute is recommended) to ensure they continue to use a valid token. k8s.io/client-go version v11.0.0+ and v0.15.0+ reload tokens automatically.

Prior work from #3105 addressed it once the token expired, however it doesn't address the current situation where the token is actually valid for 90 days, but expected to be refreshed at least every hour. This triggers audit logs events in AWS which can lead users to think their client will break once the token really expires.

This introduces a proactive aproach of refreshing the token from disk if the current token has been retrieved more than one minute ago.

For oidc implementation, I'm not knowledgeable enough to determine whether a more optimal solution could be implemented using some alternate validity timestamp.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@Vlatombe Vlatombe changed the title Refresh token every minute fix #2271: Refresh token every minute Jul 8, 2022
@Vlatombe Vlatombe marked this pull request as ready for review July 8, 2022 13:54
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

84.6% 84.6% Coverage
0.0% 0.0% Duplication

@manusa manusa added this to the 6.1.0 milestone Jul 11, 2022
@Vlatombe
Copy link
Contributor Author

@manusa Would it be possible to get a review on this work?

@manusa
Copy link
Member

manusa commented Jul 20, 2022

Yes, this is the next one going in, should be out in next 6.1.0 (with maybe some follow-ups too)

@apiwoni
Copy link

apiwoni commented Sep 22, 2022

I think this is more of a brute force approach to refresh service account token. Could we attempt to test for recommended expires_in before preemptively refreshing token or just let it fail and then refresh access token and retry?

manusa pushed a commit to manusa/kubernetes-client that referenced this pull request Sep 28, 2022
@manusa manusa removed the 5.12.x Backportable tentative label Sep 28, 2022
manusa pushed a commit that referenced this pull request Sep 28, 2022
@manusa
Copy link
Member

manusa commented Oct 7, 2022

thread continues here: #2271 (comment)

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.

4 participants