From 328250e7bef56b0b69dbea124c1170abd289900f Mon Sep 17 00:00:00 2001 From: baude Date: Wed, 20 Feb 2019 13:46:41 -0600 Subject: Improve command line validation Use the checkallandlatest function to validate flag usage as part of the cobra command args validation. Signed-off-by: baude --- cmd/podman/checkpoint.go | 8 +++----- cmd/podman/cleanup.go | 7 +++---- cmd/podman/common.go | 23 ++++++++++++++++------- cmd/podman/kill.go | 7 +++---- cmd/podman/mount.go | 3 +++ cmd/podman/pod_inspect.go | 3 --- cmd/podman/pod_kill.go | 7 +++---- cmd/podman/pod_pause.go | 7 +++---- cmd/podman/pod_restart.go | 7 +++---- cmd/podman/pod_rm.go | 6 +++--- cmd/podman/pod_start.go | 7 +++---- cmd/podman/pod_stop.go | 7 +++---- cmd/podman/pod_unpause.go | 7 +++---- cmd/podman/port.go | 3 +++ cmd/podman/restart.go | 3 +++ cmd/podman/restore.go | 7 +++---- cmd/podman/rm.go | 7 +++---- cmd/podman/save.go | 21 ++++++++++++++++++--- cmd/podman/stats.go | 3 +++ cmd/podman/stop.go | 7 +++---- cmd/podman/umount.go | 6 +++--- cmd/podman/utils.go | 7 ------- 22 files changed, 88 insertions(+), 75 deletions(-) diff --git a/cmd/podman/checkpoint.go b/cmd/podman/checkpoint.go index 3484e8957..5b043ed81 100644 --- a/cmd/podman/checkpoint.go +++ b/cmd/podman/checkpoint.go @@ -29,6 +29,9 @@ var ( checkpointCommand.GlobalFlags = MainGlobalOpts return checkpointCmd(&checkpointCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman checkpoint --keep ctrID podman checkpoint --all podman checkpoint --leave-running --latest`, @@ -63,11 +66,6 @@ func checkpointCmd(c *cliconfig.CheckpointValues) error { KeepRunning: c.LeaveRunning, TCPEstablished: c.TcpEstablished, } - - if err := checkAllAndLatest(&c.PodmanCommand); err != nil { - return err - } - containers, lastError := getAllOrLatestContainers(&c.PodmanCommand, runtime, libpod.ContainerStateRunning, "running") for _, ctr := range containers { diff --git a/cmd/podman/cleanup.go b/cmd/podman/cleanup.go index 89a4ba050..58ac80624 100644 --- a/cmd/podman/cleanup.go +++ b/cmd/podman/cleanup.go @@ -26,6 +26,9 @@ var ( cleanupCommand.GlobalFlags = MainGlobalOpts return cleanupCmd(&cleanupCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman container cleanup --latest podman container cleanup ctrID1 ctrID2 ctrID3 podman container cleanup --all`, @@ -49,10 +52,6 @@ func cleanupCmd(c *cliconfig.CleanupValues) error { } defer runtime.Shutdown(false) - if err := checkAllAndLatest(&c.PodmanCommand); err != nil { - return err - } - cleanupContainers, lastError := getAllOrLatestContainers(&c.PodmanCommand, runtime, -1, "all") ctx := getContext() diff --git a/cmd/podman/common.go b/cmd/podman/common.go index fed07de7c..e297f3921 100644 --- a/cmd/podman/common.go +++ b/cmd/podman/common.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "github.com/spf13/cobra" "os" "strings" @@ -36,16 +37,24 @@ func shortID(id string) string { } // checkAllAndLatest checks that --all and --latest are used correctly -func checkAllAndLatest(c *cliconfig.PodmanCommand) error { - argLen := len(c.InputArgs) - if (c.Bool("all") || c.Bool("latest")) && argLen > 0 { - return errors.Errorf("no arguments are needed with --all or --latest") +func checkAllAndLatest(c *cobra.Command, args []string, ignoreArgLen bool) error { + argLen := len(args) + if c.Flags().Lookup("all") == nil || c.Flags().Lookup("latest") == nil { + return errors.New("unable to lookup values for 'latest' or 'all'") } - if c.Bool("all") && c.Bool("latest") { + all, _ := c.Flags().GetBool("all") + latest, _ := c.Flags().GetBool("latest") + if all && latest { return errors.Errorf("--all and --latest cannot be used together") } - if argLen < 1 && !c.Bool("all") && !c.Bool("latest") { - return errors.Errorf("you must provide at least one pod name or id") + if ignoreArgLen { + return nil + } + if (all || latest) && argLen > 0 { + return errors.Errorf("no arguments are needed with --all or --latest") + } + if argLen < 1 && !all && !latest { + return errors.Errorf("you must provide at least one name or id") } return nil } diff --git a/cmd/podman/kill.go b/cmd/podman/kill.go index 1be4fa959..373938d97 100644 --- a/cmd/podman/kill.go +++ b/cmd/podman/kill.go @@ -28,6 +28,9 @@ var ( killCommand.GlobalFlags = MainGlobalOpts return killCmd(&killCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman kill mywebserver podman kill 860a4b23 podman kill --signal TERM ctrID`, @@ -52,10 +55,6 @@ func killCmd(c *cliconfig.KillValues) error { killSignal uint = uint(syscall.SIGTERM) ) - if err := checkAllAndLatest(&c.PodmanCommand); err != nil { - return err - } - rootless.SetSkipStorageSetup(true) runtime, err := libpodruntime.GetRuntime(&c.PodmanCommand) if err != nil { diff --git a/cmd/podman/mount.go b/cmd/podman/mount.go index ce7c22d60..8d7ab9907 100644 --- a/cmd/podman/mount.go +++ b/cmd/podman/mount.go @@ -34,6 +34,9 @@ var ( mountCommand.GlobalFlags = MainGlobalOpts return mountCmd(&mountCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, true) + }, } ) diff --git a/cmd/podman/pod_inspect.go b/cmd/podman/pod_inspect.go index 58b15328e..e06a7dce9 100644 --- a/cmd/podman/pod_inspect.go +++ b/cmd/podman/pod_inspect.go @@ -39,9 +39,6 @@ func podInspectCmd(c *cliconfig.PodInspectValues) error { var ( pod *libpod.Pod ) - if err := checkMutuallyExclusiveFlags(&c.PodmanCommand); err != nil { - return err - } args := c.InputArgs runtime, err := libpodruntime.GetRuntime(&c.PodmanCommand) if err != nil { diff --git a/cmd/podman/pod_kill.go b/cmd/podman/pod_kill.go index f6991a1c3..7e904f764 100644 --- a/cmd/podman/pod_kill.go +++ b/cmd/podman/pod_kill.go @@ -24,6 +24,9 @@ var ( podKillCommand.GlobalFlags = MainGlobalOpts return podKillCmd(&podKillCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman pod kill podID podman pod kill --signal TERM mywebserver podman pod kill --latest`, @@ -41,10 +44,6 @@ func init() { // podKillCmd kills one or more pods with a signal func podKillCmd(c *cliconfig.PodKillValues) error { - if err := checkMutuallyExclusiveFlags(&c.PodmanCommand); err != nil { - return err - } - runtime, err := libpodruntime.GetRuntime(&c.PodmanCommand) if err != nil { return errors.Wrapf(err, "could not get runtime") diff --git a/cmd/podman/pod_pause.go b/cmd/podman/pod_pause.go index ea2c80f7b..0717e6cff 100644 --- a/cmd/podman/pod_pause.go +++ b/cmd/podman/pod_pause.go @@ -21,6 +21,9 @@ var ( podPauseCommand.GlobalFlags = MainGlobalOpts return podPauseCmd(&podPauseCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman pod pause podID1 podID2 podman pod pause --latest podman pod pause --all`, @@ -36,10 +39,6 @@ func init() { } func podPauseCmd(c *cliconfig.PodPauseValues) error { - if err := checkMutuallyExclusiveFlags(&c.PodmanCommand); err != nil { - return err - } - runtime, err := libpodruntime.GetRuntime(&c.PodmanCommand) if err != nil { return errors.Wrapf(err, "error creating libpod runtime") diff --git a/cmd/podman/pod_restart.go b/cmd/podman/pod_restart.go index 2815af0c6..fe24f26ba 100644 --- a/cmd/podman/pod_restart.go +++ b/cmd/podman/pod_restart.go @@ -22,6 +22,9 @@ var ( podRestartCommand.GlobalFlags = MainGlobalOpts return podRestartCmd(&podRestartCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman pod restart podID1 podID2 podman pod restart --latest podman pod restart --all`, @@ -38,10 +41,6 @@ func init() { } func podRestartCmd(c *cliconfig.PodRestartValues) error { - if err := checkMutuallyExclusiveFlags(&c.PodmanCommand); err != nil { - return err - } - runtime, err := libpodruntime.GetRuntime(&c.PodmanCommand) if err != nil { return errors.Wrapf(err, "could not get runtime") diff --git a/cmd/podman/pod_rm.go b/cmd/podman/pod_rm.go index b615f88c9..6bb4b3d11 100644 --- a/cmd/podman/pod_rm.go +++ b/cmd/podman/pod_rm.go @@ -26,6 +26,9 @@ If --force is specified, all containers will be stopped, then removed. podRmCommand.GlobalFlags = MainGlobalOpts return podRmCmd(&podRmCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman pod rm mywebserverpod podman pod rm -f 860a4b23 podman pod rm -f -a`, @@ -44,9 +47,6 @@ func init() { // podRmCmd deletes pods func podRmCmd(c *cliconfig.PodRmValues) error { - if err := checkMutuallyExclusiveFlags(&c.PodmanCommand); err != nil { - return err - } runtime, err := adapter.GetRuntime(&c.PodmanCommand) if err != nil { return errors.Wrapf(err, "could not get runtime") diff --git a/cmd/podman/pod_start.go b/cmd/podman/pod_start.go index d093c51cf..a04029e79 100644 --- a/cmd/podman/pod_start.go +++ b/cmd/podman/pod_start.go @@ -26,6 +26,9 @@ var ( podStartCommand.GlobalFlags = MainGlobalOpts return podStartCmd(&podStartCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman pod start podID podman pod start --latest podman pod start --all`, @@ -41,10 +44,6 @@ func init() { } func podStartCmd(c *cliconfig.PodStartValues) error { - if err := checkMutuallyExclusiveFlags(&c.PodmanCommand); err != nil { - return err - } - runtime, err := libpodruntime.GetRuntime(&c.PodmanCommand) if err != nil { return errors.Wrapf(err, "could not get runtime") diff --git a/cmd/podman/pod_stop.go b/cmd/podman/pod_stop.go index a9237347e..a3816b1e1 100644 --- a/cmd/podman/pod_stop.go +++ b/cmd/podman/pod_stop.go @@ -27,6 +27,9 @@ var ( podStopCommand.GlobalFlags = MainGlobalOpts return podStopCmd(&podStopCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman pod stop mywebserverpod podman pod stop --latest podman pod stop --timeout 0 490eb 3557fb`, @@ -44,10 +47,6 @@ func init() { func podStopCmd(c *cliconfig.PodStopValues) error { timeout := -1 - if err := checkMutuallyExclusiveFlags(&c.PodmanCommand); err != nil { - return err - } - runtime, err := libpodruntime.GetRuntime(&c.PodmanCommand) if err != nil { return errors.Wrapf(err, "could not get runtime") diff --git a/cmd/podman/pod_unpause.go b/cmd/podman/pod_unpause.go index 6c131b92c..b860000af 100644 --- a/cmd/podman/pod_unpause.go +++ b/cmd/podman/pod_unpause.go @@ -22,6 +22,9 @@ var ( podUnpauseCommand.GlobalFlags = MainGlobalOpts return podUnpauseCmd(&podUnpauseCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman pod unpause podID1 podID2 podman pod unpause --all podman pod unpause --latest`, @@ -37,10 +40,6 @@ func init() { } func podUnpauseCmd(c *cliconfig.PodUnpauseValues) error { - if err := checkMutuallyExclusiveFlags(&c.PodmanCommand); err != nil { - return err - } - runtime, err := libpodruntime.GetRuntime(&c.PodmanCommand) if err != nil { return errors.Wrapf(err, "error creating libpod runtime") diff --git a/cmd/podman/port.go b/cmd/podman/port.go index 581371e4f..6a15db88d 100644 --- a/cmd/podman/port.go +++ b/cmd/podman/port.go @@ -28,6 +28,9 @@ var ( portCommand.GlobalFlags = MainGlobalOpts return portCmd(&portCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, true) + }, Example: `podman port --all podman port ctrID 80/tcp podman port --latest 80`, diff --git a/cmd/podman/restart.go b/cmd/podman/restart.go index 97b689c02..20af0e69a 100644 --- a/cmd/podman/restart.go +++ b/cmd/podman/restart.go @@ -26,6 +26,9 @@ var ( restartCommand.GlobalFlags = MainGlobalOpts return restartCmd(&restartCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman restart ctrID podman restart --latest podman restart ctrID1 ctrID2`, diff --git a/cmd/podman/restore.go b/cmd/podman/restore.go index 2911bbdd6..eb4d6a6c5 100644 --- a/cmd/podman/restore.go +++ b/cmd/podman/restore.go @@ -29,6 +29,9 @@ var ( restoreCommand.GlobalFlags = MainGlobalOpts return restoreCmd(&restoreCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman container restore ctrID podman container restore --latest podman container restore --all`, @@ -63,10 +66,6 @@ func restoreCmd(c *cliconfig.RestoreValues) error { TCPEstablished: c.TcpEstablished, } - if err := checkAllAndLatest(&c.PodmanCommand); err != nil { - return err - } - containers, lastError := getAllOrLatestContainers(&c.PodmanCommand, runtime, libpod.ContainerStateExited, "checkpointed") for _, ctr := range containers { diff --git a/cmd/podman/rm.go b/cmd/podman/rm.go index 2e5fe1dc0..8ff2ee727 100644 --- a/cmd/podman/rm.go +++ b/cmd/podman/rm.go @@ -28,6 +28,9 @@ Running containers will not be removed without the -f option. rmCommand.GlobalFlags = MainGlobalOpts return rmCmd(&rmCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman rm imageID podman rm mywebserver myflaskserver 860a4b23 podman rm --force --all`, @@ -57,10 +60,6 @@ func rmCmd(c *cliconfig.RmValues) error { } defer runtime.Shutdown(false) - if err := checkAllAndLatest(&c.PodmanCommand); err != nil { - return err - } - delContainers, err := getAllOrLatestContainers(&c.PodmanCommand, runtime, -1, "all") if err != nil { if c.Force && len(c.InputArgs) > 0 { diff --git a/cmd/podman/save.go b/cmd/podman/save.go index ba5209f34..ab421add6 100644 --- a/cmd/podman/save.go +++ b/cmd/podman/save.go @@ -1,20 +1,25 @@ package main import ( - "os" - "github.com/containers/libpod/cmd/podman/cliconfig" "github.com/containers/libpod/libpod/adapter" + "github.com/containers/libpod/pkg/util" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "os" + "strings" ) const ( ociManifestDir = "oci-dir" + ociArchive = "oci-archive" v2s2ManifestDir = "docker-dir" + v2s2Archive = "docker-archive" ) +var validFormats = []string{ociManifestDir, ociArchive, v2s2ManifestDir, v2s2Archive} + var ( saveCommand cliconfig.SaveValues saveDescription = ` @@ -30,6 +35,16 @@ var ( saveCommand.GlobalFlags = MainGlobalOpts return saveCmd(&saveCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + format, err := cmd.Flags().GetString("format") + if err != nil { + return err + } + if !util.StringInSlice(format, validFormats) { + return errors.Errorf("format value must be one of %s", strings.Join(validFormats, " ")) + } + return nil + }, Example: `podman save --quiet -o myimage.tar imageID podman save --format docker-dir -o ubuntu-dir ubuntu podman save > alpine-all.tar alpine:latest`, @@ -41,7 +56,7 @@ func init() { saveCommand.SetUsageTemplate(UsageTemplate()) flags := saveCommand.Flags() flags.BoolVar(&saveCommand.Compress, "compress", false, "Compress tarball image layers when saving to a directory using the 'dir' transport. (default is same compression type as source)") - flags.StringVar(&saveCommand.Format, "format", "docker-archive", "Save image to oci-archive, oci-dir (directory with oci manifest type), docker-dir (directory with v2s2 manifest type)") + flags.StringVar(&saveCommand.Format, "format", v2s2Archive, "Save image to oci-archive, oci-dir (directory with oci manifest type), docker-archive, docker-dir (directory with v2s2 manifest type)") flags.StringVarP(&saveCommand.Output, "output", "o", "/dev/stdout", "Write to a file, default is STDOUT") flags.BoolVarP(&saveCommand.Quiet, "quiet", "q", false, "Suppress the output") } diff --git a/cmd/podman/stats.go b/cmd/podman/stats.go index 642e54f49..3f5493102 100644 --- a/cmd/podman/stats.go +++ b/cmd/podman/stats.go @@ -41,6 +41,9 @@ var ( statsCommand.GlobalFlags = MainGlobalOpts return statsCmd(&statsCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman stats --all --no-stream podman stats ctrID podman stats --no-stream --format "table {{.ID}} {{.Name}} {{.MemUsage}}" ctrID`, diff --git a/cmd/podman/stop.go b/cmd/podman/stop.go index 94fdf321e..f2a1aa118 100644 --- a/cmd/podman/stop.go +++ b/cmd/podman/stop.go @@ -32,6 +32,9 @@ var ( stopCommand.GlobalFlags = MainGlobalOpts return stopCmd(&stopCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, false) + }, Example: `podman stop ctrID podman stop --latest podman stop --timeout 2 mywebserver 6e534f14da9d`, @@ -54,10 +57,6 @@ func stopCmd(c *cliconfig.StopValues) error { defer span.Finish() } - if err := checkAllAndLatest(&c.PodmanCommand); err != nil { - return err - } - rootless.SetSkipStorageSetup(true) runtime, err := libpodruntime.GetRuntime(&c.PodmanCommand) if err != nil { diff --git a/cmd/podman/umount.go b/cmd/podman/umount.go index afa0e86db..02bec551e 100644 --- a/cmd/podman/umount.go +++ b/cmd/podman/umount.go @@ -31,6 +31,9 @@ An unmount can be forced with the --force flag. umountCommand.GlobalFlags = MainGlobalOpts return umountCmd(&umountCommand) }, + Args: func(cmd *cobra.Command, args []string) error { + return checkAllAndLatest(cmd, args, true) + }, Example: `podman umount ctrID podman umount ctrID1 ctrID2 ctrID3 podman umount --all`, @@ -55,9 +58,6 @@ func umountCmd(c *cliconfig.UmountValues) error { force := c.Force umountAll := c.All - if err := checkAllAndLatest(&c.PodmanCommand); err != nil { - return err - } containers, err := getAllOrLatestContainers(&c.PodmanCommand, runtime, -1, "all") if err != nil { diff --git a/cmd/podman/utils.go b/cmd/podman/utils.go index c76e7f2a4..88ac62304 100644 --- a/cmd/podman/utils.go +++ b/cmd/podman/utils.go @@ -158,13 +158,6 @@ func (f *RawTtyFormatter) Format(entry *logrus.Entry) ([]byte, error) { return bytes, err } -func checkMutuallyExclusiveFlags(c *cliconfig.PodmanCommand) error { - if err := checkAllAndLatest(c); err != nil { - return err - } - return nil -} - // For pod commands that have a latest and all flag, getPodsFromContext gets // pods the user specifies. If there's an error before getting pods, the pods slice // will be empty and error will be not nil. If an error occured after, the pod slice -- cgit v1.2.3-54-g00ecf