Set hinting region to use for GetBucketRegion() in pkg/repository/con…

…fig/aws.go Signed-off-by: Tiger Kaovilai <[email protected]>
vmware-tanzu · Oct 14, 2024 · 69b456a · 69b456a
1 parent f02613d
commit 69b456a
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 17 deletions.
diff --git a/changelogs/unreleased/8297-kaovilai b/changelogs/unreleased/8297-kaovilai
@@ -0,0 +1 @@
+Set hinting region to use for GetBucketRegion() in pkg/repository/config/aws.go
diff --git a/pkg/repository/config/aws.go b/pkg/repository/config/aws.go
@@ -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)
 	}

diff --git a/pkg/repository/config/config.go b/pkg/repository/config/config.go
@@ -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)

diff --git a/pkg/repository/config/config_test.go b/pkg/repository/config/config_test.go
@@ -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
 	}{
@@ -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:    "",
@@ -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",
@@ -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",
@@ -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",
@@ -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",
@@ -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",

diff --git a/pkg/repository/provider/unified_repo.go b/pkg/repository/provider/unified_repo.go
@@ -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")
 				}

diff --git a/pkg/repository/provider/unified_repo_test.go b/pkg/repository/provider/unified_repo_test.go
@@ -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
 	}{
@@ -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",
@@ -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{},
@@ -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",
@@ -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",