From bebdb71f20b635b3a3bec1cce95cad94dca7a4f9 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Mon, 17 Dec 2018 15:55:38 +0000 Subject: [PATCH] Allow plugins to make use of the cobra `PersistentPreRun` hooks. Previously a plugin which used these hooks would overwrite the top-level plugin command's use of this hook, resulting in the dockerCli object not being fully initialised. Provide a function which plugins can use to chain to the required behaviour. This required some fairly ugly arrangements to preserve state (which was previously in-scope in `newPluginCOmmand`) to be used by the new function. Signed-off-by: Ian Campbell --- cli-plugins/examples/helloworld/main.go | 20 +++++++- cli-plugins/plugin/plugin.go | 62 ++++++++++++++++++------- e2e/cli-plugins/run_test.go | 9 ++++ 3 files changed, 74 insertions(+), 17 deletions(-) diff --git a/cli-plugins/examples/helloworld/main.go b/cli-plugins/examples/helloworld/main.go index 875e0930c256..33ec262516c6 100644 --- a/cli-plugins/examples/helloworld/main.go +++ b/cli-plugins/examples/helloworld/main.go @@ -1,6 +1,7 @@ package main import ( + "context" "fmt" cliplugins "github.com/docker/cli/cli-plugins" @@ -18,16 +19,33 @@ func main() { fmt.Fprintln(dockerCli.Out(), "Goodbye World!") }, } + apiversion := &cobra.Command{ + Use: "apiversion", + Short: "Print the API version of the server", + RunE: func(_ *cobra.Command, _ []string) error { + cli := dockerCli.Client() + ping, err := cli.Ping(context.Background()) + if err != nil { + return err + } + fmt.Println(ping.APIVersion) + return nil + }, + } 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, Run: func(cmd *cobra.Command, args []string) { fmt.Fprintln(dockerCli.Out(), "Hello World!") }, } - cmd.AddCommand(goodbye) + cmd.AddCommand(goodbye, apiversion) return cmd }, cliplugins.Metadata{ diff --git a/cli-plugins/plugin/plugin.go b/cli-plugins/plugin/plugin.go index a5ea145c3a53..57872b6130bb 100644 --- a/cli-plugins/plugin/plugin.go +++ b/cli-plugins/plugin/plugin.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "sync" "github.com/docker/cli/cli" cliplugins "github.com/docker/cli/cli-plugins" @@ -44,29 +45,53 @@ func Run(makeCmd func(command.Cli) *cobra.Command, meta cliplugins.Metadata) { } } -func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta cliplugins.Metadata) *cobra.Command { - var ( - opts *cliflags.ClientOptions - flags *pflag.FlagSet - ) +// 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 { + 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) + }) + return err +} +func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta cliplugins.Metadata) *cobra.Command { name := plugin.Use fullname := cliplugins.NamePrefix + name cmd := &cobra.Command{ - Use: "docker" + " [OPTIONS] " + name + " [ARG...]", - Short: fullname + " is a Docker CLI plugin", - SilenceUsage: true, - SilenceErrors: true, - TraverseChildren: true, - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - // flags must be the top-level command flags, not cmd.Flags() - opts.Common.SetDefaultOptions(flags) - return dockerCli.Initialize(opts) - }, + Use: "docker" + " [OPTIONS] " + name + " [ARG...]", + Short: fullname + " is a Docker CLI plugin", + SilenceUsage: true, + SilenceErrors: true, + TraverseChildren: true, + PersistentPreRunE: PersistentPreRunE, DisableFlagsInUseLine: true, } - opts, flags = cli.SetupPluginRootCommand(cmd) + opts, flags := cli.SetupPluginRootCommand(cmd) cmd.SetOutput(dockerCli.Out()) @@ -77,6 +102,11 @@ func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta cli.DisableFlagsInUseLine(cmd) + options.init.Do(func() { + options.opts = opts + options.flags = flags + options.dockerCli = dockerCli + }) return cmd } diff --git a/e2e/cli-plugins/run_test.go b/e2e/cli-plugins/run_test.go index a31b9ccca8a7..6d8f46537d17 100644 --- a/e2e/cli-plugins/run_test.go +++ b/e2e/cli-plugins/run_test.go @@ -104,3 +104,12 @@ func TestGoodHelp(t *testing.T) { golden.Assert(t, res.Stdout(), "docker-help-helloworld.golden") assert.Assert(t, is.Equal(res.Stderr(), "")) } + +// TestCliInitialized tests the code paths which ensure that the Cli +// object is initialized even if the plugin uses PersistentRunE +func TestCliInitialized(t *testing.T) { + res := icmd.RunCmd(icmd.Command("docker", "helloworld", "apiversion")) + res.Assert(t, icmd.Success) + assert.Assert(t, res.Stdout() != "") + assert.Assert(t, is.Equal(res.Stderr(), "")) +}