diff --git a/cli-plugins/examples/helloworld/main.go b/cli-plugins/examples/helloworld/main.go index cfbae368b0..eca6acbef9 100644 --- a/cli-plugins/examples/helloworld/main.go +++ b/cli-plugins/examples/helloworld/main.go @@ -44,15 +44,26 @@ func main() { }, } - var who string + var ( + who, context string + debug bool + ) cmd := &cobra.Command{ Use: "helloworld", Short: "A basic Hello World plugin for tests", - // This is redundant but included to exercise - // the path where a plugin overrides this - // hook. - PersistentPreRunE: plugin.PersistentPreRunE, RunE: func(cmd *cobra.Command, args []string) error { + if debug { + fmt.Fprintf(dockerCli.Err(), "Plugin debug mode enabled") + } + + switch context { + case "Christmas": + fmt.Fprintf(dockerCli.Out(), "Merry Christmas!\n") + return nil + case "": + // nothing + } + if who == "" { who, _ = dockerCli.ConfigFile().PluginConfig("helloworld", "who") } @@ -68,6 +79,10 @@ func main() { flags := cmd.Flags() flags.StringVar(&who, "who", "", "Who are we addressing?") + // These are intended to deliberately clash with the CLIs own top + // level arguments. + flags.BoolVarP(&debug, "debug", "D", false, "Enable debug") + flags.StringVarP(&context, "context", "c", "", "Is it Christmas?") cmd.AddCommand(goodbye, apiversion, exitStatus2) return cmd diff --git a/cli-plugins/plugin/plugin.go b/cli-plugins/plugin/plugin.go index 99ef36d845..bad632d1f8 100644 --- a/cli-plugins/plugin/plugin.go +++ b/cli-plugins/plugin/plugin.go @@ -4,18 +4,32 @@ import ( "encoding/json" "fmt" "os" - "sync" "github.com/docker/cli/cli" "github.com/docker/cli/cli-plugins/manager" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/connhelper" - cliflags "github.com/docker/cli/cli/flags" "github.com/docker/docker/client" "github.com/spf13/cobra" - "github.com/spf13/pflag" ) +func runPlugin(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager.Metadata) error { + tcmd := newPluginCommand(dockerCli, plugin, meta) + + // Doing this here avoids also calling it for the metadata + // command which needlessly initializes the client and tries + // to connect to the daemon. + plugin.PersistentPreRunE = func(_ *cobra.Command, _ []string) error { + return tcmd.Initialize(withPluginClientConn(plugin.Name())) + } + + cmd, _, err := tcmd.HandleGlobalFlags() + if err != nil { + return err + } + return cmd.Execute() +} + // Run is the top-level entry point to the CLI plugin framework. It should be called from your plugin's `main()` function. func Run(makeCmd func(command.Cli) *cobra.Command, meta manager.Metadata) { dockerCli, err := command.NewDockerCli() @@ -26,9 +40,7 @@ func Run(makeCmd func(command.Cli) *cobra.Command, meta manager.Metadata) { plugin := makeCmd(dockerCli) - cmd := newPluginCommand(dockerCli, plugin, meta) - - if err := cmd.Execute(); err != nil { + if err := runPlugin(dockerCli, plugin, meta); err != nil { if sterr, ok := err.(cli.StatusError); ok { if sterr.Status != "" { fmt.Fprintln(dockerCli.Err(), sterr.Status) @@ -45,40 +57,6 @@ func Run(makeCmd func(command.Cli) *cobra.Command, meta manager.Metadata) { } } -// options encapsulates the ClientOptions and FlagSet constructed by -// `newPluginCommand` such that they can be finalized by our -// `PersistentPreRunE`. This is necessary because otherwise a plugin's -// own use of that hook will shadow anything we add to the top-level -// command meaning the CLI is never Initialized. -var options struct { - name string - init, prerun sync.Once - opts *cliflags.ClientOptions - flags *pflag.FlagSet - dockerCli *command.DockerCli -} - -// PersistentPreRunE must be called by any plugin command (or -// subcommand) which uses the cobra `PersistentPreRun*` hook. Plugins -// which do not make use of `PersistentPreRun*` do not need to call -// this (although it remains safe to do so). Plugins are recommended -// to use `PersistenPreRunE` to enable the error to be -// returned. Should not be called outside of a commands -// PersistentPreRunE hook and must not be run unless Run has been -// called. -func PersistentPreRunE(cmd *cobra.Command, args []string) error { - var err error - options.prerun.Do(func() { - if options.opts == nil || options.flags == nil || options.dockerCli == nil { - panic("PersistentPreRunE called without Run successfully called first") - } - // flags must be the original top-level command flags, not cmd.Flags() - options.opts.Common.SetDefaultOptions(options.flags) - err = options.dockerCli.Initialize(options.opts, withPluginClientConn(options.name)) - }) - return err -} - func withPluginClientConn(name string) command.InitializeOpt { return command.WithInitializeClient(func(dockerCli *command.DockerCli) (client.APIClient, error) { cmd := "docker" @@ -111,7 +89,7 @@ func withPluginClientConn(name string) command.InitializeOpt { }) } -func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager.Metadata) *cobra.Command { +func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager.Metadata) *cli.TopLevelCommand { name := plugin.Name() fullname := manager.NamePrefix + name @@ -121,7 +99,6 @@ func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta SilenceUsage: true, SilenceErrors: true, TraverseChildren: true, - PersistentPreRunE: PersistentPreRunE, DisableFlagsInUseLine: true, } opts, flags := cli.SetupPluginRootCommand(cmd) @@ -135,13 +112,7 @@ func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta cli.DisableFlagsInUseLine(cmd) - options.init.Do(func() { - options.name = name - options.opts = opts - options.flags = flags - options.dockerCli = dockerCli - }) - return cmd + return cli.NewTopLevelCommand(cmd, dockerCli, opts, flags) } func newMetadataSubcommand(plugin *cobra.Command, meta manager.Metadata) *cobra.Command { @@ -151,8 +122,6 @@ func newMetadataSubcommand(plugin *cobra.Command, meta manager.Metadata) *cobra. cmd := &cobra.Command{ Use: manager.MetadataSubcommandName, Hidden: true, - // Suppress the global/parent PersistentPreRunE, which needlessly initializes the client and tries to connect to the daemon. - PersistentPreRun: func(cmd *cobra.Command, args []string) {}, RunE: func(cmd *cobra.Command, args []string) error { enc := json.NewEncoder(os.Stdout) enc.SetEscapeHTML(false) diff --git a/cli/cobra.go b/cli/cobra.go index 803872ee5a..1385743f4b 100644 --- a/cli/cobra.go +++ b/cli/cobra.go @@ -2,9 +2,11 @@ package cli import ( "fmt" + "os" "strings" pluginmanager "github.com/docker/cli/cli-plugins/manager" + "github.com/docker/cli/cli/command" cliconfig "github.com/docker/cli/cli/config" cliflags "github.com/docker/cli/cli/flags" "github.com/docker/docker/pkg/term" @@ -84,6 +86,79 @@ func FlagErrorFunc(cmd *cobra.Command, err error) error { } } +// TopLevelCommand encapsulates a top-level cobra command (either +// docker CLI or a plugin) and global flag handling logic necessary +// for plugins. +type TopLevelCommand struct { + cmd *cobra.Command + dockerCli *command.DockerCli + opts *cliflags.ClientOptions + flags *pflag.FlagSet + args []string +} + +// NewTopLevelCommand returns a new TopLevelCommand object +func NewTopLevelCommand(cmd *cobra.Command, dockerCli *command.DockerCli, opts *cliflags.ClientOptions, flags *pflag.FlagSet) *TopLevelCommand { + return &TopLevelCommand{cmd, dockerCli, opts, flags, os.Args[1:]} +} + +// SetArgs sets the args (default os.Args[:1] used to invoke the command +func (tcmd *TopLevelCommand) SetArgs(args []string) { + tcmd.args = args + tcmd.cmd.SetArgs(args) +} + +// SetFlag sets a flag in the local flag set of the top-level command +func (tcmd *TopLevelCommand) SetFlag(name, value string) { + tcmd.cmd.Flags().Set(name, value) +} + +// HandleGlobalFlags takes care of parsing global flags defined on the +// command, it returns the underlying cobra command and the args it +// will be called with (or an error). +// +// On success the caller is responsible for calling Initialize() +// before calling `Execute` on the returned command. +func (tcmd *TopLevelCommand) HandleGlobalFlags() (*cobra.Command, []string, error) { + cmd := tcmd.cmd + + // We manually parse the global arguments and find the + // subcommand in order to properly deal with plugins. We rely + // on the root command never having any non-flag arguments. We + // create our own FlagSet so that we can configure it + // (e.g. `SetInterspersed` below) in an idempotent way. + flags := pflag.NewFlagSet(cmd.Name(), pflag.ContinueOnError) + + // We need !interspersed to ensure we stop at the first + // potential command instead of accumulating it into + // flags.Args() and then continuing on and finding other + // arguments which we try and treat as globals (when they are + // actually arguments to the subcommand). + flags.SetInterspersed(false) + + // We need the single parse to see both sets of flags. + flags.AddFlagSet(cmd.Flags()) + flags.AddFlagSet(cmd.PersistentFlags()) + // Now parse the global flags, up to (but not including) the + // first command. The result will be that all the remaining + // arguments are in `flags.Args()`. + if err := flags.Parse(tcmd.args); err != nil { + // Our FlagErrorFunc uses the cli, make sure it is initialized + if err := tcmd.Initialize(); err != nil { + return nil, nil, err + } + return nil, nil, cmd.FlagErrorFunc()(cmd, err) + } + + return cmd, flags.Args(), nil +} + +// Initialize finalises global option parsing and initializes the docker client. +func (tcmd *TopLevelCommand) Initialize(ops ...command.InitializeOpt) error { + tcmd.opts.Common.SetDefaultOptions(tcmd.flags) + return tcmd.dockerCli.Initialize(tcmd.opts, ops...) +} + // VisitAll will traverse all commands from the root. // This is different from the VisitAll of cobra.Command where only parents // are checked. diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index dc540430d9..06f074431b 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -21,7 +21,7 @@ import ( "github.com/spf13/pflag" ) -func newDockerCommand(dockerCli *command.DockerCli) *cobra.Command { +func newDockerCommand(dockerCli *command.DockerCli) *cli.TopLevelCommand { var ( opts *cliflags.ClientOptions flags *pflag.FlagSet @@ -34,51 +34,14 @@ func newDockerCommand(dockerCli *command.DockerCli) *cobra.Command { SilenceUsage: true, SilenceErrors: true, TraverseChildren: true, - FParseErrWhitelist: cobra.FParseErrWhitelist{ - // UnknownFlags ignores any unknown - // --arguments on the top-level docker command - // only. This is necessary to allow passing - // --arguments to plugins otherwise - // e.g. `docker plugin --foo` is caught here - // in the monolithic CLI and `foo` is reported - // as an unknown argument. - UnknownFlags: true, - }, RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { return command.ShowHelp(dockerCli.Err())(cmd, args) } - plugincmd, err := pluginmanager.PluginRunCommand(dockerCli, args[0], cmd) - if pluginmanager.IsNotFound(err) { - return fmt.Errorf( - "docker: '%s' is not a docker command.\nSee 'docker --help'", args[0]) - } - if err != nil { - return err - } + return fmt.Errorf("docker: '%s' is not a docker command.\nSee 'docker --help'", args[0]) - err = plugincmd.Run() - if err != nil { - statusCode := 1 - exitErr, ok := err.(*exec.ExitError) - if !ok { - return err - } - if ws, ok := exitErr.Sys().(syscall.WaitStatus); ok { - statusCode = ws.ExitStatus() - } - return cli.StatusError{ - StatusCode: statusCode, - } - } - return nil }, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - // flags must be the top-level command flags, not cmd.Flags() - opts.Common.SetDefaultOptions(flags) - if err := dockerCli.Initialize(opts); err != nil { - return err - } return isSupported(cmd, dockerCli) }, Version: fmt.Sprintf("%s, build %s", version.Version, version.GitCommit), @@ -87,30 +50,28 @@ func newDockerCommand(dockerCli *command.DockerCli) *cobra.Command { opts, flags, helpCmd = cli.SetupRootCommand(cmd) flags.BoolP("version", "v", false, "Print version information and quit") - setFlagErrorFunc(dockerCli, cmd, flags, opts) + setFlagErrorFunc(dockerCli, cmd) - setupHelpCommand(dockerCli, cmd, helpCmd, flags, opts) - setHelpFunc(dockerCli, cmd, flags, opts) + setupHelpCommand(dockerCli, cmd, helpCmd) + setHelpFunc(dockerCli, cmd) cmd.SetOutput(dockerCli.Out()) commands.AddCommands(cmd, dockerCli) cli.DisableFlagsInUseLine(cmd) - setValidateArgs(dockerCli, cmd, flags, opts) + setValidateArgs(dockerCli, cmd) - return cmd + // flags must be the top-level command flags, not cmd.Flags() + return cli.NewTopLevelCommand(cmd, dockerCli, opts, flags) } -func setFlagErrorFunc(dockerCli *command.DockerCli, cmd *cobra.Command, flags *pflag.FlagSet, opts *cliflags.ClientOptions) { +func setFlagErrorFunc(dockerCli *command.DockerCli, 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 := initializeDockerCli(dockerCli, flags, opts); err != nil { - return err - } if err := isSupported(cmd, dockerCli); err != nil { return err } @@ -118,17 +79,12 @@ func setFlagErrorFunc(dockerCli *command.DockerCli, cmd *cobra.Command, flags *p }) } -func setupHelpCommand(dockerCli *command.DockerCli, rootCmd, helpCmd *cobra.Command, flags *pflag.FlagSet, opts *cliflags.ClientOptions) { +func setupHelpCommand(dockerCli command.Cli, rootCmd, helpCmd *cobra.Command) { origRun := helpCmd.Run origRunE := helpCmd.RunE helpCmd.Run = nil helpCmd.RunE = func(c *cobra.Command, args []string) error { - // No Persistent* hooks are called for help, so we must initialize here. - if err := initializeDockerCli(dockerCli, flags, opts); err != nil { - return err - } - if len(args) > 0 { helpcmd, err := pluginmanager.PluginRunCommand(dockerCli, args[0], rootCmd) if err == nil { @@ -163,14 +119,9 @@ func tryRunPluginHelp(dockerCli command.Cli, ccmd *cobra.Command, cargs []string return helpcmd.Run() } -func setHelpFunc(dockerCli *command.DockerCli, cmd *cobra.Command, flags *pflag.FlagSet, opts *cliflags.ClientOptions) { +func setHelpFunc(dockerCli command.Cli, cmd *cobra.Command) { defaultHelpFunc := cmd.HelpFunc() cmd.SetHelpFunc(func(ccmd *cobra.Command, args []string) { - if err := initializeDockerCli(dockerCli, flags, opts); err != nil { - ccmd.Println(err) - return - } - // Add a stub entry for every plugin so they are // included in the help output and so that // `tryRunPluginHelp` can find them or if we fall @@ -205,7 +156,7 @@ func setHelpFunc(dockerCli *command.DockerCli, cmd *cobra.Command, flags *pflag. }) } -func setValidateArgs(dockerCli *command.DockerCli, cmd *cobra.Command, flags *pflag.FlagSet, opts *cliflags.ClientOptions) { +func setValidateArgs(dockerCli *command.DockerCli, cmd *cobra.Command) { // The Args is handled by ValidateArgs in cobra, which does not allows a pre-hook. // As a result, here we replace the existing Args validation func to a wrapper, // where the wrapper will check to see if the feature is supported or not. @@ -223,9 +174,6 @@ func setValidateArgs(dockerCli *command.DockerCli, cmd *cobra.Command, flags *pf cmdArgs := ccmd.Args ccmd.Args = func(cmd *cobra.Command, args []string) error { - if err := initializeDockerCli(dockerCli, flags, opts); err != nil { - return err - } if err := isSupported(cmd, dockerCli); err != nil { return err } @@ -234,14 +182,53 @@ func setValidateArgs(dockerCli *command.DockerCli, cmd *cobra.Command, flags *pf }) } -func initializeDockerCli(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *cliflags.ClientOptions) error { - if dockerCli.Client() != nil { - return nil +func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string) error { + plugincmd, err := pluginmanager.PluginRunCommand(dockerCli, subcommand, cmd) + if err != nil { + return err } - // when using --help, PersistentPreRun is not called, so initialization is needed. - // flags must be the top-level command flags, not cmd.Flags() - opts.Common.SetDefaultOptions(flags) - return dockerCli.Initialize(opts) + + if err := plugincmd.Run(); err != nil { + statusCode := 1 + exitErr, ok := err.(*exec.ExitError) + if !ok { + return err + } + if ws, ok := exitErr.Sys().(syscall.WaitStatus); ok { + statusCode = ws.ExitStatus() + } + return cli.StatusError{ + StatusCode: statusCode, + } + } + return nil +} + +func runDocker(dockerCli *command.DockerCli) error { + tcmd := newDockerCommand(dockerCli) + + cmd, args, err := tcmd.HandleGlobalFlags() + if err != nil { + return err + } + + if err := tcmd.Initialize(); err != nil { + return err + } + + if len(args) > 0 { + if _, _, err := cmd.Find(args); err != nil { + err := tryPluginRun(dockerCli, cmd, args[0]) + if !pluginmanager.IsNotFound(err) { + return err + } + // For plugin not found we fall through to + // cmd.Execute() which deals with reporting + // "command not found" in a consistent way. + } + } + + return cmd.Execute() } func main() { @@ -252,9 +239,7 @@ func main() { } logrus.SetOutput(dockerCli.Err()) - cmd := newDockerCommand(dockerCli) - - if err := cmd.Execute(); err != nil { + if err := runDocker(dockerCli); err != nil { if sterr, ok := err.(cli.StatusError); ok { if sterr.Status != "" { fmt.Fprintln(dockerCli.Err(), sterr.Status) diff --git a/cmd/docker/docker_test.go b/cmd/docker/docker_test.go index e0f23747c0..044daa12d3 100644 --- a/cmd/docker/docker_test.go +++ b/cmd/docker/docker_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "io" "io/ioutil" "os" "testing" @@ -16,10 +17,12 @@ import ( func TestClientDebugEnabled(t *testing.T) { defer debug.Disable() - cmd := newDockerCommand(&command.DockerCli{}) - cmd.Flags().Set("debug", "true") - - err := cmd.PersistentPreRunE(cmd, []string{}) + tcmd := newDockerCommand(&command.DockerCli{}) + tcmd.SetFlag("debug", "true") + cmd, _, err := tcmd.HandleGlobalFlags() + assert.NilError(t, err) + assert.NilError(t, tcmd.Initialize()) + err = cmd.PersistentPreRunE(cmd, []string{}) assert.NilError(t, err) assert.Check(t, is.Equal("1", os.Getenv("DEBUG"))) assert.Check(t, is.Equal(logrus.DebugLevel, logrus.GetLevel())) @@ -27,31 +30,37 @@ func TestClientDebugEnabled(t *testing.T) { var discard = ioutil.NopCloser(bytes.NewBuffer(nil)) -func TestExitStatusForInvalidSubcommandWithHelpFlag(t *testing.T) { - cli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(ioutil.Discard)) +func runCliCommand(t *testing.T, r io.ReadCloser, w io.Writer, args ...string) error { + t.Helper() + if r == nil { + r = discard + } + if w == nil { + w = ioutil.Discard + } + cli, err := command.NewDockerCli(command.WithInputStream(r), command.WithCombinedStreams(w)) assert.NilError(t, err) - cmd := newDockerCommand(cli) - cmd.SetArgs([]string{"help", "invalid"}) - err = cmd.Execute() + tcmd := newDockerCommand(cli) + tcmd.SetArgs(args) + cmd, _, err := tcmd.HandleGlobalFlags() + assert.NilError(t, err) + assert.NilError(t, tcmd.Initialize()) + return cmd.Execute() +} + +func TestExitStatusForInvalidSubcommandWithHelpFlag(t *testing.T) { + err := runCliCommand(t, nil, nil, "help", "invalid") assert.Error(t, err, "unknown help topic: invalid") } func TestExitStatusForInvalidSubcommand(t *testing.T) { - cli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(ioutil.Discard)) - assert.NilError(t, err) - cmd := newDockerCommand(cli) - cmd.SetArgs([]string{"invalid"}) - err = cmd.Execute() + err := runCliCommand(t, nil, nil, "invalid") assert.Check(t, is.ErrorContains(err, "docker: 'invalid' is not a docker command.")) } func TestVersion(t *testing.T) { var b bytes.Buffer - cli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b)) - assert.NilError(t, err) - cmd := newDockerCommand(cli) - cmd.SetArgs([]string{"--version"}) - err = cmd.Execute() + err := runCliCommand(t, nil, &b, "--version") assert.NilError(t, err) assert.Check(t, is.Contains(b.String(), "Docker version")) } diff --git a/e2e/cli-plugins/flags_test.go b/e2e/cli-plugins/flags_test.go new file mode 100644 index 0000000000..a958b18413 --- /dev/null +++ b/e2e/cli-plugins/flags_test.go @@ -0,0 +1,203 @@ +package cliplugins + +import ( + "testing" + + "gotest.tools/icmd" +) + +// TestRunGoodArgument ensures correct behaviour when running a valid plugin with an `--argument`. +func TestRunGoodArgument(t *testing.T) { + run, _, cleanup := prepare(t) + defer cleanup() + + res := icmd.RunCmd(run("helloworld", "--who", "Cleveland")) + res.Assert(t, icmd.Expected{ + ExitCode: 0, + Out: "Hello Cleveland!", + }) +} + +// TestClashWithGlobalArgs ensures correct behaviour when a plugin +// has an argument with the same name as one of the globals. +func TestClashWithGlobalArgs(t *testing.T) { + run, _, cleanup := prepare(t) + defer cleanup() + + for _, tc := range []struct { + name string + args []string + expectedOut, expectedErr string + }{ + { + name: "short-without-val", + args: []string{"-D"}, + expectedOut: "Hello World!", + expectedErr: "Plugin debug mode enabled", + }, + { + name: "long-without-val", + args: []string{"--debug"}, + expectedOut: "Hello World!", + expectedErr: "Plugin debug mode enabled", + }, + { + name: "short-with-val", + args: []string{"-c", "Christmas"}, + expectedOut: "Merry Christmas!", + expectedErr: "", + }, + { + name: "short-with-val", + args: []string{"--context", "Christmas"}, + expectedOut: "Merry Christmas!", + expectedErr: "", + }, + } { + t.Run(tc.name, func(t *testing.T) { + args := append([]string{"helloworld"}, tc.args...) + res := icmd.RunCmd(run(args...)) + res.Assert(t, icmd.Expected{ + ExitCode: 0, + Out: tc.expectedOut, + Err: tc.expectedErr, + }) + }) + } +} + +// TestUnknownGlobal checks that unknown globals report errors +func TestUnknownGlobal(t *testing.T) { + run, _, cleanup := prepare(t) + defer cleanup() + + for name, args := range map[string][]string{ + "no-val": {"--unknown", "helloworld"}, + "separate-val": {"--unknown", "foo", "helloworld"}, + "joined-val": {"--unknown=foo", "helloworld"}, + } { + t.Run(name, func(t *testing.T) { + res := icmd.RunCmd(run(args...)) + res.Assert(t, icmd.Expected{ + ExitCode: 125, + Out: icmd.None, + Err: "unknown flag: --unknown", + }) + }) + } +} + +// TestCliPluginsVersion checks that `-v` and friends DTRT +func TestCliPluginsVersion(t *testing.T) { + run, _, cleanup := prepare(t) + defer cleanup() + + for _, tc := range []struct { + name string + args []string + expCode int + expOut, expErr string + }{ + { + name: "global-version", + args: []string{"version"}, + expCode: 0, + expOut: "Client:\n Version:", + expErr: icmd.None, + }, + { + name: "global-version-flag", + args: []string{"--version"}, + expCode: 0, + expOut: "Docker version", + expErr: icmd.None, + }, + { + name: "global-short-version-flag", + args: []string{"-v"}, + expCode: 0, + expOut: "Docker version", + expErr: icmd.None, + }, + { + name: "global-with-unknown-arg", + args: []string{"version", "foo"}, + expCode: 1, + expOut: icmd.None, + expErr: `"docker version" accepts no arguments.`, + }, + { + name: "global-with-plugin-arg", + args: []string{"version", "helloworld"}, + expCode: 1, + expOut: icmd.None, + expErr: `"docker version" accepts no arguments.`, + }, + { + name: "global-version-flag-with-unknown-arg", + args: []string{"--version", "foo"}, + expCode: 0, + expOut: "Docker version", + expErr: icmd.None, + }, + { + name: "global-short-version-flag-with-unknown-arg", + args: []string{"-v", "foo"}, + expCode: 0, + expOut: "Docker version", + expErr: icmd.None, + }, + { + name: "global-version-flag-with-plugin", + args: []string{"--version", "helloworld"}, + expCode: 125, + expOut: icmd.None, + expErr: "unknown flag: --version", + }, + { + name: "global-short-version-flag-with-plugin", + args: []string{"-v", "helloworld"}, + expCode: 125, + expOut: icmd.None, + expErr: "unknown shorthand flag: 'v' in -v", + }, + { + name: "plugin-with-version", + args: []string{"helloworld", "version"}, + expCode: 0, + expOut: "Hello World!", + expErr: icmd.None, + }, + { + name: "plugin-with-version-flag", + args: []string{"helloworld", "--version"}, + expCode: 125, + expOut: icmd.None, + expErr: "unknown flag: --version", + }, + { + name: "plugin-with-short-version-flag", + args: []string{"helloworld", "-v"}, + expCode: 125, + expOut: icmd.None, + expErr: "unknown shorthand flag: 'v' in -v", + }, + { + name: "", + args: []string{}, + expCode: 0, + expOut: "", + expErr: "", + }, + } { + t.Run(tc.name, func(t *testing.T) { + res := icmd.RunCmd(run(tc.args...)) + res.Assert(t, icmd.Expected{ + ExitCode: tc.expCode, + Out: tc.expOut, + Err: tc.expErr, + }) + }) + } + +} diff --git a/e2e/cli-plugins/run_test.go b/e2e/cli-plugins/run_test.go index 8e6b49ca15..171ed81c83 100644 --- a/e2e/cli-plugins/run_test.go +++ b/e2e/cli-plugins/run_test.go @@ -144,18 +144,6 @@ func TestRunGoodSubcommand(t *testing.T) { }) } -// TestRunGoodArgument ensures correct behaviour when running a valid plugin with an `--argument`. -func TestRunGoodArgument(t *testing.T) { - run, _, cleanup := prepare(t) - defer cleanup() - - res := icmd.RunCmd(run("helloworld", "--who", "Cleveland")) - res.Assert(t, icmd.Expected{ - ExitCode: 0, - Out: "Hello Cleveland!", - }) -} - // TestHelpGoodSubcommand ensures correct behaviour when invoking help on a // valid plugin subcommand. A global argument is included to ensure it does not // interfere. diff --git a/e2e/cli-plugins/testdata/docker-help-helloworld.golden b/e2e/cli-plugins/testdata/docker-help-helloworld.golden index 59959637cb..24bc424bd7 100644 --- a/e2e/cli-plugins/testdata/docker-help-helloworld.golden +++ b/e2e/cli-plugins/testdata/docker-help-helloworld.golden @@ -4,7 +4,9 @@ Usage: docker helloworld [OPTIONS] COMMAND A basic Hello World plugin for tests Options: - --who string Who are we addressing? + -c, --context string Is it Christmas? + -D, --debug Enable debug + --who string Who are we addressing? Commands: apiversion Print the API version of the server