From 8e337cf6a7c12e56b1fd0008dc6869abc99a2d9c Mon Sep 17 00:00:00 2001 From: Valentin Knabel Date: Wed, 14 Aug 2024 11:29:23 +0200 Subject: [PATCH] fix(cache): retry period (#50) closes metal-stack/metal-api#259 --- issuercache.go | 39 ++++++++++++++++++++++++------ issuercache_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/issuercache.go b/issuercache.go index 27465cb..57a0e95 100644 --- a/issuercache.go +++ b/issuercache.go @@ -19,6 +19,7 @@ type MultiIssuerCache struct { cache map[string]*Issuer cacheLock sync.RWMutex reloadInterval time.Duration + retryInterval time.Duration log *slog.Logger } @@ -41,6 +42,7 @@ func NewMultiIssuerCache(log *slog.Logger, ilp IssuerListProvider, ugp UserGette userGetterProvider: ugp, cache: make(map[string]*Issuer), reloadInterval: 30 * time.Minute, + retryInterval: 30 * time.Second, log: log, } @@ -48,27 +50,42 @@ func NewMultiIssuerCache(log *slog.Logger, ilp IssuerListProvider, ugp UserGette opt(issuerCache) } + var ( + // flush cache periodically + done = make(chan bool) + isRetrying bool + reloadTicker *time.Ticker + ) // initial update err := issuerCache.updateCache() if err != nil { issuerCache.log.Error("error updating issuer cache", "error", err) + isRetrying = true + reloadTicker = time.NewTicker(issuerCache.retryInterval) + } else { + isRetrying = false + reloadTicker = time.NewTicker(issuerCache.reloadInterval) } - // flush cache periodically - done := make(chan bool) - ticker := time.NewTicker(issuerCache.reloadInterval) - go func() { for { select { case <-done: - ticker.Stop() + reloadTicker.Stop() return - case <-ticker.C: + + case <-reloadTicker.C: issuerCache.log.Info("updating issuer cache") err := issuerCache.updateCache() if err != nil { - issuerCache.log.Error("error updating issuer cache", "error", err) + issuerCache.log.Error("error updating issuer cache, retrying...", "error", err) + isRetrying = true + reloadTicker.Reset(issuerCache.retryInterval) + continue + } + if isRetrying { + isRetrying = false + reloadTicker.Reset(issuerCache.reloadInterval) } } } @@ -88,6 +105,14 @@ func IssuerReloadInterval(duration time.Duration) MultiIssuerUserGetterOption { } } +// IssuerRetryInterval lets the client set the issuer cache retry sleep period +func IssuerRetryInterval(duration time.Duration) MultiIssuerUserGetterOption { + return func(o *MultiIssuerCache) *MultiIssuerCache { + o.retryInterval = duration + return o + } +} + func (i *MultiIssuerCache) User(rq *http.Request) (*User, error) { claims, err := ParseTokenClaimsUnvalidated(rq) diff --git a/issuercache_test.go b/issuercache_test.go index f0493aa..ee90cd3 100644 --- a/issuercache_test.go +++ b/issuercache_test.go @@ -276,6 +276,65 @@ func TestMultiIssuerCache_reload(t *testing.T) { assert.Len(t, ic.cache, 1) } +func TestMultiIssuerCache_retryFailing(t *testing.T) { + ugp := func(ic *IssuerConfig) (UserGetter, error) { + return nil, nil + } + + calls := 0 + + ilp := func() ([]*IssuerConfig, error) { + calls++ + return nil, errors.New("expected error") + } + ic, err := NewMultiIssuerCache(slog.New(slog.NewJSONHandler(os.Stdout, nil)), ilp, ugp, IssuerReloadInterval(5*time.Second), IssuerRetryInterval(500*time.Millisecond)) + require.NoError(t, err) + assert.Equal(t, 1, calls) + assert.Empty(t, ic.cache) + + // wait for reload + time.Sleep(2 * time.Second) + + assert.Equal(t, 4, calls) + assert.Empty(t, ic.cache) +} + +func TestMultiIssuerCache_retrySecondReload(t *testing.T) { + ugp := func(ic *IssuerConfig) (UserGetter, error) { + return nil, nil + } + + calls := 0 + issuerList := []*IssuerConfig{} + + ilp := func() ([]*IssuerConfig, error) { + calls++ + if calls > 1 { + return issuerList, nil + } + return nil, errors.New("expected error") + } + ic, err := NewMultiIssuerCache(slog.New(slog.NewJSONHandler(os.Stdout, nil)), ilp, ugp, IssuerReloadInterval(1*time.Second), IssuerRetryInterval(500*time.Millisecond)) + require.NoError(t, err) + assert.Equal(t, 1, calls) + assert.Empty(t, ic.cache) + + // prepare list + issuerList = []*IssuerConfig{ + { + Annotations: nil, + Tenant: "t1", + Issuer: "http://issuer/t1", + ClientID: "cli-t1", + }, + } + // wait for reload + time.Sleep(2 * time.Second) + + assert.Equal(t, 3, calls) + assert.Len(t, ic.cache, 1) +} + func TestMultiIssuerCache_syncCache(t *testing.T) { type fields struct { ilp IssuerListProvider