do not split --env (and few other flag values) by comma

cppforlife · Mar 21, 2019 · 1a23540 · 1a23540
1 parent 9cb6e13
commit 1a23540
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 20 deletions.
diff --git a/docs/cmd/knctl_build_create.md b/docs/cmd/knctl_build_create.md
@@ -32,20 +32,20 @@ knctl build create [flags]
 ### Options
 
 ```
-  -b, --build string             Specified build
-  -d, --directory string         Set source code directory
-      --generate-name            Set to generate name
-      --git-revision string      Set Git revision (examples: https://git-scm.com/docs/gitrevisions#_specifying_revisions)
-      --git-url string           Set Git URL
-  -h, --help                     help for create
-  -i, --image string             Set image URL
-  -n, --namespace string         Specified namespace ($KNCTL_NAMESPACE or default from kubeconfig)
-      --service-account string   Set service account name for building
-      --template string          Set template name
-      --template-arg strings     Set template argument (format: key=value) (can be specified multiple times)
-      --template-env strings     Set template environment variable (format: key=value) (can be specified multiple times)
-      --template-kind string     Set to 'cluster' to use ClusterBuildTemplate kind of templates
-      --timeout duration         Set timeout for building stage (Knative Build has a 10m default)
+  -b, --build string               Specified build
+  -d, --directory string           Set source code directory
+      --generate-name              Set to generate name
+      --git-revision string        Set Git revision (examples: https://git-scm.com/docs/gitrevisions#_specifying_revisions)
+      --git-url string             Set Git URL
+  -h, --help                       help for create
+  -i, --image string               Set image URL
+  -n, --namespace string           Specified namespace ($KNCTL_NAMESPACE or default from kubeconfig)
+      --service-account string     Set service account name for building
+      --template string            Set template name
+      --template-arg stringArray   Set template argument (format: key=value) (can be specified multiple times)
+      --template-env stringArray   Set template environment variable (format: key=value) (can be specified multiple times)
+      --template-kind string       Set to 'cluster' to use ClusterBuildTemplate kind of templates
+      --timeout duration           Set timeout for building stage (Knative Build has a 10m default)
 ```
 
 ### Options inherited from parent commands

diff --git a/docs/cmd/knctl_deploy.md b/docs/cmd/knctl_deploy.md
@@ -47,7 +47,7 @@ knctl deploy [flags]
   -a, --annotation strings                      Set annotation (format: key=value) (can be specified multiple times)
       --build-timeout duration                  Set timeout for building stage (Knative Build has a 10m default)
   -d, --directory string                        Set source code directory
-  -e, --env strings                             Set environment variable (format: ENV_KEY=value) (can be specified multiple times)
+  -e, --env stringArray                         Set environment variable (format: ENV_KEY=value) (can be specified multiple times)
       --env-config-map strings                  Set environment variable from a config map (format: ENV_KEY=config-map-name/key) (can be specified multiple times)
       --env-secret strings                      Set environment variable from a secret (format: ENV_KEY=secret-name/key) (can be specified multiple times)
       --generate-name                           Set to generate name
@@ -61,8 +61,8 @@ knctl deploy [flags]
       --service-account string                  Set service account name for building
   -t, --tag strings                             Set tag (format: value) (can be specified multiple times)
       --template string                         Set template name
-      --template-arg strings                    Set template argument (format: key=value) (can be specified multiple times)
-      --template-env strings                    Set template environment variable (format: key=value) (can be specified multiple times)
+      --template-arg stringArray                Set template argument (format: key=value) (can be specified multiple times)
+      --template-env stringArray                Set template environment variable (format: key=value) (can be specified multiple times)
       --template-kind string                    Set to 'cluster' to use ClusterBuildTemplate kind of templates
       --watch-pod-logs                          Watch pod logs for new revision (default true)
   -l, --watch-pod-logs-indefinitely             Watch pod logs for new revision indefinitely

