Merge pull request #79387 from tedyu/cont-helper-early

Restore early return for podSpecHasContainer

pkg/apis/core/pods/helpers.go

@@ -24,20 +24,28 @@ import (
 	"k8s.io/kubernetes/pkg/fieldpath"
 )
 
-// ContainerVisitorWithPath is called with each container and the field.Path to that container
+// ContainerVisitorWithPath is called with each container and the field.Path to that container,
-type ContainerVisitorWithPath func(container *api.Container, path *field.Path)
+// and returns true if visiting should continue.
+type ContainerVisitorWithPath func(container *api.Container, path *field.Path) bool
 
 // VisitContainersWithPath invokes the visitor function with a pointer to the spec
 // of every container in the given pod spec and the field.Path to that container.
-func VisitContainersWithPath(podSpec *api.PodSpec, visitor ContainerVisitorWithPath) {
+// If visitor returns false, visiting is short-circuited. VisitContainersWithPath returns true if visiting completes,
+// false if visiting was short-circuited.
+func VisitContainersWithPath(podSpec *api.PodSpec, visitor ContainerVisitorWithPath) bool {
 	path := field.NewPath("spec", "initContainers")
 	for i := range podSpec.InitContainers {
-		visitor(&podSpec.InitContainers[i], path.Index(i))
+		if !visitor(&podSpec.InitContainers[i], path.Index(i)) {
+			return false
+		}
 	}
 	path = field.NewPath("spec", "containers")
 	for i := range podSpec.Containers {
-		visitor(&podSpec.Containers[i], path.Index(i))
+		if !visitor(&podSpec.Containers[i], path.Index(i)) {
+			return false
 		}
+	}
+	return true
 }
 
 // ConvertDownwardAPIFieldLabel converts the specified downward API field label

pkg/apis/core/pods/helpers_test.go

@@ -73,8 +73,9 @@ func TestVisitContainersWithPath(t *testing.T) {
 
 	for _, tc := range testCases {
 		gotNames := []string{}
-		VisitContainersWithPath(tc.haveSpec, func(c *api.Container, p *field.Path) {
+		VisitContainersWithPath(tc.haveSpec, func(c *api.Container, p *field.Path) bool {
 			gotNames = append(gotNames, p.String())
+			return true
 		})
 		if !reflect.DeepEqual(gotNames, tc.wantNames) {
 			t.Errorf("VisitContainersWithPath() for test case %q visited containers %q, wanted to visit %q", tc.description, gotNames, tc.wantNames)

pkg/apis/core/validation/validation.go

@@ -3435,10 +3435,12 @@ func ValidateAppArmorPodAnnotations(annotations map[string]string, spec *core.Po
 
 func podSpecHasContainer(spec *core.PodSpec, containerName string) bool {
 	var hasContainer bool
-	podshelper.VisitContainersWithPath(spec, func(c *core.Container, _ *field.Path) {
+	podshelper.VisitContainersWithPath(spec, func(c *core.Container, _ *field.Path) bool {
 		if c.Name == containerName {
 			hasContainer = true
+			return false
 		}
+		return true
 	})
 	return hasContainer
 }

pkg/security/podsecuritypolicy/provider.go

@@ -238,8 +238,9 @@ func (s *simpleProvider) ValidatePod(pod *api.Pod) field.ErrorList {
 		allErrs = append(allErrs, validateRuntimeClassName(pod.Spec.RuntimeClassName, s.psp.Spec.RuntimeClass.AllowedRuntimeClassNames)...)
 	}
 
-	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, p *field.Path) {
+	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, p *field.Path) bool {
 		allErrs = append(allErrs, s.validateContainer(pod, c, p)...)
+		return true
 	})
 
 	return allErrs
@@ -274,12 +275,13 @@ func (s *simpleProvider) validatePodVolumes(pod *api.Pod) field.ErrorList {
 						fmt.Sprintf("is not allowed to be used")))
 				} else if mustBeReadOnly {
 					// Ensure all the VolumeMounts that use this volume are read-only
-					pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, p *field.Path) {
+					pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, p *field.Path) bool {
 						for i, cv := range c.VolumeMounts {
 							if cv.Name == v.Name && !cv.ReadOnly {
 								allErrs = append(allErrs, field.Invalid(p.Child("volumeMounts").Index(i).Child("readOnly"), cv.ReadOnly, "must be read-only"))
 							}
 						}
+						return true
 					})
 				}
 

plugin/pkg/admission/alwayspullimages/admission.go

@@ -65,8 +65,9 @@ func (a *AlwaysPullImages) Admit(attributes admission.Attributes, o admission.Ob
 		return apierrors.NewBadRequest("Resource was marked with kind Pod but was unable to be converted")
 	}
 
-	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, _ *field.Path) {
+	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, _ *field.Path) bool {
 		c.ImagePullPolicy = api.PullAlways
+		return true
 	})
 
 	return nil
@@ -84,12 +85,13 @@ func (*AlwaysPullImages) Validate(attributes admission.Attributes, o admission.O
 	}
 
 	var allErrs []error
-	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, p *field.Path) {
+	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, p *field.Path) bool {
 		if c.ImagePullPolicy != api.PullAlways {
 			allErrs = append(allErrs, admission.NewForbidden(attributes,
 				field.NotSupported(p.Child("imagePullPolicy"), c.ImagePullPolicy, []string{string(api.PullAlways)}),
 			))
 		}
+		return true
 	})
 	if len(allErrs) > 0 {
 		return utilerrors.NewAggregate(allErrs)

plugin/pkg/admission/podpreset/admission.go

@@ -185,10 +185,11 @@ func safeToApplyPodPresetsOnPod(pod *api.Pod, podPresets []*settingsv1alpha1.Pod
 	if _, err := mergeVolumes(pod.Spec.Volumes, podPresets); err != nil {
 		errs = append(errs, err)
 	}
-	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, _ *field.Path) {
+	pods.VisitContainersWithPath(&pod.Spec, func(c *api.Container, _ *field.Path) bool {
 		if err := safeToApplyPodPresetsOnContainer(c, podPresets); err != nil {
 			errs = append(errs, err)
 		}
+		return true
 	})
 
 	return utilerrors.NewAggregate(errs)