diff --git a/auth/github/client_test.go b/auth/github/client_test.go index 5dcdf6a2..d845ee45 100644 --- 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 index 5f1e25b9..7d5fcb93 100644 --- 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 index 699402d2..b4f10bd6 100644 --- 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 index aeadd19e..c961f098 100644 --- 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 index e2243756..53cb2663 100644 --- 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 index 37f7293e..1749637f 100644 --- 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 index de3fab74..5b550b38 100644 --- 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 index 31c7395d..e7ff026c 100644 --- 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 index 286130a5..f07800a2 100644 --- 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 index b52a6936..1e0295d6 100644 --- 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 index 0fb26426..405c7ef1 100644 --- 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 index ce2c02f7..ee2cf1dd 100644 --- 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 index 8aced37e..92ee1f5d 100644 --- 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=