Merge pull request #328 from andyzhangx/zero-mountpermissions

feat: skip chmod if mountPermissions is 0 after mount

docs/driver-parameters.md

@@ -8,7 +8,7 @@ Name | Meaning | Example Value | Mandatory | Default value
 --- | --- | --- | --- | ---
 server | NFS Server address | domain name `nfs-server.default.svc.cluster.local` <br>or IP address `127.0.0.1` | Yes |
 share | NFS share path | `/` | Yes |
-mountPermissions | mounted folder permissions. The default is `0777` |  | No |
+mountPermissions | mounted folder permissions. The default is `0777`, if set as `0`, driver will not perform `chmod` after mount |  | No |
 
 ### PV/PVC usage (static provisioning)
 > [`PersistentVolume` example](../deploy/example/pv-nfs-csi.yaml)

pkg/nfs/nodeserver.go

@@ -58,6 +58,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 
 	var server, baseDir string
 	mountPermissions := ns.Driver.mountPermissions
+	performChmodOp := (mountPermissions > 0)
 	for k, v := range req.GetVolumeContext() {
 		switch strings.ToLower(k) {
 		case paramServer:
@@ -71,9 +72,15 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 		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
+				}
 			}
 		}
 	}
@@ -114,8 +121,12 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	}
 
 	klog.V(2).Infof("volumeID(%v): mount targetPath(%s) with permissions(0%o)", volumeID, targetPath, mountPermissions)
-	if err := os.Chmod(targetPath, os.FileMode(mountPermissions)); err != nil {
-		return nil, status.Error(codes.Internal, err.Error())
+	if performChmodOp {
+		if err := os.Chmod(targetPath, os.FileMode(mountPermissions)); err != nil {
+			return nil, status.Error(codes.Internal, err.Error())
+		}
+	} else {
+		klog.V(2).Infof("skip chmod on targetPath(%s) since mountPermissions is set as 0", targetPath)
 	}
 	return &csi.NodePublishVolumeResponse{}, nil
 }

pkg/nfs/nodeserver_test.go

@@ -47,6 +47,11 @@ func TestNodePublishVolume(t *testing.T) {
 		"share":               "share",
 		mountPermissionsField: "0755",
 	}
+	paramsWithZeroPermissions := map[string]string{
+		"server":              "server",
+		"share":               "share",
+		mountPermissionsField: "0",
+	}
 
 	invalidParams := map[string]string{
 		"server":              "server",
@@ -121,6 +126,16 @@ func TestNodePublishVolume(t *testing.T) {
 				Readonly:         true},
 			expectedErr: nil,
 		},
+		{
+			desc: "[Success] Valid request with 0 mountPermissions",
+			req: csi.NodePublishVolumeRequest{
+				VolumeContext:    paramsWithZeroPermissions,
+				VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap},
+				VolumeId:         "vol_1",
+				TargetPath:       targetTest,
+				Readonly:         true},
+			expectedErr: nil,
+		},
 		{
 			desc: "[Error] invalid mountPermissions",
 			req: csi.NodePublishVolumeRequest{

test/e2e/dynamic_provisioning_test.go

@@ -70,7 +70,29 @@ var _ = ginkgo.Describe("Dynamic Provisioning", func() {
 			Pods:                   pods,
 			StorageClassParameters: defaultStorageClassParameters,
 		}
+		test.Run(cs, ns)
+	})
 
+	ginkgo.It("should create a volume on demand with zero mountPermissions [nfs.csi.k8s.io]", func() {
+		pods := []testsuites.PodDetails{
+			{
+				Cmd: "echo 'hello world' > /mnt/test-1/data && grep 'hello world' /mnt/test-1/data",
+				Volumes: []testsuites.VolumeDetails{
+					{
+						ClaimSize: "10Gi",
+						VolumeMount: testsuites.VolumeMountDetails{
+							NameGenerate:      "test-volume-",
+							MountPathGenerate: "/mnt/test-",
+						},
+					},
+				},
+			},
+		}
+		test := testsuites.DynamicallyProvisionedCmdVolumeTest{
+			CSIDriver:              testDriver,
+			Pods:                   pods,
+			StorageClassParameters: storageClassParametersWithZeroMountPermisssions,
+		}
 		test.Run(cs, ns)
 	})
 

test/e2e/e2e_suite_test.go

@@ -52,6 +52,13 @@ var (
 		"csi.storage.k8s.io/provisioner-secret-namespace": "default",
 		"mountPermissions": "0755",
 	}
+	storageClassParametersWithZeroMountPermisssions = map[string]string{
+		"server": nfsServerAddress,
+		"share":  nfsShare,
+		"csi.storage.k8s.io/provisioner-secret-name":      "mount-options",
+		"csi.storage.k8s.io/provisioner-secret-namespace": "default",
+		"mountPermissions": "0",
+	}
 	controllerServer *nfs.ControllerServer
 )