From 5016e4dc38b77e4dbfacfd4f4131d81ce402ddf7 Mon Sep 17 00:00:00 2001 From: ntny Date: Tue, 24 Sep 2024 22:27:17 +0300 Subject: [PATCH] feat: add configurable S3 path style support Signed-off-by: ntny --- backend/src/v2/config/env_test.go | 108 ++++++++++-------- backend/src/v2/config/s3.go | 13 +++ .../v2/config/testdata/provider_cases.yaml | 2 + backend/src/v2/objectstore/config.go | 18 ++- backend/src/v2/objectstore/object_store.go | 2 +- .../src/v2/objectstore/object_store_test.go | 15 +-- 6 files changed, 95 insertions(+), 63 deletions(-) diff --git a/backend/src/v2/config/env_test.go b/backend/src/v2/config/env_test.go index 8f120126394..a891633c238 100644 --- a/backend/src/v2/config/env_test.go +++ b/backend/src/v2/config/env_test.go @@ -197,13 +197,14 @@ func TestGetBucketSessionInfo(t *testing.T) { expectedSessionInfo: objectstore.SessionInfo{ Provider: "minio", Params: map[string]string{ - "region": "minio", - "endpoint": "minio-endpoint-5.com", - "disableSSL": "true", - "fromEnv": "false", - "secretName": "test-secret-5", - "accessKeyKey": "test-accessKeyKey-5", - "secretKeyKey": "test-secretKeyKey-5", + "region": "minio", + "endpoint": "minio-endpoint-5.com", + "disableSSL": "true", + "fromEnv": "false", + "secretName": "test-secret-5", + "accessKeyKey": "test-accessKeyKey-5", + "secretKeyKey": "test-secretKeyKey-5", + "forcePathStyle": "true", }, }, testDataCase: "case5", @@ -214,13 +215,14 @@ func TestGetBucketSessionInfo(t *testing.T) { expectedSessionInfo: objectstore.SessionInfo{ Provider: "minio", Params: map[string]string{ - "region": "minio-a", - "endpoint": "minio-endpoint-6.com", - "disableSSL": "true", - "fromEnv": "false", - "secretName": "minio-test-secret-6-a", - "accessKeyKey": "minio-test-accessKeyKey-6-a", - "secretKeyKey": "minio-test-secretKeyKey-6-a", + "region": "minio-a", + "endpoint": "minio-endpoint-6.com", + "disableSSL": "true", + "fromEnv": "false", + "secretName": "minio-test-secret-6-a", + "accessKeyKey": "minio-test-accessKeyKey-6-a", + "secretKeyKey": "minio-test-secretKeyKey-6-a", + "forcePathStyle": "true", }, }, testDataCase: "case6", @@ -263,10 +265,11 @@ func TestGetBucketSessionInfo(t *testing.T) { expectedSessionInfo: objectstore.SessionInfo{ Provider: "minio", Params: map[string]string{ - "region": "minio-a", - "endpoint": "minio-endpoint-9.com", - "disableSSL": "true", - "fromEnv": "true", + "region": "minio-a", + "endpoint": "minio-endpoint-9.com", + "disableSSL": "true", + "fromEnv": "true", + "forcePathStyle": "true", }, }, testDataCase: "case9", @@ -277,13 +280,14 @@ func TestGetBucketSessionInfo(t *testing.T) { expectedSessionInfo: objectstore.SessionInfo{ Provider: "minio", Params: map[string]string{ - "region": "minio-a", - "endpoint": "minio-endpoint-10.com", - "disableSSL": "true", - "fromEnv": "false", - "secretName": "minio-test-secret-10", - "accessKeyKey": "minio-test-accessKeyKey-10", - "secretKeyKey": "minio-test-secretKeyKey-10", + "region": "minio-a", + "endpoint": "minio-endpoint-10.com", + "disableSSL": "true", + "fromEnv": "false", + "secretName": "minio-test-secret-10", + "accessKeyKey": "minio-test-accessKeyKey-10", + "secretKeyKey": "minio-test-secretKeyKey-10", + "forcePathStyle": "true", }, }, testDataCase: "case10", @@ -294,10 +298,11 @@ func TestGetBucketSessionInfo(t *testing.T) { expectedSessionInfo: objectstore.SessionInfo{ Provider: "minio", Params: map[string]string{ - "region": "minio", - "endpoint": "minio-endpoint-10.com", - "disableSSL": "true", - "fromEnv": "true", + "region": "minio", + "endpoint": "minio-endpoint-10.com", + "disableSSL": "true", + "fromEnv": "true", + "forcePathStyle": "true", }, }, testDataCase: "case10", @@ -308,13 +313,14 @@ func TestGetBucketSessionInfo(t *testing.T) { expectedSessionInfo: objectstore.SessionInfo{ Provider: "s3", Params: map[string]string{ - "region": "us-east-1", - "endpoint": "s3.amazonaws.com", - "disableSSL": "false", - "fromEnv": "false", - "secretName": "s3-testsecret-6", - "accessKeyKey": "s3-testaccessKeyKey-6", - "secretKeyKey": "s3-testsecretKeyKey-6", + "region": "us-east-1", + "endpoint": "s3.amazonaws.com", + "disableSSL": "false", + "fromEnv": "false", + "secretName": "s3-testsecret-6", + "accessKeyKey": "s3-testaccessKeyKey-6", + "secretKeyKey": "s3-testsecretKeyKey-6", + "forcePathStyle": "false", }, }, testDataCase: "case6", @@ -325,13 +331,14 @@ func TestGetBucketSessionInfo(t *testing.T) { expectedSessionInfo: objectstore.SessionInfo{ Provider: "s3", Params: map[string]string{ - "region": "us-east-2", - "endpoint": "s3.us-east-2.amazonaws.com", - "disableSSL": "false", - "fromEnv": "false", - "secretName": "s3-test-secret-6-b", - "accessKeyKey": "s3-test-accessKeyKey-6-b", - "secretKeyKey": "s3-test-secretKeyKey-6-b", + "region": "us-east-2", + "endpoint": "s3.us-east-2.amazonaws.com", + "disableSSL": "false", + "fromEnv": "false", + "secretName": "s3-test-secret-6-b", + "accessKeyKey": "s3-test-accessKeyKey-6-b", + "secretKeyKey": "s3-test-secretKeyKey-6-b", + "forcePathStyle": "false", }, }, testDataCase: "case6", @@ -342,13 +349,14 @@ func TestGetBucketSessionInfo(t *testing.T) { expectedSessionInfo: objectstore.SessionInfo{ Provider: "s3", Params: map[string]string{ - "region": "us-east-1", - "endpoint": "s3.amazonaws.com", - "disableSSL": "false", - "fromEnv": "false", - "secretName": "s3-test-secret-6-a", - "accessKeyKey": "s3-test-accessKeyKey-6-a", - "secretKeyKey": "s3-test-secretKeyKey-6-a", + "region": "us-east-1", + "endpoint": "s3.amazonaws.com", + "disableSSL": "false", + "fromEnv": "false", + "secretName": "s3-test-secret-6-a", + "accessKeyKey": "s3-test-accessKeyKey-6-a", + "secretKeyKey": "s3-test-secretKeyKey-6-a", + "forcePathStyle": "false", }, }, testDataCase: "case6", diff --git a/backend/src/v2/config/s3.go b/backend/src/v2/config/s3.go index 8cfc86d8514..ab6748b0460 100644 --- a/backend/src/v2/config/s3.go +++ b/backend/src/v2/config/s3.go @@ -34,6 +34,8 @@ type S3ProviderDefault struct { Region string `json:"region"` // optional DisableSSL *bool `json:"disableSSL"` + // optional + ForcePathStyle *bool `json:"forcePathStyle"` } type S3Credentials struct { @@ -52,6 +54,8 @@ type S3Override struct { KeyPrefix string `json:"keyPrefix"` // required Credentials *S3Credentials `json:"credentials"` + // optional + ForcePathStyle *bool `json:"forcePathStyle"` } type S3SecretRef struct { SecretName string `json:"secretName"` @@ -99,6 +103,12 @@ func (p S3ProviderConfig) ProvideSessionInfo(path string) (objectstore.SessionIn params["disableSSL"] = strconv.FormatBool(*p.Default.DisableSSL) } + if p.Default.ForcePathStyle == nil { + params["forcePathStyle"] = strconv.FormatBool(true) + } else { + params["forcePathStyle"] = strconv.FormatBool(*p.Default.ForcePathStyle) + } + params["fromEnv"] = strconv.FormatBool(p.Default.Credentials.FromEnv) if !p.Default.Credentials.FromEnv { params["secretName"] = p.Default.Credentials.SecretRef.SecretName @@ -124,6 +134,9 @@ func (p S3ProviderConfig) ProvideSessionInfo(path string) (objectstore.SessionIn if override.DisableSSL != nil { sessionInfo.Params["disableSSL"] = strconv.FormatBool(*override.DisableSSL) } + if override.ForcePathStyle != nil { + sessionInfo.Params["forcePathStyle"] = strconv.FormatBool(*override.ForcePathStyle) + } if override.Credentials == nil { return objectstore.SessionInfo{}, invalidConfigErr(fmt.Errorf("missing override credentials")) } diff --git a/backend/src/v2/config/testdata/provider_cases.yaml b/backend/src/v2/config/testdata/provider_cases.yaml index eeba3160838..75ad95da4b4 100644 --- a/backend/src/v2/config/testdata/provider_cases.yaml +++ b/backend/src/v2/config/testdata/provider_cases.yaml @@ -95,6 +95,7 @@ cases: endpoint: s3.amazonaws.com region: us-east-1 disableSSL: false + forcePathStyle: false credentials: fromEnv: false secretRef: @@ -123,6 +124,7 @@ cases: endpoint: s3.us-east-2.amazonaws.com region: us-east-2 disableSSL: false + forcePathStyle: false credentials: fromEnv: false secretRef: diff --git a/backend/src/v2/objectstore/config.go b/backend/src/v2/objectstore/config.go index 28e82cd65de..cc8d6372eb6 100644 --- a/backend/src/v2/objectstore/config.go +++ b/backend/src/v2/objectstore/config.go @@ -53,11 +53,12 @@ type S3Params struct { FromEnv bool SecretName string // The k8s secret "Key" for "Artifact SecretKey" and "Artifact AccessKey" - AccessKeyKey string - SecretKeyKey string - Region string - Endpoint string - DisableSSL bool + AccessKeyKey string + SecretKeyKey string + Region string + Endpoint string + DisableSSL bool + ForcePathStyle bool } func (b *Config) bucketURL() string { @@ -221,6 +222,13 @@ func StructuredS3Params(p map[string]string) (*S3Params, error) { } sparams.DisableSSL = boolVal } + if val, ok := p["forcePathStyle"]; ok { + boolVal, err := strconv.ParseBool(val) + if err != nil { + return nil, err + } + sparams.ForcePathStyle = boolVal + } return sparams, nil } diff --git a/backend/src/v2/objectstore/object_store.go b/backend/src/v2/objectstore/object_store.go index a7b85565565..41b5118c49f 100644 --- a/backend/src/v2/objectstore/object_store.go +++ b/backend/src/v2/objectstore/object_store.go @@ -258,7 +258,7 @@ func createS3BucketSession(ctx context.Context, namespace string, sessionInfo *S config.Credentials = creds config.Region = aws.String(params.Region) config.DisableSSL = aws.Bool(params.DisableSSL) - config.S3ForcePathStyle = aws.Bool(true) + config.S3ForcePathStyle = aws.Bool(params.ForcePathStyle) // AWS Specific: // Path-style S3 endpoints, which are commonly used, may fall into either of two subdomains: diff --git a/backend/src/v2/objectstore/object_store_test.go b/backend/src/v2/objectstore/object_store_test.go index 7cefdeb1ee7..2faf59765fb 100644 --- a/backend/src/v2/objectstore/object_store_test.go +++ b/backend/src/v2/objectstore/object_store_test.go @@ -273,13 +273,14 @@ func Test_createS3BucketSession(t *testing.T) { sessionInfo: &SessionInfo{ Provider: "s3", Params: map[string]string{ - "region": "us-east-1", - "endpoint": "s3.amazonaws.com", - "disableSSL": "false", - "fromEnv": "false", - "secretName": "s3-provider-secret", - "accessKeyKey": "test_access_key", - "secretKeyKey": "test_secret_key", + "region": "us-east-1", + "endpoint": "s3.amazonaws.com", + "disableSSL": "false", + "fromEnv": "false", + "secretName": "s3-provider-secret", + "accessKeyKey": "test_access_key", + "secretKeyKey": "test_secret_key", + "forcePathStyle": "true", }, }, sessionSecret: &corev1.Secret{