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

docker should fail if config file is invalid #5075

Open
thaJeztah opened this issue May 15, 2024 · 0 comments
Open

docker should fail if config file is invalid #5075

thaJeztah opened this issue May 15, 2024 · 0 comments

Comments

@thaJeztah
Copy link
Member

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;

return configFile, err

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

not relevant

Additional Info

No response

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

1 participant