Skip to content

Commit

Permalink
Set hinting region to use for GetBucketRegion() in pkg/repository/con…
Browse files Browse the repository at this point in the history
…fig/aws.go

Signed-off-by: Tiger Kaovilai <[email protected]>
  • Loading branch information
kaovilai authored and reasonerjt committed Dec 11, 2024
1 parent f499025 commit acb7856
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 17 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8297-kaovilai
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Set hinting region to use for GetBucketRegion() in pkg/repository/config/aws.go
17 changes: 14 additions & 3 deletions pkg/repository/config/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,24 @@ func GetS3Credentials(config map[string]string) (*aws.Credentials, error) {

// GetAWSBucketRegion returns the AWS region that a bucket is in, or an error
// if the region cannot be determined.
func GetAWSBucketRegion(bucket string) (string, error) {
cfg, err := awsconfig.LoadDefaultConfig(context.Background())
// It will use us-east-1 as hinting server and requires config param to use as credentials
func GetAWSBucketRegion(bucket string, config map[string]string) (string, error) {
cfg, err := awsconfig.LoadDefaultConfig(context.Background(), awsconfig.WithCredentialsProvider(
aws.CredentialsProviderFunc(
func(context.Context) (aws.Credentials, error) {
s3creds, err := GetS3Credentials(config)
if s3creds == nil {
return aws.Credentials{}, err
}
return *s3creds, err
},
),
))
if err != nil {
return "", errors.WithStack(err)
}
client := s3.NewFromConfig(cfg)
region, err := s3manager.GetBucketRegion(context.Background(), client, bucket)
region, err := s3manager.GetBucketRegion(context.Background(), client, bucket, func(o *s3.Options) { o.Region = "us-east-1" })
if err != nil {
return "", errors.WithStack(err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/repository/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func getRepoPrefix(location *velerov1api.BackupStorageLocation) (string, error)
var err error
region := location.Spec.Config["region"]
if region == "" {
region, err = getAWSBucketRegion(bucket)
region, err = getAWSBucketRegion(bucket, location.Spec.Config)
}
if err != nil {
return "", errors.Wrapf(err, "failed to detect the region via bucket: %s", bucket)
Expand Down
14 changes: 7 additions & 7 deletions pkg/repository/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestGetRepoIdentifier(t *testing.T) {
name string
bsl *velerov1api.BackupStorageLocation
repoName string
getAWSBucketRegion func(string) (string, error)
getAWSBucketRegion func(s string, config map[string]string) (string, error)
expected string
expectedErr string
}{
Expand Down Expand Up @@ -101,7 +101,7 @@ func TestGetRepoIdentifier(t *testing.T) {
},
},
repoName: "repo-1",
getAWSBucketRegion: func(string) (string, error) {
getAWSBucketRegion: func(s string, config map[string]string) (string, error) {
return "", errors.New("no region found")
},
expected: "",
Expand All @@ -120,7 +120,7 @@ func TestGetRepoIdentifier(t *testing.T) {
},
},
repoName: "repo-1",
getAWSBucketRegion: func(string) (string, error) {
getAWSBucketRegion: func(string, map[string]string) (string, error) {
return "eu-west-1", nil
},
expected: "s3:s3-eu-west-1.amazonaws.com/bucket/restic/repo-1",
Expand All @@ -139,7 +139,7 @@ func TestGetRepoIdentifier(t *testing.T) {
},
},
repoName: "repo-1",
getAWSBucketRegion: func(string) (string, error) {
getAWSBucketRegion: func(s string, config map[string]string) (string, error) {
return "eu-west-1", nil
},
expected: "s3:s3-eu-west-1.amazonaws.com/bucket/prefix/restic/repo-1",
Expand All @@ -161,7 +161,7 @@ func TestGetRepoIdentifier(t *testing.T) {
},
},
repoName: "repo-1",
getAWSBucketRegion: func(string) (string, error) {
getAWSBucketRegion: func(s string, config map[string]string) (string, error) {
return "eu-west-1", nil
},
expected: "s3:alternate-url/bucket/prefix/restic/repo-1",
Expand All @@ -183,7 +183,7 @@ func TestGetRepoIdentifier(t *testing.T) {
},
},
repoName: "aws-repo",
getAWSBucketRegion: func(string) (string, error) {
getAWSBucketRegion: func(s string, config map[string]string) (string, error) {
return "eu-west-1", nil
},
expected: "s3:s3-us-west-1.amazonaws.com/bucket/prefix/restic/aws-repo",
Expand All @@ -205,7 +205,7 @@ func TestGetRepoIdentifier(t *testing.T) {
},
},
repoName: "aws-repo",
getAWSBucketRegion: func(string) (string, error) {
getAWSBucketRegion: func(s string, config map[string]string) (string, error) {
return "eu-west-1", nil
},
expected: "s3:alternate-url-with-trailing-slash/bucket/prefix/restic/aws-repo",
Expand Down
2 changes: 1 addition & 1 deletion pkg/repository/provider/unified_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ func getStorageVariables(backupLocation *velerov1api.BackupStorageLocation, repo
var err error
if s3URL == "" {
if region == "" {
region, err = getS3BucketRegion(bucket)
region, err = getS3BucketRegion(bucket, config)
if err != nil {
return map[string]string{}, errors.Wrap(err, "error get s3 bucket region")
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/repository/provider/unified_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func TestGetStorageVariables(t *testing.T) {
repoName string
repoBackend string
repoConfig map[string]string
getS3BucketRegion func(string) (string, error)
getS3BucketRegion func(bucket string, config map[string]string) (string, error)
expected map[string]string
expectedErr string
}{
Expand Down Expand Up @@ -291,7 +291,7 @@ func TestGetStorageVariables(t *testing.T) {
},
},
},
getS3BucketRegion: func(bucket string) (string, error) {
getS3BucketRegion: func(bucket string, config map[string]string) (string, error) {
return "region from bucket: " + bucket, nil
},
repoBackend: "fake-repo-type",
Expand All @@ -313,7 +313,7 @@ func TestGetStorageVariables(t *testing.T) {
Config: map[string]string{},
},
},
getS3BucketRegion: func(bucket string) (string, error) {
getS3BucketRegion: func(bucket string, config map[string]string) (string, error) {
return "", errors.New("fake error")
},
expected: map[string]string{},
Expand All @@ -339,7 +339,7 @@ func TestGetStorageVariables(t *testing.T) {
},
},
},
getS3BucketRegion: func(bucket string) (string, error) {
getS3BucketRegion: func(bucket string, config map[string]string) (string, error) {
return "region from bucket: " + bucket, nil
},
repoBackend: "fake-repo-type",
Expand Down Expand Up @@ -374,7 +374,7 @@ func TestGetStorageVariables(t *testing.T) {
},
},
},
getS3BucketRegion: func(bucket string) (string, error) {
getS3BucketRegion: func(bucket string, config map[string]string) (string, error) {
return "region from bucket: " + bucket, nil
},
repoBackend: "fake-repo-type",
Expand Down

0 comments on commit acb7856

Please sign in to comment.