Merge pull request #672 from andyzhangx/zero-mountpermissions

feat: skip chmod if mountPermissions is 0 after mount

Author: andyzhangx

docs/driver-parameters.md

@@ -27,7 +27,7 @@ secretName | specify secret name to store account key | | No |
 secretNamespace | specify the namespace of secret to store account key | `default`,`kube-system`, etc | No | pvc namespace
 isHnsEnabled | enable `Hierarchical namespace` for Azure DataLake storage account | `true`,`false` | No | `false`
 --- | **Following parameters are only for NFS protocol** | --- | --- |
-mountPermissions | mounted folder permissions | `0777` | No |
+mountPermissions | mounted folder permissions. The default is `0777`, if set as `0`, driver will not perform `chmod` after mount | `0777` | No |
 
  - `fsGroup` securityContext setting
 

pkg/blob/nodeserver.go

@@ -220,6 +220,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 	var serverAddress, storageEndpointSuffix, protocol, ephemeralVolMountOptions string
 	var ephemeralVol, isHnsEnabled bool
 	mountPermissions := d.mountPermissions
+	performChmodOp := (mountPermissions > 0)
 	for k, v := range attrib {
 		switch strings.ToLower(k) {
 		case serverNameField:
@@ -237,9 +238,15 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 		case mountPermissionsField:
 			if v != "" {
 				var err error
-				if mountPermissions, err = strconv.ParseUint(v, 8, 32); err != nil {
+				var perm uint64
+				if perm, err = strconv.ParseUint(v, 8, 32); err != nil {
 					return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid mountPermissions %s", v))
 				}
+				if perm == 0 {
+					performChmodOp = false
+				} else {
+					mountPermissions = perm
+				}
 			}
 		}
 	}
@@ -283,12 +290,17 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 			return nil, status.Error(codes.Internal, fmt.Sprintf("volume(%s) mount %q on %q failed with %v", volumeID, source, targetPath, err))
 		}
 
-		// set permissions for NFSv3 root folder
-		if err := os.Chmod(targetPath, os.FileMode(mountPermissions)); err != nil {
-			return nil, status.Error(codes.Internal, fmt.Sprintf("Chmod(%s) failed with %v", targetPath, err))
+		if performChmodOp {
+			klog.V(2).Infof("volumeID(%v): chmod targetPath(%s) with permissions(0%o)", volumeID, targetPath, mountPermissions)
+			// set permissions for NFSv3 root folder
+			if err := os.Chmod(targetPath, os.FileMode(mountPermissions)); err != nil {
+				return nil, status.Error(codes.Internal, fmt.Sprintf("Chmod(%s) failed with %v", targetPath, err))
+			}
+		} else {
+			klog.V(2).Infof("skip chmod on targetPath(%s) since mountPermissions is set as 0", targetPath)
 		}
-		klog.V(2).Infof("volume(%s) mount %q on %q with 0%o succeeded", volumeID, source, targetPath, mountPermissions)
 
+		klog.V(2).Infof("volume(%s) mount %s on %s succeeded", volumeID, source, targetPath)
 		return &csi.NodeStageVolumeResponse{}, nil
 	}
 

test/e2e/dynamic_provisioning_test.go

@@ -531,4 +531,37 @@ var _ = ginkgo.Describe("[blob-csi-e2e] Dynamic Provisioning", func() {
 		}
 		test.Run(cs, ns)
 	})
+
+	ginkgo.It("should create a NFSv3 volume on demand with zero mountPermissions [nfs]", func() {
+		if isAzureStackCloud {
+			ginkgo.Skip("test case is not available for Azure Stack")
+		}
+		pods := []testsuites.PodDetails{
+			{
+				Cmd: "echo 'hello world' > /mnt/test-1/data && grep 'hello world' /mnt/test-1/data",
+				Volumes: []testsuites.VolumeDetails{
+					{
+						ClaimSize: "10Gi",
+						MountOptions: []string{
+							"nconnect=8",
+						},
+						VolumeMount: testsuites.VolumeMountDetails{
+							NameGenerate:      "test-volume-",
+							MountPathGenerate: "/mnt/test-",
+						},
+					},
+				},
+			},
+		}
+		test := testsuites.DynamicallyProvisionedCmdVolumeTest{
+			CSIDriver: testDriver,
+			Pods:      pods,
+			StorageClassParameters: map[string]string{
+				"skuName":          "Premium_LRS",
+				"protocol":         "nfs",
+				"mountPermissions": "0",
+			},
+		}
+		test.Run(cs, ns)
+	})
 })