diff --git a/docker-files/Dockerfile.centos b/docker-files/Dockerfile.centos index e7bf05ae..37659682 100644 --- a/docker-files/Dockerfile.centos +++ b/docker-files/Dockerfile.centos @@ -14,7 +14,7 @@ LABEL vendor="Dell Inc." \ COPY licenses /licenses # dependencies, following by cleaning the cache -RUN echo "%_netsharedpath /sys:/proc" >> /etc/rpm/macros.dist && yum update -y && yum install -y e2fsprogs xfsprogs nfs-utils which device-mapper-multipath \ +RUN echo "%_netsharedpath /sys:/proc" >> /etc/rpm/macros.dist && yum update -y && yum install -y e2fsprogs xfsprogs nfs-utils nfs4-acl-tools acl which device-mapper-multipath \ && \ yum clean all \ && \ diff --git a/docker-files/Dockerfile.ubi b/docker-files/Dockerfile.ubi index bc6bb48a..d60704ff 100644 --- a/docker-files/Dockerfile.ubi +++ b/docker-files/Dockerfile.ubi @@ -15,7 +15,7 @@ COPY licenses /licenses # dependencies, following by cleaning the cache RUN yum update -y \ && \ - yum install -y e2fsprogs xfsprogs nfs-utils which device-mapper-multipath \ + yum install -y e2fsprogs xfsprogs nfs-utils nfs4-acl-tools acl which device-mapper-multipath \ && \ yum clean all \ && \ diff --git a/docker-files/Dockerfile.ubi.alt b/docker-files/Dockerfile.ubi.alt index 79487196..db31cc3d 100644 --- a/docker-files/Dockerfile.ubi.alt +++ b/docker-files/Dockerfile.ubi.alt @@ -25,6 +25,8 @@ RUN yum -y update && \ e2fsprogs.x86_64 \ xfsprogs.x86_64 \ nfs-utils.x86_64 \ + nfs4-acl-tools \ + acl \ which \ device-mapper-multipath \ && \ diff --git a/docker-files/Dockerfile.ubi.min b/docker-files/Dockerfile.ubi.min index 1ca53fd6..87b16c8b 100644 --- a/docker-files/Dockerfile.ubi.min +++ b/docker-files/Dockerfile.ubi.min @@ -19,6 +19,8 @@ RUN microdnf update -y \ e2fsprogs \ xfsprogs \ nfs-utils \ + nfs4-acl-tools \ + acl \ which \ device-mapper-multipath \ && \ diff --git a/go.mod b/go.mod index 12fbe464..8b5f54ee 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/dell/gocsi v1.5.0 github.com/dell/gofsutil v1.7.1-0.20220204052137-9928a2dc48d8 github.com/dell/goiscsi v1.2.0 - github.com/dell/gopowerstore v1.6.1-0.20211223095101-c47391fc979f + github.com/dell/gopowerstore v1.6.1-0.20220214164327-12bc4206c198 github.com/fsnotify/fsnotify v1.4.9 github.com/golang/mock v1.4.4 github.com/golang/protobuf v1.5.2 diff --git a/go.sum b/go.sum index 1058bd97..979b6c7b 100644 --- a/go.sum +++ b/go.sum @@ -83,8 +83,8 @@ github.com/dell/gofsutil v1.7.1-0.20220204052137-9928a2dc48d8/go.mod h1:0tAefmK/ github.com/dell/goiscsi v1.1.0/go.mod h1:MfuMjbKWsh/MOb0VDW20C+LFYRIOfWKGiAxWkeM5TKo= github.com/dell/goiscsi v1.2.0 h1:ocQs4pz2Fw2vr73RVAQBKwpN468SK4TZHPLhU7/FB9A= github.com/dell/goiscsi v1.2.0/go.mod h1:MfuMjbKWsh/MOb0VDW20C+LFYRIOfWKGiAxWkeM5TKo= -github.com/dell/gopowerstore v1.6.1-0.20211223095101-c47391fc979f h1:F+aAuMlcTUV/F6eWYoM94+I6MR94gVE3g8ZY65DGzbE= -github.com/dell/gopowerstore v1.6.1-0.20211223095101-c47391fc979f/go.mod h1:0ziQJ1iuZYDV+P53ua+VeH+rIylYT9WNjGSI/7aPly0= +github.com/dell/gopowerstore v1.6.1-0.20220214164327-12bc4206c198 h1:yOIMDcJ9fjuBHytDf32rjowoL8WM1VPuB7Yh8TYG1eU= +github.com/dell/gopowerstore v1.6.1-0.20220214164327-12bc4206c198/go.mod h1:0ziQJ1iuZYDV+P53ua+VeH+rIylYT9WNjGSI/7aPly0= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= diff --git a/helm/csi-powerstore/templates/controller.yaml b/helm/csi-powerstore/templates/controller.yaml index 543c350f..3cd85b58 100644 --- a/helm/csi-powerstore/templates/controller.yaml +++ b/helm/csi-powerstore/templates/controller.yaml @@ -272,6 +272,8 @@ spec: value: {{ .Values.driverName }} - name: X_CSI_POWERSTORE_EXTERNAL_ACCESS value: {{ .Values.externalAccess }} + - name: X_CSI_NFS_ACLS + value: "{{ .Values.nfsAcls }}" - name: X_CSI_POWERSTORE_CONFIG_PATH value: /powerstore-config/config - name: X_CSI_POWERSTORE_CONFIG_PARAMS_PATH diff --git a/helm/csi-powerstore/values.yaml b/helm/csi-powerstore/values.yaml index 54271607..5ce73426 100644 --- a/helm/csi-powerstore/values.yaml +++ b/helm/csi-powerstore/values.yaml @@ -34,6 +34,21 @@ externalAccess: # Default value: None imagePullPolicy: IfNotPresent +# nfsAcls: enables setting permissions on NFS mount directory +# This value acts as default value for NFS ACL (nfsAcls), if not specified for an array config in secret +# Permissions can be specified in two formats: +# 1) Unix mode (NFSv3) +# 2) NFSv4 ACLs (NFSv4) +# NFSv4 ACLs are supported on NFSv4 share only. +# Allowed values: +# 1) Unix mode: valid octal mode number +# Examples: "0777", "777", "0755" +# 2) NFSv4 acls: valid NFSv4 acls, seperated by comma +# Examples: "A::OWNER@:RWX,A::GROUP@:RWX", "A::OWNER@:rxtncy" +# Optional: true +# Default value: "0777" +nfsAcls: "0777" + # controller: configure controller specific parameters controller: # controllerCount: defines the number of csi-powerscale controller pods to deploy to diff --git a/mocks/NFSv4ACLsInterface.go b/mocks/NFSv4ACLsInterface.go new file mode 100644 index 00000000..32be3ebe --- /dev/null +++ b/mocks/NFSv4ACLsInterface.go @@ -0,0 +1,24 @@ +// Code generated by mockery v1.0.0. DO NOT EDIT. + +package mocks + +import mock "github.com/stretchr/testify/mock" + +// NFSv4ACLsInterface is an autogenerated mock type for the NFSv4ACLsInterface type +type NFSv4ACLsInterface struct { + mock.Mock +} + +// SetNfsv4Acls provides a mock function with given fields: acls, dir +func (_m *NFSv4ACLsInterface) SetNfsv4Acls(acls string, dir string) error { + ret := _m.Called(acls, dir) + + var r0 error + if rf, ok := ret.Get(0).(func(string, string) error); ok { + r0 = rf(acls, dir) + } else { + r0 = ret.Error(0) + } + + return r0 +} diff --git a/pkg/array/array.go b/pkg/array/array.go index 9ba6897d..454e9bf7 100644 --- a/pkg/array/array.go +++ b/pkg/array/array.go @@ -116,6 +116,7 @@ type PowerStoreArray struct { BlockProtocol common.TransportType `yaml:"blockProtocol"` Insecure bool `yaml:"skipCertificateValidation"` IsDefault bool `yaml:"isDefault"` + NfsAcls string `yaml:"nfsAcls"` Client gopowerstore.Client IP string diff --git a/pkg/common/common.go b/pkg/common/common.go index 8ae1ee01..d761da4b 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -72,6 +72,10 @@ const ( KeyArrayVolumeName = "Name" // KeyProtocol key value to check in request parameters for volume name KeyProtocol = "Protocol" + // KeyNfsACL key value to specify NFS ACLs for NFS volume + KeyNfsACL = "nfsAcls" + // KeyNasName key value to specify NAS server name + KeyNasName = "nasName" // VerboseName longer description of the driver VerboseName = "CSI Driver for Dell EMC PowerStore" // FcTransport indicates that FC is chosen as a SCSI transport protocol diff --git a/pkg/common/envvars.go b/pkg/common/envvars.go index 19c6d8e4..5b84b36b 100644 --- a/pkg/common/envvars.go +++ b/pkg/common/envvars.go @@ -82,4 +82,7 @@ const ( // EnvIsHealthMonitorEnabled specifies if health monitor is enabled. EnvIsHealthMonitorEnabled = "X_CSI_HEALTH_MONITOR_ENABLED" + + // EnvNfsAcls specifies acls to be set on NFS mount directory + EnvNfsAcls = "X_CSI_NFS_ACLS" ) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 97801ac6..3ab01a75 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -58,6 +58,7 @@ type Service struct { Fs fs.Interface externalAccess string + nfsAcls string array.Locker @@ -85,6 +86,13 @@ func (s *Service) Init() error { s.isHealthMonitorEnabled, _ = strconv.ParseBool(isHealthMonitorEnabled) } + s.nfsAcls = "" + if nfsAcls, ok := csictx.LookupEnv(ctx, common.EnvNfsAcls); ok { + if nfsAcls != "" { + s.nfsAcls = nfsAcls + } + } + return nil } @@ -132,6 +140,7 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest var creator VolumeCreator var protocol string + nfsAcls := s.nfsAcls if useNFS { protocol = "nfs" nasParamsName, ok := params[KeyNasName] @@ -144,6 +153,12 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest nasName: arr.GetNasName(), } } + + if params[common.KeyNfsACL] != "" { + nfsAcls = params[common.KeyNfsACL] // Storage class takes precedence + } else if arr.NfsAcls != "" { + nfsAcls = arr.NfsAcls // Secrets next + } } else { protocol = "scsi" creator = &SCSICreator{} @@ -308,6 +323,12 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest volumeResponse.VolumeContext[common.KeyArrayID] = arr.GetGlobalID() volumeResponse.VolumeContext[common.KeyArrayVolumeName] = req.Name volumeResponse.VolumeContext[common.KeyProtocol] = protocol + + if useNFS { + volumeResponse.VolumeContext[common.KeyNfsACL] = nfsAcls + volumeResponse.VolumeContext[common.KeyNasName] = arr.GetNasName() + } + volumeResponse.VolumeId = volumeResponse.VolumeId + "/" + arr.GetGlobalID() + "/" + protocol volumeResponse.AccessibleTopology = topology return &csi.CreateVolumeResponse{ diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index a2905b32..bc4c1edf 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -75,6 +75,8 @@ const ( validReplicationPrefix = "/" + controller.KeyReplicationEnabled validVolumeGroupName = "VGName" validRemoteSystemGlobalID = "PS111111111111" + validNfsAcls = "A::OWNER@:RWX" + validNfsServerID = "24aefac2-a796-47dc-886a-c73ff8c1a671" ) var ( @@ -119,6 +121,7 @@ func setVariables() { arrays[secondValidID] = second csictx.Setenv(context.Background(), common.EnvReplicationPrefix, "replication.storage.dell.com") + csictx.Setenv(context.Background(), common.EnvNfsAcls, "A::OWNER@:RWX") ctrlSvc = &controller.Service{Fs: fsMock} ctrlSvc.SetArrays(arrays) @@ -388,6 +391,126 @@ var _ = Describe("CSIControllerService", func() { common.KeyArrayVolumeName: "my-vol", common.KeyProtocol: "nfs", common.KeyArrayID: secondValidID, + common.KeyNfsACL: "A::OWNER@:RWX", + common.KeyNasName: validNasName, + }, + }, + })) + }) + }) + + When("creating nfs volume with NFS acls in array config and storage class", func() { + It("should successfully create nfs volume with storage class NFS acls in volume response", func() { + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID}, nil) + clientMock.On("CreateFS", mock.Anything, mock.Anything).Return(gopowerstore.CreateResponse{ID: validBaseVolID}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, nil) + + ctrlSvc.Arrays()[secondValidID].NfsAcls = "A::GROUP@:RWX" + + req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) + req.Parameters[common.KeyNfsACL] = "0777" + req.Parameters[common.KeyArrayID] = secondValidID + + res, err := ctrlSvc.CreateVolume(context.Background(), req) + Expect(err).To(BeNil()) + Expect(res).To(Equal(&csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: validVolSize, + VolumeId: filepath.Join(validBaseVolID, secondValidID, "nfs"), + VolumeContext: map[string]string{ + common.KeyArrayVolumeName: "my-vol", + common.KeyProtocol: "nfs", + common.KeyArrayID: secondValidID, + common.KeyNfsACL: "0777", + common.KeyNasName: validNasName, + }, + }, + })) + }) + }) + + When("creating nfs volume with NFS acls in array config and not in storage class", func() { + It("should successfully create nfs volume with array config NFS acls in volume response", func() { + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID}, nil) + clientMock.On("CreateFS", mock.Anything, mock.Anything).Return(gopowerstore.CreateResponse{ID: validBaseVolID}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, nil) + + ctrlSvc.Arrays()[secondValidID].NfsAcls = "A::GROUP@:RWX" + + req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) + req.Parameters[common.KeyArrayID] = secondValidID + + res, err := ctrlSvc.CreateVolume(context.Background(), req) + Expect(err).To(BeNil()) + Expect(res).To(Equal(&csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: validVolSize, + VolumeId: filepath.Join(validBaseVolID, secondValidID, "nfs"), + VolumeContext: map[string]string{ + common.KeyArrayVolumeName: "my-vol", + common.KeyProtocol: "nfs", + common.KeyArrayID: secondValidID, + common.KeyNfsACL: "A::GROUP@:RWX", + common.KeyNasName: validNasName, + }, + }, + })) + }) + }) + + When("creating nfs volume with NFS acls not in array config and not in storage class", func() { + It("should successfully create nfs volume with default NFS acls in volume response", func() { + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID}, nil) + clientMock.On("CreateFS", mock.Anything, mock.Anything).Return(gopowerstore.CreateResponse{ID: validBaseVolID}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, nil) + + req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) + req.Parameters[common.KeyArrayID] = secondValidID + + res, err := ctrlSvc.CreateVolume(context.Background(), req) + Expect(err).To(BeNil()) + Expect(res).To(Equal(&csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: validVolSize, + VolumeId: filepath.Join(validBaseVolID, secondValidID, "nfs"), + VolumeContext: map[string]string{ + common.KeyArrayVolumeName: "my-vol", + common.KeyProtocol: "nfs", + common.KeyArrayID: secondValidID, + common.KeyNfsACL: "A::OWNER@:RWX", + common.KeyNasName: validNasName, + }, + }, + })) + }) + }) + + When("creating nfs volume with NFS acls in not in secrets & default", func() { + It("should successfully create nfs volume with empty NFS acls in volume response", func() { + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID}, nil) + clientMock.On("CreateFS", mock.Anything, mock.Anything).Return(gopowerstore.CreateResponse{ID: validBaseVolID}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, nil) + + req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) + req.Parameters[common.KeyArrayID] = secondValidID + + ctrlSvc.Arrays()[secondValidID].NfsAcls = "" + csictx.Setenv(context.Background(), common.EnvNfsAcls, "") + + _ = ctrlSvc.Init() + + res, err := ctrlSvc.CreateVolume(context.Background(), req) + Expect(err).To(BeNil()) + Expect(res).To(Equal(&csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: validVolSize, + VolumeId: filepath.Join(validBaseVolID, secondValidID, "nfs"), + VolumeContext: map[string]string{ + common.KeyArrayVolumeName: "my-vol", + common.KeyProtocol: "nfs", + common.KeyArrayID: secondValidID, + common.KeyNfsACL: "", + common.KeyNasName: validNasName, }, }, })) @@ -457,6 +580,8 @@ var _ = Describe("CSIControllerService", func() { common.KeyArrayVolumeName: "my-vol", common.KeyProtocol: "nfs", common.KeyArrayID: secondValidID, + common.KeyNfsACL: "A::OWNER@:RWX", + common.KeyNasName: validNasName, }, }, })) @@ -1533,6 +1658,7 @@ var _ = Describe("CSIControllerService", func() { "ExportID": nfsID, "allowRoot": "", "HostIP": "127.0.0.1", + "nfsAcls": "", }, })) }) @@ -1654,6 +1780,7 @@ var _ = Describe("CSIControllerService", func() { "allowRoot": "", "HostIP": "127.0.0.1", "NatIP": "10.0.0.1", + "nfsAcls": "", }, })) }) diff --git a/pkg/controller/publisher.go b/pkg/controller/publisher.go index 5c5168a6..61946dcb 100644 --- a/pkg/controller/publisher.go +++ b/pkg/controller/publisher.go @@ -267,6 +267,7 @@ func (n *NfsPublisher) Publish(ctx context.Context, req *csi.ControllerPublishVo } publishContext[common.KeyExportID] = export.ID publishContext[common.KeyAllowRoot] = req.VolumeContext[common.KeyAllowRoot] + publishContext[common.KeyNfsACL] = req.VolumeContext[common.KeyNfsACL] return &csi.ControllerPublishVolumeResponse{PublishContext: publishContext}, nil } diff --git a/pkg/node/acl.go b/pkg/node/acl.go new file mode 100644 index 00000000..34703bd6 --- /dev/null +++ b/pkg/node/acl.go @@ -0,0 +1,109 @@ +/* + * + * Copyright © 2022 Dell Inc. or its subsidiaries. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package node + +import ( + "context" + "fmt" + "os/exec" + "regexp" + "strings" + + "github.com/dell/gopowerstore" + log "github.com/sirupsen/logrus" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// NFSv4ACLsInterface contains method definition to set NFSv4 ACLs +type NFSv4ACLsInterface interface { + // SetNfsv4Acls sets NFSv4 ACLs + SetNfsv4Acls(acls string, dir string) error +} + +// NFSv4ACLs implements setting NFSv4 ACLs +type NFSv4ACLs struct{} + +func validateAndSetACLs(ctx context.Context, s NFSv4ACLsInterface, nasName string, client gopowerstore.Client, acls string, dir string) (bool, error) { + aclsConfigured := false + if nfsv4ACLs(acls) { + if isNfsv4Enabled(ctx, client, nasName) { + if err := s.SetNfsv4Acls(acls, dir); err != nil { + log.Error(fmt.Sprintf("can't assign NFSv4 ACLs to folder %s: %s", dir, err.Error())) + return false, err + } + aclsConfigured = true + } else { + return false, status.Errorf(codes.Internal, "can't assign NFSv4 ACLs to folder %s: NAS server is not NFSv4 enabled", dir) + } + } else { + return false, status.Errorf(codes.Internal, "can't assign ACLs to folder %s: invalid NFSv4 ACL format %s", dir, acls) + } + + return aclsConfigured, nil +} + +func posixMode(acls string) bool { + if matched, _ := regexp.Match(`\d{3,4}`, []byte(acls)); matched { + return true + } + return false +} + +func nfsv4ACLs(acls string) bool { + aclsList := strings.Split(acls, ",") + for _, acl := range aclsList { + matched, err := regexp.Match(`([ADUL]:\w*:[\w.]*[@]*[\w.]*:\w*)`, []byte(acl)) + if !matched || err != nil { + return false + } + } + return true +} + +// SetNfsv4Acls sets NFSv4 ACLS +func (n *NFSv4ACLs) SetNfsv4Acls(acls string, dir string) error { + command := []string{"nfs4_setfacl", "-s", acls, dir} + log.Infof("NFSv4 ACL command: %s \n", strings.Join(command, " ")) + // arguments for exec.Command() are validated in caller + cmd := exec.Command(command[0], command[1:]...) // #nosec G204 + outStr, err := cmd.Output() + log.Infof("NFSv4 ACL output: %s \n", string(outStr)) + return err +} + +func isNfsv4Enabled(ctx context.Context, client gopowerstore.Client, nasName string) bool { + nfsv4Enabled := false + nas, err := gopowerstore.Client.GetNASByName(client, ctx, nasName) + if err == nil { + nfsServer, err := gopowerstore.Client.GetNfsServer(client, ctx, nas.NfsServers[0].Id) + if err == nil { + if nfsServer.IsNFSv4Enabled { + nfsv4Enabled = true + } else { + log.Error(fmt.Sprintf("NFS v4 not enabled on NAS server: %s\n", nasName)) + } + } else { + log.Error(fmt.Sprintf("can't fetch nfs server with id %s: %s", nas.NfsServers[0].Id, err.Error())) + } + } else { + log.Error(fmt.Sprintf("can't determine nfsv4 enabled: %s", err.Error())) + } + return nfsv4Enabled +} diff --git a/pkg/node/acl_test.go b/pkg/node/acl_test.go new file mode 100644 index 00000000..b66ecbd3 --- /dev/null +++ b/pkg/node/acl_test.go @@ -0,0 +1,186 @@ +package node + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/dell/csi-powerstore/mocks" + "github.com/dell/gopowerstore" + gopowerstoremock "github.com/dell/gopowerstore/mocks" + "github.com/stretchr/testify/mock" +) + +var nfsv4ACLsMock *mocks.NFSv4ACLsInterface + +func TestPosixMode_Success(t *testing.T) { + isPosixMode := posixMode("0755") + expected := true + if isPosixMode != expected { + t.Errorf(fmt.Sprintf("expected: %v, actual: %v", expected, isPosixMode)) + } +} + +func TestPosixMode_Fail(t *testing.T) { + isPosixMode := posixMode("abcd") + expected := false + if isPosixMode != expected { + t.Errorf(fmt.Sprintf("expected: %v, actual: %v", expected, isPosixMode)) + } +} + +func TestNfsv4Acl_Success(t *testing.T) { + isNfsv4ACLs := nfsv4ACLs("A::OWNER@:RWX") + expected := true + if isNfsv4ACLs != expected { + t.Errorf(fmt.Sprintf("expected: %v, actual: %v", expected, isNfsv4ACLs)) + } +} + +func TestNfsv4Acl_Fail(t *testing.T) { + isNfsv4ACLs := nfsv4ACLs("abcd") + expected := false + if isNfsv4ACLs != expected { + t.Errorf(fmt.Sprintf("expected: %v, actual: %v", expected, isNfsv4ACLs)) + } +} + +func TestNfsv4NasServer_Success(t *testing.T) { + clientMock = new(gopowerstoremock.Client) + + nfsServers := []gopowerstore.NFSServerInstance{ + { + Id: validNfsServerID, + IsNFSv4Enabled: true, + }, + } + + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID, NfsServers: nfsServers}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, nil) + + isNFSv4Enabled := isNfsv4Enabled(context.Background(), clientMock, validNasName) + expected := true + if isNFSv4Enabled != expected { + t.Errorf(fmt.Sprintf("expected: %v, actual: %v", expected, isNFSv4Enabled)) + } +} + +func TestNfsv4NasServer_Err_GetNASByName(t *testing.T) { + clientMock = new(gopowerstoremock.Client) + + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID}, errors.New("GetNASByName_fail")) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, nil) + + isNFSv4Enabled := isNfsv4Enabled(context.Background(), clientMock, validNasName) + expected := false + if isNFSv4Enabled != expected { + t.Errorf(fmt.Sprintf("expected: %v, actual: %v", expected, isNFSv4Enabled)) + } +} + +func TestNfsv4NasServer_Err_GetNfsServer(t *testing.T) { + clientMock = new(gopowerstoremock.Client) + + nfsServers := []gopowerstore.NFSServerInstance{ + { + Id: validNfsServerID, + IsNFSv4Enabled: true, + }, + } + + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID, NfsServers: nfsServers}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, errors.New("GetNfsServer_fail")) + + isNFSv4Enabled := isNfsv4Enabled(context.Background(), clientMock, validNasName) + expected := false + if isNFSv4Enabled != expected { + t.Errorf(fmt.Sprintf("expected: %v, actual: %v", expected, isNFSv4Enabled)) + } +} + +func TestNfsv4NasServer_Fail(t *testing.T) { + clientMock = new(gopowerstoremock.Client) + + nfsServers := []gopowerstore.NFSServerInstance{ + { + Id: validNfsServerID, + IsNFSv4Enabled: true, + }, + } + + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID, NfsServers: nfsServers}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: false}, nil) + + isNFSv4Enabled := isNfsv4Enabled(context.Background(), clientMock, validNasName) + expected := false + if isNFSv4Enabled != expected { + t.Errorf(fmt.Sprintf("expected: %v, actual: %v", expected, isNFSv4Enabled)) + } +} + +func TestValidateAndSetNfsACLs_Success_nfsv4Acls(t *testing.T) { + clientMock = new(gopowerstoremock.Client) + nfsv4ACLsMock = new(mocks.NFSv4ACLsInterface) + + nfsServers := []gopowerstore.NFSServerInstance{ + { + Id: validNfsServerID, + IsNFSv4Enabled: true, + }, + } + + nfsv4ACLsMock.On("SetNfsv4Acls", mock.Anything, mock.Anything).Return(nil) + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID, NfsServers: nfsServers}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, nil) + + aclConfigured, err := validateAndSetACLs(context.Background(), nfsv4ACLsMock, validNasName, clientMock, "A::OWNER@:RWX", "dir2") + + if err != nil || aclConfigured == false { + t.Errorf(fmt.Sprintf("expected: true, actual: %v err: %s", aclConfigured, err.Error())) + } +} + +func TestValidateAndSetNfsACLs_Fail_InvalidAcls(t *testing.T) { + clientMock = new(gopowerstoremock.Client) + nfsv4ACLsMock = new(mocks.NFSv4ACLsInterface) + + nfsServers := []gopowerstore.NFSServerInstance{ + { + Id: validNfsServerID, + IsNFSv4Enabled: true, + }, + } + + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID, NfsServers: nfsServers}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, nil) + nfsv4ACLsMock.On("setNfsv4Acls", mock.Anything, mock.Anything).Return(nil) + + aclConfigured, err := validateAndSetACLs(context.Background(), nfsv4ACLsMock, validNasName, clientMock, "abcd", "dir1") + + if err == nil || aclConfigured != false { + t.Errorf(fmt.Sprintf("expected: false, actual: %v err: %s", aclConfigured, err.Error())) + } +} + +func TestValidateAndSetNfsACLs_Fail_GetNfsServerFail(t *testing.T) { + clientMock = new(gopowerstoremock.Client) + nfsv4ACLsMock = new(mocks.NFSv4ACLsInterface) + + nfsServers := []gopowerstore.NFSServerInstance{ + { + Id: validNfsServerID, + IsNFSv4Enabled: true, + }, + } + + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID, NfsServers: nfsServers}, nil) + clientMock.On("GetNfsServer", mock.Anything, mock.Anything).Return(gopowerstore.NFSServerInstance{Id: validNfsServerID, IsNFSv4Enabled: true}, errors.New("GetNfsServer_fail")) + nfsv4ACLsMock.On("setNfsv4Acls", mock.Anything, mock.Anything).Return(nil) + + aclConfigured, err := validateAndSetACLs(context.Background(), nfsv4ACLsMock, validNasName, clientMock, "A::OWNER@:RWX", "dir1") + + if err == nil || aclConfigured != false { + t.Errorf(fmt.Sprintf("expected: false, actual: %v err: %s", aclConfigured, err.Error())) + } +} diff --git a/pkg/node/node_test.go b/pkg/node/node_test.go index eb22818f..d977b8a3 100644 --- a/pkg/node/node_test.go +++ b/pkg/node/node_test.go @@ -80,6 +80,8 @@ const ( firstValidIP = "gid1" secondValidIP = "gid2" validNasName = "my-nas-name" + validNasID = "e8f4c5f8-c2fc-4df4-bd99-c292c12b55be" + validNfsServerID = "e8f4c5f8-c2fc-4dd2-bd99-c292c12b55be" validEphemeralName = "ephemeral-39bb1b5f-5624-490d-9ece-18f7b28a904e/gid1/scsi" ephemerallockfile = "/var/lib/kubelet/plugins/kubernetes.io/csi/pv/ephemeral/39bb1b5f-5624-490d-9ece-18f7b28a904e/gid1/scsi/id" ) diff --git a/pkg/node/stager.go b/pkg/node/stager.go index da02eaa5..f4ec9ae7 100644 --- a/pkg/node/stager.go +++ b/pkg/node/stager.go @@ -128,6 +128,7 @@ func (n *NFSStager) Stage(ctx context.Context, req *csi.NodeStageVolumeRequest, exportID := req.PublishContext[common.KeyExportID] nfsExport := req.PublishContext[common.KeyNfsExportPath] allowRoot := req.PublishContext[common.KeyAllowRoot] + nasName := req.PublishContext[common.KeyNasName] natIP := "" if ip, ok := req.PublishContext[common.KeyNatIP]; ok { @@ -141,6 +142,8 @@ func (n *NFSStager) Stage(ctx context.Context, req *csi.NodeStageVolumeRequest, logFields["ExportID"] = exportID logFields["HostIP"] = hostIP logFields["NatIP"] = natIP + logFields["NFSv4ACLs"] = req.PublishContext[common.KeyNfsACL] + logFields["NasName"] = nasName ctx = common.SetLogFields(ctx, logFields) found, err := isReadyToPublishNFS(ctx, stagingPath, fs) @@ -165,15 +168,35 @@ func (n *NFSStager) Stage(ctx context.Context, req *csi.NodeStageVolumeRequest, } // Create folder with 1777 in nfs share so every user can use it - log.WithFields(logFields).Info("creating common folder") if err := fs.MkdirAll(filepath.Join(stagingPath, commonNfsVolumeFolder), 0750); err != nil { return nil, status.Errorf(codes.Internal, "can't create common folder %s: %s", filepath.Join(stagingPath, "volume"), err.Error()) } - if err := fs.Chmod(filepath.Join(stagingPath, commonNfsVolumeFolder), os.ModeSticky|os.ModePerm); err != nil { - return nil, status.Errorf(codes.Internal, - "can't change permissions of folder %s: %s", filepath.Join(stagingPath, "volume"), err.Error()) + mode := os.ModePerm + acls := req.PublishContext[common.KeyNfsACL] + aclsConfigured := false + if acls != "" { + if posixMode(acls) { + perm, err := strconv.ParseUint(acls, 8, 64) + if err == nil { + mode = os.FileMode(perm) + } else { + log.WithFields(logFields).Warn("can't parse file mode, invalid mode specified. Default mode permissions will be set.") + } + } else { + aclsConfigured, err = validateAndSetACLs(ctx, &NFSv4ACLs{}, nasName, n.array.GetClient(), acls, filepath.Join(stagingPath, commonNfsVolumeFolder)) + if err != nil || !aclsConfigured { + return nil, err + } + } + } + + if !aclsConfigured { + if err := fs.Chmod(filepath.Join(stagingPath, commonNfsVolumeFolder), os.ModeSticky|mode); err != nil { + return nil, status.Errorf(codes.Internal, + "can't change permissions of folder %s: %s", filepath.Join(stagingPath, "volume"), err.Error()) + } } if allowRoot == "false" { diff --git a/samples/secret/secret.yaml b/samples/secret/secret.yaml index 80887277..f6b2d448 100644 --- a/samples/secret/secret.yaml +++ b/samples/secret/secret.yaml @@ -51,6 +51,21 @@ arrays: # Default Value: None nasName: "nas-server" + # nfsAcls: enables setting permissions on NFS mount directory + # This value will be used if a storage class does not have the NFS ACL (nfsAcls) parameter specified + # Permissions can be specified in two formats: + # 1) Unix mode (NFSv3) + # 2) NFSv4 ACLs (NFSv4) + # NFSv4 ACLs are supported on NFSv4 share only. + # Allowed values: + # 1) Unix mode: valid octal mode number + # Examples: "0777", "777", "0755" + # 2) NFSv4 acls: valid NFSv4 acls, seperated by comma + # Examples: "A::OWNER@:RWX,A::GROUP@:RWX", "A::OWNER@:rxtncy" + # Optional: true + # Default value: "0777" + # nfsAcls: "0777" + - endpoint: "https://11.0.0.1/api/rest" globalID: "unique" username: "user" diff --git a/samples/storageclass/powerstore-nfs.yaml b/samples/storageclass/powerstore-nfs.yaml index a8b0fdc6..7ebf46c3 100644 --- a/samples/storageclass/powerstore-nfs.yaml +++ b/samples/storageclass/powerstore-nfs.yaml @@ -34,6 +34,21 @@ parameters: # Default value: false allowRoot: "false" + # nfsAcls: enables setting permissions on NFS mount directory + # This value overrides the NFS ACL (nfsAcls) attribute of corresponding array config in secret, if present + # Permissions can be specified in two formats: + # 1) Unix mode (NFSv3) + # 2) NFSv4 ACLs (NFSv4) + # NFSv4 ACLs are supported on NFSv4 share only. + # Allowed values: + # 1) Unix mode: valid octal mode number + # Examples: "0777", "777", "0755" + # 2) NFSv4 acls: valid NFSv4 acls, seperated by comma + # Examples: "A::OWNER@:RWX,A::GROUP@:RWX", "A::OWNER@:rxtncy" + # Optional: true + # Default value: "0777" + # nfsAcls: "0777" + # reclaimPolicy: PVs that are dynamically created by a StorageClass will have the reclaim policy specified here # Allowed values: # Reclaim: retain the PV after PVC deletion