daemon/config: New(): initialize config with platform-specific defaults

This centralizes more defaults, to be part of the config struct that's
created, instead of interweaving the defaults with other code in various
places.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

12 files changed, 102 insertions, 106 deletions

diff --git a/cmd/dockerd/config.go b/cmd/dockerd/config.go
index a1c5a3c42b..a4bb8699f1 100644
--- a/cmd/dockerd/config.go
+++ b/cmd/dockerd/config.go
@@ -12,20 +12,6 @@ const defaultTrustKeyFile = "key.json"
 
 // installCommonConfigFlags adds flags to the pflag.FlagSet to configure the daemon
 func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
-	var err error
-	conf.Pidfile, err = getDefaultPidFile()
-	if err != nil {
-		return err
-	}
-	conf.Root, err = getDefaultDataRoot()
-	if err != nil {
-		return err
-	}
-	conf.ExecRoot, err = getDefaultExecRoot()
-	if err != nil {
-		return err
-	}
-
 	var (
 		allowNonDistributable = opts.NewNamedListOptsRef("allow-nondistributable-artifacts", &conf.AllowNondistributableArtifacts, registry.ValidateIndexName)
 		registryMirrors       = opts.NewNamedListOptsRef("registry-mirrors", &conf.Mirrors, registry.ValidateMirror)
diff --git a/cmd/dockerd/config_unix.go b/cmd/dockerd/config_unix.go
index 1c750eb20e..89148d81c4 100644
--- a/cmd/dockerd/config_unix.go
+++ b/cmd/dockerd/config_unix.go
@@ -5,18 +5,13 @@ package main
 
 import (
 	"net"
-	"os/exec"
 	"path/filepath"
 
-	"github.com/containerd/cgroups"
-	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/pkg/homedir"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/rootless"
-	units "github.com/docker/go-units"
-	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
 )
 
@@ -27,12 +22,6 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 		return err
 	}
 
-	conf.Ulimits = make(map[string]*units.Ulimit)
-
-	// Set default value for `--default-shm-size`
-	conf.ShmSize = opts.MemBytes(config.DefaultShmSize)
-	conf.Runtimes = make(map[string]types.Runtime)
-
 	// Then platform-specific install flags
 	flags.Var(opts.NewNamedRuntimeOpt("runtimes", &conf.Runtimes, config.StockRuntimeName), "add-runtime", "Register an additional OCI compatible runtime")
 	flags.StringVarP(&conf.SocketGroup, "group", "G", "docker", "Group for the unix socket")
@@ -53,16 +42,7 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 	flags.BoolVar(&conf.BridgeConfig.InterContainerCommunication, "icc", true, "Enable inter-container communication")
 	flags.IPVar(&conf.BridgeConfig.DefaultIP, "ip", net.IPv4zero, "Default IP when binding container ports")
 	flags.BoolVar(&conf.BridgeConfig.EnableUserlandProxy, "userland-proxy", true, "Use userland proxy for loopback traffic")
-	defaultUserlandProxyPath := ""
-	if rootless.RunningWithRootlessKit() {
-		var err error
-		// use rootlesskit-docker-proxy for exposing the ports in RootlessKit netns to the initial namespace.
-		defaultUserlandProxyPath, err = exec.LookPath(rootless.RootlessKitDockerProxyBinary)
-		if err != nil {
-			return errors.Wrapf(err, "running with RootlessKit, but %s not installed", rootless.RootlessKitDockerProxyBinary)
-		}
-	}
-	flags.StringVar(&conf.BridgeConfig.UserlandProxyPath, "userland-proxy-path", defaultUserlandProxyPath, "Path to the userland proxy binary")
+	flags.StringVar(&conf.BridgeConfig.UserlandProxyPath, "userland-proxy-path", conf.BridgeConfig.UserlandProxyPath, "Path to the userland proxy binary")
 	flags.StringVar(&conf.CgroupParent, "cgroup-parent", "", "Set parent cgroup for all containers")
 	flags.StringVar(&conf.RemappedRoot, "userns-remap", "", "User/Group setting for user namespaces")
 	flags.BoolVar(&conf.LiveRestoreEnabled, "live-restore", false, "Enable live restore of docker when containers are still running")
@@ -71,19 +51,15 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 	flags.StringVar(&conf.InitPath, "init-path", "", "Path to the docker-init binary")
 	flags.Int64Var(&conf.CPURealtimePeriod, "cpu-rt-period", 0, "Limit the CPU real-time period in microseconds for the parent cgroup for all containers (not supported with cgroups v2)")
 	flags.Int64Var(&conf.CPURealtimeRuntime, "cpu-rt-runtime", 0, "Limit the CPU real-time runtime in microseconds for the parent cgroup for all containers (not supported with cgroups v2)")
