From 6dd603fbff950440245d76b9a56fde271f69291a Mon Sep 17 00:00:00 2001 From: notedop Date: Thu, 13 Feb 2025 18:05:25 +0100 Subject: [PATCH 1/8] add chunk delimiter when the chunk_delimiter is specified then the objectkey will be converted prior to sending a request to the S3 (MinIO) storage provider. The list() response creates StorageObjects so convert the key back to normal again so that downstream logic still works. --- .../chunk/client/aws/s3_storage_client.go | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/pkg/storage/chunk/client/aws/s3_storage_client.go b/pkg/storage/chunk/client/aws/s3_storage_client.go index e7891e4963fc0..05c14f0a361e5 100644 --- a/pkg/storage/chunk/client/aws/s3_storage_client.go +++ b/pkg/storage/chunk/client/aws/s3_storage_client.go @@ -76,6 +76,7 @@ type S3Config struct { SecretAccessKey flagext.Secret `yaml:"secret_access_key"` SessionToken flagext.Secret `yaml:"session_token"` Insecure bool `yaml:"insecure"` + ChunkDelimiter string `yaml:"chunk_delimiter"` HTTPConfig HTTPConfig `yaml:"http_config"` SignatureVersion string `yaml:"signature_version"` StorageClass string `yaml:"storage_class"` @@ -335,7 +336,7 @@ func (a *S3ObjectClient) objectAttributes(ctx context.Context, objectKey, method lastErr = instrument.CollectedRequest(ctx, method, s3RequestDuration, instrument.ErrorCode, func(_ context.Context) error { headObjectInput := &s3.HeadObjectInput{ Bucket: aws.String(a.bucketFromKey(objectKey)), - Key: aws.String(objectKey), + Key: aws.String(a.ConvertObjectKey(objectKey, true)), } headOutput, requestErr := a.S3.HeadObject(headObjectInput) if requestErr != nil { @@ -365,7 +366,7 @@ func (a *S3ObjectClient) DeleteObject(ctx context.Context, objectKey string) err return instrument.CollectedRequest(ctx, "S3.DeleteObject", s3RequestDuration, instrument.ErrorCode, func(ctx context.Context) error { deleteObjectInput := &s3.DeleteObjectInput{ Bucket: aws.String(a.bucketFromKey(objectKey)), - Key: aws.String(objectKey), + Key: aws.String(a.ConvertObjectKey(objectKey, true)), } _, err := a.S3.DeleteObjectWithContext(ctx, deleteObjectInput) @@ -405,7 +406,7 @@ func (a *S3ObjectClient) GetObject(ctx context.Context, objectKey string) (io.Re var requestErr error resp, requestErr = a.hedgedS3.GetObjectWithContext(ctx, &s3.GetObjectInput{ Bucket: aws.String(bucket), - Key: aws.String(objectKey), + Key: aws.String(a.ConvertObjectKey(objectKey, true)), }) return requestErr }) @@ -442,7 +443,7 @@ func (a *S3ObjectClient) GetObjectRange(ctx context.Context, objectKey string, o var requestErr error resp, requestErr = a.hedgedS3.GetObjectWithContext(ctx, &s3.GetObjectInput{ Bucket: aws.String(bucket), - Key: aws.String(objectKey), + Key: aws.String(a.ConvertObjectKey(objectKey, true)), Range: aws.String(fmt.Sprintf("bytes=%d-%d", offset, offset+length-1)), }) return requestErr @@ -467,7 +468,7 @@ func (a *S3ObjectClient) PutObject(ctx context.Context, objectKey string, object putObjectInput := &s3.PutObjectInput{ Body: readSeeker, Bucket: aws.String(a.bucketFromKey(objectKey)), - Key: aws.String(objectKey), + Key: aws.String(a.ConvertObjectKey(objectKey, true)), StorageClass: aws.String(a.cfg.StorageClass), } @@ -504,7 +505,7 @@ func (a *S3ObjectClient) List(ctx context.Context, prefix, delimiter string) ([] for _, content := range output.Contents { storageObjects = append(storageObjects, client.StorageObject{ - Key: *content.Key, + Key: a.ConvertObjectKey(*content.Key, false), ModifiedAt: *content.LastModified, }) } @@ -617,3 +618,15 @@ func IsRetryableErr(err error) bool { func (a *S3ObjectClient) IsRetryableErr(err error) bool { return IsRetryableErr(err) } + +// ConvertObjectKey modifies the object key based on a delimiter and a mode flag determining conversion. +func (a *S3ObjectClient) ConvertObjectKey(objectKey string, toS3 bool) string { + if len(a.cfg.ChunkDelimiter) == 1 { + if toS3 { + objectKey = strings.ReplaceAll(objectKey, ":", string(a.cfg.ChunkDelimiter)) + } else { + objectKey = strings.ReplaceAll(objectKey, string(a.cfg.ChunkDelimiter), ":") + } + } + return objectKey +} From 3663a6d87152cb0919a43dcf702b261bf03632e4 Mon Sep 17 00:00:00 2001 From: notedop Date: Thu, 13 Feb 2025 18:06:38 +0100 Subject: [PATCH 2/8] update tests --- pkg/loki/config_wrapper_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/loki/config_wrapper_test.go b/pkg/loki/config_wrapper_test.go index 952804d607de4..cb2994e8307d0 100644 --- a/pkg/loki/config_wrapper_test.go +++ b/pkg/loki/config_wrapper_test.go @@ -264,6 +264,7 @@ memberlist: secret_access_key: def789 insecure: true disable_dualstack: true + chunk_delimiter: "-" http_config: response_header_timeout: 5m` @@ -289,6 +290,7 @@ memberlist: assert.Equal(t, "", actual.SessionToken.String()) assert.Equal(t, true, actual.Insecure) assert.True(t, actual.DisableDualstack) + assert.Equal(t, "-", actual.ChunkDelimiter) assert.Equal(t, 5*time.Minute, actual.HTTPConfig.ResponseHeaderTimeout) assert.Equal(t, false, actual.HTTPConfig.InsecureSkipVerify) @@ -354,6 +356,7 @@ memberlist: assert.Equal(t, "456abc", actual.SessionToken.String()) assert.Equal(t, true, actual.Insecure) assert.False(t, actual.DisableDualstack) + assert.Equal(t, "", actual.ChunkDelimiter) assert.Equal(t, 5*time.Minute, actual.HTTPConfig.ResponseHeaderTimeout) assert.Equal(t, false, actual.HTTPConfig.InsecureSkipVerify) From 2a6cb6904d0108b97bc263ce242235d04531bbc2 Mon Sep 17 00:00:00 2001 From: notedop Date: Thu, 13 Feb 2025 18:08:57 +0100 Subject: [PATCH 3/8] update S3 cluster example --- docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml b/docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml index cde37ed5b2e8a..76480b091a35d 100644 --- a/docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml +++ b/docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml @@ -32,3 +32,5 @@ storage_config: aws: s3: s3://access_key:secret_access_key@custom_endpoint/bucket_name s3forcepathstyle: true + # when running MinIO on a Windows machine, you must override the default ":" chunk object key delimiter with for example a dash or underscore. Only required in this specific scenario + chunk_delimiter: "-" From 2c0c4dfee87a552a1aa41fe63ac2c2c54370fdd6 Mon Sep 17 00:00:00 2001 From: notedop Date: Wed, 26 Feb 2025 14:20:58 +0100 Subject: [PATCH 4/8] add chunk-delimiter flag adding the flag mapping as well as updating the documentation. --- docs/sources/shared/configuration.md | 7 +++++++ pkg/storage/chunk/client/aws/s3_storage_client.go | 1 + 2 files changed, 8 insertions(+) diff --git a/docs/sources/shared/configuration.md b/docs/sources/shared/configuration.md index e065282ea722c..b8f501c91c14e 100644 --- a/docs/sources/shared/configuration.md +++ b/docs/sources/shared/configuration.md @@ -4729,6 +4729,13 @@ The `s3_storage_config` block configures the connection to Amazon S3 object stor # CLI flag: -.s3.insecure [insecure: | default = false] +# Delimiter used to replace the default delimiter ':' in chunk IDs +# when storing chunks. This is mainly intended when you run a MinIO +# instance on a Windows machine. +# You should not change this value inflight. +# CLI flag: -.s3.chunk-delimiter +[chunk_delimiter: | default = ""] + http_config: # Timeout specifies a time limit for requests made by s3 Client. # CLI flag: -.s3.http.timeout diff --git a/pkg/storage/chunk/client/aws/s3_storage_client.go b/pkg/storage/chunk/client/aws/s3_storage_client.go index 05c14f0a361e5..e6c50a1d66706 100644 --- a/pkg/storage/chunk/client/aws/s3_storage_client.go +++ b/pkg/storage/chunk/client/aws/s3_storage_client.go @@ -114,6 +114,7 @@ func (cfg *S3Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { f.Var(&cfg.SecretAccessKey, prefix+"s3.secret-access-key", "AWS Secret Access Key") f.Var(&cfg.SessionToken, prefix+"s3.session-token", "AWS Session Token") f.BoolVar(&cfg.Insecure, prefix+"s3.insecure", false, "Disable https on s3 connection.") + f.StringVar(&cfg.ChunkDelimiter, prefix+"s3.chunk-delimiter", "", "Chunk delimiter for blob ID to be used") f.BoolVar(&cfg.DisableDualstack, prefix+"s3.disable-dualstack", false, "Disable forcing S3 dualstack endpoint usage.") cfg.SSEConfig.RegisterFlagsWithPrefix(prefix+"s3.sse.", f) From 58f529bb0ed5e1ee369aa0ecd2d60cc696405b69 Mon Sep 17 00:00:00 2001 From: notedop Date: Mon, 10 Mar 2025 14:57:09 +0100 Subject: [PATCH 5/8] undo changes to configuration.md configuration.md is auto generated and usage is coming from the RegisterFlagsWithPrefix definition. --- docs/sources/shared/configuration.md | 7 ------- pkg/storage/chunk/client/aws/s3_storage_client.go | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/docs/sources/shared/configuration.md b/docs/sources/shared/configuration.md index b8f501c91c14e..e065282ea722c 100644 --- a/docs/sources/shared/configuration.md +++ b/docs/sources/shared/configuration.md @@ -4729,13 +4729,6 @@ The `s3_storage_config` block configures the connection to Amazon S3 object stor # CLI flag: -.s3.insecure [insecure: | default = false] -# Delimiter used to replace the default delimiter ':' in chunk IDs -# when storing chunks. This is mainly intended when you run a MinIO -# instance on a Windows machine. -# You should not change this value inflight. -# CLI flag: -.s3.chunk-delimiter -[chunk_delimiter: | default = ""] - http_config: # Timeout specifies a time limit for requests made by s3 Client. # CLI flag: -.s3.http.timeout diff --git a/pkg/storage/chunk/client/aws/s3_storage_client.go b/pkg/storage/chunk/client/aws/s3_storage_client.go index e6c50a1d66706..aba73ff134382 100644 --- a/pkg/storage/chunk/client/aws/s3_storage_client.go +++ b/pkg/storage/chunk/client/aws/s3_storage_client.go @@ -114,7 +114,7 @@ func (cfg *S3Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { f.Var(&cfg.SecretAccessKey, prefix+"s3.secret-access-key", "AWS Secret Access Key") f.Var(&cfg.SessionToken, prefix+"s3.session-token", "AWS Session Token") f.BoolVar(&cfg.Insecure, prefix+"s3.insecure", false, "Disable https on s3 connection.") - f.StringVar(&cfg.ChunkDelimiter, prefix+"s3.chunk-delimiter", "", "Chunk delimiter for blob ID to be used") + f.StringVar(&cfg.ChunkDelimiter, prefix+"s3.chunk-delimiter", "", "Delimiter used to replace the default delimiter ':' in chunk IDs when storing chunks. This is mainly intended when you run a MinIO instance on a Windows machine. You should not change this value inflight.") f.BoolVar(&cfg.DisableDualstack, prefix+"s3.disable-dualstack", false, "Disable forcing S3 dualstack endpoint usage.") cfg.SSEConfig.RegisterFlagsWithPrefix(prefix+"s3.sse.", f) From 6ce7d6f62b7c935ddffc59b1cc56af92a9b8ceeb Mon Sep 17 00:00:00 2001 From: notedop Date: Tue, 11 Mar 2025 13:40:51 +0100 Subject: [PATCH 6/8] regenerate docs with 'make doc' --- docs/sources/shared/configuration.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/sources/shared/configuration.md b/docs/sources/shared/configuration.md index a3eb2cd788a9f..57c882ce9082e 100644 --- a/docs/sources/shared/configuration.md +++ b/docs/sources/shared/configuration.md @@ -4767,6 +4767,12 @@ The `s3_storage_config` block configures the connection to Amazon S3 object stor # CLI flag: -.s3.insecure [insecure: | default = false] +# Delimiter used to replace the default delimiter ':' in chunk IDs when storing +# chunks. This is mainly intended when you run a MinIO instance on a Windows +# machine. You should not change this value inflight. +# CLI flag: -.s3.chunk-delimiter +[chunk_delimiter: | default = ""] + http_config: # Timeout specifies a time limit for requests made by s3 Client. # CLI flag: -.s3.http.timeout From c2fd2e8d2ae62229d83685c8aa79b03b3ba1f053 Mon Sep 17 00:00:00 2001 From: Notedop Date: Tue, 11 Mar 2025 17:16:52 +0100 Subject: [PATCH 7/8] remove chunk-delimiter from example Signed-off-by: Notedop --- docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml b/docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml index 76480b091a35d..cde37ed5b2e8a 100644 --- a/docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml +++ b/docs/sources/configure/examples/yaml/2-S3-Cluster-Example.yaml @@ -32,5 +32,3 @@ storage_config: aws: s3: s3://access_key:secret_access_key@custom_endpoint/bucket_name s3forcepathstyle: true - # when running MinIO on a Windows machine, you must override the default ":" chunk object key delimiter with for example a dash or underscore. Only required in this specific scenario - chunk_delimiter: "-" From 2b3207ce7ce50edf7dc33e84152bd2c75d84e0c4 Mon Sep 17 00:00:00 2001 From: notedop Date: Tue, 11 Mar 2025 17:43:57 +0100 Subject: [PATCH 8/8] make convertObjectKey private --- .../chunk/client/aws/s3_storage_client.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/storage/chunk/client/aws/s3_storage_client.go b/pkg/storage/chunk/client/aws/s3_storage_client.go index aba73ff134382..574a5604e2abe 100644 --- a/pkg/storage/chunk/client/aws/s3_storage_client.go +++ b/pkg/storage/chunk/client/aws/s3_storage_client.go @@ -337,7 +337,7 @@ func (a *S3ObjectClient) objectAttributes(ctx context.Context, objectKey, method lastErr = instrument.CollectedRequest(ctx, method, s3RequestDuration, instrument.ErrorCode, func(_ context.Context) error { headObjectInput := &s3.HeadObjectInput{ Bucket: aws.String(a.bucketFromKey(objectKey)), - Key: aws.String(a.ConvertObjectKey(objectKey, true)), + Key: aws.String(a.convertObjectKey(objectKey, true)), } headOutput, requestErr := a.S3.HeadObject(headObjectInput) if requestErr != nil { @@ -367,7 +367,7 @@ func (a *S3ObjectClient) DeleteObject(ctx context.Context, objectKey string) err return instrument.CollectedRequest(ctx, "S3.DeleteObject", s3RequestDuration, instrument.ErrorCode, func(ctx context.Context) error { deleteObjectInput := &s3.DeleteObjectInput{ Bucket: aws.String(a.bucketFromKey(objectKey)), - Key: aws.String(a.ConvertObjectKey(objectKey, true)), + Key: aws.String(a.convertObjectKey(objectKey, true)), } _, err := a.S3.DeleteObjectWithContext(ctx, deleteObjectInput) @@ -407,7 +407,7 @@ func (a *S3ObjectClient) GetObject(ctx context.Context, objectKey string) (io.Re var requestErr error resp, requestErr = a.hedgedS3.GetObjectWithContext(ctx, &s3.GetObjectInput{ Bucket: aws.String(bucket), - Key: aws.String(a.ConvertObjectKey(objectKey, true)), + Key: aws.String(a.convertObjectKey(objectKey, true)), }) return requestErr }) @@ -444,7 +444,7 @@ func (a *S3ObjectClient) GetObjectRange(ctx context.Context, objectKey string, o var requestErr error resp, requestErr = a.hedgedS3.GetObjectWithContext(ctx, &s3.GetObjectInput{ Bucket: aws.String(bucket), - Key: aws.String(a.ConvertObjectKey(objectKey, true)), + Key: aws.String(a.convertObjectKey(objectKey, true)), Range: aws.String(fmt.Sprintf("bytes=%d-%d", offset, offset+length-1)), }) return requestErr @@ -469,7 +469,7 @@ func (a *S3ObjectClient) PutObject(ctx context.Context, objectKey string, object putObjectInput := &s3.PutObjectInput{ Body: readSeeker, Bucket: aws.String(a.bucketFromKey(objectKey)), - Key: aws.String(a.ConvertObjectKey(objectKey, true)), + Key: aws.String(a.convertObjectKey(objectKey, true)), StorageClass: aws.String(a.cfg.StorageClass), } @@ -506,7 +506,7 @@ func (a *S3ObjectClient) List(ctx context.Context, prefix, delimiter string) ([] for _, content := range output.Contents { storageObjects = append(storageObjects, client.StorageObject{ - Key: a.ConvertObjectKey(*content.Key, false), + Key: a.convertObjectKey(*content.Key, false), ModifiedAt: *content.LastModified, }) } @@ -620,8 +620,8 @@ func (a *S3ObjectClient) IsRetryableErr(err error) bool { return IsRetryableErr(err) } -// ConvertObjectKey modifies the object key based on a delimiter and a mode flag determining conversion. -func (a *S3ObjectClient) ConvertObjectKey(objectKey string, toS3 bool) string { +// convertObjectKey modifies the object key based on a delimiter and a mode flag determining conversion. +func (a *S3ObjectClient) convertObjectKey(objectKey string, toS3 bool) string { if len(a.cfg.ChunkDelimiter) == 1 { if toS3 { objectKey = strings.ReplaceAll(objectKey, ":", string(a.cfg.ChunkDelimiter))