From 2583948f72a05cf38dc9f0c4ec1feef64ab9c9af Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Mon, 14 Sep 2020 15:19:34 -0600 Subject: Usability: prevent "-l" with arguments Add new system check confirming that "podman foo -l arg" throws an error; and fix lots of instances where code was not doing this check. I'll probably need to add something similar for --all but that can wait. Signed-off-by: Ed Santiago --- cmd/podman/containers/logs.go | 2 +- cmd/podman/containers/mount.go | 3 +++ cmd/podman/containers/restart.go | 3 +++ cmd/podman/containers/restore.go | 2 +- cmd/podman/containers/start.go | 3 +++ cmd/podman/inspect/inspect.go | 2 +- cmd/podman/pods/inspect.go | 3 +++ cmd/podman/validate/args.go | 7 +++++-- test/system/015-help.bats | 14 ++++++++++++++ 9 files changed, 34 insertions(+), 5 deletions(-) diff --git a/cmd/podman/containers/logs.go b/cmd/podman/containers/logs.go index 21c00505f..acc2ab1aa 100644 --- a/cmd/podman/containers/logs.go +++ b/cmd/podman/containers/logs.go @@ -37,7 +37,7 @@ var ( case registry.IsRemote() && len(args) > 1: return errors.New(cmd.Name() + " does not support multiple containers when run remotely") case logsOptions.Latest && len(args) > 0: - return errors.New("no containers can be specified when using 'latest'") + return errors.New("--latest and containers cannot be used together") case !logsOptions.Latest && len(args) < 1: return errors.New("specify at least one container name or ID to log") } diff --git a/cmd/podman/containers/mount.go b/cmd/podman/containers/mount.go index 1c358f4a2..f2df5e99e 100644 --- a/cmd/podman/containers/mount.go +++ b/cmd/podman/containers/mount.go @@ -78,6 +78,9 @@ func mount(_ *cobra.Command, args []string) error { var ( errs utils.OutputErrors ) + if len(args) > 0 && mountOpts.Latest { + return errors.Errorf("--latest and containers cannot be used together") + } reports, err := registry.ContainerEngine().ContainerMount(registry.GetContext(), args, mountOpts) if err != nil { return err diff --git a/cmd/podman/containers/restart.go b/cmd/podman/containers/restart.go index d02097f0e..5f6f9c35c 100644 --- a/cmd/podman/containers/restart.go +++ b/cmd/podman/containers/restart.go @@ -80,6 +80,9 @@ func restart(cmd *cobra.Command, args []string) error { if len(args) < 1 && !restartOptions.Latest && !restartOptions.All { return errors.Wrapf(define.ErrInvalidArg, "you must provide at least one container name or ID") } + if len(args) > 0 && restartOptions.Latest { + return errors.Wrapf(define.ErrInvalidArg, "--latest and containers cannot be used together") + } if cmd.Flag("time").Changed { restartOptions.Timeout = &restartTimeout diff --git a/cmd/podman/containers/restore.go b/cmd/podman/containers/restore.go index 413bcbc87..c996144e3 100644 --- a/cmd/podman/containers/restore.go +++ b/cmd/podman/containers/restore.go @@ -80,7 +80,7 @@ func restore(_ *cobra.Command, args []string) error { } } if (restoreOptions.All || restoreOptions.Latest) && argLen > 0 { - return errors.Errorf("no arguments are needed with --all or --latest") + return errors.Errorf("--all or --latest and containers cannot be used together") } if argLen < 1 && !restoreOptions.All && !restoreOptions.Latest && restoreOptions.Import == "" { return errors.Errorf("you must provide at least one name or id") diff --git a/cmd/podman/containers/start.go b/cmd/podman/containers/start.go index ccbe80317..1e58498b6 100644 --- a/cmd/podman/containers/start.go +++ b/cmd/podman/containers/start.go @@ -74,6 +74,9 @@ func start(cmd *cobra.Command, args []string) error { if len(args) == 0 && !startOptions.Latest { return errors.New("start requires at least one argument") } + if len(args) > 0 && startOptions.Latest { + return errors.Errorf("--latest and containers cannot be used together") + } if len(args) > 1 && startOptions.Attach { return errors.Errorf("you cannot start and attach multiple containers at once") } diff --git a/cmd/podman/inspect/inspect.go b/cmd/podman/inspect/inspect.go index 13c6544bb..f29527412 100644 --- a/cmd/podman/inspect/inspect.go +++ b/cmd/podman/inspect/inspect.go @@ -93,7 +93,7 @@ func (i *inspector) inspect(namesOrIDs []string) error { tmpType := i.options.Type if i.options.Latest { if len(namesOrIDs) > 0 { - return errors.New("latest and containers are not allowed") + return errors.New("--latest and containers cannot be used together") } tmpType = ContainerType // -l works with --type=all } diff --git a/cmd/podman/pods/inspect.go b/cmd/podman/pods/inspect.go index 8f5aa6062..bc20352b0 100644 --- a/cmd/podman/pods/inspect.go +++ b/cmd/podman/pods/inspect.go @@ -46,6 +46,9 @@ func inspect(cmd *cobra.Command, args []string) error { if len(args) < 1 && !inspectOptions.Latest { return errors.Errorf("you must provide the name or id of a running pod") } + if len(args) > 0 && inspectOptions.Latest { + return errors.Errorf("--latest and containers cannot be used together") + } if !inspectOptions.Latest { inspectOptions.NameOrID = args[0] diff --git a/cmd/podman/validate/args.go b/cmd/podman/validate/args.go index aacb41e69..ab6460e93 100644 --- a/cmd/podman/validate/args.go +++ b/cmd/podman/validate/args.go @@ -37,6 +37,9 @@ func IDOrLatestArgs(cmd *cobra.Command, args []string) error { if len(args) == 0 && !given { return fmt.Errorf("%q requires a name, id, or the \"--latest\" flag", cmd.CommandPath()) } + if len(args) > 0 && given { + return fmt.Errorf("--latest and containers cannot be used together") + } } return nil } @@ -83,7 +86,7 @@ func CheckAllLatestAndCIDFile(c *cobra.Command, args []string, ignoreArgLen bool if argLen > 0 { if specifiedLatest { - return errors.Errorf("no arguments are needed with --latest") + return errors.Errorf("--latest and containers cannot be used together") } else if cidfile && (specifiedLatest || specifiedCIDFile) { return errors.Errorf("no arguments are needed with --latest or --cidfile") } @@ -138,7 +141,7 @@ func CheckAllLatestAndPodIDFile(c *cobra.Command, args []string, ignoreArgLen bo if argLen > 0 { if specifiedLatest { - return errors.Errorf("no arguments are needed with --latest") + return errors.Errorf("--latest and pods cannot be used together") } else if withIDFile && (specifiedLatest || specifiedPodIDFile) { return errors.Errorf("no arguments are needed with --latest or --pod-id-file") } diff --git a/test/system/015-help.bats b/test/system/015-help.bats index 4a3781012..651fdcd09 100644 --- a/test/system/015-help.bats +++ b/test/system/015-help.bats @@ -86,6 +86,20 @@ function check_help() { found[takes_no_args]=1 fi + # If command lists "-l, --latest" in help output, combine -l with arg. + # This should be disallowed with a clear message. + if expr "$full_help" : ".*-l, --latest" >/dev/null; then + local nope="exec list port ps top" # these can't be tested + if is_rootless; then + nope="$nope mount restore" # these don't work rootless + fi + if ! grep -wq "$cmd" <<<$nope; then + run_podman 125 "$@" $cmd -l nonexistent-container + is "$output" "Error: .*--latest and \(containers\|pods\|arguments\) cannot be used together" \ + "'$command_string' with both -l and container" + fi + fi + # If usage has required arguments, try running without them. # The expression here is 'first capital letter is not in [BRACKETS]'. # It is intended to handle 'podman foo [flags] ARG' but not ' [ARG]'. -- cgit v1.2.3-54-g00ecf