From a53c6f1dc82ba7581120bc36caec5bd4da63b6e2 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 14 Mar 2023 14:00:58 +0100 Subject: [PATCH] Add validation of CSIDriver.SELinuxMount --- pkg/apis/storage/validation/validation.go | 13 ++ .../storage/validation/validation_test.go | 119 ++++++++++++++++++ .../storage/csidriver/storage/storage_test.go | 4 + .../storage/csidriver/strategy_test.go | 24 ++++ 4 files changed, 160 insertions(+) diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index 73392232289..e7b68044cb2 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -27,10 +27,12 @@ import ( metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/apiserver/pkg/util/feature" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/apis/storage" + "k8s.io/kubernetes/pkg/features" ) const ( @@ -436,6 +438,7 @@ func validateCSIDriverSpec( allErrs = append(allErrs, validateFSGroupPolicy(spec.FSGroupPolicy, fldPath.Child("fsGroupPolicy"))...) allErrs = append(allErrs, validateTokenRequests(spec.TokenRequests, fldPath.Child("tokenRequests"))...) allErrs = append(allErrs, validateVolumeLifecycleModes(spec.VolumeLifecycleModes, fldPath.Child("volumeLifecycleModes"))...) + allErrs = append(allErrs, validateSELinuxMount(spec.SELinuxMount, fldPath.Child("seLinuxMount"))...) return allErrs } @@ -533,6 +536,16 @@ func validateVolumeLifecycleModes(modes []storage.VolumeLifecycleMode, fldPath * return allErrs } +// validateSELinuxMount tests if seLinuxMount is set for CSIDriver. +func validateSELinuxMount(seLinuxMount *bool, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if seLinuxMount == nil && utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) { + allErrs = append(allErrs, field.Required(fldPath, "")) + } + + return allErrs +} + // ValidateStorageCapacityName checks that a name is appropriate for a // CSIStorageCapacity object. var ValidateStorageCapacityName = apimachineryvalidation.NameIsDNSSubdomain diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index dc936a3af01..a0a45450fc7 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -23,8 +23,11 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/storage" + "k8s.io/kubernetes/pkg/features" utilpointer "k8s.io/utils/pointer" ) @@ -1657,6 +1660,8 @@ func TestCSIDriverValidation(t *testing.T) { notRequiresRepublish := false storageCapacity := true notStorageCapacity := false + seLinuxMount := true + notSELinuxMount := false supportedFSGroupPolicy := storage.FileFSGroupPolicy invalidFSGroupPolicy := storage.FSGroupPolicy("invalid-mode") successCases := []storage.CSIDriver{ @@ -1667,6 +1672,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1677,6 +1683,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, StorageCapacity: ¬StorageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1687,6 +1694,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1697,6 +1705,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1707,6 +1716,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1716,6 +1726,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1725,6 +1736,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: &podInfoOnMount, RequiresRepublish: ¬RequiresRepublish, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1734,6 +1746,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1746,6 +1759,7 @@ func TestCSIDriverValidation(t *testing.T) { VolumeLifecycleModes: []storage.VolumeLifecycleMode{ storage.VolumeLifecyclePersistent, }, + SELinuxMount: &seLinuxMount, }, }, { @@ -1758,6 +1772,7 @@ func TestCSIDriverValidation(t *testing.T) { VolumeLifecycleModes: []storage.VolumeLifecycleMode{ storage.VolumeLifecycleEphemeral, }, + SELinuxMount: &seLinuxMount, }, }, { @@ -1771,6 +1786,7 @@ func TestCSIDriverValidation(t *testing.T) { storage.VolumeLifecycleEphemeral, storage.VolumeLifecyclePersistent, }, + SELinuxMount: &seLinuxMount, }, }, { @@ -1785,6 +1801,7 @@ func TestCSIDriverValidation(t *testing.T) { storage.VolumeLifecyclePersistent, storage.VolumeLifecycleEphemeral, }, + SELinuxMount: &seLinuxMount, }, }, { @@ -1795,6 +1812,18 @@ func TestCSIDriverValidation(t *testing.T) { RequiresRepublish: ¬RequiresRepublish, StorageCapacity: &storageCapacity, FSGroupPolicy: &supportedFSGroupPolicy, + SELinuxMount: &seLinuxMount, + }, + }, + { + // SELinuxMount: false + ObjectMeta: metav1.ObjectMeta{Name: driverName}, + Spec: storage.CSIDriverSpec{ + AttachRequired: &attachNotRequired, + PodInfoOnMount: ¬PodInfoOnMount, + RequiresRepublish: ¬RequiresRepublish, + StorageCapacity: &storageCapacity, + SELinuxMount: ¬SELinuxMount, }, }, } @@ -1811,6 +1840,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachRequired, PodInfoOnMount: &podInfoOnMount, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1819,6 +1849,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: ¬PodInfoOnMount, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1828,6 +1859,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: nil, PodInfoOnMount: &podInfoOnMount, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1837,6 +1869,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: nil, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, }, { @@ -1846,6 +1879,7 @@ func TestCSIDriverValidation(t *testing.T) { AttachRequired: &attachNotRequired, PodInfoOnMount: &podInfoOnMount, StorageCapacity: nil, + SELinuxMount: &seLinuxMount, }, }, { @@ -1858,6 +1892,7 @@ func TestCSIDriverValidation(t *testing.T) { VolumeLifecycleModes: []storage.VolumeLifecycleMode{ "no-such-mode", }, + SELinuxMount: &seLinuxMount, }, }, { @@ -1868,6 +1903,16 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: ¬PodInfoOnMount, FSGroupPolicy: &invalidFSGroupPolicy, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, + }, + }, + { + // no SELinuxMount + ObjectMeta: metav1.ObjectMeta{Name: driverName}, + Spec: storage.CSIDriverSpec{ + AttachRequired: &attachNotRequired, + PodInfoOnMount: ¬PodInfoOnMount, + StorageCapacity: &storageCapacity, }, }, } @@ -1892,6 +1937,8 @@ func TestCSIDriverValidationUpdate(t *testing.T) { requiresRepublish := true notRequiresRepublish := false notStorageCapacity := false + seLinuxMount := true + notSELinuxMount := false resourceVersion := "1" old := storage.CSIDriver{ ObjectMeta: metav1.ObjectMeta{Name: driverName, ResourceVersion: resourceVersion}, @@ -1904,6 +1951,7 @@ func TestCSIDriverValidationUpdate(t *testing.T) { storage.VolumeLifecyclePersistent, }, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, } @@ -1933,6 +1981,12 @@ func TestCSIDriverValidationUpdate(t *testing.T) { new.Spec.StorageCapacity = ¬StorageCapacity }, }, + { + name: "SELinuxMount changed", + modify: func(new *storage.CSIDriver) { + new.Spec.SELinuxMount = ¬SELinuxMount + }, + }, } for _, test := range successCases { t.Run(test.name, func(t *testing.T) { @@ -2041,6 +2095,12 @@ func TestCSIDriverValidationUpdate(t *testing.T) { new.Spec.StorageCapacity = nil }, }, + { + name: "SELinuxMount not set", + modify: func(new *storage.CSIDriver) { + new.Spec.SELinuxMount = nil + }, + }, } for _, test := range errorCases { @@ -2061,12 +2121,14 @@ func TestCSIDriverStorageCapacityEnablement(t *testing.T) { podInfoOnMount := true requiresRepublish := true storageCapacity := true + seLinuxMount := false csiDriver := storage.CSIDriver{ ObjectMeta: metav1.ObjectMeta{Name: driverName}, Spec: storage.CSIDriverSpec{ AttachRequired: &attachRequired, PodInfoOnMount: &podInfoOnMount, RequiresRepublish: &requiresRepublish, + SELinuxMount: &seLinuxMount, }, } if withField { @@ -2260,8 +2322,65 @@ func TestCSIServiceAccountToken(t *testing.T) { test.csiDriver.Spec.AttachRequired = new(bool) test.csiDriver.Spec.PodInfoOnMount = new(bool) test.csiDriver.Spec.StorageCapacity = new(bool) + test.csiDriver.Spec.SELinuxMount = new(bool) if errs := ValidateCSIDriver(test.csiDriver); test.wantErr != (len(errs) != 0) { t.Errorf("ValidateCSIDriver = %v, want err: %v", errs, test.wantErr) } } } + +func TestCSIDriverValidationSELinuxMountAlpha(t *testing.T) { + tests := []struct { + name string + featureEnabled bool + seLinuxMountValue *bool + expectError bool + }{ + { + name: "feature enabled, nil value", + featureEnabled: true, + seLinuxMountValue: nil, + expectError: true, + }, + { + name: "feature enabled, non-nil value", + featureEnabled: true, + seLinuxMountValue: utilpointer.Bool(true), + expectError: false, + }, + { + name: "feature disabled, nil value", + featureEnabled: false, + seLinuxMountValue: nil, + expectError: false, + }, + { + name: "feature disabled, non-nil value", + featureEnabled: false, + seLinuxMountValue: utilpointer.Bool(true), + expectError: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SELinuxMountReadWriteOncePod, test.featureEnabled)() + csiDriver := &storage.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: storage.CSIDriverSpec{ + AttachRequired: utilpointer.Bool(true), + PodInfoOnMount: utilpointer.Bool(true), + RequiresRepublish: utilpointer.Bool(true), + StorageCapacity: utilpointer.Bool(true), + SELinuxMount: test.seLinuxMountValue, + }, + } + err := ValidateCSIDriver(csiDriver) + if test.expectError && err == nil { + t.Error("Expected validation error, got nil") + } + if !test.expectError && err != nil { + t.Errorf("Validation returned error: %s", err) + } + }) + } +} diff --git a/pkg/registry/storage/csidriver/storage/storage_test.go b/pkg/registry/storage/csidriver/storage/storage_test.go index 383f2aba7a3..13c5537ea36 100644 --- a/pkg/registry/storage/csidriver/storage/storage_test.go +++ b/pkg/registry/storage/csidriver/storage/storage_test.go @@ -50,6 +50,7 @@ func validNewCSIDriver(name string) *storageapi.CSIDriver { podInfoOnMount := true requiresRepublish := true storageCapacity := true + seLinuxMount := true return &storageapi.CSIDriver{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -59,6 +60,7 @@ func validNewCSIDriver(name string) *storageapi.CSIDriver { PodInfoOnMount: &podInfoOnMount, RequiresRepublish: &requiresRepublish, StorageCapacity: &storageCapacity, + SELinuxMount: &seLinuxMount, }, } } @@ -74,6 +76,7 @@ func TestCreate(t *testing.T) { notPodInfoOnMount := false notRequiresRepublish := false notStorageCapacity := false + notSELinuxMount := false test.TestCreate( // valid csiDriver, @@ -85,6 +88,7 @@ func TestCreate(t *testing.T) { PodInfoOnMount: ¬PodInfoOnMount, RequiresRepublish: ¬RequiresRepublish, StorageCapacity: ¬StorageCapacity, + SELinuxMount: ¬SELinuxMount, }, }, ) diff --git a/pkg/registry/storage/csidriver/strategy_test.go b/pkg/registry/storage/csidriver/strategy_test.go index 1fa74f9147f..230f92d7008 100644 --- a/pkg/registry/storage/csidriver/strategy_test.go +++ b/pkg/registry/storage/csidriver/strategy_test.go @@ -40,6 +40,7 @@ func getValidCSIDriver(name string) *storage.CSIDriver { PodInfoOnMount: &enabled, StorageCapacity: &enabled, RequiresRepublish: &enabled, + SELinuxMount: &enabled, }, } } @@ -281,6 +282,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: &enabled, StorageCapacity: &enabled, RequiresRepublish: &enabled, + SELinuxMount: &enabled, }, }, false, @@ -296,6 +298,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: &disabled, StorageCapacity: &disabled, RequiresRepublish: &disabled, + SELinuxMount: &disabled, }, }, false, @@ -311,6 +314,7 @@ func TestCSIDriverValidation(t *testing.T) { PodInfoOnMount: &enabled, StorageCapacity: &enabled, RequiresRepublish: &enabled, + SELinuxMount: &enabled, }, }, true, @@ -329,6 +333,7 @@ func TestCSIDriverValidation(t *testing.T) { storage.VolumeLifecycleMode("no-such-mode"), }, RequiresRepublish: &enabled, + SELinuxMount: &enabled, }, }, true, @@ -347,6 +352,7 @@ func TestCSIDriverValidation(t *testing.T) { storage.VolumeLifecyclePersistent, }, RequiresRepublish: &enabled, + SELinuxMount: &enabled, }, }, false, @@ -365,6 +371,7 @@ func TestCSIDriverValidation(t *testing.T) { storage.VolumeLifecycleEphemeral, }, RequiresRepublish: &enabled, + SELinuxMount: &enabled, }, }, false, @@ -384,6 +391,7 @@ func TestCSIDriverValidation(t *testing.T) { storage.VolumeLifecycleEphemeral, }, RequiresRepublish: &enabled, + SELinuxMount: &enabled, }, }, false, @@ -400,10 +408,26 @@ func TestCSIDriverValidation(t *testing.T) { StorageCapacity: &enabled, TokenRequests: []storage.TokenRequest{{Audience: gcp}}, RequiresRepublish: &enabled, + SELinuxMount: &enabled, }, }, false, }, + { + "invalid SELinuxMount", + &storage.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSIDriverSpec{ + AttachRequired: &enabled, + PodInfoOnMount: &enabled, + StorageCapacity: &enabled, + SELinuxMount: nil, + }, + }, + true, + }, } for _, test := range tests {