From c60e47dedd4dc71bad1434e8731bda46dc46f35e Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Tue, 3 Apr 2018 09:34:19 -0700 Subject: [PATCH 1/2] use cobra's arg-count validation & call Complete() before Validate() Signed-off-by: Steve Kriss --- pkg/cmd/cli/backup/create.go | 8 ++------ pkg/cmd/cli/backup/delete.go | 7 ++----- pkg/cmd/cli/backup/download.go | 7 ++----- pkg/cmd/cli/backup/logs.go | 7 +------ pkg/cmd/cli/client/config/set.go | 6 +----- pkg/cmd/cli/plugin/add.go | 5 +---- pkg/cmd/cli/plugin/remove.go | 5 +---- pkg/cmd/cli/restore/create.go | 7 ++----- pkg/cmd/cli/restore/delete.go | 7 +------ pkg/cmd/cli/restore/logs.go | 7 +------ pkg/cmd/cli/schedule/create.go | 7 ++----- pkg/cmd/cli/schedule/delete.go | 7 +------ pkg/cmd/server/plugin/plugin.go | 5 +---- 13 files changed, 18 insertions(+), 67 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 60d10c270..2d7750361 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -20,7 +20,6 @@ import ( "fmt" "time" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -39,9 +38,10 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { c := &cobra.Command{ Use: use + " NAME", Short: "Create a backup", + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - cmd.CheckError(o.Validate(c, args)) cmd.CheckError(o.Complete(args)) + cmd.CheckError(o.Validate(c, args)) cmd.CheckError(o.Run(c, f)) }, } @@ -94,10 +94,6 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { } func (o *CreateOptions) Validate(c *cobra.Command, args []string) error { - if len(args) != 1 { - return errors.New("you must specify only one argument, the backup's name") - } - if err := output.ValidateFlags(c); err != nil { return err } diff --git a/pkg/cmd/cli/backup/delete.go b/pkg/cmd/cli/backup/delete.go index d9b432092..866d2ee58 100644 --- a/pkg/cmd/cli/backup/delete.go +++ b/pkg/cmd/cli/backup/delete.go @@ -45,9 +45,10 @@ func NewDeleteCommand(f client.Factory, use string) *cobra.Command { c := &cobra.Command{ Use: fmt.Sprintf("%s NAME", use), Short: "Delete a backup", + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - cmd.CheckError(o.Validate(c, args, f)) cmd.CheckError(o.Complete(f, args)) + cmd.CheckError(o.Validate(c, args, f)) cmd.CheckError(o.Run()) }, } @@ -72,10 +73,6 @@ func (o *DeleteOptions) BindFlags(flags *pflag.FlagSet) { } func (o *DeleteOptions) Validate(c *cobra.Command, args []string, f client.Factory) error { - if len(args) != 1 { - return errors.New("you must specify only one argument, the backup's name") - } - kubeClient, err := f.KubeClient() if err != nil { return err diff --git a/pkg/cmd/cli/backup/download.go b/pkg/cmd/cli/backup/download.go index d353c97af..33c9ac079 100644 --- a/pkg/cmd/cli/backup/download.go +++ b/pkg/cmd/cli/backup/download.go @@ -37,9 +37,10 @@ func NewDownloadCommand(f client.Factory) *cobra.Command { c := &cobra.Command{ Use: "download NAME", Short: "Download a backup", + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - cmd.CheckError(o.Validate(c, args)) cmd.CheckError(o.Complete(args)) + cmd.CheckError(o.Validate(c, args)) cmd.CheckError(o.Run(c, f)) }, } @@ -70,10 +71,6 @@ func (o *DownloadOptions) BindFlags(flags *pflag.FlagSet) { } func (o *DownloadOptions) Validate(c *cobra.Command, args []string) error { - if len(args) != 1 { - return errors.New("backup name is required") - } - return nil } diff --git a/pkg/cmd/cli/backup/logs.go b/pkg/cmd/cli/backup/logs.go index 880fbc5ed..fd43427b0 100644 --- a/pkg/cmd/cli/backup/logs.go +++ b/pkg/cmd/cli/backup/logs.go @@ -20,7 +20,6 @@ import ( "os" "time" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/heptio/ark/pkg/apis/ark/v1" @@ -35,12 +34,8 @@ func NewLogsCommand(f client.Factory) *cobra.Command { c := &cobra.Command{ Use: "logs BACKUP", Short: "Get backup logs", + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - if len(args) != 1 { - err := errors.New("backup name is required") - cmd.CheckError(err) - } - arkClient, err := f.Client() cmd.CheckError(err) diff --git a/pkg/cmd/cli/client/config/set.go b/pkg/cmd/cli/client/config/set.go index a95bbeba3..dcfe6c2cf 100644 --- a/pkg/cmd/cli/client/config/set.go +++ b/pkg/cmd/cli/client/config/set.go @@ -23,7 +23,6 @@ import ( "github.com/heptio/ark/pkg/client" "github.com/heptio/ark/pkg/cmd" - "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -31,11 +30,8 @@ func NewSetCommand() *cobra.Command { c := &cobra.Command{ Use: "set KEY=VALUE [KEY=VALUE]...", Short: "Set client configuration file values", + Args: cobra.MinimumNArgs(1), Run: func(c *cobra.Command, args []string) { - if len(args) < 1 { - cmd.CheckError(errors.Errorf("At least one KEY=VALUE argument is required")) - } - config, err := client.LoadConfig() cmd.CheckError(err) diff --git a/pkg/cmd/cli/plugin/add.go b/pkg/cmd/cli/plugin/add.go index b5c9c6a6a..c6dfbf3a3 100644 --- a/pkg/cmd/cli/plugin/add.go +++ b/pkg/cmd/cli/plugin/add.go @@ -50,11 +50,8 @@ func NewAddCommand(f client.Factory) *cobra.Command { c := &cobra.Command{ Use: "add IMAGE", Short: "Add a plugin", + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - if len(args) != 1 { - cmd.CheckError(errors.New("you must specify only one argument, the plugin container image")) - } - kubeClient, err := f.KubeClient() if err != nil { cmd.CheckError(err) diff --git a/pkg/cmd/cli/plugin/remove.go b/pkg/cmd/cli/plugin/remove.go index 3f3c3f15b..0ca07db0c 100644 --- a/pkg/cmd/cli/plugin/remove.go +++ b/pkg/cmd/cli/plugin/remove.go @@ -35,11 +35,8 @@ func NewRemoveCommand(f client.Factory) *cobra.Command { c := &cobra.Command{ Use: "remove [NAME | IMAGE]", Short: "Remove a plugin", + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - if len(args) != 1 { - cmd.CheckError(errors.New("you must specify only one argument, the plugin container's name or image")) - } - kubeClient, err := f.KubeClient() if err != nil { cmd.CheckError(err) diff --git a/pkg/cmd/cli/restore/create.go b/pkg/cmd/cli/restore/create.go index e5597bfef..cd0e98562 100644 --- a/pkg/cmd/cli/restore/create.go +++ b/pkg/cmd/cli/restore/create.go @@ -45,9 +45,10 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { # create a restore with a default name ("backup-1-") from backup "backup-1" ark restore create --from-backup backup-1`, + Args: cobra.MaximumNArgs(1), Run: func(c *cobra.Command, args []string) { - cmd.CheckError(o.Validate(c, args, f)) cmd.CheckError(o.Complete(args)) + cmd.CheckError(o.Validate(c, args, f)) cmd.CheckError(o.Run(c, f)) }, } @@ -108,10 +109,6 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return errors.New("--from-backup is required") } - if len(args) > 1 { - return errors.New("you may specify at most one argument, the restore's name") - } - if err := output.ValidateFlags(c); err != nil { return err } diff --git a/pkg/cmd/cli/restore/delete.go b/pkg/cmd/cli/restore/delete.go index a390385ab..d9574ea24 100644 --- a/pkg/cmd/cli/restore/delete.go +++ b/pkg/cmd/cli/restore/delete.go @@ -18,7 +18,6 @@ package restore import ( "fmt" - "os" "github.com/spf13/cobra" @@ -30,12 +29,8 @@ func NewDeleteCommand(f client.Factory, use string) *cobra.Command { c := &cobra.Command{ Use: fmt.Sprintf("%s NAME", use), Short: "Delete a restore", + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - if len(args) != 1 { - c.Usage() - os.Exit(1) - } - arkClient, err := f.Client() cmd.CheckError(err) diff --git a/pkg/cmd/cli/restore/logs.go b/pkg/cmd/cli/restore/logs.go index ec65112cd..2a0eb352a 100644 --- a/pkg/cmd/cli/restore/logs.go +++ b/pkg/cmd/cli/restore/logs.go @@ -20,7 +20,6 @@ import ( "os" "time" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/heptio/ark/pkg/apis/ark/v1" @@ -35,12 +34,8 @@ func NewLogsCommand(f client.Factory) *cobra.Command { c := &cobra.Command{ Use: "logs RESTORE", Short: "Get restore logs", + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - if len(args) != 1 { - err := errors.New("restore name is required") - cmd.CheckError(err) - } - arkClient, err := f.Client() cmd.CheckError(err) diff --git a/pkg/cmd/cli/schedule/create.go b/pkg/cmd/cli/schedule/create.go index 9d915a391..a49c86be9 100644 --- a/pkg/cmd/cli/schedule/create.go +++ b/pkg/cmd/cli/schedule/create.go @@ -49,10 +49,10 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { | 5 | Day of Week | 0-7,* |`, Example: `ark create schedule NAME --schedule="0 */6 * * *"`, - + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - cmd.CheckError(o.Validate(c, args)) cmd.CheckError(o.Complete(args)) + cmd.CheckError(o.Validate(c, args)) cmd.CheckError(o.Run(c, f)) }, } @@ -83,9 +83,6 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { } func (o *CreateOptions) Validate(c *cobra.Command, args []string) error { - if len(args) != 1 { - return errors.New("you must specify only one argument, the schedule's name") - } if len(o.Schedule) == 0 { return errors.New("--schedule is required") } diff --git a/pkg/cmd/cli/schedule/delete.go b/pkg/cmd/cli/schedule/delete.go index de27a864d..fe2f76f64 100644 --- a/pkg/cmd/cli/schedule/delete.go +++ b/pkg/cmd/cli/schedule/delete.go @@ -18,7 +18,6 @@ package schedule import ( "fmt" - "os" "github.com/spf13/cobra" @@ -30,12 +29,8 @@ func NewDeleteCommand(f client.Factory, use string) *cobra.Command { c := &cobra.Command{ Use: fmt.Sprintf("%s NAME", use), Short: "Delete a schedule", + Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - if len(args) != 1 { - c.Usage() - os.Exit(1) - } - arkClient, err := f.Client() cmd.CheckError(err) diff --git a/pkg/cmd/server/plugin/plugin.go b/pkg/cmd/server/plugin/plugin.go index 8cd8f1878..ec575bdd8 100644 --- a/pkg/cmd/server/plugin/plugin.go +++ b/pkg/cmd/server/plugin/plugin.go @@ -60,11 +60,8 @@ func NewCommand() *cobra.Command { Use: "run-plugin [KIND] [NAME]", Hidden: true, Short: "INTERNAL COMMAND ONLY - not intended to be run directly by users", + Args: cobra.ExactArgs(2), Run: func(c *cobra.Command, args []string) { - if len(args) != 2 { - logger.Fatal("You must specify exactly two arguments, the plugin kind and the plugin name") - } - kind := args[0] name := args[1] From 80b66434c0f4166ce7d34c97b07565925f7b2c08 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Tue, 3 Apr 2018 20:43:42 -0700 Subject: [PATCH 2/2] move getting client into Complete() Signed-off-by: Steve Kriss --- pkg/cmd/cli/restore/create.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/cli/restore/create.go b/pkg/cmd/cli/restore/create.go index cd0e98562..503f7eaac 100644 --- a/pkg/cmd/cli/restore/create.go +++ b/pkg/cmd/cli/restore/create.go @@ -47,7 +47,7 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { ark restore create --from-backup backup-1`, Args: cobra.MaximumNArgs(1), Run: func(c *cobra.Command, args []string) { - cmd.CheckError(o.Complete(args)) + cmd.CheckError(o.Complete(args, f)) cmd.CheckError(o.Validate(c, args, f)) cmd.CheckError(o.Run(c, f)) }, @@ -113,27 +113,31 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return err } - client, err := f.Client() - if err != nil { - return err + if o.client == nil { + // This should never happen + return errors.New("Ark client is not set; unable to proceed") } - o.client = client - _, err = o.client.ArkV1().Backups(f.Namespace()).Get(o.BackupName, metav1.GetOptions{}) - if err != nil { + if _, err := o.client.ArkV1().Backups(f.Namespace()).Get(o.BackupName, metav1.GetOptions{}); err != nil { return err } return nil } -func (o *CreateOptions) Complete(args []string) error { +func (o *CreateOptions) Complete(args []string, f client.Factory) error { if len(args) == 1 { o.RestoreName = args[0] } else { o.RestoreName = fmt.Sprintf("%s-%s", o.BackupName, time.Now().Format("20060102150405")) } + client, err := f.Client() + if err != nil { + return err + } + o.client = client + return nil }