Merge pull request #20738 from coreydaley/bugzilla_1614155_allow_dash_in_bc_env_vars

Allow dash (-) in BuildConfig env vars

Author: openshift-merge-robot

pkg/build/apis/build/validation/validation.go

@@ -679,17 +679,15 @@ 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{}
 
 	for i, ev := range vars {
 		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"))

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{

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'