From 355a71199c9c5a9e1575438a95186a84b2b02bed Mon Sep 17 00:00:00 2001 From: Corey Daley Date: Thu, 23 Aug 2018 00:11:08 -0400 Subject: [PATCH] Allows the dash (-) character to be included in environment variable keys in the build configuration. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1614155 --- pkg/build/apis/build/validation/validation.go | 6 ++---- pkg/oc/util/env/env.go | 2 +- test/cmd/env.sh | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/build/apis/build/validation/validation.go b/pkg/build/apis/build/validation/validation.go index b107ad546e62..3e8463aa95f8 100644 --- a/pkg/build/apis/build/validation/validation.go +++ b/pkg/build/apis/build/validation/validation.go @@ -679,8 +679,6 @@ func ValidateBuildLogOptions(opts *buildapi.BuildLogOptions) field.ErrorList { return allErrs } -var cIdentifierErrorMsg string = `must be a C identifier (mixed-case letters, numbers, and underscores): e.g. "my_name" or "MyName"` - func ValidateStrategyEnv(vars []kapi.EnvVar, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -688,8 +686,8 @@ func ValidateStrategyEnv(vars []kapi.EnvVar, fldPath *field.Path) field.ErrorLis idxPath := fldPath.Index(i) if len(ev.Name) == 0 { allErrs = append(allErrs, field.Required(idxPath.Child("name"), "")) - } else if len(kvalidation.IsCIdentifier(ev.Name)) > 0 { - allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ev.Name, cIdentifierErrorMsg)) + } else if errs := kvalidation.IsEnvVarName(ev.Name); len(errs) != 0 { + allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), ev.Name, strings.Join(errs, "; "))) } if ev.ValueFrom != nil && ev.ValueFrom.ResourceFieldRef != nil { allErrs = append(allErrs, field.Invalid(idxPath.Child("valueFrom").Child("ResourceFieldRef"), ev.Name, "ResourceFieldRef is not valid for valueFrom in build environment variables")) diff --git a/pkg/oc/util/env/env.go b/pkg/oc/util/env/env.go index 94bbfc7d79bc..871714bd542f 100644 --- a/pkg/oc/util/env/env.go +++ b/pkg/oc/util/env/env.go @@ -59,7 +59,7 @@ func parseIntoEnvVar(spec []string, defaultReader io.Reader, envVarType string) parts := strings.SplitN(envSpec, "=", 2) n, v := parts[0], parts[1] if errs := validation.IsEnvVarName(n); len(errs) != 0 { - return nil, nil, fmt.Errorf("%ss must be of the form key=value, but is %q", envVarType, envSpec) + return nil, nil, fmt.Errorf("%s %s is invalid, %s", envVarType, envSpec, strings.Join(errs, "; ")) } exists.Insert(n) env = append(env, kapi.EnvVar{ diff --git a/test/cmd/env.sh b/test/cmd/env.sh index f767cc9f47d8..1880ccccd548 100755 --- a/test/cmd/env.sh +++ b/test/cmd/env.sh @@ -11,7 +11,7 @@ os::cmd::expect_success_and_text 'oc set env dc/node --list' 'deploymentconfigs/ os::cmd::expect_success_and_text 'oc set env dc --all --containers="node" key-' 'deploymentconfig.apps.openshift.io/node updated' os::cmd::expect_failure_and_text 'oc set env dc --all --containers="node"' 'error: at least one environment variable must be provided' os::cmd::expect_failure_and_not_text 'oc set env --from=secret/mysecret dc/node' 'error: at least one environment variable must be provided' -os::cmd::expect_failure_and_text 'oc set env dc/node test#abc=1234' 'environment variables must be of the form key=value, but is "test#abc=1234"' +os::cmd::expect_failure_and_text 'oc set env dc/node test#abc=1234' 'environment variable test#abc=1234 is invalid, a valid environment variable name must consist of alphabetic characters' # ensure deleting a var through --env does not result in an error message os::cmd::expect_success_and_text 'oc set env dc/node key=value' 'deploymentconfig.apps.openshift.io/node updated'