Merge pull request #648 from andyzhangx/containerNamePrefix

feat: add containerNamePrefix in storage class

Author: andyzhangx

docs/driver-parameters.md

@@ -13,7 +13,8 @@ location | Azure location | `eastus`, `westus`, etc. | No | if empty, driver wil
 resourceGroup | Azure resource group name | existing resource group name | No | if empty, driver will use the same resource group name as current k8s cluster
 storageAccount | specify Azure storage account name| STORAGE_ACCOUNT_NAME | - No for blobfuse mount </br> - Yes for NFSv3 mount |  - For blobfuse mount: if empty, driver will find a suitable storage account that matches `skuName` in the same resource group; if a storage account name is provided, storage account must exist. </br>  - For NFSv3 mount, storage account name must be provided
 protocol | specify blobfuse mount or NFSv3 mount | `fuse`, `nfs` | No | `fuse`
-containerName | specify the existing container name | existing container name | No | if empty, driver will create a new container name, starting with `pvc-fuse` for blobfuse or `pvc-nfs` for NFSv3
+containerName | specify the existing container(directory) name | existing container name | No | if empty, driver will create a new container name, starting with `pvc-fuse` for blobfuse or `pvc-nfs` for NFSv3
+containerNamePrefix | specify Azure storage directory prefix created by driver | can only contain lowercase letters, numbers, hyphens, and length should be less than 21 | No |
 server | specify Azure storage account server address | existing server address, e.g. `accountname.privatelink.blob.core.windows.net` | No | if empty, driver will use default `accountname.blob.core.windows.net` or other sovereign cloud account address
 allowBlobPublicAccess | Allow or disallow public access to all blobs or containers for storage account created by driver | `true`,`false` | No | `false`
 storageEndpointSuffix | specify Azure storage endpoint suffix | `core.windows.net` | No | if empty, driver will use default storage endpoint suffix according to cloud environment, e.g. `core.windows.net`

pkg/blob/blob.go

@@ -64,6 +64,7 @@ const (
 	secretNameField              = "secretname"
 	secretNamespaceField         = "secretnamespace"
 	containerNameField           = "containername"
+	containerNamePrefixField     = "containernameprefix"
 	storeAccountKeyField         = "storeaccountkey"
 	isHnsEnabledField            = "ishnsenabled"
 	getAccountKeyFromSecretField = "getaccountkeyfromsecret"
@@ -278,11 +279,9 @@ func getValidContainerName(volumeName, protocol string) string {
 		// now we set as 63 for maximum container name length
 		// todo: get cluster name
 		containerName = k8sutil.GenerateVolumeName(fmt.Sprintf("pvc-%s", protocol), uuid.NewUUID().String(), 63)
-		klog.Warningf("the requested volume name (%q) is invalid, so it is regenerated as (%q)", volumeName, containerName)
+		klog.Warningf("requested volume name (%s) is invalid, regenerated as (%q)", volumeName, containerName)
 	}
-	containerName = strings.Replace(containerName, "--", "-", -1)
-
-	return containerName
+	return strings.Replace(containerName, "--", "-", -1)
 }
 
 func checkContainerNameBeginAndEnd(containerName string) bool {
@@ -563,6 +562,26 @@ func isSupportedProtocol(protocol string) bool {
 	return false
 }
 
+// container names can contain only lowercase letters, numbers, and hyphens,
+// and must begin and end with a letter or a number
+func isSupportedContainerNamePrefix(prefix string) bool {
+	if prefix == "" {
+		return true
+	}
+	if len(prefix) > 20 {
+		return false
+	}
+	if prefix[0] == '-' {
+		return false
+	}
+	for _, v := range prefix {
+		if v != '-' && (v < '0' || v > '9') && (v < 'a' || v > 'z') {
+			return false
+		}
+	}
+	return true
+}
+
 // get storage account from secrets map
 func getStorageAccount(secrets map[string]string) (string, string, error) {
 	if secrets == nil {

pkg/blob/blob_test.go

@@ -816,3 +816,50 @@ func TestAppendDefaultMountOptions(t *testing.T) {
 		}
 	}
 }
+
+func TestIsSupportedContainerNamePrefix(t *testing.T) {
+	tests := []struct {
+		prefix         string
+		expectedResult bool
+	}{
+		{
+			prefix:         "",
+			expectedResult: true,
+		},
+		{
+			prefix:         "ext3",
+			expectedResult: true,
+		},
+		{
+			prefix:         "ext-2",
+			expectedResult: true,
+		},
+		{
+			prefix:         "-xfs",
+			expectedResult: false,
+		},
+		{
+			prefix:         "Absdf",
+			expectedResult: false,
+		},
+		{
+			prefix:         "tooooooooooooooooooooooooolong",
+			expectedResult: false,
+		},
+		{
+			prefix:         "+invalid",
+			expectedResult: false,
+		},
+		{
+			prefix:         " invalidspace",
+			expectedResult: false,
+		},
+	}
+
+	for _, test := range tests {
+		result := isSupportedContainerNamePrefix(test.prefix)
+		if result != test.expectedResult {
+			t.Errorf("isSupportedContainerNamePrefix(%s) returned with %v, not equal to %v", test.prefix, result, test.expectedResult)
+		}
+	}
+}

pkg/blob/controllerserver.go

@@ -67,7 +67,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	if parameters == nil {
 		parameters = make(map[string]string)
 	}
-	var storageAccountType, subsID, resourceGroup, location, account, containerName, protocol, customTags, secretName, secretNamespace, pvcNamespace string
+	var storageAccountType, subsID, resourceGroup, location, account, containerName, containerNamePrefix, protocol, customTags, secretName, secretNamespace, pvcNamespace string
 	var isHnsEnabled *bool
 	var vnetResourceGroup, vnetName, subnetName string
 	// set allowBlobPublicAccess as false by default
@@ -94,6 +94,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 			resourceGroup = v
 		case containerNameField:
 			containerName = v
+		case containerNamePrefixField:
+			containerNamePrefix = v
 		case protocolField:
 			protocol = v
 		case tagsField:
@@ -170,6 +172,13 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		return nil, status.Errorf(codes.InvalidArgument, "protocol(%s) is not supported, supported protocol list: %v", protocol, supportedProtocolList)
 	}
 
