Merge pull request #111401 from verb/111028-container-validation

Improve tests and fix bugs in container validation
This commit is contained in:
Kubernetes Prow Robot 2022-07-28 12:43:11 -07:00 committed by GitHub
commit c06031959f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 1065 additions and 350 deletions

View File

@ -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,44 +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, "ephemeralContainer requires a name"))
continue
}
// Using validateContainers() here isn't ideal because it adds an index to the error message that
// doesn't really exist for EphemeralContainers (i.e. ephemeralContainers[0].spec[0].name instead
// of ephemeralContainers[0].spec.name)
// TODO(verb): factor a validateContainer() out of validateContainers() to be used here
c := core.Container(ec.EphemeralContainerCommon)
allErrs = append(allErrs, validateContainers([]core.Container{c}, false, volumes, idxPath, opts)...)
// EphemeralContainers don't require the backwards-compatibility distinction between pod/podTemplate validation
allErrs = append(allErrs, validateContainersOnlyForPod([]core.Container{c}, idxPath)...)
c := (*core.Container)(&ec.EphemeralContainerCommon)
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
@ -2993,41 +2991,96 @@ func validateFieldAllowList(value interface{}, allowedFields map[string]bool, er
return allErrs
}
func validateInitContainers(containers []core.Container, otherContainers []core.Container, deviceVolumes 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
if len(containers) > 0 {
allErrs = append(allErrs, validateContainers(containers, true, deviceVolumes, fldPath, opts)...)
}
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))
}
if len(ctr.Name) > 0 {
} else if len(ctr.Name) > 0 {
allNames.Insert(ctr.Name)
}
// 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 validateContainers(containers []core.Container, isInitContainers bool, volumes map[string]core.VolumeSource, fldPath *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")
if len(ctr.Name) == 0 {
allErrs = append(allErrs, field.Required(namePath, ""))
} else {
allErrs = append(allErrs, ValidateDNS1123Label(ctr.Name, namePath)...)
}
// TODO: do not validate leading and trailing whitespace to preserve backward compatibility.
// for example: https://github.com/openshift/origin/issues/14659 image = " " is special token in pod template
// others may have done similar
if len(ctr.Image) == 0 {
allErrs = append(allErrs, field.Required(path.Child("image"), ""))
}
switch ctr.TerminationMessagePolicy {
case core.TerminationMessageReadFile, core.TerminationMessageFallbackToLogsOnError:
case "":
allErrs = append(allErrs, field.Required(path.Child("terminationMessagePolicy"), ""))
default:
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, 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"))...)
allErrs = append(allErrs, ValidateVolumeMounts(ctr.VolumeMounts, volDevices, volumes, ctr, path.Child("volumeMounts"))...)
allErrs = append(allErrs, ValidateVolumeDevices(ctr.VolumeDevices, volMounts, volumes, path.Child("volumeDevices"))...)
allErrs = append(allErrs, validatePullPolicy(ctr.ImagePullPolicy, path.Child("imagePullPolicy"))...)
allErrs = append(allErrs, ValidateResourceRequirements(&ctr.Resources, path.Child("resources"), opts)...)
allErrs = append(allErrs, ValidateSecurityContext(ctr.SecurityContext, path.Child("securityContext"))...)
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{}
if len(containers) == 0 {
@ -3036,74 +3089,41 @@ func validateContainers(containers []core.Container, isInitContainers bool, volu
allNames := sets.String{}
for i, ctr := range containers {
idxPath := fldPath.Index(i)
namePath := idxPath.Child("name")
volMounts := GetVolumeMountMap(ctr.VolumeMounts)
volDevices := GetVolumeDeviceMap(ctr.VolumeDevices)
path := fldPath.Index(i)
if len(ctr.Name) == 0 {
allErrs = append(allErrs, field.Required(namePath, ""))
} else {
allErrs = append(allErrs, ValidateDNS1123Label(ctr.Name, namePath)...)
}
// 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(namePath, ctr.Name))
allErrs = append(allErrs, field.Duplicate(path.Child("name"), ctr.Name))
} else {
allNames.Insert(ctr.Name)
}
// TODO: do not validate leading and trailing whitespace to preserve backward compatibility.
// for example: https://github.com/openshift/origin/issues/14659 image = " " is special token in pod template
// others may have done similar
if len(ctr.Image) == 0 {
allErrs = append(allErrs, field.Required(idxPath.Child("image"), ""))
}
// 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, idxPath.Child("lifecycle"))...)
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, path.Child("lifecycle"))...)
}
allErrs = append(allErrs, validateProbe(ctr.LivenessProbe, idxPath.Child("livenessProbe"))...)
// Readiness-specific validation
if ctr.ReadinessProbe != nil && ctr.ReadinessProbe.TerminationGracePeriodSeconds != nil {
allErrs = append(allErrs, field.Invalid(idxPath.Child("readinessProbe", "terminationGracePeriodSeconds"), ctr.ReadinessProbe.TerminationGracePeriodSeconds, "must not be set for readinessProbes"))
}
allErrs = append(allErrs, validateProbe(ctr.StartupProbe, idxPath.Child("startupProbe"))...)
// Liveness-specific validation
allErrs = append(allErrs, validateProbe(ctr.LivenessProbe, path.Child("livenessProbe"))...)
if ctr.LivenessProbe != nil && ctr.LivenessProbe.SuccessThreshold != 1 {
allErrs = append(allErrs, field.Invalid(idxPath.Child("livenessProbe", "successThreshold"), ctr.LivenessProbe.SuccessThreshold, "must be 1"))
allErrs = append(allErrs, field.Invalid(path.Child("livenessProbe", "successThreshold"), ctr.LivenessProbe.SuccessThreshold, "must be 1"))
}
allErrs = append(allErrs, validateProbe(ctr.StartupProbe, idxPath.Child("startupProbe"))...)
// Startup-specific validation
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(idxPath.Child("startupProbe", "successThreshold"), ctr.StartupProbe.SuccessThreshold, "must be 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(idxPath.Child("terminationMessagePolicy"), "must be 'File' or 'FallbackToLogsOnError'"))
default:
allErrs = append(allErrs, field.Invalid(idxPath.Child("terminationMessagePolicy"), ctr.TerminationMessagePolicy, "must be 'File' or 'FallbackToLogsOnError'"))
}
allErrs = append(allErrs, validateProbe(ctr.ReadinessProbe, idxPath.Child("readinessProbe"))...)
allErrs = append(allErrs, validateContainerPorts(ctr.Ports, idxPath.Child("ports"))...)
allErrs = append(allErrs, ValidateEnv(ctr.Env, idxPath.Child("env"), opts)...)
allErrs = append(allErrs, ValidateEnvFrom(ctr.EnvFrom, idxPath.Child("envFrom"))...)
allErrs = append(allErrs, ValidateVolumeMounts(ctr.VolumeMounts, volDevices, volumes, &ctr, idxPath.Child("volumeMounts"))...)
allErrs = append(allErrs, ValidateVolumeDevices(ctr.VolumeDevices, volMounts, volumes, idxPath.Child("volumeDevices"))...)
allErrs = append(allErrs, validatePullPolicy(ctr.ImagePullPolicy, idxPath.Child("imagePullPolicy"))...)
allErrs = append(allErrs, ValidateResourceRequirements(&ctr.Resources, idxPath.Child("resources"), opts)...)
allErrs = append(allErrs, ValidateSecurityContext(ctr.SecurityContext, idxPath.Child("securityContext"))...)
}
if isInitContainers {
// check initContainers one by one since they are running in sequential order.
for _, initContainer := range containers {
allErrs = append(allErrs, checkHostPortConflicts([]core.Container{initContainer}, fldPath)...)
}
} else {
// Check for colliding ports across all containers.
allErrs = append(allErrs, checkHostPortConflicts(containers, fldPath)...)
}
// Port conflicts are checked across all containers
allErrs = append(allErrs, checkHostPortConflicts(containers, fldPath)...)
return allErrs
}
@ -3389,14 +3409,22 @@ 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 {
idxPath := fldPath.Index(i)
if len(ctr.Image) != len(strings.TrimSpace(ctr.Image)) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("image"), ctr.Image, "must not have leading or trailing whitespace"))
}
allErrs = append(allErrs, validateContainerOnlyForPod(&ctr, fldPath.Index(i))...)
}
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)) {
allErrs = append(allErrs, field.Invalid(path.Child("image"), ctr.Image, "must not have leading or trailing whitespace"))
}
return allErrs
}
@ -3443,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
}
@ -3503,7 +3532,7 @@ func ValidatePodSpec(spec *core.PodSpec, podMeta *metav1.ObjectMeta, fldPath *fi
vols, vErrs := ValidateVolumes(spec.Volumes, podMeta, fldPath.Child("volumes"), opts)
allErrs = append(allErrs, vErrs...)
allErrs = append(allErrs, validateContainers(spec.Containers, false, vols, fldPath.Child("containers"), opts)...)
allErrs = append(allErrs, validateContainers(spec.Containers, vols, fldPath.Child("containers"), opts)...)
allErrs = append(allErrs, validateInitContainers(spec.InitContainers, spec.Containers, vols, fldPath.Child("initContainers"), opts)...)
allErrs = append(allErrs, validateEphemeralContainers(spec.EphemeralContainers, spec.Containers, spec.InitContainers, vols, fldPath.Child("ephemeralContainers"), opts)...)
allErrs = append(allErrs, validateRestartPolicy(&spec.RestartPolicy, fldPath.Child("restartPolicy"))...)

File diff suppressed because it is too large Load Diff