github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-31 15:25:57 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #48986 from timoreimann/relax-env-var-naming-restrictions

Browse Source 
Automatic merge from submit-queue (batch tested with PRs 50208, 50259, 49702, 50267, 48986)

Relax restrictions on environment variable names.

Fixes #2707

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. (Reference [here](http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html), my prose reasoning [here](https://github.com/kubernetes/kubernetes/issues/2707#issuecomment-285309156).)

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.
This commit is contained in:
Kubernetes Submit Queue 2017-08-08 01:53:08 -07:00 committed by GitHub
commit 243e655161
11 changed files with 88 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/api/types.go
View File

@ -1440,7 +1440,7 @@ type SecretKeySelector struct {
// EnvFromSource represents the source of a set of ConfigMaps
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
Prefix string
// The ConfigMap to select from.

4
pkg/api/validation/validation.go
View File

@ -1548,7 +1548,7 @@ func ValidateEnv(vars []api.EnvVar, fldPath *field.Path) field.ErrorList {
if len(ev.Name) == 0 {
allErrs = append(allErrs, field.Required(idxPath.Child("name"), ""))
} 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))
}
}
@ -1637,7 +1637,7 @@ func ValidateEnvFrom(vars []api.EnvFromSource, fldPath *field.Path) field.ErrorL
for i, ev := range vars {
idxPath := fldPath.Index(i)
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))
}
}

47
pkg/api/validation/validation_test.go
View File

@ -43,7 +43,7 @@ const (
maxLengthErrMsg = "must be no more than"
namePartErrMsg = "name part 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 {
@ -2575,6 +2575,8 @@ func TestValidateEnv(t *testing.T) {
{Name: "ABC", Value: "value"},
{Name: "AbC_123", Value: "value"},
{Name: "abc", Value: ""},
{Name: "a.b.c", Value: "value"},
{Name: "a-b-c", Value: "value"},
{
Name: "abc",
ValueFrom: &api.EnvVarSource{
@ -2676,9 +2678,24 @@ func TestValidateEnv(t *testing.T) {
expectedError: "[0].name: Required value",
},
{
name: "name not a C identifier",
envs: []api.EnvVar{{Name: "a.b.c"}},
expectedError: `[0].name: Invalid value: "a.b.c": ` + idErrMsg,
name: "illegal character",
envs: []api.EnvVar{{Name: "a!b"}},
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",
@ -2897,6 +2914,12 @@ func TestValidateEnvFrom(t *testing.T) {
LocalObjectReference: api.LocalObjectReference{Name: "abc"},
},
},
{
Prefix: "a.b",
ConfigMapRef: &api.ConfigMapEnvSource{
LocalObjectReference: api.LocalObjectReference{Name: "abc"},
},
},
{
SecretRef: &api.SecretEnvSource{
LocalObjectReference: api.LocalObjectReference{Name: "abc"},
@ -2908,6 +2931,12 @@ func TestValidateEnvFrom(t *testing.T) {
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 {
t.Errorf("expected success: %v", errs)
@ -2942,12 +2971,12 @@ func TestValidateEnvFrom(t *testing.T) {
name: "invalid prefix",
envs: []api.EnvFromSource{
{
Prefix: "a.b",
Prefix: "a!b",
ConfigMapRef: &api.ConfigMapEnvSource{
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",
@ -2973,12 +3002,12 @@ func TestValidateEnvFrom(t *testing.T) {
name: "invalid prefix",
envs: []api.EnvFromSource{
{
Prefix: "a.b",
Prefix: "a!b",
SecretRef: &api.SecretEnvSource{
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",
@ -3374,7 +3403,7 @@ func TestValidateContainers(t *testing.T) {
ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"},
},
"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": {
{Name: "abc", Image: "image", VolumeMounts: []api.VolumeMount{{Name: "anything", MountPath: "/foo"}},

2
pkg/kubectl/configmap_test.go
View File

@ -157,7 +157,7 @@ func TestConfigMapGenerate(t *testing.T) {
expectErr: true,
},
{
setup: setupEnvFile("key.1=value1"),
setup: setupEnvFile("key#1=value1"),
params: map[string]interface{}{
"name": "invalid_key",
"from-env-file": "file.env",

2
pkg/kubectl/env_file.go
View File

@ -55,7 +55,7 @@ func proccessEnvFileLine(line []byte, filePath string,
data := strings.SplitN(string(line), "=", 2)
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, ";"))
}

2
pkg/kubectl/run.go
View File

@ -868,7 +868,7 @@ func parseEnvs(envArray []string) ([]v1.EnvVar, error) {
if len(name) == 0 {
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)
}
envVar := v1.EnvVar{Name: name, Value: value}

5
pkg/kubectl/run_test.go
View File

@ -1030,6 +1030,7 @@ func TestParseEnv(t *testing.T) {
{
envArray: []string{
"THIS_ENV=isOK",
"this.dotted.env=isOKToo",
"HAS_COMMAS=foo,bar",
"HAS_EQUALS=jJnro54iUu75xNy==",
},
@ -1038,6 +1039,10 @@ func TestParseEnv(t *testing.T) {
Name: "THIS_ENV",
Value: "isOK",
},
{
Name: "this.dotted.env",
Value: "isOKToo",
},
{
Name: "HAS_COMMAS",
Value: "foo,bar",

2
pkg/kubectl/secret_test.go
View File

@ -157,7 +157,7 @@ func TestSecretGenerate(t *testing.T) {
expectErr: true,
},
{
setup: setupEnvFile("key.1=value1"),
setup: setupEnvFile("key#1=value1"),
params: map[string]interface{}{
"name": "invalid_key",
"from-env-file": "file.env",

4
pkg/kubelet/kubelet_pods.go
View File

@ -472,7 +472,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container
if len(envFrom.Prefix) > 0 {
k = envFrom.Prefix + k
}
if errMsgs := utilvalidation.IsCIdentifier(k); len(errMsgs) != 0 {
if errMsgs := utilvalidation.IsEnvVarName(k); len(errMsgs) != 0 {
invalidKeys = append(invalidKeys, k)
continue
}
@ -507,7 +507,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container
if len(envFrom.Prefix) > 0 {
k = envFrom.Prefix + k
}
if errMsgs := utilvalidation.IsCIdentifier(k); len(errMsgs) != 0 {
if errMsgs := utilvalidation.IsEnvVarName(k); len(errMsgs) != 0 {
invalidKeys = append(invalidKeys, k)
continue
}

12
pkg/kubelet/kubelet_pods_test.go
View File

@ -1219,14 +1219,14 @@ func TestMakeEnvironmentVariables(t *testing.T) {
Name: "test-secret",
},
Data: map[string][]byte{
"1234": []byte("abc"),
"1z": []byte("abc"),
"key": []byte("value"),
"1234": []byte("abc"),
"1z": []byte("abc"),
"key.1": []byte("value"),
},
},
expectedEnvs: []kubecontainer.EnvVar{
{
Name: "key",
Name: "key.1",
Value: "value",
},
},
@ -1250,12 +1250,12 @@ func TestMakeEnvironmentVariables(t *testing.T) {
Name: "test-secret",
},
Data: map[string][]byte{
"1234": []byte("abc"),
"1234.name": []byte("abc"),
},
},
expectedEnvs: []kubecontainer.EnvVar{
{
Name: "p_1234",
Name: "p_1234.name",
Value: "abc",
},
},

37
staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go
View File

@ -277,6 +277,22 @@ func IsHTTPHeaderName(value string) []string {
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 configMapKeyErrMsg string = "a valid config key must consist of alphanumeric characters, '-', '_' or '.'"
@ -291,13 +307,7 @@ func IsConfigMapKey(value string) []string {
if !configMapKeyRegexp.MatchString(value) {
errs = append(errs, RegexError(configMapKeyErrMsg, configMapKeyFmt, "key.name", "KEY_NAME", "key-name"))
}
if 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 '..'`)
}
errs = append(errs, hasChDirPrefix(value)...)
return errs
}
@ -341,3 +351,16 @@ func prefixEach(msgs []string, prefix string) []string {
func InclusiveRangeError(lo, hi int) string {
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
}