oc new-app: allow 'dot' in ENV variable names

pkg/oc/generate/app/env.go

@@ -9,7 +9,7 @@ import (
  "strings"
 
  "github.com/joho/godotenv"
- utilenv "github.com/openshift/origin/pkg/oc/util/env"
+ "k8s.io/apimachinery/pkg/util/validation"
  kapi "k8s.io/kubernetes/pkg/apis/core"
 )
 
@@ -39,10 +39,9 @@ func ParseEnvironment(vals ...string) (Environment, []string, []error) {
  duplicates := []string{}
  env := make(Environment)
  for _, s := range vals {
- valid := utilenv.IsValidEnvironmentArgument(s)
  p := strings.SplitN(s, "=", 2)
- if !valid || len(p) != 2 {
- errs = append(errs, fmt.Errorf("invalid parameter assignment in %q", s))
+ if err := validation.IsEnvVarName(p[0]); len(err) != 0 || len(p) != 2 {
+ errs = append(errs, fmt.Errorf("invalid parameter assignment in %q, %v", s, err))
  continue
  }
  key, val := p[0], p[1]
@@ -166,7 +165,7 @@ func LoadEnvironmentFile(filename string, stdin io.Reader) (Environment, error)
  return nil, fmt.Errorf("Cannot read variables from file %q: %s", errorFilename, err)
  }
  for k, v := range env {
- if !utilenv.IsValidEnvironmentArgument(fmt.Sprintf("%s=%s", k, v)) {
+ if err := validation.IsEnvVarName(k); len(err) != 0 {
  return nil, fmt.Errorf("invalid parameter assignment in %s=%s", k, v)
  }
  }

pkg/oc/util/env/env.go

@@ -8,20 +8,16 @@ import (
  "strings"
 
  "k8s.io/apimachinery/pkg/util/sets"
+ "k8s.io/apimachinery/pkg/util/validation"
  kapi "k8s.io/kubernetes/pkg/apis/core"
 )
 
 var argumentEnvironment = regexp.MustCompile(`(?ms)^(.+)\=(.*)$`)
-var validArgumentEnvironment = regexp.MustCompile(`(?ms)^(\w+)\=(.*)$`)
 
 func IsEnvironmentArgument(s string) bool {
  return argumentEnvironment.MatchString(s)
 }
 
-func IsValidEnvironmentArgument(s string) bool {
- return validArgumentEnvironment.MatchString(s)
-}
-
 func SplitEnvironmentFromResources(args []string) (resources, envArgs []string, ok bool) {
  first := true
  for _, s := range args {
@@ -50,8 +46,6 @@ func parseIntoEnvVar(spec []string, defaultReader io.Reader, envVarType string)
  var remove []string
  for _, envSpec := range spec {
  switch {
- case !IsValidEnvironmentArgument(envSpec) && !strings.HasSuffix(envSpec, "-"):
- return nil, nil, fmt.Errorf("%ss must be of the form key=value and can only contain letters, numbers, and underscores", envVarType)
  case envSpec == "-":
  if defaultReader == nil {
  return nil, nil, fmt.Errorf("when '-' is used, STDIN must be open")
@@ -63,18 +57,19 @@ func parseIntoEnvVar(spec []string, defaultReader io.Reader, envVarType string)
  env = append(env, fileEnv...)
  case strings.Contains(envSpec, "="):
  parts := strings.SplitN(envSpec, "=", 2)
- if len(parts) != 2 {
- return nil, nil, fmt.Errorf("invalid %s: %v", envVarType, envSpec)
+ 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)
  }
- exists.Insert(parts[0])
+ exists.Insert(n)
  env = append(env, kapi.EnvVar{
- Name: parts[0],
- Value: parts[1],
+ Name: n,
+ Value: v,
  })
  case strings.HasSuffix(envSpec, "-"):
  remove = append(remove, envSpec[:len(envSpec)-1])
  default:
- return nil, nil, fmt.Errorf("unknown %s: %v", envVarType, envSpec)
+ return nil, nil, fmt.Errorf("%ss must be of the form key=value, but is %q", envVarType, envSpec)
  }
  }
  for _, removeLabel := range remove {

test/cmd/env.sh

@@ -11,13 +11,17 @@ 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 "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'
+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"'
 
 # 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 "node" updated'
+os::cmd::expect_success_and_text 'oc set env dc/node dots.in.a.key=dots.in.a.value' 'deploymentconfig "node" updated'
 os::cmd::expect_success_and_text 'oc set env dc --all --containers="node" --env=key-' 'deploymentconfig "node" updated'
 # ensure deleting a var through --env actually deletes the env var
 os::cmd::expect_success_and_not_text "oc get dc/node -o jsonpath='{ .spec.template.spec.containers[?(@.name==\"node\")].env }'" 'name\:key'
+os::cmd::expect_success_and_text "oc get dc/node -o jsonpath='{ .spec.template.spec.containers[?(@.name==\"node\")].env }'" 'name\:dots.in.a.key'
+os::cmd::expect_success_and_text 'oc set env dc --all --containers="node" --env=dots.in.a.key-' 'deploymentconfig "node" updated'
+os::cmd::expect_success_and_not_text "oc get dc/node -o jsonpath='{ .spec.template.spec.containers[?(@.name==\"node\")].env }'" 'name\:dots.in.a.key'
 
 # check that env vars are not split at commas
 os::cmd::expect_success_and_text 'oc set env -o yaml dc/node PASS=x,y=z' 'value: x,y=z'

test/cmd/images.sh

@@ -98,9 +98,10 @@ os::cmd::try_until_success 'oc get imagestreamtags tag:8'
 os::cmd::expect_success 'oc create imagestreamtag tag:9 --scheduled --reference-policy=Local --from-image=mysql:latest'
 os::cmd::expect_success 'oc create imagestream tag-b'
 os::cmd::expect_success 'oc create imagestreamtag tag-b:1 --from=wildfly:12.0'
+os::cmd::expect_success 'oc create imagestreamtag tag-c:1 -A annotation.with.dots=are.ok'
 
 os::cmd::expect_failure_and_text 'oc create imagestreamtag tag-c --from-image=mysql:latest' 'must be of the form <stream_name>:<tag>'
-os::cmd::expect_failure_and_text 'oc create imagestreamtag tag-c:1 -A foo' 'annotations must be of the form key=value'
+os::cmd::expect_failure_and_text 'oc create imagestreamtag tag-c:1 -A foo' 'annotations must be of the form key=value, but is "foo"'
 os::cmd::expect_failure_and_text 'oc create imagestreamtag tag-c:2 --from=mysql --from-image=mysql:latest' '\--from and --from-image may not be used together'
 
 os::cmd::expect_success_and_text 'oc get istag/tag:1 -o jsonpath={.image.dockerImageReference}' 'wildfly.*@sha256:'