From c065d7c7b3b848afbf26583484a6f3492bec4a7a Mon Sep 17 00:00:00 2001 From: Shiming Zhang Date: Mon, 7 Jun 2021 15:56:30 +0800 Subject: [PATCH 1/2] Fix NPE for CSI mounter --- pkg/volume/csi/csi_mounter.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/volume/csi/csi_mounter.go b/pkg/volume/csi/csi_mounter.go index 7e98dea45ca..4193f121443 100644 --- a/pkg/volume/csi/csi_mounter.go +++ b/pkg/volume/csi/csi_mounter.go @@ -392,12 +392,15 @@ func (c *csiMountMgr) supportsFSGroup(fsType string, fsGroup *int64, driverPolic return false } - accessModes := c.spec.PersistentVolume.Spec.AccessModes + if c.spec.PersistentVolume == nil { + klog.V(4).Info(log("mounter.SetupAt Warning: skipping fsGroup permission change, no access mode available. The volume may only be accessible to root users.")) + return false + } if c.spec.PersistentVolume.Spec.AccessModes == nil { klog.V(4).Info(log("mounter.SetupAt WARNING: skipping fsGroup, access modes not provided")) return false } - if !hasReadWriteOnce(accessModes) { + if !hasReadWriteOnce(c.spec.PersistentVolume.Spec.AccessModes) { klog.V(4).Info(log("mounter.SetupAt WARNING: skipping fsGroup, only support ReadWriteOnce access mode")) return false } From 1eb8060dd618430cad584db4d564159275612bd9 Mon Sep 17 00:00:00 2001 From: Shiming Zhang Date: Tue, 8 Jun 2021 10:29:35 +0800 Subject: [PATCH 2/2] Add test for CSI mounter --- pkg/volume/csi/csi_mounter_test.go | 133 +++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/pkg/volume/csi/csi_mounter_test.go b/pkg/volume/csi/csi_mounter_test.go index 67046719275..a7fbdd9cac4 100644 --- a/pkg/volume/csi/csi_mounter_test.go +++ b/pkg/volume/csi/csi_mounter_test.go @@ -34,6 +34,7 @@ import ( authenticationv1 "k8s.io/api/authentication/v1" api "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -1054,3 +1055,135 @@ func TestPodServiceAccountTokenAttrs(t *testing.T) { }) } } + +func Test_csiMountMgr_supportsFSGroup(t *testing.T) { + type fields struct { + plugin *csiPlugin + driverName csiDriverName + volumeLifecycleMode storage.VolumeLifecycleMode + fsGroupPolicy storage.FSGroupPolicy + volumeID string + specVolumeID string + readOnly bool + supportsSELinux bool + spec *volume.Spec + pod *api.Pod + podUID types.UID + publishContext map[string]string + kubeVolHost volume.KubeletVolumeHost + MetricsProvider volume.MetricsProvider + } + type args struct { + fsType string + fsGroup *int64 + driverPolicy storage.FSGroupPolicy + } + tests := []struct { + name string + fields fields + args args + want bool + }{ + { + name: "empty all", + args: args{}, + want: false, + }, + { + name: "driverPolicy is FileFSGroupPolicy", + args: args{ + fsGroup: new(int64), + driverPolicy: storage.FileFSGroupPolicy, + }, + want: true, + }, + { + name: "driverPolicy is ReadWriteOnceWithFSTypeFSGroupPolicy", + args: args{ + fsGroup: new(int64), + driverPolicy: storage.ReadWriteOnceWithFSTypeFSGroupPolicy, + }, + want: false, + }, + { + name: "driverPolicy is ReadWriteOnceWithFSTypeFSGroupPolicy with empty Spec", + args: args{ + fsGroup: new(int64), + fsType: "ext4", + driverPolicy: storage.ReadWriteOnceWithFSTypeFSGroupPolicy, + }, + fields: fields{ + spec: &volume.Spec{}, + }, + want: false, + }, + { + name: "driverPolicy is ReadWriteOnceWithFSTypeFSGroupPolicy with empty PersistentVolume", + args: args{ + fsGroup: new(int64), + fsType: "ext4", + driverPolicy: storage.ReadWriteOnceWithFSTypeFSGroupPolicy, + }, + fields: fields{ + spec: volume.NewSpecFromPersistentVolume(&corev1.PersistentVolume{}, true), + }, + want: false, + }, + { + name: "driverPolicy is ReadWriteOnceWithFSTypeFSGroupPolicy with empty AccessModes", + args: args{ + fsGroup: new(int64), + fsType: "ext4", + driverPolicy: storage.ReadWriteOnceWithFSTypeFSGroupPolicy, + }, + fields: fields{ + spec: volume.NewSpecFromPersistentVolume(&api.PersistentVolume{ + Spec: api.PersistentVolumeSpec{ + AccessModes: []api.PersistentVolumeAccessMode{}, + }, + }, true), + }, + want: false, + }, + { + name: "driverPolicy is ReadWriteOnceWithFSTypeFSGroupPolicy with ReadWriteOnce AccessModes", + args: args{ + fsGroup: new(int64), + fsType: "ext4", + driverPolicy: storage.ReadWriteOnceWithFSTypeFSGroupPolicy, + }, + fields: fields{ + spec: volume.NewSpecFromPersistentVolume(&api.PersistentVolume{ + Spec: api.PersistentVolumeSpec{ + AccessModes: []api.PersistentVolumeAccessMode{api.ReadWriteOnce}, + }, + }, true), + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &csiMountMgr{ + plugin: tt.fields.plugin, + driverName: tt.fields.driverName, + volumeLifecycleMode: tt.fields.volumeLifecycleMode, + fsGroupPolicy: tt.fields.fsGroupPolicy, + volumeID: tt.fields.volumeID, + specVolumeID: tt.fields.specVolumeID, + readOnly: tt.fields.readOnly, + supportsSELinux: tt.fields.supportsSELinux, + spec: tt.fields.spec, + pod: tt.fields.pod, + podUID: tt.fields.podUID, + publishContext: tt.fields.publishContext, + kubeVolHost: tt.fields.kubeVolHost, + MetricsProvider: tt.fields.MetricsProvider, + } + if got := c.supportsFSGroup(tt.args.fsType, tt.args.fsGroup, tt.args.driverPolicy); got != tt.want { + t.Errorf("supportsFSGroup() = %v, want %v", got, tt.want) + } + }) + } +}