handling errros from thanos SG

alanprot · alanprot · commit 1828dcd6f0e4 · 2023-07-03T12:23:59.000-07:00
Signed-off-by: Alan Protasio &lt;alanprot@gmail.com&gt;
diff --git a/pkg/compactor/blocks_cleaner.go b/pkg/compactor/blocks_cleaner.go
@@ -325,7 +325,7 @@ func (c *BlocksCleaner) cleanUser(ctx context.Context, userID string, firstRun b
 	idx, err := bucketindex.ReadIndex(ctx, c.bucketClient, userID, c.cfgProvider, c.logger)
 	if errors.Is(err, bucketindex.ErrIndexCorrupted) {
 		level.Warn(userLogger).Log("msg", "found a corrupted bucket index, recreating it")
-	} else if errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	} else if errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
 		// Give up cleaning if we get access denied
 		level.Warn(userLogger).Log("msg", err.Error())
 		return nil
diff --git a/pkg/querier/blocks_finder_bucket_index.go b/pkg/querier/blocks_finder_bucket_index.go
@@ -65,7 +65,7 @@ func (f *BucketIndexBlocksFinder) GetBlocks(ctx context.Context, userID string,
 		return nil, nil, nil
 	}
 
-	if errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	if errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
 		return nil, nil, validation.AccessDeniedError(err.Error())
 	}
 
diff --git a/pkg/storage/bucket/client.go b/pkg/storage/bucket/client.go
@@ -41,7 +41,7 @@ var (
 
 	ErrUnsupportedStorageBackend = errors.New("unsupported storage backend")
 
-	ErrCustomerManagedKeyError = errors.New("access denied: customer key")
+	ErrCustomerManagedKeyAccessDenied = errors.New("access denied: customer key")
 )
 
 // Config holds configuration for accessing long-term storage.
diff --git a/pkg/storage/tsdb/bucketindex/loader.go b/pkg/storage/tsdb/bucketindex/loader.go
@@ -115,7 +115,7 @@ func (l *Loader) GetIndex(ctx context.Context, userID string) (*Index, error) {
 
 		if errors.Is(err, ErrIndexNotFound) {
 			level.Warn(l.logger).Log("msg", "bucket index not found", "user", userID)
-		} else if errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+		} else if errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
 			level.Warn(l.logger).Log("msg", "key access denied when reading bucket index", "user", userID)
 		} else {
 			// We don't track ErrIndexNotFound as failure because it's a legit case (eg. a tenant just
@@ -198,7 +198,7 @@ func (l *Loader) updateCachedIndex(ctx context.Context, userID string) {
 	l.loadAttempts.Inc()
 	startTime := time.Now()
 	idx, err := ReadIndex(readCtx, l.bkt, userID, l.cfgProvider, l.logger)
-	if err != nil && !errors.Is(err, ErrIndexNotFound) && !errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	if err != nil && !errors.Is(err, ErrIndexNotFound) && !errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
 		l.loadFailures.Inc()
 		level.Warn(l.logger).Log("msg", "unable to update bucket index", "user", userID, "err", err)
 		return
diff --git a/pkg/storage/tsdb/bucketindex/loader_test.go b/pkg/storage/tsdb/bucketindex/loader_test.go
@@ -603,7 +603,7 @@ func TestLoader_ShouldUpdateIndexInBackgroundOnPreviousKeyAcessDenied(t *testing
 	})
 
 	_, err := loader.GetIndex(ctx, user)
-	require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyError))
+	require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
 
 	// Check cached
 	require.NoError(t, loader.checkCachedIndexes(ctx))
@@ -669,7 +669,7 @@ func TestLoader_GetIndex_ShouldCacheKeyDeniedErrors(t *testing.T) {
 	// Request the index multiple times.
 	for i := 0; i < 10; i++ {
 		_, err := loader.GetIndex(ctx, "user-1")
-		require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyError))
+		require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
 	}
 
 	// Ensure metrics have been updated accordingly.
diff --git a/pkg/storage/tsdb/bucketindex/storage.go b/pkg/storage/tsdb/bucketindex/storage.go
@@ -13,6 +13,7 @@ import (
 	"github.com/cortexproject/cortex/pkg/storage/tsdb"
 
 	"github.com/cortexproject/cortex/pkg/storage/bucket"
+	cortex_errors "github.com/cortexproject/cortex/pkg/util/errors"
 	"github.com/cortexproject/cortex/pkg/util/runutil"
 )
 
