Skip to content

feat(controller): make token cache ttl more dynamic#5684

Open
krancour wants to merge 1 commit intoakuity:mainfrom
krancour:krancour/improved-token-caching
Open

feat(controller): make token cache ttl more dynamic#5684
krancour wants to merge 1 commit intoakuity:mainfrom
krancour:krancour/improved-token-caching

Conversation

@krancour
Copy link
Member

Supersedes #5615

In many cases, though not all, we have access to a token's expiry date/time.

For such cases, it is safer to derive cache TTL durations from the expiry than to make assumptions about how long the token is valid for.

This also adds debug level logging with details of token expiry (if known) and corresponding cache entry TTL.

Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>
@krancour krancour added this to the v1.10.0 milestone Jan 31, 2026
@krancour krancour self-assigned this Jan 31, 2026
@krancour krancour requested review from a team as code owners January 31, 2026 20:26
@krancour krancour added kind/enhancement An entirely new feature area/security Has security implications and needs to be handled with great caution priority/normal This is the priority for most work area/controller Affects the (main) controller labels Jan 31, 2026
@netlify
Copy link

netlify bot commented Jan 31, 2026

Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit 85adb9f
🔍 Latest deploy log https://app.netlify.com/projects/docs-kargo-io/deploys/697e6579b7552800084e062c
😎 Deploy Preview https://deploy-preview-5684.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov
Copy link

codecov bot commented Jan 31, 2026

Codecov Report

❌ Patch coverage is 73.79310% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.24%. Comparing base (cfad6be) to head (85adb9f).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
...kg/credentials/gar/workload_identity_federation.go 66.66% 10 Missing ⚠️
pkg/credentials/ecr/managed_identity.go 66.66% 9 Missing ⚠️
pkg/credentials/ecr/access_key.go 69.23% 8 Missing ⚠️
pkg/credentials/github/app.go 76.92% 6 Missing ⚠️
pkg/credentials/gar/service_account_key.go 80.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5684      +/-   ##
==========================================
+ Coverage   56.15%   56.24%   +0.09%     
==========================================
  Files         448      448              
  Lines       37656    37759     +103     
==========================================
+ Hits        21145    21239      +94     
- Misses      15253    15263      +10     
+ Partials     1258     1257       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

// Cache the encoded token, preferring a TTL derived from the actual token
// expiry when available.
ttl := cache.DefaultExpiration
if !expiry.IsZero() {
Copy link
Contributor

Choose a reason for hiding this comment

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

We are doing this in multiple places, probably better to extract it to a helper:

func CalculateCacheTTL(expiry time.Time, margin time.Duration) time.Duration {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/controller Affects the (main) controller area/security Has security implications and needs to be handled with great caution kind/enhancement An entirely new feature priority/normal This is the priority for most work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants