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 <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn
2025-03-11 11:27:20 +01:00
parent c8600e1cea
commit 9b2f831452
2 changed files with 14 additions and 7 deletions

View File

@ -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

View File

@ -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"),
},