Skip to content

Commit

Permalink
Usability: prevent "-l" with arguments
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
edsantiago committed Sep 14, 2020
1 parent fd7cdb2 commit 2583948
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 5 deletions.
2 changes: 1 addition & 1 deletion cmd/podman/containers/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
3 changes: 3 additions & 0 deletions cmd/podman/containers/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions cmd/podman/containers/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/podman/containers/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions cmd/podman/containers/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/podman/inspect/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 3 additions & 0 deletions cmd/podman/pods/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
7 changes: 5 additions & 2 deletions cmd/podman/validate/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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")
}
Expand Down
14 changes: 14 additions & 0 deletions test/system/015-help.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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]'.
Expand Down

0 comments on commit 2583948

Please sign in to comment.