From 0084262269f4e2cb94d04e0cc0d40e9666177f06 Mon Sep 17 00:00:00 2001 From: Bayan Taani <86984560+btaani@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:51:11 +0200 Subject: [PATCH] fix(operator): Configure Loki to use virtual-host-style URLs for S3 AWS endpoints (#12469) --- operator/CHANGELOG.md | 1 + .../handlers/internal/storage/secrets.go | 10 +- .../handlers/internal/storage/secrets_test.go | 57 +++++++ .../manifests/internal/config/build_test.go | 147 ++++++++++-------- .../internal/config/loki-config.yaml | 4 +- .../internal/manifests/storage/options.go | 13 +- 6 files changed, 155 insertions(+), 77 deletions(-) diff --git a/operator/CHANGELOG.md b/operator/CHANGELOG.md index ef3d8ccc0cdb8..a45aca6e2c12d 100644 --- a/operator/CHANGELOG.md +++ b/operator/CHANGELOG.md @@ -1,5 +1,6 @@ ## Main +- [12469](https://github.com/grafana/loki/pull/12469) **btaani**: Configure Loki to use virtual-host-style URLs for S3 AWS endpoints - [12181](https://github.com/grafana/loki/pull/12181) **btaani**: Improve validation of provided S3 storage configuration - [12370](https://github.com/grafana/loki/pull/12370) **periklis**: Update Loki operand to v2.9.6 - [12333](https://github.com/grafana/loki/pull/12333) **periklis**: Bump max OpenShift version to next release diff --git a/operator/internal/handlers/internal/storage/secrets.go b/operator/internal/handlers/internal/storage/secrets.go index 7188216311dff..2b591ba34f3f1 100644 --- a/operator/internal/handlers/internal/storage/secrets.go +++ b/operator/internal/handlers/internal/storage/secrets.go @@ -404,7 +404,8 @@ func extractS3ConfigSecret(s *corev1.Secret, credentialMode lokiv1.CredentialMod roleArn = s.Data[storage.KeyAWSRoleArn] audience = s.Data[storage.KeyAWSAudience] // Optional fields - region = s.Data[storage.KeyAWSRegion] + region = s.Data[storage.KeyAWSRegion] + forcePathStyle = !strings.HasSuffix(string(endpoint), awsEndpointSuffix) ) sseCfg, err := extractS3SSEConfig(s.Data) @@ -413,9 +414,10 @@ func extractS3ConfigSecret(s *corev1.Secret, credentialMode lokiv1.CredentialMod } cfg := &storage.S3StorageConfig{ - Buckets: string(buckets), - Region: string(region), - SSE: sseCfg, + Buckets: string(buckets), + Region: string(region), + SSE: sseCfg, + ForcePathStyle: forcePathStyle, } switch credentialMode { diff --git a/operator/internal/handlers/internal/storage/secrets_test.go b/operator/internal/handlers/internal/storage/secrets_test.go index a85e2f6911d6e..465ffb31d8aef 100644 --- a/operator/internal/handlers/internal/storage/secrets_test.go +++ b/operator/internal/handlers/internal/storage/secrets_test.go @@ -9,6 +9,7 @@ import ( configv1 "github.com/grafana/loki/operator/apis/config/v1" lokiv1 "github.com/grafana/loki/operator/apis/loki/v1" + "github.com/grafana/loki/operator/internal/manifests/storage" ) func TestHashSecretData(t *testing.T) { @@ -617,6 +618,62 @@ func TestS3Extract(t *testing.T) { } } +func TestS3Extract_S3ForcePathStyle(t *testing.T) { + tt := []struct { + desc string + secret *corev1.Secret + wantOptions *storage.S3StorageConfig + }{ + { + desc: "aws s3 endpoint", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Data: map[string][]byte{ + "endpoint": []byte("https://s3.region.amazonaws.com"), + "region": []byte("region"), + "bucketnames": []byte("this,that"), + "access_key_id": []byte("id"), + "access_key_secret": []byte("secret"), + }, + }, + wantOptions: &storage.S3StorageConfig{ + Endpoint: "https://s3.region.amazonaws.com", + Region: "region", + Buckets: "this,that", + }, + }, + { + desc: "non-aws s3 endpoint", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Data: map[string][]byte{ + "endpoint": []byte("https://test.default.svc.cluster.local:9000"), + "region": []byte("region"), + "bucketnames": []byte("this,that"), + "access_key_id": []byte("id"), + "access_key_secret": []byte("secret"), + }, + }, + wantOptions: &storage.S3StorageConfig{ + Endpoint: "https://test.default.svc.cluster.local:9000", + Region: "region", + Buckets: "this,that", + ForcePathStyle: true, + }, + }, + } + + for _, tc := range tt { + tc := tc + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + options, err := extractS3ConfigSecret(tc.secret, lokiv1.CredentialModeStatic) + require.NoError(t, err) + require.Equal(t, tc.wantOptions, options) + }) + } +} + func TestS3Extract_WithOpenShiftTokenCCOAuth(t *testing.T) { fg := configv1.FeatureGates{ OpenShift: configv1.OpenShiftFeatureGates{ diff --git a/operator/internal/manifests/internal/config/build_test.go b/operator/internal/manifests/internal/config/build_test.go index 53650429196e0..fa20c45a0226b 100644 --- a/operator/internal/manifests/internal/config/build_test.go +++ b/operator/internal/manifests/internal/config/build_test.go @@ -25,7 +25,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -245,9 +245,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -282,7 +283,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -592,9 +593,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -676,9 +678,10 @@ func TestBuild_ConfigAndRuntimeConfig_CreateLokiConfigFailed(t *testing.T) { ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -707,7 +710,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -1028,9 +1031,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -1065,7 +1069,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -1387,9 +1391,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -1424,7 +1429,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -1776,9 +1781,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -1813,7 +1819,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -2111,9 +2117,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -2151,7 +2158,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -2533,9 +2540,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -2570,12 +2578,11 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://s3.us-east.amazonaws.com bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} secret_access_key: ${AWS_ACCESS_KEY_SECRET} - s3forcepathstyle: true compactor_grpc_address: loki-compactor-grpc-lokistack-dev.default.svc.cluster.local:9095 ring: kvstore: @@ -2879,7 +2886,7 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", + Endpoint: "http://s3.us-east.amazonaws.com", Region: "us-east", Buckets: "loki", }, @@ -2916,7 +2923,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -3375,9 +3382,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -3412,7 +3420,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -3635,9 +3643,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -3672,7 +3681,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -3897,9 +3906,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -3934,7 +3944,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -4157,9 +4167,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -4194,7 +4205,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -4454,9 +4465,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, SSE: storage.S3SSEConfig{ Type: storage.SSEKMSType, @@ -4496,7 +4508,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -4753,9 +4765,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, SSE: storage.S3SSEConfig{ Type: storage.SSES3Type, @@ -4795,7 +4808,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} @@ -5011,9 +5024,10 @@ overrides: ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -5100,9 +5114,10 @@ func defaultOptions() Options { ObjectStorage: storage.Options{ SharedStore: lokiv1.ObjectStorageSecretS3, S3: &storage.S3StorageConfig{ - Endpoint: "http://test.default.svc.cluster.local.:9000", - Region: "us-east", - Buckets: "loki", + Endpoint: "http://test.default.svc.cluster.local.:9000", + Region: "us-east", + Buckets: "loki", + ForcePathStyle: true, }, Schemas: []lokiv1.ObjectStorageSchema{ { @@ -5284,7 +5299,7 @@ chunk_store_config: common: storage: s3: - s3: http://test.default.svc.cluster.local.:9000 + endpoint: http://test.default.svc.cluster.local.:9000 bucketnames: loki region: us-east access_key_id: ${AWS_ACCESS_KEY_ID} diff --git a/operator/internal/manifests/internal/config/loki-config.yaml b/operator/internal/manifests/internal/config/loki-config.yaml index 2b29c51806cf7..3df0ac7463881 100644 --- a/operator/internal/manifests/internal/config/loki-config.yaml +++ b/operator/internal/manifests/internal/config/loki-config.yaml @@ -33,12 +33,14 @@ common: region: {{.Region}} s3forcepathstyle: false {{- else }} - s3: {{ .Endpoint }} + endpoint: {{ .Endpoint }} bucketnames: {{ .Buckets }} region: {{ .Region }} access_key_id: ${AWS_ACCESS_KEY_ID} secret_access_key: ${AWS_ACCESS_KEY_SECRET} + {{- if .ForcePathStyle }} s3forcepathstyle: true + {{- end}} {{- end }} {{- with .SSE }} {{- if .Type }} diff --git a/operator/internal/manifests/storage/options.go b/operator/internal/manifests/storage/options.go index 47779ce554793..1f8d3d904b71a 100644 --- a/operator/internal/manifests/storage/options.go +++ b/operator/internal/manifests/storage/options.go @@ -42,12 +42,13 @@ type GCSStorageConfig struct { // S3StorageConfig for S3 storage config type S3StorageConfig struct { - Endpoint string - Region string - Buckets string - Audience string - STS bool - SSE S3SSEConfig + Endpoint string + Region string + Buckets string + Audience string + STS bool + SSE S3SSEConfig + ForcePathStyle bool } type S3SSEType string