Merge pull request #90162 from rfranzke/fix/azure-csi-translation

Fix flaws in Azure CSI translation

staging/src/k8s.io/csi-translation-lib/CONTRIBUTING.md

@@ -2,6 +2,6 @@
 
 Do not open pull requests directly against this repository, they will be ignored. Instead, please open pull requests against [kubernetes/kubernetes](https://git.k8s.io/kubernetes/).  Please follow the same [contributing guide](https://git.k8s.io/kubernetes/CONTRIBUTING.md) you would follow for any other pull request made to kubernetes/kubernetes.
 
-This repository is published from [kubernetes/kubernetes/staging/src/k8s.io/csi-api](https://git.k8s.io/kubernetes/staging/src/k8s.io/csi-api) by the [kubernetes publishing-bot](https://git.k8s.io/publishing-bot).
+This repository is published from [kubernetes/kubernetes/staging/src/k8s.io/csi-translation-lib](https://git.k8s.io/kubernetes/staging/src/k8s.io/csi-translation-lib) by the [kubernetes publishing-bot](https://git.k8s.io/publishing-bot).
 
 Please see [Staging Directory and Publishing](https://git.k8s.io/community/contributors/devel/sig-architecture/staging.md) for more information.

staging/src/k8s.io/csi-translation-lib/go.mod

@@ -5,9 +5,11 @@ module k8s.io/csi-translation-lib
 go 1.13
 
 require (
+	github.com/stretchr/testify v1.4.0
 	k8s.io/api v0.0.0
 	k8s.io/apimachinery v0.0.0
 	k8s.io/cloud-provider v0.0.0
+	k8s.io/klog v1.0.0
 )
 
 replace (

staging/src/k8s.io/csi-translation-lib/plugins/BUILD

@@ -19,6 +19,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/cloud-provider/volume:go_default_library",
+        "//vendor/k8s.io/klog:go_default_library",
     ],
 )
 
@@ -49,5 +50,7 @@ go_test(
     deps = [
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/api/storage/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
     ],
 )

staging/src/k8s.io/csi-translation-lib/plugins/azure_disk.go

@@ -110,22 +110,23 @@ func (t *azureDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
 		return nil, fmt.Errorf("pv is nil or Azure Disk source not defined on pv")
 	}
 
-	azureSource := pv.Spec.PersistentVolumeSource.AzureDisk
+	var (
+		azureSource = pv.Spec.PersistentVolumeSource.AzureDisk
 
 		// refer to https://github.com/kubernetes-sigs/azuredisk-csi-driver/blob/master/docs/driver-parameters.md
-	csiSource := &v1.CSIPersistentVolumeSource{
+		csiSource = &v1.CSIPersistentVolumeSource{
 			Driver:           AzureDiskDriverName,
-		VolumeHandle:     azureSource.DataDiskURI,
-		ReadOnly:         *azureSource.ReadOnly,
-		FSType:           *azureSource.FSType,
 			VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
+			VolumeHandle:     azureSource.DataDiskURI,
 		}
+	)
 
 	if azureSource.CachingMode != nil {
 		csiSource.VolumeAttributes[azureDiskCachingMode] = string(*azureSource.CachingMode)
 	}
 
 	if azureSource.FSType != nil {
+		csiSource.FSType = *azureSource.FSType
 		csiSource.VolumeAttributes[azureDiskFSType] = *azureSource.FSType
 	}
 
@@ -133,9 +134,12 @@ func (t *azureDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
 		csiSource.VolumeAttributes[azureDiskKind] = string(*azureSource.Kind)
 	}
 
+	if azureSource.ReadOnly != nil {
+		csiSource.ReadOnly = *azureSource.ReadOnly
+	}
+
 	pv.Spec.PersistentVolumeSource.AzureDisk = nil
 	pv.Spec.PersistentVolumeSource.CSI = csiSource
-	pv.Spec.AccessModes = backwardCompatibleAccessModes(pv.Spec.AccessModes)
 
 	return pv, nil
 }

staging/src/k8s.io/csi-translation-lib/plugins/azure_disk_test.go

@@ -20,6 +20,9 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestIsManagedDisk(t *testing.T) {
@@ -91,3 +94,139 @@ func TestGetDiskName(t *testing.T) {
 		}
 	}
 }
+
+func TestTranslateAzureDiskInTreeStorageClassToCSI(t *testing.T) {
+	translator := NewAzureDiskCSITranslator()
+
+	cases := []struct {
+		name   string
+		volume *corev1.Volume
+		expVol *corev1.PersistentVolume
+		expErr bool
+	}{
+		{
+			name:   "empty volume",
+			expErr: true,
+		},
+		{
+			name:   "no azure disk volume",
+			volume: &corev1.Volume{},
+			expErr: true,
+		},
+		{
+			name: "azure disk volume",
+			volume: &corev1.Volume{
+				VolumeSource: corev1.VolumeSource{
+					AzureDisk: &corev1.AzureDiskVolumeSource{
+						DiskName:    "diskname",
+						DataDiskURI: "datadiskuri",
+					},
+				},
+			},
+			expVol: &corev1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "disk.csi.azure.com-diskname",
+				},
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:           "disk.csi.azure.com",
+							VolumeHandle:     "datadiskuri",
+							VolumeAttributes: map[string]string{azureDiskKind: "Managed"},
+						},
+					},
+					AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Logf("Testing %v", tc.name)
+		got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.volume)
+		if err != nil && !tc.expErr {
+			t.Errorf("Did not expect error but got: %v", err)
+		}
+
+		if err == nil && tc.expErr {
+			t.Errorf("Expected error, but did not get one.")
+		}
+
+		if !reflect.DeepEqual(got, tc.expVol) {
+			t.Errorf("Got parameters: %v, expected :%v", got, tc.expVol)
+		}
+	}
+}
+
+func TestTranslateAzureDiskInTreePVToCSI(t *testing.T) {
+	translator := NewAzureDiskCSITranslator()
+
+	cachingMode := corev1.AzureDataDiskCachingMode("cachingmode")
+	fsType := "fstype"
+	readOnly := true
+
+	cases := []struct {
+		name   string
+		volume *corev1.PersistentVolume
+		expVol *corev1.PersistentVolume
+		expErr bool
+	}{
+		{
+			name:   "empty volume",
+			expErr: true,
+		},
+		{
+			name:   "no azure file volume",
+			volume: &corev1.PersistentVolume{},
+			expErr: true,
+		},
+		{
+			name: "azure file volume",
+			volume: &corev1.PersistentVolume{
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						AzureDisk: &corev1.AzureDiskVolumeSource{
+							CachingMode: &cachingMode,
+							DataDiskURI: "datadiskuri",
+							FSType:      &fsType,
+							ReadOnly:    &readOnly,
+						},
+					},
+				},
+			},
+			expVol: &corev1.PersistentVolume{
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:   "disk.csi.azure.com",
+							FSType:   "fstype",
+							ReadOnly: true,
+							VolumeAttributes: map[string]string{
+								azureDiskCachingMode: "cachingmode",
+								azureDiskFSType:      fsType,
+								azureDiskKind:        "Managed",
+							},
+							VolumeHandle: "datadiskuri",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Logf("Testing %v", tc.name)
+		got, err := translator.TranslateInTreePVToCSI(tc.volume)
+		if err != nil && !tc.expErr {
+			t.Errorf("Did not expect error but got: %v", err)
+		}
+
+		if err == nil && tc.expErr {
+			t.Errorf("Expected error, but did not get one.")
+		}
+
+		if !reflect.DeepEqual(got, tc.expVol) {
+			t.Errorf("Got parameters: %v, expected :%v", got, tc.expVol)
+		}
+	}
+}

staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go

@@ -18,11 +18,13 @@ package plugins
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/klog"
 )
 
 const (
@@ -32,14 +34,19 @@ const (
 	AzureFileInTreePluginName = "kubernetes.io/azure-file"
 
 	separator        = "#"
-	volumeIDTemplate = "%s#%s#%s"
+	volumeIDTemplate = "%s#%s#%s#%s"
 	// Parameter names defined in azure file CSI driver, refer to
 	// https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md
 	azureFileShareName = "shareName"
+
+	secretNameTemplate     = "azure-storage-account-%s-secret"
+	defaultSecretNamespace = "default"
 )
 
 var _ InTreePlugin = &azureFileCSITranslator{}
 
+var secretNameFormatRE = regexp.MustCompile(`azure-storage-account-(.+)-secret`)
+
 // azureFileCSITranslator handles translation of PV spec from In-tree
 // Azure File to CSI Azure File and vice versa
 type azureFileCSITranslator struct{}
@@ -58,12 +65,18 @@ func (t *azureFileCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.St
 // and converts the AzureFile source to a CSIPersistentVolumeSource
 func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error) {
 	if volume == nil || volume.AzureFile == nil {
-		return nil, fmt.Errorf("volume is nil or AWS EBS not defined on volume")
+		return nil, fmt.Errorf("volume is nil or Azure File not defined on volume")
 	}
 
 	azureSource := volume.AzureFile
+	accountName, err := getStorageAccountName(azureSource.SecretName)
+	if err != nil {
+		klog.Warningf("getStorageAccountName(%s) returned with error: %v", azureSource.SecretName, err)
+		accountName = azureSource.SecretName
+	}
 
-	pv := &v1.PersistentVolume{
+	var (
+		pv = &v1.PersistentVolume{
 			ObjectMeta: metav1.ObjectMeta{
 				// Must be unique per disk as it is used as the unique part of the
 				// staging path
@@ -72,18 +85,21 @@ func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
 			Spec: v1.PersistentVolumeSpec{
 				PersistentVolumeSource: v1.PersistentVolumeSource{
 					CSI: &v1.CSIPersistentVolumeSource{
-					VolumeHandle:     fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName),
+						Driver:           AzureFileDriverName,
+						VolumeHandle:     fmt.Sprintf(volumeIDTemplate, "", accountName, azureSource.ShareName, ""),
 						ReadOnly:         azureSource.ReadOnly,
 						VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
-					NodePublishSecretRef: &v1.SecretReference{
+						NodeStageSecretRef: &v1.SecretReference{
-						Name:      azureSource.ShareName,
+							Name:      azureSource.SecretName,
-						Namespace: "default",
+							Namespace: defaultSecretNamespace,
 						},
 					},
 				},
 				AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteMany},
 			},
 		}
+	)
+
 	return pv, nil
 }
 
@@ -95,23 +111,33 @@ func (t *azureFileCSITranslator) TranslateInTreePVToCSI(pv *v1.PersistentVolume)
 	}
 
 	azureSource := pv.Spec.PersistentVolumeSource.AzureFile
+	accountName, err := getStorageAccountName(azureSource.SecretName)
+	if err != nil {
+		klog.Warningf("getStorageAccountName(%s) returned with error: %v", azureSource.SecretName, err)
+		accountName = azureSource.SecretName
+	}
+	volumeID := fmt.Sprintf(volumeIDTemplate, "", accountName, azureSource.ShareName, "")
 
-	volumeID := fmt.Sprintf(volumeIDTemplate, "", azureSource.SecretName, azureSource.ShareName)
+	var (
 		// refer to https://github.com/kubernetes-sigs/azurefile-csi-driver/blob/master/docs/driver-parameters.md
-	csiSource := &v1.CSIPersistentVolumeSource{
+		csiSource = &v1.CSIPersistentVolumeSource{
-		VolumeHandle:     volumeID,
+			Driver: AzureFileDriverName,
+			NodeStageSecretRef: &v1.SecretReference{
+				Name:      azureSource.SecretName,
+				Namespace: defaultSecretNamespace,
+			},
 			ReadOnly:         azureSource.ReadOnly,
 			VolumeAttributes: map[string]string{azureFileShareName: azureSource.ShareName},
+			VolumeHandle:     volumeID,
 		}
+	)
 
-	csiSource.NodePublishSecretRef = &v1.SecretReference{
+	if azureSource.SecretNamespace != nil {
-		Name:      azureSource.ShareName,
+		csiSource.NodeStageSecretRef.Namespace = *azureSource.SecretNamespace
-		Namespace: *azureSource.SecretNamespace,
 	}
 
 	pv.Spec.PersistentVolumeSource.AzureFile = nil
 	pv.Spec.PersistentVolumeSource.CSI = csiSource
-	pv.Spec.AccessModes = backwardCompatibleAccessModes(pv.Spec.AccessModes)
 
 	return pv, nil
 }
@@ -129,22 +155,21 @@ func (t *azureFileCSITranslator) TranslateCSIPVToInTree(pv *v1.PersistentVolume)
 		ReadOnly: csiSource.ReadOnly,
 	}
 
-	if csiSource.NodePublishSecretRef != nil && csiSource.NodePublishSecretRef.Name != "" {
+	if csiSource.NodeStageSecretRef != nil && csiSource.NodeStageSecretRef.Name != "" {
-		azureSource.SecretName = csiSource.NodePublishSecretRef.Name
+		azureSource.SecretName = csiSource.NodeStageSecretRef.Name
-		azureSource.SecretNamespace = &csiSource.NodePublishSecretRef.Namespace
+		azureSource.SecretNamespace = &csiSource.NodeStageSecretRef.Namespace
 		if csiSource.VolumeAttributes != nil {
 			if shareName, ok := csiSource.VolumeAttributes[azureFileShareName]; ok {
 				azureSource.ShareName = shareName
 			}
 		}
 	} else {
-		_, _, fileShareName, err := getFileShareInfo(csiSource.VolumeHandle)
+		_, storageAccount, fileShareName, _, err := getFileShareInfo(csiSource.VolumeHandle)
 		if err != nil {
 			return nil, err
 		}
 		azureSource.ShareName = fileShareName
-		// to-do: for dynamic provision scenario in CSI, it uses cluster's identity to get storage account key
+		azureSource.SecretName = fmt.Sprintf(secretNameTemplate, storageAccount)
-		// secret for the file share is not created, we may create a serect here
 	}
 
 	pv.Spec.CSI = nil
@@ -182,12 +207,25 @@ func (t *azureFileCSITranslator) RepairVolumeHandle(volumeHandle, nodeID string)
 }
 
 // get file share info according to volume id, e.g.
-// input: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41"
+// input: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#diskname.vhd"
-// output: rg, f5713de20cde511e8ba4900, pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41
+// output: rg, f5713de20cde511e8ba4900, pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41, diskname.vhd
-func getFileShareInfo(id string) (string, string, string, error) {
+func getFileShareInfo(id string) (string, string, string, string, error) {
 	segments := strings.Split(id, separator)
 	if len(segments) < 3 {
-		return "", "", "", fmt.Errorf("error parsing volume id: %q, should at least contain two #", id)
+		return "", "", "", "", fmt.Errorf("error parsing volume id: %q, should at least contain two #", id)
 	}
-	return segments[0], segments[1], segments[2], nil
+	var diskName string
+	if len(segments) > 3 {
+		diskName = segments[3]
+	}
+	return segments[0], segments[1], segments[2], diskName, nil
+}
+
+// get storage account name from secret name
+func getStorageAccountName(secretName string) (string, error) {
+	matches := secretNameFormatRE.FindStringSubmatch(secretName)
+	if len(matches) != 2 {
+		return "", fmt.Errorf("could not get account name from %s, correct format: %s", secretName, secretNameFormatRE)
+	}
+	return matches[1], nil
 }

staging/src/k8s.io/csi-translation-lib/plugins/azure_file_test.go

@@ -20,52 +20,255 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestGetFileShareInfo(t *testing.T) {
 	tests := []struct {
-		options   string
+		id                string
-		expected1 string
+		resourceGroupName string
-		expected2 string
+		accountName       string
-		expected3 string
+		fileShareName     string
-		expected4 error
+		diskName          string
+		expectedError     error
 	}{
 		{
-			options:   "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
+			id:                "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41#diskname.vhd",
-			expected1: "rg",
+			resourceGroupName: "rg",
-			expected2: "f5713de20cde511e8ba4900",
+			accountName:       "f5713de20cde511e8ba4900",
-			expected3: "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
+			fileShareName:     "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
-			expected4: nil,
+			diskName:          "diskname.vhd",
+			expectedError:     nil,
 		},
 		{
-			options:   "rg#f5713de20cde511e8ba4900",
+			id:                "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
-			expected1: "",
+			resourceGroupName: "rg",
-			expected2: "",
+			accountName:       "f5713de20cde511e8ba4900",
-			expected3: "",
+			fileShareName:     "pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41",
-			expected4: fmt.Errorf("error parsing volume id: \"rg#f5713de20cde511e8ba4900\", should at least contain two #"),
+			diskName:          "",
+			expectedError:     nil,
 		},
 		{
-			options:   "rg",
+			id:                "rg#f5713de20cde511e8ba4900",
-			expected1: "",
+			resourceGroupName: "",
-			expected2: "",
+			accountName:       "",
-			expected3: "",
+			fileShareName:     "",
-			expected4: fmt.Errorf("error parsing volume id: \"rg\", should at least contain two #"),
+			diskName:          "",
+			expectedError:     fmt.Errorf("error parsing volume id: \"rg#f5713de20cde511e8ba4900\", should at least contain two #"),
 		},
 		{
-			options:   "",
+			id:                "rg",
-			expected1: "",
+			resourceGroupName: "",
-			expected2: "",
+			accountName:       "",
-			expected3: "",
+			fileShareName:     "",
-			expected4: fmt.Errorf("error parsing volume id: \"\", should at least contain two #"),
+			diskName:          "",
+			expectedError:     fmt.Errorf("error parsing volume id: \"rg\", should at least contain two #"),
+		},
+		{
+			id:                "",
+			resourceGroupName: "",
+			accountName:       "",
+			fileShareName:     "",
+			diskName:          "",
+			expectedError:     fmt.Errorf("error parsing volume id: \"\", should at least contain two #"),
 		},
 	}
 
 	for _, test := range tests {
-		result1, result2, result3, result4 := getFileShareInfo(test.options)
+		resourceGroupName, accountName, fileShareName, diskName, expectedError := getFileShareInfo(test.id)
-		if !reflect.DeepEqual(result1, test.expected1) || !reflect.DeepEqual(result2, test.expected2) ||
+		if resourceGroupName != test.resourceGroupName {
-			!reflect.DeepEqual(result3, test.expected3) || !reflect.DeepEqual(result4, test.expected4) {
+			t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, resourceGroupName, test.resourceGroupName)
-			t.Errorf("input: %q, getFileShareInfo result1: %q, expected1: %q, result2: %q, expected2: %q, result3: %q, expected3: %q, result4: %q, expected4: %q", test.options, result1, test.expected1, result2, test.expected2,
+		}
-				result3, test.expected3, result4, test.expected4)
+		if accountName != test.accountName {
+			t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, accountName, test.accountName)
+		}
+		if fileShareName != test.fileShareName {
+			t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, fileShareName, test.fileShareName)
+		}
+		if diskName != test.diskName {
+			t.Errorf("getFileShareInfo(%q) returned with: %q, expected: %q", test.id, diskName, test.diskName)
+		}
+		if !reflect.DeepEqual(expectedError, test.expectedError) {
+			t.Errorf("getFileShareInfo(%q) returned with: %v, expected: %v", test.id, expectedError, test.expectedError)
 		}
 	}
 }
+
+func TestTranslateAzureFileInTreeStorageClassToCSI(t *testing.T) {
+	translator := NewAzureFileCSITranslator()
+
+	cases := []struct {
+		name   string
+		volume *corev1.Volume
+		expVol *corev1.PersistentVolume
+		expErr bool
+	}{
+		{
+			name:   "empty volume",
+			expErr: true,
+		},
+		{
+			name:   "no azure file volume",
+			volume: &corev1.Volume{},
+			expErr: true,
+		},
+		{
+			name: "azure file volume",
+			volume: &corev1.Volume{
+				VolumeSource: corev1.VolumeSource{
+					AzureFile: &corev1.AzureFileVolumeSource{
+						ReadOnly:   true,
+						SecretName: "secretname",
+						ShareName:  "sharename",
+					},
+				},
+			},
+			expVol: &corev1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "file.csi.azure.com-sharename",
+				},
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver: "file.csi.azure.com",
+							NodeStageSecretRef: &corev1.SecretReference{
+								Name:      "secretname",
+								Namespace: "default",
+							},
+							ReadOnly:         true,
+							VolumeAttributes: map[string]string{azureFileShareName: "sharename"},
+							VolumeHandle:     "#secretname#sharename#",
+						},
+					},
+					AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany},
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Logf("Testing %v", tc.name)
+		got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.volume)
+		if err != nil && !tc.expErr {
+			t.Errorf("Did not expect error but got: %v", err)
+		}
+
+		if err == nil && tc.expErr {
+			t.Errorf("Expected error, but did not get one.")
+		}
+
+		if !reflect.DeepEqual(got, tc.expVol) {
+			t.Errorf("Got parameters: %v, expected :%v", got, tc.expVol)
+		}
+	}
+}
+
+func TestTranslateAzureFileInTreePVToCSI(t *testing.T) {
+	translator := NewAzureFileCSITranslator()
+
+	secretNamespace := "secretnamespace"
+
+	cases := []struct {
+		name   string
+		volume *corev1.PersistentVolume
+		expVol *corev1.PersistentVolume
+		expErr bool
+	}{
+		{
+			name:   "empty volume",
+			expErr: true,
+		},
+		{
+			name:   "no azure file volume",
+			volume: &corev1.PersistentVolume{},
+			expErr: true,
+		},
+		{
+			name: "azure file volume",
+			volume: &corev1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "file.csi.azure.com-sharename",
+				},
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						AzureFile: &corev1.AzureFilePersistentVolumeSource{
+							ShareName:       "sharename",
+							SecretName:      "secretname",
+							SecretNamespace: &secretNamespace,
+							ReadOnly:        true,
+						},
+					},
+				},
+			},
+			expVol: &corev1.PersistentVolume{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "file.csi.azure.com-sharename",
+				},
+				Spec: corev1.PersistentVolumeSpec{
+					PersistentVolumeSource: corev1.PersistentVolumeSource{
+						CSI: &corev1.CSIPersistentVolumeSource{
+							Driver:   "file.csi.azure.com",
+							ReadOnly: true,
+							NodeStageSecretRef: &corev1.SecretReference{
+								Name:      "secretname",
+								Namespace: secretNamespace,
+							},
+							VolumeAttributes: map[string]string{azureFileShareName: "sharename"},
+							VolumeHandle:     "#secretname#sharename#",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Logf("Testing %v", tc.name)
+		got, err := translator.TranslateInTreePVToCSI(tc.volume)
+		if err != nil && !tc.expErr {
+			t.Errorf("Did not expect error but got: %v", err)
+		}
+
+		if err == nil && tc.expErr {
+			t.Errorf("Expected error, but did not get one.")
+		}
+
+		if !reflect.DeepEqual(got, tc.expVol) {
+			t.Errorf("Got parameters: %v, expected :%v", got, tc.expVol)
+		}
+	}
+}
+
+func TestGetStorageAccount(t *testing.T) {
+	tests := []struct {
+		secretName     string
+		expectedError  bool
+		expectedResult string
+	}{
+		{
+			secretName:     "azure-storage-account-accountname-secret",
+			expectedError:  false,
+			expectedResult: "accountname",
+		},
+		{
+			secretName:     "azure-storage-account-accountname-dup-secret",
+			expectedError:  false,
+			expectedResult: "accountname-dup",
+		},
+		{
+			secretName:     "invalid",
+			expectedError:  true,
+			expectedResult: "",
+		},
+	}
+
+	for i, test := range tests {
+		accountName, err := getStorageAccountName(test.secretName)
+		assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]", i)
+		assert.Equal(t, test.expectedResult, accountName, "TestCase[%d]", i)
+	}
+}