feat: add mountPermissions parameter in storage class

docs/driver-parameters.md

@@ -8,6 +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 |
 
 ### PV/PVC usage (static provisioning)
 > [`PersistentVolume` example](../deploy/example/pv-nfs-csi.yaml)
@@ -16,7 +17,7 @@ Name | Meaning | Example Value | Mandatory | Default value
 --- | --- | --- | --- | ---
 volumeAttributes.server | NFS Server address | domain name `nfs-server.default.svc.cluster.local` <br>or IP address `127.0.0.1` | Yes |
 volumeAttributes.share | NFS share path | `/` |  Yes  |
-
+volumeAttributes.mountPermissions | mounted folder permissions. The default is `0777` |  | No |
 
 ### Tips
 #### provide `mountOptions` for `DeleteVolume`

pkg/nfs/controllerserver.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
@@ -78,8 +79,32 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
+	mountPermissions := cs.Driver.mountPermissions
 	reqCapacity := req.GetCapacityRange().GetRequiredBytes()
-	nfsVol, err := cs.newNFSVolume(name, reqCapacity, req.GetParameters())
+	parameters := req.GetParameters()
+	if parameters == nil {
+		parameters = make(map[string]string)
+	}
+	// validate parameters (case-insensitive)
+	for k, v := range parameters {
+		switch strings.ToLower(k) {
+		case paramServer:
+			// no op
+		case paramShare:
+			// no op
+		case mountPermissionsField:
+			if v != "" {
+				var err error
+				if mountPermissions, err = strconv.ParseUint(v, 8, 32); err != nil {
+					return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid mountPermissions %s in storage class", v))
+				}
+			}
+		default:
+			return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid parameter %q in storage class", k))
+		}
+	}
+
+	nfsVol, err := cs.newNFSVolume(name, reqCapacity, parameters)
 	if err != nil {
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
@@ -89,7 +114,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		volCap = req.GetVolumeCapabilities()[0]
 	}
 	// Mount nfs base share so we can create a subdirectory
-	if err = cs.internalMount(ctx, nfsVol, volCap); err != nil {
+	if err = cs.internalMount(ctx, nfsVol, parameters, volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err.Error())
 	}
 	defer func() {
@@ -98,7 +123,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		}
 	}()
 
-	fileMode := os.FileMode(cs.Driver.mountPermissions)
+	fileMode := os.FileMode(mountPermissions)
 	// Create subdirectory under base-dir
 	internalVolumePath := cs.getInternalVolumePath(nfsVol)
 	if err = os.Mkdir(internalVolumePath, fileMode); err != nil && !os.IsExist(err) {
@@ -108,7 +133,16 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	if err = os.Chmod(internalVolumePath, fileMode); err != nil {
 		klog.Warningf("failed to chmod subdirectory: %v", err.Error())
 	}
-	return &csi.CreateVolumeResponse{Volume: cs.nfsVolToCSI(nfsVol)}, nil
+
+	parameters[paramServer] = nfsVol.server
+	parameters[paramShare] = cs.getVolumeSharePath(nfsVol)
+	return &csi.CreateVolumeResponse{
+		Volume: &csi.Volume{
+			VolumeId:      nfsVol.id,
+			CapacityBytes: 0, // by setting it to zero, Provisioner will use PVC requested size as PV size
+			VolumeContext: parameters,
+		},
+	}, nil
 }
 
 // DeleteVolume delete a volume
@@ -138,7 +172,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 	}
 
 	// mount nfs base share so we can delete the subdirectory
-	if err = cs.internalMount(ctx, nfsVol, volCap); err != nil {
+	if err = cs.internalMount(ctx, nfsVol, nil, volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err.Error())
 	}
 	defer func() {
@@ -254,7 +288,7 @@ func (cs *ControllerServer) validateVolumeCapability(c *csi.VolumeCapability) er
 }
 
 // Mount nfs server at base-dir
-func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume, volCap *csi.VolumeCapability) error {
+func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume, volumeContext map[string]string, volCap *csi.VolumeCapability) error {
 	sharePath := filepath.Join(string(filepath.Separator) + vol.baseDir)
 	targetPath := cs.getInternalMountPath(vol)
 
@@ -266,13 +300,16 @@ func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume, v
 		}
 	}
 
+	if volumeContext == nil {
+		volumeContext = make(map[string]string)
+	}
+	volumeContext[paramServer] = vol.server
+	volumeContext[paramShare] = sharePath
+
 	klog.V(2).Infof("internally mounting %v:%v at %v", vol.server, sharePath, targetPath)
 	_, err := cs.Driver.ns.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{
-		TargetPath: targetPath,
-		VolumeContext: map[string]string{
-			paramServer: vol.server,
-			paramShare:  sharePath,
-		},
+		TargetPath:       targetPath,
+		VolumeContext:    volumeContext,
 		VolumeCapability: volCap,
 		VolumeId:         vol.id,
 	})
@@ -303,8 +340,6 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str
 			server = v
 		case paramShare:
 			baseDir = v
-		default:
-			return nil, fmt.Errorf("invalid parameter %q", k)
 		}
 	}
 
@@ -347,18 +382,6 @@ func (cs *ControllerServer) getVolumeSharePath(vol *nfsVolume) string {
 	return filepath.Join(string(filepath.Separator), vol.baseDir, vol.subDir)
 }
 
