Merge pull request #43732 from thaJeztah/daemon_fix_hosts_validation_step1c1

daemon: refactor config loading

api/server/server.go

@@ -27,6 +27,8 @@ type Config struct {
 	Version     string
 	SocketGroup string
 	TLSConfig   *tls.Config
+	// Hosts is a list of addresses for the API to listen on.
+	Hosts []string
 }
 
 // Server contains instance details for the server

cmd/dockerd/config.go

@@ -44,8 +44,8 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 	flags.StringVar(&conf.ContainerdAddr, "containerd", "", "containerd grpc address")
 	flags.BoolVar(&conf.CriContainerd, "cri-containerd", false, "start containerd with cri")
 
-	flags.IntVar(&conf.Mtu, "mtu", 0, "Set the containers network MTU")
-	flags.IntVar(&conf.NetworkControlPlaneMTU, "network-control-plane-mtu", config.DefaultNetworkMtu, "Network Control plane MTU")
+	flags.IntVar(&conf.Mtu, "mtu", conf.Mtu, "Set the containers network MTU")
+	flags.IntVar(&conf.NetworkControlPlaneMTU, "network-control-plane-mtu", conf.NetworkControlPlaneMTU, "Network Control plane MTU")
 	flags.IntVar(&conf.NetworkDiagnosticPort, "network-diagnostic-port", 0, "TCP port number of the network diagnostic server")
 	_ = flags.MarkHidden("network-diagnostic-port")
 
@@ -59,19 +59,19 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 	flags.Var(opts.NewNamedMapOpts("log-opts", conf.LogConfig.Config, nil), "log-opt", "Default log driver options for containers")
 
 	flags.StringVar(&conf.CorsHeaders, "api-cors-header", "", "Set CORS headers in the Engine API")
-	flags.IntVar(&conf.MaxConcurrentDownloads, "max-concurrent-downloads", config.DefaultMaxConcurrentDownloads, "Set the max concurrent downloads for each pull")
-	flags.IntVar(&conf.MaxConcurrentUploads, "max-concurrent-uploads", config.DefaultMaxConcurrentUploads, "Set the max concurrent uploads for each push")
-	flags.IntVar(&conf.MaxDownloadAttempts, "max-download-attempts", config.DefaultDownloadAttempts, "Set the max download attempts for each pull")
-	flags.IntVar(&conf.ShutdownTimeout, "shutdown-timeout", config.DefaultShutdownTimeout, "Set the default shutdown timeout")
+	flags.IntVar(&conf.MaxConcurrentDownloads, "max-concurrent-downloads", conf.MaxConcurrentDownloads, "Set the max concurrent downloads for each pull")
+	flags.IntVar(&conf.MaxConcurrentUploads, "max-concurrent-uploads", conf.MaxConcurrentUploads, "Set the max concurrent uploads for each push")
+	flags.IntVar(&conf.MaxDownloadAttempts, "max-download-attempts", conf.MaxDownloadAttempts, "Set the max download attempts for each pull")
+	flags.IntVar(&conf.ShutdownTimeout, "shutdown-timeout", conf.ShutdownTimeout, "Set the default shutdown timeout")
 
 	flags.StringVar(&conf.SwarmDefaultAdvertiseAddr, "swarm-default-advertise-addr", "", "Set default address or interface for swarm advertised address")
 	flags.BoolVar(&conf.Experimental, "experimental", false, "Enable experimental features")
 	flags.StringVar(&conf.MetricsAddress, "metrics-addr", "", "Set default address and port to serve the metrics api on")
 	flags.Var(opts.NewNamedListOptsRef("node-generic-resources", &conf.NodeGenericResources, opts.ValidateSingleGenericResource), "node-generic-resource", "Advertise user-defined resource")
 
-	flags.StringVar(&conf.ContainerdNamespace, "containerd-namespace", config.DefaultContainersNamespace, "Containerd namespace to use")
-	flags.StringVar(&conf.ContainerdPluginNamespace, "containerd-plugins-namespace", config.DefaultPluginNamespace, "Containerd namespace to use for plugins")
-	flags.StringVar(&conf.DefaultRuntime, "default-runtime", config.StockRuntimeName, "Default OCI runtime for containers")
+	flags.StringVar(&conf.ContainerdNamespace, "containerd-namespace", conf.ContainerdNamespace, "Containerd namespace to use")
+	flags.StringVar(&conf.ContainerdPluginNamespace, "containerd-plugins-namespace", conf.ContainerdPluginNamespace, "Containerd namespace to use for plugins")
+	flags.StringVar(&conf.DefaultRuntime, "default-runtime", conf.DefaultRuntime, "Default OCI runtime for containers")
 
 	flags.StringVar(&conf.HTTPProxy, "http-proxy", "", "HTTP proxy URL to use for outgoing traffic")
 	flags.StringVar(&conf.HTTPSProxy, "https-proxy", "", "HTTPS proxy URL to use for outgoing traffic")

