Improve command line validation

Use the checkallandlatest function to validate flag usage as part
of the cobra command args validation.

Signed-off-by: baude <bbaude@redhat.com>

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