diff --git a/pkg/api/types.go b/pkg/api/types.go index ab9b7056d4f..2f8f2b01aeb 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -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. diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index a6b49774425..c92b699737b 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -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)) } } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 4dbd31dd5eb..929a9ddfb3c 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -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"}}, diff --git a/pkg/kubectl/configmap_test.go b/pkg/kubectl/configmap_test.go index 5690c2e0f0a..b5c98b58f4c 100644 --- a/pkg/kubectl/configmap_test.go +++ b/pkg/kubectl/configmap_test.go @@ -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", diff --git a/pkg/kubectl/env_file.go b/pkg/kubectl/env_file.go index ce24c710441..19598d5354a 100644 --- a/pkg/kubectl/env_file.go +++ b/pkg/kubectl/env_file.go @@ -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, ";")) } diff --git a/pkg/kubectl/run.go b/pkg/kubectl/run.go index e1f4549d62c..dbf9ba7d0c0 100644 --- a/pkg/kubectl/run.go +++ b/pkg/kubectl/run.go @@ -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} diff --git a/pkg/kubectl/run_test.go b/pkg/kubectl/run_test.go index 16a6e9423ab..114550ff3b2 100644 --- a/pkg/kubectl/run_test.go +++ b/pkg/kubectl/run_test.go @@ -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", diff --git a/pkg/kubectl/secret_test.go b/pkg/kubectl/secret_test.go index 309c7848d65..e7c1e7478be 100644 --- a/pkg/kubectl/secret_test.go +++ b/pkg/kubectl/secret_test.go @@ -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", diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index f98a57cc5e5..b3e752f49e2 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -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 } diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index 16b8bf68b89..986de4fac8c 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -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", }, }, diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go index b1fcc570812..85ba8467bb8 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go @@ -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 +}