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
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": {}
}
Expected behavior
The problematic area here is that any error returned from loading the config-file is printed as a warning;
|
_, _ = fmt.Fprintf(stderr, "WARNING: Error loading config file: %v\n", err) |
A result of that is that if the file fails to load (malformed file, or other reason), load returns an error AND an empty configFile struct;
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 moby/moby@18c9b6c
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 moby/moby@3bae188
And moby/moby@18962d0 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
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
docker info
Additional Info
No response
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;
cli/cli/config/config.go
Line 125 in 61fe22f
A result of that is that if the file fails to load (malformed file, or other reason),
loadreturns an error AND an emptyconfigFilestruct;cli/cli/config/config.go
Line 117 in 61fe22f
In 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 moby/moby@18c9b6c
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 moby/moby@3bae188
And moby/moby@18962d0 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