diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index ffb91a497b0..880a3cd4983 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2907,6 +2907,8 @@ func validatePullPolicy(policy core.PullPolicy, fldPath *field.Path) field.Error return allErrors } +// validateEphemeralContainers is called by pod spec and template validation to validate the list of ephemeral containers. +// Note that this is called for pod template even though ephemeral containers aren't allowed in pod templates. func validateEphemeralContainers(ephemeralContainers []core.EphemeralContainer, containers, initContainers []core.Container, volumes map[string]core.VolumeSource, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { allErrs := field.ErrorList{} @@ -2914,40 +2916,40 @@ func validateEphemeralContainers(ephemeralContainers []core.EphemeralContainer, return allErrs } - allNames := sets.String{} + otherNames, allNames := sets.String{}, sets.String{} for _, c := range containers { + otherNames.Insert(c.Name) allNames.Insert(c.Name) } for _, c := range initContainers { + otherNames.Insert(c.Name) allNames.Insert(c.Name) } for i, ec := range ephemeralContainers { idxPath := fldPath.Index(i) - if ec.TargetContainerName != "" && !allNames.Has(ec.TargetContainerName) { - allErrs = append(allErrs, field.NotFound(idxPath.Child("targetContainerName"), ec.TargetContainerName)) - } - - if ec.Name == "" { - allErrs = append(allErrs, field.Required(idxPath.Child("name"), "ephemeralContainer requires a name")) - continue - } - c := (*core.Container)(&ec.EphemeralContainerCommon) - allErrs = append(allErrs, validateContainer(c, volumes, idxPath, opts)...) - // EphemeralContainers don't require the backwards-compatibility distinction between pod/podTemplate validation + allErrs = append(allErrs, validateContainerCommon(c, volumes, idxPath, opts)...) + // Ephemeral containers don't need looser constraints for pod templates, so it's convenient to apply both validations + // here where we've already converted EphemeralContainerCommon to Container. allErrs = append(allErrs, validateContainerOnlyForPod(c, idxPath)...) + // Ephemeral containers must have a name unique across all container types. if allNames.Has(ec.Name) { allErrs = append(allErrs, field.Duplicate(idxPath.Child("name"), ec.Name)) } else { allNames.Insert(ec.Name) } - // Ephemeral Containers should not be relied upon for fundamental pod services, so fields such as + // The target container name must exist and be non-ephemeral. + if ec.TargetContainerName != "" && !otherNames.Has(ec.TargetContainerName) { + allErrs = append(allErrs, field.NotFound(idxPath.Child("targetContainerName"), ec.TargetContainerName)) + } + + // Ephemeral containers should not be relied upon for fundamental pod services, so fields such as // Lifecycle, probes, resources and ports should be disallowed. This is implemented as a list - // of allowed fields so that new fields will be given consideration prior to inclusion in Ephemeral Containers. + // of allowed fields so that new fields will be given consideration prior to inclusion in ephemeral containers. allErrs = append(allErrs, validateFieldAllowList(ec.EphemeralContainerCommon, allowedEphemeralContainerFields, "cannot be set for an Ephemeral Container", idxPath)...) // VolumeMount subpaths have the potential to leak resources since they're implemented with bind mounts @@ -2989,44 +2991,52 @@ func validateFieldAllowList(value interface{}, allowedFields map[string]bool, er return allErrs } -func validateInitContainers(containers []core.Container, otherContainers []core.Container, volumes map[string]core.VolumeSource, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { +// validateInitContainers is called by pod spec and template validation to validate the list of init containers +func validateInitContainers(containers []core.Container, regularContainers []core.Container, volumes map[string]core.VolumeSource, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { var allErrs field.ErrorList allNames := sets.String{} - for _, ctr := range otherContainers { + for _, ctr := range regularContainers { allNames.Insert(ctr.Name) } for i, ctr := range containers { idxPath := fldPath.Index(i) + + // Apply the validation common to all container types + allErrs = append(allErrs, validateContainerCommon(&ctr, volumes, idxPath, opts)...) + + // Names must be unique within regular and init containers. Collisions with ephemeral containers + // will be detected by validateEphemeralContainers(). if allNames.Has(ctr.Name) { allErrs = append(allErrs, field.Duplicate(idxPath.Child("name"), ctr.Name)) } else if len(ctr.Name) > 0 { allNames.Insert(ctr.Name) } - allErrs = append(allErrs, validateContainer(&ctr, volumes, idxPath, opts)...) - // Check for port conflicts in init containers individually since init containers run one-by-one. allErrs = append(allErrs, checkHostPortConflicts([]core.Container{ctr}, fldPath)...) + // These fields are disallowed for init containers. if ctr.Lifecycle != nil { - allErrs = append(allErrs, field.Invalid(idxPath.Child("lifecycle"), ctr.Lifecycle, "must not be set for init containers")) + allErrs = append(allErrs, field.Forbidden(idxPath.Child("lifecycle"), "may not be set for init containers")) } if ctr.LivenessProbe != nil { - allErrs = append(allErrs, field.Invalid(idxPath.Child("livenessProbe"), ctr.LivenessProbe, "must not be set for init containers")) + allErrs = append(allErrs, field.Forbidden(idxPath.Child("livenessProbe"), "may not be set for init containers")) } if ctr.ReadinessProbe != nil { - allErrs = append(allErrs, field.Invalid(idxPath.Child("readinessProbe"), ctr.ReadinessProbe, "must not be set for init containers")) + allErrs = append(allErrs, field.Forbidden(idxPath.Child("readinessProbe"), "may not be set for init containers")) } if ctr.StartupProbe != nil { - allErrs = append(allErrs, field.Invalid(idxPath.Child("startupProbe"), ctr.StartupProbe, "must not be set for init containers")) + allErrs = append(allErrs, field.Forbidden(idxPath.Child("startupProbe"), "may not be set for init containers")) } } return allErrs } -func validateContainer(ctr *core.Container, volumes map[string]core.VolumeSource, path *field.Path, opts PodValidationOptions) field.ErrorList { +// validateContainerCommon applies validation common to all container types. It's called by regular, init, and ephemeral +// container list validation to require a properly formatted name, image, etc. +func validateContainerCommon(ctr *core.Container, volumes map[string]core.VolumeSource, path *field.Path, opts PodValidationOptions) field.ErrorList { allErrs := field.ErrorList{} namePath := path.Child("name") @@ -3042,37 +3052,21 @@ func validateContainer(ctr *core.Container, volumes map[string]core.VolumeSource if len(ctr.Image) == 0 { allErrs = append(allErrs, field.Required(path.Child("image"), "")) } - if ctr.Lifecycle != nil { - allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, path.Child("lifecycle"))...) - } - - allErrs = append(allErrs, validateProbe(ctr.LivenessProbe, path.Child("livenessProbe"))...) - // Readiness-specific validation - if ctr.ReadinessProbe != nil && ctr.ReadinessProbe.TerminationGracePeriodSeconds != nil { - allErrs = append(allErrs, field.Invalid(path.Child("readinessProbe", "terminationGracePeriodSeconds"), ctr.ReadinessProbe.TerminationGracePeriodSeconds, "must not be set for readinessProbes")) - } - allErrs = append(allErrs, validateProbe(ctr.StartupProbe, path.Child("startupProbe"))...) - // Liveness-specific validation - if ctr.LivenessProbe != nil && ctr.LivenessProbe.SuccessThreshold != 1 { - allErrs = append(allErrs, field.Invalid(path.Child("livenessProbe", "successThreshold"), ctr.LivenessProbe.SuccessThreshold, "must be 1")) - } - allErrs = append(allErrs, validateProbe(ctr.StartupProbe, path.Child("startupProbe"))...) - // Startup-specific validation - if ctr.StartupProbe != nil && ctr.StartupProbe.SuccessThreshold != 1 { - allErrs = append(allErrs, field.Invalid(path.Child("startupProbe", "successThreshold"), ctr.StartupProbe.SuccessThreshold, "must be 1")) - } switch ctr.TerminationMessagePolicy { case core.TerminationMessageReadFile, core.TerminationMessageFallbackToLogsOnError: case "": - allErrs = append(allErrs, field.Required(path.Child("terminationMessagePolicy"), "must be 'File' or 'FallbackToLogsOnError'")) + allErrs = append(allErrs, field.Required(path.Child("terminationMessagePolicy"), "")) default: - allErrs = append(allErrs, field.Invalid(path.Child("terminationMessagePolicy"), ctr.TerminationMessagePolicy, "must be 'File' or 'FallbackToLogsOnError'")) + supported := []string{ + string(core.TerminationMessageReadFile), + string(core.TerminationMessageFallbackToLogsOnError), + } + allErrs = append(allErrs, field.NotSupported(path.Child("terminationMessagePolicy"), ctr.TerminationMessagePolicy, supported)) } volMounts := GetVolumeMountMap(ctr.VolumeMounts) volDevices := GetVolumeDeviceMap(ctr.VolumeDevices) - allErrs = append(allErrs, validateProbe(ctr.ReadinessProbe, path.Child("readinessProbe"))...) allErrs = append(allErrs, validateContainerPorts(ctr.Ports, path.Child("ports"))...) allErrs = append(allErrs, ValidateEnv(ctr.Env, path.Child("env"), opts)...) allErrs = append(allErrs, ValidateEnvFrom(ctr.EnvFrom, path.Child("envFrom"))...) @@ -3085,6 +3079,7 @@ func validateContainer(ctr *core.Container, volumes map[string]core.VolumeSource return allErrs } +// validateContainers is called by pod spec and template validation to validate the list of regular containers. func validateContainers(containers []core.Container, volumes map[string]core.VolumeSource, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { allErrs := field.ErrorList{} @@ -3094,18 +3089,40 @@ func validateContainers(containers []core.Container, volumes map[string]core.Vol allNames := sets.String{} for i, ctr := range containers { - idxPath := fldPath.Index(i) + path := fldPath.Index(i) + // Apply validation common to all containers + allErrs = append(allErrs, validateContainerCommon(&ctr, volumes, path, opts)...) + + // Container names must be unique within the list of regular containers. + // Collisions with init or ephemeral container names will be detected by the init or ephemeral + // container validation to prevent duplicate error messages. if allNames.Has(ctr.Name) { - allErrs = append(allErrs, field.Duplicate(idxPath.Child("name"), ctr.Name)) + allErrs = append(allErrs, field.Duplicate(path.Child("name"), ctr.Name)) } else { allNames.Insert(ctr.Name) } - allErrs = append(allErrs, validateContainer(&ctr, volumes, idxPath, opts)...) + // These fields are only allowed for regular containers, so only check supported values here. + // Init and ephemeral container validation will return field.Forbidden() for these paths. + if ctr.Lifecycle != nil { + allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, path.Child("lifecycle"))...) + } + allErrs = append(allErrs, validateProbe(ctr.LivenessProbe, path.Child("livenessProbe"))...) + if ctr.LivenessProbe != nil && ctr.LivenessProbe.SuccessThreshold != 1 { + allErrs = append(allErrs, field.Invalid(path.Child("livenessProbe", "successThreshold"), ctr.LivenessProbe.SuccessThreshold, "must be 1")) + } + allErrs = append(allErrs, validateProbe(ctr.ReadinessProbe, path.Child("readinessProbe"))...) + if ctr.ReadinessProbe != nil && ctr.ReadinessProbe.TerminationGracePeriodSeconds != nil { + allErrs = append(allErrs, field.Invalid(path.Child("readinessProbe", "terminationGracePeriodSeconds"), ctr.ReadinessProbe.TerminationGracePeriodSeconds, "must not be set for readinessProbes")) + } + allErrs = append(allErrs, validateProbe(ctr.StartupProbe, path.Child("startupProbe"))...) + if ctr.StartupProbe != nil && ctr.StartupProbe.SuccessThreshold != 1 { + allErrs = append(allErrs, field.Invalid(path.Child("startupProbe", "successThreshold"), ctr.StartupProbe.SuccessThreshold, "must be 1")) + } } - // Check for colliding ports across all containers. + // Port conflicts are checked across all containers allErrs = append(allErrs, checkHostPortConflicts(containers, fldPath)...) return allErrs @@ -3392,7 +3409,8 @@ func ValidateTolerations(tolerations []core.Toleration, fldPath *field.Path) fie } // validateContainersOnlyForPod does additional validation for containers on a pod versus a pod template -// it only does additive validation of fields not covered in validateContainers +// it only does additive validation of fields not covered in validateContainers and is not called for +// ephemeral containers which require a conversion to core.Container. func validateContainersOnlyForPod(containers []core.Container, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} for i, ctr := range containers { @@ -3401,6 +3419,8 @@ func validateContainersOnlyForPod(containers []core.Container, fldPath *field.Pa return allErrs } +// validateContainerOnlyForPod does pod-only (i.e. not pod template) validation for a single container. +// This is called by validateContainersOnlyForPod and validateEphemeralContainers directly. func validateContainerOnlyForPod(ctr *core.Container, path *field.Path) field.ErrorList { allErrs := field.ErrorList{} if len(ctr.Image) != len(strings.TrimSpace(ctr.Image)) { @@ -3451,6 +3471,7 @@ func validatePodMetadataAndSpec(pod *core.Pod, opts PodValidationOptions) field. allErrs = append(allErrs, validateContainersOnlyForPod(pod.Spec.Containers, specPath.Child("containers"))...) allErrs = append(allErrs, validateContainersOnlyForPod(pod.Spec.InitContainers, specPath.Child("initContainers"))...) + // validateContainersOnlyForPod() is checked for ephemeral containers by validateEphemeralContainers() return allErrs } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 00ea5fbcbb3..6e827b7a591 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -21,11 +21,13 @@ import ( "fmt" "math" "reflect" + "runtime" "strings" "testing" "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -51,6 +53,25 @@ const ( envVarNameErrMsg = "a valid environment variable name must consist of" ) +func line() string { + _, _, line, ok := runtime.Caller(1) + var s string + if ok { + s = fmt.Sprintf("%d", line) + } else { + s = "" + } + return s +} + +func prettyErrorList(errs field.ErrorList) string { + var s string + for _, e := range errs { + s += fmt.Sprintf("\t%s\n", e) + } + return s +} + func newHostPathType(pathType string) *core.HostPathType { hostPathType := new(core.HostPathType) *hostPathType = core.HostPathType(pathType) @@ -6714,74 +6735,102 @@ func TestValidateEphemeralContainers(t *testing.T) { // Failure Cases tcs := []struct { - title string + title, line string ephemeralContainers []core.EphemeralContainer - expectedError field.Error + expectedErrors field.ErrorList }{ { "Name Collision with Container.Containers", + line(), []core.EphemeralContainer{ {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug1", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, }, - field.Error{Type: field.ErrorTypeDuplicate, Field: "ephemeralContainers[0].name"}, + field.ErrorList{{Type: field.ErrorTypeDuplicate, Field: "ephemeralContainers[0].name"}}, }, { "Name Collision with Container.InitContainers", + line(), []core.EphemeralContainer{ {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "ictr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug1", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, }, - field.Error{Type: field.ErrorTypeDuplicate, Field: "ephemeralContainers[0].name"}, + field.ErrorList{{Type: field.ErrorTypeDuplicate, Field: "ephemeralContainers[0].name"}}, }, { "Name Collision with EphemeralContainers", + line(), []core.EphemeralContainer{ {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug1", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug1", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, }, - field.Error{Type: field.ErrorTypeDuplicate, Field: "ephemeralContainers[1].name"}, + field.ErrorList{{Type: field.ErrorTypeDuplicate, Field: "ephemeralContainers[1].name"}}, }, { - "empty Container Container", + "empty Container", + line(), []core.EphemeralContainer{ {EphemeralContainerCommon: core.EphemeralContainerCommon{}}, }, - field.Error{Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].name"}, + field.ErrorList{ + {Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].name"}, + {Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].image"}, + {Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].terminationMessagePolicy"}, + {Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].imagePullPolicy"}, + }, }, { "empty Container Name", + line(), []core.EphemeralContainer{ {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, }, - field.Error{Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].name"}, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].name"}}, }, { "whitespace padded image name", + line(), []core.EphemeralContainer{ {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: " image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, }, - field.Error{Type: field.ErrorTypeInvalid, Field: "ephemeralContainers[0].image"}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "ephemeralContainers[0].image"}}, }, { "invalid image pull policy", + line(), []core.EphemeralContainer{ {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: "image", ImagePullPolicy: "PullThreeTimes", TerminationMessagePolicy: "File"}}, }, - field.Error{Type: field.ErrorTypeNotSupported, Field: "ephemeralContainers[0].imagePullPolicy"}, + field.ErrorList{{Type: field.ErrorTypeNotSupported, Field: "ephemeralContainers[0].imagePullPolicy"}}, }, { "TargetContainerName doesn't exist", + line(), []core.EphemeralContainer{ { EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, TargetContainerName: "bogus", }, }, - field.Error{Type: field.ErrorTypeNotFound, Field: "ephemeralContainers[0].targetContainerName"}, + field.ErrorList{{Type: field.ErrorTypeNotFound, Field: "ephemeralContainers[0].targetContainerName"}}, + }, + { + "Targets an ephemeral container", + line(), + []core.EphemeralContainer{ + { + EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, + }, + { + EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debugception", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, + TargetContainerName: "debug", + }, + }, + field.ErrorList{{Type: field.ErrorTypeNotFound, Field: "ephemeralContainers[1].targetContainerName"}}, }, { "Container uses disallowed field: Lifecycle", + line(), []core.EphemeralContainer{ { EphemeralContainerCommon: core.EphemeralContainerCommon{ @@ -6797,10 +6846,11 @@ func TestValidateEphemeralContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].lifecycle"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].lifecycle"}}, }, { "Container uses disallowed field: LivenessProbe", + line(), []core.EphemeralContainer{ { EphemeralContainerCommon: core.EphemeralContainerCommon{ @@ -6817,10 +6867,11 @@ func TestValidateEphemeralContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].livenessProbe"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].livenessProbe"}}, }, { "Container uses disallowed field: Ports", + line(), []core.EphemeralContainer{ { EphemeralContainerCommon: core.EphemeralContainerCommon{ @@ -6834,10 +6885,11 @@ func TestValidateEphemeralContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].ports"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].ports"}}, }, { "Container uses disallowed field: ReadinessProbe", + line(), []core.EphemeralContainer{ { EphemeralContainerCommon: core.EphemeralContainerCommon{ @@ -6853,10 +6905,11 @@ func TestValidateEphemeralContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].readinessProbe"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].readinessProbe"}}, }, { "Container uses disallowed field: StartupProbe", + line(), []core.EphemeralContainer{ { EphemeralContainerCommon: core.EphemeralContainerCommon{ @@ -6873,10 +6926,11 @@ func TestValidateEphemeralContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].startupProbe"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].startupProbe"}}, }, { "Container uses disallowed field: Resources", + line(), []core.EphemeralContainer{ { EphemeralContainerCommon: core.EphemeralContainerCommon{ @@ -6892,10 +6946,11 @@ func TestValidateEphemeralContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].resources"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].resources"}}, }, { "Container uses disallowed field: VolumeMount.SubPath", + line(), []core.EphemeralContainer{ { EphemeralContainerCommon: core.EphemeralContainerCommon{ @@ -6910,10 +6965,11 @@ func TestValidateEphemeralContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPath"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPath"}}, }, { "Container uses disallowed field: VolumeMount.SubPathExpr", + line(), []core.EphemeralContainer{ { EphemeralContainerCommon: core.EphemeralContainerCommon{ @@ -6928,25 +6984,40 @@ func TestValidateEphemeralContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPathExpr"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPathExpr"}}, + }, + { + "Disallowed field with other errors should only return a single Forbidden", + line(), + []core.EphemeralContainer{ + { + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + Lifecycle: &core.Lifecycle{ + PreStop: &core.LifecycleHandler{ + Exec: &core.ExecAction{Command: []string{}}, + }, + }, + }, + }, + }, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].lifecycle"}}, }, } for _, tc := range tcs { - t.Run(tc.title, func(t *testing.T) { + t.Run(tc.title+"__@L"+tc.line, func(t *testing.T) { errs := validateEphemeralContainers(tc.ephemeralContainers, containers, initContainers, vols, field.NewPath("ephemeralContainers"), PodValidationOptions{}) if len(errs) == 0 { t.Fatal("expected error but received none") } - if len(errs) > 1 { - t.Errorf("expected 1 error but received %d: %q", len(errs), errs) - } - if errs[0].Type != tc.expectedError.Type { - t.Errorf("expected error type %q but received %q: %q", string(tc.expectedError.Type), string(errs[0].Type), errs) - } - if errs[0].Field != tc.expectedError.Field { - t.Errorf("expected error for field %q but received error for field %q: %q", tc.expectedError.Field, errs[0].Field, errs) + if diff := cmp.Diff(tc.expectedErrors, errs, cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail")); diff != "" { + t.Errorf("unexpected diff in errors (-want, +got):\n%s", diff) + t.Errorf("INFO: all errors:\n%s", prettyErrorList(errs)) } }) } @@ -7226,70 +7297,80 @@ func TestValidateContainers(t *testing.T) { AllowPrivileged: false, }) errorCases := []struct { - title string + title, line string containers []core.Container - expectedErrors []field.Error + expectedErrors field.ErrorList }{ { "zero-length name", + line(), []core.Container{{Name: "", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, - []field.Error{{Type: field.ErrorTypeRequired, Field: "containers[0].name"}}, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "containers[0].name"}}, }, { "zero-length-image", + line(), []core.Container{{Name: "abc", Image: "", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, - []field.Error{{Type: field.ErrorTypeRequired, Field: "containers[0].image"}}, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "containers[0].image"}}, }, { "name > 63 characters", + line(), []core.Container{{Name: strings.Repeat("a", 64), Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].name"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].name"}}, }, { "name not a DNS label", + line(), []core.Container{{Name: "a.b.c", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].name"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].name"}}, }, { "name not unique", + line(), []core.Container{ {Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, {Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, }, - []field.Error{{Type: field.ErrorTypeDuplicate, Field: "containers[1].name"}}, + field.ErrorList{{Type: field.ErrorTypeDuplicate, Field: "containers[1].name"}}, }, { "zero-length image", + line(), []core.Container{{Name: "abc", Image: "", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, - []field.Error{{Type: field.ErrorTypeRequired, Field: "containers[0].image"}}, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "containers[0].image"}}, }, { "host port not unique", + line(), []core.Container{ {Name: "abc", Image: "image", Ports: []core.ContainerPort{{ContainerPort: 80, HostPort: 80, Protocol: "TCP"}}, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, {Name: "def", Image: "image", Ports: []core.ContainerPort{{ContainerPort: 81, HostPort: 80, Protocol: "TCP"}}, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, }, - []field.Error{{Type: field.ErrorTypeDuplicate, Field: "containers[1].ports[0].hostPort"}}, + field.ErrorList{{Type: field.ErrorTypeDuplicate, Field: "containers[1].ports[0].hostPort"}}, }, { "invalid env var name", + line(), []core.Container{ {Name: "abc", Image: "image", Env: []core.EnvVar{{Name: "ev!1"}}, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].env[0].name"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].env[0].name"}}, }, { "unknown volume name", + line(), []core.Container{ {Name: "abc", Image: "image", VolumeMounts: []core.VolumeMount{{Name: "anything", MountPath: "/foo"}}, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, }, - []field.Error{{Type: field.ErrorTypeNotFound, Field: "containers[0].volumeMounts[0].name"}}, + field.ErrorList{{Type: field.ErrorTypeNotFound, Field: "containers[0].volumeMounts[0].name"}}, }, { "invalid lifecycle, no exec command.", + line(), []core.Container{ { Name: "life-123", @@ -7303,10 +7384,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeRequired, Field: "containers[0].lifecycle.preStop.exec.command"}}, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "containers[0].lifecycle.preStop.exec.command"}}, }, { "invalid lifecycle, no http path.", + line(), []core.Container{ { Name: "life-123", @@ -7323,10 +7405,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeRequired, Field: "containers[0].lifecycle.preStop.httpGet.path"}}, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "containers[0].lifecycle.preStop.httpGet.path"}}, }, { "invalid lifecycle, no http port.", + line(), []core.Container{ { Name: "life-123", @@ -7343,10 +7426,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].lifecycle.preStop.httpGet.port"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].lifecycle.preStop.httpGet.port"}}, }, { "invalid lifecycle, no http scheme.", + line(), []core.Container{ { Name: "life-123", @@ -7363,10 +7447,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeNotSupported, Field: "containers[0].lifecycle.preStop.httpGet.scheme"}}, + field.ErrorList{{Type: field.ErrorTypeNotSupported, Field: "containers[0].lifecycle.preStop.httpGet.scheme"}}, }, { "invalid lifecycle, no tcp socket port.", + line(), []core.Container{ { Name: "life-123", @@ -7380,10 +7465,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].lifecycle.preStop.tcpSocket.port"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].lifecycle.preStop.tcpSocket.port"}}, }, { "invalid lifecycle, zero tcp socket port.", + line(), []core.Container{ { Name: "life-123", @@ -7399,10 +7485,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].lifecycle.preStop.tcpSocket.port"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].lifecycle.preStop.tcpSocket.port"}}, }, { "invalid lifecycle, no action.", + line(), []core.Container{ { Name: "life-123", @@ -7414,10 +7501,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeRequired, Field: "containers[0].lifecycle.preStop"}}, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "containers[0].lifecycle.preStop"}}, }, { "invalid readiness probe, terminationGracePeriodSeconds set.", + line(), []core.Container{ { Name: "life-123", @@ -7434,10 +7522,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].readinessProbe.terminationGracePeriodSeconds"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].readinessProbe.terminationGracePeriodSeconds"}}, }, { "invalid liveness probe, no tcp socket port.", + line(), []core.Container{ { Name: "live-123", @@ -7452,10 +7541,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.tcpSocket.port"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.tcpSocket.port"}}, }, { "invalid liveness probe, no action.", + line(), []core.Container{ { Name: "live-123", @@ -7468,10 +7558,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeRequired, Field: "containers[0].livenessProbe"}}, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "containers[0].livenessProbe"}}, }, { "invalid liveness probe, successThreshold != 1", + line(), []core.Container{ { Name: "live-123", @@ -7488,10 +7579,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.successThreshold"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.successThreshold"}}, }, { "invalid startup probe, successThreshold != 1", + line(), []core.Container{ { Name: "startup-123", @@ -7508,10 +7600,116 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].startupProbe.successThreshold"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].startupProbe.successThreshold"}}, + }, + { + "invalid liveness probe, negative numbers", + line(), + []core.Container{ + { + Name: "live-123", + Image: "image", + LivenessProbe: &core.Probe{ + ProbeHandler: core.ProbeHandler{ + TCPSocket: &core.TCPSocketAction{ + Port: intstr.FromInt(80), + }, + }, + InitialDelaySeconds: -1, + TimeoutSeconds: -1, + PeriodSeconds: -1, + SuccessThreshold: -1, + FailureThreshold: -1, + TerminationGracePeriodSeconds: utilpointer.Int64Ptr(-1), + }, + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + field.ErrorList{ + {Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.initialDelaySeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.timeoutSeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.periodSeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.successThreshold"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.failureThreshold"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.terminationGracePeriodSeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].livenessProbe.successThreshold"}, + }, + }, + { + "invalid readiness probe, negative numbers", + line(), + []core.Container{ + { + Name: "ready-123", + Image: "image", + ReadinessProbe: &core.Probe{ + ProbeHandler: core.ProbeHandler{ + TCPSocket: &core.TCPSocketAction{ + Port: intstr.FromInt(80), + }, + }, + InitialDelaySeconds: -1, + TimeoutSeconds: -1, + PeriodSeconds: -1, + SuccessThreshold: -1, + FailureThreshold: -1, + TerminationGracePeriodSeconds: utilpointer.Int64Ptr(-1), + }, + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + field.ErrorList{ + {Type: field.ErrorTypeInvalid, Field: "containers[0].readinessProbe.initialDelaySeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].readinessProbe.timeoutSeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].readinessProbe.periodSeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].readinessProbe.successThreshold"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].readinessProbe.failureThreshold"}, + // terminationGracePeriodSeconds returns multiple validation errors here: + // containers[0].readinessProbe.terminationGracePeriodSeconds: Invalid value: -1: must be greater than 0 + {Type: field.ErrorTypeInvalid, Field: "containers[0].readinessProbe.terminationGracePeriodSeconds"}, + // containers[0].readinessProbe.terminationGracePeriodSeconds: Invalid value: -1: must not be set for readinessProbes + {Type: field.ErrorTypeInvalid, Field: "containers[0].readinessProbe.terminationGracePeriodSeconds"}, + }, + }, + { + "invalid startup probe, negative numbers", + line(), + []core.Container{ + { + Name: "startup-123", + Image: "image", + StartupProbe: &core.Probe{ + ProbeHandler: core.ProbeHandler{ + TCPSocket: &core.TCPSocketAction{ + Port: intstr.FromInt(80), + }, + }, + InitialDelaySeconds: -1, + TimeoutSeconds: -1, + PeriodSeconds: -1, + SuccessThreshold: -1, + FailureThreshold: -1, + TerminationGracePeriodSeconds: utilpointer.Int64Ptr(-1), + }, + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + field.ErrorList{ + {Type: field.ErrorTypeInvalid, Field: "containers[0].startupProbe.initialDelaySeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].startupProbe.timeoutSeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].startupProbe.periodSeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].startupProbe.successThreshold"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].startupProbe.failureThreshold"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].startupProbe.terminationGracePeriodSeconds"}, + {Type: field.ErrorTypeInvalid, Field: "containers[0].startupProbe.successThreshold"}, + }, }, { "invalid message termination policy", + line(), []core.Container{ { Name: "life-123", @@ -7520,10 +7718,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "Unknown", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].terminationMessagePolicy"}}, + field.ErrorList{{Type: field.ErrorTypeNotSupported, Field: "containers[0].terminationMessagePolicy"}}, }, { "empty message termination policy", + line(), []core.Container{ { Name: "life-123", @@ -7532,10 +7731,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "", }, }, - []field.Error{{Type: field.ErrorTypeRequired, Field: "containers[0].terminationMessagePolicy"}}, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "containers[0].terminationMessagePolicy"}}, }, { "privilege disabled", + line(), []core.Container{ { Name: "abc", @@ -7545,10 +7745,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeForbidden, Field: "containers[0].securityContext.privileged"}}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "containers[0].securityContext.privileged"}}, }, { "invalid compute resource", + line(), []core.Container{ { Name: "abc-123", @@ -7562,13 +7763,14 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{ + field.ErrorList{ {Type: field.ErrorTypeInvalid, Field: "containers[0].resources.limits[disk]"}, {Type: field.ErrorTypeInvalid, Field: "containers[0].resources.limits[disk]"}, }, }, { "Resource CPU invalid", + line(), []core.Container{ { Name: "abc-123", @@ -7580,10 +7782,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.limits[cpu]"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.limits[cpu]"}}, }, { "Resource Requests CPU invalid", + line(), []core.Container{ { Name: "abc-123", @@ -7595,10 +7798,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.requests[cpu]"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.requests[cpu]"}}, }, { "Resource Memory invalid", + line(), []core.Container{ { Name: "abc-123", @@ -7610,10 +7814,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.limits[memory]"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.limits[memory]"}}, }, { "Request limit simple invalid", + line(), []core.Container{ { Name: "abc-123", @@ -7626,10 +7831,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.requests"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.requests"}}, }, { "Invalid storage limit request", + line(), []core.Container{ { Name: "abc-123", @@ -7643,13 +7849,14 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{ + field.ErrorList{ {Type: field.ErrorTypeInvalid, Field: "containers[0].resources.limits[attachable-volumes-aws-ebs]"}, {Type: field.ErrorTypeInvalid, Field: "containers[0].resources.limits[attachable-volumes-aws-ebs]"}, }, }, { "CPU request limit multiple invalid", + line(), []core.Container{ { Name: "abc-123", @@ -7662,10 +7869,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.requests"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.requests"}}, }, { "Memory request limit multiple invalid", + line(), []core.Container{ { Name: "abc-123", @@ -7678,10 +7886,11 @@ func TestValidateContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.requests"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].resources.requests"}}, }, { "Invalid env from", + line(), []core.Container{ { Name: "env-from-source", @@ -7699,27 +7908,19 @@ func TestValidateContainers(t *testing.T) { }, }, }, - []field.Error{{Type: field.ErrorTypeInvalid, Field: "containers[0].envFrom[0].configMapRef.name"}}, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "containers[0].envFrom[0].configMapRef.name"}}, }, } for _, tc := range errorCases { - t.Run(tc.title, func(t *testing.T) { + t.Run(tc.title+"__@L"+tc.line, func(t *testing.T) { errs := validateContainers(tc.containers, volumeDevices, field.NewPath("containers"), PodValidationOptions{}) if len(errs) == 0 { - t.Fatalf("expected error but received none") + t.Fatal("expected error but received none") } - if len(errs) != len(tc.expectedErrors) { - t.Fatalf("unexpected number of errors (got %d, want %d) in validation result %q", len(errs), len(tc.expectedErrors), errs) - } - - for i, err := range errs { - if err.Type != tc.expectedErrors[i].Type { - t.Errorf("returned error %d of %d: got error type %q but wanted %q: %q", i+1, len(errs), string(err.Type), string(tc.expectedErrors[i].Type), err) - } - if errs[i].Field != tc.expectedErrors[i].Field { - t.Errorf("returned error %d of %d: got error for field %q but wanted error for field %q: %q", i+1, len(errs), err.Field, tc.expectedErrors[i].Field, err) - } + if diff := cmp.Diff(tc.expectedErrors, errs, cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail")); diff != "" { + t.Errorf("unexpected diff in errors (-want, +got):\n%s", diff) + t.Errorf("INFO: all errors:\n%s", prettyErrorList(errs)) } }) } @@ -7770,12 +7971,26 @@ func TestValidateInitContainers(t *testing.T) { AllowPrivileged: false, }) errorCases := []struct { - title string + title, line string initContainers []core.Container - expectedError field.Error + expectedErrors field.ErrorList }{ + { + "empty name", + line(), + []core.Container{ + { + Name: "", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + }, + }, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "initContainers[0].name", BadValue: ""}}, + }, { "name collision with regular container", + line(), []core.Container{ { Name: "app", @@ -7784,22 +7999,24 @@ func TestValidateInitContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - field.Error{Type: field.ErrorTypeDuplicate, Field: "initContainers[0].name"}, + field.ErrorList{{Type: field.ErrorTypeDuplicate, Field: "initContainers[0].name", BadValue: "app"}}, }, { "invalid termination message policy", + line(), []core.Container{ { Name: "init", Image: "image", ImagePullPolicy: "IfNotPresent", - TerminationMessagePolicy: "INVALID_POLICY_NAME", + TerminationMessagePolicy: "Unknown", }, }, - field.Error{Type: field.ErrorTypeInvalid, Field: "initContainers[0].terminationMessagePolicy"}, + field.ErrorList{{Type: field.ErrorTypeNotSupported, Field: "initContainers[0].terminationMessagePolicy", BadValue: core.TerminationMessagePolicy("Unknown")}}, }, { "duplicate names", + line(), []core.Container{ { Name: "init", @@ -7814,10 +8031,11 @@ func TestValidateInitContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - field.Error{Type: field.ErrorTypeDuplicate, Field: "initContainers[1].name"}, + field.ErrorList{{Type: field.ErrorTypeDuplicate, Field: "initContainers[1].name", BadValue: "init"}}, }, { "duplicate ports", + line(), []core.Container{ { Name: "abc", @@ -7834,10 +8052,11 @@ func TestValidateInitContainers(t *testing.T) { TerminationMessagePolicy: "File", }, }, - field.Error{Type: field.ErrorTypeDuplicate, Field: "initContainers[0].ports[1].hostPort"}, + field.ErrorList{{Type: field.ErrorTypeDuplicate, Field: "initContainers[0].ports[1].hostPort", BadValue: "TCP//8080"}}, }, { "uses disallowed field: Lifecycle", + line(), []core.Container{ { Name: "debug", @@ -7851,10 +8070,11 @@ func TestValidateInitContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeInvalid, Field: "initContainers[0].lifecycle"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "initContainers[0].lifecycle", BadValue: ""}}, }, { "uses disallowed field: LivenessProbe", + line(), []core.Container{ { Name: "debug", @@ -7869,10 +8089,11 @@ func TestValidateInitContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeInvalid, Field: "initContainers[0].livenessProbe"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "initContainers[0].livenessProbe", BadValue: ""}}, }, { "uses disallowed field: ReadinessProbe", + line(), []core.Container{ { Name: "debug", @@ -7886,10 +8107,11 @@ func TestValidateInitContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeInvalid, Field: "initContainers[0].readinessProbe"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "initContainers[0].readinessProbe", BadValue: ""}}, }, { "Container uses disallowed field: StartupProbe", + line(), []core.Container{ { Name: "debug", @@ -7904,24 +8126,43 @@ func TestValidateInitContainers(t *testing.T) { }, }, }, - field.Error{Type: field.ErrorTypeInvalid, Field: "initContainers[0].startupProbe"}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "initContainers[0].startupProbe", BadValue: ""}}, + }, + { + "Disallowed field with other errors should only return a single Forbidden", + line(), + []core.Container{ + { + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + StartupProbe: &core.Probe{ + ProbeHandler: core.ProbeHandler{ + TCPSocket: &core.TCPSocketAction{Port: intstr.FromInt(80)}, + }, + InitialDelaySeconds: -1, + TimeoutSeconds: -1, + PeriodSeconds: -1, + SuccessThreshold: -1, + FailureThreshold: -1, + TerminationGracePeriodSeconds: utilpointer.Int64Ptr(-1), + }, + }, + }, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "initContainers[0].startupProbe", BadValue: ""}}, }, } for _, tc := range errorCases { - t.Run(tc.title, func(t *testing.T) { + t.Run(tc.title+"__@L"+tc.line, func(t *testing.T) { errs := validateInitContainers(tc.initContainers, containers, volumeDevices, field.NewPath("initContainers"), PodValidationOptions{}) if len(errs) == 0 { t.Fatal("expected error but received none") } - if len(errs) > 1 { - t.Errorf("expected 1 error but received %d: %q", len(errs), errs) - } - if errs[0].Type != tc.expectedError.Type { - t.Errorf("expected error type %q but received %q: %q", string(tc.expectedError.Type), string(errs[0].Type), errs) - } - if errs[0].Field != tc.expectedError.Field { - t.Errorf("expected error for field %q but received error for field %q: %q", tc.expectedError.Field, errs[0].Field, errs) + if diff := cmp.Diff(tc.expectedErrors, errs, cmpopts.IgnoreFields(field.Error{}, "Detail")); diff != "" { + t.Errorf("unexpected diff in errors (-want, +got):\n%s", diff) + t.Errorf("INFO: all errors:\n%s", prettyErrorList(errs)) } }) }