Skip to content

Commit fcdefa6

Browse files
carolynvsJeff Peeler
authored and
Jeff Peeler
committed
Workaround spf13/viper stringarray bug (openshift#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
1 parent ebbeb8c commit fcdefa6

17 files changed

+294
-98
lines changed

cmd/svcat/binding/bind_cmd.go

+9-6
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,8 @@ func NewBindCmd(cxt *command.Context) *cobra.Command {
4747
svcat bind wordpress
4848
svcat bind wordpress-mysql-instance --name wordpress-mysql-binding --secret-name wordpress-mysql-secret
4949
`,
50-
RunE: func(cmd *cobra.Command, args []string) error {
51-
return bindCmd.run(args)
52-
},
50+
PreRunE: command.PreRunE(bindCmd),
51+
RunE: command.RunE(bindCmd),
5352
}
5453
cmd.Flags().StringVarP(
5554
&bindCmd.ns,
@@ -72,15 +71,15 @@ func NewBindCmd(cxt *command.Context) *cobra.Command {
7271
"",
7372
"The name of the secret. Defaults to the name of the instance.",
7473
)
75-
cmd.Flags().StringArrayVarP(&bindCmd.rawParams, "param", "p", nil,
74+
cmd.Flags().StringSliceVarP(&bindCmd.rawParams, "param", "p", nil,
7675
"Additional parameter to use when binding the instance, format: NAME=VALUE")
77-
cmd.Flags().StringArrayVarP(&bindCmd.rawSecrets, "secret", "s", nil,
76+
cmd.Flags().StringSliceVarP(&bindCmd.rawSecrets, "secret", "s", nil,
7877
"Additional parameter, whose value is stored in a secret, to use when binding the instance, format: SECRET[KEY]")
7978

8079
return cmd
8180
}
8281

83-
func (c *bindCmd) run(args []string) error {
82+
func (c *bindCmd) Validate(args []string) error {
8483
if len(args) == 0 {
8584
return fmt.Errorf("instance is required")
8685
}
@@ -97,6 +96,10 @@ func (c *bindCmd) run(args []string) error {
9796
return fmt.Errorf("invalid --secret value (%s)", err)
9897
}
9998

99+
return nil
100+
}
101+
102+
func (c *bindCmd) Run() error {
100103
return c.bind()
101104
}
102105

cmd/svcat/binding/describe_cmd.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,8 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
4141
Example: `
4242
svcat describe binding wordpress-mysql-binding
4343
`,
44-
RunE: func(cmd *cobra.Command, args []string) error {
45-
return describeCmd.run(args)
46-
},
44+
PreRunE: command.PreRunE(describeCmd),
45+
RunE: command.RunE(describeCmd),
4746
}
4847
cmd.Flags().StringVarP(
4948
&describeCmd.ns,
@@ -62,12 +61,16 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
6261
return cmd
6362
}
6463

65-
func (c *describeCmd) run(args []string) error {
64+
func (c *describeCmd) Validate(args []string) error {
6665
if len(args) == 0 {
6766
return fmt.Errorf("name is required")
6867
}
6968
c.name = args[0]
7069

70+
return nil
71+
}
72+
73+
func (c *describeCmd) Run() error {
7174
return c.describe()
7275
}
7376

cmd/svcat/binding/get_cmd.go

+13-7
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type getCmd struct {
3030

3131
// NewGetCmd builds a "svcat get bindings" command
3232
func NewGetCmd(cxt *command.Context) *cobra.Command {
33-
getCmd := getCmd{Context: cxt}
33+
getCmd := &getCmd{Context: cxt}
3434
cmd := &cobra.Command{
3535
Use: "bindings [name]",
3636
Aliases: []string{"binding", "bnd"},
@@ -40,9 +40,8 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
4040
svcat get binding wordpress-mysql-binding
4141
svcat get binding -n ci concourse-postgres-binding
4242
`,
43-
RunE: func(cmd *cobra.Command, args []string) error {
44-
return getCmd.run(args)
45-
},
43+
PreRunE: command.PreRunE(getCmd),
44+
RunE: command.RunE(getCmd),
4645
}
4746

4847
cmd.Flags().StringVarP(
@@ -55,12 +54,19 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
5554
return cmd
5655
}
5756

58-
func (c *getCmd) run(args []string) error {
59-
if len(args) == 0 {
57+
func (c *getCmd) Validate(args []string) error {
58+
if len(args) > 0 {
59+
c.name = args[0]
60+
}
61+
62+
return nil
63+
}
64+
65+
func (c *getCmd) Run() error {
66+
if c.name == "" {
6067
return c.getAll()
6168
}
6269

63-
c.name = args[0]
6470
return c.get()
6571
}
6672

cmd/svcat/binding/unbind_cmd.go

+20-7
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,16 @@ type unbindCmd struct {
3232

3333
// NewUnbindCmd builds a "svcat unbind" command
3434
func NewUnbindCmd(cxt *command.Context) *cobra.Command {
35-
unbindCmd := unbindCmd{Context: cxt}
35+
unbindCmd := &unbindCmd{Context: cxt}
3636
cmd := &cobra.Command{
3737
Use: "unbind INSTANCE_NAME",
3838
Short: "Unbinds an instance. When an instance name is specified, all of its bindings are removed, otherwise use --name to remove a specific binding",
3939
Example: `
4040
svcat unbind wordpress-mysql-instance
4141
svcat unbind --name wordpress-mysql-binding
4242
`,
43-
RunE: func(cmd *cobra.Command, args []string) error {
44-
return unbindCmd.run(args)
45-
},
43+
PreRunE: command.PreRunE(unbindCmd),
44+
RunE: command.RunE(unbindCmd),
4645
}
4746

4847
cmd.Flags().StringVarP(
@@ -61,15 +60,29 @@ func NewUnbindCmd(cxt *command.Context) *cobra.Command {
6160
return cmd
6261
}
6362

64-
func (c *unbindCmd) run(args []string) error {
63+
func (c *unbindCmd) Validate(args []string) error {
6564
if len(args) == 0 {
6665
if c.bindingName == "" {
6766
return fmt.Errorf("an instance or binding name is required")
6867
}
68+
} else {
69+
c.instanceName = args[0]
70+
}
71+
72+
return nil
73+
}
6974

70-
return c.App.DeleteBinding(c.ns, c.bindingName)
75+
func (c *unbindCmd) Run() error {
76+
if c.instanceName != "" {
77+
return c.unbindInstance()
7178
}
79+
return c.deleteBinding()
80+
}
81+
82+
func (c *unbindCmd) deleteBinding() error {
83+
return c.App.DeleteBinding(c.ns, c.bindingName)
84+
}
7285

73-
c.instanceName = args[0]
86+
func (c *unbindCmd) unbindInstance() error {
7487
return c.App.Unbind(c.ns, c.instanceName)
7588
}

cmd/svcat/broker/describe_cmd.go

+9-6
Original file line numberDiff line numberDiff line change
@@ -41,23 +41,26 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
4141
Example: `
4242
svcat describe broker asb
4343
`,
44-
RunE: func(cmd *cobra.Command, args []string) error {
45-
return describeCmd.run(args)
46-
},
44+
PreRunE: command.PreRunE(describeCmd),
45+
RunE: command.RunE(describeCmd),
4746
}
4847
return cmd
4948
}
5049

51-
func (c *describeCmd) run(args []string) error {
50+
func (c *describeCmd) Validate(args []string) error {
5251
if len(args) == 0 {
5352
return fmt.Errorf("name is required")
5453
}
5554
c.name = args[0]
5655

57-
return c.describe()
56+
return nil
57+
}
58+
59+
func (c *describeCmd) Run() error {
60+
return c.Describe()
5861
}
5962

60-
func (c *describeCmd) describe() error {
63+
func (c *describeCmd) Describe() error {
6164
broker, err := c.App.RetrieveBroker(c.name)
6265
if err != nil {
6366
return err

cmd/svcat/broker/get_cmd.go

+13-7
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type getCmd struct {
2929

3030
// NewGetCmd builds a "svcat get brokers" command
3131
func NewGetCmd(cxt *command.Context) *cobra.Command {
32-
getCmd := getCmd{Context: cxt}
32+
getCmd := &getCmd{Context: cxt}
3333
cmd := &cobra.Command{
3434
Use: "brokers [name]",
3535
Aliases: []string{"broker", "brk"},
@@ -38,20 +38,26 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
3838
svcat get brokers
3939
svcat get broker asb
4040
`,
41-
RunE: func(cmd *cobra.Command, args []string) error {
42-
return getCmd.run(args)
43-
},
41+
PreRunE: command.PreRunE(getCmd),
42+
RunE: command.RunE(getCmd),
4443
}
4544

4645
return cmd
4746
}
4847

49-
func (c *getCmd) run(args []string) error {
50-
if len(args) == 0 {
48+
func (c *getCmd) Validate(args []string) error {
49+
if len(args) > 0 {
50+
c.name = args[0]
51+
}
52+
53+
return nil
54+
}
55+
56+
func (c *getCmd) Run() error {
57+
if c.name == "" {
5158
return c.getAll()
5259
}
5360

54-
c.name = args[0]
5561
return c.get()
5662
}
5763

cmd/svcat/broker/sync_cmd.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,25 @@ type syncCmd struct {
3030

3131
// NewSyncCmd builds a "svcat sync broker" command
3232
func NewSyncCmd(cxt *command.Context) *cobra.Command {
33-
syncCmd := syncCmd{Context: cxt}
33+
syncCmd := &syncCmd{Context: cxt}
3434
rootCmd := &cobra.Command{
35-
Use: "broker [name]",
36-
Short: "Syncs service catalog for a service broker",
37-
RunE: func(cmd *cobra.Command, args []string) error {
38-
return syncCmd.run(args)
39-
},
35+
Use: "broker [name]",
36+
Short: "Syncs service catalog for a service broker",
37+
PreRunE: command.PreRunE(syncCmd),
38+
RunE: command.RunE(syncCmd),
4039
}
4140
return rootCmd
4241
}
4342

44-
func (c *syncCmd) run(args []string) error {
43+
func (c *syncCmd) Validate(args []string) error {
4544
if len(args) != 1 {
4645
return fmt.Errorf("name is required")
4746
}
4847
c.name = args[0]
48+
return nil
49+
}
50+
51+
func (c *syncCmd) Run() error {
4952
return c.sync()
5053
}
5154

cmd/svcat/class/describe_cmd.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,8 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
4444
svcat describe class mysqldb
4545
svcat describe class -uuid 997b8372-8dac-40ac-ae65-758b4a5075a5
4646
`,
47-
RunE: func(cmd *cobra.Command, args []string) error {
48-
return describeCmd.run(args)
49-
},
47+
PreRunE: command.PreRunE(describeCmd),
48+
RunE: command.RunE(describeCmd),
5049
}
5150
cmd.Flags().BoolVarP(
5251
&describeCmd.traverse,
@@ -65,7 +64,7 @@ func NewDescribeCmd(cxt *command.Context) *cobra.Command {
6564
return cmd
6665
}
6766

68-
func (c *describeCmd) run(args []string) error {
67+
func (c *describeCmd) Validate(args []string) error {
6968
if len(args) == 0 {
7069
return fmt.Errorf("name or uuid is required")
7170
}
@@ -76,6 +75,10 @@ func (c *describeCmd) run(args []string) error {
7675
c.name = args[0]
7776
}
7877

78+
return nil
79+
}
80+
81+
func (c *describeCmd) Run() error {
7982
return c.describe()
8083
}
8184

cmd/svcat/class/get_cmd.go

+15-10
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,8 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
4242
svcat get class mysqldb
4343
svcat get class --uuid 997b8372-8dac-40ac-ae65-758b4a5075a5
4444
`,
45-
RunE: func(cmd *cobra.Command, args []string) error {
46-
return getCmd.run(args)
47-
},
45+
PreRunE: command.PreRunE(getCmd),
46+
RunE: command.RunE(getCmd),
4847
}
4948
cmd.Flags().BoolVarP(
5049
&getCmd.lookupByUUID,
@@ -56,15 +55,21 @@ func NewGetCmd(cxt *command.Context) *cobra.Command {
5655
return cmd
5756
}
5857

59-
func (c *getCmd) run(args []string) error {
60-
if len(args) == 0 {
61-
return c.getAll()
58+
func (c *getCmd) Validate(args []string) error {
59+
if len(args) > 0 {
60+
if c.lookupByUUID {
61+
c.uuid = args[0]
62+
} else {
63+
c.name = args[0]
64+
}
6265
}
6366

64-
if c.lookupByUUID {
65-
c.uuid = args[0]
66-
} else {
67-
c.name = args[0]
67+
return nil
68+
}
69+
70+
func (c *getCmd) Run() error {
71+
if c.uuid == "" && c.name == "" {
72+
return c.getAll()
6873
}
6974

7075
return c.get()

cmd/svcat/command/command.go

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
Copyright 2018 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package command
18+
19+
import "github.com/spf13/cobra"
20+
21+
// Command represents an svcat command.
22+
type Command interface {
23+
// Validate and load the arguments passed to the svcat command.
24+
Validate(args []string) error
25+
26+
// Run a validated svcat command.
27+
Run() error
28+
}
29+
30+
// PreRunE validates os args, and then saves them on the svcat command.
31+
func PreRunE(cmd Command) func(*cobra.Command, []string) error {
32+
return func(_ *cobra.Command, args []string) error {
33+
return cmd.Validate(args)
34+
}
35+
}
36+
37+
// RunE executes a validated svcat command.
38+
func RunE(cmd Command) func(*cobra.Command, []string) error {
39+
return func(_ *cobra.Command, args []string) error {
40+
return cmd.Run()
41+
}
42+
}

0 commit comments

Comments
 (0)