Add support for AWS STS (#978)

* Add support for AWS STS Signed-off-by: Pavol Loffay <[email protected]> * Add changelog Signed-off-by: Pavol Loffay <[email protected]> * Fix Signed-off-by: Pavol Loffay <[email protected]> * Fix Signed-off-by: Pavol Loffay <[email protected]> * Fix Signed-off-by: Pavol Loffay <[email protected]> * Fix Signed-off-by: Pavol Loffay <[email protected]> * Fix params Signed-off-by: Pavol Loffay <[email protected]> * Add more comments Signed-off-by: Pavol Loffay <[email protected]> * validation for short and long Signed-off-by: Pavol Loffay <[email protected]> * Fix SA for oauth-proxy Signed-off-by: Pavol Loffay <[email protected]> --------- Signed-off-by: Pavol Loffay <[email protected]>
grafana · Jul 12, 2024 · 5d1772f · 5d1772f
1 parent 89d1bb7
commit 5d1772f
Show file tree

Hide file tree

Showing 30 changed files with 611 additions and 107 deletions.
diff --git a/.chloggen/aws-sts-monolithic.yaml b/.chloggen/aws-sts-monolithic.yaml
@@ -0,0 +1,23 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. tempostack, tempomonolithic, github action)
+component: tempostack
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Add support for AWS S3 STS authentication.
+
+# One or more tracking issues related to the change
+issues: [978]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  Now storage secret for S3 can contain
+  ```
+  data:
+    bucket:      # Bucket name
+    region:      # A valid AWS region, e.g. us-east-1
+    role_arn:    # The AWS IAM Role associated with a trust relationship to Tempo serviceaccount
+  ```
diff --git a/.chloggen/aws-sts-tempostack.yaml b/.chloggen/aws-sts-tempostack.yaml
@@ -0,0 +1,23 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. tempostack, tempomonolithic, github action)
+component: tempostack
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Add support for AWS S3 STS authentication.
+
+# One or more tracking issues related to the change
+issues: [978]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: | 
+  Now storage secret for S3 can contain
+  ```
+  data:
+    bucket:      # Bucket name
+    region:      # A valid AWS region, e.g. us-east-1
+    role_arn:    # The AWS IAM Role associated with a trust relationship to Tempo serviceaccount
+  ```
diff --git a/cmd/generate/main.go b/cmd/generate/main.go
@@ -189,8 +189,10 @@ func NewGenerateCommand() *cobra.Command {
 				}
 			case s3Endpoint != "":
 				params.StorageParams.S3 = &manifestutils.S3{
-					Endpoint: s3Endpoint,
-					Bucket:   s3Bucket,
+					LongLived: &manifestutils.S3LongLived{
+						Endpoint: s3Endpoint,
+						Bucket:   s3Bucket,
+					},
 				}
 			}
 