cmd/dockerd/config_unix.go

@@ -28,7 +28,6 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 	}
 
 	conf.Ulimits = make(map[string]*units.Ulimit)
-	conf.NetworkConfig.DefaultAddressPools = opts.PoolsOpt{}
 
 	// Set default value for `--default-shm-size`
 	conf.ShmSize = opts.MemBytes(config.DefaultShmSize)

cmd/dockerd/daemon.go

@@ -76,8 +76,6 @@ func NewDaemonCli() *DaemonCli {
 }
 
 func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
-	opts.setDefaultOptions()
-
 	if cli.Config, err = loadDaemonCliConfig(opts); err != nil {
 		return err
 	}
@@ -92,7 +90,7 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 
 	if opts.Validate {
 		// If config wasn't OK we wouldn't have made it this far.
-		fmt.Fprintln(os.Stderr, "configuration OK")
+		_, _ = fmt.Fprintln(os.Stderr, "configuration OK")
 		return nil
 	}
 
@@ -386,6 +384,11 @@ func shutdownDaemon(d *daemon.Daemon) {
 }
 
 func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
+	if !opts.flags.Parsed() {
+		return nil, errors.New(`cannot load CLI config before flags are parsed`)
+	}
+	opts.setDefaultOptions()
+
 	conf := opts.daemonConfig
 	flags := opts.flags
 	conf.Debug = opts.Debug
@@ -612,32 +615,35 @@ func (cli *DaemonCli) getContainerdDaemonOpts() ([]supervisor.DaemonOpt, error)
 }
 
 func newAPIServerConfig(config *config.Config) (*apiserver.Config, error) {
-	serverConfig := &apiserver.Config{
-		SocketGroup: config.SocketGroup,
-		Version:     dockerversion.Version,
-		CorsHeaders: config.CorsHeaders,
-	}
-
+	var tlsConfig *tls.Config
 	if config.TLS != nil && *config.TLS {
-		tlsOptions := tlsconfig.Options{
+		var (
+			clientAuth tls.ClientAuthType
+			err        error
+		)
+		if config.TLSVerify == nil || *config.TLSVerify {
+			// server requires and verifies client's certificate
+			clientAuth = tls.RequireAndVerifyClientCert
+		}
+		tlsConfig, err = tlsconfig.Server(tlsconfig.Options{
 			CAFile:             config.CommonTLSOptions.CAFile,
 			CertFile:           config.CommonTLSOptions.CertFile,
 			KeyFile:            config.CommonTLSOptions.KeyFile,
 			ExclusiveRootPools: true,
-		}
-
-		if config.TLSVerify == nil || *config.TLSVerify {
-			// server requires and verifies client's certificate
-			tlsOptions.ClientAuth = tls.RequireAndVerifyClientCert
-		}
-		tlsConfig, err := tlsconfig.Server(tlsOptions)
+			ClientAuth:         clientAuth,
+		})
 		if err != nil {
 			return nil, errors.Wrap(err, "invalid TLS configuration")
 		}
-		serverConfig.TLSConfig = tlsConfig
 	}
 
-	return serverConfig, nil
+	return &apiserver.Config{
+		SocketGroup: config.SocketGroup,
+		Version:     dockerversion.Version,
+		CorsHeaders: config.CorsHeaders,
+		TLSConfig:   tlsConfig,
+		Hosts:       config.Hosts,
+	}, nil
 }
 
 // checkTLSAuthOK checks basically for an explicitly disabled TLS/TLSVerify
