Browse Source

Merge pull request #47512 from robmry/46329_internal_resolver_ipv6_upstream

Add IPv6 nameserver to the internal DNS's upstreams.
Sebastiaan van Stijn 1 year ago
parent
commit
1abf17c779

+ 13 - 27
libnetwork/internal/resolvconf/resolvconf.go

@@ -235,7 +235,7 @@ func (rc *ResolvConf) TransformForLegacyNw(ipv6 bool) {
 // use in a network sandbox that has an internal DNS resolver.
 //   - Add internalNS as a nameserver.
 //   - Remove other nameservers, stashing them as ExtNameServers for the
-//     internal resolver to use. (Apart from IPv6 nameservers, if keepIPv6.)
+//     internal resolver to use.
 //   - Mark ExtNameServers that must be used in the host namespace.
 //   - If no ExtNameServer addresses are found, use the defaults.
 //   - Return an error if an "ndots" option inherited from the host's config, or
@@ -244,7 +244,7 @@ func (rc *ResolvConf) TransformForLegacyNw(ipv6 bool) {
 //     option includes a ':', and an option with a matching prefix exists, it
 //     is not modified.
 func (rc *ResolvConf) TransformForIntNS(
-	keepIPv6 bool,
+	ipv6 bool,
 	internalNS netip.Addr,
 	reqdOptions []string,
 ) ([]ExtDNSEntry, error) {
@@ -254,30 +254,16 @@ func (rc *ResolvConf) TransformForIntNS(
 	// internal nameserver.
 	rc.md.ExtNameServers = nil
 	for _, addr := range rc.nameServers {
-		// The internal resolver only uses IPv4 addresses so, keep IPv6 nameservers in
-		// the container's file if keepIPv6, else drop them.
-		if addr.Is6() {
-			if keepIPv6 {
-				newNSs = append(newNSs, addr)
-			}
-		} else {
-			// Extract this NS. Mark loopback addresses that did not come from an override as
-			// 'HostLoopback'. Upstream requests for these servers will be made in the host's
-			// network namespace. (So, '--dns 127.0.0.53' means use a nameserver listening on
-			// the container's loopback interface. But, if the host's resolv.conf contains
-			// 'nameserver 127.0.0.53', the host's resolver will be used.)
-			//
-			//  TODO(robmry) - why only loopback addresses?
-			//   Addresses from the host's resolv.conf must be usable in the host's namespace,
-			//   and a lookup from the container's namespace is more expensive? And, for
-			//   example, if the host has a nameserver with an IPv6 LL address with a zone-id,
-			//   it won't work from the container's namespace (now, while the address is left in
-			//   the container's resolv.conf, or in future for the internal resolver).
-			rc.md.ExtNameServers = append(rc.md.ExtNameServers, ExtDNSEntry{
-				Addr:         addr,
-				HostLoopback: addr.IsLoopback() && !rc.md.NSOverride,
-			})
-		}
+		// Extract this NS. Mark addresses that did not come from an override, but will
+		// definitely not work in the container's namespace as 'HostLoopback'. Upstream
+		// requests for these servers will be made in the host's network namespace. (So,
+		// '--dns 127.0.0.53' means use a nameserver listening on the container's
+		// loopback interface. But, if the host's resolv.conf contains 'nameserver
+		// 127.0.0.53', the host's resolver will be used.)
+		rc.md.ExtNameServers = append(rc.md.ExtNameServers, ExtDNSEntry{
+			Addr:         addr,
+			HostLoopback: !rc.md.NSOverride && (addr.IsLoopback() || (addr.Is6() && !ipv6) || addr.Zone() != ""),
+		})
 	}
 	rc.nameServers = newNSs
 
@@ -285,7 +271,7 @@ func (rc *ResolvConf) TransformForIntNS(
 	// 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(keepIPv6) {
+		for _, addr := range defaultNSAddrs(ipv6) {
 			rc.md.ExtNameServers = append(rc.md.ExtNameServers, ExtDNSEntry{Addr: addr})
 		}
 		rc.md.UsedDefaultNS = true

+ 46 - 31
libnetwork/internal/resolvconf/resolvconf_test.go

@@ -351,16 +351,22 @@ func TestRCTransformForIntNS(t *testing.T) {
 			expExtServers: []ExtDNSEntry{mke("10.0.0.1", false)},
 		},
 		{
-			name:          "IPv4 and IPv6, ipv6 enabled",
-			input:         "nameserver 10.0.0.1\nnameserver fdb6:b8fe:b528::1",
-			ipv6:          true,
-			expExtServers: []ExtDNSEntry{mke("10.0.0.1", false)},
+			name:  "IPv4 and IPv6, ipv6 enabled",
+			input: "nameserver 10.0.0.1\nnameserver fdb6:b8fe:b528::1",
+			ipv6:  true,
+			expExtServers: []ExtDNSEntry{
+				mke("10.0.0.1", false),
+				mke("fdb6:b8fe:b528::1", false),
+			},
 		},
 		{
-			name:          "IPv4 and IPv6, ipv6 disabled",
-			input:         "nameserver 10.0.0.1\nnameserver fdb6:b8fe:b528::1",
-			ipv6:          false,
-			expExtServers: []ExtDNSEntry{mke("10.0.0.1", false)},
+			name:  "IPv4 and IPv6, ipv6 disabled",
+			input: "nameserver 10.0.0.1\nnameserver fdb6:b8fe:b528::1",
+			ipv6:  false,
+			expExtServers: []ExtDNSEntry{
+				mke("10.0.0.1", false),
+				mke("fdb6:b8fe:b528::1", true),
+			},
 		},
 		{
 			name:          "IPv4 localhost",
@@ -384,37 +390,46 @@ func TestRCTransformForIntNS(t *testing.T) {
 			expExtServers: []ExtDNSEntry{mke("127.0.0.53", true)},
 		},
 		{
-			name:  "IPv6 addr, IPv6 enabled",
-			input: "nameserver fd14:6e0e:f855::1",
-			ipv6:  true,
-			// Note that there are no ext servers in this case, the internal resolver
-			// will only look up container names. The default nameservers aren't added
-			// because the host's IPv6 nameserver remains in the container's resolv.conf,
-			// (because only IPv4 ext servers are currently allowed).
+			name:          "IPv6 addr, IPv6 enabled",
+			input:         "nameserver fd14:6e0e:f855::1",
+			ipv6:          true,
+			expExtServers: []ExtDNSEntry{mke("fd14:6e0e:f855::1", false)},
 		},
 		{
-			name:          "IPv4 and IPv6 localhost, IPv6 disabled",
-			input:         "nameserver 127.0.0.53\nnameserver ::1",
-			ipv6:          false,
-			expExtServers: []ExtDNSEntry{mke("127.0.0.53", true)},
+			name:  "IPv4 and IPv6 localhost, IPv6 disabled",
+			input: "nameserver 127.0.0.53\nnameserver ::1",
+			ipv6:  false,
+			expExtServers: []ExtDNSEntry{
+				mke("127.0.0.53", true),
+				mke("::1", true),
+			},
 		},
 		{
-			name:          "IPv4 and IPv6 localhost, ipv6 enabled",
-			input:         "nameserver 127.0.0.53\nnameserver ::1",
-			ipv6:          true,
-			expExtServers: []ExtDNSEntry{mke("127.0.0.53", true)},
+			name:  "IPv4 and IPv6 localhost, ipv6 enabled",
+			input: "nameserver 127.0.0.53\nnameserver ::1",
+			ipv6:  true,
+			expExtServers: []ExtDNSEntry{
+				mke("127.0.0.53", true),
+				mke("::1", true),
+			},
 		},
 		{
-			name:          "IPv4 localhost, IPv6 private, IPv6 enabled",
-			input:         "nameserver 127.0.0.53\nnameserver fd3e:2d1a:1f5a::1",
-			ipv6:          true,
-			expExtServers: []ExtDNSEntry{mke("127.0.0.53", true)},
+			name:  "IPv4 localhost, IPv6 private, IPv6 enabled",
+			input: "nameserver 127.0.0.53\nnameserver fd3e:2d1a:1f5a::1",
+			ipv6:  true,
+			expExtServers: []ExtDNSEntry{
+				mke("127.0.0.53", true),
+				mke("fd3e:2d1a:1f5a::1", false),
+			},
 		},
 		{
-			name:          "IPv4 localhost, IPv6 private, IPv6 disabled",
-			input:         "nameserver 127.0.0.53\nnameserver fd3e:2d1a:1f5a::1",
-			ipv6:          false,
-			expExtServers: []ExtDNSEntry{mke("127.0.0.53", true)},
+			name:  "IPv4 localhost, IPv6 private, IPv6 disabled",
+			input: "nameserver 127.0.0.53\nnameserver fd3e:2d1a:1f5a::1",
+			ipv6:  false,
+			expExtServers: []ExtDNSEntry{
+				mke("127.0.0.53", true),
+				mke("fd3e:2d1a:1f5a::1", true),
+			},
 		},
 		{
 			name:  "No host nameserver, no iv6",

+ 1 - 1
libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv4_and_IPv6,_ipv6_disabled.golden

@@ -1,5 +1,5 @@
 nameserver 127.0.0.11
 
 # Based on host file: '/etc/resolv.conf' (internal resolver)
-# ExtServers: [10.0.0.1]
+# ExtServers: [10.0.0.1 host(fdb6:b8fe:b528::1)]
 # Overrides: []

+ 1 - 2
libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv4_and_IPv6,_ipv6_enabled.golden

@@ -1,6 +1,5 @@
 nameserver 127.0.0.11
-nameserver fdb6:b8fe:b528::1
 
 # Based on host file: '/etc/resolv.conf' (internal resolver)
-# ExtServers: [10.0.0.1]
+# ExtServers: [10.0.0.1 fdb6:b8fe:b528::1]
 # Overrides: []

+ 1 - 1
libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv4_and_IPv6_localhost,_IPv6_disabled.golden

@@ -1,5 +1,5 @@
 nameserver 127.0.0.11
 
 # Based on host file: '/etc/resolv.conf' (internal resolver)
-# ExtServers: [host(127.0.0.53)]
+# ExtServers: [host(127.0.0.53) host(::1)]
 # Overrides: []

+ 1 - 2
libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv4_and_IPv6_localhost,_ipv6_enabled.golden

@@ -1,6 +1,5 @@
 nameserver 127.0.0.11
-nameserver ::1
 
 # Based on host file: '/etc/resolv.conf' (internal resolver)
-# ExtServers: [host(127.0.0.53)]
+# ExtServers: [host(127.0.0.53) host(::1)]
 # Overrides: []

+ 1 - 1
libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv4_localhost,_IPv6_private,_IPv6_disabled.golden

@@ -1,5 +1,5 @@
 nameserver 127.0.0.11
 
 # Based on host file: '/etc/resolv.conf' (internal resolver)
-# ExtServers: [host(127.0.0.53)]
+# ExtServers: [host(127.0.0.53) host(fd3e:2d1a:1f5a::1)]
 # Overrides: []

+ 1 - 2
libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv4_localhost,_IPv6_private,_IPv6_enabled.golden

@@ -1,6 +1,5 @@
 nameserver 127.0.0.11
-nameserver fd3e:2d1a:1f5a::1
 
 # Based on host file: '/etc/resolv.conf' (internal resolver)
-# ExtServers: [host(127.0.0.53)]
+# ExtServers: [host(127.0.0.53) fd3e:2d1a:1f5a::1]
 # Overrides: []

+ 1 - 1
libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv6_addr,_IPv6_enabled.golden

@@ -1,5 +1,5 @@
 nameserver 127.0.0.11
-nameserver fd14:6e0e:f855::1
 
 # Based on host file: '/etc/resolv.conf' (internal resolver)
+# ExtServers: [fd14:6e0e:f855::1]
 # Overrides: []

+ 1 - 1
libnetwork/libnetwork_linux_test.go

@@ -1838,7 +1838,7 @@ func TestResolvConf(t *testing.T) {
 			makeNet:          makeTestIPv6Network,
 			delNet:           true,
 			originResolvConf: "search pommesfrites.fr\nnameserver 12.34.56.78\nnameserver 2001:4860:4860::8888\n",
-			expResolvConf:    "nameserver 127.0.0.11\nnameserver 2001:4860:4860::8888\nsearch pommesfrites.fr\noptions ndots:0",
+			expResolvConf:    "nameserver 127.0.0.11\nsearch pommesfrites.fr\noptions ndots:0",
 		},
 		{
 			name:             "host network",

+ 8 - 11
libnetwork/sandbox_dns_unix.go

@@ -327,20 +327,17 @@ func (sb *Sandbox) rebuildDNS() error {
 		return err
 	}
 
-	// Check for IPv6 endpoints in this sandbox. If there are any, IPv6 nameservers
-	// will be left in the container's 'resolv.conf'.
-	// TODO(robmry) - preserving old behaviour, but ...
-	//   IPv6 nameservers should be treated like IPv4 ones, and used as upstream
-	//   servers for the internal resolver (if it has IPv6 connectivity). This
-	//   doesn't need to depend on whether there are currently any IPv6 endpoints.
-	//   Removing IPv6 nameservers from the container's resolv.conf will avoid the
-	//   problem that musl-libc's resolver tries all nameservers in parallel, so an
-	//   external IPv6 resolver can return NXDOMAIN before the internal resolver
-	//   returns the address of a container.
+	// Check for IPv6 endpoints in this sandbox. If there are any, and the container has
+	// IPv6 enabled, upstream requests from the internal DNS resolver can be made from
+	// the container's namespace.
+	// TODO(robmry) - this can only check networks connected when the resolver is set up,
+	//  the configuration won't be updated if the container gets an IPv6 address later.
 	ipv6 := false
 	for _, ep := range sb.endpoints {
 		if ep.network.enableIPv6 {
-			ipv6 = true
+			if en, ok := sb.ipv6Enabled(); ok {
+				ipv6 = en
+			}
 			break
 		}
 	}