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

[rfc] should the CLI warn if a plugin-directory is not accessible? #5656

Open
thaJeztah opened this issue Nov 29, 2024 · 3 comments
Open

[rfc] should the CLI warn if a plugin-directory is not accessible? #5656

thaJeztah opened this issue Nov 29, 2024 · 3 comments

Comments

@thaJeztah
Copy link
Member

Description

The PR above updates the CLI to ignore plugin directories that are not accessible. Question is if we should always to so silently as there may be situations were that should still surface to the user; (extra) paths may have been configured for a reason, and silently ignoring and falling back to other locations may be hiding an actual issue that's not immediately visible (a CLI plugin may still be present but an older version).

From the above PR;

The only thing I was considering (but perhaps it's not a real issue) is if we need to distinguish default ("try -> try next") paths and path(s) that are explicitly configured in ~/.docker/config.json as "additional paths" (cliPluginsExtraDirs).

My train of thought there is that if it's an explicit configuration, then failing to traverse the location (perhaps except for "not exist") could be considered a hard failure.

Happy to hear thoughts on that though!

The attempt to use a plugin which isn't accessible will result in a hard failure anyway. Erroring out during plugin scan would prevent the CLI from running in the non-plugin usage.

Yeah, perhaps I'm looking for issues that aren't important, but mostly considering if it should be completely silent. Here's the order of preference in which paths are considered;

// getPluginDirs returns the platform-specific locations to search for plugins
// in order of preference.
//
// Plugin-discovery is performed in the following order of preference:
//
// 1. The "cli-plugins" directory inside the CLIs [config.Path] (usually "~/.docker/cli-plugins").
// 2. Additional plugin directories as configured through [ConfigFile.CLIPluginsExtraDirs].
// 3. Platform-specific defaultSystemPluginDirs.
//
// [ConfigFile.CLIPluginsExtraDirs]: https://pkg.go.dev/github.com/docker/[email protected]+incompatible/cli/config/configfile#ConfigFile.CLIPluginsExtraDirs
func getPluginDirs(cfg *configfile.ConfigFile) ([]string, error) {

Based on that, the custom path is intended to have a higher priority than the system-wide installation paths. Which could mean "the cli is configured to use these overridden versions". On Docker Desktop, this could be some of the additional plugins shipping with it, but perhaps it's a situation where (say) the system-wide version is provided by the distro you're running and has a vulnerability, so the intent is to use a fixed version through one of the higher-priority paths. If we silently ignore that we aren't able to use that path, it means we're silently falling back to either an older (perhaps vulnerable) version, or missing extra plugins that were supposed to be available.

For the system-wide paths, perhaps that's OK (it's a system-wide path, and current user isn't in the right group to use it), so even for those, I somewhat wonder if we should have some warning. Not sure if anything documents expected permissions though, or at least I couldn't find that in the FHS (Filesystem Hierarchy Standard) docs.

Quick check on some machines showed me that they are accessible;

# CentOS, Fedora machine

ls -ld /usr/libexec /usr/libexec/docker /usr/libexec/docker/cli-plugins /usr/local/libexec /usr/local/libexec/docker /usr/local/libexec/docker/cli-plugins
ls: cannot access '/usr/local/libexec/docker': No such file or directory
ls: cannot access '/usr/local/libexec/docker/cli-plugins': No such file or directory
drwxr-xr-x. 26 root root 4096 Nov 25 10:52 /usr/libexec
drwxr-xr-x.  3 root root   44 Nov 25 10:53 /usr/libexec/docker
drwxr-xr-x.  2 root root   49 Nov 25 10:52 /usr/libexec/docker/cli-plugins
drwxr-xr-x.  2 root root    6 Aug  9  2021 /usr/local/libexec

# Ubuntu machine
ls -ld /usr/libexec /usr/libexec/docker /usr/libexec/docker/cli-plugins /usr/local/libexec /usr/local/libexec/docker /usr/local/libexec/docker/cli-plugins
ls: cannot access '/usr/local/libexec': No such file or directory
ls: cannot access '/usr/local/libexec/docker': No such file or directory
ls: cannot access '/usr/local/libexec/docker/cli-plugins': No such file or directory
drwxr-xr-x 12 root root 4096 Nov 19 16:02 /usr/libexec
drwxr-xr-x  3 root root 4096 Nov 19 16:02 /usr/libexec/docker
drwxr-xr-x  2 root root 4096 Nov 19 16:02 /usr/libexec/docker/cli-plugins
@vvoland
Copy link
Collaborator

vvoland commented Nov 29, 2024

Would be nice to have a warning, but I don't think we have a good output to print it to. stdout or stderr might not be the best option as it can make the CLI output non deterministic.

I'm wondering if we should have a CLI log we could log such things to (like ~/.docker/cli.log).

@mihalyr
Copy link

mihalyr commented Nov 29, 2024

Just wanted to add that in case of config folder, docker cli prints a warning like below to stderr

➜  cli-plugins chmod 000 ..
➜  cli-plugins ll -d ..
d---------. 1 mihalyr mihalyr 186 Nov 22 19:22 ..
➜  cli-plugins docker buildx
WARNING: Error loading config file: open /var/home/mihalyr/.docker/config.json: permission denied
docker: 'buildx' is not a docker command.
See 'docker --help'
➜  cli-plugins 

@thaJeztah
Copy link
Member Author

Thanks @mihalyr !

I recall there actually was an issue with that code-path, where "log and continue" could cause actual issues, but due some unfortunate code-paths we weren't able to change that currently. I think that situation was more when reading the actual file though (if a user does a docker login and we fail to read the config-file, it was discarding the content of the file and replaced it with a "clean" copy 😬).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants