diff --git a/.github/workflows/snyk.yaml b/.github/workflows/snyk.yaml new file mode 100644 index 000000000..8cb46b687 --- /dev/null +++ b/.github/workflows/snyk.yaml @@ -0,0 +1,30 @@ +--- +name: Security scanning +# yamllint disable-line rule:truthy +on: + schedule: + # Run weekly on every Monday + - cron: '0 0 * * 1' + push: + tags: + - v* + branches: + - release-* + +permissions: + contents: read + +jobs: + security: + if: github.repository == 'ceph/ceph-csi' + runs-on: ubuntu-latest + steps: + - name: checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: run Snyk to check for code vulnerabilities + uses: snyk/actions/golang@master + env: + SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }} diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index 1ea10ba84..203f61f49 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -9,7 +9,7 @@ ## Features -RBD - - Support for configuring read affinity for individuals cluster within the ceph-csi-config ConfigMap in [PR](https://github.com/ceph/ceph-csi/pull/4165) + +- Support for CephFS kernel and fuse mount options per cluster in [PR](https://github.com/ceph/ceph-csi/pull/4245) diff --git a/deploy/csi-config-map-sample.yaml b/deploy/csi-config-map-sample.yaml index 1766d3227..d44f96905 100644 --- a/deploy/csi-config-map-sample.yaml +++ b/deploy/csi-config-map-sample.yaml @@ -24,6 +24,12 @@ kind: ConfigMap # The "cephFS.netNamespaceFilePath" fields are the various network namespace # path for the Ceph cluster identified by the , This will be used # by the CephFS CSI plugin to execute the mount -t in the +# The "cephFS.kernelMountOptions" fields are comma separated mount options +# for `Ceph Kernel client`. Setting this will override the kernelmountoptions +# command line flag. +# The "cephFS.fuseMountOptions" fields are common separated mount options +# for `Ceph FUSE driver`. Setting this will override the fusemountoptions +# command line flag. # network namespace specified by the "cephFS.netNamespaceFilePath". # The "nfs.netNamespaceFilePath" fields are the various network namespace # path for the Ceph cluster identified by the , This will be used @@ -68,6 +74,8 @@ data: "cephFS": { "subvolumeGroup": "" "netNamespaceFilePath": "/plugins/cephfs.csi.ceph.com/net", + "kernelMountOptions": "", + "fuseMountOptions": "" } "nfs": { "netNamespaceFilePath": "/plugins/nfs.csi.ceph.com/net", diff --git a/docs/deploy-cephfs.md b/docs/deploy-cephfs.md index 8428993ad..48043877a 100644 --- a/docs/deploy-cephfs.md +++ b/docs/deploy-cephfs.md @@ -44,8 +44,8 @@ make image-cephcsi | `--timeout` | `3s` | Probe timeout in seconds | | `--clustername` | _empty_ | Cluster name to set on subvolume | | `--forcecephkernelclient` | `false` | Force enabling Ceph Kernel clients for mounting on kernels < 4.17 | -| `--kernelmountoptions` | _empty_ | Comma separated string of mount options accepted by cephfs kernel mounter | -| `--fusemountoptions` | _empty_ | Comma separated string of mount options accepted by ceph-fuse mounter | +| `--kernelmountoptions` | _empty_ | Comma separated string of mount options accepted by cephfs kernel mounter.
`Note: These options will be replaced if kernelMountOptions are defined in the ceph-csi-config ConfigMap for the specific cluster.` | +| `--fusemountoptions` | _empty_ | Comma separated string of mount options accepted by ceph-fuse mounter.
`Note: These options will be replaced if fuseMountOptions are defined in the ceph-csi-config ConfigMap for the specific cluster.` | | `--domainlabels` | _empty_ | Kubernetes node labels to use as CSI domain labels for topology aware provisioning, should be a comma separated value (ex:= "failure-domain/region,failure-domain/zone") | **NOTE:** The parameter `-forcecephkernelclient` enables the Kernel diff --git a/internal/cephfs/nodeserver.go b/internal/cephfs/nodeserver.go index 94a9cb492..5d353cc66 100644 --- a/internal/cephfs/nodeserver.go +++ b/internal/cephfs/nodeserver.go @@ -311,34 +311,11 @@ func (ns *NodeServer) mount( log.DebugLog(ctx, "cephfs: mounting volume %s with %s", volID, mnt.Name()) - var mountOptions []string - if m := volCap.GetMount(); m != nil { - mountOptions = m.GetMountFlags() - } + err = ns.setMountOptions(mnt, volOptions, volCap, util.CsiConfigFile) + if err != nil { + log.ErrorLog(ctx, "failed to set mount options for volume %s: %v", volID, err) - switch mnt.(type) { - case *mounter.FuseMounter: - volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, ns.fuseMountOptions) - volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, mountOptions...) - case *mounter.KernelMounter: - volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, ns.kernelMountOptions) - volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, mountOptions...) - } - - const readOnly = "ro" - - if volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY || - volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { - switch mnt.(type) { - case *mounter.FuseMounter: - if !csicommon.MountOptionContains(strings.Split(volOptions.FuseMountOptions, ","), readOnly) { - volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, readOnly) - } - case *mounter.KernelMounter: - if !csicommon.MountOptionContains(strings.Split(volOptions.KernelMountOptions, ","), readOnly) { - volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, readOnly) - } - } + return status.Error(codes.Internal, err.Error()) } if err = mnt.Mount(ctx, stagingTargetPath, cr, volOptions); err != nil { @@ -779,3 +756,67 @@ func (ns *NodeServer) NodeGetVolumeStats( return nil, status.Errorf(codes.InvalidArgument, "targetpath %q is not a directory or device", targetPath) } + +// setMountOptions updates the kernel/fuse mount options from CSI config file if it exists. +// If not, it falls back to returning the kernelMountOptions/fuseMountOptions from the command line. +func (ns *NodeServer) setMountOptions( + mnt mounter.VolumeMounter, + volOptions *store.VolumeOptions, + volCap *csi.VolumeCapability, + csiConfigFile string, +) error { + var ( + configuredMountOptions string + kernelMountOptions string + fuseMountOptions string + mountOptions []string + err error + ) + if m := volCap.GetMount(); m != nil { + mountOptions = m.GetMountFlags() + } + + if volOptions.ClusterID != "" { + kernelMountOptions, fuseMountOptions, err = util.GetCephFSMountOptions(csiConfigFile, volOptions.ClusterID) + if err != nil { + return err + } + } + + switch mnt.(type) { + case *mounter.FuseMounter: + configuredMountOptions = ns.fuseMountOptions + // override if fuseMountOptions are set + if fuseMountOptions != "" { + configuredMountOptions = fuseMountOptions + } + volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, configuredMountOptions) + volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, mountOptions...) + case *mounter.KernelMounter: + configuredMountOptions = ns.kernelMountOptions + // override of kernelMountOptions are set + if kernelMountOptions != "" { + configuredMountOptions = kernelMountOptions + } + volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, configuredMountOptions) + volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, mountOptions...) + } + + const readOnly = "ro" + + if volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY || + volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { + switch mnt.(type) { + case *mounter.FuseMounter: + if !csicommon.MountOptionContains(strings.Split(volOptions.FuseMountOptions, ","), readOnly) { + volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, readOnly) + } + case *mounter.KernelMounter: + if !csicommon.MountOptionContains(strings.Split(volOptions.KernelMountOptions, ","), readOnly) { + volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, readOnly) + } + } + } + + return nil +} diff --git a/internal/cephfs/nodeserver_test.go b/internal/cephfs/nodeserver_test.go new file mode 100644 index 000000000..e610d95a6 --- /dev/null +++ b/internal/cephfs/nodeserver_test.go @@ -0,0 +1,166 @@ +/* +Copyright 2023 The Ceph-CSI Authors. + +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 cephfs + +import ( + "encoding/json" + "os" + "strings" + "testing" + + "github.com/container-storage-interface/spec/lib/go/csi" + + "github.com/ceph/ceph-csi/internal/cephfs/mounter" + "github.com/ceph/ceph-csi/internal/cephfs/store" + "github.com/ceph/ceph-csi/internal/util" +) + +func Test_setMountOptions(t *testing.T) { + t.Parallel() + + cliKernelMountOptions := "noexec,nodev" + cliFuseMountOptions := "default_permissions,auto_cache" + + configKernelMountOptions := "crc" + configFuseMountOptions := "allow_other" + + csiConfig := []util.ClusterInfo{ + { + ClusterID: "cluster-1", + CephFS: util.CephFS{ + KernelMountOptions: configKernelMountOptions, + FuseMountOptions: configFuseMountOptions, + }, + }, + { + ClusterID: "cluster-2", + CephFS: util.CephFS{ + KernelMountOptions: "", + FuseMountOptions: "", + }, + }, + } + + csiConfigFileContent, err := json.Marshal(csiConfig) + if err != nil { + t.Errorf("failed to marshal csi config info %v", err) + } + tmpConfPath := t.TempDir() + "/ceph-csi.json" + t.Logf("path = %s", tmpConfPath) + err = os.WriteFile(tmpConfPath, csiConfigFileContent, 0o600) + if err != nil { + t.Errorf("failed to write %s file content: %v", util.CsiConfigFile, err) + } + + tests := []struct { + name string + ns NodeServer + mnt mounter.VolumeMounter + volOptions *store.VolumeOptions + want string + }{ + { + name: "KernelMountOptions set in cluster-1 config and not set in CLI", + ns: NodeServer{}, + mnt: mounter.VolumeMounter(&mounter.KernelMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-1", + }, + want: configKernelMountOptions, + }, + { + name: "FuseMountOptions set in cluster-1 config and not set in CLI", + ns: NodeServer{}, + mnt: mounter.VolumeMounter(&mounter.FuseMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-1", + }, + want: configFuseMountOptions, + }, + { + name: "KernelMountOptions set in cluster-1 config and set in CLI", + ns: NodeServer{ + kernelMountOptions: cliKernelMountOptions, + }, + mnt: mounter.VolumeMounter(&mounter.KernelMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-1", + }, + want: configKernelMountOptions, + }, + { + name: "FuseMountOptions not set in cluster-2 config and set in CLI", + ns: NodeServer{ + fuseMountOptions: cliFuseMountOptions, + }, + mnt: mounter.VolumeMounter(&mounter.FuseMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-1", + }, + want: configFuseMountOptions, + }, + { + name: "KernelMountOptions not set in cluster-2 config and set in CLI", + ns: NodeServer{ + kernelMountOptions: cliKernelMountOptions, + }, + mnt: mounter.VolumeMounter(&mounter.KernelMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-2", + }, + want: cliKernelMountOptions, + }, + { + name: "FuseMountOptions not set in cluster-1 config and set in CLI", + ns: NodeServer{ + fuseMountOptions: cliFuseMountOptions, + }, + mnt: mounter.VolumeMounter(&mounter.FuseMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-2", + }, + want: cliFuseMountOptions, + }, + } + + volCap := &csi.VolumeCapability{ + AccessMode: &csi.VolumeCapability_AccessMode{}, + } + + for _, tt := range tests { + tc := tt + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + err := tc.ns.setMountOptions(tc.mnt, tc.volOptions, volCap, tmpConfPath) + if err != nil { + t.Errorf("setMountOptions() = %v", err) + } + + switch tc.mnt.(type) { + case *mounter.FuseMounter: + if !strings.Contains(tc.volOptions.FuseMountOptions, tc.want) { + t.Errorf("Set FuseMountOptions = %v Required FuseMountOptions = %v", tc.volOptions.FuseMountOptions, tc.want) + } + case *mounter.KernelMounter: + if !strings.Contains(tc.volOptions.KernelMountOptions, tc.want) { + t.Errorf("Set KernelMountOptions = %v Required KernelMountOptions = %v", tc.volOptions.KernelMountOptions, tc.want) + } + } + }) + } +} diff --git a/internal/util/csiconfig.go b/internal/util/csiconfig.go index abacab329..f2355fabd 100644 --- a/internal/util/csiconfig.go +++ b/internal/util/csiconfig.go @@ -43,30 +43,40 @@ type ClusterInfo struct { // Monitors is monitor list for corresponding cluster ID Monitors []string `json:"monitors"` // CephFS contains CephFS specific options - CephFS struct { - // symlink filepath for the network namespace where we need to execute commands. - NetNamespaceFilePath string `json:"netNamespaceFilePath"` - // SubvolumeGroup contains the name of the SubvolumeGroup for CSI volumes - SubvolumeGroup string `json:"subvolumeGroup"` - } `json:"cephFS"` - + CephFS CephFS `json:"cephFS"` // RBD Contains RBD specific options - RBD struct { - // symlink filepath for the network namespace where we need to execute commands. - NetNamespaceFilePath string `json:"netNamespaceFilePath"` - // RadosNamespace is a rados namespace in the pool - RadosNamespace string `json:"radosNamespace"` - } `json:"rbd"` + RBD RBD `json:"rbd"` // NFS contains NFS specific options - NFS struct { - // symlink filepath for the network namespace where we need to execute commands. - NetNamespaceFilePath string `json:"netNamespaceFilePath"` - } `json:"nfs"` + NFS NFS `json:"nfs"` // Read affinity map options - ReadAffinity struct { - Enabled bool `json:"enabled"` - CrushLocationLabels []string `json:"crushLocationLabels"` - } `json:"readAffinity"` + ReadAffinity ReadAffinity `json:"readAffinity"` +} + +type CephFS struct { + // symlink filepath for the network namespace where we need to execute commands. + NetNamespaceFilePath string `json:"netNamespaceFilePath"` + // SubvolumeGroup contains the name of the SubvolumeGroup for CSI volumes + SubvolumeGroup string `json:"subvolumeGroup"` + // KernelMountOptions contains the kernel mount options for CephFS volumes + KernelMountOptions string `json:"kernelMountOptions"` + // FuseMountOptions contains the fuse mount options for CephFS volumes + FuseMountOptions string `json:"fuseMountOptions"` +} +type RBD struct { + // symlink filepath for the network namespace where we need to execute commands. + NetNamespaceFilePath string `json:"netNamespaceFilePath"` + // RadosNamespace is a rados namespace in the pool + RadosNamespace string `json:"radosNamespace"` +} + +type NFS struct { + // symlink filepath for the network namespace where we need to execute commands. + NetNamespaceFilePath string `json:"netNamespaceFilePath"` +} + +type ReadAffinity struct { + Enabled bool `json:"enabled"` + CrushLocationLabels []string `json:"crushLocationLabels"` } // Expected JSON structure in the passed in config file is, @@ -226,3 +236,13 @@ func GetCrushLocationLabels(pathToConfig, clusterID string) (bool, string, error return true, crushLocationLabels, nil } + +// GetCephFSMountOptions returns the `kernelMountOptions` and `fuseMountOptions` for CephFS volumes. +func GetCephFSMountOptions(pathToConfig, clusterID string) (string, string, error) { + cluster, err := readClusterInfo(pathToConfig, clusterID) + if err != nil { + return "", "", err + } + + return cluster.CephFS.KernelMountOptions, cluster.CephFS.FuseMountOptions, nil +} diff --git a/internal/util/csiconfig_test.go b/internal/util/csiconfig_test.go index 40e1b4d5e..1e00f80aa 100644 --- a/internal/util/csiconfig_test.go +++ b/internal/util/csiconfig_test.go @@ -168,20 +168,14 @@ func TestGetRBDNetNamespaceFilePath(t *testing.T) { { ClusterID: "cluster-1", Monitors: []string{"ip-1", "ip-2"}, - RBD: struct { - NetNamespaceFilePath string `json:"netNamespaceFilePath"` - RadosNamespace string `json:"radosNamespace"` - }{ + RBD: RBD{ NetNamespaceFilePath: "/var/lib/kubelet/plugins/rbd.ceph.csi.com/cluster1-net", }, }, { ClusterID: "cluster-2", Monitors: []string{"ip-3", "ip-4"}, - RBD: struct { - NetNamespaceFilePath string `json:"netNamespaceFilePath"` - RadosNamespace string `json:"radosNamespace"` - }{ + RBD: RBD{ NetNamespaceFilePath: "/var/lib/kubelet/plugins/rbd.ceph.csi.com/cluster2-net", }, }, @@ -244,20 +238,14 @@ func TestGetCephFSNetNamespaceFilePath(t *testing.T) { { ClusterID: "cluster-1", Monitors: []string{"ip-1", "ip-2"}, - CephFS: struct { - NetNamespaceFilePath string `json:"netNamespaceFilePath"` - SubvolumeGroup string `json:"subvolumeGroup"` - }{ + CephFS: CephFS{ NetNamespaceFilePath: "/var/lib/kubelet/plugins/cephfs.ceph.csi.com/cluster1-net", }, }, { ClusterID: "cluster-2", Monitors: []string{"ip-3", "ip-4"}, - CephFS: struct { - NetNamespaceFilePath string `json:"netNamespaceFilePath"` - SubvolumeGroup string `json:"subvolumeGroup"` - }{ + CephFS: CephFS{ NetNamespaceFilePath: "/var/lib/kubelet/plugins/cephfs.ceph.csi.com/cluster2-net", }, }, @@ -320,18 +308,14 @@ func TestGetNFSNetNamespaceFilePath(t *testing.T) { { ClusterID: "cluster-1", Monitors: []string{"ip-1", "ip-2"}, - NFS: struct { - NetNamespaceFilePath string `json:"netNamespaceFilePath"` - }{ + NFS: NFS{ NetNamespaceFilePath: "/var/lib/kubelet/plugins/nfs.ceph.csi.com/cluster1-net", }, }, { ClusterID: "cluster-2", Monitors: []string{"ip-3", "ip-4"}, - NFS: struct { - NetNamespaceFilePath string `json:"netNamespaceFilePath"` - }{ + NFS: NFS{ NetNamespaceFilePath: "/var/lib/kubelet/plugins/nfs.ceph.csi.com/cluster2-net", }, }, @@ -413,10 +397,7 @@ func TestGetReadAffinityOptions(t *testing.T) { csiConfig := []ClusterInfo{ { ClusterID: "cluster-1", - ReadAffinity: struct { - Enabled bool `json:"enabled"` - CrushLocationLabels []string `json:"crushLocationLabels"` - }{ + ReadAffinity: ReadAffinity{ Enabled: true, CrushLocationLabels: []string{ "topology.kubernetes.io/region", @@ -427,10 +408,7 @@ func TestGetReadAffinityOptions(t *testing.T) { }, { ClusterID: "cluster-2", - ReadAffinity: struct { - Enabled bool `json:"enabled"` - CrushLocationLabels []string `json:"crushLocationLabels"` - }{ + ReadAffinity: ReadAffinity{ Enabled: true, CrushLocationLabels: []string{ "topology.kubernetes.io/region", @@ -439,10 +417,7 @@ func TestGetReadAffinityOptions(t *testing.T) { }, { ClusterID: "cluster-3", - ReadAffinity: struct { - Enabled bool `json:"enabled"` - CrushLocationLabels []string `json:"crushLocationLabels"` - }{ + ReadAffinity: ReadAffinity{ Enabled: false, CrushLocationLabels: []string{ "topology.io/rack", @@ -478,3 +453,78 @@ func TestGetReadAffinityOptions(t *testing.T) { }) } } + +func TestGetCephFSMountOptions(t *testing.T) { + t.Parallel() + tests := []struct { + name string + clusterID string + wantKernelMntOptions string + wantFuseMntOptions string + }{ + { + name: "cluster-1 with non-empty mount options", + clusterID: "cluster-1", + wantKernelMntOptions: "crc", + wantFuseMntOptions: "ro", + }, + { + name: "cluster-2 with empty mount options", + clusterID: "cluster-2", + wantKernelMntOptions: "", + wantFuseMntOptions: "", + }, + { + name: "cluster-3 with no mount options", + clusterID: "cluster-3", + wantKernelMntOptions: "", + wantFuseMntOptions: "", + }, + } + + csiConfig := []ClusterInfo{ + { + ClusterID: "cluster-1", + CephFS: CephFS{ + KernelMountOptions: "crc", + FuseMountOptions: "ro", + }, + }, + { + ClusterID: "cluster-2", + CephFS: CephFS{ + KernelMountOptions: "", + FuseMountOptions: "", + }, + }, + { + ClusterID: "cluster-3", + CephFS: CephFS{}, + }, + } + csiConfigFileContent, err := json.Marshal(csiConfig) + if err != nil { + t.Errorf("failed to marshal csi config info %v", err) + } + tmpConfPath := t.TempDir() + "/ceph-csi.json" + err = os.WriteFile(tmpConfPath, csiConfigFileContent, 0o600) + if err != nil { + t.Errorf("failed to write %s file content: %v", CsiConfigFile, err) + } + + for _, tt := range tests { + tc := tt + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + kernelMntOptions, fuseMntOptions, err := GetCephFSMountOptions(tmpConfPath, tc.clusterID) + if err != nil { + t.Errorf("GetCephFSMountOptions() error = %v", err) + } + if kernelMntOptions != tc.wantKernelMntOptions || fuseMntOptions != tc.wantFuseMntOptions { + t.Errorf("GetCephFSMountOptions() = (%v, %v), want (%v, %v)", + kernelMntOptions, fuseMntOptions, tc.wantKernelMntOptions, tc.wantFuseMntOptions, + ) + } + }) + } +}