Merge pull request moby#48290 from robmry/v6only/dns

IPv6 only: DNS changes

libnetwork/etchosts/etchosts.go

@@ -143,6 +143,10 @@ func mergeRecords(path string, recs []Record) ([]byte, error) {
 }
 
 // Delete deletes an arbitrary number of Records already existing in /etc/hosts file
+//
+// FIXME(robmry) - this only matches on hostname, not address. So, if a container
+// is connected to two networks then disconnected from one of them, the hosts
+// entries for both networks are deleted.
 func Delete(path string, recs []Record) error {
 	defer pathLock(path)()
 

libnetwork/internal/resolvconf/resolvconf.go

@@ -239,46 +239,30 @@ func (rc *ResolvConf) TransformForLegacyNw(ipv6 bool) {
 //   - Add internalNS as a nameserver.
 //   - Remove other nameservers, stashing them as ExtNameServers for the
 //     internal resolver to use.
-//   - Mark ExtNameServers that must be used in the host namespace.
+//   - Mark ExtNameServers that must be accessed from 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
-//     supplied in an override is not valid.
 //   - Ensure there's an 'options' value for each entry in reqdOptions. If the
 //     option includes a ':', and an option with a matching prefix exists, it
 //     is not modified.
 func (rc *ResolvConf) TransformForIntNS(
-	ipv6 bool,
 	internalNS netip.Addr,
 	reqdOptions []string,
 ) ([]ExtDNSEntry, error) {
-	// The transformed config must list the internal nameserver.
-	newNSs := []netip.Addr{internalNS}
-	// Filter out other nameservers, keeping them for use as upstream nameservers by the
-	// internal nameserver.
+	// Add each of the nameservers read from the host's /etc/hosts or supplied as an
+	// override to ExtNameServers, for the internal resolver to talk to. Addresses
+	// read from host config should be accessed from the host's network namespace
+	// (HostLoopback=true). Addresses supplied as overrides are accessed from the
+	// container's namespace.
 	rc.md.ExtNameServers = nil
 	for _, addr := range rc.nameServers {
-		// 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() != ""),
+			HostLoopback: !rc.md.NSOverride,
 		})
 	}
-	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
-	}
+	// The transformed config only lists the internal nameserver.
+	rc.nameServers = []netip.Addr{internalNS}
 
 	// 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

libnetwork/internal/resolvconf/resolvconf_test.go

@@ -340,7 +340,6 @@ func TestRCTransformForIntNS(t *testing.T) {
 		name            string
 		input           string
 		intNameServer   string
-		ipv6            bool
 		overrideNS      []string
 		overrideOptions []string
 		reqdOptions     []string
@@ -350,109 +349,42 @@ func TestRCTransformForIntNS(t *testing.T) {
 		{
 			name:          "IPv4 only",
 			input:         "nameserver 10.0.0.1",
-			expExtServers: []ExtDNSEntry{mke("10.0.0.1", false)},
+			expExtServers: []ExtDNSEntry{mke("10.0.0.1", true)},
 		},
 		{
 			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),
+				mke("10.0.0.1", true),
 				mke("fdb6:b8fe:b528::1", true),
 			},
 		},
 		{
 			name:          "IPv4 localhost",
 			input:         "nameserver 127.0.0.53",
-			ipv6:          false,
 			expExtServers: []ExtDNSEntry{mke("127.0.0.53", true)},
 		},
 		{
 			// Overriding the nameserver with a localhost address means use the container's
 			// loopback interface, not the host's.
 			name:          "IPv4 localhost override",
 			input:         "nameserver 10.0.0.1",
-			ipv6:          false,
 			overrideNS:    []string{"127.0.0.53"},
 			expExtServers: []ExtDNSEntry{mke("127.0.0.53", false)},
 		},
 		{
-			name:          "IPv4 localhost, ipv6 enabled",
-			input:         "nameserver 127.0.0.53",
-			ipv6:          true,
-			expExtServers: []ExtDNSEntry{mke("127.0.0.53", true)},
-		},
-		{
-			name:          "IPv6 addr, IPv6 enabled",
+			name:          "IPv6 only",
 			input:         "nameserver fd14:6e0e:f855::1",
-			ipv6:          true,
-			expExtServers: []ExtDNSEntry{mke("fd14:6e0e:f855::1", false)},
+			expExtServers: []ExtDNSEntry{mke("fd14:6e0e:f855::1", true)},
 		},
 		{
-			name:  "IPv4 and IPv6 localhost, IPv6 disabled",
+			name:  "IPv4 and IPv6 localhost",
 			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),
