diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 084e12b3bf0..03016151669 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -393,6 +393,8 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po opts.AllowRelaxedEnvironmentVariableValidation = useRelaxedEnvironmentVariableValidation(podSpec, oldPodSpec) opts.AllowRelaxedDNSSearchValidation = useRelaxedDNSSearchValidation(oldPodSpec) + opts.AllowOnlyRecursiveSELinuxChangePolicy = useOnlyRecursiveSELinuxChangePolicy(oldPodSpec) + if oldPodSpec != nil { // if old spec has status.hostIPs downwardAPI set, we must allow it opts.AllowHostIPsField = opts.AllowHostIPsField || hasUsedDownwardAPIFieldPathWithPodSpec(oldPodSpec, "status.hostIPs") @@ -723,6 +725,7 @@ func dropDisabledFields( dropPodLifecycleSleepAction(podSpec, oldPodSpec) dropImageVolumes(podSpec, oldPodSpec) + dropSELinuxChangePolicy(podSpec, oldPodSpec) } func dropPodLifecycleSleepAction(podSpec, oldPodSpec *api.PodSpec) { @@ -1361,3 +1364,34 @@ func imageVolumesInUse(podSpec *api.PodSpec) bool { return false } + +func dropSELinuxChangePolicy(podSpec, oldPodSpec *api.PodSpec) { + if utilfeature.DefaultFeatureGate.Enabled(features.SELinuxChangePolicy) || seLinuxChangePolicyInUse(oldPodSpec) { + return + } + if podSpec == nil || podSpec.SecurityContext == nil { + return + } + podSpec.SecurityContext.SELinuxChangePolicy = nil +} + +func seLinuxChangePolicyInUse(podSpec *api.PodSpec) bool { + if podSpec == nil || podSpec.SecurityContext == nil { + return false + } + return podSpec.SecurityContext.SELinuxChangePolicy != nil +} + +func useOnlyRecursiveSELinuxChangePolicy(oldPodSpec *api.PodSpec) bool { + if utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMount) { + // All policies are allowed + return false + } + + if seLinuxChangePolicyInUse(oldPodSpec) { + // The old pod spec has *any* policy: we need to keep that object update-able. + return false + } + // No feature gate + no value in the old object -> only Recursive is allowed + return true +} diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index a99deac1a4c..84985fa05ba 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -25,6 +25,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/component-base/featuregate" + "k8s.io/utils/ptr" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -3901,3 +3903,142 @@ func TestDropImageVolumes(t *testing.T) { }) } } + +func TestDropSELinuxChangePolicy(t *testing.T) { + podRecursive := &api.Pod{ + Spec: api.PodSpec{ + SecurityContext: &api.PodSecurityContext{ + SELinuxChangePolicy: ptr.To(api.SELinuxChangePolicyRecursive), + }, + }, + } + podMountOption := &api.Pod{ + Spec: api.PodSpec{ + SecurityContext: &api.PodSecurityContext{ + SELinuxChangePolicy: ptr.To(api.SELinuxChangePolicyMountOption), + }, + }, + } + podNull := &api.Pod{ + Spec: api.PodSpec{ + SecurityContext: &api.PodSecurityContext{ + SELinuxChangePolicy: nil, + }, + }, + } + + tests := []struct { + name string + oldPod *api.Pod + newPod *api.Pod + gates []featuregate.Feature + wantPod *api.Pod + }{ + { + name: "no old pod, new pod with Recursive, all features disabled", + oldPod: nil, + newPod: podRecursive, + gates: nil, + wantPod: podNull, + }, + { + name: "no old pod, new pod with MountOption, all features disabled", + oldPod: nil, + newPod: podMountOption, + gates: nil, + wantPod: podNull, + }, + { + name: "old pod with Recursive, new pod with Recursive, all features disabled", + oldPod: podRecursive, + newPod: podRecursive, + gates: nil, + wantPod: podRecursive, + }, + { + name: "old pod with MountOption, new pod with Recursive, all features disabled", + oldPod: podMountOption, + newPod: podRecursive, + gates: nil, + wantPod: podRecursive, + }, + { + name: "no old pod, new pod with Recursive, SELinuxChangePolicy feature enabled", + oldPod: nil, + newPod: podRecursive, + gates: []featuregate.Feature{features.SELinuxChangePolicy}, + wantPod: podRecursive, + }, + { + name: "no old pod, new pod with MountOption, SELinuxChangePolicy feature enabled", + oldPod: nil, + newPod: podMountOption, + gates: []featuregate.Feature{features.SELinuxChangePolicy}, + wantPod: podMountOption, + }, + { + name: "old pod with Recursive, new pod with Recursive, SELinuxChangePolicy feature enabled", + oldPod: podRecursive, + newPod: podRecursive, + gates: []featuregate.Feature{features.SELinuxChangePolicy}, + wantPod: podRecursive, + }, + { + name: "old pod with MountOption, new pod with Recursive, SELinuxChangePolicy feature enabled", + oldPod: podMountOption, + newPod: podRecursive, + gates: []featuregate.Feature{features.SELinuxChangePolicy}, + wantPod: podRecursive, + }, + // In theory, SELinuxMount does not have any effect on dropping SELinuxChangePolicy field, but for completeness: + { + name: "no old pod, new pod with Recursive, SELinuxChangePolicy + SELinuxMount features enabled", + oldPod: nil, + newPod: podRecursive, + gates: []featuregate.Feature{features.SELinuxChangePolicy, features.SELinuxMount}, + wantPod: podRecursive, + }, + { + name: "no old pod, new pod with MountOption, SELinuxChangePolicy + SELinuxMount features enabled", + oldPod: nil, + newPod: podMountOption, + gates: []featuregate.Feature{features.SELinuxChangePolicy, features.SELinuxMount}, + wantPod: podMountOption, + }, + { + name: "old pod with Recursive, new pod with Recursive, SELinuxChangePolicy + SELinuxMount features enabled", + oldPod: podRecursive, + newPod: podRecursive, + gates: []featuregate.Feature{features.SELinuxChangePolicy, features.SELinuxMount}, + wantPod: podRecursive, + }, + { + name: "old pod with MountOption, new pod with Recursive, SELinuxChangePolicy + SELinuxMount features enabled", + oldPod: podMountOption, + newPod: podRecursive, + gates: []featuregate.Feature{features.SELinuxChangePolicy, features.SELinuxMount}, + wantPod: podRecursive, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + + for _, gate := range tc.gates { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, gate, true) + } + + oldPod := tc.oldPod.DeepCopy() + newPod := tc.newPod.DeepCopy() + DropDisabledPodFields(newPod, oldPod) + + // old pod should never be changed + if diff := cmp.Diff(oldPod, tc.oldPod); diff != "" { + t.Errorf("old pod changed: %s", diff) + } + + if diff := cmp.Diff(tc.wantPod, newPod); diff != "" { + t.Errorf("new pod changed (- want, + got): %s", diff) + } + }) + } +} diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 3cb76ab2624..7d5527e881c 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4058,6 +4058,8 @@ type PodValidationOptions struct { AllowRelaxedDNSSearchValidation bool // Allows zero value for Pod Lifecycle Sleep Action AllowPodLifecycleSleepActionZeroValue bool + // Allow only Recursive value of SELinuxChangePolicy. + AllowOnlyRecursiveSELinuxChangePolicy bool } // validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set, @@ -4356,6 +4358,9 @@ func validateWindows(spec *core.PodSpec, fldPath *field.Path) field.ErrorList { if securityContext.SupplementalGroupsPolicy != nil { allErrs = append(allErrs, field.Forbidden(fldPath.Child("securityContext").Child("supplementalGroupsPolicy"), "cannot be set for a windows pod")) } + if securityContext.SELinuxChangePolicy != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("securityContext").Child("seLinuxChangePolicy"), "cannot be set for a windows pod")) + } } podshelper.VisitContainersWithPath(spec, fldPath, func(c *core.Container, cFldPath *field.Path) bool { // validate container security context @@ -4941,6 +4946,28 @@ func ValidateHostSysctl(sysctl string, securityContext *core.PodSecurityContext, return nil } +var validSELinuxChangePolicies = sets.New(core.SELinuxChangePolicyRecursive, core.SELinuxChangePolicyMountOption) + +func validateSELinuxChangePolicy(seLinuxChangePolicy *core.PodSELinuxChangePolicy, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { + if seLinuxChangePolicy == nil { + return nil + } + + allErrs := field.ErrorList{} + + if opts.AllowOnlyRecursiveSELinuxChangePolicy { + if *seLinuxChangePolicy != core.SELinuxChangePolicyRecursive { + allErrs = append(allErrs, field.NotSupported(fldPath, *seLinuxChangePolicy, []core.PodSELinuxChangePolicy{core.SELinuxChangePolicyRecursive})) + } + } else { + // Allow any valid SELinuxChangePolicy value. + if !validSELinuxChangePolicies.Has(*seLinuxChangePolicy) { + allErrs = append(allErrs, field.NotSupported(fldPath, *seLinuxChangePolicy, sets.List(validSELinuxChangePolicies))) + } + } + return allErrs +} + // validatePodSpecSecurityContext verifies the SecurityContext of a PodSpec, // whether that is defined in a Pod or in an embedded PodSpec (e.g. a // Deployment's pod template). @@ -4987,6 +5014,10 @@ func validatePodSpecSecurityContext(securityContext *core.PodSecurityContext, sp if securityContext.SupplementalGroupsPolicy != nil { allErrs = append(allErrs, validateSupplementalGroupsPolicy(securityContext.SupplementalGroupsPolicy, fldPath.Child("supplementalGroupsPolicy"))...) } + + if securityContext.SELinuxChangePolicy != nil { + allErrs = append(allErrs, validateSELinuxChangePolicy(securityContext.SELinuxChangePolicy, fldPath.Child("seLinuxChangePolicy"), opts)...) + } } return allErrs diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index d1649032573..a755f0cc8a4 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -20557,6 +20557,7 @@ func TestValidateOSFields(t *testing.T) { "SecurityContext.RunAsGroup", "SecurityContext.RunAsUser", "SecurityContext.SELinuxOptions", + "SecurityContext.SELinuxChangePolicy", "SecurityContext.SeccompProfile", "SecurityContext.ShareProcessNamespace", "SecurityContext.SupplementalGroups", @@ -25000,3 +25001,78 @@ func TestValidateContainerStatusAllocatedResourcesStatus(t *testing.T) { }) } } + +func TestValidateSELinuxChangePolicy(t *testing.T) { + tests := []struct { + name string + pod *core.Pod + allowOnlyRecursive bool + wantErrs field.ErrorList + }{ + { + name: "nil is valid", + pod: podtest.MakePod("pod", podtest.SetSecurityContext(&core.PodSecurityContext{ + SELinuxChangePolicy: nil, + })), + allowOnlyRecursive: false, + wantErrs: nil, + }, + { + name: "Recursive is always valid", + pod: podtest.MakePod("pod", podtest.SetSecurityContext(&core.PodSecurityContext{ + SELinuxChangePolicy: ptr.To(core.SELinuxChangePolicyRecursive), + })), + allowOnlyRecursive: false, + wantErrs: nil, + }, + { + name: "MountOption is not valid when AllowOnlyRecursiveSELinuxChangePolicy", + pod: podtest.MakePod("pod", podtest.SetSecurityContext(&core.PodSecurityContext{ + SELinuxChangePolicy: ptr.To(core.SELinuxChangePolicyMountOption), + })), + allowOnlyRecursive: true, + wantErrs: field.ErrorList{ + field.NotSupported( + field.NewPath("spec", "securityContext", "seLinuxChangePolicy"), + core.PodSELinuxChangePolicy("MountOption"), + []string{"Recursive"}), + }, + }, + { + name: "MountOption is valid when not AllowOnlyRecursiveSELinuxChangePolicy", + pod: podtest.MakePod("pod", podtest.SetSecurityContext(&core.PodSecurityContext{ + SELinuxChangePolicy: ptr.To(core.SELinuxChangePolicyMountOption), + })), + allowOnlyRecursive: false, + wantErrs: nil, + }, + { + name: "invalid value", + pod: podtest.MakePod("pod", podtest.SetSecurityContext(&core.PodSecurityContext{ + SELinuxChangePolicy: ptr.To(core.PodSELinuxChangePolicy("InvalidValue")), + })), + allowOnlyRecursive: false, + wantErrs: field.ErrorList{ + field.NotSupported(field.NewPath("spec", "securityContext", "seLinuxChangePolicy"), + core.PodSELinuxChangePolicy("InvalidValue"), + []string{"MountOption", "Recursive"}), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + opts := PodValidationOptions{ + AllowOnlyRecursiveSELinuxChangePolicy: tc.allowOnlyRecursive, + } + errs := ValidatePodSpec(&tc.pod.Spec, &tc.pod.ObjectMeta, field.NewPath("spec"), opts) + if len(errs) == 0 { + errs = nil + } + if diff := cmp.Diff(tc.wantErrs, errs); diff != "" { + t.Errorf("unexpected field errors (-want, +got):\n%s", diff) + } + + }) + } +}