Skip to content

Commit

Permalink
Allows the dash (-) character to be included in environment variable …
Browse files Browse the repository at this point in the history
…keys

in the build configuration.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1614155
  • Loading branch information
coreydaley committed Aug 23, 2018
1 parent cd7d048 commit 355a711
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 6 deletions.
6 changes: 2 additions & 4 deletions pkg/build/apis/build/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
2 changes: 1 addition & 1 deletion pkg/oc/util/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion test/cmd/env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down

0 comments on commit 355a711

Please sign in to comment.