diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index e7e65f4a32..f55951d25c 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -322,31 +322,18 @@ func dropDisabledRunAsGroupField(podSpec, oldPodSpec *api.PodSpec) { } // dropDisabledProcMountField removes disabled fields from PodSpec related -// to ProcMount +// to ProcMount only if it is not already used by the old spec func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) { - if !utilfeature.DefaultFeatureGate.Enabled(features.ProcMountType) { - defProcMount := api.DefaultProcMount + if !utilfeature.DefaultFeatureGate.Enabled(features.ProcMountType) && !procMountInUse(oldPodSpec) { + defaultProcMount := api.DefaultProcMount for i := range podSpec.Containers { if podSpec.Containers[i].SecurityContext != nil { - podSpec.Containers[i].SecurityContext.ProcMount = &defProcMount + podSpec.Containers[i].SecurityContext.ProcMount = &defaultProcMount } } for i := range podSpec.InitContainers { if podSpec.InitContainers[i].SecurityContext != nil { - podSpec.InitContainers[i].SecurityContext.ProcMount = &defProcMount - } - } - - if oldPodSpec != nil { - for i := range oldPodSpec.Containers { - if oldPodSpec.Containers[i].SecurityContext != nil { - oldPodSpec.Containers[i].SecurityContext.ProcMount = &defProcMount - } - } - for i := range oldPodSpec.InitContainers { - if oldPodSpec.InitContainers[i].SecurityContext != nil { - oldPodSpec.InitContainers[i].SecurityContext.ProcMount = &defProcMount - } + podSpec.InitContainers[i].SecurityContext.ProcMount = &defaultProcMount } } } @@ -406,3 +393,29 @@ func runtimeClassInUse(podSpec *api.PodSpec) bool { } return false } + +// procMountInUse returns true if the pod spec is non-nil and has a SecurityContext's ProcMount field set +func procMountInUse(podSpec *api.PodSpec) bool { + if podSpec == nil { + return false + } + for i := range podSpec.Containers { + if podSpec.Containers[i].SecurityContext != nil { + if podSpec.Containers[i].SecurityContext.ProcMount != nil { + if *podSpec.Containers[i].SecurityContext.ProcMount != api.DefaultProcMount { + return true + } + } + } + } + for i := range podSpec.InitContainers { + if podSpec.InitContainers[i].SecurityContext != nil { + if podSpec.InitContainers[i].SecurityContext.ProcMount != nil { + if *podSpec.InitContainers[i].SecurityContext.ProcMount != api.DefaultProcMount { + return true + } + } + } + } + return false +} diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 54301b65c8..923e39eb67 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -518,3 +518,97 @@ func TestDropRuntimeClass(t *testing.T) { } } } + +func TestDropProcMount(t *testing.T) { + procMount := api.UnmaskedProcMount + defaultProcMount := api.DefaultProcMount + podWithProcMount := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + Containers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: &api.SecurityContext{ProcMount: &procMount}}}, + InitContainers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: &api.SecurityContext{ProcMount: &procMount}}}, + }, + } + } + podWithoutProcMount := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + Containers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: &api.SecurityContext{ProcMount: &defaultProcMount}}}, + InitContainers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: &api.SecurityContext{ProcMount: &defaultProcMount}}}, + }, + } + } + + podInfo := []struct { + description string + hasProcMount bool + pod func() *api.Pod + }{ + { + description: "has ProcMount", + hasProcMount: true, + pod: podWithProcMount, + }, + { + description: "does not have ProcMount", + hasProcMount: false, + pod: podWithoutProcMount, + }, + { + description: "is nil", + hasProcMount: false, + pod: func() *api.Pod { return nil }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldPodInfo := range podInfo { + for _, newPodInfo := range podInfo { + oldPodHasProcMount, oldPod := oldPodInfo.hasProcMount, oldPodInfo.pod() + newPodHasProcMount, newPod := newPodInfo.hasProcMount, newPodInfo.pod() + if newPod == nil { + continue + } + + t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ProcMountType, enabled)() + + var oldPodSpec *api.PodSpec + if oldPod != nil { + oldPodSpec = &oldPod.Spec + } + DropDisabledFields(&newPod.Spec, oldPodSpec) + + // old pod should never be changed + if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { + t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod())) + } + + switch { + case enabled || oldPodHasProcMount: + // new pod should not be changed if the feature is enabled, or if the old pod had ProcMount + if !reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) + } + case newPodHasProcMount: + // new pod should be changed + if reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod was not changed") + } + // new pod should not have ProcMount + if !reflect.DeepEqual(newPod, podWithoutProcMount()) { + t.Errorf("new pod had ProcMount: %v", diff.ObjectReflectDiff(newPod, podWithoutProcMount())) + } + default: + // new pod should not need to be changed + if !reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) + } + } + }) + } + } + } +} diff --git a/pkg/apis/core/fuzzer/fuzzer.go b/pkg/apis/core/fuzzer/fuzzer.go index 391226ba00..2a6b66e456 100644 --- a/pkg/apis/core/fuzzer/fuzzer.go +++ b/pkg/apis/core/fuzzer/fuzzer.go @@ -354,6 +354,10 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { c.Fuzz(&sc.Capabilities.Add) c.Fuzz(&sc.Capabilities.Drop) } + if sc.ProcMount == nil { + defProcMount := core.DefaultProcMount + sc.ProcMount = &defProcMount + } }, func(s *core.Secret, c fuzz.Continue) { c.FuzzNoCustom(s) // fuzz self without calling this function again diff --git a/pkg/apis/core/v1/defaults.go b/pkg/apis/core/v1/defaults.go index 172d3797bf..6a5064aeaf 100644 --- a/pkg/apis/core/v1/defaults.go +++ b/pkg/apis/core/v1/defaults.go @@ -423,3 +423,10 @@ func SetDefaults_HostPathVolumeSource(obj *v1.HostPathVolumeSource) { obj.Type = &typeVol } } + +func SetDefaults_SecurityContext(obj *v1.SecurityContext) { + if obj.ProcMount == nil { + defProcMount := v1.DefaultProcMount + obj.ProcMount = &defProcMount + } +} diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index ba9463d563..9996ebbdd2 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -3468,7 +3468,6 @@ func ValidatePodSecurityContext(securityContext *core.PodSecurityContext, spec * allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsGroup"), *(securityContext.RunAsGroup), msg)) } } - for g, gid := range securityContext.SupplementalGroups { for _, msg := range validation.IsValidGroupID(gid) { allErrs = append(allErrs, field.Invalid(fldPath.Child("supplementalGroups").Index(g), gid, msg)) @@ -5272,6 +5271,12 @@ func ValidateSecurityContext(sc *core.SecurityContext, fldPath *field.Path) fiel } } + if sc.ProcMount != nil { + if err := IsValidProcMount(*sc.ProcMount); err != nil { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("procMount"), *sc.ProcMount, []string{string(core.DefaultProcMount), string(core.UnmaskedProcMount)})) + } + } + if sc.AllowPrivilegeEscalation != nil && !*sc.AllowPrivilegeEscalation { if sc.Privileged != nil && *sc.Privileged { allErrs = append(allErrs, field.Invalid(fldPath, sc, "cannot set `allowPrivilegeEscalation` to false and `privileged` to true")) @@ -5372,3 +5377,14 @@ func IsDecremented(update, old *int32) bool { } return *update < *old } + +// IsValidProcMount tests that the argument is a valid ProcMountType. +func IsValidProcMount(procMountType core.ProcMountType) error { + switch procMountType { + case core.DefaultProcMount: + case core.UnmaskedProcMount: + default: + return fmt.Errorf("unsupported ProcMount type %s", procMountType) + } + return nil +} diff --git a/pkg/securitycontext/fake.go b/pkg/securitycontext/fake.go index 3303db2126..a0a4ae193a 100644 --- a/pkg/securitycontext/fake.go +++ b/pkg/securitycontext/fake.go @@ -25,9 +25,11 @@ import ( // empty container defaults. Used for testing. func ValidSecurityContextWithContainerDefaults() *v1.SecurityContext { priv := false + defProcMount := v1.DefaultProcMount return &v1.SecurityContext{ Capabilities: &v1.Capabilities{}, Privileged: &priv, + ProcMount: &defProcMount, } }