Merge pull request vmware-tanzu#5512 from Lyndon-Li/issue-fix-5505

Issue fix 5505
kaovilai · Nov 1, 2022 · 5db3da5 · 5db3da5
2 parents ae1e42c + 3571339
commit 5db3da5
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 22 deletions.
diff --git a/changelogs/unreleased/5512-lyndon b/changelogs/unreleased/5512-lyndon
@@ -0,0 +1 @@
+Fix issue 5505: the pod volume backups/restores except the first one fail under the kopia path if "AZURE_CLOUD_NAME" is specified
diff --git a/pkg/repository/config/azure.go b/pkg/repository/config/azure.go
@@ -18,6 +18,7 @@ package config
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"strings"
 
@@ -211,13 +212,15 @@ func getRequiredValues(getValue func(string) string, keys ...string) error {
 }
 
 // GetAzureStorageDomain gets the Azure storage domain required by a Azure blob connection,
-// if the provided config doean't have the value, get it from system's environment variables
-func GetAzureStorageDomain(config map[string]string) string {
-	if domain, exist := config[storageDomainConfigKey]; exist {
-		return domain
-	} else {
-		return os.Getenv(cloudNameEnvVar)
+// if the provided credential file doesn't have the value, get it from system's environment variables
+func GetAzureStorageDomain(config map[string]string) (string, error) {
+	credentialsFile := selectCredentialsFile(config)
+
+	if err := loadCredentialsIntoEnv(credentialsFile); err != nil {
+		return "", err
 	}
+
+	return getStorageDomainFromCloudName(os.Getenv(cloudNameEnvVar))
 }
 
 func GetAzureCredentials(config map[string]string) (string, string, error) {
@@ -228,3 +231,12 @@ func GetAzureCredentials(config map[string]string) (string, string, error) {
 
 	return config[storageAccountConfigKey], storageAccountKey, nil
 }
+
+func getStorageDomainFromCloudName(cloudName string) (string, error) {
+	env, err := parseAzureEnvironment(cloudName)
+	if err != nil {
+		return "", errors.Wrapf(err, "unable to parse azure env from cloud name %s", cloudName)
+	}
+
+	return fmt.Sprintf("blob.%s", env.StorageEndpointSuffix), nil
+}
diff --git a/pkg/repository/config/azure_test.go b/pkg/repository/config/azure_test.go
@@ -20,6 +20,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -86,3 +87,48 @@ func TestSelectCredentialsFile(t *testing.T) {
 		})
 	}
 }
+
+func TestGetStorageDomainFromCloudName(t *testing.T) {
+	testCases := []struct {
+		name        string
+		cloudName   string
+		expected    string
+		expectedErr string
+	}{
+		{
+			name:        "get azure env fail",
+			cloudName:   "fake-cloud",
+			expectedErr: "unable to parse azure env from cloud name fake-cloud: autorest/azure: There is no cloud environment matching the name \"FAKE-CLOUD\"",
+		},
+		{
+			name:      "cloud name is empty",
+			cloudName: "",
+			expected:  "blob.core.windows.net",
+		},
+		{
+			name:      "azure public cloud",
+			cloudName: "AzurePublicCloud",
+			expected:  "blob.core.windows.net",
+		},
+		{
+
+			name:      "azure China cloud",
+			cloudName: "AzureChinaCloud",
+			expected:  "blob.core.chinacloudapi.cn",
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			domain, err := getStorageDomainFromCloudName(tc.cloudName)
+
+			require.Equal(t, tc.expected, domain)
+
+			if tc.expectedErr == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.EqualError(t, err, tc.expectedErr)
+				assert.Empty(t, domain)
+			}
+		})
+	}
+}
diff --git a/pkg/repository/provider/unified_repo.go b/pkg/repository/provider/unified_repo.go
@@ -466,7 +466,12 @@ func getStorageVariables(backupLocation *velerov1api.BackupStorageLocation, repo
 		result[udmrepo.StoreOptionS3DisableTlsVerify] = config["insecureSkipTLSVerify"]
 		result[udmrepo.StoreOptionS3DisableTls] = strconv.FormatBool(disableTls)
 	} else if backendType == repoconfig.AzureBackend {
-		result[udmrepo.StoreOptionAzureDomain] = getAzureStorageDomain(config)
+		domain, err := getAzureStorageDomain(config)
+		if err != nil {
+			return map[string]string{}, errors.Wrapf(err, "error to get azure storage domain")
+		}
+
+		result[udmrepo.StoreOptionAzureDomain] = domain
 	}
 
 	result[udmrepo.StoreOptionOssBucket] = bucket

