From 9b2f8314523059e3db1b5909ced528de0d6af34b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 11 Mar 2025 11:27:20 +0100 Subject: [PATCH] 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 --- cli-plugins/manager/manager.go | 16 +++++++++++++--- cli-plugins/manager/manager_test.go | 5 +---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/cli-plugins/manager/manager.go b/cli-plugins/manager/manager.go index 009ea1795..48a0db341 100644 --- a/cli-plugins/manager/manager.go +++ b/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 diff --git a/cli-plugins/manager/manager_test.go b/cli-plugins/manager/manager_test.go index d1223ed4e..16c43b29d 100644 --- a/cli-plugins/manager/manager_test.go +++ b/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"), },