+	if containerName != "" && containerNamePrefix != "" {
+		return nil, status.Errorf(codes.InvalidArgument, "containerName(%s) and containerNamePrefix(%s) could not be specified together", containerName, containerNamePrefix)
+	}
+	if !isSupportedContainerNamePrefix(containerNamePrefix) {
+		return nil, status.Errorf(codes.InvalidArgument, "containerNamePrefix(%s) can only contain lowercase letters, numbers, hyphens, and length should be less than 21", containerNamePrefix)
+	}
+
 	enableHTTPSTrafficOnly := true
 	accountKind := string(storage.KindStorageV2)
 	var (
@@ -268,7 +277,11 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 
 	validContainerName := containerName
 	if validContainerName == "" {
-		validContainerName = getValidContainerName(volName, protocol)
+		validContainerName = volName
+		if containerNamePrefix != "" {
+			validContainerName = containerNamePrefix + "-" + volName
+		}
+		validContainerName = getValidContainerName(validContainerName, protocol)
 		parameters[containerNameField] = validContainerName
 	}
 

pkg/blob/controllerserver_test.go

@@ -171,6 +171,51 @@ func TestCreateVolume(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "containerName and containerNamePrefix could not be specified together",
+			testFunc: func(t *testing.T) {
+				d := NewFakeDriver()
+				d.cloud = &azure.Cloud{}
+				mp := make(map[string]string)
+				mp[containerNameField] = "containerName"
+				mp[containerNamePrefixField] = "containerNamePrefix"
+				req := &csi.CreateVolumeRequest{
+					Name:               "unit-test",
+					VolumeCapabilities: stdVolumeCapabilities,
+					Parameters:         mp,
+				}
+				d.Cap = []*csi.ControllerServiceCapability{
+					controllerServiceCapability,
+				}
+				_, err := d.CreateVolume(context.Background(), req)
+				expectedErr := status.Errorf(codes.InvalidArgument, "containerName(containerName) and containerNamePrefix(containerNamePrefix) could not be specified together")
+				if !reflect.DeepEqual(err, expectedErr) {
+					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
+				}
+			},
+		},
+		{
+			name: "invalid containerNamePrefix",
+			testFunc: func(t *testing.T) {
+				d := NewFakeDriver()
+				d.cloud = &azure.Cloud{}
+				mp := make(map[string]string)
+				mp[containerNamePrefixField] = "UpperCase"
+				req := &csi.CreateVolumeRequest{
+					Name:               "unit-test",
+					VolumeCapabilities: stdVolumeCapabilities,
+					Parameters:         mp,
+				}
+				d.Cap = []*csi.ControllerServiceCapability{
+					controllerServiceCapability,
+				}
+				_, err := d.CreateVolume(context.Background(), req)
+				expectedErr := status.Errorf(codes.InvalidArgument, "containerNamePrefix(UpperCase) can only contain lowercase letters, numbers, hyphens, and length should be less than 21")
+				if !reflect.DeepEqual(err, expectedErr) {
+					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
+				}
+			},
+		},
 		{
 			name: "tags error",
 			testFunc: func(t *testing.T) {

test/e2e/dynamic_provisioning_test.go

@@ -115,8 +115,9 @@ var _ = ginkgo.Describe("[blob-csi-e2e] Dynamic Provisioning", func() {
 			CSIDriver: testDriver,
 			Pods:      pods,
 			StorageClassParameters: map[string]string{
-				"skuName":         "Standard_LRS",
-				"secretNamespace": "default",
+				"skuName":             "Standard_LRS",
+				"secretNamespace":     "default",
+				"containerNamePrefix": "nameprefix",
 			},
 		}
 		test.Run(cs, ns)