cli: remove support for comma-separated template values

Author: mmilata

docs/man/man1/oc-new-app.1

@@ -108,7 +108,7 @@ If you provide source code, a new build will be automatically triggered. You can
 
 .PP
 \fB\-p\fP, \fB\-\-param\fP=[]
-    Specify a list of key value pairs (e.g., \-p FOO=BAR,BAR=FOO) to set/override parameter values in the template.
+    Specify a key\-value pair (e.g., \-p FOO=BAR) to set/override parameter value in the template.
 
 .PP
 \fB\-S\fP, \fB\-\-search\fP=false

docs/man/man1/oc-process.1

@@ -54,7 +54,7 @@ The output of the process command is always a list of one or more resources. You
 
 .PP
 \fB\-v\fP, \fB\-\-value\fP=[]
-    Specify a list of key\-value pairs (eg. \-v FOO=BAR,BAR=FOO) to set/override parameter values
+    Specify a key\-value pair (eg. \-v FOO=BAR) to set/override parameter value in the template.
 
 
 .SH OPTIONS INHERITED FROM PARENT COMMANDS

docs/man/man1/openshift-cli-new-app.1

@@ -108,7 +108,7 @@ If you provide source code, a new build will be automatically triggered. You can
 
 .PP
 \fB\-p\fP, \fB\-\-param\fP=[]
-    Specify a list of key value pairs (e.g., \-p FOO=BAR,BAR=FOO) to set/override parameter values in the template.
+    Specify a key\-value pair (e.g., \-p FOO=BAR) to set/override parameter value in the template.
 
 .PP
 \fB\-S\fP, \fB\-\-search\fP=false

docs/man/man1/openshift-cli-process.1

@@ -54,7 +54,7 @@ The output of the process command is always a list of one or more resources. You
 
 .PP
 \fB\-v\fP, \fB\-\-value\fP=[]
-    Specify a list of key\-value pairs (eg. \-v FOO=BAR,BAR=FOO) to set/override parameter values
+    Specify a key\-value pair (eg. \-v FOO=BAR) to set/override parameter value in the template.
 
 
 .SH OPTIONS INHERITED FROM PARENT COMMANDS

pkg/cmd/cli/cli.go

@@ -156,7 +156,7 @@ func NewCommandCLI(name, fullName string, in io.Reader, out, errout io.Writer) *
 				cmd.NewCmdReplace(fullName, f, out),
 				cmd.NewCmdApply(fullName, f, out),
 				cmd.NewCmdPatch(fullName, f, out),
-				cmd.NewCmdProcess(fullName, f, out),
+				cmd.NewCmdProcess(fullName, f, out, errout),
 				cmd.NewCmdExport(fullName, f, in, out),
 				cmd.NewCmdExtract(fullName, f, in, out, errout),
 				observe.NewCmdObserve(fullName, f, out, errout),

pkg/cmd/cli/cmd/helper.go

@@ -1,6 +1,11 @@
 package cmd
 
-import "strings"
+import (
+	"strings"
+
+	"github.com/golang/glog"
+	"github.com/spf13/cobra"
+)
 
 // parseNamespaceResourceName parses the value and returns namespace, resource and the
 // value (resource name) itself. The valid syntax is:
@@ -20,3 +25,11 @@ func parseNamespaceResourceName(v, defaultNamespace string) (ns, resource, name
 	}
 	return "", "", "", false
 }
+
+func getFlagStringArray(cmd *cobra.Command, flag string) []string {
+	s, err := cmd.Flags().GetStringArray(flag)
+	if err != nil {
+		glog.Fatalf("error accessing flag %s for command %s: %v", flag, cmd.Name(), err)
+	}
+	return s
+}

pkg/cmd/cli/cmd/newapp.go

