fix: added DiscardCacheError in fetcher

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
notaryproject · Sep 20, 2024 · 00e8bce · 00e8bce
1 parent 7ed8899
commit 00e8bce
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 3 deletions.
diff --git a/revocation/crl/fetcher.go b/revocation/crl/fetcher.go
@@ -51,6 +51,9 @@ type HTTPFetcher struct {
  // If Cache is nil, no cache is used.
  Cache Cache
 
+ // DiscardCacheError specifies whether to discard the cache on error.
+ DiscardCacheError bool
+
  httpClient *http.Client
 }
 
@@ -84,7 +87,9 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*Bundle, error) {
  return bundle, nil
  }
  }
- // ignore the cache error as it is not critical
+ if err != nil && !f.DiscardCacheError {
+ return nil, fmt.Errorf("failed to retrieve CRL from cache: %w", err)
+ }
  }
 
  bundle, err := f.fetch(ctx, url)
@@ -93,8 +98,10 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*Bundle, error) {
  }
 
  if f.Cache != nil {
- // ignore the cache error as it is not critical
- _ = f.Cache.Set(ctx, url, bundle)
+ err = f.Cache.Set(ctx, url, bundle)
+ if err != nil && !f.DiscardCacheError {
+ return nil, fmt.Errorf("failed to store CRL to cache: %w", err)
+ }
  }
 
  return bundle, nil

diff --git a/revocation/crl/fetcher_test.go b/revocation/crl/fetcher_test.go
@@ -140,6 +140,7 @@ func TestFetch(t *testing.T) {
  t.Errorf("NewHTTPFetcher() error = %v, want nil", err)
  }
  f.Cache = c
+ f.DiscardCacheError = true
  bundle, err := f.Fetch(context.Background(), uncachedURL)
  if err != nil {
  t.Errorf("Fetcher.Fetch() error = %v, want nil", err)
@@ -182,6 +183,7 @@ func TestFetch(t *testing.T) {
  t.Errorf("NewHTTPFetcher() error = %v, want nil", err)
  }
  f.Cache = c
+ f.DiscardCacheError = true
  bundle, err = f.Fetch(context.Background(), expiredCRLURL)
  if err != nil {
  t.Errorf("Fetcher.Fetch() error = %v, want nil", err)
@@ -218,6 +220,7 @@ func TestFetch(t *testing.T) {
  t.Errorf("NewHTTPFetcher() error = %v, want nil", err)
  }
  f.Cache = c
+ f.DiscardCacheError = true
  _, err = f.Fetch(context.Background(), uncachedURL)
  if !strings.Contains(err.Error(), "delta CRL is not supported") {
  t.Errorf("Fetcher.Fetch() error = %v, want delta CRL is not supported", err)
@@ -234,6 +237,7 @@ func TestFetch(t *testing.T) {
  t.Errorf("NewHTTPFetcher() error = %v, want nil", err)
  }
  f.Cache = c
+ f.DiscardCacheError = true
  bundle, err = f.Fetch(context.Background(), exampleURL)
  if err != nil {
  t.Errorf("Fetcher.Fetch() error = %v, want nil", err)

diff --git a/revocation/internal/crl/crl_test.go b/revocation/internal/crl/crl_test.go
@@ -127,6 +127,7 @@ func TestCertCheckStatus(t *testing.T) {
  t.Fatal(err)
  }
  fetcher.Cache = memoryCache
+ fetcher.DiscardCacheError = true
  r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{
  Fetcher: fetcher,
  })
@@ -166,6 +167,7 @@ func TestCertCheckStatus(t *testing.T) {
  }
 
  fetcher.Cache = memoryCache
+ fetcher.DiscardCacheError = true
  r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{
  Fetcher: fetcher,
  })
@@ -192,6 +194,7 @@ func TestCertCheckStatus(t *testing.T) {
  t.Fatal(err)
  }
  fetcher.Cache = memoryCache
+ fetcher.DiscardCacheError = true
  r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{
  Fetcher: fetcher,
  })
@@ -223,6 +226,7 @@ func TestCertCheckStatus(t *testing.T) {
  t.Fatal(err)
  }
  fetcher.Cache = memoryCache
+ fetcher.DiscardCacheError = true
  r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{
  Fetcher: fetcher,
  })
@@ -264,6 +268,7 @@ func TestCertCheckStatus(t *testing.T) {
  t.Fatal(err)
  }
  fetcher.Cache = memoryCache
+ fetcher.DiscardCacheError = true
  r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{
  Fetcher: fetcher,
  })
@@ -285,6 +290,7 @@ func TestCertCheckStatus(t *testing.T) {
  t.Fatal(err)
  }
  fetcher.Cache = memoryCache
+ fetcher.DiscardCacheError = true
  r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{
  Fetcher: fetcher,
  })
@@ -314,6 +320,7 @@ func TestCertCheckStatus(t *testing.T) {
  t.Fatal(err)
  }
  fetcher.Cache = memoryCache
+ fetcher.DiscardCacheError = true
  r := CertCheckStatus(context.Background(), chain[0].Cert, issuerCert, CertCheckStatusOptions{
  Fetcher: fetcher,
  })