-// Convert into nfsVolume into a csi.Volume
-func (cs *ControllerServer) nfsVolToCSI(vol *nfsVolume) *csi.Volume {
-	return &csi.Volume{
-		CapacityBytes: 0, // by setting it to zero, Provisioner will use PVC requested size as PV size
-		VolumeId:      vol.id,
-		VolumeContext: map[string]string{
-			paramServer: vol.server,
-			paramShare:  cs.getVolumeSharePath(vol),
-		},
-	}
-}
-
 // Given a nfsVolume, return a CSI volume id
 func (cs *ControllerServer) getVolumeIDFromNfsVol(vol *nfsVolume) string {
 	idElements := make([]string, totalIDElements)

pkg/nfs/controllerserver_test.go

@@ -100,16 +100,18 @@ func TestCreateVolume(t *testing.T) {
 					},
 				},
 				Parameters: map[string]string{
-					paramServer: testServer,
-					paramShare:  testBaseDir,
+					paramServer:           testServer,
+					paramShare:            testBaseDir,
+					mountPermissionsField: "0750",
 				},
 			},
 			resp: &csi.CreateVolumeResponse{
 				Volume: &csi.Volume{
 					VolumeId: newTestVolumeID,
 					VolumeContext: map[string]string{
-						paramServer: testServer,
-						paramShare:  testShare,
+						paramServer:           testServer,
+						paramShare:            testShare,
+						mountPermissionsField: "0750",
 					},
 				},
 			},
@@ -201,6 +203,28 @@ func TestCreateVolume(t *testing.T) {
 			},
 			expectErr: true,
 		},
+		{
+			name: "[Error] invalid mountPermissions",
+			req: &csi.CreateVolumeRequest{
+				Name: testCSIVolume,
+				VolumeCapabilities: []*csi.VolumeCapability{
+					{
+						AccessType: &csi.VolumeCapability_Mount{
+							Mount: &csi.VolumeCapability_MountVolume{},
+						},
+						AccessMode: &csi.VolumeCapability_AccessMode{
+							Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER,
+						},
+					},
+				},
+				Parameters: map[string]string{
+					paramServer:           testServer,
+					paramShare:            testBaseDir,
+					mountPermissionsField: "07ab",
+				},
+			},
+			expectErr: true,
+		},
 	}
 
 	for _, test := range cases {

pkg/nfs/nfs.go

@@ -55,8 +55,9 @@ const (
 	// The base directory must be a direct child of the root directory.
 	// The root directory is omitted from the string, for example:
 	//     "base" instead of "/base"
-	paramShare        = "share"
-	mountOptionsField = "mountoptions"
+	paramShare            = "share"
+	mountOptionsField     = "mountoptions"
+	mountPermissionsField = "mountpermissions"
 )
 
 func NewDriver(options *DriverOptions) *Driver {

pkg/nfs/nodeserver.go

@@ -19,6 +19,7 @@ package nfs
 import (
 	"fmt"
 	"os"
+	"strconv"
 	"strings"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
@@ -56,6 +57,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	}
 
 	var server, baseDir string
+	mountPermissions := ns.Driver.mountPermissions
 	for k, v := range req.GetVolumeContext() {
 		switch strings.ToLower(k) {
 		case paramServer:
@@ -66,6 +68,13 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 			if v != "" {
 				mountOptions = append(mountOptions, v)
 			}
+		case mountPermissionsField:
+			if v != "" {
+				var err error
+				if mountPermissions, err = strconv.ParseUint(v, 8, 32); err != nil {
+					return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid mountPermissions %s", v))
+				}
+			}
 		}
 	}
 
@@ -80,7 +89,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath)
 	if err != nil {
 		if os.IsNotExist(err) {
-			if err := os.MkdirAll(targetPath, os.FileMode(ns.Driver.mountPermissions)); err != nil {
+			if err := os.MkdirAll(targetPath, os.FileMode(mountPermissions)); err != nil {
 				return nil, status.Error(codes.Internal, err.Error())
 			}
 			notMnt = true
@@ -104,8 +113,8 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
-	klog.V(2).Infof("volumeID(%v): mount targetPath(%s) with permissions(0%o)", volumeID, targetPath, ns.Driver.mountPermissions)
-	if err := os.Chmod(targetPath, os.FileMode(ns.Driver.mountPermissions)); err != nil {
+	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())
 	}
 	return &csi.NodePublishVolumeResponse{}, nil

pkg/nfs/nodeserver_test.go

@@ -41,8 +41,15 @@ func TestNodePublishVolume(t *testing.T) {
 	}
 
 	params := map[string]string{
-		"server": "server",
-		"share":  "share",
+		"server":              "server",
+		"share":               "share",
+		mountPermissionsField: "0755",
+	}
+
+	invalidParams := map[string]string{
+		"server":              "server",
+		"share":               "share",
+		mountPermissionsField: "07ab",
 	}
 
 	volumeCap := csi.VolumeCapability_AccessMode{Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER}
@@ -112,6 +119,16 @@ func TestNodePublishVolume(t *testing.T) {
 				Readonly:         true},
 			expectedErr: nil,
 		},
+		{
+			desc: "[Error] invalid mountPermissions",
+			req: csi.NodePublishVolumeRequest{
+				VolumeContext:    invalidParams,
+				VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap},
+				VolumeId:         "vol_1",
+				TargetPath:       targetTest,
+				Readonly:         true},
+			expectedErr: status.Error(codes.InvalidArgument, "invalid mountPermissions 07ab"),
+		},
 	}
 
 	// setup

test/e2e/e2e_suite_test.go

@@ -50,6 +50,7 @@ var (
 		"share":  nfsShare,
 		"csi.storage.k8s.io/provisioner-secret-name":      "mount-options",
 		"csi.storage.k8s.io/provisioner-secret-namespace": "default",
+		"mountPermissions": "0755",
 	}
 	controllerServer *nfs.ControllerServer
 )