Skip to content

Commit

Permalink
Do not encrypt deletion marker with CMK key (cortexproject#5575)
Browse files Browse the repository at this point in the history
Signed-off-by: Alan Protasio <[email protected]>
  • Loading branch information
alanprot authored Sep 22, 2023
1 parent 179c0d6 commit 189e6c5
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* [CHANGE] Bucket Index: Add `series_max_size` and `chunk_max_size` to bucket index. #5489
* [CHANGE] StoreGateway: Rename `cortex_bucket_store_chunk_pool_returned_bytes_total` and `cortex_bucket_store_chunk_pool_requested_bytes_total` to `cortex_bucket_store_chunk_pool_operation_bytes_total`. #5552
* [CHANGE] Query Frontend/Querier: Make build info API disabled by default and add feature flag `api.build-info-enabled` to enable it. #5533
* [CHANGE] Purger: Do no use S3 tenant kms key when uploading deletion marker. #5575
* [FEATURE] Store Gateway: Add `max_downloaded_bytes_per_request` to limit max bytes to download per store gateway request.
* [FEATURE] Added 2 flags `-alertmanager.alertmanager-client.grpc-max-send-msg-size` and ` -alertmanager.alertmanager-client.grpc-max-recv-msg-size` to configure alert manager grpc client message size limits. #5338
* [FEATURE] Query Frontend: Add `cortex_rejected_queries_total` metric for throttled queries. #5356
Expand Down
2 changes: 1 addition & 1 deletion pkg/compactor/blocks_cleaner.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ func (c *BlocksCleaner) deleteUserMarkedForDeletion(ctx context.Context, userID
if deletedBlocks > 0 || mark.FinishedTime == 0 {
level.Info(userLogger).Log("msg", "updating finished time in tenant deletion mark")
mark.FinishedTime = time.Now().Unix()
return errors.Wrap(cortex_tsdb.WriteTenantDeletionMark(ctx, c.bucketClient, userID, c.cfgProvider, mark), "failed to update tenant deletion mark")
return errors.Wrap(cortex_tsdb.WriteTenantDeletionMark(ctx, c.bucketClient, userID, mark), "failed to update tenant deletion mark")
}

if time.Since(time.Unix(mark.FinishedTime, 0)) < c.cfg.TenantCleanupDelay {
Expand Down
4 changes: 2 additions & 2 deletions pkg/compactor/blocks_cleaner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ func testBlocksCleanerWithOptions(t *testing.T, options testBlocksCleanerOptions
createDeletionMark(t, bucketClient, "user-2", block7, now.Add(-deletionDelay).Add(-time.Hour)) // Block reached the deletion threshold.

// Blocks for user-3, marked for deletion.
require.NoError(t, tsdb.WriteTenantDeletionMark(context.Background(), bucketClient, "user-3", nil, tsdb.NewTenantDeletionMark(time.Now())))
require.NoError(t, tsdb.WriteTenantDeletionMark(context.Background(), bucketClient, "user-3", tsdb.NewTenantDeletionMark(time.Now())))
block9 := createTSDBBlock(t, bucketClient, "user-3", 10, 30, nil)
block10 := createTSDBBlock(t, bucketClient, "user-3", 30, 50, nil)

// User-4 with no more blocks, but couple of mark and debug files. Should be fully deleted.
user4Mark := tsdb.NewTenantDeletionMark(time.Now())
user4Mark.FinishedTime = time.Now().Unix() - 60 // Set to check final user cleanup.
require.NoError(t, tsdb.WriteTenantDeletionMark(context.Background(), bucketClient, "user-4", nil, user4Mark))
require.NoError(t, tsdb.WriteTenantDeletionMark(context.Background(), bucketClient, "user-4", user4Mark))
user4DebugMetaFile := path.Join("user-4", block.DebugMetas, "meta.json")
require.NoError(t, bucketClient.Upload(context.Background(), user4DebugMetaFile, strings.NewReader("some random content here")))

Expand Down
4 changes: 2 additions & 2 deletions pkg/ingester/ingester_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2731,7 +2731,7 @@ func TestIngester_dontShipBlocksWhenTenantDeletionMarkerIsPresent(t *testing.T)
numObjects := len(bucket.Objects())
require.NotZero(t, numObjects)

require.NoError(t, cortex_tsdb.WriteTenantDeletionMark(context.Background(), bucket, userID, nil, cortex_tsdb.NewTenantDeletionMark(time.Now())))
require.NoError(t, cortex_tsdb.WriteTenantDeletionMark(context.Background(), bucket, userID, cortex_tsdb.NewTenantDeletionMark(time.Now())))
numObjects++ // For deletion marker

db := i.getTSDB(userID)
Expand Down Expand Up @@ -2763,7 +2763,7 @@ func TestIngester_seriesCountIsCorrectAfterClosingTSDBForDeletedTenant(t *testin
bucket := objstore.NewInMemBucket()

// Write tenant deletion mark.
require.NoError(t, cortex_tsdb.WriteTenantDeletionMark(context.Background(), bucket, userID, nil, cortex_tsdb.NewTenantDeletionMark(time.Now())))
require.NoError(t, cortex_tsdb.WriteTenantDeletionMark(context.Background(), bucket, userID, cortex_tsdb.NewTenantDeletionMark(time.Now())))

i.TSDBState.bucket = bucket
require.NoError(t, services.StartAndAwaitRunning(context.Background(), i))
Expand Down
2 changes: 1 addition & 1 deletion pkg/purger/tenant_deletion_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (api *TenantDeletionAPI) DeleteTenant(w http.ResponseWriter, r *http.Reques
return
}

err = cortex_tsdb.WriteTenantDeletionMark(r.Context(), api.bucketClient, userID, api.cfgProvider, cortex_tsdb.NewTenantDeletionMark(time.Now()))
err = cortex_tsdb.WriteTenantDeletionMark(r.Context(), api.bucketClient, userID, cortex_tsdb.NewTenantDeletionMark(time.Now()))
if err != nil {
level.Error(api.logger).Log("msg", "failed to write tenant deletion mark", "user", userID, "err", err)

Expand Down
7 changes: 3 additions & 4 deletions pkg/storage/tsdb/tenant_deletion_mark.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/pkg/errors"
"github.com/thanos-io/objstore"

"github.com/cortexproject/cortex/pkg/storage/bucket"
util_log "github.com/cortexproject/cortex/pkg/util/log"
)

Expand All @@ -38,15 +37,15 @@ func TenantDeletionMarkExists(ctx context.Context, bkt objstore.BucketReader, us
}

// Uploads deletion mark to the tenant location in the bucket.
func WriteTenantDeletionMark(ctx context.Context, bkt objstore.Bucket, userID string, cfgProvider bucket.TenantConfigProvider, mark *TenantDeletionMark) error {
bkt = bucket.NewUserBucketClient(userID, bkt, cfgProvider)
func WriteTenantDeletionMark(ctx context.Context, bkt objstore.Bucket, userID string, mark *TenantDeletionMark) error {
markerFile := path.Join(userID, TenantDeletionMarkPath)

data, err := json.Marshal(mark)
if err != nil {
return errors.Wrap(err, "serialize tenant deletion mark")
}

return errors.Wrap(bkt.Upload(ctx, TenantDeletionMarkPath, bytes.NewReader(data)), "upload tenant deletion mark")
return errors.Wrap(bkt.Upload(ctx, markerFile, bytes.NewReader(data)), "upload tenant deletion mark")
}

// Returns tenant deletion mark for given user, if it exists. If it doesn't exist, returns nil mark, and no error.
Expand Down
16 changes: 14 additions & 2 deletions pkg/storage/tsdb/tenant_deletion_mark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ func TestTenantDeletionMarkExists(t *testing.T) {
const username = "user"

for name, tc := range map[string]struct {
objects map[string][]byte
exists bool
objects map[string][]byte
exists bool
deletedUsers []string
}{
"empty": {
objects: nil,
Expand All @@ -35,6 +36,13 @@ func TestTenantDeletionMarkExists(t *testing.T) {
},
exists: true,
},
"mark exists - upload via WriteTenantDeletionMark": {
objects: map[string][]byte{
"user/01EQK4QKFHVSZYVJ908Y7HH9E0/meta.json": []byte("data"),
},
deletedUsers: []string{"user"},
exists: true,
},
} {
t.Run(name, func(t *testing.T) {
bkt := objstore.NewInMemBucket()
Expand All @@ -43,6 +51,10 @@ func TestTenantDeletionMarkExists(t *testing.T) {
require.NoError(t, bkt.Upload(context.Background(), objName, bytes.NewReader(data)))
}

for _, user := range tc.deletedUsers {
require.NoError(t, WriteTenantDeletionMark(context.Background(), bkt, user, &TenantDeletionMark{}))
}

res, err := TenantDeletionMarkExists(context.Background(), bkt, username)
require.NoError(t, err)
require.Equal(t, tc.exists, res)
Expand Down

0 comments on commit 189e6c5

Please sign in to comment.