Skip to content

no encrypting bucket index #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
4 changes: 0 additions & 4 deletions pkg/compactor/blocks_cleaner.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,6 @@ 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.ErrCustomerManagedKeyAccessDenied) {
// Give up cleaning if we get access denied
level.Warn(userLogger).Log("msg", err.Error())
return nil
} else if err != nil && !errors.Is(err, bucketindex.ErrIndexNotFound) {
return err
}
Expand Down
6 changes: 0 additions & 6 deletions pkg/querier/blocks_finder_bucket_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/thanos-io/objstore"

"github.com/cortexproject/cortex/pkg/util/validation"

"github.com/cortexproject/cortex/pkg/storage/bucket"
"github.com/cortexproject/cortex/pkg/storage/tsdb/bucketindex"
"github.com/cortexproject/cortex/pkg/util/services"
Expand Down Expand Up @@ -65,10 +63,6 @@ func (f *BucketIndexBlocksFinder) GetBlocks(ctx context.Context, userID string,
return nil, nil, nil
}

if errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
return nil, nil, validation.AccessDeniedError(err.Error())
}

if err != nil {
return nil, nil, err
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/storage/bucket/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ var (
SupportedBackends = []string{S3, GCS, Azure, Swift, Filesystem}

ErrUnsupportedStorageBackend = errors.New("unsupported storage backend")

ErrCustomerManagedKeyAccessDenied = errors.New("access denied: customer key")
)

// Config holds configuration for accessing long-term storage.
Expand Down
4 changes: 1 addition & 3 deletions pkg/storage/tsdb/bucketindex/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@ 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.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
// started to remote write and its blocks haven't uploaded to storage yet).
Expand Down Expand Up @@ -198,7 +196,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.ErrCustomerManagedKeyAccessDenied) {
if err != nil && !errors.Is(err, ErrIndexNotFound) {
l.loadFailures.Inc()
level.Warn(l.logger).Log("msg", "unable to update bucket index", "user", userID, "err", err)
return
Expand Down
50 changes: 0 additions & 50 deletions pkg/storage/tsdb/bucketindex/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,11 @@ import (

"github.com/go-kit/log"
"github.com/oklog/ulid"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/cortexproject/cortex/pkg/storage/bucket"

cortex_testutil "github.com/cortexproject/cortex/pkg/storage/tsdb/testutil"
"github.com/cortexproject/cortex/pkg/util/services"
"github.com/cortexproject/cortex/pkg/util/test"
Expand Down Expand Up @@ -602,9 +599,6 @@ func TestLoader_ShouldUpdateIndexInBackgroundOnPreviousKeyAcessDenied(t *testing
require.NoError(t, services.StopAndAwaitTerminated(ctx, loader))
})

_, err := loader.GetIndex(ctx, user)
require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))

// Check cached
require.NoError(t, loader.checkCachedIndexes(ctx))

Expand Down Expand Up @@ -646,50 +640,6 @@ func TestLoader_ShouldUpdateIndexInBackgroundOnPreviousKeyAcessDenied(t *testing
))
}

func TestLoader_GetIndex_ShouldCacheKeyDeniedErrors(t *testing.T) {
user := "user-1"
ctx := context.Background()
reg := prometheus.NewPedanticRegistry()
bkt, _ := cortex_testutil.PrepareFilesystemBucket(t)

bkt = &cortex_testutil.MockBucketFailure{
Bucket: bkt,
GetFailures: map[string]error{
path.Join(user, "bucket-index.json.gz"): cortex_testutil.ErrKeyAccessDeniedError,
},
}

// Create the loader.
loader := NewLoader(prepareLoaderConfig(), bkt, nil, log.NewNopLogger(), reg)
require.NoError(t, services.StartAndAwaitRunning(ctx, loader))
t.Cleanup(func() {
require.NoError(t, services.StopAndAwaitTerminated(ctx, loader))
})

// Request the index multiple times.
for i := 0; i < 10; i++ {
_, err := loader.GetIndex(ctx, "user-1")
require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
}

// Ensure metrics have been updated accordingly.
assert.NoError(t, testutil.GatherAndCompare(reg, bytes.NewBufferString(`
# HELP cortex_bucket_index_load_failures_total Total number of bucket index loading failures.
# TYPE cortex_bucket_index_load_failures_total counter
cortex_bucket_index_load_failures_total 0
# HELP cortex_bucket_index_loaded Number of bucket indexes currently loaded in-memory.
# TYPE cortex_bucket_index_loaded gauge
cortex_bucket_index_loaded 0
# HELP cortex_bucket_index_loads_total Total number of bucket index loading attempts.
# TYPE cortex_bucket_index_loads_total counter
cortex_bucket_index_loads_total 1
`),
"cortex_bucket_index_loads_total",
"cortex_bucket_index_load_failures_total",
"cortex_bucket_index_loaded",
))
}

