Merge pull request containers#13738 from Luap99/remote-command

cli commands: better error for unsupported commands
giuseppe · Mar 31, 2022 · d1f3a2d · d1f3a2d
2 parents 4ba71f9 + e574513
commit d1f3a2d
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`))
+	})
 })