Skip to content

Commit

Permalink
add validation method at the top level
Browse files Browse the repository at this point in the history
  • Loading branch information
HirazawaUi committed Mar 5, 2024
1 parent 96a16a7 commit e56240b
Show file tree
Hide file tree
Showing 5 changed files with 759 additions and 45 deletions.
84 changes: 84 additions & 0 deletions pkg/api/pod/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metavalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
utilfeature "k8s.io/apiserver/pkg/util/feature"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/core/helper"
Expand Down Expand Up @@ -387,6 +389,10 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
AllowNonLocalProjectedTokenPath: false,
}

// If old spec uses relaxed validation or enabled the RelaxedEnvironmentVariableValidation feature gate,
// we must allow it
opts.AllowRelaxedEnvironmentVariableValidation = useRelaxedEnvironmentVariableValidation(podSpec, oldPodSpec)

if oldPodSpec != nil {
// if old spec has status.hostIPs downwardAPI set, we must allow it
opts.AllowHostIPsField = opts.AllowHostIPsField || hasUsedDownwardAPIFieldPathWithPodSpec(oldPodSpec, "status.hostIPs")
Expand Down Expand Up @@ -420,6 +426,84 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
return opts
}

func useRelaxedEnvironmentVariableValidation(podSpec, oldPodSpec *api.PodSpec) bool {
var oldPodEnvVarNames, podEnvVarNames sets.Set[string]
if oldPodSpec != nil {
oldPodEnvVarNames = gatherPodEnvVarNames(oldPodSpec)
}

if podSpec != nil {
podEnvVarNames = gatherPodEnvVarNames(podSpec)
}

for env := range podEnvVarNames {
if utilfeature.DefaultFeatureGate.Enabled(features.RelaxedEnvironmentVariableValidation) || relaxedEnvVarUsed(env, oldPodEnvVarNames) {
return true
}
}

return false
}

func gatherPodEnvVarNames(podSpec *api.PodSpec) sets.Set[string] {
podEnvVarNames := sets.Set[string]{}

for _, c := range podSpec.Containers {
for _, env := range c.Env {
podEnvVarNames.Insert(env.Name)
}

for _, env := range c.EnvFrom {
podEnvVarNames.Insert(env.Prefix)
}
}

for _, c := range podSpec.InitContainers {
for _, env := range c.Env {
podEnvVarNames.Insert(env.Name)
}

for _, env := range c.EnvFrom {
podEnvVarNames.Insert(env.Prefix)
}
}

for _, c := range podSpec.EphemeralContainers {
for _, env := range c.Env {
podEnvVarNames.Insert(env.Name)
}

for _, env := range c.EnvFrom {
podEnvVarNames.Insert(env.Prefix)
}
}

return podEnvVarNames
}

func relaxedEnvVarUsed(name string, oldPodEnvVarNames sets.Set[string]) bool {
// A length of 0 means this is not an update request,
// or the old pod does not exist in the env.
// We will let the feature gate decide whether to use relaxed rules.
if oldPodEnvVarNames.Len() == 0 {
return false
}

if len(validation.IsEnvVarName(name)) == 0 || len(validation.IsRelaxedEnvVarName(name)) != 0 {
// It's either a valid name by strict rules or an invalid name under relaxed rules.
// Either way, we'll use strict rules to validate.
return false
}

// The name in question failed strict rules but passed relaxed rules.
if oldPodEnvVarNames.Has(name) {
// This relaxed-rules name was already in use.
return true
}

return false
}

func hasUsedDownwardAPIFieldPathWithPodSpec(podSpec *api.PodSpec, fieldPath string) bool {
if podSpec == nil {
return false
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/core/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2068,7 +2068,11 @@ type VolumeDevice struct {

// EnvVar represents an environment variable present in a Container.
type EnvVar struct {
// Required: This must be a C_IDENTIFIER.
// Required: Name of the environment variable.
// When the RelaxedEnvironmentVariableValidation feature gate is disabled, this must consist of alphabetic characters,
// digits, '_', '-', or '.', and must not start with a digit.
// When the RelaxedEnvironmentVariableValidation feature gate is enabled,
// this may contain any printable ASCII characters except '='.
Name string
// Optional: no more than one of the following may be specified.
// Optional: Defaults to ""; variable references $(VAR_NAME) are expanded
Expand Down
26 changes: 20 additions & 6 deletions pkg/apis/core/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -2557,8 +2557,14 @@ func ValidateEnv(vars []core.EnvVar, fldPath *field.Path, opts PodValidationOpti
if len(ev.Name) == 0 {
allErrs = append(allErrs, field.Required(idxPath.Child("name"), ""))
} else {
for _, msg := range validation.IsEnvVarName(ev.Name) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ev.Name, msg))
if opts.AllowRelaxedEnvironmentVariableValidation {
for _, msg := range validation.IsRelaxedEnvVarName(ev.Name) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ev.Name, msg))
}
} else {
for _, msg := range validation.IsEnvVarName(ev.Name) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ev.Name, msg))
}
}
}
allErrs = append(allErrs, validateEnvVarValueFrom(ev, idxPath.Child("valueFrom"), opts)...)
Expand Down Expand Up @@ -2703,13 +2709,19 @@ func validateContainerResourceFieldSelector(fs *core.ResourceFieldSelector, expr
return allErrs
}

func ValidateEnvFrom(vars []core.EnvFromSource, fldPath *field.Path) field.ErrorList {
func ValidateEnvFrom(vars []core.EnvFromSource, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
allErrs := field.ErrorList{}
for i, ev := range vars {
idxPath := fldPath.Index(i)
if len(ev.Prefix) > 0 {
for _, msg := range validation.IsEnvVarName(ev.Prefix) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("prefix"), ev.Prefix, msg))
if opts.AllowRelaxedEnvironmentVariableValidation {
for _, msg := range validation.IsRelaxedEnvVarName(ev.Prefix) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("prefix"), ev.Prefix, msg))
}
} else {
for _, msg := range validation.IsEnvVarName(ev.Prefix) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("prefix"), ev.Prefix, msg))
}
}
}

Expand Down Expand Up @@ -3532,7 +3544,7 @@ func validateContainerCommon(ctr *core.Container, volumes map[string]core.Volume
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, ValidateEnvFrom(ctr.EnvFrom, path.Child("envFrom"), opts)...)
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"))...)
Expand Down Expand Up @@ -3983,6 +3995,8 @@ type PodValidationOptions struct {
// The top-level resource being validated is a Pod, not just a PodSpec
// embedded in some other resource.
ResourceIsPod bool
// Allow relaxed validation of environment variable names
AllowRelaxedEnvironmentVariableValidation bool
}

// validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set,
Expand Down

0 comments on commit e56240b

Please sign in to comment.