diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 959f16aafc4..a10e3746431 100644 --- a/pkg/api/pod/util.go +++ b/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" @@ -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") @@ -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 diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 8195cf4bd36..2cfc06c0ba3 100644 --- a/pkg/apis/core/types.go +++ b/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 diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 61b543cc587..8d846774a88 100644 --- a/pkg/apis/core/validation/validation.go +++ b/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"))...) @@ -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, diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index a2e09cbb556..c201b6c2074 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -50,10 +50,11 @@ import ( ) const ( - dnsLabelErrMsg = "a lowercase RFC 1123 label must consist of" - dnsSubdomainLabelErrMsg = "a lowercase RFC 1123 subdomain" - envVarNameErrMsg = "a valid environment variable name must consist of" - defaultGracePeriod = int64(30) + dnsLabelErrMsg = "a lowercase RFC 1123 label must consist of" + dnsSubdomainLabelErrMsg = "a lowercase RFC 1123 subdomain" + envVarNameErrMsg = "a valid environment variable name must consist of" + relaxedEnvVarNameFmtErrMsg string = "a valid environment variable names must be printable ASCII characters other than '=' character" + defaultGracePeriod = int64(30) ) var ( @@ -5981,6 +5982,361 @@ func TestHugePagesEnv(t *testing.T) { } } +func TestRelaxedValidateEnv(t *testing.T) { + successCase := []core.EnvVar{ + {Name: "!\"#$%&'()", Value: "value"}, + {Name: "* +,-./0123456789", Value: "value"}, + {Name: ":;<>?@", Value: "value"}, + {Name: "ABCDEFG", Value: "value"}, + {Name: "abcdefghijklmn", Value: "value"}, + {Name: "[\\]^_`{}|~", Value: "value"}, + { + Name: "!\"#$%&'()", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.annotations['key']", + }, + }, + }, { + Name: "!\"#$%&'()", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.labels['key']", + }, + }, + }, { + Name: "* +,-./0123456789", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.name", + }, + }, + }, { + Name: "* +,-./0123456789", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.namespace", + }, + }, + }, { + Name: "* +,-./0123456789", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.uid", + }, + }, + }, { + Name: ":;<>?@", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "spec.nodeName", + }, + }, + }, { + Name: ":;<>?@", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "spec.serviceAccountName", + }, + }, + }, { + Name: ":;<>?@", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "status.hostIP", + }, + }, + }, { + Name: ":;<>?@", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "status.podIP", + }, + }, + }, { + Name: "abcdefghijklmn", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "status.podIPs", + }, + }, + }, + { + Name: "abcdefghijklmn", + ValueFrom: &core.EnvVarSource{ + SecretKeyRef: &core.SecretKeySelector{ + LocalObjectReference: core.LocalObjectReference{ + Name: "some-secret", + }, + Key: "secret-key", + }, + }, + }, { + Name: "!\"#$%&'()", + ValueFrom: &core.EnvVarSource{ + ConfigMapKeyRef: &core.ConfigMapKeySelector{ + LocalObjectReference: core.LocalObjectReference{ + Name: "some-config-map", + }, + Key: "some-key", + }, + }, + }, + } + if errs := ValidateEnv(successCase, field.NewPath("field"), PodValidationOptions{AllowRelaxedEnvironmentVariableValidation: true}); len(errs) != 0 { + t.Errorf("expected success, got: %v", errs) + } + + errorCases := []struct { + name string + envs []core.EnvVar + expectedError string + }{{ + name: "illegal character", + envs: []core.EnvVar{{Name: "=abc"}}, + expectedError: `[0].name: Invalid value: "=abc": ` + relaxedEnvVarNameFmtErrMsg, + }, { + name: "zero-length name", + envs: []core.EnvVar{{Name: ""}}, + expectedError: "[0].name: Required value", + }, { + name: "value and valueFrom specified", + envs: []core.EnvVar{{ + Name: "abc", + Value: "foo", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.name", + }, + }, + }}, + expectedError: "[0].valueFrom: Invalid value: \"\": may not be specified when `value` is not empty", + }, { + name: "valueFrom without a source", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{}, + }}, + expectedError: "[0].valueFrom: Invalid value: \"\": must specify one of: `fieldRef`, `resourceFieldRef`, `configMapKeyRef` or `secretKeyRef`", + }, { + name: "valueFrom.fieldRef and valueFrom.secretKeyRef specified", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.name", + }, + SecretKeyRef: &core.SecretKeySelector{ + LocalObjectReference: core.LocalObjectReference{ + Name: "a-secret", + }, + Key: "a-key", + }, + }, + }}, + expectedError: "[0].valueFrom: Invalid value: \"\": may not have more than one field specified at a time", + }, { + name: "valueFrom.fieldRef and valueFrom.configMapKeyRef set", + envs: []core.EnvVar{{ + Name: "some_var_name", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.name", + }, + ConfigMapKeyRef: &core.ConfigMapKeySelector{ + LocalObjectReference: core.LocalObjectReference{ + Name: "some-config-map", + }, + Key: "some-key", + }, + }, + }}, + expectedError: `[0].valueFrom: Invalid value: "": may not have more than one field specified at a time`, + }, { + name: "valueFrom.fieldRef and valueFrom.secretKeyRef specified", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.name", + }, + SecretKeyRef: &core.SecretKeySelector{ + LocalObjectReference: core.LocalObjectReference{ + Name: "a-secret", + }, + Key: "a-key", + }, + ConfigMapKeyRef: &core.ConfigMapKeySelector{ + LocalObjectReference: core.LocalObjectReference{ + Name: "some-config-map", + }, + Key: "some-key", + }, + }, + }}, + expectedError: `[0].valueFrom: Invalid value: "": may not have more than one field specified at a time`, + }, { + name: "valueFrom.secretKeyRef.name invalid", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + SecretKeyRef: &core.SecretKeySelector{ + LocalObjectReference: core.LocalObjectReference{ + Name: "$%^&*#", + }, + Key: "a-key", + }, + }, + }}, + }, { + name: "valueFrom.configMapKeyRef.name invalid", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + ConfigMapKeyRef: &core.ConfigMapKeySelector{ + LocalObjectReference: core.LocalObjectReference{ + Name: "$%^&*#", + }, + Key: "some-key", + }, + }, + }}, + }, { + name: "missing FieldPath on ObjectFieldSelector", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + APIVersion: "v1", + }, + }, + }}, + expectedError: `[0].valueFrom.fieldRef.fieldPath: Required value`, + }, { + name: "missing APIVersion on ObjectFieldSelector", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + FieldPath: "metadata.name", + }, + }, + }}, + expectedError: `[0].valueFrom.fieldRef.apiVersion: Required value`, + }, { + name: "invalid fieldPath", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + FieldPath: "metadata.whoops", + APIVersion: "v1", + }, + }, + }}, + expectedError: `[0].valueFrom.fieldRef.fieldPath: Invalid value: "metadata.whoops": error converting fieldPath`, + }, { + name: "metadata.name with subscript", + envs: []core.EnvVar{{ + Name: "labels", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + FieldPath: "metadata.name['key']", + APIVersion: "v1", + }, + }, + }}, + expectedError: `[0].valueFrom.fieldRef.fieldPath: Invalid value: "metadata.name['key']": error converting fieldPath: field label does not support subscript`, + }, { + name: "metadata.labels without subscript", + envs: []core.EnvVar{{ + Name: "labels", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + FieldPath: "metadata.labels", + APIVersion: "v1", + }, + }, + }}, + expectedError: `[0].valueFrom.fieldRef.fieldPath: Unsupported value: "metadata.labels": supported values: "metadata.name", "metadata.namespace", "metadata.uid", "spec.nodeName", "spec.serviceAccountName", "status.hostIP", "status.hostIPs", "status.podIP", "status.podIPs"`, + }, { + name: "metadata.annotations without subscript", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + FieldPath: "metadata.annotations", + APIVersion: "v1", + }, + }, + }}, + expectedError: `[0].valueFrom.fieldRef.fieldPath: Unsupported value: "metadata.annotations": supported values: "metadata.name", "metadata.namespace", "metadata.uid", "spec.nodeName", "spec.serviceAccountName", "status.hostIP", "status.hostIPs", "status.podIP", "status.podIPs"`, + }, { + name: "metadata.annotations with invalid key", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + FieldPath: "metadata.annotations['invalid~key']", + APIVersion: "v1", + }, + }, + }}, + expectedError: `field[0].valueFrom.fieldRef: Invalid value: "invalid~key"`, + }, { + name: "metadata.labels with invalid key", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + FieldPath: "metadata.labels['Www.k8s.io/test']", + APIVersion: "v1", + }, + }, + }}, + expectedError: `field[0].valueFrom.fieldRef: Invalid value: "Www.k8s.io/test"`, + }, { + name: "unsupported fieldPath", + envs: []core.EnvVar{{ + Name: "abc", + ValueFrom: &core.EnvVarSource{ + FieldRef: &core.ObjectFieldSelector{ + FieldPath: "status.phase", + APIVersion: "v1", + }, + }, + }}, + expectedError: `valueFrom.fieldRef.fieldPath: Unsupported value: "status.phase": supported values: "metadata.name", "metadata.namespace", "metadata.uid", "spec.nodeName", "spec.serviceAccountName", "status.hostIP", "status.hostIPs", "status.podIP", "status.podIPs"`, + }, + } + for _, tc := range errorCases { + if errs := ValidateEnv(tc.envs, field.NewPath("field"), PodValidationOptions{AllowRelaxedEnvironmentVariableValidation: true}); len(errs) == 0 { + t.Errorf("expected failure for %s", tc.name) + } else { + for i := range errs { + str := errs[i].Error() + if str != "" && !strings.Contains(str, tc.expectedError) { + t.Errorf("%s: expected error detail either empty or %q, got %q", tc.name, tc.expectedError, str) + } + } + } + } +} + func TestValidateEnv(t *testing.T) { successCase := []core.EnvVar{ {Name: "abc", Value: "value"}, @@ -6094,6 +6450,67 @@ func TestValidateEnv(t *testing.T) { t.Errorf("expected success, got: %v", errs) } + updateSuccessCase := []core.EnvVar{ + {Name: "!\"#$%&'()", Value: "value"}, + {Name: "* +,-./0123456789", Value: "value"}, + {Name: ":;<>?@", Value: "value"}, + {Name: "ABCDEFG", Value: "value"}, + {Name: "abcdefghijklmn", Value: "value"}, + {Name: "[\\]^_`{}|~", Value: "value"}, + } + + if errs := ValidateEnv(updateSuccessCase, field.NewPath("field"), PodValidationOptions{AllowRelaxedEnvironmentVariableValidation: true}); len(errs) != 0 { + t.Errorf("expected success, got: %v", errs) + } + + updateErrorCase := []struct { + name string + envs []core.EnvVar + expectedError string + }{ + { + name: "invalid name a", + envs: []core.EnvVar{ + {Name: "!\"#$%&'()", Value: "value"}, + }, + expectedError: `field[0].name: Invalid value: ` + "\"!\\\"#$%&'()\": " + envVarNameErrMsg, + }, + { + name: "invalid name b", + envs: []core.EnvVar{ + {Name: "* +,-./0123456789", Value: "value"}, + }, + expectedError: `field[0].name: Invalid value: ` + "\"* +,-./0123456789\": " + envVarNameErrMsg, + }, + { + name: "invalid name c", + envs: []core.EnvVar{ + {Name: ":;<>?@", Value: "value"}, + }, + expectedError: `field[0].name: Invalid value: ` + "\":;<>?@\": " + envVarNameErrMsg, + }, + { + name: "invalid name d", + envs: []core.EnvVar{ + {Name: "[\\]^_{}|~", Value: "value"}, + }, + expectedError: `field[0].name: Invalid value: ` + "\"[\\\\]^_{}|~\": " + envVarNameErrMsg, + }, + } + + for _, tc := range updateErrorCase { + if errs := ValidateEnv(tc.envs, field.NewPath("field"), PodValidationOptions{}); len(errs) == 0 { + t.Errorf("expected failure for %s", tc.name) + } else { + for i := range errs { + str := errs[i].Error() + if str != "" && !strings.Contains(str, tc.expectedError) { + t.Errorf("%s: expected error detail either empty or %q, got %q", tc.name, tc.expectedError, str) + } + } + } + } + errorCases := []struct { name string envs []core.EnvVar @@ -6102,22 +6519,6 @@ func TestValidateEnv(t *testing.T) { name: "zero-length name", envs: []core.EnvVar{{Name: ""}}, expectedError: "[0].name: Required value", - }, { - name: "illegal character", - envs: []core.EnvVar{{Name: "a!b"}}, - expectedError: `[0].name: Invalid value: "a!b": ` + envVarNameErrMsg, - }, { - name: "dot only", - envs: []core.EnvVar{{Name: "."}}, - expectedError: `[0].name: Invalid value: ".": must not be`, - }, { - name: "double dots only", - envs: []core.EnvVar{{Name: ".."}}, - expectedError: `[0].name: Invalid value: "..": must not be`, - }, { - name: "leading double dots", - envs: []core.EnvVar{{Name: "..abc"}}, - expectedError: `[0].name: Invalid value: "..abc": must not start with`, }, { name: "value and valueFrom specified", envs: []core.EnvVar{{ @@ -6377,10 +6778,112 @@ func TestValidateEnvFrom(t *testing.T) { }, }, } - if errs := ValidateEnvFrom(successCase, field.NewPath("field")); len(errs) != 0 { + if errs := ValidateEnvFrom(successCase, nil, PodValidationOptions{}); len(errs) != 0 { t.Errorf("expected success: %v", errs) } + updateSuccessCase := []core.EnvFromSource{{ + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + Prefix: "* +,-./0123456789", + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + Prefix: ":;<>?@", + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + Prefix: "abcdefghijklmn", + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + Prefix: "[\\]^_`{}|~", + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }} + + if errs := ValidateEnvFrom(updateSuccessCase, field.NewPath("field"), PodValidationOptions{AllowRelaxedEnvironmentVariableValidation: true}); len(errs) != 0 { + t.Errorf("expected success, got: %v", errs) + } + + updateErrorCase := []struct { + name string + envs []core.EnvFromSource + expectedError string + }{ + { + name: "invalid name a", + envs: []core.EnvFromSource{ + { + Prefix: "!\"#$%&'()", + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, + }, + expectedError: `field[0].prefix: Invalid value: ` + "\"!\\\"#$%&'()\": " + envVarNameErrMsg, + }, + { + name: "invalid name b", + envs: []core.EnvFromSource{ + { + Prefix: "* +,-./0123456789", + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, + }, + expectedError: `field[0].prefix: Invalid value: ` + "\"* +,-./0123456789\": " + envVarNameErrMsg, + }, + { + name: "invalid name c", + envs: []core.EnvFromSource{ + { + Prefix: ":;<>?@", + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, + }, + expectedError: `field[0].prefix: Invalid value: ` + "\":;<>?@\": " + envVarNameErrMsg, + }, + { + name: "invalid name d", + envs: []core.EnvFromSource{ + { + Prefix: "[\\]^_{}|~", + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, + }, + expectedError: `field[0].prefix: Invalid value: ` + "\"[\\\\]^_{}|~\": " + envVarNameErrMsg, + }, + } + + for _, tc := range updateErrorCase { + if errs := ValidateEnvFrom(tc.envs, field.NewPath("field"), PodValidationOptions{}); len(errs) == 0 { + t.Errorf("expected failure for %s", tc.name) + } else { + for i := range errs { + str := errs[i].Error() + if str != "" && !strings.Contains(str, tc.expectedError) { + t.Errorf("%s: expected error detail either empty or %q, got %q", tc.name, tc.expectedError, str) + } + } + } + } + errorCases := []struct { name string envs []core.EnvFromSource @@ -6399,14 +6902,6 @@ func TestValidateEnvFrom(t *testing.T) { LocalObjectReference: core.LocalObjectReference{Name: "$"}}, }}, expectedError: "field[0].configMapRef.name: Invalid value", - }, { - name: "invalid prefix", - envs: []core.EnvFromSource{{ - Prefix: "a!b", - ConfigMapRef: &core.ConfigMapEnvSource{ - LocalObjectReference: core.LocalObjectReference{Name: "abc"}}, - }}, - expectedError: `field[0].prefix: Invalid value: "a!b": ` + envVarNameErrMsg, }, { name: "zero-length name", envs: []core.EnvFromSource{{ @@ -6421,14 +6916,6 @@ func TestValidateEnvFrom(t *testing.T) { LocalObjectReference: core.LocalObjectReference{Name: "&"}}, }}, expectedError: "field[0].secretRef.name: Invalid value", - }, { - name: "invalid prefix", - envs: []core.EnvFromSource{{ - Prefix: "a!b", - SecretRef: &core.SecretEnvSource{ - LocalObjectReference: core.LocalObjectReference{Name: "abc"}}, - }}, - expectedError: `field[0].prefix: Invalid value: "a!b": ` + envVarNameErrMsg, }, { name: "no refs", envs: []core.EnvFromSource{ @@ -6461,7 +6948,123 @@ func TestValidateEnvFrom(t *testing.T) { }, } for _, tc := range errorCases { - if errs := ValidateEnvFrom(tc.envs, field.NewPath("field")); len(errs) == 0 { + if errs := ValidateEnvFrom(tc.envs, field.NewPath("field"), PodValidationOptions{}); len(errs) == 0 { + t.Errorf("expected failure for %s", tc.name) + } else { + for i := range errs { + str := errs[i].Error() + if str != "" && !strings.Contains(str, tc.expectedError) { + t.Errorf("%s: expected error detail either empty or %q, got %q", tc.name, tc.expectedError, str) + } + } + } + } +} + +func TestRelaxedValidateEnvFrom(t *testing.T) { + successCase := []core.EnvFromSource{{ + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + Prefix: "!\"#$%&'()", + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + Prefix: "* +,-./0123456789", + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + Prefix: ":;<>?@", + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, { + Prefix: "[\\]^_`{}|~", + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}, + }, + }, + } + if errs := ValidateEnvFrom(successCase, field.NewPath("field"), PodValidationOptions{AllowRelaxedEnvironmentVariableValidation: true}); len(errs) != 0 { + t.Errorf("expected success: %v", errs) + } + + errorCases := []struct { + name string + envs []core.EnvFromSource + expectedError string + }{ + { + name: "zero-length name", + envs: []core.EnvFromSource{{ + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: ""}}, + }}, + expectedError: "field[0].configMapRef.name: Required value", + }, + { + name: "invalid prefix", + envs: []core.EnvFromSource{{ + Prefix: "=abc", + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}}, + }}, + expectedError: `field[0].prefix: Invalid value: "=abc": ` + relaxedEnvVarNameFmtErrMsg, + }, + { + name: "zero-length name", + envs: []core.EnvFromSource{{ + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: ""}}, + }}, + expectedError: "field[0].secretRef.name: Required value", + }, { + name: "invalid name", + envs: []core.EnvFromSource{{ + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "&"}}, + }}, + expectedError: "field[0].secretRef.name: Invalid value", + }, { + name: "no refs", + envs: []core.EnvFromSource{ + {}, + }, + expectedError: "field: Invalid value: \"\": must specify one of: `configMapRef` or `secretRef`", + }, { + name: "multiple refs", + envs: []core.EnvFromSource{{ + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}}, + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "abc"}}, + }}, + expectedError: "field: Invalid value: \"\": may not have more than one field specified at a time", + }, { + name: "invalid secret ref name", + envs: []core.EnvFromSource{{ + SecretRef: &core.SecretEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "$%^&*#"}}, + }}, + expectedError: "field[0].secretRef.name: Invalid value: \"$%^&*#\": " + dnsSubdomainLabelErrMsg, + }, { + name: "invalid config ref name", + envs: []core.EnvFromSource{{ + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "$%^&*#"}}, + }}, + expectedError: "field[0].configMapRef.name: Invalid value: \"$%^&*#\": " + dnsSubdomainLabelErrMsg, + }, + } + for _, tc := range errorCases { + if errs := ValidateEnvFrom(tc.envs, field.NewPath("field"), PodValidationOptions{AllowRelaxedEnvironmentVariableValidation: true}); len(errs) == 0 { t.Errorf("expected failure for %s", tc.name) } else { for i := range errs { diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 3f9f55ffde6..27d3ba0fd7a 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -646,6 +646,13 @@ const ( // Allow users to recover from volume expansion failure RecoverVolumeExpansionFailure featuregate.Feature = "RecoverVolumeExpansionFailure" + // owner: @HirazawaUi + // kep: https://kep.k8s.io/4369 + // alpha: v1.30 + // + // Allow almost all printable ASCII characters in environment variables + RelaxedEnvironmentVariableValidation featuregate.Feature = "RelaxedEnvironmentVariableValidation" + // owner: @mikedanese // alpha: v1.7 // beta: v1.12 @@ -1103,6 +1110,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS RecoverVolumeExpansionFailure: {Default: false, PreRelease: featuregate.Alpha}, + RelaxedEnvironmentVariableValidation: {Default: false, PreRelease: featuregate.Alpha}, + RotateKubeletServerCertificate: {Default: true, PreRelease: featuregate.Beta}, RuntimeClassInImageCriAPI: {Default: false, PreRelease: featuregate.Alpha},