Skip to content

Commit

Permalink
Merge pull request moby#47041 from robmry/46968_refactor_resolvconf
Browse files Browse the repository at this point in the history
Refactor 'resolv.conf' generation.
  • Loading branch information
thaJeztah authored Feb 29, 2024
2 parents d40b140 + beb97f7 commit 6c3b352
Show file tree
Hide file tree
Showing 50 changed files with 1,811 additions and 781 deletions.
20 changes: 11 additions & 9 deletions daemon/container_operations_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ func serviceDiscoveryOnDefaultNetwork() bool {

func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error {
var err error
var 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"
Expand All @@ -432,8 +433,8 @@ func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Con
*sboxOptions = append(
*sboxOptions,
libnetwork.OptionOriginHostsPath("/etc/hosts"),
libnetwork.OptionOriginResolvConfPath("/etc/resolv.conf"),
)
originResolvConfPath = "/etc/resolv.conf"
case container.HostConfig.NetworkMode.IsUserDefined():
// The container uses a user-defined network. We use the embedded DNS
// server for container name resolution and to act as a DNS forwarder
Expand All @@ -446,10 +447,7 @@ func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Con
// If systemd-resolvd is used, the "upstream" DNS servers can be found in
// /run/systemd/resolve/resolv.conf. We do not query those DNS servers
// directly, as they can be dynamically reconfigured.
*sboxOptions = append(
*sboxOptions,
libnetwork.OptionOriginResolvConfPath("/etc/resolv.conf"),
)
originResolvConfPath = "/etc/resolv.conf"
default:
// For other situations, such as the default bridge network, container
// discovery / name resolution is handled through /etc/hosts, and no
Expand All @@ -462,11 +460,15 @@ func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Con
// DNS servers on the host can be dynamically updated.
//
// Copy the host's resolv.conf for the container (/run/systemd/resolve/resolv.conf or /etc/resolv.conf)
*sboxOptions = append(
*sboxOptions,
libnetwork.OptionOriginResolvConfPath(cfg.GetResolvConf()),
)
originResolvConfPath = cfg.GetResolvConf()
}

// Allow tests to point at their own resolv.conf file.
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))

