In situations where the config-file is invalid, we ignore the error, and proceed, which can lead to the config file being overwritten for an "empty" file (only some defaults included)
Reproduce
docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock docker:26.1.2-cli sh
mkdir -p ~/.docker
echo '{"imagesFormat":"some special format I have carefully hand-crafted, artisinally"' > ~/.docker/config.json
docker logout
WARNING: Error loading config file: /root/.docker/config.json: unexpected EOF
Removing login credentials for https://index.docker.io/v1/
cat ~/.docker/config.json
{
"auths": {}
}
In this case, we notify the user (warning), but continue the regular flow, which may involve "updating the file" (login / logout, perhaps docker context use)
I was curious though about the "warning" instead of "error"; wondering if it was intended to ignore "not found" errors only, but I don't think that's the case; I think this is mostly because code moved around too much. First time this became a warning looks to be from https://github.com/moby/moby/commit/18c9b6c6455f116ae59cde8544413b3d7d294a5e
So Tl;DR; I don't think there's a good reason / motivation for ignoring the error other than "convenience" (prefer happy path) but changing will mean that we need to change the signature of some functions to return.
docker version
Client:
Version: 26.1.2
API version: 1.44 (downgraded from 1.45)
Go version: go1.21.10
Git commit: 211e74b
Built: Wed May 8 13:59:48 2024
OS/Arch: linux/arm64
Context: default
Description
In situations where the config-file is invalid, we ignore the error, and proceed, which can lead to the config file being overwritten for an "empty" file (only some defaults included)
Reproduce
Expected behavior
The problematic area here is that any error returned from loading the config-file is printed as a warning; https://github.com/docker/cli/blob/61fe22f21a1a618d73cba67fec498f3f9d3b1422/cli/config/config.go#L125
A result of that is that if the file fails to load (malformed file, or other reason),
load
returns an error AND an emptyconfigFile
struct; https://github.com/docker/cli/blob/61fe22f21a1a618d73cba67fec498f3f9d3b1422/cli/config/config.go#L117In this case, we notify the user (warning), but continue the regular flow, which may involve "updating the file" (
login
/logout
, perhapsdocker context use
)I was curious though about the "warning" instead of "error"; wondering if it was intended to ignore "not found" errors only, but I don't think that's the case; I think this is mostly because code moved around too much. First time this became a warning looks to be from https://github.com/moby/moby/commit/18c9b6c6455f116ae59cde8544413b3d7d294a5e
Which unlined some of the code; before that, the separate function returned an error, but was not handled. Going back further; it looks like this commit started to ignore errors https://github.com/moby/moby/commit/3bae188b8dc51911a44ea1c7b5681f9f07f9d3af And https://github.com/moby/moby/commit/18962d0ff33cfa6ba2976aa459c766acfd23c1bf looks to settle that by fully ignoring.
So Tl;DR; I don't think there's a good reason / motivation for ignoring the error other than "convenience" (prefer happy path) but changing will mean that we need to change the signature of some functions to return.
docker version
docker info
Additional Info
No response