Merge pull request #302 from andyzhangx/arbitrary-volumeid

feat: support arbitrary volume id

Author: andyzhangx

pkg/blob/blob.go

@@ -259,16 +259,20 @@ func isSASToken(key string) bool {
 
 // GetAuthEnv return <accountName, containerName, authEnv, error>
 func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attrib, secrets map[string]string) (string, string, []string, error) {
+	rgName, accountName, containerName, err := GetContainerInfo(volumeID)
+	if err != nil {
+		// ignore volumeID parsing error
+		klog.Warningf("parsing volumeID(%s) return with error: %v", volumeID, err)
+		err = nil
+	}
+
 	var (
-		accountName           string
 		accountKey            string
 		accountSasToken       string
-		containerName         string
 		keyVaultURL           string
 		keyVaultSecretName    string
 		keyVaultSecretVersion string
 		authEnv               []string
-		err                   error
 	)
 
 	for k, v := range attrib {
@@ -306,6 +310,7 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
 	klog.V(2).Infof("volumeID(%s) authEnv: %s", volumeID, authEnv)
 
 	if protocol == nfs {
+		// nfs protocol does not need account key, return directly
 		return accountName, containerName, authEnv, err
 	}
 
@@ -324,23 +329,16 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
 		}
 	} else {
 		if len(secrets) == 0 {
-			var resourceGroupName string
-			resourceGroupName, accountName, containerName, err = GetContainerInfo(volumeID)
-			if err != nil {
-				return accountName, containerName, authEnv, err
-			}
-
-			if resourceGroupName == "" {
-				resourceGroupName = d.cloud.ResourceGroup
-			}
-
 			// read from k8s secret first
 			accountKey, err = d.GetStorageAccesskeyFromSecret(accountName, attrib[secretNamespaceField])
 			if err != nil {
 				klog.V(2).Infof("could not get account(%s) key from secret, error: %v, use cluster identity to get account key instead", accountName, err)
-				accountKey, err = d.cloud.GetStorageAccesskey(accountName, resourceGroupName)
+				if rgName == "" {
+					rgName = d.cloud.ResourceGroup
+				}
+				accountKey, err = d.cloud.GetStorageAccesskey(accountName, rgName)
 				if err != nil {
-					return accountName, containerName, authEnv, fmt.Errorf("no key for storage account(%s) under resource group(%s), err %v", accountName, resourceGroupName, err)
+					return accountName, containerName, authEnv, fmt.Errorf("no key for storage account(%s) under resource group(%s), err %v", accountName, rgName, err)
 				}
 			}
 		} else {
@@ -385,17 +383,14 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
 // only for e2e testing
 func (d *Driver) GetStorageAccountAndContainer(ctx context.Context, volumeID string, attrib, secrets map[string]string) (string, string, string, string, error) {
 	var (
-		accountName     string
-		accountKey      string
-		accountSasToken string
-
-		containerName string
-
+		accountName           string
+		accountKey            string
+		accountSasToken       string
+		containerName         string
 		keyVaultURL           string
 		keyVaultSecretName    string
 		keyVaultSecretVersion string
-
-		err error
+		err                   error
 	)
 
 	for k, v := range attrib {
@@ -430,19 +425,19 @@ func (d *Driver) GetStorageAccountAndContainer(ctx context.Context, volumeID str
 		}
 	} else {
 		if len(secrets) == 0 {
-			var resourceGroupName string
-			resourceGroupName, accountName, containerName, err = GetContainerInfo(volumeID)
+			var rgName string
+			rgName, accountName, containerName, err = GetContainerInfo(volumeID)
 			if err != nil {
 				return "", "", "", "", err
 			}
 
-			if resourceGroupName == "" {
-				resourceGroupName = d.cloud.ResourceGroup
+			if rgName == "" {
+				rgName = d.cloud.ResourceGroup
 			}
 
-			accountKey, err = d.cloud.GetStorageAccesskey(accountName, resourceGroupName)
+			accountKey, err = d.cloud.GetStorageAccesskey(accountName, rgName)
 			if err != nil {
-				return "", "", "", "", fmt.Errorf("no key for storage account(%s) under resource group(%s), err %v", accountName, resourceGroupName, err)
+				return "", "", "", "", fmt.Errorf("no key for storage account(%s) under resource group(%s), err %v", accountName, rgName, err)
 			}
 		}
 	}

pkg/blob/blob_test.go

@@ -453,7 +453,7 @@ func TestGetAuthEnv(t *testing.T) {
 				attrib["containername"] = "unit-test"
 				attrib["keyvaultsecretname"] = "unit-test"
 				attrib["keyvaultsecretversion"] = "unit-test"
-				attrib["storageaccountname"] = "unit-test"
+				attrib["storageaccountname"] = "storageaccountname"
 				attrib["azurestorageidentityclientid"] = "unit-test"
 				attrib["azurestorageidentityobjectid"] = "unit-test"
 				attrib["azurestorageidentityresourceid"] = "unit-test"
@@ -473,7 +473,7 @@ func TestGetAuthEnv(t *testing.T) {
 				}
 				mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any()).Return(accountListKeysResult, rerr).AnyTimes()
 				_, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
-				expectedErr := fmt.Errorf("no key for storage account(f5713de20cde511e8ba4900) under resource group(rg), err Retriable: false, RetryAfter: 0s, HTTPStatusCode: 0, RawError: test")
+				expectedErr := fmt.Errorf("no key for storage account(storageaccountname) under resource group(rg), err Retriable: false, RetryAfter: 0s, HTTPStatusCode: 0, RawError: test")
 				if !reflect.DeepEqual(err, expectedErr) {
 					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
 				}
@@ -514,19 +514,20 @@ func TestGetAuthEnv(t *testing.T) {
 				d := NewFakeDriver()
 				attrib := make(map[string]string)
 				secret := make(map[string]string)
-				volumeID := "rg#f5713de20cde511e8ba4900#pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41"
-				secret["accountname"] = "unit-test"
-				secret["azurestorageaccountname"] = "unit-test"
+				volumeID := "rg#f5713de20cde511e8ba4900#containername"
+				secret["accountname"] = "accountname"
+				secret["azurestorageaccountname"] = "accountname"
 				secret["accountkey"] = "unit-test"
 				secret["azurestorageaccountkey"] = "unit-test"
 				secret["azurestorageaccountsastoken"] = "unit-test"
 				secret["msisecret"] = "unit-test"
 				secret["azurestoragespnclientsecret"] = "unit-test"
-				_, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
-				expectedErr := fmt.Errorf("could not find containerName from attributes(map[]) or volumeID(rg#f5713de20cde511e8ba4900#pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41)")
-				if !reflect.DeepEqual(err, expectedErr) {
-					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
+				accountName, containerName, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
+				if err != nil {
+					t.Errorf("actualErr: (%v), expectedErr: nil", err)
 				}
+				assert.Equal(t, accountName, "accountname")
+				assert.Equal(t, containerName, "containername")
 			},
 		},
 		{