Workaround spf13/viper stringarray bug (#1700)

* Workaround StringArray bug in Viper

Switching from StringArray to StringSlice to workaround
spf13/viper#398.

* Add failing test for viper bug

Split up validation and command execution so that we can test them
separately

* Test remaining svcat command validation

cmd/svcat/binding/bind_cmd.go

@@ -47,9 +47,8 @@ func NewBindCmd(cxt *command.Context) *cobra.Command {
   svcat bind wordpress
   svcat bind wordpress-mysql-instance --name wordpress-mysql-binding --secret-name wordpress-mysql-secret
 `,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return bindCmd.run(args)
-		},
+		PreRunE: command.PreRunE(bindCmd),
+		RunE:    command.RunE(bindCmd),
 	}
 	cmd.Flags().StringVarP(
 		&bindCmd.ns,
@@ -72,15 +71,15 @@ func NewBindCmd(cxt *command.Context) *cobra.Command {
 		"",
 		"The name of the secret. Defaults to the name of the instance.",
 	)
-	cmd.Flags().StringArrayVarP(&bindCmd.rawParams, "param", "p", nil,
+	cmd.Flags().StringSliceVarP(&bindCmd.rawParams, "param", "p", nil,
 		"Additional parameter to use when binding the instance, format: NAME=VALUE")
-	cmd.Flags().StringArrayVarP(&bindCmd.rawSecrets, "secret", "s", nil,
+	cmd.Flags().StringSliceVarP(&bindCmd.rawSecrets, "secret", "s", nil,
 		"Additional parameter, whose value is stored in a secret, to use when binding the instance, format: SECRET[KEY]")
 
 	return cmd
 }
 
-func (c *bindCmd) run(args []string) error {
+func (c *bindCmd) Validate(args []string) error {
 	if len(args) == 0 {
 		return fmt.Errorf("instance is required")
 	}
@@ -97,6 +96,10 @@ func (c *bindCmd) run(args []string) error {
 		return fmt.Errorf("invalid --secret value (%s)", err)
 	}
 
+	return nil
+}
+
+func (c *bindCmd) Run() error {
 	return c.bind()
 }
 

cmd/svcat/binding/describe_cmd.go

@@ -41,9 +41,8 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
 		Example: `
   svcat describe binding wordpress-mysql-binding
 `,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return describeCmd.run(args)
-		},
+		PreRunE: command.PreRunE(describeCmd),
+		RunE:    command.RunE(describeCmd),
 	}
 	cmd.Flags().StringVarP(
 		&describeCmd.ns,
@@ -62,12 +61,16 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
 	return cmd
 }
 
-func (c *describeCmd) run(args []string) error {
+func (c *describeCmd) Validate(args []string) error {
 	if len(args) == 0 {
 		return fmt.Errorf("name is required")
 	}
 	c.name = args[0]
 
+	return nil
+}
+
+func (c *describeCmd) Run() error {
 	return c.describe()
 }
 

cmd/svcat/binding/get_cmd.go

@@ -30,7 +30,7 @@ type getCmd struct {
 
 // NewGetCmd builds a "svcat get bindings" command
 func NewGetCmd(cxt *command.Context) *cobra.Command {
-	getCmd := getCmd{Context: cxt}
+	getCmd := &getCmd{Context: cxt}
 	cmd := &cobra.Command{
 		Use:     "bindings [name]",
 		Aliases: []string{"binding", "bnd"},
@@ -40,9 +40,8 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
   svcat get binding wordpress-mysql-binding
   svcat get binding -n ci concourse-postgres-binding
 `,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return getCmd.run(args)
-		},
+		PreRunE: command.PreRunE(getCmd),
+		RunE:    command.RunE(getCmd),
 	}
 
 	cmd.Flags().StringVarP(
@@ -55,12 +54,19 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
 	return cmd
 }
 
