From 25e990f738bb98b505142ede7350d591b3fb904c Mon Sep 17 00:00:00 2001 From: Brian McQueen Date: Sun, 27 Nov 2022 08:59:26 -0800 Subject: [PATCH] added validation check to block adding an ephemeral container to a static pod and test cases --- pkg/apis/core/validation/validation.go | 5 + pkg/apis/core/validation/validation_test.go | 28 +++ pkg/registry/core/pod/strategy_test.go | 256 ++++++++++++++++++++ 3 files changed, 289 insertions(+) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index d53339863b3..2f28c030d2b 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4688,6 +4688,11 @@ func ValidatePodEphemeralContainersUpdate(newPod, oldPod *core.Pod, opts PodVali allErrs = append(allErrs, validatePodMetadataAndSpec(newPod, opts)...) allErrs = append(allErrs, ValidatePodSpecificAnnotationUpdates(newPod, oldPod, fldPath.Child("annotations"), opts)...) + // static pods don't support ephemeral containers #113935 + if _, ok := oldPod.Annotations[core.MirrorPodAnnotationKey]; ok { + return field.ErrorList{field.Forbidden(field.NewPath(""), "static pods do not support ephemeral containers")} + } + // Part 2: Validate that the changes between oldPod.Spec.EphemeralContainers and // newPod.Spec.EphemeralContainers are allowed. // diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index bf669a7fbf3..c1e24e51e4f 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -12776,6 +12776,34 @@ func TestValidatePodEphemeralContainersUpdate(t *testing.T) { makeWindowsHostPod(nil), "spec.ephemeralContainers[0].securityContext.windowsOptions.hostProcess: Invalid value: false: pod hostProcess value must be identical", }, + { + "Add ephemeral container to static pod", + func() *core.Pod { + p := makePod(nil) + p.Spec.NodeName = "some-name" + p.ObjectMeta.Annotations = map[string]string{ + core.MirrorPodAnnotationKey: "foo", + } + p.Spec.EphemeralContainers = []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debugger1", + Image: "debian", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }} + return p + }(), + func() *core.Pod { + p := makePod(nil) + p.Spec.NodeName = "some-name" + p.ObjectMeta.Annotations = map[string]string{ + core.MirrorPodAnnotationKey: "foo", + } + return p + }(), + "Forbidden: static pods do not support ephemeral containers", + }, } for _, tc := range tests { diff --git a/pkg/registry/core/pod/strategy_test.go b/pkg/registry/core/pod/strategy_test.go index 65a4a10c03f..c6d50ddacac 100644 --- a/pkg/registry/core/pod/strategy_test.go +++ b/pkg/registry/core/pod/strategy_test.go @@ -1267,6 +1267,262 @@ func TestPodStrategyValidate(t *testing.T) { } } +func TestEphemeralContainerStrategyValidateUpdate(t *testing.T) { + + test := []struct { + name string + newPod *api.Pod + oldPod *api.Pod + }{ + { + name: "add ephemeral container to regular pod and expect success", + oldPod: &api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + ResourceVersion: "1", + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSDefault, + Containers: []api.Container{ + { + Name: "container", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + }, + }, + newPod: &api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + ResourceVersion: "1", + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSDefault, + Containers: []api.Container{ + { + Name: "container", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + EphemeralContainers: []api.EphemeralContainer{ + { + EphemeralContainerCommon: api.EphemeralContainerCommon{ + Name: "debugger", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + }, + }, + }, + }, + } + + // expect no errors + for _, tc := range test { + t.Run(tc.name, func(t *testing.T) { + if errs := EphemeralContainersStrategy.ValidateUpdate(genericapirequest.NewContext(), tc.newPod, tc.oldPod); len(errs) != 0 { + t.Errorf("unexpected error:%v", errs) + } + }) + } + + test = []struct { + name string + newPod *api.Pod + oldPod *api.Pod + }{ + { + name: "add ephemeral container to static pod and expect failure", + oldPod: &api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + ResourceVersion: "1", + Annotations: map[string]string{api.MirrorPodAnnotationKey: "someVal"}, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSDefault, + Containers: []api.Container{ + { + Name: "container", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + NodeName: "example.com", + }, + }, + newPod: &api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + ResourceVersion: "1", + Annotations: map[string]string{api.MirrorPodAnnotationKey: "someVal"}, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSDefault, + Containers: []api.Container{ + { + Name: "container", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + EphemeralContainers: []api.EphemeralContainer{ + { + EphemeralContainerCommon: api.EphemeralContainerCommon{ + Name: "debugger", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + }, + NodeName: "example.com", + }, + }, + }, + { + name: "remove ephemeral container from regular pod and expect failure", + newPod: &api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + ResourceVersion: "1", + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSDefault, + Containers: []api.Container{ + { + Name: "container", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + }, + }, + oldPod: &api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + ResourceVersion: "1", + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSDefault, + Containers: []api.Container{ + { + Name: "container", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + EphemeralContainers: []api.EphemeralContainer{ + { + EphemeralContainerCommon: api.EphemeralContainerCommon{ + Name: "debugger", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + }, + }, + }, + }, + { + name: "change ephemeral container from regular pod and expect failure", + newPod: &api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + ResourceVersion: "1", + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSDefault, + Containers: []api.Container{ + { + Name: "container", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + EphemeralContainers: []api.EphemeralContainer{ + { + EphemeralContainerCommon: api.EphemeralContainerCommon{ + Name: "debugger", + Image: "image2", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + }, + }, + }, + oldPod: &api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-ns", + ResourceVersion: "1", + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSDefault, + Containers: []api.Container{ + { + Name: "container", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + EphemeralContainers: []api.EphemeralContainer{ + { + EphemeralContainerCommon: api.EphemeralContainerCommon{ + Name: "debugger", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + }, + }, + }, + }, + } + + // expect one error + for _, tc := range test { + t.Run(tc.name, func(t *testing.T) { + errs := EphemeralContainersStrategy.ValidateUpdate(genericapirequest.NewContext(), tc.newPod, tc.oldPod) + if len(errs) == 0 { + t.Errorf("unexpected success:ephemeral containers are not supported for static pods") + } else if len(errs) != 1 { + t.Errorf("unexpected errors:expected one error about ephemeral containers are not supported for static pods:got:%v:", errs) + } + }) + } +} + func TestPodStrategyValidateUpdate(t *testing.T) { test := []struct { name string