From 38384d5c13756d72f924c8349e6f822da188a15a Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 9 Feb 2021 12:15:42 +0100 Subject: [PATCH] PSP: conditional support for generic volume type When introducing the new "generic" volume type for generic ephemeral inline volumes, the storage policy for PodSecurityPolicy objects should have been extended so that this new type is valid only if the generic ephemeral volume feature is enabled or an existing object already has it. Adding the new type to the internal API was also missed. --- pkg/api/podsecuritypolicy/util.go | 22 ++++++ pkg/api/podsecuritypolicy/util_test.go | 79 +++++++++++++++++++++ pkg/apis/policy/types.go | 1 + pkg/security/podsecuritypolicy/util/util.go | 4 ++ 4 files changed, 106 insertions(+) diff --git a/pkg/api/podsecuritypolicy/util.go b/pkg/api/podsecuritypolicy/util.go index a0df4f83ac4..acd856a73c0 100644 --- a/pkg/api/podsecuritypolicy/util.go +++ b/pkg/api/podsecuritypolicy/util.go @@ -31,6 +31,16 @@ func DropDisabledFields(pspSpec, oldPSPSpec *policy.PodSecurityPolicySpec) { if !utilfeature.DefaultFeatureGate.Enabled(features.CSIInlineVolume) { pspSpec.AllowedCSIDrivers = nil } + var volumes []policy.FSType + for _, volume := range pspSpec.Volumes { + if volume != policy.Ephemeral || + utilfeature.DefaultFeatureGate.Enabled(features.GenericEphemeralVolume) || + volumeInUse(oldPSPSpec, volume) { + // Keep it. + volumes = append(volumes, volume) + } + } + pspSpec.Volumes = volumes } func allowedProcMountTypesInUse(oldPSPSpec *policy.PodSecurityPolicySpec) bool { @@ -45,3 +55,15 @@ func allowedProcMountTypesInUse(oldPSPSpec *policy.PodSecurityPolicySpec) bool { return false } + +func volumeInUse(oldPSPSpec *policy.PodSecurityPolicySpec, volume policy.FSType) bool { + if oldPSPSpec == nil { + return false + } + for _, v := range oldPSPSpec.Volumes { + if v == volume { + return true + } + } + return false +} diff --git a/pkg/api/podsecuritypolicy/util_test.go b/pkg/api/podsecuritypolicy/util_test.go index 1535b04128c..775b6a29b2c 100644 --- a/pkg/api/podsecuritypolicy/util_test.go +++ b/pkg/api/podsecuritypolicy/util_test.go @@ -107,3 +107,82 @@ func TestDropAllowedProcMountTypes(t *testing.T) { } } } + +func TestDropEphemeralVolumeType(t *testing.T) { + allowedVolumeTypes := []policy.FSType{policy.Ephemeral} + pspWithoutGenericVolume := func() *policy.PodSecurityPolicySpec { + return &policy.PodSecurityPolicySpec{} + } + pspWithGenericVolume := func() *policy.PodSecurityPolicySpec { + return &policy.PodSecurityPolicySpec{ + Volumes: allowedVolumeTypes, + } + } + + pspInfo := []struct { + description string + hasGenericVolume bool + psp func() *policy.PodSecurityPolicySpec + }{ + { + description: "PodSecurityPolicySpec Without GenericVolume", + hasGenericVolume: false, + psp: pspWithoutGenericVolume, + }, + { + description: "PodSecurityPolicySpec With GenericVolume", + hasGenericVolume: true, + psp: pspWithGenericVolume, + }, + { + description: "is nil", + hasGenericVolume: false, + psp: func() *policy.PodSecurityPolicySpec { return nil }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldPSPSpecInfo := range pspInfo { + for _, newPSPSpecInfo := range pspInfo { + oldPSPSpecHasGenericVolume, oldPSPSpec := oldPSPSpecInfo.hasGenericVolume, oldPSPSpecInfo.psp() + newPSPSpecHasGenericVolume, newPSPSpec := newPSPSpecInfo.hasGenericVolume, newPSPSpecInfo.psp() + if newPSPSpec == nil { + continue + } + + t.Run(fmt.Sprintf("feature enabled=%v, old PodSecurityPolicySpec %v, new PodSecurityPolicySpec %v", enabled, oldPSPSpecInfo.description, newPSPSpecInfo.description), func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GenericEphemeralVolume, enabled)() + + DropDisabledFields(newPSPSpec, oldPSPSpec) + + // old PodSecurityPolicySpec should never be changed + if !reflect.DeepEqual(oldPSPSpec, oldPSPSpecInfo.psp()) { + t.Errorf("old PodSecurityPolicySpec changed: %v", diff.ObjectReflectDiff(oldPSPSpec, oldPSPSpecInfo.psp())) + } + + switch { + case enabled || oldPSPSpecHasGenericVolume: + // new PodSecurityPolicySpec should not be changed if the feature is enabled, or if the old PodSecurityPolicySpec had GenericVolume + if !reflect.DeepEqual(newPSPSpec, newPSPSpecInfo.psp()) { + t.Errorf("new PodSecurityPolicySpec changed: %v", diff.ObjectReflectDiff(newPSPSpec, newPSPSpecInfo.psp())) + } + case newPSPSpecHasGenericVolume: + // new PodSecurityPolicySpec should be changed + if reflect.DeepEqual(newPSPSpec, newPSPSpecInfo.psp()) { + t.Errorf("new PodSecurityPolicySpec was not changed") + } + // new PodSecurityPolicySpec should not have GenericVolume + if !reflect.DeepEqual(newPSPSpec, pspWithoutGenericVolume()) { + t.Errorf("new PodSecurityPolicySpec had PodSecurityPolicySpecGenericVolume: %v", diff.ObjectReflectDiff(newPSPSpec, pspWithoutGenericVolume())) + } + default: + // new PodSecurityPolicySpec should not need to be changed + if !reflect.DeepEqual(newPSPSpec, newPSPSpecInfo.psp()) { + t.Errorf("new PodSecurityPolicySpec changed: %v", diff.ObjectReflectDiff(newPSPSpec, newPSPSpecInfo.psp())) + } + } + }) + } + } + } +} diff --git a/pkg/apis/policy/types.go b/pkg/apis/policy/types.go index e4ef8360a73..b3884d5e480 100644 --- a/pkg/apis/policy/types.go +++ b/pkg/apis/policy/types.go @@ -309,6 +309,7 @@ const ( PortworxVolume FSType = "portworxVolume" ScaleIO FSType = "scaleIO" CSI FSType = "csi" + Ephemeral FSType = "ephemeral" All FSType = "*" ) diff --git a/pkg/security/podsecuritypolicy/util/util.go b/pkg/security/podsecuritypolicy/util/util.go index c607a23a640..1405714f8bb 100644 --- a/pkg/security/podsecuritypolicy/util/util.go +++ b/pkg/security/podsecuritypolicy/util/util.go @@ -29,6 +29,8 @@ const ( ValidatedPSPAnnotation = "kubernetes.io/psp" ) +// GetAllFSTypesExcept returns the result of GetAllFSTypesAsSet minus +// the given exceptions. func GetAllFSTypesExcept(exceptions ...string) sets.String { fstypes := GetAllFSTypesAsSet() for _, e := range exceptions { @@ -37,6 +39,8 @@ func GetAllFSTypesExcept(exceptions ...string) sets.String { return fstypes } +// GetAllFSTypesAsSet returns all actual volume types, regardless +// of feature gates. The special policy.All pseudo type is not included. func GetAllFSTypesAsSet() sets.String { fstypes := sets.NewString() fstypes.Insert(