diff --git a/cli-plugins/plugin/plugin.go b/cli-plugins/plugin/plugin.go index 62ab11c9e6..7bd5f50c60 100644 --- a/cli-plugins/plugin/plugin.go +++ b/cli-plugins/plugin/plugin.go @@ -114,11 +114,14 @@ func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta fullname := manager.NamePrefix + name cmd := &cobra.Command{ - Use: fmt.Sprintf("docker [OPTIONS] %s [ARG...]", name), - Short: fullname + " is a Docker CLI plugin", - SilenceUsage: true, - SilenceErrors: true, - PersistentPreRunE: PersistentPreRunE, + Use: fmt.Sprintf("docker [OPTIONS] %s [ARG...]", name), + Short: fullname + " is a Docker CLI plugin", + SilenceUsage: true, + SilenceErrors: true, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + // We can't use this as the hook directly since it is initialised later (in runPlugin) + return PersistentPreRunE(cmd, args) + }, TraverseChildren: true, DisableFlagsInUseLine: true, } diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 22b978de9b..dafaea0703 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -69,16 +69,22 @@ func newDockerCommand(dockerCli *command.DockerCli) *cli.TopLevelCommand { return cli.NewTopLevelCommand(cmd, dockerCli, opts, flags) } -func setFlagErrorFunc(dockerCli *command.DockerCli, cmd *cobra.Command) { +func setFlagErrorFunc(dockerCli command.Cli, cmd *cobra.Command) { // When invoking `docker stack --nonsense`, we need to make sure FlagErrorFunc return appropriate // output if the feature is not supported. // As above cli.SetupRootCommand(cmd) have already setup the FlagErrorFunc, we will add a pre-check before the FlagErrorFunc // is called. flagErrorFunc := cmd.FlagErrorFunc() cmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { + if err := pluginmanager.AddPluginCommandStubs(dockerCli, cmd.Root()); err != nil { + return err + } if err := isSupported(cmd, dockerCli); err != nil { return err } + if err := hideUnsupportedFeatures(cmd, dockerCli); err != nil { + return err + } return flagErrorFunc(cmd, err) }) } diff --git a/e2e/cli-plugins/help_test.go b/e2e/cli-plugins/help_test.go index 8e57ccc876..b73054d4a3 100644 --- a/e2e/cli-plugins/help_test.go +++ b/e2e/cli-plugins/help_test.go @@ -74,18 +74,36 @@ func TestGlobalHelp(t *testing.T) { assert.Assert(t, is.Equal(badmetacount, 1)) // Running with `--help` should produce the same. - res2 := icmd.RunCmd(run("--help")) - res2.Assert(t, icmd.Expected{ - ExitCode: 0, + t.Run("help_flag", func(t *testing.T) { + res2 := icmd.RunCmd(run("--help")) + res2.Assert(t, icmd.Expected{ + ExitCode: 0, + }) + assert.Assert(t, is.Equal(res2.Stdout(), res.Stdout())) + assert.Assert(t, is.Equal(res2.Stderr(), "")) }) - assert.Assert(t, is.Equal(res2.Stdout(), res.Stdout())) - assert.Assert(t, is.Equal(res2.Stderr(), "")) // Running just `docker` (without `help` nor `--help`) should produce the same thing, except on Stderr. - res2 = icmd.RunCmd(run()) - res2.Assert(t, icmd.Expected{ - ExitCode: 0, + t.Run("bare", func(t *testing.T) { + res2 := icmd.RunCmd(run()) + res2.Assert(t, icmd.Expected{ + ExitCode: 0, + }) + assert.Assert(t, is.Equal(res2.Stdout(), "")) + assert.Assert(t, is.Equal(res2.Stderr(), res.Stdout())) + }) + + t.Run("badopt", func(t *testing.T) { + // Running `docker --badopt` should also produce the + // same thing, give or take the leading error message + // and a trailing carriage return (due to main() using + // Println in the error case). + res2 := icmd.RunCmd(run("--badopt")) + res2.Assert(t, icmd.Expected{ + ExitCode: 125, + }) + assert.Assert(t, is.Equal(res2.Stdout(), "")) + exp := "unknown flag: --badopt\nSee 'docker --help'.\n" + res.Stdout() + "\n" + assert.Assert(t, is.Equal(res2.Stderr(), exp)) }) - assert.Assert(t, is.Equal(res2.Stdout(), "")) - assert.Assert(t, is.Equal(res2.Stderr(), res.Stdout())) } diff --git a/e2e/cli-plugins/plugins/nopersistentprerun/main.go b/e2e/cli-plugins/plugins/nopersistentprerun/main.go new file mode 100644 index 0000000000..caaddb69c4 --- /dev/null +++ b/e2e/cli-plugins/plugins/nopersistentprerun/main.go @@ -0,0 +1,36 @@ +package main + +import ( + "context" + "fmt" + + "github.com/docker/cli/cli-plugins/manager" + "github.com/docker/cli/cli-plugins/plugin" + "github.com/docker/cli/cli/command" + "github.com/spf13/cobra" +) + +func main() { + plugin.Run(func(dockerCli command.Cli) *cobra.Command { + cmd := &cobra.Command{ + Use: "nopersistentprerun", + Short: "Testing without PersistentPreRun hooks", + //PersistentPreRunE: Not specified, we need to test that it works in the absence of an explicit call + RunE: func(cmd *cobra.Command, args []string) error { + cli := dockerCli.Client() + ping, err := cli.Ping(context.Background()) + if err != nil { + return err + } + fmt.Println(ping.APIVersion) + return nil + }, + } + return cmd + }, + manager.Metadata{ + SchemaVersion: "0.1.0", + Vendor: "Docker Inc.", + Version: "testing", + }) +} diff --git a/e2e/cli-plugins/run_test.go b/e2e/cli-plugins/run_test.go index 3c9426e042..91d384645e 100644 --- a/e2e/cli-plugins/run_test.go +++ b/e2e/cli-plugins/run_test.go @@ -213,15 +213,24 @@ func TestGoodSubcommandHelp(t *testing.T) { } // TestCliInitialized tests the code paths which ensure that the Cli -// object is initialized even if the plugin uses PersistentRunE +// object is initialized whether the plugin uses PersistentRunE or not func TestCliInitialized(t *testing.T) { run, _, cleanup := prepare(t) defer cleanup() - res := icmd.RunCmd(run("helloworld", "--pre-run", "apiversion")) - res.Assert(t, icmd.Success) - assert.Assert(t, res.Stdout() != "") - assert.Assert(t, is.Equal(res.Stderr(), "Plugin PersistentPreRunE called")) + var apiversion string + t.Run("withhook", func(t *testing.T) { + res := icmd.RunCmd(run("helloworld", "--pre-run", "apiversion")) + res.Assert(t, icmd.Success) + assert.Assert(t, res.Stdout() != "") + apiversion = res.Stdout() + assert.Assert(t, is.Equal(res.Stderr(), "Plugin PersistentPreRunE called")) + }) + t.Run("withouthook", func(t *testing.T) { + res := icmd.RunCmd(run("nopersistentprerun")) + res.Assert(t, icmd.Success) + assert.Assert(t, is.Equal(res.Stdout(), apiversion)) + }) } // TestPluginErrorCode tests when the plugin return with a given exit status.