Merge pull request #877 from fluxcd/token-cache-max-duration

Add max duration to token cache
fluxcd · Mar 4, 2025 · 6be31fc · 6be31fc
2 parents ff04927 + 1ec89e0
commit 6be31fc
Show file tree

Hide file tree

Showing 13 changed files with 151 additions and 27 deletions.
diff --git a/auth/github/client_test.go b/auth/github/client_test.go
@@ -156,6 +156,8 @@ func TestClient_Options(t *testing.T) {
 }
 
 func TestClient_GetToken(t *testing.T) {
+	g := NewWithT(t)
+
 	expiresAt := time.Now().UTC().Add(time.Hour)
 	tests := []struct {
 		name         string
@@ -180,14 +182,17 @@ func TestClient_GetToken(t *testing.T) {
 		{
 			name: "Get cached token",
 			opts: []OptFunc{func(client *Client) {
-				c := cache.NewTokenCache(1)
-				c.GetOrSet(context.Background(), client.buildCacheKey(), func(context.Context) (cache.Token, error) {
+				c, err := cache.NewTokenCache(1)
+				g.Expect(err).NotTo(HaveOccurred())
+				_, ok, err := c.GetOrSet(context.Background(), client.buildCacheKey(), func(context.Context) (cache.Token, error) {
 					return &AppToken{
 						Token:     "access-token",
 						ExpiresAt: expiresAt,
 					}, nil
 				})
-				client.cache = c
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(ok).To(BeFalse())
+				WithCache(c, "", "", "")(client)
 			}},
 			statusCode: http.StatusInternalServerError, // error status code to make the test fail if the token is not cached
 			wantAppToken: &AppToken{

diff --git a/auth/go.mod b/auth/go.mod
@@ -11,7 +11,7 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.0
 	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.2
 	github.com/bradleyfalzon/ghinstallation/v2 v2.14.0
-	github.com/fluxcd/pkg/cache v0.4.0
+	github.com/fluxcd/pkg/cache v0.5.0
 	github.com/fluxcd/pkg/ssh v0.17.0
 	github.com/onsi/gomega v1.36.2
 	golang.org/x/net v0.35.0

diff --git a/cache/store.go b/cache/store.go
@@ -47,6 +47,7 @@ type storeOptions struct {
 	interval       time.Duration
 	registerer     prometheus.Registerer
 	metricsPrefix  string
+	maxDuration    time.Duration
 	involvedObject *InvolvedObject
 	debugKey       string
 	debugValueFunc func(any) any
@@ -88,6 +89,14 @@ func WithMetricsPrefix(prefix string) Options {
 	}
 }
 
+// WithMaxDuration sets the maximum duration for the cache items.
+func WithMaxDuration(duration time.Duration) Options {
+	return func(o *storeOptions) error {
+		o.maxDuration = duration
+		return nil
+	}
+}
+
 // WithInvolvedObject sets the involved object for the cache metrics.
 func WithInvolvedObject(kind, name, namespace string) Options {
 	return func(o *storeOptions) error {

diff --git a/cache/token.go b/cache/token.go
@@ -21,6 +21,11 @@ import (
 	"time"
 )
 
+// TokenMaxDuration is the maximum duration that a token can have in the
+// TokenCache. This is used to cap the duration of tokens to avoid storing
+// tokens that are valid for too long.
+const TokenMaxDuration = time.Hour
+
 // Token is an interface that represents an access token that can be used
 // to authenticate with a cloud provider. The only common method is to get the
 // duration of the token, because different providers may have different ways to
@@ -45,7 +50,8 @@ type Token interface {
 // lifetime, which is the same strategy used by kubelet for rotating
 // ServiceAccount tokens.
 type TokenCache struct {
-	cache *LRU[*tokenItem]
+	cache       *LRU[*tokenItem]
+	maxDuration time.Duration
 }
 
 type tokenItem struct {
@@ -55,9 +61,20 @@ type tokenItem struct {
 }
 
 // NewTokenCache returns a new TokenCache with the given capacity.
-func NewTokenCache(capacity int, opts ...Options) *TokenCache {
-	cache, _ := NewLRU[*tokenItem](capacity, opts...)
-	return &TokenCache{cache: cache}
+func NewTokenCache(capacity int, opts ...Options) (*TokenCache, error) {
+	o := storeOptions{maxDuration: TokenMaxDuration}
+	o.apply(opts...)
+
+	if o.maxDuration > TokenMaxDuration {
+		o.maxDuration = TokenMaxDuration
+	}
+
+	cache, err := NewLRU[*tokenItem](capacity, opts...)
+	if err != nil {
+		return nil, err
+	}
+
+	return &TokenCache{cache, o.maxDuration}, nil
 }
 
 // GetOrSet returns the token for the given key if present and not expired, or
@@ -112,6 +129,10 @@ func (c *TokenCache) newItem(token Token) *tokenItem {
 	// Ref: https://github.com/kubernetes/kubernetes/blob/4032177faf21ae2f99a2012634167def2376b370/pkg/kubelet/token/token_manager.go#L172-L174
 	d := (token.GetDuration() * 8) / 10
 
+	if m := c.maxDuration; d > m {
+		d = m
+	}
+
 	mono := time.Now().Add(d)
 	unix := time.Unix(mono.Unix(), 0)
 

diff --git a/cache/token_test.go b/cache/token_test.go
@@ -18,6 +18,7 @@ package cache_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -35,11 +36,14 @@ func (t *testToken) GetDuration() time.Duration {
 }
 
 func TestTokenCache_Lifecycle(t *testing.T) {
+	t.Parallel()
+
 	g := NewWithT(t)
 
 	ctx := context.Background()
 
-	tc := cache.NewTokenCache(1)
+	tc, err := cache.NewTokenCache(1)
+	g.Expect(err).NotTo(HaveOccurred())
 
 	token, retrieved, err := tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) {
 		return &testToken{duration: 2 * time.Second}, nil
@@ -48,19 +52,108 @@ func TestTokenCache_Lifecycle(t *testing.T) {
 	g.Expect(retrieved).To(BeFalse())
 	g.Expect(err).To(BeNil())
 
-	time.Sleep(4 * time.Second)
+	token, retrieved, err = tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) { return nil, nil })
+
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(token).To(Equal(&testToken{duration: 2 * time.Second}))
+	g.Expect(retrieved).To(BeTrue())
+
+	time.Sleep(2 * time.Second)
 
 	token, retrieved, err = tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) {
-		return &testToken{duration: 100 * time.Second}, nil
+		return &testToken{duration: time.Hour}, nil
 	})
-	g.Expect(token).To(Equal(&testToken{duration: 100 * time.Second}))
+	g.Expect(token).To(Equal(&testToken{duration: time.Hour}))
 	g.Expect(retrieved).To(BeFalse())
 	g.Expect(err).To(BeNil())
+}
 
-	time.Sleep(2 * time.Second)
+func TestTokenCache_80PercentLifetime(t *testing.T) {
+	t.Parallel()
+
+	g := NewWithT(t)
+
+	ctx := context.Background()
+
+	tc, err := cache.NewTokenCache(1)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	token, retrieved, err := tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) {
+		return &testToken{duration: 5 * time.Second}, nil
+	})
+
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(token).To(Equal(&testToken{duration: 5 * time.Second}))
+	g.Expect(retrieved).To(BeFalse())
 
 	token, retrieved, err = tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) { return nil, nil })
-	g.Expect(token).To(Equal(&testToken{duration: 100 * time.Second}))
+
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(token).To(Equal(&testToken{duration: 5 * time.Second}))
 	g.Expect(retrieved).To(BeTrue())
-	g.Expect(err).To(BeNil())
+
+	time.Sleep(4 * time.Second)
+
+	token, retrieved, err = tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) {
+		return &testToken{duration: time.Hour}, nil
+	})
+
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(token).To(Equal(&testToken{duration: time.Hour}))
+	g.Expect(retrieved).To(BeFalse())
+}
+
+func TestTokenCache_MaxDuration(t *testing.T) {
+	t.Parallel()
+
+	g := NewWithT(t)
+
+	ctx := context.Background()
+
+	tc, err := cache.NewTokenCache(1, cache.WithMaxDuration(time.Second))
+	g.Expect(err).NotTo(HaveOccurred())
+
+	token, retrieved, err := tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) {
+		return &testToken{duration: time.Hour}, nil
+	})
+
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(token).To(Equal(&testToken{duration: time.Hour}))
+	g.Expect(retrieved).To(BeFalse())
+
+	token, retrieved, err = tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) { return nil, nil })
+
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(token).To(Equal(&testToken{duration: time.Hour}))
+	g.Expect(retrieved).To(BeTrue())
+
+	time.Sleep(2 * time.Second)
+
+	token, retrieved, err = tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) {
+		return &testToken{duration: 10 * time.Millisecond}, nil
+	})
+
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(token).To(Equal(&testToken{duration: 10 * time.Millisecond}))
+	g.Expect(retrieved).To(BeFalse())
+}
+
+func TestTokenCache_GetOrSet_Error(t *testing.T) {
+	t.Parallel()
+
+	g := NewWithT(t)
+
+	ctx := context.Background()
+
+	tc, err := cache.NewTokenCache(1)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	token, retrieved, err := tc.GetOrSet(ctx, "test", func(context.Context) (cache.Token, error) {
+		return nil, fmt.Errorf("failed")
+	})
+
+	g.Expect(err).To(HaveOccurred())
+	g.Expect(err).To(MatchError("failed"))
+	g.Expect(token).To(BeNil())
+	g.Expect(retrieved).To(BeFalse())
 }
