Merge pull request kubernetes#123385 from HirazawaUi/allow-special-characters

Allow almost all printable ASCII characters in environment variables

Author: k8s-ci-robot

pkg/api/pod/util.go

@@ -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"
@@ -386,6 +388,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")
@@ -419,6 +425,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

pkg/apis/core/types.go

@@ -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

pkg/apis/core/validation/validation.go

@@ -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)...)
@@ -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))
+				}
 			}
 		}
 
@@ -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"))...)
@@ -3981,6 +3993,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,