-				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),
-				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),
-				mke("fd3e:2d1a:1f5a::1", true),
-			},
-		},
-		{
-			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",
 			input:         "nameserver 127.0.0.53\noptions ndots:1",
@@ -496,7 +428,7 @@ func TestRCTransformForIntNS(t *testing.T) {
 				rc.OverrideOptions(tc.overrideOptions)
 			}
 			intNS := netip.MustParseAddr(tc.intNameServer)
-			extNameServers, err := rc.TransformForIntNS(tc.ipv6, intNS, tc.reqdOptions)
+			extNameServers, err := rc.TransformForIntNS(intNS, tc.reqdOptions)
 			if tc.expErr != "" {
 				assert.Check(t, is.ErrorContains(err, tc.expErr))
 				return
@@ -559,7 +491,7 @@ func TestRCTransformForIntNSInvalidNdots(t *testing.T) {
 			content := "nameserver 8.8.8.8\n" + tc.options
 			rc, err := Parse(bytes.NewBuffer([]byte(content)), "/etc/resolv.conf")
 			assert.NilError(t, err)
-			_, err = rc.TransformForIntNS(false, netip.MustParseAddr("127.0.0.11"), tc.reqdOptions)
+			_, err = rc.TransformForIntNS(netip.MustParseAddr("127.0.0.11"), tc.reqdOptions)
 			assert.NilError(t, err)
 
 			val, found := rc.Option("ndots")

libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv4_and_IPv6,_ipv6_enabled.golden

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

libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv4_only.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: [host(10.0.0.1)]
 # Overrides: []

libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv4_localhost,_ipv6_enabled.golden → libnetwork/internal/resolvconf/testdata/TestRCTransformForIntNS/IPv6_only.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(fd14:6e0e:f855::1)]
 # Overrides: []

libnetwork/libnetwork_internal_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/docker/docker/internal/testutils/netnsutils"
 	"github.com/docker/docker/libnetwork/config"
 	"github.com/docker/docker/libnetwork/driverapi"
+	"github.com/docker/docker/libnetwork/etchosts"
 	"github.com/docker/docker/libnetwork/ipams/defaultipam"
 	"github.com/docker/docker/libnetwork/ipamutils"
 	"github.com/docker/docker/libnetwork/netlabel"
@@ -358,6 +359,91 @@ func TestAuxAddresses(t *testing.T) {
 	}
 }
 
+func TestUpdateSvcRecord(t *testing.T) {
+	skip.If(t, runtime.GOOS == "windows", "bridge driver and IPv6, only works on linux")
+
+	tests := []struct {
+		name       string
+		epName     string
+		addr4      string
+		addr6      string
+		expSvcRecs []etchosts.Record
+	}{
+		{
+			name:   "v4only",
+			epName: "ep4",
+			addr4:  "172.16.0.2/24",
+			expSvcRecs: []etchosts.Record{
+				{Hosts: "id-ep4", IP: "172.16.0.2"},
+			},
+		},
+		/* TODO(robmry) - add this test when the bridge driver understands v6-only
+		{
+			name:   "v6only",
+			epName: "ep6",
+			addr6:  "fde6:045d:b2aa::2/64",
+			expSvcRecs: []etchosts.Record{
+				{Hosts: "id-ep6", IP: "fde6:45d:b2aa::2"},
+			},
+		},
+		*/
+		{
+			name:   "dual-stack",
+			epName: "ep46",
+			addr4:  "172.16.1.2/24",
+			addr6:  "fd60:8677:5a4c::2/64",
+			expSvcRecs: []etchosts.Record{
+				{Hosts: "id-ep46", IP: "172.16.1.2"},
+				{Hosts: "id-ep46", IP: "fd60:8677:5a4c::2"},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			defer netnsutils.SetupTestOSContext(t)()
+			ctrlr, err := New(OptionBoltdbWithRandomDBFile(t))
+			assert.NilError(t, err)
+			defer ctrlr.Stop()
+
+			var ipam4, ipam6 []*IpamConf
+			var ip4, ip6 net.IP
+			if tc.addr4 != "" {
+				var net4 *net.IPNet
+				ip4, net4, err = net.ParseCIDR(tc.addr4)
+				assert.NilError(t, err)
+				ipam4 = []*IpamConf{{PreferredPool: net4.String()}}
+			}
+			if tc.addr6 != "" {
+				var net6 *net.IPNet
+				ip6, net6, err = net.ParseCIDR(tc.addr6)
+				assert.NilError(t, err)
+				ipam6 = []*IpamConf{{PreferredPool: net6.String()}}
+			}
+			n, err := ctrlr.NewNetwork("bridge", "net1", "", nil,
+				NetworkOptionEnableIPv4(tc.addr4 != ""),
+				NetworkOptionEnableIPv6(tc.addr6 != ""),
+				NetworkOptionIpam(defaultipam.DriverName, "", ipam4, ipam6, nil),
+			)
+			assert.NilError(t, err)
+			ep, err := n.CreateEndpoint(context.Background(), tc.epName,
+				CreateOptionDNSNames([]string{tc.epName, "id-" + tc.epName}),
+				CreateOptionIpam(ip4, ip6, nil, nil),
+			)
+			assert.NilError(t, err)
+
+			n.updateSvcRecord(context.Background(), ep, true)
+			recs := n.getSvcRecords(ep)
+			assert.Check(t, is.DeepEqual(recs, tc.expSvcRecs))
+
+			n.updateSvcRecord(context.Background(), ep, false)
+			recs = n.getSvcRecords(ep)
+			assert.Check(t, is.Nil(recs))
+		})
+	}
+}
+
 func TestSRVServiceQuery(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows", "test only works on linux")