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 <ijc@docker.com>

cli-plugins/examples/helloworld/main.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"fmt"
 
 	"github.com/docker/cli/cli-plugins/manager"
@@ -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
 	},
 		manager.Metadata{

cli-plugins/plugin/plugin.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"sync"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli-plugins/manager"
@@ -42,29 +43,53 @@ func Run(makeCmd func(command.Cli) *cobra.Command, meta manager.Metadata) {
 	}
 }
 
-func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager.Metadata) *cobra.Command {
+// options encapsulates the ClientOptions and FlagSet constructed by
-	var (
+// `newPluginCommand` such that they can be finalized by our
-		opts  *cliflags.ClientOptions
+// `PersistentPreRunE`. This is necessary because otherwise a plugin's
-		flags *pflag.FlagSet
+// 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 manager.Metadata) *cobra.Command {
 	name := plugin.Use
 	fullname := manager.NamePrefix + name
 
 	cmd := &cobra.Command{
-		Use:              fmt.Sprintf("docker [OPTIONS] %s [ARG...]", name),
+		Use:                   fmt.Sprintf("docker [OPTIONS] %s [ARG...]", name),
-		Short:            fullname + " is a Docker CLI plugin",
+		Short:                 fullname + " is a Docker CLI plugin",
-		SilenceUsage:     true,
+		SilenceUsage:          true,
-		SilenceErrors:    true,
+		SilenceErrors:         true,
-		TraverseChildren: true,
+		TraverseChildren:      true,
-		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+		PersistentPreRunE:     PersistentPreRunE,
-			// flags must be the top-level command flags, not cmd.Flags()
-			opts.Common.SetDefaultOptions(flags)
-			return dockerCli.Initialize(opts)
-		},
 		DisableFlagsInUseLine: true,
 	}
-	opts, flags = cli.SetupPluginRootCommand(cmd)
+	opts, flags := cli.SetupPluginRootCommand(cmd)
 
 	cmd.SetOutput(dockerCli.Out())
 
@@ -75,6 +100,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
 }
 

e2e/cli-plugins/run_test.go

@@ -170,3 +170,15 @@ func TestGoodSubcommandHelp(t *testing.T) {
 	golden.Assert(t, res.Stdout(), "docker-help-helloworld-goodbye.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) {
+	run, cleanup := prepare(t)
+	defer cleanup()
+
+	res := icmd.RunCmd(run("helloworld", "apiversion"))
+	res.Assert(t, icmd.Success)
+	assert.Assert(t, res.Stdout() != "")
+	assert.Assert(t, is.Equal(res.Stderr(), ""))
+}

e2e/cli-plugins/testdata/docker-help-helloworld.golden

@@ -4,6 +4,7 @@ Usage:	docker helloworld COMMAND
 A basic Hello World plugin for tests
 
 Commands:
+  apiversion  Print the API version of the server
   goodbye     Say Goodbye instead of Hello
 
 Run 'docker helloworld COMMAND --help' for more information on a command.