add UT

Signed-off-by: Lyndon-Li <[email protected]>

pkg/repository/repoconfig/aws.go

@@ -17,10 +17,14 @@ limitations under the License.
 package repoconfig
 
 import (
-	"errors"
+	"context"
 	"os"
 
 	"github.com/aws/aws-sdk-go/aws/credentials"
+	"github.com/aws/aws-sdk-go/aws/endpoints"
+	"github.com/aws/aws-sdk-go/aws/session"
+	"github.com/aws/aws-sdk-go/service/s3/s3manager"
+	"github.com/pkg/errors"
 )
 
 const (
@@ -67,3 +71,29 @@ func GetS3Credentials(config map[string]string) (credentials.Value, error) {
 
 	return credValue, nil
 }
+
+// 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) {
+	var region string
+
+	sess, err := session.NewSession()
+	if err != nil {
+		return "", errors.WithStack(err)
+	}
+
+	for _, partition := range endpoints.DefaultPartitions() {
+		for regionHint := range partition.Regions() {
+			region, _ = s3manager.GetBucketRegion(context.Background(), sess, bucket, regionHint)
+
+			// we only need to try a single region hint per partition, so break after the first
+			break
+		}
+
+		if region != "" {
+			return region, nil
+		}
+	}
+
+	return "", errors.New("unable to determine bucket's region")
+}

pkg/repository/repoconfig/config.go

