daemon.json parsing ignores unknown fields

[dsheets]

Description

Unknown fields in daemon.json are ignored by dockerd. At the least, I would expect a warning to be logged about use of unknown fields. The combination of ignored unknown fields and command line flags that differ from configuration fields is very user unfriendly. In the case of #32528, "default-ulimit": {} (cf. --default-ulimit) is accepted but "default-ulimits": {} is rejected which is very confusing. Additionally, the behavior of #32528 (the following directives don't match any configuration option: default-ulimits) implies that unknown fields should halt daemon start.

I've been told that unknown fields are ignored as this reflects the behavior of the golang json unmarshaller. This behavior seems unsuitable for a configuration file format with a fixed specification. Perhaps a different JSON parsing solution needs to be used?

Possible solutions include:

1. Show a warning when unknown fields are used.
2. Error when unknown fields are used. This is a user-friendly way to catch typos.
3. Introduce a command-line option to enable/disable strict field checking (in the case where a user/other software abuses this behavior and stores its own configuration in daemon.json).
4. Add an alias for every field that differs from its command-line argument equivalent so that the command-line argument name is also accepted. This is the principle of least surprise.

Steps to reproduce the issue:

1. Put { "jjj": {}, "default-ulimit": {} } in daemon.json
2. Start dockerd

Describe the results you received:

No failure occurred nor message logged that unknown fields were used in configuration.

Describe the results you expected:

dockerd should not start and the log file should contain an explanation.

Additional information you deem important (e.g. issue happens only occasionally):

N/A

Output of docker version:

Client:
 Version:      17.04.0-ce
 API version:  1.28
 Go version:   go1.7.5
 Git commit:   4845c56
 Built:        Wed Apr  5 06:06:36 2017
 OS/Arch:      darwin/amd64

Server:
 Version:      17.04.0-ce
 API version:  1.28 (minimum version 1.12)
 Go version:   go1.7.5
 Git commit:   4845c56
 Built:        Tue Apr  4 00:37:25 2017
 OS/Arch:      linux/amd64
 Experimental: true

Output of docker info:

Containers: 0
 Running: 0
 Paused: 0
 Stopped: 0
Images: 0
Server Version: 17.04.0-ce
Storage Driver: overlay2
 Backing Filesystem: extfs
 Supports d_type: true
 Native Overlay Diff: true
Logging Driver: json-file
Cgroup Driver: cgroupfs
Plugins:
 Volume: local
 Network: bridge host ipvlan macvlan null overlay
Swarm: inactive
Runtimes: runc
Default Runtime: runc
Init Binary:
containerd version: 422e31ce907fd9c3833a38d7b8fdd023e5a76e73
runc version: 9c2d8d184e5da67c95d601382adf14862e4f2228
init version: 949e6fa
Security Options:
 seccomp
  Profile: default
Kernel Version: 4.9.19-moby
Operating System: Alpine Linux v3.5
OSType: linux
Architecture: x86_64
CPUs: 4
Total Memory: 1.952GiB
Name: moby
ID: LBRL:MJIS:5WS3:NVTP:MYHG:DD54:XVPJ:CQN6:TWVY:XL4Y:WPDX:PKAD
Docker Root Dir: /var/lib/docker
Debug Mode (client): false
Debug Mode (server): true
 File Descriptors: 16
 Goroutines: 26
 System Time: 2017-04-11T15:17:48.302951361Z
 EventsListeners: 1
No Proxy: *.local, 169.254/16
Username: dsheets
Registry: https://index.docker.io/v1/
Experimental: true
Insecure Registries:
 127.0.0.0/8
Live Restore Enabled: false

Additional environment details (AWS, VirtualBox, physical, etc.):

N/A

[Matt3o12]

I tried to investigate this issue but a few things are not clear for me.

In the case of #32528, "default-ulimit": {} (cf. --default-ulimit) is accepted

This only seems to be the case if default-ulimit is an empty map (JSON Object). That means"default-ulimit": true, and "default-ulimit": {"foo": "bar"} report errors and prevent the client from starting, though the error message is a bit confusing, more regarding that later.

I've been told that unknown fields are ignored as this reflects the behavior of the golang json unmarshaller. This behavior seems unsuitable for a configuration file format with a fixed specification. Perhaps a different JSON parsing solution needs to be used?

This has been worked around, otherwise the error message following directives don't match any configuration option: default-ulimits would not have appeared (it does not appear anymore thanks to #32547)

---

In Matt3o12/moby@4b7d86e I added a fix to the config parser so that { "jjj": {}, "default-ulimit": {} } fails to start:

unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives don't match any configuration option: jjj

Having said all that, I do not understand what config.configValuesSet() is supposed to accomplish. It deflattens the configuration map, except for certain values as defined in config.flatOptions

What this means is that {"test": {"foo": "bar"}} essentially becomes {"foo": "bar"} and test is completely dropped (since 'test' is not defined in flatOptions). Maybe @calavera remembers what this is supposed to accomplish.

This also has the nasty side effect because "test": {"foo": "bar"}} will actually report:

unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives don't match any configuration option: foo

Instead of:

unable to configure the Docker daemon with file /etc/docker/daemon.json: the following directives don't match any configuration option: test

[Myeongjoon]

@dsheets
hello.

4.Add an alias for every field that differs from its command-line argument equivalent so that the command-line argument name is also accepted. This is the principle of least surprise.

Does it means that if "default-ulimits" mistyped then docker start and notify "default-ulimits" replaced to "default-ulimit"?
How about use algorithm like Levenshtein_distance

[thaJeztah]

Also see the discussion on #34093 (comment)

[jeremija]

Would the recent addition of DisallowUnknownFields to Golang's JSON Decoder be helpful for this issue: golang/go#15314? Additional link describing the changes: https://go-review.googlesource.com/c/go/+/74830

[stefanct]

(standard) json does not support comments. the usual workaround is to add a key that is not used by the application such as __comment and us its value for the comment. This already does not work (but completely fails to start with the following directives don't match any configuration option: __comment. A warning would be fine but failing due to unknown keys and having no way to specify comments IS an unpleasant surprise. :(