Allow enable TLS when STS is enabled (#1066)

* Allow enable TLS when STS is enabled Signed-off-by: Ruben Vargas <[email protected]> * bump lint version Signed-off-by: Ruben Vargas <[email protected]> * bump lint version Signed-off-by: Ruben Vargas <[email protected]> * fix test Signed-off-by: Ruben Vargas <[email protected]> * fix changelog entry Signed-off-by: Ruben Vargas <[email protected]> --------- Signed-off-by: Ruben Vargas <[email protected]> Co-authored-by: Andreas Gerstmayr <[email protected]>
grafana · Oct 23, 2024 · bd9087c · bd9087c
1 parent 712891e
commit bd9087c
Show file tree

Hide file tree

Showing 8 changed files with 34 additions and 17 deletions.
diff --git a/.chloggen/fix_sts_when_tls_is_enabled.yaml b/.chloggen/fix_sts_when_tls_is_enabled.yaml
@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# 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:  Fix panic when toggling spec.storage.tls.enabled to true, when using Tempo with AWS STS
+
+# One or more tracking issues related to the change
+issues: [1067]
+
+# (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:
diff --git a/internal/handlers/storage/secret.go b/internal/handlers/storage/secret.go
@@ -128,10 +128,10 @@ func getS3Params(storageSecret corev1.Secret, path *field.Path) (*manifestutils.
 	endpoint = strings.TrimPrefix(endpoint, "http://")
 
 	return &manifestutils.S3{
+		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
@@ -22,7 +22,7 @@ func TestGetS3ParamsInsecure(t *testing.T) {
 	s3, errs := getS3Params(storageSecret, nil)
 	require.Len(t, errs, 0)
 	require.Equal(t, "minio:9000", s3.LongLived.Endpoint)
-	require.True(t, s3.LongLived.Insecure)
+	require.True(t, s3.Insecure)
 	require.Equal(t, "testbucket", s3.LongLived.Bucket)
 }
 
@@ -39,7 +39,7 @@ func TestGetS3ParamsSecure(t *testing.T) {
 	s3, errs := getS3Params(storageSecret, nil)
 	require.Len(t, errs, 0)
 	require.Equal(t, "minio:9000", s3.LongLived.Endpoint)
-	require.False(t, s3.LongLived.Insecure)
+	require.False(t, s3.Insecure)
 	require.Equal(t, "testbucket", s3.LongLived.Bucket)
 }
 

diff --git a/internal/handlers/storage/storage.go b/internal/handlers/storage/storage.go
@@ -31,7 +31,9 @@ func GetStorageParamsForTempoStack(ctx context.Context, client client.Client, te
 			return manifestutils.StorageParams{}, errs
 		}
 
-		if tempo.Spec.Storage.TLS.Enabled {
+		storageParams.S3.Insecure = !tempo.Spec.Storage.TLS.Enabled
+
+		if tempo.Spec.Storage.TLS.Enabled && storageParams.S3.LongLived != nil {
 			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

diff --git a/internal/manifests/config/build_test.go b/internal/manifests/config/build_test.go
@@ -119,10 +119,10 @@ query_frontend:
 		},
 		StorageParams: manifestutils.StorageParams{
 			S3: &manifestutils.S3{
+				Insecure: true,
 				LongLived: &manifestutils.S3LongLived{
 					Endpoint: "minio:9000",
 					Bucket:   "tempo",
-					Insecure: true,
 				},
 			},
 		},
@@ -982,10 +982,10 @@ query_frontend:
 				},
 				StorageParams: manifestutils.StorageParams{
 					S3: &manifestutils.S3{
+						Insecure: true,
 						LongLived: &manifestutils.S3LongLived{
 							Endpoint: "minio:9000",
 							Bucket:   "tempo",
-							Insecure: true,
 						},
 					},
 				},
@@ -1359,10 +1359,10 @@ query_frontend:
 		},
 		StorageParams: manifestutils.StorageParams{
 			S3: &manifestutils.S3{
+				Insecure: true,
 				LongLived: &manifestutils.S3LongLived{
 					Endpoint: "minio:9000",
 					Bucket:   "tempo",
-					Insecure: true,
 				},
 			},
 		},
@@ -1651,10 +1651,10 @@ ingester_client:
 				},
 				StorageParams: manifestutils.StorageParams{
 					S3: &manifestutils.S3{
+						Insecure: true,
 						LongLived: &manifestutils.S3LongLived{
 							Endpoint: "minio:9000",
 							Bucket:   "tempo",
-							Insecure: true,
 						},
 					},
 				},
@@ -1816,10 +1816,10 @@ ingester_client:
 		},
 		StorageParams: manifestutils.StorageParams{
 			S3: &manifestutils.S3{
+				Insecure: true,
 				LongLived: &manifestutils.S3LongLived{
 					Endpoint: "minio:9000",
 					Bucket:   "tempo",
-					Insecure: true,
 				},
 			},
 		},
