From e61edaa5b32ef976d694cfe6ed85eae413d3f4db Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 13 Aug 2026 16:01:50 +0100 Subject: [PATCH] R59: honor backend TTL on cache repopulation --- review_r59_backend_ttl_test.go | 63 ++++++++++++++++++++++++++++++++++ universal_cache.go | 18 ++++++---- 2 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 review_r59_backend_ttl_test.go diff --git a/review_r59_backend_ttl_test.go b/review_r59_backend_ttl_test.go new file mode 100644 index 0000000..b4323a9 --- /dev/null +++ b/review_r59_backend_ttl_test.go @@ -0,0 +1,63 @@ +package traefikoidc + +import ( + "context" + "testing" + "time" +) + +// stubBackend implements backends.CacheBackend with a controllable TTL so the +// UniversalCache.Get re-population path can be tested deterministically. +type stubBackend struct { + val []byte + ttl time.Duration + exists bool +} + +func (s *stubBackend) Set(ctx context.Context, key string, value []byte, ttl time.Duration) error { + return nil +} +func (s *stubBackend) Get(ctx context.Context, key string) ([]byte, time.Duration, bool, error) { + return s.val, s.ttl, s.exists, nil +} +func (s *stubBackend) Delete(ctx context.Context, key string) (bool, error) { return true, nil } +func (s *stubBackend) Exists(ctx context.Context, key string) (bool, error) { return s.exists, nil } +func (s *stubBackend) Clear(ctx context.Context) error { return nil } +func (s *stubBackend) GetStats() map[string]interface{} { return nil } +func (s *stubBackend) Close() error { return nil } +func (s *stubBackend) Ping(ctx context.Context) error { return nil } + +// TestUniversalCacheBackendRepopulateKeepsRealTTL guards against Get() re-caching a +// backend value under the federated DefaultTTL instead of the entry's real +// remaining TTL. The old code re-populated the local LRU with DefaultTTL +// (often 1h), so once the distributed backend became unreachable the local +// fast-path served data whose authoritative TTL had already lapsed (e.g. a +// 5-minute token served for up to an hour). +func TestUniversalCacheBackendRepopulateKeepsRealTTL(t *testing.T) { + backend := &stubBackend{ + val: []byte("42"), + ttl: 5 * time.Second, + exists: true, + } + + cache := NewUniversalCacheWithBackend(UniversalCacheConfig{ + Type: CacheTypeMetadata, + DefaultTTL: time.Hour, + MaxSize: 100, + Logger: NewLogger(DefaultLogLevel), + }, backend) + defer cache.Close() + + if _, ok := cache.Get("key"); !ok { + t.Fatalf("backend hit should return a value") + } + + item := cache.items["key"] + if item == nil { + t.Fatalf("local cache should have been populated from backend") + } + remaining := time.Until(item.ExpiresAt) + if remaining > 10*time.Second || remaining <= 0 { + t.Fatalf("local cache TTL should track backend real TTL (~5s), got %v (old code used DefaultTTL 1h)", remaining) + } +} diff --git a/universal_cache.go b/universal_cache.go index de9bc67..02b7c43 100644 --- a/universal_cache.go +++ b/universal_cache.go @@ -340,7 +340,7 @@ func (c *UniversalCache) Get(key string) (interface{}, bool) { ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) defer cancel() - data, _, exists, err := c.backend.Get(ctx, c.prefixKey(key)) + data, ttl, exists, err := c.backend.Get(ctx, c.prefixKey(key)) if err != nil { c.logger.Debugf("Backend get error for key %s: %v", key, err) // Fall through to local cache @@ -352,11 +352,17 @@ func (c *UniversalCache) Get(key string) (interface{}, bool) { // Fall through to local cache } else { atomic.AddInt64(&c.hits, 1) - // Update local cache with backend value synchronously. - // Under yaegi, goroutine spawn is 5-10x costlier than compiled Go, - // and this path fires per-request on cold local cache. - // updateLocalCache is cheap (map write under mutex). - _ = c.updateLocalCache(key, value, c.config.DefaultTTL) + // Re-populate local cache with the backend entry's REAL remaining + // TTL, not the federated DefaultTTL, so the in-memory copy + // cannot outlive the authoritative backend entry. Previously a + // short-TTL backend value (e.g. a 5-minute token) was cached + // locally for up to DefaultTTL (often 1h); if the backend then + // became unreachable, getLocal kept serving it well past its + // intended expiry. + if ttl <= 0 { + ttl = c.config.DefaultTTL + } + _ = c.updateLocalCache(key, value, ttl) return value, true } }