[23.0 backport] improve and load plugin command stubs when required

cli-plugins/manager/candidate_test.go

@@ -74,7 +74,7 @@ func TestValidateCandidate(t *testing.T) {
 		{name: "experimental + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental}},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			p, err := newPlugin(tc.c, fakeroot)
+			p, err := newPlugin(tc.c, fakeroot.Commands())
 			if tc.err != "" {
 				assert.ErrorContains(t, err, tc.err)
 			} else if tc.invalid != "" {

cli-plugins/manager/cobra.go

@@ -3,6 +3,7 @@ package manager
 import (
 	"fmt"
 	"os"
+	"sync"
 
 	"github.com/docker/cli/cli/command"
 	"github.com/spf13/cobra"
@@ -31,64 +32,69 @@ const (
 	CommandAnnotationPluginInvalid = "com.docker.cli.plugin-invalid"
 )
 
+var pluginCommandStubsOnce sync.Once
+
 // AddPluginCommandStubs adds a stub cobra.Commands for each valid and invalid
 // plugin. The command stubs will have several annotations added, see
 // `CommandAnnotationPlugin*`.
-func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) error {
-	plugins, err := ListPlugins(dockerCli, rootCmd)
-	if err != nil {
-		return err
-	}
-	for _, p := range plugins {
-		p := p
-		vendor := p.Vendor
-		if vendor == "" {
-			vendor = "unknown"
-		}
-		annotations := map[string]string{
-			CommandAnnotationPlugin:        "true",
-			CommandAnnotationPluginVendor:  vendor,
-			CommandAnnotationPluginVersion: p.Version,
+func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) (err error) {
+	pluginCommandStubsOnce.Do(func() {
+		var plugins []Plugin
+		plugins, err = ListPlugins(dockerCli, rootCmd)
+		if err != nil {
+			return
 		}
-		if p.Err != nil {
-			annotations[CommandAnnotationPluginInvalid] = p.Err.Error()
-		}
-		rootCmd.AddCommand(&cobra.Command{
-			Use:                p.Name,
-			Short:              p.ShortDescription,
-			Run:                func(_ *cobra.Command, _ []string) {},
-			Annotations:        annotations,
-			DisableFlagParsing: true,
-			RunE: func(cmd *cobra.Command, args []string) error {
-				flags := rootCmd.PersistentFlags()
-				flags.SetOutput(nil)
-				err := flags.Parse(args)
-				if err != nil {
-					return err
-				}
-				if flags.Changed("help") {
-					cmd.HelpFunc()(rootCmd, args)
-					return nil
-				}
-				return fmt.Errorf("docker: '%s' is not a docker command.\nSee 'docker --help'", cmd.Name())
-			},
-			ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-				// Delegate completion to plugin
-				cargs := []string{p.Path, cobra.ShellCompRequestCmd, p.Name}
-				cargs = append(cargs, args...)
-				cargs = append(cargs, toComplete)
-				os.Args = cargs
-				runCommand, err := PluginRunCommand(dockerCli, p.Name, cmd)
-				if err != nil {
+		for _, p := range plugins {
+			p := p
+			vendor := p.Vendor
+			if vendor == "" {
+				vendor = "unknown"
+			}
+			annotations := map[string]string{
+				CommandAnnotationPlugin:        "true",
+				CommandAnnotationPluginVendor:  vendor,
+				CommandAnnotationPluginVersion: p.Version,
+			}
+			if p.Err != nil {
+				annotations[CommandAnnotationPluginInvalid] = p.Err.Error()
+			}
+			rootCmd.AddCommand(&cobra.Command{
+				Use:                p.Name,
+				Short:              p.ShortDescription,
+				Run:                func(_ *cobra.Command, _ []string) {},
+				Annotations:        annotations,
+				DisableFlagParsing: true,
+				RunE: func(cmd *cobra.Command, args []string) error {
+					flags := rootCmd.PersistentFlags()
+					flags.SetOutput(nil)
+					perr := flags.Parse(args)
+					if perr != nil {
+						return err
+					}
+					if flags.Changed("help") {
+						cmd.HelpFunc()(rootCmd, args)
+						return nil
+					}
+					return fmt.Errorf("docker: '%s' is not a docker command.\nSee 'docker --help'", cmd.Name())
+				},
+				ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+					// Delegate completion to plugin
+					cargs := []string{p.Path, cobra.ShellCompRequestCmd, p.Name}
+					cargs = append(cargs, args...)
+					cargs = append(cargs, toComplete)
+					os.Args = cargs
+					runCommand, runErr := PluginRunCommand(dockerCli, p.Name, cmd)
+					if runErr != nil {
+						return nil, cobra.ShellCompDirectiveError
+					}
+					runErr = runCommand.Run()
+					if runErr == nil {
+						os.Exit(0) // plugin already rendered complete data
+					}
 					return nil, cobra.ShellCompDirectiveError
-				}
-				err = runCommand.Run()
-				if err == nil {
-					os.Exit(0) // plugin already rendered complete data
-				}
-				return nil, cobra.ShellCompDirectiveError
-			},
-		})
-	}
-	return nil
+				},
+			})
+		}
+	})
+	return err
 }

cli-plugins/manager/manager.go

@@ -1,15 +1,18 @@
 package manager
 
 import (
+	"context"
 	"os"
 	"path/filepath"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/config"
 	"github.com/fvbommel/sortorder"
 	"github.com/spf13/cobra"
+	"golang.org/x/sync/errgroup"
 	exec "golang.org/x/sys/execabs"
 )
 
@@ -120,7 +123,7 @@ func GetPlugin(name string, dockerCli command.Cli, rootcmd *cobra.Command) (*Plu
 			return nil, errPluginNotFound(name)
 		}
 		c := &candidate{paths[0]}
-		p, err := newPlugin(c, rootcmd)
+		p, err := newPlugin(c, rootcmd.Commands())
 		if err != nil {
 			return nil, err
 		}
@@ -146,19 +149,32 @@ func ListPlugins(dockerCli command.Cli, rootcmd *cobra.Command) ([]Plugin, error
 	}
 
 	var plugins []Plugin
+	var mu sync.Mutex
+	eg, _ := errgroup.WithContext(context.TODO())
+	cmds := rootcmd.Commands()
 	for _, paths := range candidates {
-		if len(paths) == 0 {
-			continue
-		}
-		c := &candidate{paths[0]}
-		p, err := newPlugin(c, rootcmd)
-		if err != nil {
-			return nil, err
-		}
-		if !IsNotFound(p.Err) {
-			p.ShadowedPaths = paths[1:]
-			plugins = append(plugins, p)
-		}
+		func(paths []string) {
+			eg.Go(func() error {
+				if len(paths) == 0 {
+					return nil
+				}
+				c := &candidate{paths[0]}
+				p, err := newPlugin(c, cmds)
+				if err != nil {
+					return err
+				}
+				if !IsNotFound(p.Err) {
+					p.ShadowedPaths = paths[1:]
+					mu.Lock()
+					defer mu.Unlock()
+					plugins = append(plugins, p)
+				}
+				return nil
+			})
+		}(paths)
+	}
+	if err := eg.Wait(); err != nil {
+		return nil, err
 	}
 
 	sort.Slice(plugins, func(i, j int) bool {
@@ -199,7 +215,7 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command
 		}
 
 		c := &candidate{path: path}
-		plugin, err := newPlugin(c, rootcmd)
+		plugin, err := newPlugin(c, rootcmd.Commands())
 		if err != nil {
 			return nil, err
 		}

cli-plugins/manager/plugin.go

@@ -31,7 +31,7 @@ type Plugin struct {
 // is set, and is always a `pluginError`, but the `Plugin` is still
 // returned with no error. An error is only returned due to a
 // non-recoverable error.
-func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
+func newPlugin(c Candidate, cmds []*cobra.Command) (Plugin, error) {
 	path := c.Path()
 	if path == "" {
 		return Plugin{}, errors.New("plugin candidate path cannot be empty")
@@ -62,22 +62,20 @@ func newPlugin(c Candidate, rootcmd *cobra.Command) (Plugin, error) {
 		return p, nil
 	}
 
-	if rootcmd != nil {
-		for _, cmd := range rootcmd.Commands() {
-			// Ignore conflicts with commands which are
-			// just plugin stubs (i.e. from a previous
-			// call to AddPluginCommandStubs).
-			if IsPluginCommand(cmd) {
-				continue
-			}
-			if cmd.Name() == p.Name {
-				p.Err = NewPluginError("plugin %q duplicates builtin command", p.Name)
-				return p, nil
-			}
-			if cmd.HasAlias(p.Name) {
-				p.Err = NewPluginError("plugin %q duplicates an alias of builtin command %q", p.Name, cmd.Name())
-				return p, nil
-			}
+	for _, cmd := range cmds {
+		// Ignore conflicts with commands which are
+		// just plugin stubs (i.e. from a previous
+		// call to AddPluginCommandStubs).
+		if IsPluginCommand(cmd) {
+			continue
+		}
+		if cmd.Name() == p.Name {
+			p.Err = NewPluginError("plugin %q duplicates builtin command", p.Name)
+			return p, nil
+		}
+		if cmd.HasAlias(p.Name) {
+			p.Err = NewPluginError("plugin %q duplicates an alias of builtin command %q", p.Name, cmd.Name())
+			return p, nil
 		}
 	}
 

cli/cobra.go

@@ -204,6 +204,16 @@ func DisableFlagsInUseLine(cmd *cobra.Command) {
 	})
 }
 
+// HasCompletionArg returns true if a cobra completion arg request is found.
+func HasCompletionArg(args []string) bool {
+	for _, arg := range args {
+		if arg == cobra.ShellCompRequestCmd || arg == cobra.ShellCompNoDescRequestCmd {
+			return true
+		}
+	}
+	return false
+}
+
 var helpCommand = &cobra.Command{
 	Use:               "help [command]",
 	Short:             "Help about the command",

cmd/docker/docker.go

@@ -133,13 +133,20 @@ func tryRunPluginHelp(dockerCli command.Cli, ccmd *cobra.Command, cargs []string
 func setHelpFunc(dockerCli command.Cli, cmd *cobra.Command) {
 	defaultHelpFunc := cmd.HelpFunc()
 	cmd.SetHelpFunc(func(ccmd *cobra.Command, args []string) {
-		if pluginmanager.IsPluginCommand(ccmd) {
+		if err := pluginmanager.AddPluginCommandStubs(dockerCli, ccmd.Root()); err != nil {
+			ccmd.Println(err)
+			return
+		}
+
+		if len(args) >= 1 {
 			err := tryRunPluginHelp(dockerCli, ccmd, args)
+			if err == nil {
+				return
+			}
 			if !pluginmanager.IsNotFound(err) {
 				ccmd.Println(err)
+				return
 			}
-			cmd.PrintErrf("unknown help topic: %v\n", ccmd.Name())
-			return
 		}
 
 		if err := isSupported(ccmd, dockerCli); err != nil {
@@ -227,9 +234,14 @@ func runDocker(dockerCli *command.DockerCli) error {
 		return err
 	}
 
-	err = pluginmanager.AddPluginCommandStubs(dockerCli, cmd)
-	if err != nil {
-		return err
+	if cli.HasCompletionArg(args) {
+		// We add plugin command stubs early only for completion. We don't
+		// want to add them for normal command execution as it would cause
+		// a significant performance hit.
+		err = pluginmanager.AddPluginCommandStubs(dockerCli, cmd)
+		if err != nil {
+			return err
+		}
 	}
 
 	if len(args) > 0 {

e2e/cli-plugins/run_test.go

@@ -83,7 +83,7 @@ func TestHelpBad(t *testing.T) {
 
 	res := icmd.RunCmd(run("help", "badmeta"))
 	res.Assert(t, icmd.Expected{
-		ExitCode: 0,
+		ExitCode: 1,
 		Out:      icmd.None,
 	})
 	golden.Assert(t, res.Stderr(), "docker-help-badmeta-err.golden")
@@ -110,8 +110,8 @@ func TestBadHelp(t *testing.T) {
 	res.Assert(t, icmd.Expected{
 		ExitCode: 0,
 		// This should be identical to the --help case above
-		Out: usage,
-		Err: shortHFlagDeprecated,
+		Out: shortHFlagDeprecated + usage,
+		Err: icmd.None,
 	})
 }
 

vendor.mod

@@ -37,6 +37,7 @@ require (
 	github.com/theupdateframework/notary v0.7.1-0.20210315103452-bf96a202a09a
 	github.com/tonistiigi/go-rosetta v0.0.0-20200727161949-f79598599c5d
 	github.com/xeipuuv/gojsonschema v1.2.0
+	golang.org/x/sync v0.1.0
 	golang.org/x/sys v0.4.0
 	golang.org/x/term v0.4.0
 	golang.org/x/text v0.6.0

vendor.sum

@@ -524,6 +524,8 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
+golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=