cli-plugins/manager: ignore broken symlinks

Before this patch, a broken symlink would print a warning;

    docker info > /dev/null
    WARNING: Plugin "/Users/thajeztah/.docker/cli-plugins/docker-feedback" is not valid: failed to fetch metadata: fork/exec /Users/thajeztah/.docker/cli-plugins/docker-feedback: no such file or directory

After this patch, such symlinks are ignored:

    docker info > /dev/null

With debug enabled, we don't ignore the faulty plugin, which will
make the warning shown on docker info;

    mkdir -p ~/.docker/cli-plugins
    ln -s nosuchplugin ~/.docker/cli-plugins/docker-brokenplugin
    docker --debug info
    Client:
     Version:    29.0.0-dev
     Context:    default
     Debug Mode: true
     Plugins:
      buildx: Docker Buildx (Docker Inc.)
        Version:  v0.25.0
        Path:     /usr/libexec/docker/cli-plugins/docker-buildx
    WARNING: Plugin "/Users/thajeztah/.docker/cli-plugins/docker-brokenplugin" is not valid: failed to fetch metadata: fork/exec /Users/thajeztah/.docker/cli-plugins/docker-brokenplugin: no such file or directory

    # ...

We should als consider passing a "seen" map to de-duplicate entries.
Entries can be either a direct symlink or in a symlinked path (for
which we can filepath.EvalSymlinks). We need to benchmark the overhead
of resolving the symlink vs possibly calling the plugin (to get their
metadata) further down the line.

Signed-off-by: Sebastiaan van Stijn <[email protected]>

Author: thaJeztah

cli-plugins/manager/manager.go

@@ -2,6 +2,7 @@ package manager
 
 import (
 	"context"
+	"errors"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -13,6 +14,7 @@ import (
 	"github.com/docker/cli/cli-plugins/metadata"
 	"github.com/docker/cli/cli/config"
 	"github.com/docker/cli/cli/config/configfile"
+	"github.com/docker/cli/cli/debug"
 	"github.com/fvbommel/sortorder"
 	"github.com/spf13/cobra"
 	"golang.org/x/sync/errgroup"
@@ -57,9 +59,17 @@ func addPluginCandidatesFromDir(res map[string][]string, d string) {
 		return
 	}
 	for _, dentry := range dentries {
-		switch dentry.Type() & os.ModeType { //nolint:exhaustive,nolintlint // no need to include all possible file-modes in this list
-		case 0, os.ModeSymlink:
-			// Regular file or symlink, keep going
+		switch mode := dentry.Type() & os.ModeType; mode { //nolint:exhaustive,nolintlint // no need to include all possible file-modes in this list
+		case os.ModeSymlink:
+			if !debug.IsEnabled() {
+				// Skip broken symlinks unless debug is enabled. With debug
+				// enabled, this will print a warning in "docker info".
+				if _, err := os.Stat(filepath.Join(d, dentry.Name())); errors.Is(err, os.ErrNotExist) {
+					continue
+				}
+			}
+		case 0:
+			// Regular file, keep going
 		default:
 			// Something else, ignore.
 			continue

cli-plugins/manager/manager_test.go

@@ -38,7 +38,7 @@ func TestListPluginCandidates(t *testing.T) {
 			"plugins3-target", // Will be referenced as a symlink from below
 			fs.WithFile("docker-plugin1", ""),
 			fs.WithDir("ignored3"),
-			fs.WithSymlink("docker-brokensymlink", "broken"),           // A broken symlink is still a candidate (but would fail tests later)
+			fs.WithSymlink("docker-brokensymlink", "broken"),           // A broken symlink is ignored
 			fs.WithFile("non-plugin-symlinked", ""),                    // This shouldn't appear, but ...
 			fs.WithSymlink("docker-symlinked", "non-plugin-symlinked"), // ... this link to it should.
 		),
@@ -72,9 +72,6 @@ func TestListPluginCandidates(t *testing.T) {
 		"hardlink2": {
 			dir.Join("plugins2", "docker-hardlink2"),
 		},
-		"brokensymlink": {
-			dir.Join("plugins3", "docker-brokensymlink"),
-		},
 		"symlinked": {
 			dir.Join("plugins3", "docker-symlinked"),
 		},