diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index c7eb7713924..0fe02c5dc8d 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1960,6 +1960,23 @@ func ValidatePodSecurityContext(securityContext *api.PodSecurityContext, spec *a return allErrs } +func validateContainerUpdates(newContainers, oldContainers []api.Container, fldPath *field.Path) (allErrs field.ErrorList, stop bool) { + allErrs = field.ErrorList{} + if len(newContainers) != len(oldContainers) { + //TODO: Pinpoint the specific container that causes the invalid error after we have strategic merge diff + allErrs = append(allErrs, field.Forbidden(fldPath, "pod updates may not add or remove containers")) + return allErrs, true + } + + // validate updated container images + for i, ctr := range newContainers { + if len(ctr.Image) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Index(i).Child("image"), "")) + } + } + return allErrs, false +} + // ValidatePodUpdate tests to see if the update is legal for an end user to make. newPod is updated with fields // that cannot be changed. func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList { @@ -1967,21 +1984,21 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList { allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath) allErrs = append(allErrs, ValidatePodSpecificAnnotations(newPod.ObjectMeta.Annotations, fldPath.Child("annotations"))...) specPath := field.NewPath("spec") - if len(newPod.Spec.Containers) != len(oldPod.Spec.Containers) { - //TODO: Pinpoint the specific container that causes the invalid error after we have strategic merge diff - allErrs = append(allErrs, field.Forbidden(specPath.Child("containers"), "pod updates may not add or remove containers")) - return allErrs - } // validate updateable fields: // 1. containers[*].image - // 2. spec.activeDeadlineSeconds + // 2. initContainers[*].image + // 3. spec.activeDeadlineSeconds - // validate updated container images - for i, ctr := range newPod.Spec.Containers { - if len(ctr.Image) == 0 { - allErrs = append(allErrs, field.Required(specPath.Child("containers").Index(i).Child("image"), "")) - } + containerErrs, stop := validateContainerUpdates(newPod.Spec.Containers, oldPod.Spec.Containers, specPath.Child("containers")) + allErrs = append(allErrs, containerErrs...) + if stop { + return allErrs + } + containerErrs, stop = validateContainerUpdates(newPod.Spec.InitContainers, oldPod.Spec.InitContainers, specPath.Child("initContainers")) + allErrs = append(allErrs, containerErrs...) + if stop { + return allErrs } // validate updated spec.activeDeadlineSeconds. two types of updates are allowed: @@ -2013,6 +2030,13 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList { newContainers = append(newContainers, container) } mungedPod.Spec.Containers = newContainers + // munge initContainers[*].image + var newInitContainers []api.Container + for ix, container := range mungedPod.Spec.InitContainers { + container.Image = oldPod.Spec.InitContainers[ix].Image + newInitContainers = append(newInitContainers, container) + } + mungedPod.Spec.InitContainers = newInitContainers // munge spec.activeDeadlineSeconds mungedPod.Spec.ActiveDeadlineSeconds = nil if oldPod.Spec.ActiveDeadlineSeconds != nil { diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index ee48e7c0122..d3fe378ec84 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -1907,8 +1907,9 @@ func TestValidatePodSpec(t *testing.T) { Volumes: []api.Volume{ {Name: "vol", VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}, }, - Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}}, - RestartPolicy: api.RestartPolicyAlways, + Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}}, + InitContainers: []api.Container{{Name: "ictr", Image: "iimage", ImagePullPolicy: "IfNotPresent"}}, + RestartPolicy: api.RestartPolicyAlways, NodeSelector: map[string]string{ "key": "value", }, @@ -1999,6 +2000,12 @@ func TestValidatePodSpec(t *testing.T) { RestartPolicy: api.RestartPolicyAlways, DNSPolicy: api.DNSClusterFirst, }, + "bad init container": { + Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent"}}, + InitContainers: []api.Container{{}}, + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + }, "bad DNS policy": { DNSPolicy: api.DNSPolicy("invalid"), RestartPolicy: api.RestartPolicyAlways, @@ -3022,6 +3029,35 @@ func TestValidatePodUpdate(t *testing.T) { false, "more containers", }, + { + api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + }, + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Image: "foo:V1", + }, + }, + }, + }, + api.Pod{ + ObjectMeta: api.ObjectMeta{Name: "foo"}, + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Image: "foo:V2", + }, + { + Image: "bar:V2", + }, + }, + }, + }, + false, + "more init containers", + }, { api.Pod{ ObjectMeta: api.ObjectMeta{Name: "foo"}, @@ -3070,6 +3106,30 @@ func TestValidatePodUpdate(t *testing.T) { true, "image change", }, + { + api.Pod{ + ObjectMeta: api.ObjectMeta{Name: "foo"}, + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Image: "foo:V1", + }, + }, + }, + }, + api.Pod{ + ObjectMeta: api.ObjectMeta{Name: "foo"}, + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Image: "foo:V2", + }, + }, + }, + }, + true, + "init container image change", + }, { api.Pod{ ObjectMeta: api.ObjectMeta{Name: "foo"}, @@ -3092,6 +3152,28 @@ func TestValidatePodUpdate(t *testing.T) { false, "image change to empty", }, + { + api.Pod{ + ObjectMeta: api.ObjectMeta{Name: "foo"}, + Spec: api.PodSpec{ + InitContainers: []api.Container{ + {}, + }, + }, + }, + api.Pod{ + ObjectMeta: api.ObjectMeta{Name: "foo"}, + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Image: "foo:V2", + }, + }, + }, + }, + false, + "init container image change to empty", + }, { api.Pod{ Spec: api.PodSpec{}, diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 835ae380b3e..ca7b89ba2a2 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -131,7 +131,6 @@ func addGrouplessTypes() { api.Scheme.AddKnownTypes(grouplessGroupVersion, &apiservertesting.Simple{}, &apiservertesting.SimpleList{}, &ListOptions{}, &api.DeleteOptions{}, &apiservertesting.SimpleGetOptions{}, &apiservertesting.SimpleRoot{}) - api.Scheme.AddKnownTypes(grouplessGroupVersion, &api.Pod{}) api.Scheme.AddKnownTypes(grouplessInternalGroupVersion, &apiservertesting.Simple{}, &apiservertesting.SimpleList{}, &api.ListOptions{}, &apiservertesting.SimpleGetOptions{}, &apiservertesting.SimpleRoot{}) diff --git a/pkg/apiserver/resthandler_test.go b/pkg/apiserver/resthandler_test.go index 5726fc70474..1fe870aa560 100644 --- a/pkg/apiserver/resthandler_test.go +++ b/pkg/apiserver/resthandler_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/kubernetes/pkg/api/rest" "k8s.io/kubernetes/pkg/api/testapi" "k8s.io/kubernetes/pkg/api/unversioned" + "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util/diff" @@ -186,12 +187,7 @@ func (tc *patchTestCase) Run(t *testing.T) { namer := &testNamer{namespace, name} copier := runtime.ObjectCopier(api.Scheme) resource := unversioned.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} - - versionedObj, err := api.Scheme.ConvertToVersion(&api.Pod{}, unversioned.GroupVersion{Version: "v1"}) - if err != nil { - t.Errorf("%s: unexpected error: %v", tc.name, err) - return - } + versionedObj := &v1.Pod{} for _, patchType := range []api.PatchType{api.JSONPatchType, api.MergePatchType, api.StrategicMergePatchType} { // TODO SUPPORT THIS!