diff --git a/pkg/knctl/cmd/build/create_flags.go b/pkg/knctl/cmd/build/create_flags.go
@@ -67,8 +67,8 @@ func (s *CreateArgsFlags) setWithPrefix(prefix string, cmd *cobra.Command, flags
 
 	cmd.Flags().StringVar(&s.TemplateKind, "template-kind", "", "Set to 'cluster' to use ClusterBuildTemplate kind of templates")
 	cmd.Flags().StringVar(&s.TemplateName, "template", "", "Set template name")
-	cmd.Flags().StringSliceVar(&s.TemplateArgs, "template-arg", nil, "Set template argument (format: key=value) (can be specified multiple times)")
-	cmd.Flags().StringSliceVar(&s.TemplateEnv, "template-env", nil, "Set template environment variable (format: key=value) (can be specified multiple times)")
+	cmd.Flags().StringArrayVar(&s.TemplateArgs, "template-arg", nil, "Set template argument (format: key=value) (can be specified multiple times)")
+	cmd.Flags().StringArrayVar(&s.TemplateEnv, "template-env", nil, "Set template environment variable (format: key=value) (can be specified multiple times)")
 
 	cmd.Flags().DurationVar(&s.Timeout, prefix+"timeout", time.Duration(0), "Set timeout for building stage (Knative Build has a 10m default)")
 }

diff --git a/pkg/knctl/cmd/service/deploy_flags.go b/pkg/knctl/cmd/service/deploy_flags.go
@@ -66,7 +66,7 @@ func (s *DeployFlags) Set(cmd *cobra.Command, flagsFactory cmdcore.FlagsFactory)
 	cmd.Flags().BoolVarP(&s.WatchPodLogsIndefinitely, "watch-pod-logs-indefinitely", "l",
 		false, "Watch pod logs for new revision indefinitely")
 
-	cmd.Flags().StringSliceVarP(&s.EnvVars, "env", "e", nil, "Set environment variable (format: ENV_KEY=value) (can be specified multiple times)")
+	cmd.Flags().StringArrayVarP(&s.EnvVars, "env", "e", nil, "Set environment variable (format: ENV_KEY=value) (can be specified multiple times)")
 	cmd.Flags().StringSliceVar(&s.EnvSecrets, "env-secret", nil, "Set environment variable from a secret (format: ENV_KEY=secret-name/key) (can be specified multiple times)")
 	cmd.Flags().StringSliceVar(&s.EnvConfigMaps, "env-config-map", nil, "Set environment variable from a config map (format: ENV_KEY=config-map-name/key) (can be specified multiple times)")
 

diff --git a/pkg/knctl/cmd/service/deploy_test.go b/pkg/knctl/cmd/service/deploy_test.go
@@ -200,6 +200,31 @@ func TestNewDeployCmd_ManagedRouteDisabled(t *testing.T) {
 	})
 }
 
+func TestNewDeployCmd_EnvValueWithCommas(t *testing.T) {
+	realCmd := NewDeployOptions(nil, cmdcore.NewConfigFactoryImpl(), cmdcore.NewDepsFactory())
+	cmd := NewTestCmd(t, NewDeployCmd(realCmd, cmdcore.FlagsFactory{}))
+	cmd.Execute([]string{
+		"--namespace", "test-namespace",
+		"--service", "test-service",
+		"--image", "test-image",
+		"--env", "env-key=env-val1,env-val2,env-val3",
+	})
+	cmd.ExpectReachesExecution()
+
+	DeepEqual(t, realCmd.ServiceFlags,
+		cmdflags.ServiceFlags{cmdcore.NamespaceFlags{"test-namespace"}, "test-service"})
+
+	DeepEqual(t, realCmd.DeployFlags, DeployFlags{
+		Image:   "test-image",
+		EnvVars: []string{"env-key=env-val1,env-val2,env-val3"},
+
+		WatchRevisionReady:        true,
+		WatchRevisionReadyTimeout: 5 * time.Minute,
+		WatchPodLogs:              true,
+		ManagedRoute:              true,
+	})
+}
+
 func TestNewDeployCmd_RequiredFlags(t *testing.T) {
 	realCmd := NewDeployOptions(nil, cmdcore.NewConfigFactoryImpl(), cmdcore.NewDepsFactory())
 	cmd := NewTestCmd(t, NewDeployCmd(realCmd, cmdcore.FlagsFactory{}))