-	flags.StringVar(&conf.SeccompProfile, "seccomp-profile", config.SeccompProfileDefault, `Path to seccomp profile. Use "unconfined" to disable the default seccomp profile`)
+	flags.StringVar(&conf.SeccompProfile, "seccomp-profile", conf.SeccompProfile, `Path to seccomp profile. Use "unconfined" to disable the default seccomp profile`)
 	flags.Var(&conf.ShmSize, "default-shm-size", "Default shm size for containers")
 	flags.BoolVar(&conf.NoNewPrivileges, "no-new-privileges", false, "Set no-new-privileges by default for new containers")
-	flags.StringVar(&conf.IpcMode, "default-ipc-mode", string(config.DefaultIpcMode), `Default mode for containers ipc ("shareable" | "private")`)
+	flags.StringVar(&conf.IpcMode, "default-ipc-mode", conf.IpcMode, `Default mode for containers ipc ("shareable" | "private")`)
 	flags.Var(&conf.NetworkConfig.DefaultAddressPools, "default-address-pool", "Default address pools for node specific local networks")
 	// rootless needs to be explicitly specified for running "rootful" dockerd in rootless dockerd (#38702)
-	// Note that defaultUserlandProxyPath and honorXDG are configured according to the value of rootless.RunningWithRootlessKit, not the value of --rootless.
-	flags.BoolVar(&conf.Rootless, "rootless", rootless.RunningWithRootlessKit(), "Enable rootless mode; typically used with RootlessKit")
-	defaultCgroupNamespaceMode := config.DefaultCgroupNamespaceMode
-	if cgroups.Mode() != cgroups.Unified {
-		defaultCgroupNamespaceMode = config.DefaultCgroupV1NamespaceMode
-	}
-	flags.StringVar(&conf.CgroupNamespaceMode, "default-cgroupns-mode", string(defaultCgroupNamespaceMode), `Default mode for containers cgroup namespace ("host" | "private")`)
+	// Note that conf.BridgeConfig.UserlandProxyPath and honorXDG are configured according to the value of rootless.RunningWithRootlessKit, not the value of --rootless.
+	flags.BoolVar(&conf.Rootless, "rootless", conf.Rootless, "Enable rootless mode; typically used with RootlessKit")
+	flags.StringVar(&conf.CgroupNamespaceMode, "default-cgroupns-mode", conf.CgroupNamespaceMode, `Default mode for containers cgroup namespace ("host" | "private")`)
 	return nil
 }
 
@@ -97,36 +73,3 @@ func configureCertsDir() {
 		}
 	}
 }
