From 91a42fefcbd5372c8701f5a0f5d6da85216cc465 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Wed, 29 Apr 2020 10:03:12 -0600 Subject: [PATCH 1/2] System tests: help messages: check required-arg If a usage message is of the form '... [flags] ARGNAME', where ARGNAME is all-caps and not in brackets, it must be a required argument. Try running podman subcommand without ARGNAME, and make sure that podman bails out with an informative message. (Since this message is freeform in each subcommand, not Cobra-generated, we have a lot of possible variations to check for). Fix podman login/logout Use messages to indicate that REGISTRY is now optional (as of #5233). This test has actually been in place for over a year but due to a typo on my part -- a missing space -- it was not being run. "For want of a space, much testing was lost". Signed-off-by: Ed Santiago --- cmd/podman/login.go | 2 +- cmd/podman/logout.go | 2 +- test/system/015-help.bats | 21 +++++++++++++++++---- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/cmd/podman/login.go b/cmd/podman/login.go index 1843a764d2..9de805d159 100644 --- a/cmd/podman/login.go +++ b/cmd/podman/login.go @@ -19,7 +19,7 @@ type loginOptionsWrapper struct { var ( loginOptions = loginOptionsWrapper{} loginCommand = &cobra.Command{ - Use: "login [flags] REGISTRY", + Use: "login [flags] [REGISTRY]", Short: "Login to a container registry", Long: "Login to a container registry on a specified server.", RunE: login, diff --git a/cmd/podman/logout.go b/cmd/podman/logout.go index 77bdc92b47..c21711fc08 100644 --- a/cmd/podman/logout.go +++ b/cmd/podman/logout.go @@ -14,7 +14,7 @@ import ( var ( logoutOptions = auth.LogoutOptions{} logoutCommand = &cobra.Command{ - Use: "logout [flags] REGISTRY", + Use: "logout [flags] [REGISTRY]", Short: "Logout of a container registry", Long: "Remove the cached username and password for the registry.", RunE: logout, diff --git a/test/system/015-help.bats b/test/system/015-help.bats index fd4be87b2e..231e521956 100644 --- a/test/system/015-help.bats +++ b/test/system/015-help.bats @@ -55,11 +55,24 @@ function check_help() { # If usage has required arguments, try running without them if expr "$usage" : '.*\[flags\] [A-Z]' >/dev/null; then - if [ "$cmd" != "stats"]; then - dprint "podman $@ $cmd (without required args)" - run_podman 125 "$@" $cmd - is "$output" "Error:" + # Exceptions: these commands don't work rootless + if is_rootless; then + # "pause is not supported for rootless containers" + if [ "$cmd" = "pause" -o "$cmd" = "unpause" ]; then + continue + fi + # "network rm" too + if [ "$@" = "network" -a "$cmd" = "rm" ]; then + continue + fi fi + + # The Date: Thu, 30 Apr 2020 12:31:48 -0500 Subject: [PATCH 2/2] fix commands without input in cases where commands require input and we dont provide it, we often would segv. This can be attributed in many cases to the subcommand not picked up the cobra Args attribute or neither had them. Signed-off-by: baude --- cmd/podman/containers/attach.go | 9 +++------ cmd/podman/containers/commit.go | 1 + cmd/podman/containers/create.go | 1 + cmd/podman/containers/exec.go | 22 ++++++++++++++-------- cmd/podman/containers/export.go | 1 + cmd/podman/containers/kill.go | 3 +++ cmd/podman/containers/rm.go | 3 +++ cmd/podman/containers/run.go | 2 ++ cmd/podman/containers/stop.go | 3 +++ cmd/podman/containers/unmount.go | 3 +++ cmd/podman/containers/wait.go | 1 + cmd/podman/images/pull.go | 1 + test/system/015-help.bats | 2 +- 13 files changed, 37 insertions(+), 15 deletions(-) diff --git a/cmd/podman/containers/attach.go b/cmd/podman/containers/attach.go index ee4d811d71..119b47d3f8 100644 --- a/cmd/podman/containers/attach.go +++ b/cmd/podman/containers/attach.go @@ -4,6 +4,7 @@ import ( "os" "github.com/containers/libpod/cmd/podman/registry" + "github.com/containers/libpod/cmd/podman/validate" "github.com/containers/libpod/pkg/domain/entities" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -17,12 +18,7 @@ var ( Short: "Attach to a running container", Long: attachDescription, RunE: attach, - Args: func(cmd *cobra.Command, args []string) error { - if len(args) > 1 || (len(args) == 0 && !cmd.Flag("latest").Changed) { - return errors.Errorf("attach requires the name or id of one running container or the latest flag") - } - return nil - }, + Args: validate.IdOrLatestArgs, Example: `podman attach ctrID podman attach 1234 podman attach --no-stdin foobar`, @@ -33,6 +29,7 @@ var ( Short: attachCommand.Short, Long: attachCommand.Long, RunE: attachCommand.RunE, + Args: validate.IdOrLatestArgs, Example: `podman container attach ctrID podman container attach 1234 podman container attach --no-stdin foobar`, diff --git a/cmd/podman/containers/commit.go b/cmd/podman/containers/commit.go index 137e486ebb..b3c3d76260 100644 --- a/cmd/podman/containers/commit.go +++ b/cmd/podman/containers/commit.go @@ -30,6 +30,7 @@ var ( } containerCommitCommand = &cobra.Command{ + Args: cobra.MinimumNArgs(1), Use: commitCommand.Use, Short: commitCommand.Short, Long: commitCommand.Long, diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index 3e47a8b4f5..f3f9d18f3a 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -33,6 +33,7 @@ var ( } containerCreateCommand = &cobra.Command{ + Args: cobra.MinimumNArgs(1), Use: createCommand.Use, Short: createCommand.Short, Long: createCommand.Long, diff --git a/cmd/podman/containers/exec.go b/cmd/podman/containers/exec.go index 2bff8ae335..0992b38624 100644 --- a/cmd/podman/containers/exec.go +++ b/cmd/podman/containers/exec.go @@ -16,20 +16,22 @@ var ( execDescription = `Execute the specified command inside a running container. ` execCommand = &cobra.Command{ - Use: "exec [flags] CONTAINER [COMMAND [ARG...]]", - Short: "Run a process in a running container", - Long: execDescription, - RunE: exec, + Use: "exec [flags] CONTAINER [COMMAND [ARG...]]", + Short: "Run a process in a running container", + Long: execDescription, + RunE: exec, + DisableFlagsInUseLine: true, Example: `podman exec -it ctrID ls podman exec -it -w /tmp myCtr pwd podman exec --user root ctrID ls`, } containerExecCommand = &cobra.Command{ - Use: execCommand.Use, - Short: execCommand.Short, - Long: execCommand.Long, - RunE: execCommand.RunE, + Use: execCommand.Use, + Short: execCommand.Short, + Long: execCommand.Long, + RunE: execCommand.RunE, + DisableFlagsInUseLine: true, Example: `podman container exec -it ctrID ls podman container exec -it -w /tmp myCtr pwd podman container exec --user root ctrID ls`, @@ -79,6 +81,10 @@ func init() { func exec(cmd *cobra.Command, args []string) error { var nameOrId string + + if len(args) == 0 && !execOpts.Latest { + return errors.New("exec requires the name or ID of a container or the --latest flag") + } execOpts.Cmd = args if !execOpts.Latest { execOpts.Cmd = args[1:] diff --git a/cmd/podman/containers/export.go b/cmd/podman/containers/export.go index fb5bd468fc..bbb6a6bc95 100644 --- a/cmd/podman/containers/export.go +++ b/cmd/podman/containers/export.go @@ -28,6 +28,7 @@ var ( } containerExportCommand = &cobra.Command{ + Args: cobra.MinimumNArgs(1), Use: exportCommand.Use, Short: exportCommand.Short, Long: exportCommand.Long, diff --git a/cmd/podman/containers/kill.go b/cmd/podman/containers/kill.go index 8b4a384fe7..ef85aad7df 100644 --- a/cmd/podman/containers/kill.go +++ b/cmd/podman/containers/kill.go @@ -30,6 +30,9 @@ var ( } containerKillCommand = &cobra.Command{ + Args: func(cmd *cobra.Command, args []string) error { + return parse.CheckAllLatestAndCIDFile(cmd, args, false, false) + }, Use: killCommand.Use, Short: killCommand.Short, Long: killCommand.Long, diff --git a/cmd/podman/containers/rm.go b/cmd/podman/containers/rm.go index 3021853a93..96549ceada 100644 --- a/cmd/podman/containers/rm.go +++ b/cmd/podman/containers/rm.go @@ -38,6 +38,9 @@ var ( Short: rmCommand.Use, Long: rmCommand.Long, RunE: rmCommand.RunE, + Args: func(cmd *cobra.Command, args []string) error { + return parse.CheckAllLatestAndCIDFile(cmd, args, false, true) + }, Example: `podman container rm imageID podman container rm mywebserver myflaskserver 860a4b23 podman container rm --force --all diff --git a/cmd/podman/containers/run.go b/cmd/podman/containers/run.go index e3fe4cd0b9..1e4d47b42c 100644 --- a/cmd/podman/containers/run.go +++ b/cmd/podman/containers/run.go @@ -20,6 +20,7 @@ import ( var ( runDescription = "Runs a command in a new container from the given image" runCommand = &cobra.Command{ + Args: cobra.MinimumNArgs(1), Use: "run [flags] IMAGE [COMMAND [ARG...]]", Short: "Run a command in a new container", Long: runDescription, @@ -30,6 +31,7 @@ var ( } containerRunCommand = &cobra.Command{ + Args: cobra.MinimumNArgs(1), Use: runCommand.Use, Short: runCommand.Short, Long: runCommand.Long, diff --git a/cmd/podman/containers/stop.go b/cmd/podman/containers/stop.go index 4a451134aa..22c4879611 100644 --- a/cmd/podman/containers/stop.go +++ b/cmd/podman/containers/stop.go @@ -34,6 +34,9 @@ var ( Short: stopCommand.Short, Long: stopCommand.Long, RunE: stopCommand.RunE, + Args: func(cmd *cobra.Command, args []string) error { + return parse.CheckAllLatestAndCIDFile(cmd, args, false, true) + }, Example: `podman container stop ctrID podman container stop --latest podman container stop --time 2 mywebserver 6e534f14da9d`, diff --git a/cmd/podman/containers/unmount.go b/cmd/podman/containers/unmount.go index 7b6eb55532..d0ca202fe4 100644 --- a/cmd/podman/containers/unmount.go +++ b/cmd/podman/containers/unmount.go @@ -40,6 +40,9 @@ var ( Short: umountCommand.Short, Long: umountCommand.Long, RunE: umountCommand.RunE, + Args: func(cmd *cobra.Command, args []string) error { + return parse.CheckAllLatestAndCIDFile(cmd, args, false, false) + }, Example: `podman container umount ctrID podman container umount ctrID1 ctrID2 ctrID3 podman container umount --all`, diff --git a/cmd/podman/containers/wait.go b/cmd/podman/containers/wait.go index eac1e2956f..1f4d4159bb 100644 --- a/cmd/podman/containers/wait.go +++ b/cmd/podman/containers/wait.go @@ -34,6 +34,7 @@ var ( Short: waitCommand.Short, Long: waitCommand.Long, RunE: waitCommand.RunE, + Args: validate.IdOrLatestArgs, Example: `podman container wait --latest podman container wait --interval 5000 ctrID podman container wait ctrID1 ctrID2`, diff --git a/cmd/podman/images/pull.go b/cmd/podman/images/pull.go index fead5f7ed0..9f4cbc50ed 100644 --- a/cmd/podman/images/pull.go +++ b/cmd/podman/images/pull.go @@ -45,6 +45,7 @@ var ( Short: pullCmd.Short, Long: pullCmd.Long, RunE: pullCmd.RunE, + Args: cobra.ExactArgs(1), Example: `podman image pull imageName podman image pull fedora:latest`, } diff --git a/test/system/015-help.bats b/test/system/015-help.bats index 231e521956..6c3d617dc0 100644 --- a/test/system/015-help.bats +++ b/test/system/015-help.bats @@ -71,7 +71,7 @@ function check_help() { # try to read username/password from stdin. dprint "podman $@ $cmd (without required args)" run_podman 125 "$@" $cmd