From 15f78b752c793b7bb410956c154647dc93beecb3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 1 Aug 2025 17:41:48 +0200 Subject: [PATCH] daemon: make buildSandboxOptions, buildSandboxPlatformOptions more atomic The buildSandboxPlatformOptions function was given a pointer to the sboxOptions and modified it in-place. Similarly, a pointer to the container was passed and `container.HostsPath` and `container.ResolvConfPath` mutated. In cases where either of those failed, we would return an error, but the container (and sboxOptions) would already be modified. This patch; - updates the signature of buildSandboxPlatformOptions to return a fresh slice of sandbox options, which can be appended to the sboxOptions by the caller. - uses intermediate variables for `hostsPath` and `resolvConfPath`, and only mutates the container if both were obtained successfully. Signed-off-by: Sebastiaan van Stijn --- daemon/container_operations.go | 7 +++-- daemon/container_operations_unix.go | 38 ++++++++++++++------------ daemon/container_operations_windows.go | 4 +-- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/daemon/container_operations.go b/daemon/container_operations.go index cf51847f2a..9b9f957b54 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -56,10 +56,13 @@ func buildSandboxOptions(cfg *config.Config, ctr *container.Container) ([]libnet sboxOptions = append(sboxOptions, libnetwork.OptionUseExternalKey()) } - // Add platform-specific Sandbox options. - if err := buildSandboxPlatformOptions(ctr, cfg, &sboxOptions); err != nil { + // Update the container with platform-specific options, and + // add platform-specific Sandbox options. + platformOpts, err := buildSandboxPlatformOptions(ctr, cfg) + if err != nil { return nil, err } + sboxOptions = append(sboxOptions, platformOpts...) if len(ctr.HostConfig.DNS) > 0 { sboxOptions = append(sboxOptions, libnetwork.OptionDNS(ctr.HostConfig.DNS)) diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index af387fbebd..a4069414c8 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -504,9 +504,11 @@ func serviceDiscoveryOnDefaultNetwork() bool { return false } -func buildSandboxPlatformOptions(ctr *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error { - var err error - var originResolvConfPath string +func buildSandboxPlatformOptions(ctr *container.Container, cfg *config.Config) ([]libnetwork.SandboxOption, error) { + var ( + sboxOptions []libnetwork.SandboxOption + originResolvConfPath string + ) // Set the correct paths for /etc/hosts and /etc/resolv.conf, based on the // networking-mode of the container. Note that containers with "container" @@ -517,10 +519,7 @@ func buildSandboxPlatformOptions(ctr *container.Container, cfg *config.Config, s // In host-mode networking, the container does not have its own networking // namespace, so both `/etc/hosts` and `/etc/resolv.conf` should be the same // as on the host itself. The container gets a copy of these files. - *sboxOptions = append( - *sboxOptions, - libnetwork.OptionOriginHostsPath("/etc/hosts"), - ) + sboxOptions = append(sboxOptions, libnetwork.OptionOriginHostsPath("/etc/hosts")) originResolvConfPath = "/etc/resolv.conf" case ctr.HostConfig.NetworkMode.IsUserDefined(): // The container uses a user-defined network. We use the embedded DNS @@ -550,26 +549,31 @@ func buildSandboxPlatformOptions(ctr *container.Container, cfg *config.Config, s originResolvConfPath = cfg.GetResolvConf() } - // Allow tests to point at their own resolv.conf file. + // Allow tests to point at their own resolv.conf file. Note that + // this only overrides the resolvConf path, not "/etc/hosts", which + // for containers using the "host" network namespace is set above. if envPath := os.Getenv("DOCKER_TEST_RESOLV_CONF_PATH"); envPath != "" { log.G(context.TODO()).Infof("Using OriginResolvConfPath from env: %s", envPath) originResolvConfPath = envPath } - *sboxOptions = append(*sboxOptions, libnetwork.OptionOriginResolvConfPath(originResolvConfPath)) + sboxOptions = append(sboxOptions, libnetwork.OptionOriginResolvConfPath(originResolvConfPath)) - ctr.HostsPath, err = ctr.GetRootResourcePath("hosts") + hostsPath, err := ctr.GetRootResourcePath("hosts") if err != nil { - return err + return nil, err } - *sboxOptions = append(*sboxOptions, libnetwork.OptionHostsPath(ctr.HostsPath)) - - ctr.ResolvConfPath, err = ctr.GetRootResourcePath("resolv.conf") + resolvConfPath, err := ctr.GetRootResourcePath("resolv.conf") if err != nil { - return err + return nil, err } - *sboxOptions = append(*sboxOptions, libnetwork.OptionResolvConfPath(ctr.ResolvConfPath)) - return nil + ctr.HostsPath, ctr.ResolvConfPath = hostsPath, resolvConfPath + sboxOptions = append(sboxOptions, + libnetwork.OptionHostsPath(hostsPath), + libnetwork.OptionResolvConfPath(resolvConfPath), + ) + + return sboxOptions, nil } func (daemon *Daemon) initializeNetworkingPaths(ctr *container.Container, nc *container.Container) error { diff --git a/daemon/container_operations_windows.go b/daemon/container_operations_windows.go index c5745c74d4..ddd79352cb 100644 --- a/daemon/container_operations_windows.go +++ b/daemon/container_operations_windows.go @@ -170,8 +170,8 @@ func serviceDiscoveryOnDefaultNetwork() bool { return true } -func buildSandboxPlatformOptions(ctr *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error { - return nil +func buildSandboxPlatformOptions(ctr *container.Container, cfg *config.Config) ([]libnetwork.SandboxOption, error) { + return nil, nil } func (daemon *Daemon) initializeNetworkingPaths(ctr *container.Container, nc *container.Container) error {