cli commands: better error for unsupported commands

When you run podman-remote unsahre for example you currently get: Error: unrecognized command `podman-remote unshare` This is because we do not add the command to the cobra tree when we run in remote mode. However this is a bad user experience since it is not clear that the command is only supported for local podman. Users are left wondering why this does not work and could think the documentation is wrong. To fix it we add a clear error message: Error: cannot use command "podman-remote unshare" with the remote podman client Signed-off-by: Paul Holzinger <[email protected]>
giuseppe · Mar 31, 2022 · e574513 · e574513
1 parent 4ba71f9
commit e574513
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 19 deletions.
diff --git a/cmd/podman/main.go b/cmd/podman/main.go
@@ -3,7 +3,6 @@ package main
 import (
 	"fmt"
 	"os"
-	"strings"
 
 	_ "github.com/containers/podman/v4/cmd/podman/completion"
 	_ "github.com/containers/podman/v4/cmd/podman/containers"
@@ -20,6 +19,7 @@ import (
 	_ "github.com/containers/podman/v4/cmd/podman/system"
 	_ "github.com/containers/podman/v4/cmd/podman/system/connection"
 	_ "github.com/containers/podman/v4/cmd/podman/volumes"
+	"github.com/containers/podman/v4/pkg/domain/entities"
 	"github.com/containers/podman/v4/pkg/rootless"
 	"github.com/containers/podman/v4/pkg/terminal"
 	"github.com/containers/storage/pkg/reexec"
@@ -44,7 +44,29 @@ func parseCommands() *cobra.Command {
 	cfg := registry.PodmanConfig()
 	for _, c := range registry.Commands {
 		if supported, found := c.Command.Annotations[registry.EngineMode]; found {
-			if !strings.Contains(cfg.EngineMode.String(), supported) {
+			if cfg.EngineMode.String() != supported {
+				var client string
+				switch cfg.EngineMode {
+				case entities.TunnelMode:
+					client = "remote"
+				case entities.ABIMode:
+					client = "local"
+				}
+
+				// add error message to the command so the user knows that this command is not supported with local/remote
+				c.Command.RunE = func(cmd *cobra.Command, args []string) error {
+					return fmt.Errorf("cannot use command %q with the %s podman client", cmd.CommandPath(), client)
+				}
+				// turn of flag parsing to make we do not get flag errors
+				c.Command.DisableFlagParsing = true
+
+				// mark command as hidden so it is not shown in --help
+				c.Command.Hidden = true
+
+				// overwrite persistent pre/post function to skip setup
+				c.Command.PersistentPostRunE = noop
+				c.Command.PersistentPreRunE = noop
+				addCommand(c)
 				continue
 			}
 		}
@@ -65,22 +87,9 @@ func parseCommands() *cobra.Command {
 				}
 			}
 		}
-
-		parent := rootCmd
-		if c.Parent != nil {
-			parent = c.Parent
-		}
-		parent.AddCommand(c.Command)
-
-		c.Command.SetFlagErrorFunc(flagErrorFuncfunc)
-
-		// - templates need to be set here, as PersistentPreRunE() is
-		// not called when --help is used.
-		// - rootCmd uses cobra default template not ours
-		c.Command.SetHelpTemplate(helpTemplate)
-		c.Command.SetUsageTemplate(usageTemplate)
-		c.Command.DisableFlagsInUseLine = true
+		addCommand(c)
 	}
+
 	if err := terminal.SetConsole(); err != nil {
 		logrus.Error(err)
 		os.Exit(1)
@@ -94,3 +103,24 @@ func flagErrorFuncfunc(c *cobra.Command, e error) error {
 	e = fmt.Errorf("%w\nSee '%s --help'", e, c.CommandPath())
 	return e
 }
+
+func addCommand(c registry.CliCommand) {
+	parent := rootCmd
+	if c.Parent != nil {
+		parent = c.Parent
+	}
+	parent.AddCommand(c.Command)
+
+	c.Command.SetFlagErrorFunc(flagErrorFuncfunc)
+
+	// - templates need to be set here, as PersistentPreRunE() is
+	// not called when --help is used.
+	// - rootCmd uses cobra default template not ours
+	c.Command.SetHelpTemplate(helpTemplate)
+	c.Command.SetUsageTemplate(usageTemplate)
+	c.Command.DisableFlagsInUseLine = true
+}
+
+func noop(cmd *cobra.Command, args []string) error {
+	return nil
+}
diff --git a/test/e2e/unshare_test.go b/test/e2e/unshare_test.go
@@ -16,7 +16,6 @@ var _ = Describe("Podman unshare", func() {
 		podmanTest *PodmanTestIntegration
 	)
 	BeforeEach(func() {
-		SkipIfRemote("podman-remote unshare is not supported")
 		if _, err := os.Stat("/proc/self/uid_map"); err != nil {
 			Skip("User namespaces not supported.")
 		}
@@ -43,21 +42,24 @@ var _ = Describe("Podman unshare", func() {
 	})
 
 	It("podman unshare", func() {
+		SkipIfRemote("podman-remote unshare is not supported")
 		userNS, _ := os.Readlink("/proc/self/ns/user")
 		session := podmanTest.Podman([]string{"unshare", "readlink", "/proc/self/ns/user"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 		Expect(session.OutputToString()).ToNot(ContainSubstring(userNS))
 	})
 
-	It("podman unshare --rootles-cni", func() {
+	It("podman unshare --rootless-cni", func() {
+		SkipIfRemote("podman-remote unshare is not supported")
 		session := podmanTest.Podman([]string{"unshare", "--rootless-netns", "ip", "addr"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 		Expect(session.OutputToString()).To(ContainSubstring("tap0"))
 	})
 
 	It("podman unshare exit codes", func() {
+		SkipIfRemote("podman-remote unshare is not supported")
 		session := podmanTest.Podman([]string{"unshare", "false"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(1))
@@ -88,4 +90,12 @@ var _ = Describe("Podman unshare", func() {
 		Expect(session.OutputToString()).Should(Equal(""))
 		Expect(session.ErrorToString()).Should(ContainSubstring("unknown flag: --bogus"))
 	})
+
+	It("podman unshare check remote error", func() {
+		SkipIfNotRemote("check for podman-remote unshare error")
+		session := podmanTest.Podman([]string{"unshare"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(session.OutputToString()).To(Equal(`Error: cannot use command "podman-remote unshare" with the remote podman client`))
+	})
 })