diff --git a/pkg/repository/provider/unified_repo_test.go b/pkg/repository/provider/unified_repo_test.go
@@ -235,7 +235,7 @@ func TestGetStorageVariables(t *testing.T) {
 		repoName              string
 		repoBackend           string
 		getS3BucketRegion     func(string) (string, error)
-		getAzureStorageDomain func(map[string]string) string
+		getAzureStorageDomain func(map[string]string) (string, error)
 		expected              map[string]string
 		expectedErr           string
 	}{
@@ -366,17 +366,42 @@ func TestGetStorageVariables(t *testing.T) {
 				"skipTLSVerify": "false",
 			},
 		},
+		{
+			name: "azure, getAzureStorageDomain fail",
+			backupLocation: velerov1api.BackupStorageLocation{
+				Spec: velerov1api.BackupStorageLocationSpec{
+					Provider: "velero.io/azure",
+					Config: map[string]string{
+						"bucket": "fake-bucket-config",
+						"prefix": "fake-prefix-config",
+						"region": "fake-region",
+						"fspath": "",
+					},
+					StorageType: velerov1api.StorageType{
+						ObjectStorage: &velerov1api.ObjectStorageLocation{
+							Bucket: "fake-bucket-object-store",
+							Prefix: "fake-prefix-object-store",
+						},
+					},
+				},
+			},
+			getAzureStorageDomain: func(config map[string]string) (string, error) {
+				return "", errors.New("fake error")
+			},
+			repoBackend: "fake-repo-type",
+			expected:    map[string]string{},
+			expectedErr: "error to get azure storage domain: fake error",
+		},
 		{
 			name: "azure, ObjectStorage section exists in BSL",
 			backupLocation: velerov1api.BackupStorageLocation{
 				Spec: velerov1api.BackupStorageLocationSpec{
 					Provider: "velero.io/azure",
 					Config: map[string]string{
-						"bucket":        "fake-bucket-config",
-						"prefix":        "fake-prefix-config",
-						"region":        "fake-region",
-						"fspath":        "",
-						"storageDomain": "fake-domain",
+						"bucket": "fake-bucket-config",
+						"prefix": "fake-prefix-config",
+						"region": "fake-region",
+						"fspath": "",
 					},
 					StorageType: velerov1api.StorageType{
 						ObjectStorage: &velerov1api.ObjectStorageLocation{
@@ -386,8 +411,8 @@ func TestGetStorageVariables(t *testing.T) {
 					},
 				},
 			},
-			getAzureStorageDomain: func(config map[string]string) string {
-				return config["storageDomain"]
+			getAzureStorageDomain: func(config map[string]string) (string, error) {
+				return "fake-domain", nil
 			},
 			repoBackend: "fake-repo-type",
 			expected: map[string]string{
@@ -404,18 +429,17 @@ func TestGetStorageVariables(t *testing.T) {
 				Spec: velerov1api.BackupStorageLocationSpec{
 					Provider: "velero.io/azure",
 					Config: map[string]string{
-						"bucket":        "fake-bucket",
-						"prefix":        "fake-prefix",
-						"region":        "fake-region",
-						"fspath":        "",
-						"storageDomain": "fake-domain",
+						"bucket": "fake-bucket",
+						"prefix": "fake-prefix",
+						"region": "fake-region",
+						"fspath": "",
 					},
 				},
 			},
 			repoName:    "//fake-name//",
 			repoBackend: "fake-repo-type",
-			getAzureStorageDomain: func(config map[string]string) string {
-				return config["storageDomain"]
+			getAzureStorageDomain: func(config map[string]string) (string, error) {
+				return "fake-domain", nil
 			},
 			expected: map[string]string{
 				"bucket":        "fake-bucket",