From 702f00c2af2f8ebb6ed7108692bcc44d0bb7cbe8 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 10 Jun 2019 21:47:00 -0400 Subject: [PATCH] Fix incorrect procMount defaulting --- pkg/api/pod/util.go | 16 ++++++++++--- pkg/api/pod/util_test.go | 20 +++++++++++++--- pkg/apis/apps/v1/zz_generated.defaults.go | 24 ------------------- .../apps/v1beta1/zz_generated.defaults.go | 12 ---------- .../apps/v1beta2/zz_generated.defaults.go | 24 ------------------- pkg/apis/batch/v1/zz_generated.defaults.go | 6 ----- .../batch/v1beta1/zz_generated.defaults.go | 12 ---------- .../batch/v2alpha1/zz_generated.defaults.go | 12 ---------- pkg/apis/core/fuzzer/fuzzer.go | 4 ---- pkg/apis/core/v1/defaults.go | 7 ------ pkg/apis/core/v1/zz_generated.defaults.go | 18 -------------- .../v1beta1/zz_generated.defaults.go | 18 -------------- test/e2e/framework/deployment_util.go | 5 ++-- test/e2e/framework/jobs_util.go | 1 + test/e2e/framework/rs_util.go | 5 ++-- test/e2e/framework/statefulset_utils.go | 7 +++--- test/e2e/upgrades/apps/daemonsets.go | 7 +++--- 17 files changed, 45 insertions(+), 153 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 3bffd1bf29..3b21ae8196 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -406,12 +406,22 @@ func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) { defaultProcMount := api.DefaultProcMount for i := range podSpec.Containers { if podSpec.Containers[i].SecurityContext != nil { - podSpec.Containers[i].SecurityContext.ProcMount = &defaultProcMount + if podSpec.Containers[i].SecurityContext.ProcMount != nil { + // The ProcMount field was improperly forced to non-nil in 1.12. + // If the feature is disabled, and the existing object is not using any non-default values, and the ProcMount field is present in the incoming object, force to the default value. + // Note: we cannot force the field to nil when the feature is disabled because it causes a diff against previously persisted data. + podSpec.Containers[i].SecurityContext.ProcMount = &defaultProcMount + } } } for i := range podSpec.InitContainers { if podSpec.InitContainers[i].SecurityContext != nil { - podSpec.InitContainers[i].SecurityContext.ProcMount = &defaultProcMount + if podSpec.InitContainers[i].SecurityContext.ProcMount != nil { + // The ProcMount field was improperly forced to non-nil in 1.12. + // If the feature is disabled, and the existing object is not using any non-default values, and the ProcMount field is present in the incoming object, force to the default value. + // Note: we cannot force the field to nil when the feature is disabled because it causes a diff against previously persisted data. + podSpec.InitContainers[i].SecurityContext.ProcMount = &defaultProcMount + } } } } @@ -473,7 +483,7 @@ 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 +// procMountInUse returns true if the pod spec is non-nil and has a SecurityContext's ProcMount field set to a non-default value func procMountInUse(podSpec *api.PodSpec) bool { if podSpec == nil { return false diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index aff0726cd7..7b0b3e3474 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -616,7 +616,7 @@ func TestDropProcMount(t *testing.T) { }, } } - podWithoutProcMount := func() *api.Pod { + podWithDefaultProcMount := func() *api.Pod { return &api.Pod{ Spec: api.PodSpec{ RestartPolicy: api.RestartPolicyNever, @@ -625,6 +625,15 @@ func TestDropProcMount(t *testing.T) { }, } } + podWithoutProcMount := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + Containers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: &api.SecurityContext{ProcMount: nil}}}, + InitContainers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: &api.SecurityContext{ProcMount: nil}}}, + }, + } + } podInfo := []struct { description string @@ -636,6 +645,11 @@ func TestDropProcMount(t *testing.T) { hasProcMount: true, pod: podWithProcMount, }, + { + description: "has default ProcMount", + hasProcMount: false, + pod: podWithDefaultProcMount, + }, { description: "does not have ProcMount", hasProcMount: false, @@ -683,8 +697,8 @@ func TestDropProcMount(t *testing.T) { 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())) + if procMountInUse(&newPod.Spec) { + t.Errorf("new pod had ProcMount: %#v", &newPod.Spec) } default: // new pod should not need to be changed diff --git a/pkg/apis/apps/v1/zz_generated.defaults.go b/pkg/apis/apps/v1/zz_generated.defaults.go index 1c63c1917c..4b541a3b15 100644 --- a/pkg/apis/apps/v1/zz_generated.defaults.go +++ b/pkg/apis/apps/v1/zz_generated.defaults.go @@ -136,9 +136,6 @@ func SetObjectDefaults_DaemonSet(in *v1.DaemonSet) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -181,9 +178,6 @@ func SetObjectDefaults_DaemonSet(in *v1.DaemonSet) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -289,9 +283,6 @@ func SetObjectDefaults_Deployment(in *v1.Deployment) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -334,9 +325,6 @@ func SetObjectDefaults_Deployment(in *v1.Deployment) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -442,9 +430,6 @@ func SetObjectDefaults_ReplicaSet(in *v1.ReplicaSet) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -487,9 +472,6 @@ func SetObjectDefaults_ReplicaSet(in *v1.ReplicaSet) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -595,9 +577,6 @@ func SetObjectDefaults_StatefulSet(in *v1.StatefulSet) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -640,9 +619,6 @@ func SetObjectDefaults_StatefulSet(in *v1.StatefulSet) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.VolumeClaimTemplates { a := &in.Spec.VolumeClaimTemplates[i] diff --git a/pkg/apis/apps/v1beta1/zz_generated.defaults.go b/pkg/apis/apps/v1beta1/zz_generated.defaults.go index 4a31e2e169..656f61edc9 100644 --- a/pkg/apis/apps/v1beta1/zz_generated.defaults.go +++ b/pkg/apis/apps/v1beta1/zz_generated.defaults.go @@ -132,9 +132,6 @@ func SetObjectDefaults_Deployment(in *v1beta1.Deployment) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -177,9 +174,6 @@ func SetObjectDefaults_Deployment(in *v1beta1.Deployment) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -285,9 +279,6 @@ func SetObjectDefaults_StatefulSet(in *v1beta1.StatefulSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -330,9 +321,6 @@ func SetObjectDefaults_StatefulSet(in *v1beta1.StatefulSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.VolumeClaimTemplates { a := &in.Spec.VolumeClaimTemplates[i] diff --git a/pkg/apis/apps/v1beta2/zz_generated.defaults.go b/pkg/apis/apps/v1beta2/zz_generated.defaults.go index 847a56b0d0..713bcaa781 100644 --- a/pkg/apis/apps/v1beta2/zz_generated.defaults.go +++ b/pkg/apis/apps/v1beta2/zz_generated.defaults.go @@ -136,9 +136,6 @@ func SetObjectDefaults_DaemonSet(in *v1beta2.DaemonSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -181,9 +178,6 @@ func SetObjectDefaults_DaemonSet(in *v1beta2.DaemonSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -289,9 +283,6 @@ func SetObjectDefaults_Deployment(in *v1beta2.Deployment) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -334,9 +325,6 @@ func SetObjectDefaults_Deployment(in *v1beta2.Deployment) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -442,9 +430,6 @@ func SetObjectDefaults_ReplicaSet(in *v1beta2.ReplicaSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -487,9 +472,6 @@ func SetObjectDefaults_ReplicaSet(in *v1beta2.ReplicaSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -595,9 +577,6 @@ func SetObjectDefaults_StatefulSet(in *v1beta2.StatefulSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -640,9 +619,6 @@ func SetObjectDefaults_StatefulSet(in *v1beta2.StatefulSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.VolumeClaimTemplates { a := &in.Spec.VolumeClaimTemplates[i] diff --git a/pkg/apis/batch/v1/zz_generated.defaults.go b/pkg/apis/batch/v1/zz_generated.defaults.go index 8c0f02b469..201e3b2285 100644 --- a/pkg/apis/batch/v1/zz_generated.defaults.go +++ b/pkg/apis/batch/v1/zz_generated.defaults.go @@ -130,9 +130,6 @@ func SetObjectDefaults_Job(in *v1.Job) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -175,9 +172,6 @@ func SetObjectDefaults_Job(in *v1.Job) { } } } - if a.SecurityContext != nil { - corev1.SetDefaults_SecurityContext(a.SecurityContext) - } } } diff --git a/pkg/apis/batch/v1beta1/zz_generated.defaults.go b/pkg/apis/batch/v1beta1/zz_generated.defaults.go index a9a1a5d067..d90031c32a 100644 --- a/pkg/apis/batch/v1beta1/zz_generated.defaults.go +++ b/pkg/apis/batch/v1beta1/zz_generated.defaults.go @@ -131,9 +131,6 @@ func SetObjectDefaults_CronJob(in *v1beta1.CronJob) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.JobTemplate.Spec.Template.Spec.Containers { a := &in.Spec.JobTemplate.Spec.Template.Spec.Containers[i] @@ -176,9 +173,6 @@ func SetObjectDefaults_CronJob(in *v1beta1.CronJob) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -283,9 +277,6 @@ func SetObjectDefaults_JobTemplate(in *v1beta1.JobTemplate) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Template.Spec.Template.Spec.Containers { a := &in.Template.Spec.Template.Spec.Containers[i] @@ -328,8 +319,5 @@ func SetObjectDefaults_JobTemplate(in *v1beta1.JobTemplate) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } diff --git a/pkg/apis/batch/v2alpha1/zz_generated.defaults.go b/pkg/apis/batch/v2alpha1/zz_generated.defaults.go index 8156eaac3f..367c56b15e 100644 --- a/pkg/apis/batch/v2alpha1/zz_generated.defaults.go +++ b/pkg/apis/batch/v2alpha1/zz_generated.defaults.go @@ -131,9 +131,6 @@ func SetObjectDefaults_CronJob(in *v2alpha1.CronJob) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.JobTemplate.Spec.Template.Spec.Containers { a := &in.Spec.JobTemplate.Spec.Template.Spec.Containers[i] @@ -176,9 +173,6 @@ func SetObjectDefaults_CronJob(in *v2alpha1.CronJob) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -283,9 +277,6 @@ func SetObjectDefaults_JobTemplate(in *v2alpha1.JobTemplate) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Template.Spec.Template.Spec.Containers { a := &in.Template.Spec.Template.Spec.Containers[i] @@ -328,8 +319,5 @@ func SetObjectDefaults_JobTemplate(in *v2alpha1.JobTemplate) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } diff --git a/pkg/apis/core/fuzzer/fuzzer.go b/pkg/apis/core/fuzzer/fuzzer.go index 2a6b66e456..391226ba00 100644 --- a/pkg/apis/core/fuzzer/fuzzer.go +++ b/pkg/apis/core/fuzzer/fuzzer.go @@ -354,10 +354,6 @@ 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 d06356a89d..4f681a5a77 100644 --- a/pkg/apis/core/v1/defaults.go +++ b/pkg/apis/core/v1/defaults.go @@ -421,10 +421,3 @@ 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/v1/zz_generated.defaults.go b/pkg/apis/core/v1/zz_generated.defaults.go index 0ea5e0fae0..00e0b384aa 100644 --- a/pkg/apis/core/v1/zz_generated.defaults.go +++ b/pkg/apis/core/v1/zz_generated.defaults.go @@ -263,9 +263,6 @@ func SetObjectDefaults_Pod(in *v1.Pod) { } } } - if a.SecurityContext != nil { - SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Containers { a := &in.Spec.Containers[i] @@ -308,9 +305,6 @@ func SetObjectDefaults_Pod(in *v1.Pod) { } } } - if a.SecurityContext != nil { - SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -415,9 +409,6 @@ func SetObjectDefaults_PodTemplate(in *v1.PodTemplate) { } } } - if a.SecurityContext != nil { - SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Template.Spec.Containers { a := &in.Template.Spec.Containers[i] @@ -460,9 +451,6 @@ func SetObjectDefaults_PodTemplate(in *v1.PodTemplate) { } } } - if a.SecurityContext != nil { - SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -569,9 +557,6 @@ func SetObjectDefaults_ReplicationController(in *v1.ReplicationController) { } } } - if a.SecurityContext != nil { - SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -614,9 +599,6 @@ func SetObjectDefaults_ReplicationController(in *v1.ReplicationController) { } } } - if a.SecurityContext != nil { - SetDefaults_SecurityContext(a.SecurityContext) - } } } } diff --git a/pkg/apis/extensions/v1beta1/zz_generated.defaults.go b/pkg/apis/extensions/v1beta1/zz_generated.defaults.go index ce23c6b801..0fa4c321c2 100644 --- a/pkg/apis/extensions/v1beta1/zz_generated.defaults.go +++ b/pkg/apis/extensions/v1beta1/zz_generated.defaults.go @@ -138,9 +138,6 @@ func SetObjectDefaults_DaemonSet(in *v1beta1.DaemonSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -183,9 +180,6 @@ func SetObjectDefaults_DaemonSet(in *v1beta1.DaemonSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -291,9 +285,6 @@ func SetObjectDefaults_Deployment(in *v1beta1.Deployment) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -336,9 +327,6 @@ func SetObjectDefaults_Deployment(in *v1beta1.Deployment) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } @@ -466,9 +454,6 @@ func SetObjectDefaults_ReplicaSet(in *v1beta1.ReplicaSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } for i := range in.Spec.Template.Spec.Containers { a := &in.Spec.Template.Spec.Containers[i] @@ -511,9 +496,6 @@ func SetObjectDefaults_ReplicaSet(in *v1beta1.ReplicaSet) { } } } - if a.SecurityContext != nil { - v1.SetDefaults_SecurityContext(a.SecurityContext) - } } } diff --git a/test/e2e/framework/deployment_util.go b/test/e2e/framework/deployment_util.go index a525d47ecc..9c84aefdd8 100644 --- a/test/e2e/framework/deployment_util.go +++ b/test/e2e/framework/deployment_util.go @@ -107,8 +107,9 @@ func NewDeployment(deploymentName string, replicas int32, podLabels map[string]s TerminationGracePeriodSeconds: &zero, Containers: []v1.Container{ { - Name: imageName, - Image: image, + Name: imageName, + Image: image, + SecurityContext: &v1.SecurityContext{}, }, }, }, diff --git a/test/e2e/framework/jobs_util.go b/test/e2e/framework/jobs_util.go index a5839dc36f..e54be2829e 100644 --- a/test/e2e/framework/jobs_util.go +++ b/test/e2e/framework/jobs_util.go @@ -83,6 +83,7 @@ func NewTestJob(behavior, name string, rPol v1.RestartPolicy, parallelism, compl Name: "data", }, }, + SecurityContext: &v1.SecurityContext{}, }, }, }, diff --git a/test/e2e/framework/rs_util.go b/test/e2e/framework/rs_util.go index cfdf0975ad..93bb2e4390 100644 --- a/test/e2e/framework/rs_util.go +++ b/test/e2e/framework/rs_util.go @@ -148,8 +148,9 @@ func NewReplicaSet(name, namespace string, replicas int32, podLabels map[string] Spec: v1.PodSpec{ Containers: []v1.Container{ { - Name: imageName, - Image: image, + Name: imageName, + Image: image, + SecurityContext: &v1.SecurityContext{}, }, }, }, diff --git a/test/e2e/framework/statefulset_utils.go b/test/e2e/framework/statefulset_utils.go index 24d8a39964..6bcde3c0b2 100644 --- a/test/e2e/framework/statefulset_utils.go +++ b/test/e2e/framework/statefulset_utils.go @@ -807,9 +807,10 @@ func NewStatefulSet(name, ns, governingSvcName string, replicas int32, statefulP Spec: v1.PodSpec{ Containers: []v1.Container{ { - Name: "nginx", - Image: imageutils.GetE2EImage(imageutils.Nginx), - VolumeMounts: mounts, + Name: "nginx", + Image: imageutils.GetE2EImage(imageutils.Nginx), + VolumeMounts: mounts, + SecurityContext: &v1.SecurityContext{}, }, }, Volumes: vols, diff --git a/test/e2e/upgrades/apps/daemonsets.go b/test/e2e/upgrades/apps/daemonsets.go index 98997480a4..4529a25a8b 100644 --- a/test/e2e/upgrades/apps/daemonsets.go +++ b/test/e2e/upgrades/apps/daemonsets.go @@ -65,9 +65,10 @@ func (t *DaemonSetUpgradeTest) Setup(f *framework.Framework) { }, Containers: []v1.Container{ { - Name: daemonSetName, - Image: image, - Ports: []v1.ContainerPort{{ContainerPort: 9376}}, + Name: daemonSetName, + Image: image, + Ports: []v1.ContainerPort{{ContainerPort: 9376}}, + SecurityContext: &v1.SecurityContext{}, }, }, },