diff --git a/api/server/server.go b/api/server/server.go index 89c2902427..5f01a8b1dd 100644 --- a/api/server/server.go +++ b/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 diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go index 7b74250fad..4e3b4f2f07 100644 --- a/cmd/dockerd/config.go +++ b/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") diff --git a/cmd/dockerd/config_unix.go b/cmd/dockerd/config_unix.go index 6f7d5daee2..1c750eb20e 100644 --- a/cmd/dockerd/config_unix.go +++ b/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) diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 5c8eb1df73..a5ff9b6ba4 100644 --- a/cmd/dockerd/daemon.go +++ b/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...) diff --git a/cmd/dockerd/daemon_test.go b/cmd/dockerd/daemon_test.go index c4e6ee3548..805243d2e2 100644 --- a/cmd/dockerd/daemon_test.go +++ b/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 } diff --git a/cmd/dockerd/daemon_unix.go b/cmd/dockerd/daemon_unix.go index 3072678b61..9869043a7f 100644 --- a/cmd/dockerd/daemon_unix.go +++ b/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 diff --git a/daemon/config/config.go b/daemon/config/config.go index f3c97a673b..0afcbd4437 100644 --- a/daemon/config/config.go +++ b/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) } diff --git a/daemon/config/config_linux_test.go b/daemon/config/config_linux_test.go index cfc635567d..a9f571f223 100644 --- a/daemon/config/config_linux_test.go +++ b/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) + 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")) - 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()) + 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) } } diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index 6da6ec2aba..ae571c0182 100644 --- a/daemon/config/config_test.go +++ b/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) { diff --git a/daemon/config/config_windows_test.go b/daemon/config/config_windows_test.go index 95cfaf255a..94b198b944 100644 --- a/daemon/config/config_windows_test.go +++ b/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", "") + 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(c, flags, configFile) + 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)) diff --git a/daemon/daemon.go b/daemon/daemon.go index 9ae5bfd297..54b45d5f6b 100644 --- a/daemon/daemon.go +++ b/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