-func (c *getCmd) run(args []string) error {
-	if len(args) == 0 {
+func (c *getCmd) Validate(args []string) error {
+	if len(args) > 0 {
+		c.name = args[0]
+	}
+
+	return nil
+}
+
+func (c *getCmd) Run() error {
+	if c.name == "" {
 		return c.getAll()
 	}
 
-	c.name = args[0]
 	return c.get()
 }
 

cmd/svcat/binding/unbind_cmd.go

@@ -32,17 +32,16 @@ type unbindCmd struct {
 
 // NewUnbindCmd builds a "svcat unbind" command
 func NewUnbindCmd(cxt *command.Context) *cobra.Command {
-	unbindCmd := unbindCmd{Context: cxt}
+	unbindCmd := &unbindCmd{Context: cxt}
 	cmd := &cobra.Command{
 		Use:   "unbind INSTANCE_NAME",
 		Short: "Unbinds an instance. When an instance name is specified, all of its bindings are removed, otherwise use --name to remove a specific binding",
 		Example: `
   svcat unbind wordpress-mysql-instance
   svcat unbind --name wordpress-mysql-binding
 `,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return unbindCmd.run(args)
-		},
+		PreRunE: command.PreRunE(unbindCmd),
+		RunE:    command.RunE(unbindCmd),
 	}
 
 	cmd.Flags().StringVarP(
@@ -61,15 +60,29 @@ func NewUnbindCmd(cxt *command.Context) *cobra.Command {
 	return cmd
 }
 
-func (c *unbindCmd) run(args []string) error {
+func (c *unbindCmd) Validate(args []string) error {
 	if len(args) == 0 {
 		if c.bindingName == "" {
 			return fmt.Errorf("an instance or binding name is required")
 		}
+	} else {
+		c.instanceName = args[0]
+	}
+
+	return nil
+}
 
-		return c.App.DeleteBinding(c.ns, c.bindingName)
+func (c *unbindCmd) Run() error {
+	if c.instanceName != "" {
+		return c.unbindInstance()
 	}
+	return c.deleteBinding()
+}
+
+func (c *unbindCmd) deleteBinding() error {
+	return c.App.DeleteBinding(c.ns, c.bindingName)
+}
 
-	c.instanceName = args[0]
+func (c *unbindCmd) unbindInstance() error {
 	return c.App.Unbind(c.ns, c.instanceName)
 }

cmd/svcat/broker/describe_cmd.go

@@ -41,23 +41,26 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
 		Example: `
   svcat describe broker asb
 `,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return describeCmd.run(args)
-		},
+		PreRunE: command.PreRunE(describeCmd),
+		RunE:    command.RunE(describeCmd),
 	}
 	return cmd
 }
 
-func (c *describeCmd) run(args []string) error {
+func (c *describeCmd) Validate(args []string) error {
 	if len(args) == 0 {
 		return fmt.Errorf("name is required")
 	}
 	c.name = args[0]
 
-	return c.describe()
+	return nil
+}
+
+func (c *describeCmd) Run() error {
+	return c.Describe()
 }
 
-func (c *describeCmd) describe() error {
+func (c *describeCmd) Describe() error {
 	broker, err := c.App.RetrieveBroker(c.name)
 	if err != nil {
 		return err

cmd/svcat/broker/get_cmd.go

@@ -29,7 +29,7 @@ type getCmd struct {
 
 // NewGetCmd builds a "svcat get brokers" command
 func NewGetCmd(cxt *command.Context) *cobra.Command {
-	getCmd := getCmd{Context: cxt}
+	getCmd := &getCmd{Context: cxt}
 	cmd := &cobra.Command{
 		Use:     "brokers [name]",
 		Aliases: []string{"broker", "brk"},
@@ -38,20 +38,26 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
   svcat get brokers
   svcat get broker asb
 `,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return getCmd.run(args)
-		},
+		PreRunE: command.PreRunE(getCmd),
+		RunE:    command.RunE(getCmd),
 	}
 
 	return cmd
 }
 
-func (c *getCmd) run(args []string) error {
-	if len(args) == 0 {
+func (c *getCmd) Validate(args []string) error {
+	if len(args) > 0 {
+		c.name = args[0]
+	}
+
+	return nil
+}
+
+func (c *getCmd) Run() error {
+	if c.name == "" {
 		return c.getAll()
 	}
 
-	c.name = args[0]
 	return c.get()
 }
 

cmd/svcat/broker/sync_cmd.go

@@ -30,22 +30,25 @@ type syncCmd struct {
 
 // NewSyncCmd builds a "svcat sync broker" command
 func NewSyncCmd(cxt *command.Context) *cobra.Command {
-	syncCmd := syncCmd{Context: cxt}
+	syncCmd := &syncCmd{Context: cxt}
 	rootCmd := &cobra.Command{
-		Use:   "broker [name]",
-		Short: "Syncs service catalog for a service broker",
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return syncCmd.run(args)
-		},
+		Use:     "broker [name]",
+		Short:   "Syncs service catalog for a service broker",
+		PreRunE: command.PreRunE(syncCmd),
+		RunE:    command.RunE(syncCmd),
 	}
 	return rootCmd
 }
 
-func (c *syncCmd) run(args []string) error {
+func (c *syncCmd) Validate(args []string) error {
 	if len(args) != 1 {
 		return fmt.Errorf("name is required")
 	}
 	c.name = args[0]
+	return nil
+}
+
+func (c *syncCmd) Run() error {
 	return c.sync()
 }
 

cmd/svcat/class/describe_cmd.go

@@ -44,9 +44,8 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
   svcat describe class mysqldb
   svcat describe class -uuid 997b8372-8dac-40ac-ae65-758b4a5075a5
 `,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return describeCmd.run(args)
-		},
+		PreRunE: command.PreRunE(describeCmd),
+		RunE:    command.RunE(describeCmd),
 	}
 	cmd.Flags().BoolVarP(
 		&describeCmd.traverse,
@@ -65,7 +64,7 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
 	return cmd
 }
 
-func (c *describeCmd) run(args []string) error {
+func (c *describeCmd) Validate(args []string) error {
 	if len(args) == 0 {
 		return fmt.Errorf("name or uuid is required")
 	}
@@ -76,6 +75,10 @@ func (c *describeCmd) run(args []string) error {
 		c.name = args[0]
 	}
 
+	return nil
+}
+
+func (c *describeCmd) Run() error {
 	return c.describe()
 }
 

cmd/svcat/class/get_cmd.go

@@ -42,9 +42,8 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
   svcat get class mysqldb
   svcat get class --uuid 997b8372-8dac-40ac-ae65-758b4a5075a5
 `,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return getCmd.run(args)
-		},
+		PreRunE: command.PreRunE(getCmd),
+		RunE:    command.RunE(getCmd),
 	}
 	cmd.Flags().BoolVarP(
 		&getCmd.lookupByUUID,
@@ -56,15 +55,21 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
 	return cmd
 }
 
-func (c *getCmd) run(args []string) error {
-	if len(args) == 0 {
-		return c.getAll()
+func (c *getCmd) Validate(args []string) error {
+	if len(args) > 0 {
+		if c.lookupByUUID {
+			c.uuid = args[0]
+		} else {
+			c.name = args[0]
+		}
 	}
 
-	if c.lookupByUUID {
-		c.uuid = args[0]
-	} else {
-		c.name = args[0]
+	return nil
+}
+
+func (c *getCmd) Run() error {
+	if c.uuid == "" && c.name == "" {
+		return c.getAll()
 	}
 
 	return c.get()

cmd/svcat/command/command.go

@@ -0,0 +1,42 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package command
+
+import "github.com/spf13/cobra"
+
+// Command represents an svcat command.
+type Command interface {
+	// Validate and load the arguments passed to the svcat command.
+	Validate(args []string) error
+
+	// Run a validated svcat command.
+	Run() error
+}
+
+// PreRunE validates os args, and then saves them on the svcat command.
+func PreRunE(cmd Command) func(*cobra.Command, []string) error {
+	return func(_ *cobra.Command, args []string) error {
+		return cmd.Validate(args)
+	}
+}
+
+// RunE executes a validated svcat command.
+func RunE(cmd Command) func(*cobra.Command, []string) error {
+	return func(_ *cobra.Command, args []string) error {
+		return cmd.Run()
+	}
+}