Commit 604dfb31 authored by Timo Reimann's avatar Timo Reimann

Relax restrictions on environment variable names.

The POSIX standard restricts environment variable names to uppercase letters, digits, and the underscore character in shell contexts only. For generic application usage, it is stated that all other characters shall be tolerated. This change relaxes the rules to some degree. Namely, we stop requiring environment variable names to be strict C_IDENTIFIERS and start permitting lowercase, dot, and dash characters. Public container images using environment variable names beyond the shell-only context can benefit from this relaxation. Elasticsearch is one popular example.
parent 12cc6425
...@@ -1440,7 +1440,7 @@ type SecretKeySelector struct { ...@@ -1440,7 +1440,7 @@ type SecretKeySelector struct {
// EnvFromSource represents the source of a set of ConfigMaps // EnvFromSource represents the source of a set of ConfigMaps
type EnvFromSource struct { type EnvFromSource struct {
// An optional identifier to prepend to each key in the ConfigMap. Must be a C_IDENTIFIER. // An optional identifier to prepend to each key in the ConfigMap.
// +optional // +optional
Prefix string Prefix string
// The ConfigMap to select from. // The ConfigMap to select from.
......
...@@ -1548,7 +1548,7 @@ func ValidateEnv(vars []api.EnvVar, fldPath *field.Path) field.ErrorList { ...@@ -1548,7 +1548,7 @@ func ValidateEnv(vars []api.EnvVar, fldPath *field.Path) field.ErrorList {
if len(ev.Name) == 0 { if len(ev.Name) == 0 {
allErrs = append(allErrs, field.Required(idxPath.Child("name"), "")) allErrs = append(allErrs, field.Required(idxPath.Child("name"), ""))
} else { } else {
for _, msg := range validation.IsCIdentifier(ev.Name) { for _, msg := range validation.IsEnvVarName(ev.Name) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ev.Name, msg)) allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ev.Name, msg))
} }
} }
...@@ -1637,7 +1637,7 @@ func ValidateEnvFrom(vars []api.EnvFromSource, fldPath *field.Path) field.ErrorL ...@@ -1637,7 +1637,7 @@ func ValidateEnvFrom(vars []api.EnvFromSource, fldPath *field.Path) field.ErrorL
for i, ev := range vars { for i, ev := range vars {
idxPath := fldPath.Index(i) idxPath := fldPath.Index(i)
if len(ev.Prefix) > 0 { if len(ev.Prefix) > 0 {
for _, msg := range validation.IsCIdentifier(ev.Prefix) { for _, msg := range validation.IsEnvVarName(ev.Prefix) {
allErrs = append(allErrs, field.Invalid(idxPath.Child("prefix"), ev.Prefix, msg)) allErrs = append(allErrs, field.Invalid(idxPath.Child("prefix"), ev.Prefix, msg))
} }
} }
......
...@@ -43,7 +43,7 @@ const ( ...@@ -43,7 +43,7 @@ const (
maxLengthErrMsg = "must be no more than" maxLengthErrMsg = "must be no more than"
namePartErrMsg = "name part must consist of" namePartErrMsg = "name part must consist of"
nameErrMsg = "a qualified name must consist of" nameErrMsg = "a qualified name must consist of"
idErrMsg = "a valid C identifier must" envVarNameErrMsg = "a valid environment variable name must consist of"
) )
func testVolume(name string, namespace string, spec api.PersistentVolumeSpec) *api.PersistentVolume { func testVolume(name string, namespace string, spec api.PersistentVolumeSpec) *api.PersistentVolume {
...@@ -2575,6 +2575,8 @@ func TestValidateEnv(t *testing.T) { ...@@ -2575,6 +2575,8 @@ func TestValidateEnv(t *testing.T) {
{Name: "ABC", Value: "value"}, {Name: "ABC", Value: "value"},
{Name: "AbC_123", Value: "value"}, {Name: "AbC_123", Value: "value"},
{Name: "abc", Value: ""}, {Name: "abc", Value: ""},
{Name: "a.b.c", Value: "value"},
{Name: "a-b-c", Value: "value"},
{ {
Name: "abc", Name: "abc",
ValueFrom: &api.EnvVarSource{ ValueFrom: &api.EnvVarSource{
...@@ -2676,9 +2678,24 @@ func TestValidateEnv(t *testing.T) { ...@@ -2676,9 +2678,24 @@ func TestValidateEnv(t *testing.T) {
expectedError: "[0].name: Required value", expectedError: "[0].name: Required value",
}, },
{ {
name: "name not a C identifier", name: "illegal character",
envs: []api.EnvVar{{Name: "a.b.c"}}, envs: []api.EnvVar{{Name: "a!b"}},
expectedError: `[0].name: Invalid value: "a.b.c": ` + idErrMsg, expectedError: `[0].name: Invalid value: "a!b": ` + envVarNameErrMsg,
},
{
name: "dot only",
envs: []api.EnvVar{{Name: "."}},
expectedError: `[0].name: Invalid value: ".": must not be`,
},
{
name: "double dots only",
envs: []api.EnvVar{{Name: ".."}},
expectedError: `[0].name: Invalid value: "..": must not be`,
},
{
name: "leading double dots",
envs: []api.EnvVar{{Name: "..abc"}},
expectedError: `[0].name: Invalid value: "..abc": must not start with`,
}, },
{ {
name: "value and valueFrom specified", name: "value and valueFrom specified",
...@@ -2898,6 +2915,12 @@ func TestValidateEnvFrom(t *testing.T) { ...@@ -2898,6 +2915,12 @@ func TestValidateEnvFrom(t *testing.T) {
}, },
}, },
{ {
Prefix: "a.b",
ConfigMapRef: &api.ConfigMapEnvSource{
LocalObjectReference: api.LocalObjectReference{Name: "abc"},
},
},
{
SecretRef: &api.SecretEnvSource{ SecretRef: &api.SecretEnvSource{
LocalObjectReference: api.LocalObjectReference{Name: "abc"}, LocalObjectReference: api.LocalObjectReference{Name: "abc"},
}, },
...@@ -2908,6 +2931,12 @@ func TestValidateEnvFrom(t *testing.T) { ...@@ -2908,6 +2931,12 @@ func TestValidateEnvFrom(t *testing.T) {
LocalObjectReference: api.LocalObjectReference{Name: "abc"}, LocalObjectReference: api.LocalObjectReference{Name: "abc"},
}, },
}, },
{
Prefix: "a.b",
SecretRef: &api.SecretEnvSource{
LocalObjectReference: api.LocalObjectReference{Name: "abc"},
},
},
} }
if errs := ValidateEnvFrom(successCase, field.NewPath("field")); len(errs) != 0 { if errs := ValidateEnvFrom(successCase, field.NewPath("field")); len(errs) != 0 {
t.Errorf("expected success: %v", errs) t.Errorf("expected success: %v", errs)
...@@ -2942,12 +2971,12 @@ func TestValidateEnvFrom(t *testing.T) { ...@@ -2942,12 +2971,12 @@ func TestValidateEnvFrom(t *testing.T) {
name: "invalid prefix", name: "invalid prefix",
envs: []api.EnvFromSource{ envs: []api.EnvFromSource{
{ {
Prefix: "a.b", Prefix: "a!b",
ConfigMapRef: &api.ConfigMapEnvSource{ ConfigMapRef: &api.ConfigMapEnvSource{
LocalObjectReference: api.LocalObjectReference{Name: "abc"}}, LocalObjectReference: api.LocalObjectReference{Name: "abc"}},
}, },
}, },
expectedError: `field[0].prefix: Invalid value: "a.b": ` + idErrMsg, expectedError: `field[0].prefix: Invalid value: "a!b": ` + envVarNameErrMsg,
}, },
{ {
name: "zero-length name", name: "zero-length name",
...@@ -2973,12 +3002,12 @@ func TestValidateEnvFrom(t *testing.T) { ...@@ -2973,12 +3002,12 @@ func TestValidateEnvFrom(t *testing.T) {
name: "invalid prefix", name: "invalid prefix",
envs: []api.EnvFromSource{ envs: []api.EnvFromSource{
{ {
Prefix: "a.b", Prefix: "a!b",
SecretRef: &api.SecretEnvSource{ SecretRef: &api.SecretEnvSource{
LocalObjectReference: api.LocalObjectReference{Name: "abc"}}, LocalObjectReference: api.LocalObjectReference{Name: "abc"}},
}, },
}, },
expectedError: `field[0].prefix: Invalid value: "a.b": ` + idErrMsg, expectedError: `field[0].prefix: Invalid value: "a!b": ` + envVarNameErrMsg,
}, },
{ {
name: "no refs", name: "no refs",
...@@ -3374,7 +3403,7 @@ func TestValidateContainers(t *testing.T) { ...@@ -3374,7 +3403,7 @@ func TestValidateContainers(t *testing.T) {
ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"},
}, },
"invalid env var name": { "invalid env var name": {
{Name: "abc", Image: "image", Env: []api.EnvVar{{Name: "ev.1"}}, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, {Name: "abc", Image: "image", Env: []api.EnvVar{{Name: "ev!1"}}, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"},
}, },
"unknown volume name": { "unknown volume name": {
{Name: "abc", Image: "image", VolumeMounts: []api.VolumeMount{{Name: "anything", MountPath: "/foo"}}, {Name: "abc", Image: "image", VolumeMounts: []api.VolumeMount{{Name: "anything", MountPath: "/foo"}},
......
...@@ -157,7 +157,7 @@ func TestConfigMapGenerate(t *testing.T) { ...@@ -157,7 +157,7 @@ func TestConfigMapGenerate(t *testing.T) {
expectErr: true, expectErr: true,
}, },
{ {
setup: setupEnvFile("key.1=value1"), setup: setupEnvFile("key#1=value1"),
params: map[string]interface{}{ params: map[string]interface{}{
"name": "invalid_key", "name": "invalid_key",
"from-env-file": "file.env", "from-env-file": "file.env",
......
...@@ -55,7 +55,7 @@ func proccessEnvFileLine(line []byte, filePath string, ...@@ -55,7 +55,7 @@ func proccessEnvFileLine(line []byte, filePath string,
data := strings.SplitN(string(line), "=", 2) data := strings.SplitN(string(line), "=", 2)
key = data[0] key = data[0]
if errs := validation.IsCIdentifier(key); len(errs) != 0 { if errs := validation.IsEnvVarName(key); len(errs) != 0 {
return ``, ``, fmt.Errorf("%q is not a valid key name: %s", key, strings.Join(errs, ";")) return ``, ``, fmt.Errorf("%q is not a valid key name: %s", key, strings.Join(errs, ";"))
} }
......
...@@ -868,7 +868,7 @@ func parseEnvs(envArray []string) ([]v1.EnvVar, error) { ...@@ -868,7 +868,7 @@ func parseEnvs(envArray []string) ([]v1.EnvVar, error) {
if len(name) == 0 { if len(name) == 0 {
return nil, fmt.Errorf("invalid env: %v", env) return nil, fmt.Errorf("invalid env: %v", env)
} }
if len(validation.IsCIdentifier(name)) != 0 { if len(validation.IsEnvVarName(name)) != 0 {
return nil, fmt.Errorf("invalid env: %v", env) return nil, fmt.Errorf("invalid env: %v", env)
} }
envVar := v1.EnvVar{Name: name, Value: value} envVar := v1.EnvVar{Name: name, Value: value}
......
...@@ -1030,6 +1030,7 @@ func TestParseEnv(t *testing.T) { ...@@ -1030,6 +1030,7 @@ func TestParseEnv(t *testing.T) {
{ {
envArray: []string{ envArray: []string{
"THIS_ENV=isOK", "THIS_ENV=isOK",
"this.dotted.env=isOKToo",
"HAS_COMMAS=foo,bar", "HAS_COMMAS=foo,bar",
"HAS_EQUALS=jJnro54iUu75xNy==", "HAS_EQUALS=jJnro54iUu75xNy==",
}, },
...@@ -1039,6 +1040,10 @@ func TestParseEnv(t *testing.T) { ...@@ -1039,6 +1040,10 @@ func TestParseEnv(t *testing.T) {
Value: "isOK", Value: "isOK",
}, },
{ {
Name: "this.dotted.env",
Value: "isOKToo",
},
{
Name: "HAS_COMMAS", Name: "HAS_COMMAS",
Value: "foo,bar", Value: "foo,bar",
}, },
......
...@@ -157,7 +157,7 @@ func TestSecretGenerate(t *testing.T) { ...@@ -157,7 +157,7 @@ func TestSecretGenerate(t *testing.T) {
expectErr: true, expectErr: true,
}, },
{ {
setup: setupEnvFile("key.1=value1"), setup: setupEnvFile("key#1=value1"),
params: map[string]interface{}{ params: map[string]interface{}{
"name": "invalid_key", "name": "invalid_key",
"from-env-file": "file.env", "from-env-file": "file.env",
......
...@@ -472,7 +472,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container ...@@ -472,7 +472,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container
if len(envFrom.Prefix) > 0 { if len(envFrom.Prefix) > 0 {
k = envFrom.Prefix + k k = envFrom.Prefix + k
} }
if errMsgs := utilvalidation.IsCIdentifier(k); len(errMsgs) != 0 { if errMsgs := utilvalidation.IsEnvVarName(k); len(errMsgs) != 0 {
invalidKeys = append(invalidKeys, k) invalidKeys = append(invalidKeys, k)
continue continue
} }
...@@ -507,7 +507,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container ...@@ -507,7 +507,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container
if len(envFrom.Prefix) > 0 { if len(envFrom.Prefix) > 0 {
k = envFrom.Prefix + k k = envFrom.Prefix + k
} }
if errMsgs := utilvalidation.IsCIdentifier(k); len(errMsgs) != 0 { if errMsgs := utilvalidation.IsEnvVarName(k); len(errMsgs) != 0 {
invalidKeys = append(invalidKeys, k) invalidKeys = append(invalidKeys, k)
continue continue
} }
......
...@@ -1219,14 +1219,14 @@ func TestMakeEnvironmentVariables(t *testing.T) { ...@@ -1219,14 +1219,14 @@ func TestMakeEnvironmentVariables(t *testing.T) {
Name: "test-secret", Name: "test-secret",
}, },
Data: map[string][]byte{ Data: map[string][]byte{
"1234": []byte("abc"), "1234": []byte("abc"),
"1z": []byte("abc"), "1z": []byte("abc"),
"key": []byte("value"), "key.1": []byte("value"),
}, },
}, },
expectedEnvs: []kubecontainer.EnvVar{ expectedEnvs: []kubecontainer.EnvVar{
{ {
Name: "key", Name: "key.1",
Value: "value", Value: "value",
}, },
}, },
...@@ -1250,12 +1250,12 @@ func TestMakeEnvironmentVariables(t *testing.T) { ...@@ -1250,12 +1250,12 @@ func TestMakeEnvironmentVariables(t *testing.T) {
Name: "test-secret", Name: "test-secret",
}, },
Data: map[string][]byte{ Data: map[string][]byte{
"1234": []byte("abc"), "1234.name": []byte("abc"),
}, },
}, },
expectedEnvs: []kubecontainer.EnvVar{ expectedEnvs: []kubecontainer.EnvVar{
{ {
Name: "p_1234", Name: "p_1234.name",
Value: "abc", Value: "abc",
}, },
}, },
......
...@@ -277,6 +277,22 @@ func IsHTTPHeaderName(value string) []string { ...@@ -277,6 +277,22 @@ func IsHTTPHeaderName(value string) []string {
return nil return nil
} }
const envVarNameFmt = "[-._a-zA-Z][-._a-zA-Z0-9]*"
const envVarNameFmtErrMsg string = "a valid environment variable name must consist of alphabetic characters, digits, '_', '-', or '.', and must not start with a digit"
var envVarNameRegexp = regexp.MustCompile("^" + envVarNameFmt + "$")
// IsEnvVarName tests if a string is a valid environment variable name.
func IsEnvVarName(value string) []string {
var errs []string
if !envVarNameRegexp.MatchString(value) {
errs = append(errs, RegexError(envVarNameFmtErrMsg, envVarNameFmt, "my.env-name", "MY_ENV.NAME", "MyEnvName1"))
}
errs = append(errs, hasChDirPrefix(value)...)
return errs
}
const configMapKeyFmt = `[-._a-zA-Z0-9]+` const configMapKeyFmt = `[-._a-zA-Z0-9]+`
const configMapKeyErrMsg string = "a valid config key must consist of alphanumeric characters, '-', '_' or '.'" const configMapKeyErrMsg string = "a valid config key must consist of alphanumeric characters, '-', '_' or '.'"
...@@ -291,13 +307,7 @@ func IsConfigMapKey(value string) []string { ...@@ -291,13 +307,7 @@ func IsConfigMapKey(value string) []string {
if !configMapKeyRegexp.MatchString(value) { if !configMapKeyRegexp.MatchString(value) {
errs = append(errs, RegexError(configMapKeyErrMsg, configMapKeyFmt, "key.name", "KEY_NAME", "key-name")) errs = append(errs, RegexError(configMapKeyErrMsg, configMapKeyFmt, "key.name", "KEY_NAME", "key-name"))
} }
if value == "." { errs = append(errs, hasChDirPrefix(value)...)
errs = append(errs, `must not be '.'`)
} else if value == ".." {
errs = append(errs, `must not be '..'`)
} else if strings.HasPrefix(value, "..") {
errs = append(errs, `must not start with '..'`)
}
return errs return errs
} }
...@@ -341,3 +351,16 @@ func prefixEach(msgs []string, prefix string) []string { ...@@ -341,3 +351,16 @@ func prefixEach(msgs []string, prefix string) []string {
func InclusiveRangeError(lo, hi int) string { func InclusiveRangeError(lo, hi int) string {
return fmt.Sprintf(`must be between %d and %d, inclusive`, lo, hi) return fmt.Sprintf(`must be between %d and %d, inclusive`, lo, hi)
} }
func hasChDirPrefix(value string) []string {
var errs []string
switch {
case value == ".":
errs = append(errs, `must not be '.'`)
case value == "..":
errs = append(errs, `must not be '..'`)
case strings.HasPrefix(value, ".."):
errs = append(errs, `must not start with '..'`)
}
return errs
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment