Normalize repo suffix paths (#1153)

## Description

Repo `--prefix` values should be normalized with a trailing `/`

## Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🐹 Trivial/Minor

## Issue(s)

* Fixes #1152  

## Test Plan

<!-- How will this be tested prior to merging.-->
- [] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
Georgi Matev 2022-10-12 19:11:13 -07:00 committed by GitHub
parent 523386a720
commit 0c921e8b40
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 38 additions and 8 deletions

View File

@ -22,7 +22,7 @@ const (
configFileTemplate = ` configFileTemplate = `
` + BucketNameKey + ` = '%s' ` + BucketNameKey + ` = '%s'
` + EndpointKey + ` = 's3.amazonaws.com' ` + EndpointKey + ` = 's3.amazonaws.com'
` + PrefixKey + ` = 'test-prefix' ` + PrefixKey + ` = 'test-prefix/'
` + StorageProviderTypeKey + ` = 'S3' ` + StorageProviderTypeKey + ` = 'S3'
` + AccountProviderTypeKey + ` = 'M365' ` + AccountProviderTypeKey + ` = 'M365'
` + TenantIDKey + ` = '%s' ` + TenantIDKey + ` = '%s'
@ -208,7 +208,7 @@ func (suite *ConfigIntegrationSuite) TestGetStorageAndAccount() {
const ( const (
bkt = "get-storage-and-account-bucket" bkt = "get-storage-and-account-bucket"
end = "https://get-storage-and-account.com" end = "https://get-storage-and-account.com"
pfx = "get-storage-and-account-prefix" pfx = "get-storage-and-account-prefix/"
tid = "3a2faa4e-a882-445c-9d27-f552ef189381" tid = "3a2faa4e-a882-445c-9d27-f552ef189381"
) )
@ -253,7 +253,7 @@ func (suite *ConfigIntegrationSuite) TestGetStorageAndAccount_noFileOnlyOverride
const ( const (
bkt = "get-storage-and-account-no-file-bucket" bkt = "get-storage-and-account-no-file-bucket"
end = "https://get-storage-and-account.com/no-file" end = "https://get-storage-and-account.com/no-file"
pfx = "get-storage-and-account-no-file-prefix" pfx = "get-storage-and-account-no-file-prefix/"
tid = "88f8522b-18e4-4d0f-b514-2d7b34d4c5a1" tid = "88f8522b-18e4-4d0f-b514-2d7b34d4c5a1"
) )

View File

@ -61,6 +61,10 @@ func configureStorage(
overrides[storage.Bucket] = common.NormalizeBucket(b) overrides[storage.Bucket] = common.NormalizeBucket(b)
} }
if p, ok := overrides[storage.Prefix]; ok {
overrides[storage.Prefix] = common.NormalizePrefix(p)
}
if err := mustMatchConfig(vpr, s3Overrides(overrides)); err != nil { if err := mustMatchConfig(vpr, s3Overrides(overrides)); err != nil {
return store, errors.Wrap(err, "verifying s3 configs in corso config file") return store, errors.Wrap(err, "verifying s3 configs in corso config file")
} }

View File

@ -11,3 +11,15 @@ import "strings"
func NormalizeBucket(b string) string { func NormalizeBucket(b string) string {
return strings.TrimPrefix(b, "s3://") return strings.TrimPrefix(b, "s3://")
} }
// NormalizePrefix ensures that a bucket prefix is always treated as
// object store folder prefix.
func NormalizePrefix(p string) string {
tp := strings.TrimRight(p, "/")
if len(tp) > 0 {
tp = tp + "/"
}
return tp
}

View File

@ -17,7 +17,7 @@ func TestCommonBucketsSuite(t *testing.T) {
suite.Run(t, new(CommonBucketsSuite)) suite.Run(t, new(CommonBucketsSuite))
} }
func (suite *CommonBucketsSuite) TestDoesThings() { func (suite *CommonBucketsSuite) TestBucketPrefix() {
t := suite.T() t := suite.T()
trimmablePrefixes := []string{"s3://"} trimmablePrefixes := []string{"s3://"}
@ -26,3 +26,17 @@ func (suite *CommonBucketsSuite) TestDoesThings() {
assert.Equal(t, "smarf", "smarf") assert.Equal(t, "smarf", "smarf")
} }
} }
func (suite *CommonBucketsSuite) TestPrefixSuffix() {
t := suite.T()
prefixBase := "repo-prefix"
properPrefix := prefixBase + "/"
assert.Equal(t, properPrefix, common.NormalizePrefix(prefixBase), "Trailing '/' should be added")
assert.Equal(t, properPrefix, common.NormalizePrefix(properPrefix), "Properly formatted prefix should not change")
assert.Equal(t, properPrefix, common.NormalizePrefix(prefixBase+"///"), "Only one trailing / should exist")
assert.Equal(t, properPrefix+"/sub/", common.NormalizePrefix(properPrefix+"/sub"), "Only affect trailing /")
assert.Equal(t, "", common.NormalizePrefix(""), "Only normalize actual prefix.")
assert.Equal(t, "", common.NormalizePrefix("//"), "Only normalize actual prefix.")
}

View File

@ -30,7 +30,7 @@ func (c S3Config) Normalize() S3Config {
return S3Config{ return S3Config{
Bucket: common.NormalizeBucket(c.Bucket), Bucket: common.NormalizeBucket(c.Bucket),
Endpoint: c.Endpoint, Endpoint: c.Endpoint,
Prefix: c.Prefix, Prefix: common.NormalizePrefix(c.Prefix),
} }
} }

View File

@ -22,13 +22,13 @@ var (
goodS3Config = storage.S3Config{ goodS3Config = storage.S3Config{
Bucket: "bkt", Bucket: "bkt",
Endpoint: "end", Endpoint: "end",
Prefix: "pre", Prefix: "pre/",
} }
goodS3Map = map[string]string{ goodS3Map = map[string]string{
"s3_bucket": "bkt", "s3_bucket": "bkt",
"s3_endpoint": "end", "s3_endpoint": "end",
"s3_prefix": "pre", "s3_prefix": "pre/",
} }
) )
@ -78,7 +78,7 @@ func (suite *S3CfgSuite) TestStorage_S3Config_invalidCases() {
name string name string
cfg storage.S3Config cfg storage.S3Config
}{ }{
{"missing bucket", makeTestS3Cfg("", "end", "pre")}, {"missing bucket", makeTestS3Cfg("", "end", "pre/")},
} }
for _, test := range table { for _, test := range table {
suite.T().Run(test.name, func(t *testing.T) { suite.T().Run(test.name, func(t *testing.T) {