Skip to content
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

[27.x backport] cli-plugins: Fix searching inaccessible directories #5652

Merged
merged 2 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
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
36 changes: 9 additions & 27 deletions cli-plugins/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down
26 changes: 24 additions & 2 deletions cli-plugins/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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", `
Expand Down
Loading