container.HostsPath, err = container.GetRootResourcePath("hosts")
if err != nil {
Expand Down
24 changes: 14 additions & 10 deletions integration-cli/docker_cli_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1275,10 +1275,11 @@ func (s *DockerCLIRunSuite) TestRunDNSDefaultOptions(c *testing.T) {
}

actual := cli.DockerCmd(c, "run", "busybox", "cat", "/etc/resolv.conf").Combined()
// check that the actual defaults are appended to the commented out
// localhost resolver (which should be preserved)
actual = regexp.MustCompile("(?m)^#.*$").ReplaceAllString(actual, "")
actual = strings.ReplaceAll(strings.Trim(actual, "\r\n"), "\n", " ")
// NOTE: if we ever change the defaults from google dns, this will break
expected := "#nameserver 127.0.2.1\n\nnameserver 8.8.8.8\nnameserver 8.8.4.4\n"
expected := "nameserver 8.8.8.8 nameserver 8.8.4.4"

if actual != expected {
c.Fatalf("expected resolv.conf be: %q, but was: %q", expected, actual)
}
Expand All @@ -1295,14 +1296,16 @@ func (s *DockerCLIRunSuite) TestRunDNSOptions(c *testing.T) {
c.Fatalf("Expected warning on stderr about localhost resolver, but got %q", result.Stderr())
}

actual := strings.ReplaceAll(strings.Trim(result.Stdout(), "\r\n"), "\n", " ")
if actual != "search mydomain nameserver 127.0.0.1 options ndots:9" {
c.Fatalf("expected 'search mydomain nameserver 127.0.0.1 options ndots:9', but says: %q", actual)
actual := regexp.MustCompile("(?m)^#.*$").ReplaceAllString(result.Stdout(), "")
actual = strings.ReplaceAll(strings.Trim(actual, "\r\n"), "\n", " ")
if actual != "nameserver 127.0.0.1 search mydomain options ndots:9" {
c.Fatalf("nameserver 127.0.0.1 expected 'search mydomain options ndots:9', but says: %q", actual)
}

out := cli.DockerCmd(c, "run", "--dns=1.1.1.1", "--dns-search=.", "--dns-opt=ndots:3", "busybox", "cat", "/etc/resolv.conf").Combined()

actual = strings.ReplaceAll(strings.Trim(strings.Trim(out, "\r\n"), " "), "\n", " ")
actual = regexp.MustCompile("(?m)^#.*$").ReplaceAllString(out, "")
actual = strings.ReplaceAll(strings.Trim(strings.Trim(actual, "\r\n"), " "), "\n", " ")
if actual != "nameserver 1.1.1.1 options ndots:3" {
c.Fatalf("expected 'nameserver 1.1.1.1 options ndots:3', but says: %q", actual)
}
Expand All @@ -1312,9 +1315,10 @@ func (s *DockerCLIRunSuite) TestRunDNSRepeatOptions(c *testing.T) {
testRequires(c, DaemonIsLinux)
out := cli.DockerCmd(c, "run", "--dns=1.1.1.1", "--dns=2.2.2.2", "--dns-search=mydomain", "--dns-search=mydomain2", "--dns-opt=ndots:9", "--dns-opt=timeout:3", "busybox", "cat", "/etc/resolv.conf").Stdout()

actual := strings.ReplaceAll(strings.Trim(out, "\r\n"), "\n", " ")
if actual != "search mydomain mydomain2 nameserver 1.1.1.1 nameserver 2.2.2.2 options ndots:9 timeout:3" {
c.Fatalf("expected 'search mydomain mydomain2 nameserver 1.1.1.1 nameserver 2.2.2.2 options ndots:9 timeout:3', but says: %q", actual)
actual := regexp.MustCompile("(?m)^#.*$").ReplaceAllString(out, "")
actual = strings.ReplaceAll(strings.Trim(actual, "\r\n"), "\n", " ")
if actual != "nameserver 1.1.1.1 nameserver 2.2.2.2 search mydomain mydomain2 options ndots:9 timeout:3" {
c.Fatalf("expected 'nameserver 1.1.1.1 nameserver 2.2.2.2 search mydomain mydomain2 options ndots:9 timeout:3', but says: %q", actual)
}
}

Expand Down
72 changes: 72 additions & 0 deletions integration/networking/resolvconf_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package networking

import (
"os"
"strings"
"testing"

containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/integration/internal/container"
"github.com/docker/docker/integration/internal/network"
"github.com/docker/docker/testutil/daemon"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
"gotest.tools/v3/skip"
)

// Regression test for https://github.com/moby/moby/issues/46968
func TestResolvConfLocalhostIPv6(t *testing.T) {
// No "/etc/resolv.conf" on Windows.
skip.If(t, testEnv.DaemonInfo.OSType == "windows")

ctx := setupTest(t)

// Write a resolv.conf that only contains a loopback address.
// Not using t.TempDir() here because in rootless mode, while the temporary
// directory gets mode 0777, it's a subdir of an 0700 directory owned by root.
// So, it's not accessible by the daemon.
f, err := os.CreateTemp("", "resolv.conf")
assert.NilError(t, err)
defer os.Remove(f.Name())
err = f.Chmod(0644)
assert.NilError(t, err)
f.Write([]byte("nameserver 127.0.0.53\n"))

d := daemon.New(t, daemon.WithEnvVars("DOCKER_TEST_RESOLV_CONF_PATH="+f.Name()))
d.StartWithBusybox(ctx, t, "--experimental", "--ip6tables")
defer d.Stop(t)

c := d.NewClientT(t)
defer c.Close()

netName := "nnn"
network.CreateNoError(ctx, t, c, netName,
network.WithDriver("bridge"),
network.WithIPv6(),
network.WithIPAM("fd49:b5ef:36d9::/64", "fd49:b5ef:36d9::1"),
)
defer network.RemoveNoError(ctx, t, c, netName)

result := container.RunAttach(ctx, t, c,
container.WithImage("busybox:latest"),
container.WithNetworkMode(netName),
container.WithCmd("cat", "/etc/resolv.conf"),
)
defer c.ContainerRemove(ctx, result.ContainerID, containertypes.RemoveOptions{
Force: true,
})

output := strings.ReplaceAll(result.Stdout.String(), f.Name(), "RESOLV.CONF")
assert.Check(t, is.Equal(output, `# Generated by Docker Engine.
# This file can be edited; Docker Engine will not make further changes once it
# has been modified.
nameserver 127.0.0.11
options ndots:0
# Based on host file: 'RESOLV.CONF' (internal resolver)
# ExtServers: [host(127.0.0.53)]
# Overrides: []
# Option ndots from: internal
`))
}
Loading

0 comments on commit 6c3b352

Please sign in to comment.