Skip to content

Commit 85fdd73

Browse files
authored
feat(backend): add configurable S3 path style support (kubeflow#11246)
Signed-off-by: ntny <[email protected]>
1 parent feacb52 commit 85fdd73

File tree

6 files changed

+95
-63
lines changed

6 files changed

+95
-63
lines changed

backend/src/v2/config/env_test.go

Lines changed: 58 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,14 @@ func TestGetBucketSessionInfo(t *testing.T) {
197197
expectedSessionInfo: objectstore.SessionInfo{
198198
Provider: "minio",
199199
Params: map[string]string{
200-
"region": "minio",
201-
"endpoint": "minio-endpoint-5.com",
202-
"disableSSL": "true",
203-
"fromEnv": "false",
204-
"secretName": "test-secret-5",
205-
"accessKeyKey": "test-accessKeyKey-5",
206-
"secretKeyKey": "test-secretKeyKey-5",
200+
"region": "minio",
201+
"endpoint": "minio-endpoint-5.com",
202+
"disableSSL": "true",
203+
"fromEnv": "false",
204+
"secretName": "test-secret-5",
205+
"accessKeyKey": "test-accessKeyKey-5",
206+
"secretKeyKey": "test-secretKeyKey-5",
207+
"forcePathStyle": "true",
207208
},
208209
},
209210
testDataCase: "case5",
@@ -214,13 +215,14 @@ func TestGetBucketSessionInfo(t *testing.T) {
214215
expectedSessionInfo: objectstore.SessionInfo{
215216
Provider: "minio",
216217
Params: map[string]string{
217-
"region": "minio-a",
218-
"endpoint": "minio-endpoint-6.com",
219-
"disableSSL": "true",
220-
"fromEnv": "false",
221-
"secretName": "minio-test-secret-6-a",
222-
"accessKeyKey": "minio-test-accessKeyKey-6-a",
223-
"secretKeyKey": "minio-test-secretKeyKey-6-a",
218+
"region": "minio-a",
219+
"endpoint": "minio-endpoint-6.com",
220+
"disableSSL": "true",
221+
"fromEnv": "false",
222+
"secretName": "minio-test-secret-6-a",
223+
"accessKeyKey": "minio-test-accessKeyKey-6-a",
224+
"secretKeyKey": "minio-test-secretKeyKey-6-a",
225+
"forcePathStyle": "true",
224226
},
225227
},
226228
testDataCase: "case6",
@@ -263,10 +265,11 @@ func TestGetBucketSessionInfo(t *testing.T) {
263265
expectedSessionInfo: objectstore.SessionInfo{
264266
Provider: "minio",
265267
Params: map[string]string{
266-
"region": "minio-a",
267-
"endpoint": "minio-endpoint-9.com",
268-
"disableSSL": "true",
269-
"fromEnv": "true",
268+
"region": "minio-a",
269+
"endpoint": "minio-endpoint-9.com",
270+
"disableSSL": "true",
271+
"fromEnv": "true",
272+
"forcePathStyle": "true",
270273
},
271274
},
272275
testDataCase: "case9",
@@ -277,13 +280,14 @@ func TestGetBucketSessionInfo(t *testing.T) {
277280
expectedSessionInfo: objectstore.SessionInfo{
278281
Provider: "minio",
279282
Params: map[string]string{
280-
"region": "minio-a",
281-
"endpoint": "minio-endpoint-10.com",
282-
"disableSSL": "true",
283-
"fromEnv": "false",
284-
"secretName": "minio-test-secret-10",
285-
"accessKeyKey": "minio-test-accessKeyKey-10",
286-
"secretKeyKey": "minio-test-secretKeyKey-10",
283+
"region": "minio-a",
284+
"endpoint": "minio-endpoint-10.com",
285+
"disableSSL": "true",
286+
"fromEnv": "false",
287+
"secretName": "minio-test-secret-10",
288+
"accessKeyKey": "minio-test-accessKeyKey-10",
289+
"secretKeyKey": "minio-test-secretKeyKey-10",
290+
"forcePathStyle": "true",
287291
},
288292
},
289293
testDataCase: "case10",
@@ -294,10 +298,11 @@ func TestGetBucketSessionInfo(t *testing.T) {
294298
expectedSessionInfo: objectstore.SessionInfo{
295299
Provider: "minio",
296300
Params: map[string]string{
297-
"region": "minio",
298-
"endpoint": "minio-endpoint-10.com",
299-
"disableSSL": "true",
300-
"fromEnv": "true",
301+
"region": "minio",
302+
"endpoint": "minio-endpoint-10.com",
303+
"disableSSL": "true",
304+
"fromEnv": "true",
305+
"forcePathStyle": "true",
301306
},
302307
},
303308
testDataCase: "case10",
@@ -308,13 +313,14 @@ func TestGetBucketSessionInfo(t *testing.T) {
308313
expectedSessionInfo: objectstore.SessionInfo{
309314
Provider: "s3",
310315
Params: map[string]string{
311-
"region": "us-east-1",
312-
"endpoint": "s3.amazonaws.com",
313-
"disableSSL": "false",
314-
"fromEnv": "false",
315-
"secretName": "s3-testsecret-6",
316-
"accessKeyKey": "s3-testaccessKeyKey-6",
317-
"secretKeyKey": "s3-testsecretKeyKey-6",
316+
"region": "us-east-1",
317+
"endpoint": "s3.amazonaws.com",
318+
"disableSSL": "false",
319+
"fromEnv": "false",
320+
"secretName": "s3-testsecret-6",
321+
"accessKeyKey": "s3-testaccessKeyKey-6",
322+
"secretKeyKey": "s3-testsecretKeyKey-6",
323+
"forcePathStyle": "false",
318324
},
319325
},
320326
testDataCase: "case6",
@@ -325,13 +331,14 @@ func TestGetBucketSessionInfo(t *testing.T) {
325331
expectedSessionInfo: objectstore.SessionInfo{
326332
Provider: "s3",
327333
Params: map[string]string{
328-
"region": "us-east-2",
329-
"endpoint": "s3.us-east-2.amazonaws.com",
330-
"disableSSL": "false",
331-
"fromEnv": "false",
332-
"secretName": "s3-test-secret-6-b",
333-
"accessKeyKey": "s3-test-accessKeyKey-6-b",
334-
"secretKeyKey": "s3-test-secretKeyKey-6-b",
334+
"region": "us-east-2",
335+
"endpoint": "s3.us-east-2.amazonaws.com",
336+
"disableSSL": "false",
337+
"fromEnv": "false",
338+
"secretName": "s3-test-secret-6-b",
339+
"accessKeyKey": "s3-test-accessKeyKey-6-b",
340+
"secretKeyKey": "s3-test-secretKeyKey-6-b",
341+
"forcePathStyle": "false",
335342
},
336343
},
337344
testDataCase: "case6",
@@ -342,13 +349,14 @@ func TestGetBucketSessionInfo(t *testing.T) {
342349
expectedSessionInfo: objectstore.SessionInfo{
343350
Provider: "s3",
344351
Params: map[string]string{
345-
"region": "us-east-1",
346-
"endpoint": "s3.amazonaws.com",
347-
"disableSSL": "false",
348-
"fromEnv": "false",
349-
"secretName": "s3-test-secret-6-a",
350-
"accessKeyKey": "s3-test-accessKeyKey-6-a",
351-
"secretKeyKey": "s3-test-secretKeyKey-6-a",
352+
"region": "us-east-1",
353+
"endpoint": "s3.amazonaws.com",
354+
"disableSSL": "false",
355+
"fromEnv": "false",
356+
"secretName": "s3-test-secret-6-a",
357+
"accessKeyKey": "s3-test-accessKeyKey-6-a",
358+
"secretKeyKey": "s3-test-secretKeyKey-6-a",
359+
"forcePathStyle": "false",
352360
},
353361
},
354362
testDataCase: "case6",

backend/src/v2/config/s3.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ type S3ProviderDefault struct {
3434
Region string `json:"region"`
3535
// optional
3636
DisableSSL *bool `json:"disableSSL"`
37+
// optional
38+
ForcePathStyle *bool `json:"forcePathStyle"`
3739
}
3840

3941
type S3Credentials struct {
@@ -52,6 +54,8 @@ type S3Override struct {
5254
KeyPrefix string `json:"keyPrefix"`
5355
// required
5456
Credentials *S3Credentials `json:"credentials"`
57+
// optional
58+
ForcePathStyle *bool `json:"forcePathStyle"`
5559
}
5660
type S3SecretRef struct {
5761
SecretName string `json:"secretName"`
@@ -99,6 +103,12 @@ func (p S3ProviderConfig) ProvideSessionInfo(path string) (objectstore.SessionIn
99103
params["disableSSL"] = strconv.FormatBool(*p.Default.DisableSSL)
100104
}
101105

106+
if p.Default.ForcePathStyle == nil {
107+
params["forcePathStyle"] = strconv.FormatBool(true)
108+
} else {
109+
params["forcePathStyle"] = strconv.FormatBool(*p.Default.ForcePathStyle)
110+
}
111+
102112
params["fromEnv"] = strconv.FormatBool(p.Default.Credentials.FromEnv)
103113
if !p.Default.Credentials.FromEnv {
104114
params["secretName"] = p.Default.Credentials.SecretRef.SecretName
@@ -124,6 +134,9 @@ func (p S3ProviderConfig) ProvideSessionInfo(path string) (objectstore.SessionIn
124134
if override.DisableSSL != nil {
125135
sessionInfo.Params["disableSSL"] = strconv.FormatBool(*override.DisableSSL)
126136
}
137+
if override.ForcePathStyle != nil {
138+
sessionInfo.Params["forcePathStyle"] = strconv.FormatBool(*override.ForcePathStyle)
139+
}
127140
if override.Credentials == nil {
128141
return objectstore.SessionInfo{}, invalidConfigErr(fmt.Errorf("missing override credentials"))
129142
}

backend/src/v2/config/testdata/provider_cases.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ cases:
9595
endpoint: s3.amazonaws.com
9696
region: us-east-1
9797
disableSSL: false
98+
forcePathStyle: false
9899
credentials:
99100
fromEnv: false
100101
secretRef:
@@ -123,6 +124,7 @@ cases:
123124
endpoint: s3.us-east-2.amazonaws.com
124125
region: us-east-2
125126
disableSSL: false
127+
forcePathStyle: false
126128
credentials:
127129
fromEnv: false
128130
secretRef:

backend/src/v2/objectstore/config.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,12 @@ type S3Params struct {
5353
FromEnv bool
5454
SecretName string
5555
// The k8s secret "Key" for "Artifact SecretKey" and "Artifact AccessKey"
56-
AccessKeyKey string
57-
SecretKeyKey string
58-
Region string
59-
Endpoint string
60-
DisableSSL bool
56+
AccessKeyKey string
57+
SecretKeyKey string
58+
Region string
59+
Endpoint string
60+
DisableSSL bool
61+
ForcePathStyle bool
6162
}
6263

6364
func (b *Config) bucketURL() string {
@@ -221,6 +222,13 @@ func StructuredS3Params(p map[string]string) (*S3Params, error) {
221222
}
222223
sparams.DisableSSL = boolVal
223224
}
225+
if val, ok := p["forcePathStyle"]; ok {
226+
boolVal, err := strconv.ParseBool(val)
227+
if err != nil {
228+
return nil, err
229+
}
230+
sparams.ForcePathStyle = boolVal
231+
}
224232
return sparams, nil
225233
}
226234

backend/src/v2/objectstore/object_store.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ func createS3BucketSession(ctx context.Context, namespace string, sessionInfo *S
258258
config.Credentials = creds
259259
config.Region = aws.String(params.Region)
260260
config.DisableSSL = aws.Bool(params.DisableSSL)
261-
config.S3ForcePathStyle = aws.Bool(true)
261+
config.S3ForcePathStyle = aws.Bool(params.ForcePathStyle)
262262

263263
// AWS Specific:
264264
// Path-style S3 endpoints, which are commonly used, may fall into either of two subdomains:

backend/src/v2/objectstore/object_store_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -273,13 +273,14 @@ func Test_createS3BucketSession(t *testing.T) {
273273
sessionInfo: &SessionInfo{
274274
Provider: "s3",
275275
Params: map[string]string{
276-
"region": "us-east-1",
277-
"endpoint": "s3.amazonaws.com",
278-
"disableSSL": "false",
279-
"fromEnv": "false",
280-
"secretName": "s3-provider-secret",
281-
"accessKeyKey": "test_access_key",
282-
"secretKeyKey": "test_secret_key",
276+
"region": "us-east-1",
277+
"endpoint": "s3.amazonaws.com",
278+
"disableSSL": "false",
279+
"fromEnv": "false",
280+
"secretName": "s3-provider-secret",
281+
"accessKeyKey": "test_access_key",
282+
"secretKeyKey": "test_secret_key",
283+
"forcePathStyle": "true",
283284
},
284285
},
285286
sessionSecret: &corev1.Secret{

0 commit comments

Comments
 (0)