@@ -33,7 +34,7 @@ func ReadIndex(ctx context.Context, bkt objstore.Bucket, userID string, cfgProvi
 		}
 
 		if userBkt.IsCustomerManagedKeyError(err) {
-			return nil, bucket.ErrCustomerManagedKeyError
+			return nil, cortex_errors.WithCause(bucket.ErrCustomerManagedKeyAccessDenied, err)
 		}
 
 		return nil, errors.Wrap(err, "read bucket index")
diff --git a/pkg/storage/tsdb/bucketindex/storage_test.go b/pkg/storage/tsdb/bucketindex/storage_test.go
@@ -2,6 +2,7 @@ package bucketindex
 
 import (
 	"context"
+	"errors"
 	"path"
 	"strings"
 	"testing"
@@ -47,7 +48,7 @@ func TestReadIndex_ShouldReturnErrorIfKeyAccessDeniedErr(t *testing.T) {
 		},
 	}
 	idx, err := ReadIndex(context.Background(), bkt, "user-1", nil, log.NewNopLogger())
-	require.Equal(t, bucket.ErrCustomerManagedKeyError, err)
+	require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
 	require.Nil(t, idx)
 }
 
diff --git a/pkg/storegateway/bucket_index_metadata_fetcher.go b/pkg/storegateway/bucket_index_metadata_fetcher.go
@@ -68,7 +68,7 @@ func (f *BucketIndexMetadataFetcher) Fetch(ctx context.Context) (metas map[ulid.
 	start := time.Now()
 	defer func() {
 		f.metrics.SyncDuration.Observe(time.Since(start).Seconds())
-		if err != nil && !errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+		if err != nil && !errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
 			f.metrics.SyncFailures.Inc()
 		}
 	}()
@@ -95,14 +95,14 @@ func (f *BucketIndexMetadataFetcher) Fetch(ctx context.Context) (metas map[ulid.
 		return nil, nil, nil
 	}
 
-	if errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	if errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
 		// stop the job and return the error
 		// this error should be used to return Access Denied to the caller
 		level.Error(f.logger).Log("msg", "bucket index key permission revoked", "user", f.userID, "err", err)
 		f.metrics.Synced.WithLabelValues(keyAccessDenied).Set(1)
 		f.metrics.Submit()
 
-		return nil, nil, bucket.ErrCustomerManagedKeyError
+		return nil, nil, err
 	}
 
 	if err != nil {
diff --git a/pkg/storegateway/bucket_index_metadata_fetcher_test.go b/pkg/storegateway/bucket_index_metadata_fetcher_test.go
@@ -3,6 +3,7 @@ package storegateway
 import (
 	"bytes"
 	"context"
+	"errors"
 	"path"
 	"strings"
 	"testing"
@@ -106,11 +107,11 @@ func TestBucketIndexMetadataFetcher_Fetch_KeyPermissionDenied(t *testing.T) {
 	reg := prometheus.NewPedanticRegistry()
 	ctx := context.Background()
 
-	bkt.MockGet(userID+"/bucket-index.json.gz", "c", bucket.ErrCustomerManagedKeyError)
+	bkt.MockGet(userID+"/bucket-index.json.gz", "c", bucket.ErrCustomerManagedKeyAccessDenied)
 
 	fetcher := NewBucketIndexMetadataFetcher(userID, bkt, NewNoShardingStrategy(), nil, log.NewNopLogger(), reg, nil)
 	metas, _, err := fetcher.Fetch(ctx)
-	require.ErrorIs(t, bucket.ErrCustomerManagedKeyError, err)
+	require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
 	assert.Empty(t, metas)
 
 	assert.NoError(t, testutil.GatherAndCompare(reg, bytes.NewBufferString(`
diff --git a/pkg/storegateway/bucket_stores.go b/pkg/storegateway/bucket_stores.go
@@ -35,6 +35,7 @@ import (
 	"github.com/cortexproject/cortex/pkg/storage/bucket"
 	"github.com/cortexproject/cortex/pkg/storage/tsdb"
 	"github.com/cortexproject/cortex/pkg/util/backoff"
+	cortex_errors "github.com/cortexproject/cortex/pkg/util/errors"
 	util_log "github.com/cortexproject/cortex/pkg/util/log"
 	"github.com/cortexproject/cortex/pkg/util/spanlogger"
 	"github.com/cortexproject/cortex/pkg/util/validation"
@@ -232,7 +233,7 @@ func (u *BucketStores) syncUsersBlocks(ctx context.Context, f func(context.Conte
 
 			for job := range jobs {
 				if err := f(ctx, job.store); err != nil {
-					if errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+					if errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
 						u.storesErrorsMu.Lock()
 						u.storesErrors[job.userID] = err
 						u.storesErrorsMu.Unlock()
@@ -304,7 +305,7 @@ func (u *BucketStores) Series(req *storepb.SeriesRequest, srv storepb.Store_Seri
 
 	err := u.getStoreError(userID)
 
-	if err != nil && errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	if err != nil && cortex_errors.ErrorIs(err, u.bucket.IsCustomerManagedKeyError) {
 		return httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", err)
 	}
 
@@ -313,7 +314,7 @@ func (u *BucketStores) Series(req *storepb.SeriesRequest, srv storepb.Store_Seri
 		ctx:                spanCtx,
 	})
 
-	if err != nil && errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	if err != nil && cortex_errors.ErrorIs(err, u.bucket.IsCustomerManagedKeyError) {
 		return httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", err)
 	}
 
@@ -337,13 +338,13 @@ func (u *BucketStores) LabelNames(ctx context.Context, req *storepb.LabelNamesRe
 
 	err := u.getStoreError(userID)
 
-	if err != nil && errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	if err != nil && cortex_errors.ErrorIs(err, u.bucket.IsCustomerManagedKeyError) {
 		return nil, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", err)
 	}
 
 	resp, err := store.LabelNames(ctx, req)
 
-	if err != nil && errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	if err != nil && cortex_errors.ErrorIs(err, u.bucket.IsCustomerManagedKeyError) {
 		return resp, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", err)
 	}
 
@@ -367,13 +368,13 @@ func (u *BucketStores) LabelValues(ctx context.Context, req *storepb.LabelValues
 
 	err := u.getStoreError(userID)
 
-	if err != nil && errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	if err != nil && cortex_errors.ErrorIs(err, u.bucket.IsCustomerManagedKeyError) {
 		return nil, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", err)
 	}
 
 	resp, err := store.LabelValues(ctx, req)
 
-	if err != nil && errors.Is(err, bucket.ErrCustomerManagedKeyError) {
+	if err != nil && cortex_errors.ErrorIs(err, u.bucket.IsCustomerManagedKeyError) {
 		return resp, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", err)
 	}
 
diff --git a/pkg/storegateway/bucket_stores_test.go b/pkg/storegateway/bucket_stores_test.go
@@ -76,24 +76,24 @@ func TestBucketStores_CustomerKeyError(t *testing.T) {
 
 	// Should set the error on user-1
 	require.NoError(t, stores.InitialSync(ctx))
-	require.ErrorIs(t, stores.storesErrors["user-1"], bucket.ErrCustomerManagedKeyError)
+	require.ErrorIs(t, stores.storesErrors["user-1"], bucket.ErrCustomerManagedKeyAccessDenied)
 	require.ErrorIs(t, stores.storesErrors["user-2"], nil)
 	require.NoError(t, stores.SyncBlocks(context.Background()))
-	require.ErrorIs(t, stores.storesErrors["user-1"], bucket.ErrCustomerManagedKeyError)
+	require.ErrorIs(t, stores.storesErrors["user-1"], bucket.ErrCustomerManagedKeyAccessDenied)
 	require.ErrorIs(t, stores.storesErrors["user-2"], nil)
 
 	_, _, err = querySeries(stores, "user-1", "anything", 0, 100)
-	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyError))
+	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyAccessDenied))
 	_, err = queryLabelsNames(stores, "user-1", "anything")
-	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyError))
+	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyAccessDenied))
 	_, err = queryLabelsValues(stores, "user-1", "anything")
-	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyError))
+	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyAccessDenied))
 	_, _, err = querySeries(stores, "user-2", "anything", 0, 100)
 	require.NoError(t, err)
 	_, err = queryLabelsNames(stores, "user-1", "anything")
-	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyError))
+	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyAccessDenied))
 	_, err = queryLabelsValues(stores, "user-1", "anything")
-	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyError))
+	require.Equal(t, err, httpgrpc.Errorf(int(codes.ResourceExhausted), "store error: %s", bucket.ErrCustomerManagedKeyAccessDenied))
 
 	// Cleaning the error
 	mBucket.GetFailures = map[string]error{}
diff --git a/pkg/util/errors/errors.go b/pkg/util/errors/errors.go
@@ -0,0 +1,47 @@
+package errors
+
+import "errors"
+
+type errWithCause struct {
+	error
+	cause error
+}
+
+func (e errWithCause) Error() string {
+	return e.error.Error()
+}
+
+// Cause To support errors.Cause().
+func (e errWithCause) Cause() error {
+	return e.cause
+}
+
+// Is To support errors.Is().
+func (e errWithCause) Is(err error) bool {
+	return errors.Is(err, e.error) || errors.Is(err, e.cause)
+}
+
+// Unwrap To support errors.Unwrap().
+func (e errWithCause) Unwrap() error {
+	return e.cause
+}
+
+// WithCause wrappers err with a error cause
+func WithCause(err, cause error) error {
+	return errWithCause{
+		error: err,
+		cause: cause,
+	}
+}
+
+// ErrorIs is similar to `errors.Is` but receives a function to compare
+func ErrorIs(err error, f func(err error) bool) bool {
+	for {
+		if f(err) {
+			return true
+		}
+		if err = errors.Unwrap(err); err == nil {
+			return false
+		}
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ func (f *BucketIndexBlocksFinder) GetBlocks(ctx context.Context, userID string,`
`65`	`65`	`return nil, nil, nil`
`66`	`66`	`}`
`67`	`67`
`68`		`- if errors.Is(err, bucket.ErrCustomerManagedKeyError) {`
	`68`	`+ if errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {`
`69`	`69`	`return nil, nil, validation.AccessDeniedError(err.Error())`
`70`	`70`	`}`
`71`	`71`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ var (`
`41`	`41`
`42`	`42`	`ErrUnsupportedStorageBackend = errors.New("unsupported storage backend")`
`43`	`43`
`44`		`- ErrCustomerManagedKeyError = errors.New("access denied: customer key")`
	`44`	`+ ErrCustomerManagedKeyAccessDenied = errors.New("access denied: customer key")`
`45`	`45`	`)`
`46`	`46`
`47`	`47`	`// Config holds configuration for accessing long-term storage.`
Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,7 @@ import (`
`13`	`13`	`"github.com/cortexproject/cortex/pkg/storage/tsdb"`
`14`	`14`
`15`	`15`	`"github.com/cortexproject/cortex/pkg/storage/bucket"`
	`16`	`+ cortex_errors "github.com/cortexproject/cortex/pkg/util/errors"`
`16`	`17`	`"github.com/cortexproject/cortex/pkg/util/runutil"`
`17`	`18`	`)`
`18`	`19`
`@@ -33,7 +34,7 @@ func ReadIndex(ctx context.Context, bkt objstore.Bucket, userID string, cfgProvi`
`33`	`34`	`}`
`34`	`35`
`35`	`36`	`if userBkt.IsCustomerManagedKeyError(err) {`
`36`		`- return nil, bucket.ErrCustomerManagedKeyError`
	`37`	`+ return nil, cortex_errors.WithCause(bucket.ErrCustomerManagedKeyAccessDenied, err)`
`37`	`38`	`}`
`38`	`39`
`39`	`40`	`return nil, errors.Wrap(err, "read bucket index")`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@ package bucketindex`
`2`	`2`
`3`	`3`	`import (`
`4`	`4`	`"context"`
	`5`	`+ "errors"`
`5`	`6`	`"path"`
`6`	`7`	`"strings"`
`7`	`8`	`"testing"`
`@@ -47,7 +48,7 @@ func TestReadIndex_ShouldReturnErrorIfKeyAccessDeniedErr(t *testing.T) {`
`47`	`48`	`},`
`48`	`49`	`}`
`49`	`50`	`idx, err := ReadIndex(context.Background(), bkt, "user-1", nil, log.NewNopLogger())`
`50`		`- require.Equal(t, bucket.ErrCustomerManagedKeyError, err)`
	`51`	`+ require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))`
`51`	`52`	`require.Nil(t, idx)`
`52`	`53`	`}`
`53`	`54`