Added S3 SSE support to alertmanager storage (cortexproject#3906)

Signed-off-by: Marco Pracucci <marco@pracucci.com>

CHANGELOG.md

@@ -12,7 +12,7 @@
 * [CHANGE] Alertmanager: the `DELETE /api/v1/alerts` is now idempotent. No error is returned if the alertmanager config doesn't exist. #3888
 * [FEATURE] Experimental Ruler Storage: Add a separate set of configuration options to configure the ruler storage backend under the `-ruler-storage.` flag prefix. All blocks storage bucket clients and the config service are currently supported. Clients using this implementation will only be enabled if the existing `-ruler.storage` flags are left unset. #3805 #3864
 * [FEATURE] Experimental Alertmanager Storage: Add a separate set of configuration options to configure the alertmanager storage backend under the `-alertmanager-storage.` flag prefix. All blocks storage bucket clients and the config service are currently supported. Clients using this implementation will only be enabled if the existing `-alertmanager.storage` flags are left unset. #3888
-* [FEATURE] Adds support to S3 server-side encryption using KMS. The S3 server-side encryption config can be overridden on a per-tenant basis for the blocks storage and ruler. Deprecated `-<prefix>.s3.sse-encryption`, you should use the following CLI flags that have been added. #3651 #3810 #3811 #3870 #3886
+* [FEATURE] Adds support to S3 server-side encryption using KMS. The S3 server-side encryption config can be overridden on a per-tenant basis for the blocks storage, ruler and alertmanager. Deprecated `-<prefix>.s3.sse-encryption`, you should use the following CLI flags that have been added. #3651 #3810 #3811 #3870 #3886 #3906
   - `-<prefix>.s3.sse.type`
   - `-<prefix>.s3.sse.kms-key-id`
   - `-<prefix>.s3.sse.kms-encryption-context`

docs/guides/encryption-at-rest.md

@@ -46,9 +46,13 @@ The [chunks storage](../chunks-storage/_index.md) S3 server-side encryption can
 
 The ruler S3 server-side encryption can be configured similarly to the blocks storage. The per-tenant overrides are supported when using the storage backend configurable the `-ruler-storage.` flag prefix (or their respective YAML config options).
 
+### Alertmanager
+
+The alertmanager S3 server-side encryption can be configured similarly to the blocks storage. The per-tenant overrides are supported when using the storage backend configurable the `-alertmanager-storage.` flag prefix (or their respective YAML config options).
+
 ### Per-tenant config overrides
 
-The S3 client used by the blocks storage and ruler supports S3 SSE config overrides on a per-tenant basis, using the [runtime configuration file](../configuration/arguments.md#runtime-configuration-file).
+The S3 client used by the blocks storage, ruler and alertmanager supports S3 SSE config overrides on a per-tenant basis, using the [runtime configuration file](../configuration/arguments.md#runtime-configuration-file).
 The following settings can ben overridden for each tenant:
 
 - **`s3_sse_type`**<br />

docs/guides/encryption-at-rest.template

@@ -28,9 +28,13 @@ The [chunks storage](../chunks-storage/_index.md) S3 server-side encryption can
 
 The ruler S3 server-side encryption can be configured similarly to the blocks storage. The per-tenant overrides are supported when using the storage backend configurable the `-ruler-storage.` flag prefix (or their respective YAML config options).
 
+### Alertmanager
+
+The alertmanager S3 server-side encryption can be configured similarly to the blocks storage. The per-tenant overrides are supported when using the storage backend configurable the `-alertmanager-storage.` flag prefix (or their respective YAML config options).
+
 ### Per-tenant config overrides
 
-The S3 client used by the blocks storage and ruler supports S3 SSE config overrides on a per-tenant basis, using the [runtime configuration file](../configuration/arguments.md#runtime-configuration-file).
+The S3 client used by the blocks storage, ruler and alertmanager supports S3 SSE config overrides on a per-tenant basis, using the [runtime configuration file](../configuration/arguments.md#runtime-configuration-file).
 The following settings can ben overridden for each tenant:
 
 - **`s3_sse_type`**<br />

pkg/alertmanager/alertstore/bucketclient/bucket_client.go

@@ -96,20 +96,22 @@ func (s *BucketAlertStore) SetAlertConfig(ctx context.Context, cfg alertspb.Aler
 		return err
 	}
 
-	return s.bucket.Upload(ctx, cfg.User, bytes.NewBuffer(cfgBytes))
+	return s.getUserBucket(cfg.User).Upload(ctx, cfg.User, bytes.NewBuffer(cfgBytes))
 }
 
 // DeleteAlertConfig implements alertstore.AlertStore.
 func (s *BucketAlertStore) DeleteAlertConfig(ctx context.Context, userID string) error {
-	err := s.bucket.Delete(ctx, userID)
-	if s.bucket.IsObjNotFoundErr(err) {
+	userBkt := s.getUserBucket(userID)
+
+	err := userBkt.Delete(ctx, userID)
+	if userBkt.IsObjNotFoundErr(err) {
 		return nil
 	}
 	return err
 }
 
-func (s *BucketAlertStore) getAlertConfig(ctx context.Context, key string) (alertspb.AlertConfigDesc, error) {
-	readCloser, err := s.bucket.Get(ctx, key)
+func (s *BucketAlertStore) getAlertConfig(ctx context.Context, userID string) (alertspb.AlertConfigDesc, error) {
+	readCloser, err := s.getUserBucket(userID).Get(ctx, userID)
 	if err != nil {
 		return alertspb.AlertConfigDesc{}, err
 	}
@@ -129,3 +131,8 @@ func (s *BucketAlertStore) getAlertConfig(ctx context.Context, key string) (aler
 
 	return config, nil
 }
+
+func (s *BucketAlertStore) getUserBucket(userID string) objstore.Bucket {
+	// Inject server-side encryption based on the tenant config.
+	return bucket.NewSSEBucketClient(userID, s.bucket, s.cfgProvider)
+}

pkg/alertmanager/alertstore/store_test.go

@@ -2,6 +2,7 @@ package alertstore
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	"github.com/go-kit/kit/log"
@@ -16,7 +17,7 @@ import (
 )
 
 func TestAlertStore_ListAllUsers(t *testing.T) {
-	runForEachAlertStore(t, func(t *testing.T, store AlertStore) {
+	runForEachAlertStore(t, func(t *testing.T, store AlertStore, client interface{}) {
 		ctx := context.Background()
 		user1Cfg := alertspb.AlertConfigDesc{User: "user-1", RawConfig: "content-1"}
 		user2Cfg := alertspb.AlertConfigDesc{User: "user-2", RawConfig: "content-2"}
@@ -41,7 +42,7 @@ func TestAlertStore_ListAllUsers(t *testing.T) {
 }
 
 func TestAlertStore_SetAndGetAlertConfig(t *testing.T) {
-	runForEachAlertStore(t, func(t *testing.T, store AlertStore) {
+	runForEachAlertStore(t, func(t *testing.T, store AlertStore, client interface{}) {
 		ctx := context.Background()
 		user1Cfg := alertspb.AlertConfigDesc{User: "user-1", RawConfig: "content-1"}
 		user2Cfg := alertspb.AlertConfigDesc{User: "user-2", RawConfig: "content-2"}
@@ -64,12 +65,22 @@ func TestAlertStore_SetAndGetAlertConfig(t *testing.T) {
 			config, err = store.GetAlertConfig(ctx, "user-2")
 			require.NoError(t, err)
 			assert.Equal(t, user2Cfg, config)
+
+			// Ensure the config is stored at the expected location. Without this check
+			// we have no guarantee that the objects are stored at the expected location.
+			exists, err := objectExists(client, "alerts/user-1")
+			require.NoError(t, err)
+			assert.True(t, exists)
+
+			exists, err = objectExists(client, "alerts/user-2")
+			require.NoError(t, err)
+			assert.True(t, exists)
 		}
 	})
 }
 
 func TestStore_GetAlertConfigs(t *testing.T) {
-	runForEachAlertStore(t, func(t *testing.T, store AlertStore) {
+	runForEachAlertStore(t, func(t *testing.T, store AlertStore, client interface{}) {
 		ctx := context.Background()
 		user1Cfg := alertspb.AlertConfigDesc{User: "user-1", RawConfig: "content-1"}
 		user2Cfg := alertspb.AlertConfigDesc{User: "user-2", RawConfig: "content-2"}
@@ -105,7 +116,7 @@ func TestStore_GetAlertConfigs(t *testing.T) {
 }
 
 func TestAlertStore_DeleteAlertConfig(t *testing.T) {
-	runForEachAlertStore(t, func(t *testing.T, store AlertStore) {
+	runForEachAlertStore(t, func(t *testing.T, store AlertStore, client interface{}) {
 		ctx := context.Background()
 		user1Cfg := alertspb.AlertConfigDesc{User: "user-1", RawConfig: "content-1"}
 		user2Cfg := alertspb.AlertConfigDesc{User: "user-2", RawConfig: "content-2"}
@@ -139,21 +150,43 @@ func TestAlertStore_DeleteAlertConfig(t *testing.T) {
 	})
 }
 
-func runForEachAlertStore(t *testing.T, testFn func(t *testing.T, store AlertStore)) {
+func runForEachAlertStore(t *testing.T, testFn func(t *testing.T, store AlertStore, client interface{})) {
 	legacyClient := chunk.NewMockStorage()
 	legacyStore := objectclient.NewAlertStore(legacyClient, log.NewNopLogger())
 
 	bucketClient := objstore.NewInMemBucket()
 	bucketStore := bucketclient.NewBucketAlertStore(bucketClient, nil, log.NewNopLogger())
 
-	stores := map[string]AlertStore{
-		"legacy": legacyStore,
-		"bucket": bucketStore,
+	stores := map[string]struct {
+		store  AlertStore
+		client interface{}
+	}{
+		"legacy": {store: legacyStore, client: legacyClient},
+		"bucket": {store: bucketStore, client: bucketClient},
 	}
 
-	for name, store := range stores {
+	for name, data := range stores {
 		t.Run(name, func(t *testing.T) {
-			testFn(t, store)
+			testFn(t, data.store, data.client)
 		})
 	}
 }
+
+func objectExists(bucketClient interface{}, key string) (bool, error) {
+	if typed, ok := bucketClient.(*chunk.MockStorage); ok {
+		_, err := typed.GetObject(context.Background(), key)
+		if errors.Is(err, chunk.ErrStorageObjectNotFound) {
+			return false, nil
+		}
+		if err == nil {
+			return true, nil
+		}
+		return false, err
+	}
+
+	if typed, ok := bucketClient.(*objstore.InMemBucket); ok {
+		return typed.Exists(context.Background(), key)
+	}
+
+	panic("unexpected bucket client")
+}