-
-func getDefaultPidFile() (string, error) {
-	if !honorXDG {
-		return "/var/run/docker.pid", nil
-	}
-	runtimeDir, err := homedir.GetRuntimeDir()
-	if err != nil {
-		return "", err
-	}
-	return filepath.Join(runtimeDir, "docker.pid"), nil
-}
-
-func getDefaultDataRoot() (string, error) {
-	if !honorXDG {
-		return "/var/lib/docker", nil
-	}
-	dataHome, err := homedir.GetDataHome()
-	if err != nil {
-		return "", err
-	}
-	return filepath.Join(dataHome, "docker"), nil
-}
-
-func getDefaultExecRoot() (string, error) {
-	if !honorXDG {
-		return "/var/run/docker", nil
-	}
-	runtimeDir, err := homedir.GetRuntimeDir()
-	if err != nil {
-		return "", err
-	}
-	return filepath.Join(runtimeDir, "docker"), nil
-}
diff --git a/cmd/dockerd/config_unix_test.go b/cmd/dockerd/config_unix_test.go
index 1ffe4c1fbf..b07ce8a24b 100644
--- a/cmd/dockerd/config_unix_test.go
+++ b/cmd/dockerd/config_unix_test.go
@@ -15,8 +15,9 @@ import (
 func TestDaemonParseShmSize(t *testing.T) {
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 
-	conf := &config.Config{}
-	err := installConfigFlags(conf, flags)
+	conf, err := config.New()
+	assert.NilError(t, err)
+	err = installConfigFlags(conf, flags)
 	assert.NilError(t, err)
 	// By default `--default-shm-size=64M`
 	assert.Check(t, is.Equal(int64(64*1024*1024), conf.ShmSize.Value()))
diff --git a/cmd/dockerd/config_windows.go b/cmd/dockerd/config_windows.go
index df96861066..d8bebf4486 100644
--- a/cmd/dockerd/config_windows.go
+++ b/cmd/dockerd/config_windows.go
@@ -1,25 +1,10 @@
 package main
 
 import (
-	"os"
-	"path/filepath"
-
 	"github.com/docker/docker/daemon/config"
 	"github.com/spf13/pflag"
 )
 
-func getDefaultPidFile() (string, error) {
-	return "", nil
-}
-
-func getDefaultDataRoot() (string, error) {
-	return filepath.Join(os.Getenv("programdata"), "docker"), nil
-}
-
-func getDefaultExecRoot() (string, error) {
-	return filepath.Join(os.Getenv("programdata"), "docker", "exec-root"), nil
-}
-
 // installConfigFlags adds flags to the pflag.FlagSet to configure the daemon
 func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
 	// First handle install flags which are consistent cross-platform
diff --git a/cmd/dockerd/docker.go b/cmd/dockerd/docker.go
index 4b6d55d653..90df537bdd 100644
--- a/cmd/dockerd/docker.go
+++ b/cmd/dockerd/docker.go
@@ -21,7 +21,11 @@ var (
 )
 
 func newDaemonCommand() (*cobra.Command, error) {
-	opts := newDaemonOptions(config.New())
+	cfg, err := config.New()
+	if err != nil {
+		return nil, err
+	}
+	opts := newDaemonOptions(cfg)
 
 	cmd := &cobra.Command{
 		Use:           "dockerd [OPTIONS]",
diff --git a/daemon/config/config.go b/daemon/config/config.go
index 645beb8c03..482bbe2d62 100644
--- a/daemon/config/config.go
+++ b/daemon/config/config.go
@@ -275,9 +275,10 @@ func (conf *Config) IsValueSet(name string) bool {
 	return ok
 }
 
-// New returns a new fully initialized Config struct
-func New() *Config {
-	return &Config{
+// New returns a new fully initialized Config struct with default values set.
+func New() (*Config, error) {
+	// platform-agnostic default values for the Config.
+	cfg := &Config{
 		CommonConfig: CommonConfig{
 			ShutdownTimeout: DefaultShutdownTimeout,
 			LogConfig: LogConfig{
@@ -295,6 +296,12 @@ func New() *Config {
 			DefaultRuntime:            StockRuntimeName,
 		},
 	}
+
+	if err := setPlatformDefaults(cfg); err != nil {
+		return nil, err
+	}
+
+	return cfg, nil
 }
 
 // GetConflictFreeLabels validates Labels for conflict
@@ -329,7 +336,10 @@ func Reload(configFile string, flags *pflag.FlagSet, reload func(*Config)) error
 		if flags.Changed("config-file") || !os.IsNotExist(err) {
 			return errors.Wrapf(err, "unable to configure the Docker daemon with file %s", configFile)
 		}
-		newConfig = New()
+		newConfig, err = New()
+		if err != nil {
+			return err
+		}
 	}
 
 	// Check if duplicate label-keys with different values are found
diff --git a/daemon/config/config_linux.go b/daemon/config/config_linux.go
index 27d1c11806..70d1efda8d 100644
--- a/daemon/config/config_linux.go
+++ b/daemon/config/config_linux.go
@@ -3,11 +3,17 @@ package config // import "github.com/docker/docker/daemon/config"
 import (
 	"fmt"
 	"net"
+	"os/exec"
+	"path/filepath"
 
+	"github.com/containerd/cgroups"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/opts"
+	"github.com/docker/docker/pkg/homedir"
+	"github.com/docker/docker/rootless"
 	units "github.com/docker/go-units"
+	"github.com/pkg/errors"
 )
 
 const (
@@ -161,3 +167,47 @@ func (conf *Config) ValidatePlatformConfig() error {
 func (conf *Config) IsRootless() bool {
 	return conf.Rootless
 }
+
+func setPlatformDefaults(cfg *Config) error {
+	cfg.Ulimits = make(map[string]*units.Ulimit)
+	cfg.ShmSize = opts.MemBytes(DefaultShmSize)
+	cfg.SeccompProfile = SeccompProfileDefault
+	cfg.IpcMode = string(DefaultIpcMode)
+	cfg.Runtimes = make(map[string]types.Runtime)
+
+	if cgroups.Mode() != cgroups.Unified {
+		cfg.CgroupNamespaceMode = string(DefaultCgroupV1NamespaceMode)
+	} else {
+		cfg.CgroupNamespaceMode = string(DefaultCgroupNamespaceMode)
+	}
+
+	if rootless.RunningWithRootlessKit() {
+		cfg.Rootless = true
+
+		var err error
+		// use rootlesskit-docker-proxy for exposing the ports in RootlessKit netns to the initial namespace.
+		cfg.BridgeConfig.UserlandProxyPath, err = exec.LookPath(rootless.RootlessKitDockerProxyBinary)
+		if err != nil {
+			return errors.Wrapf(err, "running with RootlessKit, but %s not installed", rootless.RootlessKitDockerProxyBinary)
+		}
+
+		dataHome, err := homedir.GetDataHome()
+		if err != nil {
+			return err
+		}
+		runtimeDir, err := homedir.GetRuntimeDir()
+		if err != nil {
+			return err
+		}
+
+		cfg.Root = filepath.Join(dataHome, "docker")
+		cfg.ExecRoot = filepath.Join(runtimeDir, "docker")
+		cfg.Pidfile = filepath.Join(runtimeDir, "docker.pid")
+	} else {
+		cfg.Root = "/var/lib/docker"
+		cfg.ExecRoot = "/var/run/docker"
+		cfg.Pidfile = "/var/run/docker.pid"
+	}
+
+	return nil
+}
diff --git a/daemon/config/config_linux_test.go b/daemon/config/config_linux_test.go
index a9f571f223..7b8d776281 100644
--- a/daemon/config/config_linux_test.go
+++ b/daemon/config/config_linux_test.go
@@ -69,7 +69,8 @@ func TestDaemonConfigurationMerge(t *testing.T) {
 	file := fs.NewFile(t, "docker-config", fs.WithContent(configFileData))
 	defer file.Remove()
 
-	conf := New()
+	conf, err := New()
+	assert.NilError(t, err)
 
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 	flags.BoolVarP(&conf.Debug, "debug", "D", false, "")
diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go
index ae571c0182..882dcaedff 100644
--- a/daemon/config/config_test.go
+++ b/daemon/config/config_test.go
@@ -338,13 +338,14 @@ func TestValidateConfigurationErrors(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			cfg := New()
+			cfg, err := New()
+			assert.NilError(t, err)
 			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)
+			err = Validate(cfg)
 			assert.Error(t, err, tc.expectedErr)
 		})
 	}
@@ -481,12 +482,13 @@ func TestValidateConfiguration(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			// Start with a config with all defaults set, so that we only
-			cfg := New()
+			cfg, err := New()
+			assert.NilError(t, err)
 			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)
+			err = Validate(cfg)
 			assert.NilError(t, err)
 		})
 	}
diff --git a/daemon/config/config_windows.go b/daemon/config/config_windows.go
index 05b3b667bc..7958abaac1 100644
--- a/daemon/config/config_windows.go
+++ b/daemon/config/config_windows.go
@@ -1,6 +1,9 @@
 package config // import "github.com/docker/docker/daemon/config"
 
 import (
+	"os"
+	"path/filepath"
+
 	"github.com/docker/docker/api/types"
 )
 
@@ -62,3 +65,10 @@ func (conf *Config) ValidatePlatformConfig() error {
 func (conf *Config) IsRootless() bool {
 	return false
 }
+
+func setPlatformDefaults(cfg *Config) error {
+	cfg.Root = filepath.Join(os.Getenv("programdata"), "docker")
+	cfg.ExecRoot = filepath.Join(os.Getenv("programdata"), "docker", "exec-root")
+	cfg.Pidfile = filepath.Join(cfg.Root, "docker.pid")
+	return nil
+}
diff --git a/daemon/config/config_windows_test.go b/daemon/config/config_windows_test.go
index 94b198b944..69c3c97372 100644
--- a/daemon/config/config_windows_test.go
+++ b/daemon/config/config_windows_test.go
@@ -23,7 +23,8 @@ func TestDaemonConfigurationMerge(t *testing.T) {
 
 	f.Close()
 
-	conf := New()
+	conf, err := New()
+	assert.NilError(t, err)
 
 	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 	flags.BoolVarP(&conf.Debug, "debug", "D", false, "")
diff --git a/daemon/runtime_unix_test.go b/daemon/runtime_unix_test.go
index 9d98b0da15..f4b55ac6e4 100644
--- a/daemon/runtime_unix_test.go
+++ b/daemon/runtime_unix_test.go
@@ -24,7 +24,10 @@ func TestGetRuntime(t *testing.T) {
 	const configuredRtName = "my/custom.shim.v1"
 	configuredRuntime := types.Runtime{Path: "/bin/true"}
 
-	d := &Daemon{configStore: config.New()}
+	cfg, err := config.New()
+	assert.NilError(t, err)
+
+	d := &Daemon{configStore: cfg}
 	d.configStore.Root = t.TempDir()
 	assert.Assert(t, os.Mkdir(filepath.Join(d.configStore.Root, "runtimes"), 0700))
 	d.configStore.Runtimes = map[string]types.Runtime{