From 4451138bfa784942e59dc092fc8d707e73eaa284 Mon Sep 17 00:00:00 2001
From: Lee Verberne <verb@google.com>
Date: Mon, 27 Sep 2021 22:39:06 +0200
Subject: [PATCH] Validate PodSpec in EphemeralContainersUpdate

Previously this only validated the ephemeral containers, but it's safer
to validate the entire PodSpec in case other parts of validation add
logic that checks ephemeral containers.
---
 pkg/apis/core/validation/validation.go      | 17 +++++++-------
 pkg/apis/core/validation/validation_test.go | 26 +++++++++++++++++++--
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go
index 0441a9a8702..781e9787e8e 100644
--- a/pkg/apis/core/validation/validation.go
+++ b/pkg/apis/core/validation/validation.go
@@ -4213,17 +4213,18 @@ func validatePodConditions(conditions []core.PodCondition, fldPath *field.Path)
 // ValidatePodEphemeralContainersUpdate tests that a user update to EphemeralContainers is valid.
 // newPod and oldPod must only differ in their EphemeralContainers.
 func ValidatePodEphemeralContainersUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) field.ErrorList {
-	spec := newPod.Spec
-	specPath := field.NewPath("spec").Child("ephemeralContainers")
-
-	vols := make(map[string]core.VolumeSource)
-	for _, vol := range spec.Volumes {
-		vols[vol.Name] = vol.VolumeSource
-	}
-	allErrs := validateEphemeralContainers(spec.EphemeralContainers, spec.Containers, spec.InitContainers, vols, specPath, opts)
+	// Part 1: Validate newPod's spec and updates to metadata
+	fldPath := field.NewPath("metadata")
+	allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath)
+	allErrs = append(allErrs, validatePodMetadataAndSpec(newPod, opts)...)
+	allErrs = append(allErrs, ValidatePodSpecificAnnotationUpdates(newPod, oldPod, fldPath.Child("annotations"), opts)...)
 
+	// Part 2: Validate that the changes between oldPod.Spec.EphemeralContainers and
+	// newPod.Spec.EphemeralContainers are allowed.
+	//
 	// Existing EphemeralContainers may not be changed. Order isn't preserved by patch, so check each individually.
 	newContainerIndex := make(map[string]*core.EphemeralContainer)
+	specPath := field.NewPath("spec").Child("ephemeralContainers")
 	for i := range newPod.Spec.EphemeralContainers {
 		newContainerIndex[newPod.Spec.EphemeralContainers[i].Name] = &newPod.Spec.EphemeralContainers[i]
 	}
diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go
index cc2c52845c8..8de1dc8b172 100644
--- a/pkg/apis/core/validation/validation_test.go
+++ b/pkg/apis/core/validation/validation_test.go
@@ -10616,9 +10616,31 @@ func TestValidatePodEphemeralContainersUpdate(t *testing.T) {
 		},
 	}
 
+	makePod := func(ephemeralContainers []core.EphemeralContainer) core.Pod {
+		return core.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations:     map[string]string{},
+				Labels:          map[string]string{},
+				Name:            "pod",
+				Namespace:       "ns",
+				ResourceVersion: "1",
+			},
+			Spec: core.PodSpec{
+				Containers: []core.Container{{
+					Name:                     "cnt",
+					Image:                    "image",
+					ImagePullPolicy:          "IfNotPresent",
+					TerminationMessagePolicy: "File",
+				}},
+				DNSPolicy:           core.DNSClusterFirst,
+				EphemeralContainers: ephemeralContainers,
+				RestartPolicy:       core.RestartPolicyOnFailure,
+			},
+		}
+	}
+
 	for _, test := range tests {
-		new := core.Pod{Spec: core.PodSpec{EphemeralContainers: test.new}}
-		old := core.Pod{Spec: core.PodSpec{EphemeralContainers: test.old}}
+		new, old := makePod(test.new), makePod(test.old)
 		errs := ValidatePodEphemeralContainersUpdate(&new, &old, PodValidationOptions{})
 		if test.err == "" {
 			if len(errs) != 0 {