@@ -17,14 +17,10 @@ limitations under the License.
 package repoconfig
 
 import (
-	"context"
 	"fmt"
 	"path"
 	"strings"
 
-	"github.com/aws/aws-sdk-go/aws/endpoints"
-	"github.com/aws/aws-sdk-go/aws/session"
-	"github.com/aws/aws-sdk-go/service/s3/s3manager"
 	"github.com/pkg/errors"
 
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
@@ -78,7 +74,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)
 			}
 			if err != nil {
 				return "", errors.Wrapf(err, "failed to detect the region via bucket: %s", bucket)
@@ -104,6 +100,10 @@ func GetBackendType(provider string) BackendType {
 	return BackendType(provider)
 }
 
+func IsBackendTypeValid(backendType BackendType) bool {
+	return (backendType == AWSBackend || backendType == AzureBackend || backendType == GCPBackend || backendType == FSBackend)
+}
+
 // GetRepoIdentifier returns the string to be used as the value of the --repo flag in
 // restic commands for the given repository.
 func GetRepoIdentifier(location *velerov1api.BackupStorageLocation, name string) (string, error) {
@@ -114,29 +114,3 @@ func GetRepoIdentifier(location *velerov1api.BackupStorageLocation, name string)
 
 	return fmt.Sprintf("%s/%s", strings.TrimSuffix(prefix, "/"), name), nil
 }
-
-// GetBucketRegion returns the AWS region that a bucket is in, or an error
-// if the region cannot be determined.
-func GetAWSBucketRegion(bucket string) (string, error) {
-	var region string
-
-	sess, err := session.NewSession()
-	if err != nil {
-		return "", errors.WithStack(err)
-	}
-
-	for _, partition := range endpoints.DefaultPartitions() {
-		for regionHint := range partition.Regions() {
-			region, _ = s3manager.GetBucketRegion(context.Background(), sess, bucket, regionHint)
-
-			// we only need to try a single region hint per partition, so break after the first
-			break
-		}
-
-		if region != "" {
-			return region, nil
-		}
-	}
-
-	return "", errors.New("unable to determine bucket's region")
-}

pkg/repository/repoprovider/unified_repo.go

@@ -6,6 +6,7 @@ import (
 	"path"
 	"strings"
 
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/vmware-tanzu/velero/internal/credentials"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
@@ -25,6 +26,14 @@ type unifiedRepoProvider struct {
 	log                  *logrus.Logger
 }
 
+// this func is assigned to a package-level variable so it can be
+// replaced when unit-testing
+var getAzureCredentials = repoconfig.GetAzureCredentials
+var getS3Credentials = repoconfig.GetS3Credentials
+var getGCPCredentials = repoconfig.GetGCPCredentials
+var getS3BucketRegion = repoconfig.GetAWSBucketRegion
+var getAzureStorageDomain = repoconfig.GetAzureStorageDomain
+
 func NewUnifiedRepoProvider(
 	ctx context.Context,
 	credentialFileStore credentials.FileStore,
@@ -139,7 +148,7 @@ func (urp *unifiedRepoProvider) getRepoOption() (udmrepo.RepoOptions, error) {
 		repoOption.StorageOptions[k] = v
 	}
 
-	storeCred, err := getStorageCredentials(urp.backupLocation, urp.credentialsFileStore, urp.log)
+	storeCred, err := getStorageCredentials(urp.backupLocation, urp.credentialsFileStore)
 	if err != nil {
 		log.WithError(err).Error("Failed to get repo credential env")
 		return repoOption, err
@@ -169,10 +178,14 @@ func getStorageType(backupLocation *velerov1api.BackupStorageLocation) string {
 	}
 }
 
-func getStorageCredentials(backupLocation *velerov1api.BackupStorageLocation, credentialsFileStore credentials.FileStore, log *logrus.Logger) (map[string]string, error) {
+func getStorageCredentials(backupLocation *velerov1api.BackupStorageLocation, credentialsFileStore credentials.FileStore) (map[string]string, error) {
 	result := make(map[string]string)
 	var err error
+
 	backendType := repoconfig.GetBackendType(backupLocation.Spec.Provider)
+	if !repoconfig.IsBackendTypeValid(backendType) {
+		return map[string]string{}, errors.New("invalid storage provider")
+	}
 
 	config := backupLocation.Spec.Config
 	if config == nil {
@@ -182,42 +195,43 @@ func getStorageCredentials(backupLocation *velerov1api.BackupStorageLocation, cr
 	if backupLocation.Spec.Credential != nil {
 		config[repoconfig.CredentialsFileKey], err = credentialsFileStore.Path(backupLocation.Spec.Credential)
 		if err != nil {
-			log.WithError(err).Error("Failed to get credential file in BSL")
-			return map[string]string{}, err
+			return map[string]string{}, errors.Wrap(err, "error get credential file in bsl")
 		}
 	}
 
 	switch backendType {
 	case repoconfig.AWSBackend:
-		credValue, err := repoconfig.GetS3Credentials(config)
+		credValue, err := getS3Credentials(config)
 		if err != nil {
-			log.WithError(err).Error("Failed to get S3 credentials")
-			return map[string]string{}, err
+			return map[string]string{}, errors.Wrap(err, "error get s3 credentials")
 		}
 		result[udmrepo.UNIFIED_REPO_STORE_OPTION_S3_KEY_ID] = credValue.AccessKeyID
 		result[udmrepo.UNIFIED_REPO_STORE_OPTION_S3_PROVIDER] = credValue.ProviderName
 		result[udmrepo.UNIFIED_REPO_STORE_OPTION_S3_SECRET_KEY] = credValue.SecretAccessKey
 		result[udmrepo.UNIFIED_REPO_STORE_OPTION_S3_TOKEN] = credValue.SessionToken
 
 	case repoconfig.AzureBackend:
-		storageAccount, accountKey, err := repoconfig.GetAzureCredentials(config)
+		storageAccount, accountKey, err := getAzureCredentials(config)
 		if err != nil {
-			log.WithError(err).Error("Failed to get Azure credentials")
-			return map[string]string{}, err
+			return map[string]string{}, errors.Wrap(err, "error get azure credentials")
 		}
 		result[udmrepo.UNIFIED_REPO_STORE_OPTION_AZ_STORAGE_ACCOUNT] = storageAccount
 		result[udmrepo.UNIFIED_REPO_STORE_OPTION_AZ_KEY] = accountKey
 
 	case repoconfig.GCPBackend:
-		result[udmrepo.UNIFIED_REPO_STORE_OPTION_CRED_FILE] = repoconfig.GetGCPCredentials(config)
+		result[udmrepo.UNIFIED_REPO_STORE_OPTION_CRED_FILE] = getGCPCredentials(config)
 	}
 
 	return result, nil
 }
 
 func getStorageVariables(backupLocation *velerov1api.BackupStorageLocation, repoName string) (map[string]string, error) {
 	result := make(map[string]string)
+
 	backendType := repoconfig.GetBackendType(backupLocation.Spec.Provider)
+	if !repoconfig.IsBackendTypeValid(backendType) {
+		return map[string]string{}, errors.New("invalid storage provider")
+	}
 
 	config := backupLocation.Spec.Config
 	if config == nil {
@@ -233,30 +247,30 @@ func getStorageVariables(backupLocation *velerov1api.BackupStorageLocation, repo
 
 	prefix = path.Join(prefix, udmrepo.UNIFIED_REPO_STORE_OPTION_PREFIX_NAME, repoName) + "/"
 
-	s3Url := config["s3Url"]
 	region := config["region"]
-	var err error
 
 	if backendType == repoconfig.AWSBackend {
-		if s3Url == "" && region == "" {
-			region, err = repoconfig.GetAWSBucketRegion(bucket)
+		s3Url := config["s3Url"]
+
+		var err error
+		if s3Url == "" {
+			region, err = getS3BucketRegion(bucket)
 			if err != nil {
-				return map[string]string{}, err
+				return map[string]string{}, errors.Wrap(err, "error get s3 bucket region")
 			}
 
 			s3Url = fmt.Sprintf("s3-%s.amazonaws.com", region)
 		}
+
+		result[udmrepo.UNIFIED_REPO_STORE_OPTION_S3_ENDPOINT] = strings.Trim(s3Url, "/")
+		result[udmrepo.UNIFIED_REPO_STORE_OPTION_S3_DISABLE_TLS_VERIFY] = config["insecureSkipTLSVerify"]
+	} else if backendType == repoconfig.AzureBackend {
+		result[udmrepo.UNIFIED_REPO_STORE_OPTION_AZ_DOMAIN] = getAzureStorageDomain(config)
 	}
 
 	result[udmrepo.UNIFIED_REPO_STORE_OPTION_OSS_BUCKET] = bucket
 	result[udmrepo.UNIFIED_REPO_STORE_OPTION_PREFIX] = prefix
 	result[udmrepo.UNIFIED_REPO_STORE_OPTION_OSS_REGION] = strings.Trim(region, "/")
-
-	result[udmrepo.UNIFIED_REPO_STORE_OPTION_S3_ENDPOINT] = strings.Trim(s3Url, "/")
-	result[udmrepo.UNIFIED_REPO_STORE_OPTION_S3_DISABLE_TLS_VERIFY] = config["insecureSkipTLSVerify"]
-
-	result[udmrepo.UNIFIED_REPO_STORE_OPTION_AZ_DOMAIN] = repoconfig.GetAzureStorageDomain(config)
-
 	result[udmrepo.UNIFIED_REPO_STORE_OPTION_FS_PATH] = config["fspath"]
 
 	return result, nil