From 6d35673504e65af490c02c34b4f3b136149d3401 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Mon, 17 Jun 2024 19:55:25 +0100 Subject: [PATCH 1/2] Revert "No default nameservers for internal resolver" This reverts commit d365702dbd389f3a61ad0862d0c2889022105f42. Because buildkit doesn't run an internal resolver, and it bases its /etc/resolv.conf on the host's ... when buildkit is run in a container that has 'nameserver 127.0.0.11', its build containers will use Google's DNS servers as a fallback (unless the build container uses host networking). Before, when the 127.0.0.11 resolver was not used for the default network, the buildkit container would have inherited a site-local nameserver. So, the build containers it created would also have inherited that DNS server - and they'd be able to resolve site-local hostnames. By replacing the site-local nameserver with Google's, we broke access to local DNS and its hostnames. Signed-off-by: Rob Murray --- libnetwork/internal/resolvconf/resolvconf.go | 10 ++++++++++ .../internal/resolvconf/resolvconf_test.go | 17 ++++++++++++++++- .../No_host_nameserver,_iv6.golden | 6 ++++++ ...golden => No_host_nameserver,_no_iv6.golden} | 2 ++ 4 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver,_iv6.golden rename libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/{No_host_nameserver.golden => No_host_nameserver,_no_iv6.golden} (62%) diff --git a/libnetwork/internal/resolvconf/resolvconf.go b/libnetwork/internal/resolvconf/resolvconf.go index e22983ac135a8..6621de64eb3ea 100644 --- a/libnetwork/internal/resolvconf/resolvconf.go +++ b/libnetwork/internal/resolvconf/resolvconf.go @@ -270,6 +270,16 @@ func (rc *ResolvConf) TransformForIntNS( } rc.nameServers = newNSs + // If there are no external nameservers, and the only nameserver left is the + // internal resolver, use the defaults as ext nameservers. + if len(rc.md.ExtNameServers) == 0 && len(rc.nameServers) == 1 { + log.G(context.TODO()).Info("No non-localhost DNS nameservers are left in resolv.conf. Using default external servers") + for _, addr := range defaultNSAddrs(ipv6) { + rc.md.ExtNameServers = append(rc.md.ExtNameServers, ExtDNSEntry{Addr: addr}) + } + rc.md.UsedDefaultNS = true + } + // For each option required by the nameserver, add it if not already present. If // the option is already present, don't override it. Apart from ndots - if the // ndots value is invalid and an ndots option is required, replace the existing diff --git a/libnetwork/internal/resolvconf/resolvconf_test.go b/libnetwork/internal/resolvconf/resolvconf_test.go index 7e3956610e5f4..86217a108543d 100644 --- a/libnetwork/internal/resolvconf/resolvconf_test.go +++ b/libnetwork/internal/resolvconf/resolvconf_test.go @@ -432,9 +432,24 @@ func TestRCTransformForIntNS(t *testing.T) { }, }, { - name: "No host nameserver", + name: "No host nameserver, no iv6", + input: "", + ipv6: false, + expExtServers: []ExtDNSEntry{ + mke("8.8.8.8", false), + mke("8.8.4.4", false), + }, + }, + { + name: "No host nameserver, iv6", input: "", ipv6: true, + expExtServers: []ExtDNSEntry{ + mke("8.8.8.8", false), + mke("8.8.4.4", false), + mke("2001:4860:4860::8888", false), + mke("2001:4860:4860::8844", false), + }, }, { name: "ndots present and required", diff --git a/libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver,_iv6.golden b/libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver,_iv6.golden new file mode 100644 index 0000000000000..cde7c90bd9852 --- /dev/null +++ b/libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver,_iv6.golden @@ -0,0 +1,6 @@ +nameserver 127.0.0.11 + +# Based on host file: '/etc/resolv.conf' (internal resolver) +# Used default nameservers. +# ExtServers: [8.8.8.8 8.8.4.4 2001:4860:4860::8888 2001:4860:4860::8844] +# Overrides: [] diff --git a/libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver.golden b/libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver,_no_iv6.golden similarity index 62% rename from libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver.golden rename to libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver,_no_iv6.golden index 29e3120392546..c620d3442d1e2 100644 --- a/libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver.golden +++ b/libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/No_host_nameserver,_no_iv6.golden @@ -1,4 +1,6 @@ nameserver 127.0.0.11 # Based on host file: '/etc/resolv.conf' (internal resolver) +# Used default nameservers. +# ExtServers: [8.8.8.8 8.8.4.4] # Overrides: [] From 74d77d8811603adb7d69103b5ae18a7eb5bc19e5 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Mon, 17 Jun 2024 20:00:11 +0100 Subject: [PATCH 2/2] Revert "Internal resolver for default bridge network" This reverts commit 18f4f775ed75149505d33d04d0984f34ce317348. Because buildkit doesn't run an internal resolver, and it bases its /etc/resolv.conf on the host's ... when buildkit is run in a container that has 'nameserver 127.0.0.11', its build containers will use Google's DNS servers as a fallback (unless the build container uses host networking). Before, when the 127.0.0.11 resolver was not used for the default network, the buildkit container would have inherited a site-local nameserver. So, the build containers it created would also have inherited that DNS server - and they'd be able to resolve site-local hostnames. By replacing the site-local nameserver with Google's, we broke access to local DNS and its hostnames. Signed-off-by: Rob Murray --- daemon/config/config_linux.go | 10 +- daemon/container_operations_unix.go | 45 +++++-- daemon/daemon.go | 3 + daemon/daemon_linux.go | 13 ++ daemon/daemon_unsupported.go | 2 + daemon/daemon_windows.go | 2 + daemon/network.go | 3 +- integration-cli/docker_cli_run_test.go | 138 +++++++++++++++++++--- integration-cli/docker_cli_swarm_test.go | 4 +- integration/networking/resolvconf_test.go | 59 --------- libnetwork/endpoint.go | 3 + libnetwork/sandbox_dns_unix.go | 37 ++++-- libnetwork/sandbox_dns_windows.go | 4 + 13 files changed, 227 insertions(+), 96 deletions(-) diff --git a/daemon/config/config_linux.go b/daemon/config/config_linux.go index eae3f03d80634..d22874fee34c6 100644 --- a/daemon/config/config_linux.go +++ b/daemon/config/config_linux.go @@ -87,7 +87,9 @@ type Config struct { NoNewPrivileges bool `json:"no-new-privileges,omitempty"` IpcMode string `json:"default-ipc-mode,omitempty"` CgroupNamespaceMode string `json:"default-cgroupns-mode,omitempty"` - Rootless bool `json:"rootless,omitempty"` + // ResolvConf is the path to the configuration of the host resolver + ResolvConf string `json:"resolv-conf,omitempty"` + Rootless bool `json:"rootless,omitempty"` } // GetExecRoot returns the user configured Exec-root @@ -134,6 +136,12 @@ func (conf *Config) LookupInitPath() (string, error) { return exec.LookPath(binary) } +// GetResolvConf returns the appropriate resolv.conf +// Check setupResolvConf on how this is selected +func (conf *Config) GetResolvConf() string { + return conf.ResolvConf +} + // IsSwarmCompatible defines if swarm mode can be enabled in this config func (conf *Config) IsSwarmCompatible() error { if conf.LiveRestoreEnabled { diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index 3e3d2a1766877..4c4b63b8a7018 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -419,21 +419,50 @@ func serviceDiscoveryOnDefaultNetwork() bool { func buildSandboxPlatformOptions(ctr *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error { var err error + var originResolvConfPath string - // In host-mode networking, the container does not have its own networking - // namespace, so `/etc/hosts` should be the same as on the host itself. Setting - // OptionOriginHostsPath means the container will get a copy of '/etc/hosts' from - // the host filesystem. - // Note that containers with "container" networking have been handled in - // "initializeNetworking()", so do not have to be accounted for here. - if ctr.HostConfig.NetworkMode.IsHost() { + // Set the correct paths for /etc/hosts and /etc/resolv.conf, based on the + // networking-mode of the container. Note that containers with "container" + // networking are already handled in "initializeNetworking()" before we reach + // this function, so do not have to be accounted for here. + switch { + case ctr.HostConfig.NetworkMode.IsHost(): + // 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"), ) + originResolvConfPath = "/etc/resolv.conf" + case ctr.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 + // for external DNS resolution. + // We parse the DNS server(s) that are defined in /etc/resolv.conf on + // the host, which may be a local DNS server (for example, if DNSMasq or + // systemd-resolvd are in use). The embedded DNS server forwards DNS + // resolution to the DNS server configured on the host, which in itself + // may act as a forwarder for external DNS servers. + // 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. + 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 + // embedded DNS server is available. Without the embedded DNS, we + // cannot use local DNS servers on the host (for example, if DNSMasq or + // systemd-resolvd is used). If systemd-resolvd is used, we try to + // determine the external DNS servers that are used on the host. + // This situation is not ideal, because DNS servers configured in the + // container are not updated after the container is created, but the + // 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) + originResolvConfPath = cfg.GetResolvConf() } - originResolvConfPath := "/etc/resolv.conf" // 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) diff --git a/daemon/daemon.go b/daemon/daemon.go index 5aa19f5af733a..28f3609351732 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -851,6 +851,9 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S // Do we have a disabled network? config.DisableBridge = isBridgeNetworkDisabled(config) + // Setup the resolv.conf + setupResolvConf(config) + idMapping, err := setupRemappedRoot(config) if err != nil { return nil, err diff --git a/daemon/daemon_linux.go b/daemon/daemon_linux.go index 89eff5c0fdf19..fa1b90fe0ff4f 100644 --- a/daemon/daemon_linux.go +++ b/daemon/daemon_linux.go @@ -14,6 +14,7 @@ import ( "github.com/containerd/log" "github.com/docker/docker/daemon/config" "github.com/docker/docker/libnetwork/ns" + "github.com/docker/docker/libnetwork/resolvconf" "github.com/moby/sys/mount" "github.com/moby/sys/mountinfo" "github.com/pkg/errors" @@ -135,6 +136,18 @@ func shouldUnmountRoot(root string, info *mountinfo.Info) bool { return hasMountInfoOption(info.Optional, sharedPropagationOption) } +// setupResolvConf sets the appropriate resolv.conf file if not specified +// When systemd-resolved is running the default /etc/resolv.conf points to +// localhost. In this case fetch the alternative config file that is in a +// different path so that containers can use it +// In all the other cases fallback to the default one +func setupResolvConf(config *config.Config) { + if config.ResolvConf != "" { + return + } + config.ResolvConf = resolvconf.Path() +} + // ifaceAddrs returns the IPv4 and IPv6 addresses assigned to the network // interface with name linkName. // diff --git a/daemon/daemon_unsupported.go b/daemon/daemon_unsupported.go index 4da0f2c4f8b0e..c3d419306cf53 100644 --- a/daemon/daemon_unsupported.go +++ b/daemon/daemon_unsupported.go @@ -12,6 +12,8 @@ func checkSystem() error { return errors.New("the Docker daemon is not supported on this platform") } +func setupResolvConf(_ *interface{}) {} + func getSysInfo(_ *Daemon) *sysinfo.SysInfo { return sysinfo.New() } diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index 13cad49d3f892..5cb91fee4e18f 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -555,6 +555,8 @@ func (daemon *Daemon) setupSeccompProfile(*config.Config) error { return nil } +func setupResolvConf(config *config.Config) {} + func getSysInfo(*config.Config) *sysinfo.SysInfo { return sysinfo.New() } diff --git a/daemon/network.go b/daemon/network.go index da99c193bec2e..c1ea0e6ad6788 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -878,8 +878,7 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e createOptions = append(createOptions, libnetwork.CreateOptionService(svcCfg.Name, svcCfg.ID, vip, portConfigs, svcCfg.Aliases[nwID])) } - // Don't run an internal DNS resolver for host/container/none networks. - if nm := containertypes.NetworkMode(nwName); nm.IsHost() || nm.IsContainer() || nm.IsNone() { + if !containertypes.NetworkMode(nwName).IsUserDefined() { createOptions = append(createOptions, libnetwork.CreateOptionDisableResolution()) } diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 35a41184363b7..8ddce7b4aa23a 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -27,6 +27,7 @@ import ( "github.com/docker/docker/integration-cli/cli/build" "github.com/docker/docker/integration-cli/daemon" "github.com/docker/docker/internal/testutils/specialimage" + "github.com/docker/docker/libnetwork/resolvconf" "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/runconfig" "github.com/docker/docker/testutil" @@ -1248,8 +1249,45 @@ func (s *DockerCLIRunSuite) TestRunDisallowBindMountingRootToRoot(c *testing.T) } } +// Verify that a container gets default DNS when only localhost resolvers exist +func (s *DockerCLIRunSuite) TestRunDNSDefaultOptions(c *testing.T) { + // Not applicable on Windows as this is testing Unix specific functionality + testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux) + + // preserve original resolv.conf for restoring after test + origResolvConf, err := os.ReadFile("/etc/resolv.conf") + if os.IsNotExist(err) { + c.Fatalf("/etc/resolv.conf does not exist") + } + // defer restored original conf + defer func() { + if err := os.WriteFile("/etc/resolv.conf", origResolvConf, 0o644); err != nil { + c.Fatal(err) + } + }() + + // test 3 cases: standard IPv4 localhost, commented out localhost, and IPv6 localhost + // 2 are removed from the file at container start, and the 3rd (commented out) one is ignored by + // GetNameservers(), leading to a replacement of nameservers with the default set + tmpResolvConf := []byte("nameserver 127.0.0.1\n#nameserver 127.0.2.1\nnameserver ::1") + if err := os.WriteFile("/etc/resolv.conf", tmpResolvConf, 0o644); err != nil { + c.Fatal(err) + } + + actual := cli.DockerCmd(c, "run", "busybox", "cat", "/etc/resolv.conf").Combined() + 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 8.8.8.8 nameserver 8.8.4.4" + + if actual != expected { + c.Fatalf("expected resolv.conf be: %q, but was: %q", expected, actual) + } +} + func (s *DockerCLIRunSuite) TestRunDNSOptions(c *testing.T) { - // Not applicable on Windows as Windows does not support the Unix-specific functionality of resolv.conf. + // Not applicable on Windows as Windows does not support --dns*, or + // the Unix-specific functionality of resolv.conf. testRequires(c, DaemonIsLinux) result := cli.DockerCmd(c, "run", "--dns=127.0.0.1", "--dns-search=mydomain", "--dns-opt=ndots:9", "busybox", "cat", "/etc/resolv.conf") @@ -1260,22 +1298,16 @@ func (s *DockerCLIRunSuite) TestRunDNSOptions(c *testing.T) { actual := regexp.MustCompile("(?m)^#.*$").ReplaceAllString(result.Stdout(), "") actual = strings.ReplaceAll(strings.Trim(actual, "\r\n"), "\n", " ") - if actual != "nameserver 127.0.0.11 search mydomain options ndots:9" { - c.Fatalf("nameserver 127.0.0.11 expected 'search mydomain options ndots:9', but says: %q", actual) - } - if !strings.Contains(result.Stdout(), "ExtServers: [127.0.0.1]") { - c.Fatalf("expected 'ExtServers: [127.0.0.1]' was not found in %q", result.Stdout()) + 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 = regexp.MustCompile("(?m)^#.*$").ReplaceAllString(out, "") actual = strings.ReplaceAll(strings.Trim(strings.Trim(actual, "\r\n"), " "), "\n", " ") - if actual != "nameserver 127.0.0.11 options ndots:3" { - c.Fatalf("expected 'nameserver 127.0.0.11 options ndots:3', but says: %q", actual) - } - if !strings.Contains(out, "ExtServers: [1.1.1.1]") { - c.Fatalf("expected 'ExtServers: [1.1.1.1]' was not found in %q", out) + 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) } } @@ -1285,11 +1317,87 @@ func (s *DockerCLIRunSuite) TestRunDNSRepeatOptions(c *testing.T) { actual := regexp.MustCompile("(?m)^#.*$").ReplaceAllString(out, "") actual = strings.ReplaceAll(strings.Trim(actual, "\r\n"), "\n", " ") - if actual != "nameserver 127.0.0.11 search mydomain mydomain2 options ndots:9 timeout:3" { - c.Fatalf("expected 'nameserver 127.0.0.11 search mydomain mydomain2 options ndots:9 timeout:3', but says: %q", actual) + 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) + } +} + +func (s *DockerCLIRunSuite) TestRunDNSOptionsBasedOnHostResolvConf(c *testing.T) { + // Not applicable on Windows as testing Unix specific functionality + testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux) + + origResolvConf, err := os.ReadFile("/etc/resolv.conf") + if os.IsNotExist(err) { + c.Fatalf("/etc/resolv.conf does not exist") + } + + hostNameservers := resolvconf.GetNameservers(origResolvConf, resolvconf.IP) + hostSearch := resolvconf.GetSearchDomains(origResolvConf) + + out := cli.DockerCmd(c, "run", "--dns=127.0.0.1", "busybox", "cat", "/etc/resolv.conf").Combined() + + if actualNameservers := resolvconf.GetNameservers([]byte(out), resolvconf.IP); actualNameservers[0] != "127.0.0.1" { + c.Fatalf("expected '127.0.0.1', but says: %q", actualNameservers[0]) } - if !strings.Contains(out, "ExtServers: [1.1.1.1 2.2.2.2]") { - c.Fatalf("expected 'ExtServers: [1.1.1.1 2.2.2.2]' was not found in %q", out) + + actualSearch := resolvconf.GetSearchDomains([]byte(out)) + if len(actualSearch) != len(hostSearch) { + c.Fatalf("expected %q search domain(s), but it has: %q", len(hostSearch), len(actualSearch)) + } + for i := range actualSearch { + if actualSearch[i] != hostSearch[i] { + c.Fatalf("expected %q domain, but says: %q", actualSearch[i], hostSearch[i]) + } + } + + out = cli.DockerCmd(c, "run", "--dns-search=mydomain", "busybox", "cat", "/etc/resolv.conf").Combined() + + actualNameservers := resolvconf.GetNameservers([]byte(out), resolvconf.IP) + if len(actualNameservers) != len(hostNameservers) { + c.Fatalf("expected %q nameserver(s), but it has: %q", len(hostNameservers), len(actualNameservers)) + } + for i := range actualNameservers { + if actualNameservers[i] != hostNameservers[i] { + c.Fatalf("expected %q nameserver, but says: %q", actualNameservers[i], hostNameservers[i]) + } + } + + if actualSearch = resolvconf.GetSearchDomains([]byte(out)); actualSearch[0] != "mydomain" { + c.Fatalf("expected 'mydomain', but says: %q", actualSearch[0]) + } + + // test with file + tmpResolvConf := []byte("search example.com\nnameserver 12.34.56.78\nnameserver 127.0.0.1") + if err := os.WriteFile("/etc/resolv.conf", tmpResolvConf, 0o644); err != nil { + c.Fatal(err) + } + // put the old resolvconf back + defer func() { + if err := os.WriteFile("/etc/resolv.conf", origResolvConf, 0o644); err != nil { + c.Fatal(err) + } + }() + + resolvConf, err := os.ReadFile("/etc/resolv.conf") + if os.IsNotExist(err) { + c.Fatalf("/etc/resolv.conf does not exist") + } + + hostSearch = resolvconf.GetSearchDomains(resolvConf) + + out = cli.DockerCmd(c, "run", "busybox", "cat", "/etc/resolv.conf").Combined() + if actualNameservers = resolvconf.GetNameservers([]byte(out), resolvconf.IP); actualNameservers[0] != "12.34.56.78" || len(actualNameservers) != 1 { + c.Fatalf("expected '12.34.56.78', but has: %v", actualNameservers) + } + + actualSearch = resolvconf.GetSearchDomains([]byte(out)) + if len(actualSearch) != len(hostSearch) { + c.Fatalf("expected %q search domain(s), but it has: %q", len(hostSearch), len(actualSearch)) + } + for i := range actualSearch { + if actualSearch[i] != hostSearch[i] { + c.Fatalf("expected %q domain, but says: %q", actualSearch[i], hostSearch[i]) + } } } diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index 17c82ac93963e..50479f496dd25 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -974,16 +974,14 @@ func (s *DockerSwarmSuite) TestDNSConfig(c *testing.T) { id := strings.TrimSpace(out) // Compare against expected output. - expectedOutput1 := "nameserver 127.0.0.11" + expectedOutput1 := "nameserver 1.2.3.4" expectedOutput2 := "search example.com" expectedOutput3 := "options timeout:3" - expectedOutput4 := "ExtServers: [1.2.3.4]" out, err = d.Cmd("exec", id, "cat", "/etc/resolv.conf") assert.NilError(c, err, out) assert.Assert(c, strings.Contains(out, expectedOutput1), "Expected '%s', but got %q", expectedOutput1, out) assert.Assert(c, strings.Contains(out, expectedOutput2), "Expected '%s', but got %q", expectedOutput2, out) assert.Assert(c, strings.Contains(out, expectedOutput3), "Expected '%s', but got %q", expectedOutput3, out) - assert.Assert(c, strings.Contains(out, expectedOutput4), "Expected '%s', but got %q", expectedOutput4, out) } func (s *DockerSwarmSuite) TestDNSConfigUpdate(c *testing.T) { diff --git a/integration/networking/resolvconf_test.go b/integration/networking/resolvconf_test.go index ff2ea3f2fa65b..33663c7a7a66b 100644 --- a/integration/networking/resolvconf_test.go +++ b/integration/networking/resolvconf_test.go @@ -10,10 +10,8 @@ import ( containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/mount" - networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/network" - "github.com/docker/docker/testutil" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" @@ -212,60 +210,3 @@ func TestNslookupWindows(t *testing.T) { // can only be changed in daemon.json using feature flag "windows-dns-proxy". assert.Check(t, is.Contains(res.Stdout.String(), "Addresses:")) } - -// Check that containers on the default bridge network can use a host's resolver -// running on a loopback interface (via the internal resolver), but the internal -// resolver is not populated with DNS names for containers (no service discovery -// on the legacy/default bridge network). -// (Assumes the host does not already have a DNS server on 127.0.0.1.) -func TestDefaultBridgeDNS(t *testing.T) { - skip.If(t, testEnv.DaemonInfo.OSType == "windows", "No resolv.conf on Windows") - skip.If(t, testEnv.IsRootless, "Can't use resolver on host in rootless mode") - ctx := setupTest(t) - - // Start a DNS server on the loopback interface. - network.StartDaftDNS(t, "127.0.0.1") - - // Set up a temp resolv.conf pointing at that DNS server, and a daemon using it. - tmpFileName := network.WriteTempResolvConf(t, "127.0.0.1") - d := daemon.New(t, daemon.WithEnvVars("DOCKER_TEST_RESOLV_CONF_PATH="+tmpFileName)) - d.StartWithBusybox(ctx, t) - defer d.Stop(t) - - c := d.NewClientT(t) - defer c.Close() - - // Create a container on the default bridge network. - const ctrName = "ctrname" - ctrId := container.Run(ctx, t, c, container.WithName(ctrName)) - defer c.ContainerRemove(ctx, ctrId, containertypes.RemoveOptions{Force: true}) - - // Expect the external DNS server to respond to a request from the container. - t.Run("external dns", func(t *testing.T) { - ctx := testutil.StartSpan(ctx, t) - res, err := container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"}) - assert.NilError(t, err) - assert.Check(t, is.Equal(res.ExitCode, 0)) - assert.Check(t, is.Contains(res.Stdout(), network.DNSRespAddr)) - }) - - // Expect the external DNS server to respond to a request from the container - // for the container's own name - it won't be recognised as a container name - // because there's no service resolution on the default bridge. - t.Run("no internal name resolution", func(t *testing.T) { - ctx := testutil.StartSpan(ctx, t) - res, err := container.Exec(ctx, c, ctrId, []string{"nslookup", ctrName}) - assert.NilError(t, err) - assert.Check(t, is.Equal(res.ExitCode, 0)) - assert.Check(t, is.Contains(res.Stdout(), network.DNSRespAddr)) - }) - - // Check that inspect output has no DNSNames for the container. - t.Run("no dnsnames in inspect", func(t *testing.T) { - ctx := testutil.StartSpan(ctx, t) - inspect := container.Inspect(ctx, t, c, ctrId) - net, ok := inspect.NetworkSettings.Networks[networktypes.NetworkBridge] - assert.Check(t, ok, "expected to find bridge network in inspect output") - assert.Check(t, is.Nil(net.DNSNames)) - }) -} diff --git a/libnetwork/endpoint.go b/libnetwork/endpoint.go index 09101633f5d8b..332f4b3e91ee4 100644 --- a/libnetwork/endpoint.go +++ b/libnetwork/endpoint.go @@ -543,6 +543,9 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint if err := sb.updateHostsFile(ctx, ep.getEtcHostsAddrs()); err != nil { return err } + if err = sb.updateDNS(n.enableIPv6); err != nil { + return err + } // Current endpoint providing external connectivity for the sandbox extEp := sb.getGatewayEndpoint() diff --git a/libnetwork/sandbox_dns_unix.go b/libnetwork/sandbox_dns_unix.go index 56122cea1865d..dc86ca645648b 100644 --- a/libnetwork/sandbox_dns_unix.go +++ b/libnetwork/sandbox_dns_unix.go @@ -302,6 +302,27 @@ func (sb *Sandbox) setupDNS() error { return rc.WriteFile(sb.config.resolvConfPath, sb.config.resolvConfHashFile, filePerm) } +// Called when an endpoint has joined the sandbox. +func (sb *Sandbox) updateDNS(ipv6Enabled bool) error { + if mod, err := resolvconf.UserModified(sb.config.resolvConfPath, sb.config.resolvConfHashFile); err != nil || mod { + return err + } + + // Load the host's resolv.conf as a starting point. + rc, err := sb.loadResolvConf(sb.config.getOriginResolvConfPath()) + if err != nil { + return err + } + // For host-networking, no further change is needed. + if !sb.config.useDefaultSandBox { + // The legacy bridge network has no internal nameserver. So, strip localhost + // nameservers from the host's config, then add default nameservers if there + // are none remaining. + rc.TransformForLegacyNw(ipv6Enabled) + } + return rc.WriteFile(sb.config.resolvConfPath, sb.config.resolvConfHashFile, filePerm) +} + // Embedded DNS server has to be enabled for this sandbox. Rebuild the container's resolv.conf. func (sb *Sandbox) rebuildDNS() error { // Don't touch the file if the user has modified it. @@ -347,14 +368,14 @@ func (sb *Sandbox) rebuildDNS() error { // upstream nameservers. sb.setExternalResolvers(extNameServers) - // Write the file for the container - not updating the hash file (so, no further - // updates will be made). - // - // Once the default resolver is configured, there's not normally any need to change - // the container's resolv.conf. A possible exception is if an IPv6 endpoint is added - // late, so it becomes possible to make upstream requests to IPv6 resolvers from - // the container's network namespace. But, the resolver's ext-servers won't currently - // be reconfigured once it's started. + // Write the file for the container - preserving old behaviour, not updating the + // hash file (so, no further updates will be made). + // TODO(robmry) - I think that's probably accidental, I can't find a reason for it, + // and the old resolvconf.Build() function wrote the file but not the hash, which + // is surprising. But, before fixing it, a guard/flag needs to be added to + // sb.updateDNS() to make sure that when an endpoint joins a sandbox that already + // has an internal resolver, the container's resolv.conf is still (re)configured + // for an internal resolver. return rc.WriteFile(sb.config.resolvConfPath, "", filePerm) } diff --git a/libnetwork/sandbox_dns_windows.go b/libnetwork/sandbox_dns_windows.go index 88082a92feddb..d3dc8b95957ca 100644 --- a/libnetwork/sandbox_dns_windows.go +++ b/libnetwork/sandbox_dns_windows.go @@ -23,3 +23,7 @@ func (sb *Sandbox) updateHostsFile(_ context.Context, ifaceIP []string) error { } func (sb *Sandbox) deleteHostsEntries(recs []etchosts.Record) {} + +func (sb *Sandbox) updateDNS(ipv6Enabled bool) error { + return nil +}