@@ -666,14 +672,14 @@ func checkTLSAuthOK(c *config.Config) bool {
 }
 
 func loadListeners(cli *DaemonCli, serverConfig *apiserver.Config) ([]string, error) {
-	if len(cli.Config.Hosts) == 0 {
+	if len(serverConfig.Hosts) == 0 {
 		return nil, errors.New("no hosts configured")
 	}
 	var hosts []string
 
-	for i := 0; i < len(cli.Config.Hosts); i++ {
-		protoAddr := cli.Config.Hosts[i]
-		protoAddrParts := strings.SplitN(cli.Config.Hosts[i], "://", 2)
+	for i := 0; i < len(serverConfig.Hosts); i++ {
+		protoAddr := serverConfig.Hosts[i]
+		protoAddrParts := strings.SplitN(serverConfig.Hosts[i], "://", 2)
 		if len(protoAddrParts) != 2 {
 			return nil, fmt.Errorf("bad format %s, expected PROTO://ADDR", protoAddr)
 		}
@@ -719,16 +725,16 @@ func loadListeners(cli *DaemonCli, serverConfig *apiserver.Config) ([]string, er
 				}
 			}
 		}
-		ls, err := listeners.Init(proto, addr, serverConfig.SocketGroup, serverConfig.TLSConfig)
-		if err != nil {
-			return nil, err
-		}
 		// If we're binding to a TCP port, make sure that a container doesn't try to use it.
 		if proto == "tcp" {
 			if err := allocateDaemonPort(addr); err != nil {
 				return nil, err
 			}
 		}
+		ls, err := listeners.Init(proto, addr, serverConfig.SocketGroup, serverConfig.TLSConfig)
+		if err != nil {
+			return nil, err
+		}
 		logrus.Debugf("Listener created for HTTP on %s (%s)", proto, addr)
 		hosts = append(hosts, protoAddrParts[1])
 		cli.api.Accept(addr, ls...)

cmd/dockerd/daemon_test.go

@@ -22,6 +22,8 @@ func defaultOptions(t *testing.T, configFile string) *daemonOptions {
 	assert.NilError(t, err)
 	opts.flags.StringVar(&opts.configFile, "config-file", defaultDaemonConfigFile, "")
 	opts.configFile = configFile
+	err = opts.flags.Parse([]string{})
+	assert.NilError(t, err)
 	return opts
 }
 

cmd/dockerd/daemon_unix.go

@@ -5,7 +5,6 @@ package main
 
 import (
 	"context"
-	"fmt"
 	"net"
 	"os"
 	"os/signal"
@@ -51,7 +50,7 @@ func setDefaultUmask() error {
 	desiredUmask := 0022
 	unix.Umask(desiredUmask)
 	if umask := unix.Umask(desiredUmask); umask != desiredUmask {
-		return fmt.Errorf("failed to set umask: expected %#o, got %#o", desiredUmask, umask)
+		return errors.Errorf("failed to set umask: expected %#o, got %#o", desiredUmask, umask)
 	}
 
 	return nil
@@ -91,25 +90,25 @@ func (cli *DaemonCli) getSwarmRunRoot() string {
 func allocateDaemonPort(addr string) error {
 	host, port, err := net.SplitHostPort(addr)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "error parsing tcp address")
 	}
 
 	intPort, err := strconv.Atoi(port)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "error parsing tcp address")
 	}
 
 	var hostIPs []net.IP
 	if parsedIP := net.ParseIP(host); parsedIP != nil {
 		hostIPs = append(hostIPs, parsedIP)
 	} else if hostIPs, err = net.LookupIP(host); err != nil {
-		return fmt.Errorf("failed to lookup %s address in host specification", host)
+		return errors.Errorf("failed to lookup %s address in host specification", host)
 	}
 
 	pa := portallocator.Get()
 	for _, hostIP := range hostIPs {
 		if _, err := pa.RequestPort(hostIP, "tcp", intPort); err != nil {
-			return fmt.Errorf("failed to allocate daemon listening port %d (err: %v)", intPort, err)
+			return errors.Errorf("failed to allocate daemon listening port %d (err: %v)", intPort, err)
 		}
 	}
 	return nil

daemon/config/config.go

@@ -296,9 +296,20 @@ func (conf *Config) IsValueSet(name string) bool {
 func New() *Config {
 	return &Config{
 		CommonConfig: CommonConfig{
+			ShutdownTimeout: DefaultShutdownTimeout,
 			LogConfig: LogConfig{
 				Config: make(map[string]string),
 			},
+			MaxConcurrentDownloads: DefaultMaxConcurrentDownloads,
+			MaxConcurrentUploads:   DefaultMaxConcurrentUploads,
+			MaxDownloadAttempts:    DefaultDownloadAttempts,
+			Mtu:                    DefaultNetworkMtu,
+			NetworkConfig: NetworkConfig{
+				NetworkControlPlaneMTU: DefaultNetworkMtu,
+			},
+			ContainerdNamespace:       DefaultContainersNamespace,
+			ContainerdPluginNamespace: DefaultPluginNamespace,
+			DefaultRuntime:            StockRuntimeName,
 		},
 	}
 }
@@ -597,6 +608,9 @@ func Validate(config *Config) error {
 	}
 
 	// TODO(thaJeztah) Validations below should not accept "0" to be valid; see Validate() for a more in-depth description of this problem
+	if config.Mtu < 0 {
+		return fmt.Errorf("invalid default MTU: %d", config.Mtu)
+	}
 	if config.MaxConcurrentDownloads < 0 {
 		return fmt.Errorf("invalid max concurrent downloads: %d", config.MaxConcurrentDownloads)
 	}

daemon/config/config_linux_test.go

@@ -63,33 +63,25 @@ func TestDaemonConfigurationMerge(t *testing.T) {
 					"Hard": 2048,
 					"Soft": 1024
 				}
-			},
-			"log-opts": {
-				"tag": "test_tag"
 			}
 		}`
 
 	file := fs.NewFile(t, "docker-config", fs.WithContent(configFileData))
 	defer file.Remove()
 
-	c := &Config{
-		CommonConfig: CommonConfig{
-			AutoRestart: true,
-			LogConfig: LogConfig{
-				Type:   "syslog",
-				Config: map[string]string{"tag": "test"},
-			},
-		},
-	}
+	conf := New()
 
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
-
-	var debug bool
-	flags.BoolVarP(&debug, "debug", "D", false, "")
-	flags.Var(opts.NewNamedUlimitOpt("default-ulimits", nil), "default-ulimit", "")
-	flags.Var(opts.NewNamedMapOpts("log-opts", nil, nil), "log-opt", "")
-
-	cc, err := MergeDaemonConfigurations(c, flags, file.Path())
+	flags.BoolVarP(&conf.Debug, "debug", "D", false, "")
+	flags.BoolVarP(&conf.AutoRestart, "restart", "r", true, "")
+	flags.Var(opts.NewNamedUlimitOpt("default-ulimits", &conf.Ulimits), "default-ulimit", "")
+	flags.StringVar(&conf.LogConfig.Type, "log-driver", "json-file", "")
+	flags.Var(opts.NewNamedMapOpts("log-opts", conf.LogConfig.Config, nil), "log-opt", "")
+	assert.Check(t, flags.Set("restart", "true"))
+	assert.Check(t, flags.Set("log-driver", "syslog"))
+	assert.Check(t, flags.Set("log-opt", "tag=from_flag"))
+
+	cc, err := MergeDaemonConfigurations(conf, flags, file.Path())
 	assert.NilError(t, err)
 
 	assert.Check(t, cc.Debug)
@@ -97,7 +89,7 @@ func TestDaemonConfigurationMerge(t *testing.T) {
 
 	expectedLogConfig := LogConfig{
 		Type:   "syslog",
-		Config: map[string]string{"tag": "test_tag"},
+		Config: map[string]string{"tag": "from_flag"},
 	}
 
 	assert.Check(t, is.DeepEqual(expectedLogConfig, cc.LogConfig))
@@ -134,18 +126,21 @@ func TestDaemonConfigurationMergeShmSize(t *testing.T) {
 
 func TestUnixValidateConfigurationErrors(t *testing.T) {
 	testCases := []struct {
-		config *Config
+		doc         string
+		config      *Config
+		expectedErr string
 	}{
-		// Can't override the stock runtime
 		{
+			doc: `cannot override the stock runtime`,
 			config: &Config{
 				Runtimes: map[string]types.Runtime{
 					StockRuntimeName: {},
 				},
 			},
+			expectedErr: `runtime name 'runc' is reserved`,
 		},
-		// Default runtime should be present in runtimes
 		{
+			doc: `default runtime should be present in runtimes`,
 			config: &Config{
 				Runtimes: map[string]types.Runtime{
 					"foo": {},
@@ -154,13 +149,15 @@ func TestUnixValidateConfigurationErrors(t *testing.T) {
 					DefaultRuntime: "bar",
 				},
 			},
+			expectedErr: `specified default runtime 'bar' does not exist`,
 		},
 	}
 	for _, tc := range testCases {
-		err := Validate(tc.config)
-		if err == nil {
-			t.Fatalf("expected error, got nil for config %v", tc.config)
-		}
+		tc := tc
+		t.Run(tc.doc, func(t *testing.T) {
+			err := Validate(tc.config)
+			assert.ErrorContains(t, err, tc.expectedErr)
+		})
 	}
 }
 
@@ -194,9 +191,6 @@ func TestUnixGetInitPath(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		initPath := tc.config.GetInitPath()
-		if initPath != tc.expectedInitPath {
-			t.Fatalf("expected initPath to be %v, got %v", tc.expectedInitPath, initPath)
-		}
+		assert.Equal(t, tc.config.GetInitPath(), tc.expectedInitPath)
 	}
 }

daemon/config/config_test.go

@@ -2,11 +2,15 @@ package config // import "github.com/docker/docker/daemon/config"
 
 import (
 	"os"
+	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/docker/docker/libnetwork/ipamutils"
 	"github.com/docker/docker/opts"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/imdario/mergo"
 	"github.com/spf13/pflag"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
@@ -16,25 +20,19 @@ import (
 
 func TestDaemonConfigurationNotFound(t *testing.T) {
 	_, err := MergeDaemonConfigurations(&Config{}, nil, "/tmp/foo-bar-baz-docker")
-	if err == nil || !os.IsNotExist(err) {
-		t.Fatalf("expected does not exist error, got %v", err)
-	}
+	assert.Check(t, os.IsNotExist(err), "got: %[1]T: %[1]v", err)
 }
 
 func TestDaemonBrokenConfiguration(t *testing.T) {
 	f, err := os.CreateTemp("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	assert.NilError(t, err)
 
 	configFile := f.Name()
 	f.Write([]byte(`{"Debug": tru`))
 	f.Close()
 
 	_, err = MergeDaemonConfigurations(&Config{}, nil, configFile)
-	if err == nil {
-		t.Fatalf("expected error, got %v", err)
-	}
+	assert.ErrorContains(t, err, `invalid character ' ' in literal true`)
 }
 
 func TestFindConfigurationConflicts(t *testing.T) {
@@ -59,9 +57,7 @@ func TestFindConfigurationConflictsWithNamedOptions(t *testing.T) {
 
 func TestDaemonConfigurationMergeConflicts(t *testing.T) {
 	f, err := os.CreateTemp("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	assert.NilError(t, err)
 
 	configFile := f.Name()
 	f.Write([]byte(`{"debug": true}`))
@@ -69,7 +65,7 @@ func TestDaemonConfigurationMergeConflicts(t *testing.T) {
 
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 	flags.Bool("debug", false, "")
-	flags.Set("debug", "false")
+	assert.Check(t, flags.Set("debug", "false"))
 
 	_, err = MergeDaemonConfigurations(&Config{}, flags, configFile)
 	if err == nil {
@@ -82,41 +78,31 @@ func TestDaemonConfigurationMergeConflicts(t *testing.T) {
 
 func TestDaemonConfigurationMergeConcurrent(t *testing.T) {
 	f, err := os.CreateTemp("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	assert.NilError(t, err)
 
 	configFile := f.Name()
 	f.Write([]byte(`{"max-concurrent-downloads": 1}`))
 	f.Close()
 
 	_, err = MergeDaemonConfigurations(&Config{}, nil, configFile)
-	if err != nil {
-		t.Fatal("expected error, got nil")
-	}
+	assert.NilError(t, err)
 }
 
 func TestDaemonConfigurationMergeConcurrentError(t *testing.T) {
 	f, err := os.CreateTemp("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	assert.NilError(t, err)
 
 	configFile := f.Name()
 	f.Write([]byte(`{"max-concurrent-downloads": -1}`))
 	f.Close()
 
 	_, err = MergeDaemonConfigurations(&Config{}, nil, configFile)
-	if err == nil {
-		t.Fatalf("expected no error, got error %v", err)
-	}
+	assert.ErrorContains(t, err, `invalid max concurrent downloads: -1`)
 }
 
 func TestDaemonConfigurationMergeConflictsWithInnerStructs(t *testing.T) {
 	f, err := os.CreateTemp("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	assert.NilError(t, err)
 
 	configFile := f.Name()
 	f.Write([]byte(`{"tlscacert": "/etc/certificates/ca.pem"}`))
@@ -124,15 +110,10 @@ func TestDaemonConfigurationMergeConflictsWithInnerStructs(t *testing.T) {
 
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 	flags.String("tlscacert", "", "")
-	flags.Set("tlscacert", "~/.docker/ca.pem")
+	assert.Check(t, flags.Set("tlscacert", "~/.docker/ca.pem"))
 
 	_, err = MergeDaemonConfigurations(&Config{}, flags, configFile)
-	if err == nil {
-		t.Fatal("expected error, got nil")
-	}
-	if !strings.Contains(err.Error(), "tlscacert") {
-		t.Fatalf("expected tlscacert conflict, got %v", err)
-	}
+	assert.ErrorContains(t, err, `the following directives are specified both as a flag and in the configuration file: tlscacert`)
 }
 
 // Test for #40711
@@ -148,7 +129,7 @@ func TestDaemonConfigurationMergeDefaultAddressPools(t *testing.T) {
 		var conf = Config{}
 		flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 		flags.Var(&conf.NetworkConfig.DefaultAddressPools, "default-address-pool", "")
-		flags.Set("default-address-pool", "base=10.123.0.0/16,size=24")
+		assert.Check(t, flags.Set("default-address-pool", "base=10.123.0.0/16,size=24"))
 
 		config, err := MergeDaemonConfigurations(&conf, flags, emptyConfigFile.Path())
 		assert.NilError(t, err)
@@ -169,7 +150,7 @@ func TestDaemonConfigurationMergeDefaultAddressPools(t *testing.T) {
 		var conf = Config{}
 		flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 		flags.Var(&conf.NetworkConfig.DefaultAddressPools, "default-address-pool", "")
-		flags.Set("default-address-pool", "base=10.123.0.0/16,size=24")
+		assert.Check(t, flags.Set("default-address-pool", "base=10.123.0.0/16,size=24"))
 
 		_, err := MergeDaemonConfigurations(&conf, flags, configFile.Path())
 		assert.ErrorContains(t, err, "the following directives are specified both as a flag and in the configuration file")
@@ -183,12 +164,7 @@ func TestFindConfigurationConflictsWithUnknownKeys(t *testing.T) {
 
 	flags.Bool("tlsverify", false, "")
 	err := findConfigurationConflicts(config, flags)
-	if err == nil {
-		t.Fatal("expected error, got nil")
-	}
-	if !strings.Contains(err.Error(), "the following directives don't match any configuration option: tls-verify") {
-		t.Fatalf("expected tls-verify conflict, got %v", err)
-	}
+	assert.ErrorContains(t, err, "the following directives don't match any configuration option: tls-verify")
 }
 
 func TestFindConfigurationConflictsWithMergedValues(t *testing.T) {
@@ -198,23 +174,17 @@ func TestFindConfigurationConflictsWithMergedValues(t *testing.T) {
 	flags.VarP(opts.NewNamedListOptsRef("hosts", &hosts, nil), "host", "H", "")
 
 	err := findConfigurationConflicts(config, flags)
-	if err != nil {
-		t.Fatal(err)
-	}
+	assert.NilError(t, err)
 
-	flags.Set("host", "unix:///var/run/docker.sock")
+	assert.Check(t, flags.Set("host", "unix:///var/run/docker.sock"))
 	err = findConfigurationConflicts(config, flags)
-	if err == nil {
-		t.Fatal("expected error, got nil")
-	}
-	if !strings.Contains(err.Error(), "hosts: (from flag: [unix:///var/run/docker.sock], from file: tcp://127.0.0.1:2345)") {
-		t.Fatalf("expected hosts conflict, got %v", err)
-	}
+	assert.ErrorContains(t, err, "hosts: (from flag: [unix:///var/run/docker.sock], from file: tcp://127.0.0.1:2345)")
 }
 
 func TestValidateConfigurationErrors(t *testing.T) {
 	testCases := []struct {
 		name        string
+		field       string
 		config      *Config
 		expectedErr string
 	}{
@@ -280,6 +250,15 @@ func TestValidateConfigurationErrors(t *testing.T) {
 			},
 			expectedErr: "123456 is not a valid domain",
 		},
+		{
+			name: "negative MTU",
+			config: &Config{
+				CommonConfig: CommonConfig{
+					Mtu: -10,
+				},
+			},
+			expectedErr: "invalid default MTU: -10",
+		},
 		{
 			name: "negative max-concurrent-downloads",
 			config: &Config{
@@ -310,7 +289,8 @@ func TestValidateConfigurationErrors(t *testing.T) {
 		// TODO(thaJeztah) temporarily excluding this test as it assumes defaults are set before validating and applying updated configs
 		/*
 			{
-				name: "zero max-download-attempts",
+				name:  "zero max-download-attempts",
+				field: "MaxDownloadAttempts",
 				config: &Config{
 					CommonConfig: CommonConfig{
 						MaxDownloadAttempts: 0,
@@ -358,19 +338,45 @@ func TestValidateConfigurationErrors(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			err := Validate(tc.config)
+			cfg := New()
+			if tc.field != "" {
+				assert.Check(t, mergo.Merge(cfg, tc.config, mergo.WithOverride, withForceOverwrite(tc.field)))
+			} else {
+				assert.Check(t, mergo.Merge(cfg, tc.config, mergo.WithOverride))
+			}
+			err := Validate(cfg)
 			assert.Error(t, err, tc.expectedErr)
 		})
 	}
 }
 
+func withForceOverwrite(fieldName string) func(config *mergo.Config) {
+	return mergo.WithTransformers(overwriteTransformer{fieldName: fieldName})
+}
+
+type overwriteTransformer struct {
+	fieldName string
+}
+
+func (tf overwriteTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error {
+	if typ == reflect.TypeOf(CommonConfig{}) {
+		return func(dst, src reflect.Value) error {
+			dst.FieldByName(tf.fieldName).Set(src.FieldByName(tf.fieldName))
+			return nil
+		}
+	}
+	return nil
+}
+
 func TestValidateConfiguration(t *testing.T) {
 	testCases := []struct {
 		name   string
+		field  string
 		config *Config
 	}{
 		{
-			name: "with label",
+			name:  "with label",
+			field: "Labels",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					Labels: []string{"one=two"},
@@ -378,7 +384,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 		},
 		{
-			name: "with dns",
+			name:  "with dns",
+			field: "DNSConfig",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					DNSConfig: DNSConfig{
@@ -388,7 +395,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 		},
 		{
-			name: "with dns-search",
+			name:  "with dns-search",
+			field: "DNSConfig",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					DNSConfig: DNSConfig{
@@ -398,7 +406,17 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 		},
 		{
-			name: "with max-concurrent-downloads",
+			name:  "with mtu",
+			field: "Mtu",
+			config: &Config{
+				CommonConfig: CommonConfig{
+					Mtu: 1234,
+				},
+			},
+		},
+		{
+			name:  "with max-concurrent-downloads",
+			field: "MaxConcurrentDownloads",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					MaxConcurrentDownloads: 4,
@@ -406,7 +424,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 		},
 		{
-			name: "with max-concurrent-uploads",
+			name:  "with max-concurrent-uploads",
+			field: "MaxConcurrentUploads",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					MaxConcurrentUploads: 4,
@@ -414,7 +433,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 		},
 		{
-			name: "with max-download-attempts",
+			name:  "with max-download-attempts",
+			field: "MaxDownloadAttempts",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					MaxDownloadAttempts: 4,
@@ -422,7 +442,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 		},
 		{
-			name: "with multiple node generic resources",
+			name:  "with multiple node generic resources",
+			field: "NodeGenericResources",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					NodeGenericResources: []string{"foo=bar", "foo=baz"},
@@ -430,7 +451,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 		},
 		{
-			name: "with node generic resources",
+			name:  "with node generic resources",
+			field: "NodeGenericResources",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					NodeGenericResources: []string{"foo=1"},
@@ -438,7 +460,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 		},
 		{
-			name: "with hosts",
+			name:  "with hosts",
+			field: "Hosts",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					Hosts: []string{"tcp://127.0.0.1:2375"},
@@ -446,7 +469,8 @@ func TestValidateConfiguration(t *testing.T) {
 			},
 		},
 		{
-			name: "with log-level warn",
+			name:  "with log-level warn",
+			field: "LogLevel",
 			config: &Config{
 				CommonConfig: CommonConfig{
 					LogLevel: "warn",
@@ -456,19 +480,36 @@ func TestValidateConfiguration(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			err := Validate(tc.config)
+			// Start with a config with all defaults set, so that we only
+			cfg := New()
+			assert.Check(t, mergo.Merge(cfg, tc.config, mergo.WithOverride))
+
+			// Check that the override happened :)
+			assert.Check(t, is.DeepEqual(cfg, tc.config, field(tc.field)))
+			err := Validate(cfg)
 			assert.NilError(t, err)
 		})
 	}
 }
 
+func field(field string) cmp.Option {
+	tmp := reflect.TypeOf(Config{})
+	ignoreFields := make([]string, 0, tmp.NumField())
+	for i := 0; i < tmp.NumField(); i++ {
+		if tmp.Field(i).Name != field {
+			ignoreFields = append(ignoreFields, tmp.Field(i).Name)
+		}
+	}
+	return cmpopts.IgnoreFields(Config{}, ignoreFields...)
+}
+
 // TestReloadSetConfigFileNotExist tests that when `--config-file` is set
 // and it doesn't exist the `Reload` function returns an error.
 func TestReloadSetConfigFileNotExist(t *testing.T) {
 	configFile := "/tmp/blabla/not/exists/config.json"
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 	flags.String("config-file", "", "")
-	flags.Set("config-file", configFile)
+	assert.Check(t, flags.Set("config-file", configFile))
 
 	err := Reload(configFile, flags, func(c *Config) {})
 	assert.Check(t, is.ErrorContains(err, "unable to configure the Docker daemon with file"))
@@ -478,10 +519,10 @@ func TestReloadSetConfigFileNotExist(t *testing.T) {
 // doesn't exist the daemon still will be reloaded.
 func TestReloadDefaultConfigNotExist(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
-	reloaded := false
 	defaultConfigFile := "/tmp/blabla/not/exists/daemon.json"
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 	flags.String("config-file", defaultConfigFile, "")
+	reloaded := false
 	err := Reload(defaultConfigFile, flags, func(c *Config) {
 		reloaded = true
 	})
@@ -493,9 +534,7 @@ func TestReloadDefaultConfigNotExist(t *testing.T) {
 // and the default configuration file exists and is bad return an error
 func TestReloadBadDefaultConfig(t *testing.T) {
 	f, err := os.CreateTemp("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	assert.NilError(t, err)
 
 	configFile := f.Name()
 	f.Write([]byte(`{wrong: "configuration"}`))
@@ -503,8 +542,12 @@ func TestReloadBadDefaultConfig(t *testing.T) {
 
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 	flags.String("config-file", configFile, "")
-	err = Reload(configFile, flags, func(c *Config) {})
+	reloaded := false
+	err = Reload(configFile, flags, func(c *Config) {
+		reloaded = true
+	})
 	assert.Check(t, is.ErrorContains(err, "unable to configure the Docker daemon with file"))
+	assert.Check(t, reloaded == false)
 }
 
 func TestReloadWithConflictingLabels(t *testing.T) {
@@ -516,8 +559,12 @@ func TestReloadWithConflictingLabels(t *testing.T) {
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 	flags.String("config-file", configFile, "")
 	flags.StringSlice("labels", lbls, "")
-	err := Reload(configFile, flags, func(c *Config) {})
+	reloaded := false
+	err := Reload(configFile, flags, func(c *Config) {
+		reloaded = true
+	})
 	assert.Check(t, is.ErrorContains(err, "conflict labels for foo=baz and foo=bar"))
+	assert.Check(t, reloaded == false)
 }
 
 func TestReloadWithDuplicateLabels(t *testing.T) {
@@ -529,8 +576,13 @@ func TestReloadWithDuplicateLabels(t *testing.T) {
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 	flags.String("config-file", configFile, "")
 	flags.StringSlice("labels", lbls, "")
-	err := Reload(configFile, flags, func(c *Config) {})
+	reloaded := false
+	err := Reload(configFile, flags, func(c *Config) {
+		reloaded = true
+		assert.Check(t, is.DeepEqual(c.Labels, []string{"foo=the-same"}))
+	})
 	assert.Check(t, err)
+	assert.Check(t, reloaded)
 }
 
 func TestMaskURLCredentials(t *testing.T) {

daemon/config/config_windows_test.go

@@ -12,38 +12,29 @@ import (
 
 func TestDaemonConfigurationMerge(t *testing.T) {
 	f, err := os.CreateTemp("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	assert.NilError(t, err)
 
 	configFile := f.Name()
 
 	f.Write([]byte(`
 		{
-			"debug": true,
-			"log-opts": {
-				"tag": "test_tag"
-			}
+			"debug": true
 		}`))
 
 	f.Close()
 
-	c := &Config{
-		CommonConfig: CommonConfig{
-			AutoRestart: true,
-			LogConfig: LogConfig{
-				Type:   "syslog",
-				Config: map[string]string{"tag": "test"},
-			},
-		},
-	}
+	conf := New()
 
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
-	var debug bool
-	flags.BoolVarP(&debug, "debug", "D", false, "")
-	flags.Var(opts.NewNamedMapOpts("log-opts", nil, nil), "log-opt", "")
-
-	cc, err := MergeDaemonConfigurations(c, flags, configFile)
+	flags.BoolVarP(&conf.Debug, "debug", "D", false, "")
+	flags.BoolVarP(&conf.AutoRestart, "restart", "r", true, "")
+	flags.StringVar(&conf.LogConfig.Type, "log-driver", "json-file", "")
+	flags.Var(opts.NewNamedMapOpts("log-opts", conf.LogConfig.Config, nil), "log-opt", "")
+	assert.Check(t, flags.Set("restart", "true"))
+	assert.Check(t, flags.Set("log-driver", "syslog"))
+	assert.Check(t, flags.Set("log-opt", "tag=from_flag"))
+
+	cc, err := MergeDaemonConfigurations(conf, flags, configFile)
 	assert.NilError(t, err)
 
 	assert.Check(t, cc.Debug)
@@ -51,7 +42,7 @@ func TestDaemonConfigurationMerge(t *testing.T) {
 
 	expectedLogConfig := LogConfig{
 		Type:   "syslog",
-		Config: map[string]string{"tag": "test_tag"},
+		Config: map[string]string{"tag": "from_flag"},
 	}
 
 	assert.Check(t, is.DeepEqual(expectedLogConfig, cc.LogConfig))

daemon/daemon.go

@@ -703,8 +703,6 @@ func (daemon *Daemon) IsSwarmCompatible() error {
 // NewDaemon sets up everything for the daemon to be able to service
 // requests from the webserver.
 func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.Store) (daemon *Daemon, err error) {
-	setDefaultMtu(config)
-
 	registryService, err := registry.NewService(config.ServiceOptions)
 	if err != nil {
 		return nil, err
@@ -1349,14 +1347,6 @@ func (daemon *Daemon) setGenericResources(conf *config.Config) error {
 	return nil
 }
 
-func setDefaultMtu(conf *config.Config) {
-	// do nothing if the config does not have the default 0 value.
-	if conf.Mtu != 0 {
-		return
-	}
-	conf.Mtu = config.DefaultNetworkMtu
-}
-
 // IsShuttingDown tells whether the daemon is shutting down or not
 func (daemon *Daemon) IsShuttingDown() bool {
 	return daemon.shutdown