func prepareLoaderConfig() LoaderConfig {
return LoaderConfig{
CheckInterval: time.Minute,
Expand Down
6 changes: 0 additions & 6 deletions pkg/storage/tsdb/bucketindex/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ 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"
)

Expand All @@ -32,11 +31,6 @@ func ReadIndex(ctx context.Context, bkt objstore.Bucket, userID string, cfgProvi
if userBkt.IsObjNotFoundErr(err) {
return nil, ErrIndexNotFound
}

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

return nil, errors.Wrap(err, "read bucket index")
}
defer runutil.CloseWithLogOnErr(logger, reader, "close bucket index reader")
Expand Down
16 changes: 0 additions & 16 deletions pkg/storage/tsdb/bucketindex/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package bucketindex

import (
"context"
"errors"
"path"
"strings"
"testing"
Expand All @@ -11,8 +10,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/cortexproject/cortex/pkg/storage/bucket"

"github.com/cortexproject/cortex/pkg/storage/tsdb/testutil"
cortex_testutil "github.com/cortexproject/cortex/pkg/storage/tsdb/testutil"
)
Expand All @@ -39,19 +36,6 @@ func TestReadIndex_ShouldReturnErrorIfIndexIsCorrupted(t *testing.T) {
require.Nil(t, idx)
}

func TestReadIndex_ShouldReturnErrorIfKeyAccessDeniedErr(t *testing.T) {
bkt, _ := cortex_testutil.PrepareFilesystemBucket(t)
bkt = &cortex_testutil.MockBucketFailure{
Bucket: bkt,
GetFailures: map[string]error{
path.Join("user-1", "bucket-index.json.gz"): cortex_testutil.ErrKeyAccessDeniedError,
},
}
idx, err := ReadIndex(context.Background(), bkt, "user-1", nil, log.NewNopLogger())
require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
require.Nil(t, idx)
}

func TestReadIndex_ShouldReturnTheParsedIndexOnSuccess(t *testing.T) {
const userID = "user-1"

Expand Down
3 changes: 1 addition & 2 deletions pkg/storage/tsdb/bucketindex/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ func (w *Updater) updateBlocks(ctx context.Context, old []*Block) (blocks []*Blo
continue
}
if errors.Is(err, errBlockMetaKeyAccessDeniedErr) {
partials[id] = err
level.Warn(w.logger).Log("msg", "skipped partial block when updating bucket index due key permission", "block", id.String())
level.Warn(w.logger).Log("msg", "skipped block when updating bucket index due key permission", "block", id.String())
continue
}
if errors.Is(err, ErrBlockMetaCorrupted) {
Expand Down
13 changes: 1 addition & 12 deletions pkg/storegateway/bucket_index_metadata_fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (

const (
corruptedBucketIndex = "corrupted-bucket-index"
keyAccessDenied = "key-access-denied"
noBucketIndex = "no-bucket-index"
)

Expand Down Expand Up @@ -68,7 +67,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.ErrCustomerManagedKeyAccessDenied) {
if err != nil {
f.metrics.SyncFailures.Inc()
}
}()
Expand All @@ -95,16 +94,6 @@ func (f *BucketIndexMetadataFetcher) Fetch(ctx context.Context) (metas map[ulid.
return nil, nil, nil
}

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, err
}

if err != nil {
f.metrics.Synced.WithLabelValues(block.FailedMeta).Set(1)
f.metrics.Submit()
Expand Down
48 changes: 0 additions & 48 deletions pkg/storegateway/bucket_index_metadata_fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package storegateway
import (
"bytes"
"context"
"errors"
"path"
"strings"
"testing"
Expand Down Expand Up @@ -100,53 +99,6 @@ func TestBucketIndexMetadataFetcher_Fetch(t *testing.T) {
))
}

func TestBucketIndexMetadataFetcher_Fetch_KeyPermissionDenied(t *testing.T) {
const userID = "user-1"

bkt := &bucket.ClientMock{}
reg := prometheus.NewPedanticRegistry()
ctx := context.Background()

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.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
assert.Empty(t, metas)

assert.NoError(t, testutil.GatherAndCompare(reg, bytes.NewBufferString(`
# HELP blocks_meta_modified Number of blocks whose metadata changed
# TYPE blocks_meta_modified gauge
blocks_meta_modified{modified="replica-label-removed"} 0
# HELP blocks_meta_sync_failures_total Total blocks metadata synchronization failures
# TYPE blocks_meta_sync_failures_total counter
blocks_meta_sync_failures_total 0
# HELP blocks_meta_synced Number of block metadata synced
# TYPE blocks_meta_synced gauge
blocks_meta_synced{state="corrupted-bucket-index"} 0
blocks_meta_synced{state="corrupted-meta-json"} 0
blocks_meta_synced{state="duplicate"} 0
blocks_meta_synced{state="failed"} 0
blocks_meta_synced{state="key-access-denied"} 1
blocks_meta_synced{state="label-excluded"} 0
blocks_meta_synced{state="loaded"} 0
blocks_meta_synced{state="marked-for-deletion"} 0
blocks_meta_synced{state="marked-for-no-compact"} 0
blocks_meta_synced{state="no-bucket-index"} 0
blocks_meta_synced{state="no-meta-json"} 0
blocks_meta_synced{state="time-excluded"} 0
blocks_meta_synced{state="too-fresh"} 0
# HELP blocks_meta_syncs_total Total blocks metadata synchronization attempts
# TYPE blocks_meta_syncs_total counter
blocks_meta_syncs_total 1
`),
"blocks_meta_modified",
"blocks_meta_sync_failures_total",
"blocks_meta_synced",
"blocks_meta_syncs_total",
))
}

func TestBucketIndexMetadataFetcher_Fetch_NoBucketIndex(t *testing.T) {
t.Parallel()
const userID = "user-1"
Expand Down
Loading