Merge pull request #5936 from thaJeztah/plugin_manager_cleanups

cli-plugins/manager: minor cleanups and refactoring

Author: thaJeztah

cli-plugins/manager/hooks.go

@@ -64,13 +64,10 @@ func invokeAndCollectHooks(ctx context.Context, cfg *configfile.ConfigFile, root
 		return nil
 	}
 
-	pluginDirs, err := getPluginDirs(cfg)
-	if err != nil {
-		return nil
-	}
+	pluginDirs := getPluginDirs(cfg)
 	nextSteps := make([]string, 0, len(pluginsCfg))
-	for pluginName, cfg := range pluginsCfg {
-		match, ok := pluginMatch(cfg, subCmdStr)
+	for pluginName, pluginCfg := range pluginsCfg {
+		match, ok := pluginMatch(pluginCfg, subCmdStr)
 		if !ok {
 			continue
 		}

cli-plugins/manager/manager.go

@@ -61,20 +61,16 @@ func IsNotFound(err error) bool {
 // 3. Platform-specific defaultSystemPluginDirs.
 //
 // [ConfigFile.CLIPluginsExtraDirs]: https://pkg.go.dev/github.com/docker/[email protected]+incompatible/cli/config/configfile#ConfigFile.CLIPluginsExtraDirs
-func getPluginDirs(cfg *configfile.ConfigFile) ([]string, error) {
+func getPluginDirs(cfg *configfile.ConfigFile) []string {
 	var pluginDirs []string
 
 	if cfg != nil {
 		pluginDirs = append(pluginDirs, cfg.CLIPluginsExtraDirs...)
 	}
-	pluginDir, err := config.Path("cli-plugins")
-	if err != nil {
-		return nil, err
-	}
-
+	pluginDir := filepath.Join(config.Dir(), "cli-plugins")
 	pluginDirs = append(pluginDirs, pluginDir)
 	pluginDirs = append(pluginDirs, defaultSystemPluginDirs...)
-	return pluginDirs, nil
+	return pluginDirs
 }
 
 func addPluginCandidatesFromDir(res map[string][]string, d string) {
@@ -116,10 +112,7 @@ func listPluginCandidates(dirs []string) map[string][]string {
 
 // GetPlugin returns a plugin on the system by its name
 func GetPlugin(name string, dockerCLI config.Provider, rootcmd *cobra.Command) (*Plugin, error) {
-	pluginDirs, err := getPluginDirs(dockerCLI.ConfigFile())
-	if err != nil {
-		return nil, err
-	}
+	pluginDirs := getPluginDirs(dockerCLI.ConfigFile())
 	return getPlugin(name, pluginDirs, rootcmd)
 }
 
@@ -145,16 +138,20 @@ func getPlugin(name string, pluginDirs []string, rootcmd *cobra.Command) (*Plugi
 
 // ListPlugins produces a list of the plugins available on the system
 func ListPlugins(dockerCli config.Provider, rootcmd *cobra.Command) ([]Plugin, error) {
-	pluginDirs, err := getPluginDirs(dockerCli.ConfigFile())
-	if err != nil {
-		return nil, err
-	}
-
+	pluginDirs := getPluginDirs(dockerCli.ConfigFile())
 	candidates := listPluginCandidates(pluginDirs)
+	if len(candidates) == 0 {
+		return nil, nil
+	}
 
 	var plugins []Plugin
 	var mu sync.Mutex
-	eg, _ := errgroup.WithContext(context.TODO())
+	ctx := rootcmd.Context()
+	if ctx == nil {
+		// Fallback, mostly for tests that pass a bare cobra.command
+		ctx = context.Background()
+	}
+	eg, _ := errgroup.WithContext(ctx)
 	cmds := rootcmd.Commands()
 	for _, paths := range candidates {
 		func(paths []string) {
@@ -202,10 +199,7 @@ func PluginRunCommand(dockerCli config.Provider, name string, rootcmd *cobra.Com
 		return nil, errPluginNotFound(name)
 	}
 	exename := addExeSuffix(metadata.NamePrefix + name)
-	pluginDirs, err := getPluginDirs(dockerCli.ConfigFile())
-	if err != nil {
-		return nil, err
-	}
+	pluginDirs := getPluginDirs(dockerCli.ConfigFile())
 
 	for _, d := range pluginDirs {
 		path := filepath.Join(d, exename)

cli-plugins/manager/manager_test.go

@@ -1,6 +1,7 @@
 package manager
 
 import (
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -81,6 +82,12 @@ func TestListPluginCandidates(t *testing.T) {
 	assert.DeepEqual(t, candidates, exp)
 }
 
+func TestListPluginCandidatesEmpty(t *testing.T) {
+	tmpDir := t.TempDir()
+	candidates := listPluginCandidates([]string{tmpDir, filepath.Join(tmpDir, "no-such-dir")})
+	assert.Assert(t, len(candidates) == 0)
+}
+
 // Regression test for https://github.com/docker/cli/issues/5643.
 // Check that inaccessible directories that come before accessible ones are ignored
 // and do not prevent the latter from being processed.
@@ -166,14 +173,11 @@ func TestErrPluginNotFound(t *testing.T) {
 func TestGetPluginDirs(t *testing.T) {
 	cli := test.NewFakeCli(nil)
 
-	pluginDir, err := config.Path("cli-plugins")
-	assert.NilError(t, err)
+	pluginDir := filepath.Join(config.Dir(), "cli-plugins")
 	expected := append([]string{pluginDir}, defaultSystemPluginDirs...)
 
-	var pluginDirs []string
-	pluginDirs, err = getPluginDirs(cli.ConfigFile())
+	pluginDirs := getPluginDirs(cli.ConfigFile())
 	assert.Equal(t, strings.Join(expected, ":"), strings.Join(pluginDirs, ":"))
-	assert.NilError(t, err)
 
 	extras := []string{
 		"foo", "bar", "baz",
@@ -182,7 +186,6 @@ func TestGetPluginDirs(t *testing.T) {
 	cli.SetConfigFile(&configfile.ConfigFile{
 		CLIPluginsExtraDirs: extras,
 	})
-	pluginDirs, err = getPluginDirs(cli.ConfigFile())
+	pluginDirs = getPluginDirs(cli.ConfigFile())
 	assert.DeepEqual(t, expected, pluginDirs)
-	assert.NilError(t, err)
 }