diff --git a/git/go.mod b/git/go.mod
@@ -4,6 +4,7 @@ go 1.23.0
 
 replace (
 	github.com/fluxcd/pkg/auth => ../auth
+	github.com/fluxcd/pkg/cache => ../cache
 	github.com/fluxcd/pkg/ssh => ../ssh
 )
 
@@ -24,7 +25,7 @@ require (
 	github.com/bradleyfalzon/ghinstallation/v2 v2.14.0 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/cloudflare/circl v1.5.0 // indirect
-	github.com/fluxcd/pkg/cache v0.4.0 // indirect
+	github.com/fluxcd/pkg/cache v0.5.0 // indirect
 	github.com/go-logr/logr v1.4.2 // indirect
 	github.com/golang-jwt/jwt/v4 v4.5.1 // indirect
 	github.com/golang-jwt/jwt/v5 v5.2.1 // indirect

diff --git a/git/go.sum b/git/go.sum
@@ -26,8 +26,6 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1
 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78=
 github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc=
-github.com/fluxcd/pkg/cache v0.4.0 h1:Jzy1dT6WNPE+srNM9aKQ4MQ5HKfYhHqaXlT99AvNL0M=
-github.com/fluxcd/pkg/cache v0.4.0/go.mod h1:VNMLzJa62iDHfojoywykJ2pdUlgSjVzTLrOgkNlPxEo=
 github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
 github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
 github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI=

diff --git a/git/gogit/go.mod b/git/gogit/go.mod
@@ -4,6 +4,7 @@ go 1.23.0
 
 replace (
 	github.com/fluxcd/pkg/auth => ../../auth
+	github.com/fluxcd/pkg/cache => ../../cache
 	github.com/fluxcd/pkg/git => ../../git
 	github.com/fluxcd/pkg/gittestserver => ../../gittestserver
 	github.com/fluxcd/pkg/ssh => ../../ssh
@@ -40,7 +41,7 @@ require (
 	github.com/cloudflare/circl v1.5.0 // indirect
 	github.com/cyphar/filepath-securejoin v0.4.1 // indirect
 	github.com/emirpasic/gods v1.18.1 // indirect
-	github.com/fluxcd/pkg/cache v0.4.0 // indirect
+	github.com/fluxcd/pkg/cache v0.5.0 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
 	github.com/go-logr/logr v1.4.2 // indirect
 	github.com/gofrs/uuid v4.4.0+incompatible // indirect

diff --git a/git/gogit/go.sum b/git/gogit/go.sum
@@ -45,8 +45,6 @@ github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc
 github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ=
 github.com/fluxcd/gitkit v0.6.0 h1:iNg5LTx6ePo+Pl0ZwqHTAkhbUHxGVSY3YCxCdw7VIFg=
 github.com/fluxcd/gitkit v0.6.0/go.mod h1:svOHuKi0fO9HoawdK4HfHAJJseZDHHjk7I3ihnCIqNo=
-github.com/fluxcd/pkg/cache v0.4.0 h1:Jzy1dT6WNPE+srNM9aKQ4MQ5HKfYhHqaXlT99AvNL0M=
-github.com/fluxcd/pkg/cache v0.4.0/go.mod h1:VNMLzJa62iDHfojoywykJ2pdUlgSjVzTLrOgkNlPxEo=
 github.com/gliderlabs/ssh v0.3.8 h1:a4YXD1V7xMF9g5nTkdfnja3Sxy1PVDCj1Zg4Wb8vY6c=
 github.com/gliderlabs/ssh v0.3.8/go.mod h1:xYoytBv1sV0aL3CavoDuJIQNURXkkfPA/wxQ1pL1fAU=
 github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI=

diff --git a/git/internal/e2e/go.mod b/git/internal/e2e/go.mod
@@ -4,6 +4,7 @@ go 1.23.0
 
 replace (
 	github.com/fluxcd/pkg/auth => ../../../auth
+	github.com/fluxcd/pkg/cache => ../../../cache
 	github.com/fluxcd/pkg/git => ../../../git
 	github.com/fluxcd/pkg/git/gogit => ../../gogit
 	github.com/fluxcd/pkg/gittestserver => ../../../gittestserver
@@ -41,7 +42,7 @@ require (
 	github.com/cyphar/filepath-securejoin v0.4.1 // indirect
 	github.com/emirpasic/gods v1.18.1 // indirect
 	github.com/fluxcd/gitkit v0.6.0 // indirect
-	github.com/fluxcd/pkg/cache v0.4.0 // indirect
+	github.com/fluxcd/pkg/cache v0.5.0 // indirect
 	github.com/fluxcd/pkg/version v0.6.0 // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect

diff --git a/git/internal/e2e/go.sum b/git/internal/e2e/go.sum
@@ -49,8 +49,6 @@ github.com/fluxcd/gitkit v0.6.0 h1:iNg5LTx6ePo+Pl0ZwqHTAkhbUHxGVSY3YCxCdw7VIFg=
 github.com/fluxcd/gitkit v0.6.0/go.mod h1:svOHuKi0fO9HoawdK4HfHAJJseZDHHjk7I3ihnCIqNo=
 github.com/fluxcd/go-git-providers v0.22.0 h1:THHUcCZkFQBWE5FlipRJMpUwA9v/Vj+tf965guSRKnU=
 github.com/fluxcd/go-git-providers v0.22.0/go.mod h1:qkdWXdcA/9ILOkesIXB1rDtcvi3h1FgnmbCn84yYpTk=
-github.com/fluxcd/pkg/cache v0.4.0 h1:Jzy1dT6WNPE+srNM9aKQ4MQ5HKfYhHqaXlT99AvNL0M=
-github.com/fluxcd/pkg/cache v0.4.0/go.mod h1:VNMLzJa62iDHfojoywykJ2pdUlgSjVzTLrOgkNlPxEo=
 github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
 github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
 github.com/gliderlabs/ssh v0.3.8 h1:a4YXD1V7xMF9g5nTkdfnja3Sxy1PVDCj1Zg4Wb8vY6c=

diff --git a/oci/tests/integration/go.mod b/oci/tests/integration/go.mod
@@ -4,6 +4,7 @@ go 1.23.0
 
 replace (
 	github.com/fluxcd/pkg/auth => ../../../auth
+	github.com/fluxcd/pkg/cache => ../../../cache
 	github.com/fluxcd/pkg/git => ../../../git
 	github.com/fluxcd/pkg/git/gogit => ../../../git/gogit
 	github.com/fluxcd/pkg/oci => ../../
@@ -65,7 +66,7 @@ require (
 	github.com/emirpasic/gods v1.18.1 // indirect
 	github.com/evanphx/json-patch v5.7.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.9.0 // indirect
-	github.com/fluxcd/pkg/cache v0.4.0 // indirect
+	github.com/fluxcd/pkg/cache v0.5.0 // indirect
 	github.com/fluxcd/pkg/ssh v0.17.0 // indirect
 	github.com/fluxcd/pkg/version v0.6.0 // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect

diff --git a/oci/tests/integration/go.sum b/oci/tests/integration/go.sum
@@ -92,8 +92,6 @@ github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM=
 github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE=
 github.com/fluxcd/gitkit v0.6.0 h1:iNg5LTx6ePo+Pl0ZwqHTAkhbUHxGVSY3YCxCdw7VIFg=
 github.com/fluxcd/gitkit v0.6.0/go.mod h1:svOHuKi0fO9HoawdK4HfHAJJseZDHHjk7I3ihnCIqNo=
-github.com/fluxcd/pkg/cache v0.4.0 h1:Jzy1dT6WNPE+srNM9aKQ4MQ5HKfYhHqaXlT99AvNL0M=
-github.com/fluxcd/pkg/cache v0.4.0/go.mod h1:VNMLzJa62iDHfojoywykJ2pdUlgSjVzTLrOgkNlPxEo=
 github.com/fluxcd/pkg/gittestserver v0.16.0 h1:HXbxW6F24B3qgnkNm/UKz7Wpt1kKtmOsE2bVQUPWOhk=
 github.com/fluxcd/pkg/gittestserver v0.16.0/go.mod h1:sGjpkv/X1NkJs43PSjlUxKTCit84Y1YyYn4U5ywBbFo=
 github.com/fluxcd/pkg/ssh v0.17.0 h1:o+MgdM/OB8R/+KEc3W3ml/inEKZqCwT8V71dkbTAbm4=