cli-plugins: fix when plugin does not use PersistentPreRun* hooks

This regressed in 3af168c7df ("Ensure plugins can use `PersistentPreRunE`
again.") but this wasn't noticed because the helloworld test plugin has it's
own `PersistentPreRunE` which has the effect of deferring the resolution of the
global variable. In the case where the hook isn't used the variable is resolved
during `newPluginCommand` which is before the global variable was set.

Initialize the plugin command with a stub function wrapping the call to the
(global) hook, this defers resolving the variable until after it has been set,
otherwise the initial value (`nil`) is used in the struct.

Signed-off-by: Ian Campbell <ijc@docker.com>

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),
+		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,
-		PersistentPreRunE:     PersistentPreRunE,
+		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,
 	}

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",
+		})
+}

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"))
+	var apiversion string
-	res.Assert(t, icmd.Success)
+	t.Run("withhook", func(t *testing.T) {
-	assert.Assert(t, res.Stdout() != "")
+		res := icmd.RunCmd(run("helloworld", "--pre-run", "apiversion"))
-	assert.Assert(t, is.Equal(res.Stderr(), "Plugin PersistentPreRunE called"))
+		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.