diff --git a/internal/handlers/storage/secret.go b/internal/handlers/storage/secret.go
@@ -45,15 +45,49 @@ func ensureNotEmpty(storageSecret corev1.Secret, fields []string, path *field.Pa
 }
 
 func validateS3Secret(storageSecret corev1.Secret, path *field.Path) field.ErrorList {
-	var allErrs field.ErrorList
-	secretFields := []string{
-		"endpoint",
+	shortLivedFields := []string{
+		"bucket",
+		"region",
+		"role_arn",
+	}
+	longLivedFields := []string{
 		"bucket",
+		"endpoint",
 		"access_key_id",
 		"access_key_secret",
 	}
 
-	allErrs = append(allErrs, ensureNotEmpty(storageSecret, secretFields, path)...)
+	// ship bucket as it is common for both
+	var isShortLived bool
+	for _, v := range shortLivedFields[1:] {
+		_, ok := storageSecret.Data[v]
+		if ok {
+			isShortLived = true
+		}
+	}
+	var isLongLived bool
+	for _, v := range longLivedFields[1:] {
+		_, ok := storageSecret.Data[v]
+		if ok {
+			isLongLived = true
+		}
+	}
+
+	if isShortLived && isLongLived {
+		return field.ErrorList{field.Invalid(
+			path,
+			storageSecret.Name,
+			"storage secret contains fields for long lived and short lived configuration",
+		)}
+	}
+
+	// check short-lived first
+	if storageSecret.Data["role_arn"] != nil || storageSecret.Data["region"] != nil {
+		return ensureNotEmpty(storageSecret, shortLivedFields, path)
+	}
+
+	var allErrs field.ErrorList
+	allErrs = append(allErrs, ensureNotEmpty(storageSecret, longLivedFields, path)...)
 
 	if endpoint, ok := storageSecret.Data["endpoint"]; ok {
 		u, err := url.ParseRequestURI(string(endpoint))
@@ -78,15 +112,27 @@ func getS3Params(storageSecret corev1.Secret, path *field.Path) (*manifestutils.
 		return nil, errs
 	}
 
+	if storageSecret.Data["role_arn"] != nil || storageSecret.Data["region"] != nil {
+		return &manifestutils.S3{
+			ShortLived: &manifestutils.S3ShortLived{
+				Bucket:  string(storageSecret.Data["bucket"]),
+				RoleARN: string(storageSecret.Data["role_arn"]),
+				Region:  string(storageSecret.Data["region"]),
+			},
+		}, nil
+	}
+
 	endpoint := string(storageSecret.Data["endpoint"])
 	insecure := !strings.HasPrefix(endpoint, "https://")
 	endpoint = strings.TrimPrefix(endpoint, "https://")
 	endpoint = strings.TrimPrefix(endpoint, "http://")
 
 	return &manifestutils.S3{
-		Endpoint: endpoint,
-		Bucket:   string(storageSecret.Data["bucket"]),
-		Insecure: insecure,
+		LongLived: &manifestutils.S3LongLived{
+			Endpoint: endpoint,
+			Bucket:   string(storageSecret.Data["bucket"]),
+			Insecure: insecure,
+		},
 	}, nil
 }
 

diff --git a/internal/handlers/storage/secret_test.go b/internal/handlers/storage/secret_test.go
@@ -5,6 +5,8 @@ import (
 
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+
+	"github.com/grafana/tempo-operator/internal/manifests/manifestutils"
 )
 
 func TestGetS3ParamsInsecure(t *testing.T) {
@@ -19,9 +21,9 @@ func TestGetS3ParamsInsecure(t *testing.T) {
 
 	s3, errs := getS3Params(storageSecret, nil)
 	require.Len(t, errs, 0)
-	require.Equal(t, "minio:9000", s3.Endpoint)
-	require.True(t, s3.Insecure)
-	require.Equal(t, "testbucket", s3.Bucket)
+	require.Equal(t, "minio:9000", s3.LongLived.Endpoint)
+	require.True(t, s3.LongLived.Insecure)
+	require.Equal(t, "testbucket", s3.LongLived.Bucket)
 }
 
 func TestGetS3ParamsSecure(t *testing.T) {
@@ -36,7 +38,25 @@ func TestGetS3ParamsSecure(t *testing.T) {
 
 	s3, errs := getS3Params(storageSecret, nil)
 	require.Len(t, errs, 0)
-	require.Equal(t, "minio:9000", s3.Endpoint)
-	require.False(t, s3.Insecure)
-	require.Equal(t, "testbucket", s3.Bucket)
+	require.Equal(t, "minio:9000", s3.LongLived.Endpoint)
+	require.False(t, s3.LongLived.Insecure)
+	require.Equal(t, "testbucket", s3.LongLived.Bucket)
+}
+
+func TestGetS3Params_short_lived(t *testing.T) {
+	storageSecret := corev1.Secret{
+		Data: map[string][]byte{
+			"bucket":   []byte("testbucket"),
+			"role_arn": []byte("abc"),
+			"region":   []byte("rrrr"),
+		},
+	}
+
+	s3, errs := getS3Params(storageSecret, nil)
+	require.Len(t, errs, 0)
+	require.Equal(t, &manifestutils.S3ShortLived{
+		Bucket:  "testbucket",
+		RoleARN: "abc",
+		Region:  "rrrr",
+	}, s3.ShortLived)
 }
diff --git a/internal/handlers/storage/storage.go b/internal/handlers/storage/storage.go
@@ -32,7 +32,7 @@ func GetStorageParamsForTempoStack(ctx context.Context, client client.Client, te
 		}
 
 		if tempo.Spec.Storage.TLS.Enabled {
-			storageParams.S3.TLS, errs = getTLSParams(ctx, client, tempo.Namespace, tempo.Spec.Storage.TLS, tlsPath.Child("caName"))
+			storageParams.S3.LongLived.TLS, errs = getTLSParams(ctx, client, tempo.Namespace, tempo.Spec.Storage.TLS, tlsPath.Child("caName"))
 			if len(errs) > 0 {
 				return manifestutils.StorageParams{}, errs
 			}
@@ -115,7 +115,7 @@ func GetStorageParamsForTempoMonolithic(ctx context.Context, client client.Clien
 
 		if tempo.Spec.Storage.Traces.S3.TLS != nil && tempo.Spec.Storage.Traces.S3.TLS.Enabled {
 			caPath := tracesPath.Child("s3", "tls", "caName")
-			storageParams.S3.TLS, errs = getTLSParams(ctx, client, tempo.Namespace, *tempo.Spec.Storage.Traces.S3.TLS, caPath)
+			storageParams.S3.LongLived.TLS, errs = getTLSParams(ctx, client, tempo.Namespace, *tempo.Spec.Storage.Traces.S3.TLS, caPath)
 			if len(errs) > 0 {
 				return manifestutils.StorageParams{}, errs
 			}

diff --git a/internal/manifests/compactor/compactor.go b/internal/manifests/compactor/compactor.go
@@ -142,7 +142,7 @@ func deployment(params manifestutils.Params) (*v1.Deployment, error) {
 		},
 	}
 
-	err := manifestutils.ConfigureStorage(tempo, &d.Spec.Template.Spec, "tempo")
+	err := manifestutils.ConfigureStorage(params.StorageParams, tempo, &d.Spec.Template.Spec, "tempo")
 	if err != nil {
 		return nil, err
 	}

diff --git a/internal/manifests/config/build.go b/internal/manifests/config/build.go
@@ -128,7 +128,7 @@ func buildS3StorageTLSConfig(params manifestutils.Params) storageTLSOptions {
 		MinTLSVersion: params.Tempo.Spec.Storage.TLS.MinVersion,
 	}
 	if tempo.Spec.Storage.TLS.CA != "" {
-		opts.CA = path.Join(manifestutils.StorageTLSCADir, params.StorageParams.S3.TLS.CAFilename)
+		opts.CA = path.Join(manifestutils.StorageTLSCADir, params.StorageParams.S3.LongLived.TLS.CAFilename)
 	}
 	if tempo.Spec.Storage.TLS.Cert != "" {
 		opts.Certificate = path.Join(manifestutils.StorageTLSCertDir, manifestutils.TLSCertFilename)