Merge pull request #5651 from vvoland/fix-plugin-paths

cli-plugins: Fix searching inaccessible directories

cli-plugins/manager/manager.go

@@ -75,10 +75,12 @@ func getPluginDirs(cfg *configfile.ConfigFile) ([]string, error) {
 	return pluginDirs, nil
 }
 
-func addPluginCandidatesFromDir(res map[string][]string, d string) error {
+func addPluginCandidatesFromDir(res map[string][]string, d string) {
 	dentries, err := os.ReadDir(d)
+	// Silently ignore any directories which we cannot list (e.g. due to
+	// permissions or anything else) or which is not a directory
 	if err != nil {
-		return err
+		return
 	}
 	for _, dentry := range dentries {
 		switch dentry.Type() & os.ModeType {
@@ -99,28 +101,15 @@ func addPluginCandidatesFromDir(res map[string][]string, d string) error {
 		}
 		res[name] = append(res[name], filepath.Join(d, dentry.Name()))
 	}
-	return nil
 }
 
 // listPluginCandidates returns a map from plugin name to the list of (unvalidated) Candidates. The list is in descending order of priority.
-func listPluginCandidates(dirs []string) (map[string][]string, error) {
+func listPluginCandidates(dirs []string) map[string][]string {
 	result := make(map[string][]string)
 	for _, d := range dirs {
-		// Silently ignore any directories which we cannot
-		// Stat (e.g. due to permissions or anything else) or
-		// which is not a directory.
-		if fi, err := os.Stat(d); err != nil || !fi.IsDir() {
-			continue
-		}
-		if err := addPluginCandidatesFromDir(result, d); err != nil {
-			// Silently ignore paths which don't exist.
-			if os.IsNotExist(err) {
-				continue
-			}
-			return nil, err // Or return partial result?
-		}
+		addPluginCandidatesFromDir(result, d)
 	}
-	return result, nil
+	return result
 }
 
 // GetPlugin returns a plugin on the system by its name
@@ -130,11 +119,7 @@ func GetPlugin(name string, dockerCli command.Cli, rootcmd *cobra.Command) (*Plu
 		return nil, err
 	}
 
-	candidates, err := listPluginCandidates(pluginDirs)
-	if err != nil {
-		return nil, err
-	}
-
+	candidates := listPluginCandidates(pluginDirs)
 	if paths, ok := candidates[name]; ok {
 		if len(paths) == 0 {
 			return nil, errPluginNotFound(name)
@@ -160,10 +145,7 @@ func ListPlugins(dockerCli command.Cli, rootcmd *cobra.Command) ([]Plugin, error
 		return nil, err
 	}
 
-	candidates, err := listPluginCandidates(pluginDirs)
-	if err != nil {
-		return nil, err
-	}
+	candidates := listPluginCandidates(pluginDirs)
 
 	var plugins []Plugin
 	var mu sync.Mutex

cli-plugins/manager/manager_test.go

@@ -51,8 +51,7 @@ func TestListPluginCandidates(t *testing.T) {
 		dirs = append(dirs, dir.Join(d))
 	}
 
-	candidates, err := listPluginCandidates(dirs)
-	assert.NilError(t, err)
+	candidates := listPluginCandidates(dirs)
 	exp := map[string][]string{
 		"plugin1": {
 			dir.Join("plugins1", "docker-plugin1"),
@@ -82,6 +81,29 @@ func TestListPluginCandidates(t *testing.T) {
 	assert.DeepEqual(t, candidates, exp)
 }
 
+// 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.
+func TestListPluginCandidatesInaccesibleDir(t *testing.T) {
+	dir := fs.NewDir(t, t.Name(),
+		fs.WithDir("no-perm", fs.WithMode(0)),
+		fs.WithDir("plugins",
+			fs.WithFile("docker-buildx", ""),
+		),
+	)
+	defer dir.Remove()
+
+	candidates := listPluginCandidates([]string{
+		dir.Join("no-perm"),
+		dir.Join("plugins"),
+	})
+	assert.DeepEqual(t, candidates, map[string][]string{
+		"buildx": {
+			dir.Join("plugins", "docker-buildx"),
+		},
+	})
+}
+
 func TestGetPlugin(t *testing.T) {
 	dir := fs.NewDir(t, t.Name(),
 		fs.WithFile("docker-bbb", `