Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ require (
github.com/sony/gobreaker v0.5.0
github.com/spf13/afero v1.9.5
github.com/stretchr/testify v1.8.4
github.com/thanos-io/objstore v0.0.0-20230804084840-c042a6a16c58
github.com/thanos-io/objstore v0.0.0-20230816175749-20395bffdf26
github.com/thanos-io/promql-engine v0.0.0-20230816062837-c64fc7b373db
github.com/thanos-io/thanos v0.0.0-20230816172224-2b4f2a7061f9
github.com/uber/jaeger-client-go v2.30.0+incompatible
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1206,8 +1206,8 @@ github.com/subosito/gotenv v1.4.1/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNG
github.com/tencentyun/cos-go-sdk-v5 v0.7.40 h1:W6vDGKCHe4wBACI1d2UgE6+50sJFhRWU4O8IB2ozzxM=
github.com/thanos-community/galaxycache v0.0.0-20211122094458-3a32041a1f1e h1:f1Zsv7OAU9iQhZwigp50Yl38W10g/vd5NC8Rdk1Jzng=
github.com/thanos-community/galaxycache v0.0.0-20211122094458-3a32041a1f1e/go.mod h1:jXcofnrSln/cLI6/dhlBxPQZEEQHVPCcFaH75M+nSzM=
github.com/thanos-io/objstore v0.0.0-20230804084840-c042a6a16c58 h1:4cDXsvm3mb1NvW1B1qJ9/fy6h+OOYit0h8oVA957hLM=
github.com/thanos-io/objstore v0.0.0-20230804084840-c042a6a16c58/go.mod h1:oJ82xgcBDzGJrEgUsjlTj6n01+ZWUMMUR8BlZzX5xDE=
github.com/thanos-io/objstore v0.0.0-20230816175749-20395bffdf26 h1:q1lin/af0lw+I3sS79ccHs2CLjFOPc190J9saeQ5qQ4=
github.com/thanos-io/objstore v0.0.0-20230816175749-20395bffdf26/go.mod h1:oJ82xgcBDzGJrEgUsjlTj6n01+ZWUMMUR8BlZzX5xDE=
github.com/thanos-io/promql-engine v0.0.0-20230816062837-c64fc7b373db h1:05Tp4pfeTTJlRnwLtgvXCJvKYeZCRBoxwDFC+uYqGyM=
github.com/thanos-io/promql-engine v0.0.0-20230816062837-c64fc7b373db/go.mod h1:eIgPaXWgOhNAv6CPPrgu09r0AtT7byBTZy+7WkX0D18=
github.com/thanos-io/thanos v0.0.0-20230816172224-2b4f2a7061f9 h1:KuVECxBG1Q8WoYWlY8dk1wi3OtPSSxv+tWPV9S9qGFk=
Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/bucket/client_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ func (m *ClientMock) IsObjNotFoundErr(err error) bool {
return err == errObjectDoesNotExist
}

// IsCustomerManagedKeyError mocks objstore.Bucket.IsCustomerManagedKeyError()
func (m *ClientMock) IsCustomerManagedKeyError(err error) bool {
// IsAccessDeniedErr mocks objstore.Bucket.IsAccessDeniedErr()
func (m *ClientMock) IsAccessDeniedErr(err error) bool {
return err == errKeyPermissionDenied
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/bucket/prefixed_bucket_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ func (b *PrefixedBucketClient) IsObjNotFoundErr(err error) bool {
return b.bucket.IsObjNotFoundErr(err)
}

// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
func (b *PrefixedBucketClient) IsCustomerManagedKeyError(err error) bool {
return b.bucket.IsCustomerManagedKeyError(err)
// IsAccessDeniedErr returns true if access to object is denied.
func (b *PrefixedBucketClient) IsAccessDeniedErr(err error) bool {
return b.bucket.IsAccessDeniedErr(err)
}

// Attributes returns attributes of the specified object.
Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/bucket/s3/bucket_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (b *BucketWithRetries) retry(ctx context.Context, f func() error, operation
if lastErr == nil {
return nil
}
if b.bucket.IsObjNotFoundErr(lastErr) || b.bucket.IsCustomerManagedKeyError(lastErr) {
if b.bucket.IsObjNotFoundErr(lastErr) || b.bucket.IsAccessDeniedErr(lastErr) {
return lastErr
}
retries.Wait()
Expand Down Expand Up @@ -209,8 +209,8 @@ func (b *BucketWithRetries) IsObjNotFoundErr(err error) bool {
return b.bucket.IsObjNotFoundErr(err)
}

func (b *BucketWithRetries) IsCustomerManagedKeyError(err error) bool {
return b.bucket.IsCustomerManagedKeyError(err)
func (b *BucketWithRetries) IsAccessDeniedErr(err error) bool {
return b.bucket.IsAccessDeniedErr(err)
}

func (b *BucketWithRetries) Close() error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/bucket/s3/bucket_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ func (m *mockBucket) IsObjNotFoundErr(err error) bool {
return err == errNotFound
}

// IsCustomerManagedKeyError mocks objstore.Bucket.IsCustomerManagedKeyError()
func (m *mockBucket) IsCustomerManagedKeyError(err error) bool {
// IsAccessDeniedErr mocks objstore.Bucket.IsAccessDeniedErr()
func (m *mockBucket) IsAccessDeniedErr(err error) bool {
return err == errKeyDenied
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/storage/bucket/sse_bucket_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (b *SSEBucketClient) Iter(ctx context.Context, dir string, f func(string) e
func (b *SSEBucketClient) Get(ctx context.Context, name string) (io.ReadCloser, error) {
r, err := b.bucket.Get(ctx, name)

if err != nil && b.IsCustomerManagedKeyError(err) {
if err != nil && b.IsAccessDeniedErr(err) {
// Store gateway will return the status if the returned error is an `status.Error`
return nil, cortex_errors.WithCause(err, status.Error(codes.PermissionDenied, err.Error()))
}
Expand All @@ -118,7 +118,7 @@ func (b *SSEBucketClient) Get(ctx context.Context, name string) (io.ReadCloser,
// GetRange implements objstore.Bucket.
func (b *SSEBucketClient) GetRange(ctx context.Context, name string, off, length int64) (io.ReadCloser, error) {
r, err := b.bucket.GetRange(ctx, name, off, length)
if err != nil && b.IsCustomerManagedKeyError(err) {
if err != nil && b.IsAccessDeniedErr(err) {
return nil, cortex_errors.WithCause(err, status.Error(codes.PermissionDenied, err.Error()))
}

Expand All @@ -135,13 +135,13 @@ func (b *SSEBucketClient) IsObjNotFoundErr(err error) bool {
return b.bucket.IsObjNotFoundErr(err)
}

// IsCustomerManagedKeyError implements objstore.Bucket.
func (b *SSEBucketClient) IsCustomerManagedKeyError(err error) bool {
// IsAccessDeniedErr implements objstore.Bucket.
func (b *SSEBucketClient) IsAccessDeniedErr(err error) bool {
// unwrap error
if se, ok := err.(interface{ Err() error }); ok {
return b.bucket.IsCustomerManagedKeyError(se.Err()) || b.bucket.IsCustomerManagedKeyError(err)
return b.bucket.IsAccessDeniedErr(se.Err()) || b.bucket.IsAccessDeniedErr(err)
}
return b.bucket.IsCustomerManagedKeyError(err)
return b.bucket.IsAccessDeniedErr(err)
}

// Attributes implements objstore.Bucket.
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/bucket/sse_bucket_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func Test_shouldWrapSSeErrors(t *testing.T) {
sseBkt := NewSSEBucketClient("user-1", bkt, cfgProvider)

_, err := sseBkt.Get(context.Background(), "Test")
require.True(t, sseBkt.IsCustomerManagedKeyError(err))
require.True(t, sseBkt.IsAccessDeniedErr(err))
}

type mockTenantConfigProvider struct {
Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/tsdb/bucketindex/markers_bucket_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,9 @@ func (b *globalMarkersBucket) IsObjNotFoundErr(err error) bool {
return b.parent.IsObjNotFoundErr(err)
}

// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
func (b *globalMarkersBucket) IsCustomerManagedKeyError(err error) bool {
return b.parent.IsCustomerManagedKeyError(err)
// IsAccessDeniedErr returns true if access to object is denied.
func (b *globalMarkersBucket) IsAccessDeniedErr(err error) bool {
return b.parent.IsAccessDeniedErr(err)
}

// Attributes implements objstore.Bucket.
Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/tsdb/bucketindex/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ func ReadIndex(ctx context.Context, bkt objstore.Bucket, userID string, cfgProvi
userBkt := bucket.NewUserBucketClient(userID, bkt, cfgProvider)

// Get the bucket index.
reader, err := userBkt.WithExpectedErrs(tsdb.IsOneOfTheExpectedErrors(userBkt.IsCustomerManagedKeyError, userBkt.IsObjNotFoundErr)).Get(ctx, IndexCompressedFilename)
reader, err := userBkt.WithExpectedErrs(tsdb.IsOneOfTheExpectedErrors(userBkt.IsAccessDeniedErr, userBkt.IsObjNotFoundErr)).Get(ctx, IndexCompressedFilename)
if err != nil {
if userBkt.IsObjNotFoundErr(err) {
return nil, ErrIndexNotFound
}

if userBkt.IsCustomerManagedKeyError(err) {
if userBkt.IsAccessDeniedErr(err) {
return nil, cortex_errors.WithCause(bucket.ErrCustomerManagedKeyAccessDenied, err)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/tsdb/bucketindex/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ func (w *Updater) updateBlockIndexEntry(ctx context.Context, id ulid.ULID) (*Blo
metaFile := path.Join(id.String(), block.MetaFilename)

// Get the block's meta.json file.
r, err := w.bkt.ReaderWithExpectedErrs(tsdb.IsOneOfTheExpectedErrors(w.bkt.IsObjNotFoundErr, w.bkt.IsCustomerManagedKeyError)).Get(ctx, metaFile)
r, err := w.bkt.ReaderWithExpectedErrs(tsdb.IsOneOfTheExpectedErrors(w.bkt.IsObjNotFoundErr, w.bkt.IsAccessDeniedErr)).Get(ctx, metaFile)
if w.bkt.IsObjNotFoundErr(err) {
return nil, ErrBlockMetaNotFound
}
if w.bkt.IsCustomerManagedKeyError(err) {
if w.bkt.IsAccessDeniedErr(err) {
return nil, errBlockMetaKeyAccessDeniedErr
}
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/tsdb/testutil/objstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,6 @@ func (m *MockBucketFailure) ReaderWithExpectedErrs(expectedFunc objstore.IsOpFai
return m
}

func (m *MockBucketFailure) IsCustomerManagedKeyError(err error) bool {
func (m *MockBucketFailure) IsAccessDeniedErr(err error) bool {
return ErrKeyAccessDeniedError == err
}
57 changes: 32 additions & 25 deletions pkg/storegateway/bucket_stores.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ func (u *BucketStores) syncUsersBlocks(ctx context.Context, f func(context.Conte
if err := f(ctx, job.store); err != nil {
if errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
u.storesErrorsMu.Lock()
u.storesErrors[job.userID] = err
u.storesErrors[job.userID] = httpgrpc.Errorf(int(codes.PermissionDenied), "store error: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we wrap this to httpgrpc error?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems it is used in testing only?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This get translated to a 4xx here:

if s.Code() == codes.PermissionDenied {
return validation.AccessDeniedError(s.Message())
}

This was causing 5xx in this case.

u.storesErrorsMu.Unlock()
} else {
errsMx.Lock()
Expand Down Expand Up @@ -298,16 +298,19 @@ func (u *BucketStores) Series(req *storepb.SeriesRequest, srv storepb.Store_Seri
return fmt.Errorf("no userID")
}

store := u.getStore(userID)
err := u.getStoreError(userID)
userBkt := bucket.NewUserBucketClient(userID, u.bucket, u.limits)
if store == nil {
return nil
}
if err != nil {
if cortex_errors.ErrorIs(err, userBkt.IsAccessDeniedErr) {
return httpgrpc.Errorf(int(codes.PermissionDenied), "store error: %s", err)
}

err := u.getStoreError(userID)
return err
}

if err != nil && cortex_errors.ErrorIs(err, userBkt.IsCustomerManagedKeyError) {
return httpgrpc.Errorf(int(codes.PermissionDenied), "store error: %s", err)
store := u.getStore(userID)
if store == nil {
return nil
}

err = store.Series(req, spanSeriesServer{
Expand All @@ -328,16 +331,19 @@ func (u *BucketStores) LabelNames(ctx context.Context, req *storepb.LabelNamesRe
return nil, fmt.Errorf("no userID")
}

store := u.getStore(userID)
err := u.getStoreError(userID)
userBkt := bucket.NewUserBucketClient(userID, u.bucket, u.limits)
if store == nil {
return &storepb.LabelNamesResponse{}, nil
}
if err != nil {
if cortex_errors.ErrorIs(err, userBkt.IsAccessDeniedErr) {
return nil, httpgrpc.Errorf(int(codes.PermissionDenied), "store error: %s", err)
}

err := u.getStoreError(userID)
return nil, err
}

if err != nil && cortex_errors.ErrorIs(err, userBkt.IsCustomerManagedKeyError) {
return nil, httpgrpc.Errorf(int(codes.PermissionDenied), "store error: %s", err)
store := u.getStore(userID)
if store == nil {
return &storepb.LabelNamesResponse{}, nil
}

resp, err := store.LabelNames(ctx, req)
Expand All @@ -355,21 +361,22 @@ func (u *BucketStores) LabelValues(ctx context.Context, req *storepb.LabelValues
return nil, fmt.Errorf("no userID")
}

store := u.getStore(userID)
userBkt := bucket.NewUserBucketClient(userID, u.bucket, u.limits)
if store == nil {
return &storepb.LabelValuesResponse{}, nil
}

err := u.getStoreError(userID)
userBkt := bucket.NewUserBucketClient(userID, u.bucket, u.limits)
if err != nil {
if cortex_errors.ErrorIs(err, userBkt.IsAccessDeniedErr) {
return nil, httpgrpc.Errorf(int(codes.PermissionDenied), "store error: %s", err)
}

if err != nil && cortex_errors.ErrorIs(err, userBkt.IsCustomerManagedKeyError) {
return nil, httpgrpc.Errorf(int(codes.PermissionDenied), "store error: %s", err)
return nil, err
}

resp, err := store.LabelValues(ctx, req)
store := u.getStore(userID)
if store == nil {
return &storepb.LabelValuesResponse{}, nil
}

return resp, err
return store.LabelValues(ctx, req)
}

// scanUsers in the bucket and return the list of found users. If an error occurs while
Expand Down
8 changes: 6 additions & 2 deletions pkg/storegateway/bucket_stores_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,16 @@ func TestBucketStores_CustomerKeyError(t *testing.T) {
// Should set the error on user-1
require.NoError(t, stores.InitialSync(ctx))
if tc.mockInitialSync {
require.ErrorIs(t, stores.storesErrors["user-1"], bucket.ErrCustomerManagedKeyAccessDenied)
s, ok := status.FromError(stores.storesErrors["user-1"])
require.True(t, ok)
require.Equal(t, s.Code(), codes.PermissionDenied)
require.ErrorIs(t, stores.storesErrors["user-2"], nil)
}
require.NoError(t, stores.SyncBlocks(context.Background()))
if tc.mockInitialSync {
require.ErrorIs(t, stores.storesErrors["user-1"], bucket.ErrCustomerManagedKeyAccessDenied)
s, ok := status.FromError(stores.storesErrors["user-1"])
require.True(t, ok)
require.Equal(t, s.Code(), codes.PermissionDenied)
require.ErrorIs(t, stores.storesErrors["user-2"], nil)
}

Expand Down
2 changes: 1 addition & 1 deletion vendor/github.com/thanos-io/objstore/CHANGELOG.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion vendor/github.com/thanos-io/objstore/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vendor/github.com/thanos-io/objstore/inmem.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions vendor/github.com/thanos-io/objstore/objstore.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions vendor/github.com/thanos-io/objstore/prefixed_bucket.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 6 additions & 3 deletions vendor/github.com/thanos-io/objstore/providers/azure/azure.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions vendor/github.com/thanos-io/objstore/providers/gcs/gcs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading