Merge pull request #264 from andyzhangx/nfs-pv-volumeid-fix

fix: volumeid error in nfs pv

Author: andyzhangx

pkg/blob/blob.go

@@ -155,7 +155,7 @@ func (d *Driver) Run(endpoint, kubeconfig string, testBool bool) {
 	s.Wait()
 }
 
-// get container info according to volume id, e.g.
+// GetContainerInfo get container info according to volume id, e.g.
 // input: "rg#f5713de20cde511e8ba4900#pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41"
 // output: rg, f5713de20cde511e8ba4900, pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41
 func GetContainerInfo(id string) (string, string, string, error) {
@@ -246,7 +246,7 @@ func isSASToken(key string) bool {
 }
 
 // GetAuthEnv return <accountName, containerName, authEnv, error>
-func (d *Driver) GetAuthEnv(ctx context.Context, volumeID string, attrib, secrets map[string]string) (string, string, []string, error) {
+func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attrib, secrets map[string]string) (string, string, []string, error) {
 	var (
 		accountName           string
 		accountKey            string
@@ -269,7 +269,9 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID string, attrib, secret
 			keyVaultSecretName = v
 		case "keyvaultsecretversion":
 			keyVaultSecretVersion = v
-		case "storageaccountname":
+		case storageAccountField:
+			accountName = v
+		case "storageaccountname": // for compatibility
 			accountName = v
 		case "azurestorageauthtype":
 			authEnv = append(authEnv, "AZURE_STORAGE_AUTH_TYPE="+v)
@@ -291,6 +293,10 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID string, attrib, secret
 	}
 	klog.V(2).Infof("volumeID(%s) authEnv: %s", volumeID, authEnv)
 
+	if protocol == nfs {
+		return accountName, containerName, authEnv, err
+	}
+
 	// 1. If keyVaultURL is not nil, preferentially use the key stored in key vault.
 	// 2. Then if secrets map is not nil, use the key stored in the secrets map.
 	// 3. Finally if both keyVaultURL and secrets map are nil, get the key from Azure.
@@ -390,6 +396,8 @@ func (d *Driver) GetStorageAccountAndContainer(ctx context.Context, volumeID str
 			keyVaultSecretName = v
 		case "keyvaultsecretversion":
 			keyVaultSecretVersion = v
+		case storageAccountField:
+			accountName = v
 		case "storageaccountname":
 			accountName = v
 		}

pkg/blob/blob_test.go

@@ -440,7 +440,6 @@ func TestIsSupportedProtocol(t *testing.T) {
 }
 
 func TestGetAuthEnv(t *testing.T) {
-
 	testCases := []struct {
 		name     string
 		testFunc func(t *testing.T)
@@ -473,7 +472,7 @@ func TestGetAuthEnv(t *testing.T) {
 					RawError: fmt.Errorf("test"),
 				}
 				mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any()).Return(accountListKeysResult, rerr).AnyTimes()
-				_, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, attrib, secret)
+				_, _, _, 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")
 				if !reflect.DeepEqual(err, expectedErr) {
 					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
@@ -502,7 +501,7 @@ func TestGetAuthEnv(t *testing.T) {
 					Keys: &accountkeylist,
 				}
 				mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any()).Return(list, nil).AnyTimes()
-				_, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, attrib, secret)
+				_, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
 				expectedErr := error(nil)
 				if !reflect.DeepEqual(err, expectedErr) {
 					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
@@ -523,7 +522,7 @@ func TestGetAuthEnv(t *testing.T) {
 				secret["azurestorageaccountsastoken"] = "unit-test"
 				secret["msisecret"] = "unit-test"
 				secret["azurestoragespnclientsecret"] = "unit-test"
-				_, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, attrib, secret)
+				_, _, _, 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)

pkg/blob/nodeserver.go

@@ -123,11 +123,6 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 	attrib := req.GetVolumeContext()
 	secrets := req.GetSecrets()
 
-	accountName, containerName, authEnv, err := d.GetAuthEnv(ctx, volumeID, attrib, secrets)
-	if err != nil {
-		return nil, err
-	}
-
 	var blobStorageEndPoint, protocol string
 	for k, v := range attrib {
 		switch strings.ToLower(k) {
@@ -137,6 +132,12 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 			protocol = v
 		}
 	}
+
+	accountName, containerName, authEnv, err := d.GetAuthEnv(ctx, volumeID, protocol, attrib, secrets)
+	if err != nil {
+		return nil, err
+	}
+
 	if strings.TrimSpace(blobStorageEndPoint) == "" {
 		// server address is "accountname.blob.core.windows.net" by default
 		blobStorageEndPoint = fmt.Sprintf("%s.blob.%s", accountName, d.cloud.Environment.StorageEndpointSuffix)

pkg/blob/nodeserver_test.go

@@ -317,40 +317,6 @@ func TestNodeStageVolume(t *testing.T) {
 				}
 			},
 		},
-		{
-			name: "volume mount sensitive fail",
-			testFunc: func(t *testing.T) {
-				attrib := make(map[string]string)
-				attrib["containername"] = "ut-container"
-				attrib["server"] = "blob-endpoint"
-				attrib["protocol"] = "nfs"
-				secret := make(map[string]string)
-				secret["accountname"] = "unit-test"
-				accessType := csi.VolumeCapability_Mount{}
-				req := &csi.NodeStageVolumeRequest{
-					VolumeId:          "rg#f5713de20cde511e8ba4900#pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
-					StagingTargetPath: "right_mount",
-					VolumeContext:     attrib,
-					Secrets:           secret,
-					VolumeCapability: &csi.VolumeCapability{
-						AccessType: &accessType,
-					},
-				}
-				d := NewFakeDriver()
-				fakeMounter := &fakeMounter{}
-				d.mounter = &mount.SafeFormatAndMount{
-					Interface: fakeMounter,
-				}
-				_, err := d.NodeStageVolume(context.TODO(), req)
-				expectedErr := status.Error(codes.Internal, "volume(rg#f5713de20cde511e8ba4900#pvc-fuse-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41) mount \"blob-endpoint:/unit-test/ut-container\" on \"right_mount\" failed with fake MountSensitive: source error")
-				if !reflect.DeepEqual(err, expectedErr) {
-					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
-				}
-				// Clean up
-				err = os.RemoveAll(req.StagingTargetPath)
-				assert.NoError(t, err)
-			},
-		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, tc.testFunc)