Skip to content

cli-plugins/manager: ignore broken symlinks #5922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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

We should consider what the best approach is for these, as this
patch will make them completely invisible, but we may still be
iterating over them for discovery.

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>
  • Loading branch information
thaJeztah committed Mar 22, 2025
commit 68164c42f5f26556cc8f878321047542619099ba
12 changes: 9 additions & 3 deletions cli-plugins/manager/manager.go
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@ package manager

import (
"context"
"errors"
"os"
"os/exec"
"path/filepath"
@@ -81,9 +82,14 @@ func addPluginCandidatesFromDir(res map[string][]string, d string) {
return
}
for _, dentry := range dentries {
switch dentry.Type() & os.ModeType {
case 0, os.ModeSymlink:
// Regular file or symlink, keep going
switch mode := dentry.Type() & os.ModeType; mode {
case os.ModeSymlink:
if _, err := os.Stat(filepath.Join(d, dentry.Name())); errors.Is(err, os.ErrNotExist) {
// Ignore broken symlink
continue
}
case 0:
// Regular file, keep going
default:
// Something else, ignore.
continue
5 changes: 1 addition & 4 deletions cli-plugins/manager/manager_test.go
Original file line number Diff line number Diff line change
@@ -37,7 +37,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.
),
@@ -71,9 +71,6 @@ func TestListPluginCandidates(t *testing.T) {
"hardlink2": {
dir.Join("plugins2", "docker-hardlink2"),
},
"brokensymlink": {
dir.Join("plugins3", "docker-brokensymlink"),
},
"symlinked": {
dir.Join("plugins3", "docker-symlinked"),
},
Loading
Oops, something went wrong.