@@ -2184,10 +2184,10 @@ query_frontend:
 				},
 				StorageParams: manifestutils.StorageParams{
 					S3: &manifestutils.S3{
+						Insecure: true,
 						LongLived: &manifestutils.S3LongLived{
 							Endpoint: "minio:9000",
 							Bucket:   "tempo",
-							Insecure: true,
 						},
 					},
 				},
@@ -2403,10 +2403,10 @@ query_frontend:
 				},
 				StorageParams: manifestutils.StorageParams{
 					S3: &manifestutils.S3{
+						Insecure: true,
 						LongLived: &manifestutils.S3LongLived{
 							Endpoint: "minio:9000",
 							Bucket:   "tempo",
-							Insecure: true,
 						},
 					},
 				},
@@ -2642,8 +2642,8 @@ query_frontend:
 				LongLived: &manifestutils.S3LongLived{
 					Endpoint: "minio:9000",
 					Bucket:   "tempo",
-					Insecure: true,
 				},
+				Insecure: true,
 			},
 		},
 		TLSProfile: tlsprofile.TLSProfileOptions{

diff --git a/internal/manifests/config/tempo-config.yaml b/internal/manifests/config/tempo-config.yaml
@@ -231,7 +231,7 @@ storage:
     s3:
       endpoint: {{ .StorageParams.S3.LongLived.Endpoint }}
       bucket: {{ .StorageParams.S3.LongLived.Bucket }}
-      insecure: {{ .StorageParams.S3.LongLived.Insecure }}
+      insecure: {{ .StorageParams.S3.Insecure }}
     {{- if .S3StorageTLS.Enabled }}
     {{- if .S3StorageTLS.CA }}
       tls_ca_path: {{ .S3StorageTLS.CA }}

diff --git a/internal/manifests/manifestutils/params.go b/internal/manifests/manifestutils/params.go
@@ -40,6 +40,7 @@ type GCS struct {
 type S3 struct {
 	LongLived  *S3LongLived
 	ShortLived *S3ShortLived
+	Insecure   bool
 }
 
 // S3LongLived holds long-lived S3 configuration.
@@ -48,9 +49,7 @@ type S3LongLived struct {
 	// Endpoint without http/https
 	Endpoint string
 	Bucket   string
-	Insecure bool
-
-	TLS StorageTLS
+	TLS      StorageTLS
 }
 
 // S3ShortLived holds short-lived S3 configuration.

diff --git a/internal/manifests/monolithic/configmap.go b/internal/manifests/monolithic/configmap.go
@@ -201,7 +201,7 @@ func buildTempoConfig(opts Options) ([]byte, error) {
 			config.Storage.Trace.S3 = &tempoS3Config{}
 			if opts.StorageParams.S3.LongLived != nil {
 				config.Storage.Trace.S3.Endpoint = opts.StorageParams.S3.LongLived.Endpoint
-				config.Storage.Trace.S3.Insecure = opts.StorageParams.S3.LongLived.Insecure
+				config.Storage.Trace.S3.Insecure = opts.StorageParams.S3.Insecure
 				config.Storage.Trace.S3.Bucket = opts.StorageParams.S3.LongLived.Bucket
 				if tempo.Spec.Storage.Traces.S3 != nil && tempo.Spec.Storage.Traces.S3.TLS != nil && tempo.Spec.Storage.Traces.S3.TLS.Enabled {
 					if tempo.Spec.Storage.Traces.S3.TLS.CA != "" {