@@ -166,7 +166,7 @@ func NewCmdNewApplication(name, baseName string, f *clientcmd.Factory, out, erro
 	cmd.Flags().StringSliceVar(&config.Templates, "template", config.Templates, "Name of a stored template to use in the app.")
 	cmd.Flags().StringSliceVarP(&config.TemplateFiles, "file", "f", config.TemplateFiles, "Path to a template file to use for the app.")
 	cmd.MarkFlagFilename("file", "yaml", "yml", "json")
-	cmd.Flags().StringSliceVarP(&config.TemplateParameters, "param", "p", config.TemplateParameters, "Specify a list of key value pairs (e.g., -p FOO=BAR,BAR=FOO) to set/override parameter values in the template.")
+	cmd.Flags().StringArrayVarP(&config.TemplateParameters, "param", "p", config.TemplateParameters, "Specify a key-value pair (e.g., -p FOO=BAR) to set/override parameter value in the template.")
 	cmd.Flags().StringSliceVar(&config.Groups, "group", config.Groups, "Indicate components that should be grouped together as <comp1>+<comp2>.")
 	cmd.Flags().StringSliceVarP(&config.Environment, "env", "e", config.Environment, "Specify key-value pairs of environment variables to set into each container. This doesn't apply to objects created from a template, use parameters instead.")
 	cmd.Flags().StringVar(&config.Name, "name", "", "Set name to use for generated application artifacts")

pkg/cmd/cli/cmd/process.go

@@ -57,20 +57,20 @@ var (
 )
 
 // NewCmdProcess implements the OpenShift cli process command
-func NewCmdProcess(fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command {
+func NewCmdProcess(fullName string, f *clientcmd.Factory, out, errout io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "process (TEMPLATE | -f FILENAME) [-v=KEY=VALUE]",
 		Short:   "Process a template into list of resources",
 		Long:    processLong,
 		Example: fmt.Sprintf(processExample, fullName),
 		Run: func(cmd *cobra.Command, args []string) {
-			err := RunProcess(f, out, cmd, args)
+			err := RunProcess(f, out, errout, cmd, args)
 			kcmdutil.CheckErr(err)
 		},
 	}
 	cmd.Flags().StringP("filename", "f", "", "Filename or URL to file to read a template")
 	cmd.MarkFlagFilename("filename", "yaml", "yml", "json")
-	cmd.Flags().StringSliceP("value", "v", nil, "Specify a list of key-value pairs (eg. -v FOO=BAR,BAR=FOO) to set/override parameter values")
+	cmd.Flags().StringArrayP("value", "v", nil, "Specify a key-value pair (eg. -v FOO=BAR) to set/override parameter value in the template.")
 	cmd.Flags().BoolP("parameters", "", false, "Do not process but only print available parameters")
 	cmd.Flags().StringP("labels", "l", "", "Label to set in all resources for this template")
 
@@ -83,7 +83,7 @@ func NewCmdProcess(fullName string, f *clientcmd.Factory, out io.Writer) *cobra.
 }
 
 // RunProcess contains all the necessary functionality for the OpenShift cli process command
-func RunProcess(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []string) error {
+func RunProcess(f *clientcmd.Factory, out, errout io.Writer, cmd *cobra.Command, args []string) error {
 	templateName, valueArgs := "", []string{}
 	for _, s := range args {
 		isValue := strings.Contains(s, "=")
@@ -102,7 +102,13 @@ func RunProcess(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []
 
 	var flagValues []string
 	if cmd.Flag("value").Changed {
-		flagValues = kcmdutil.GetFlagStringSlice(cmd, "value")
+		flagValues = getFlagStringArray(cmd, "value")
+	}
+
+	for _, value := range flagValues {
+		if strings.ContainsRune(value, ',') {
+			fmt.Fprintf(errout, "warning: --value no longer accepts comma-separated list of values. %q will be treated as a single key-value pair.\n", value)
+		}
 	}
 
 	for _, value := range flagValues {
@@ -238,7 +244,7 @@ func RunProcess(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []
 	// Override the values for the current template parameters
 	// when user specify the --value
 	if cmd.Flag("value").Changed {
-		values := kcmdutil.GetFlagStringSlice(cmd, "value")
+		values := getFlagStringArray(cmd, "value")
 		if errs := injectUserVars(values, obj); errs != nil {
 			return kerrors.NewAggregate(errs)
 		}

pkg/generate/app/cmd/newapp.go

@@ -563,6 +563,12 @@ func (c *AppConfig) RunQuery() (*QueryResult, error) {
 func (c *AppConfig) validate() (cmdutil.Environment, cmdutil.Environment, error) {
 	var errs []error
 
+	for _, value := range c.TemplateParameters {
+		if strings.ContainsRune(value, ',') {
+			fmt.Fprintf(c.ErrOut, "warning: --param no longer accepts comma-separated list of values. %q will be treated as a single key-value pair.\n", value)
+		}
+	}
+
 	env, duplicateEnv, envErrs := cmdutil.ParseEnvironmentArguments(c.Environment)
 	for _, s := range duplicateEnv {
 		fmt.Fprintf(c.ErrOut, "warning: The environment variable %q was overwritten", s)

test/cmd/newapp.sh

@@ -63,6 +63,11 @@ os::cmd::expect_success_and_text 'oc new-app ruby-helloworld-sample -o yaml' 'AD
 os::cmd::expect_success_and_text 'oc new-app ruby-helloworld-sample -o yaml' 'ADMIN_PASSWORD'
 os::cmd::expect_success_and_text 'oc new-app ruby-helloworld-sample --param MYSQL_PASSWORD=hello -o yaml' 'hello'
 
+# check that values are not split on commas
+os::cmd::expect_success_and_text 'oc new-app ruby-helloworld-sample --param MYSQL_PASSWORD=hello,MYSQL_USER=fail -o yaml' 'value: hello,MYSQL_USER=fail'
+# check that warning is printed when --param PARAM1=VAL1,PARAM2=VAL2 is used
+os::cmd::expect_success_and_text 'oc new-app ruby-helloworld-sample --param MYSQL_PASSWORD=hello,MYSQL_USER=fail -o yaml' 'no longer accepts comma-separated list'
+
 # verify we can create from a template when some objects in the template declare an app label
 # the app label will not be applied to any objects in the template.
 os::cmd::expect_success_and_not_text 'oc new-app -f test/testdata/template-with-app-label.json -o yaml' 'app: ruby-helloworld-sample'

test/cmd/process.sh

@@ -55,5 +55,12 @@ os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=req
 # failure on labels fails the entire call
 os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --labels======" 'error parsing labels'
 
+# values are not split on commas, required parameter is not recognized
+os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=optional_param=a,required_param=b" 'parameter required_param is required and must be specified'
+# warning is printed when --value has comma in it
+os::cmd::expect_success_and_text "oc process -f '${required_params}' --value=required_param=a,b,c,d" 'no longer accepts comma-separated list'
+# warning is not printed for template values passed as positional arguments
+os::cmd::expect_success_and_not_text "oc process -f '${required_params}' required_param=a,b,c,d" 'no longer accepts comma-separated list'
+
 echo "process: ok"
 os::test::junit::declare_suite_end

test/cmd/templates.sh

@@ -37,9 +37,6 @@ echo "template+config: ok"
 os::test::junit::declare_suite_end
 
 os::test::junit::declare_suite_start "cmd/templates/parameters"
-# Joined parameter values are honored
-os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json -v ADMIN_USERNAME=myuser,ADMIN_PASSWORD=mypassword'    '"myuser"'
-os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json -v ADMIN_USERNAME=myuser,ADMIN_PASSWORD=mypassword'    '"mypassword"'
 # Individually specified parameter values are honored
 os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json -v ADMIN_USERNAME=myuser -v ADMIN_PASSWORD=mypassword' '"myuser"'
 os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json -v ADMIN_USERNAME=myuser -v ADMIN_PASSWORD=mypassword' '"mypassword"'
@@ -48,8 +45,10 @@ os::cmd::expect_success_and_text 'oc process ADMIN_USERNAME=myuser ADMIN_PASSWOR
 os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json ADMIN_USERNAME=myuser ADMIN_PASSWORD=mypassword'       '"mypassword"'
 # Argument values with commas are honored
 os::cmd::expect_success 'oc create -f examples/sample-app/application-template-stibuild.json'
-os::cmd::expect_success_and_text 'oc process ruby-helloworld-sample MYSQL_USER=myself MYSQL_PASSWORD=my,1%pass'  '"myself"'
-os::cmd::expect_success_and_text 'oc process MYSQL_USER=myself MYSQL_PASSWORD=my,1%pass ruby-helloworld-sample'  '"my,1%pass"'
+os::cmd::expect_success_and_text 'oc process ruby-helloworld-sample MYSQL_USER=myself MYSQL_PASSWORD=my,1%pass'        '"myself"'
+os::cmd::expect_success_and_text 'oc process MYSQL_USER=myself MYSQL_PASSWORD=my,1%pass ruby-helloworld-sample'        '"my,1%pass"'
+os::cmd::expect_success_and_text 'oc process ruby-helloworld-sample -v MYSQL_USER=myself -v MYSQL_PASSWORD=my,1%pass'  '"myself"'
+os::cmd::expect_success_and_text 'oc process -v MYSQL_USER=myself -v MYSQL_PASSWORD=my,1%pass ruby-helloworld-sample'  '"my,1%pass"'
 os::cmd::expect_success 'oc delete template ruby-helloworld-sample'
 echo "template+parameters: ok"
 os::test::junit::declare_suite_end