Use netip.Addr instead of string when building /etc/hosts

Also, libnetwork: Sandbox.buildHostsFile: rename var that shadowed type

Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Rob Murray <rob.murray@docker.com>

integration/networking/etchosts_test.go

@@ -45,8 +45,8 @@ func TestEtcHostsIpv6(t *testing.T) {
 			expIPv6Enabled: true,
 			expEtcHosts: `127.0.0.1	localhost
 ::1	localhost ip6-localhost ip6-loopback
-fe00::0	ip6-localnet
-ff00::0	ip6-mcastprefix
+fe00::	ip6-localnet
+ff00::	ip6-mcastprefix
 ff02::1	ip6-allnodes
 ff02::2	ip6-allrouters
 `,

libnetwork/docs/vagrant.md

@@ -57,8 +57,8 @@ Start a container and check the content of `/etc/hosts`.
     172.21.0.3	df479e660658
     127.0.0.1	localhost
     ::1	localhost ip6-localhost ip6-loopback
-    fe00::0	ip6-localnet
-    ff00::0	ip6-mcastprefix
+    fe00::	ip6-localnet
+    ff00::	ip6-mcastprefix
     ff02::1	ip6-allnodes
     ff02::2	ip6-allrouters
     172.21.0.3	distracted_bohr

libnetwork/endpoint.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net"
+	"net/netip"
 	"strings"
 	"sync"
 
@@ -938,7 +939,7 @@ func (ep *Endpoint) getSandbox() (*Sandbox, bool) {
 }
 
 // Return a list of this endpoint's addresses to add to '/etc/hosts'.
-func (ep *Endpoint) getEtcHostsAddrs() []string {
+func (ep *Endpoint) getEtcHostsAddrs() []netip.Addr {
 	ep.mu.Lock()
 	defer ep.mu.Unlock()
 
@@ -947,12 +948,16 @@ func (ep *Endpoint) getEtcHostsAddrs() []string {
 		return nil
 	}
 
-	var addresses []string
+	var addresses []netip.Addr
 	if ep.iface.addr != nil {
-		addresses = append(addresses, ep.iface.addr.IP.String())
+		if addr, ok := netip.AddrFromSlice(ep.iface.addr.IP); ok {
+			addresses = append(addresses, addr)
+		}
 	}
 	if ep.iface.addrv6 != nil {
-		addresses = append(addresses, ep.iface.addrv6.IP.String())
+		if addr, ok := netip.AddrFromSlice(ep.iface.addrv6.IP); ok {
+			addresses = append(addresses, addr)
+		}
 	}
 	return addresses
 }

libnetwork/endpoint_unix_test.go

@@ -17,8 +17,8 @@ func TestHostsEntries(t *testing.T) {
 
 	expectedHostsFile := `127.0.0.1	localhost
 ::1	localhost ip6-localhost ip6-loopback
-fe00::0	ip6-localnet
-ff00::0	ip6-mcastprefix
+fe00::	ip6-localnet
+ff00::	ip6-mcastprefix
 ff02::1	ip6-allnodes
 ff02::2	ip6-allrouters
 192.168.222.2	somehost.example.com somehost

libnetwork/etchosts/etchosts.go

@@ -14,7 +14,7 @@ import (
 // Record Structure for a single host record
 type Record struct {
 	Hosts string
-	IP    string
+	IP    netip.Addr
 }
 
 // WriteTo writes record to file and returns bytes written or error
@@ -26,14 +26,14 @@ func (r Record) WriteTo(w io.Writer) (int64, error) {
 var (
 	// Default hosts config records slice
 	defaultContentIPv4 = []Record{
-		{Hosts: "localhost", IP: "127.0.0.1"},
+		{Hosts: "localhost", IP: netip.MustParseAddr("127.0.0.1")},
 	}
 	defaultContentIPv6 = []Record{
-		{Hosts: "localhost ip6-localhost ip6-loopback", IP: "::1"},
-		{Hosts: "ip6-localnet", IP: "fe00::0"},
-		{Hosts: "ip6-mcastprefix", IP: "ff00::0"},
-		{Hosts: "ip6-allnodes", IP: "ff02::1"},
-		{Hosts: "ip6-allrouters", IP: "ff02::2"},
+		{Hosts: "localhost ip6-localhost ip6-loopback", IP: netip.IPv6Loopback()},
+		{Hosts: "ip6-localnet", IP: netip.MustParseAddr("fe00::")},
+		{Hosts: "ip6-mcastprefix", IP: netip.MustParseAddr("ff00::")},
+		{Hosts: "ip6-allnodes", IP: netip.MustParseAddr("ff02::1")},
+		{Hosts: "ip6-allrouters", IP: netip.MustParseAddr("ff02::2")},
 	}
 
 	// A cache of path level locks for synchronizing /etc/hosts
@@ -79,8 +79,7 @@ func Build(path string, extraContent []Record) error {
 func BuildNoIPv6(path string, extraContent []Record) error {
 	var ipv4ExtraContent []Record
 	for _, rec := range extraContent {
-		addr, err := netip.ParseAddr(rec.IP)
-		if err != nil || !addr.Is6() {
+		if !rec.IP.Is6() {
 			ipv4ExtraContent = append(ipv4ExtraContent, rec)
 		}
 	}

libnetwork/etchosts/etchosts_test.go

@@ -3,6 +3,7 @@ package etchosts
 import (
 	"bytes"
 	"fmt"
+	"net/netip"
 	"os"
 	"path/filepath"
 	"testing"
@@ -30,10 +31,11 @@ func TestBuildDefault(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		expected := "127.0.0.1\tlocalhost\n::1\tlocalhost ip6-localhost ip6-loopback\nfe00::0\tip6-localnet\nff00::0\tip6-mcastprefix\nff02::1\tip6-allnodes\nff02::2\tip6-allrouters\n"
+		expected := "127.0.0.1\tlocalhost\n::1\tlocalhost ip6-localhost ip6-loopback\nfe00::\tip6-localnet\nff00::\tip6-mcastprefix\nff02::1\tip6-allnodes\nff02::2\tip6-allrouters\n"
 
-		if expected != string(content) {
-			t.Fatalf("Expected to find '%s' got '%s'", expected, content)
+		actual := string(content)
+		if expected != actual {
+			assert.Check(t, is.Equal(actual, expected))
 		}
 	}
 }
@@ -45,11 +47,11 @@ func TestBuildNoIPv6(t *testing.T) {
 	err := BuildNoIPv6(filename, []Record{
 		{
 			Hosts: "another.example",
-			IP:    "fdbb:c59c:d015::3",
+			IP:    netip.MustParseAddr("fdbb:c59c:d015::3"),
 		},
 		{
 			Hosts: "another.example",
-			IP:    "10.11.12.13",
+			IP:    netip.MustParseAddr("10.11.12.13"),
 		},
 	})
 	assert.NilError(t, err)
@@ -68,7 +70,7 @@ func TestUpdate(t *testing.T) {
 	if err := Build(file.Name(), []Record{
 		{
 			"testhostname.testdomainname testhostname",
-			"10.11.12.13",
+			netip.MustParseAddr("10.11.12.13"),
 		},
 	}); err != nil {
 		t.Fatal(err)
@@ -114,15 +116,15 @@ func TestUpdateIgnoresPrefixedHostname(t *testing.T) {
 	if err := Build(file.Name(), []Record{
 		{
 			Hosts: "prefix",
-			IP:    "2.2.2.2",
+			IP:    netip.MustParseAddr("2.2.2.2"),
 		},
 		{
 			Hosts: "prefixAndMore",
-			IP:    "3.3.3.3",
+			IP:    netip.MustParseAddr("3.3.3.3"),
 		},
 		{
 			Hosts: "unaffectedHost",
-			IP:    "4.4.4.4",
+			IP:    netip.MustParseAddr("4.4.4.4"),
 		},
 	}); err != nil {
 		t.Fatal(err)
@@ -170,11 +172,11 @@ func TestDeleteIgnoresPrefixedHostname(t *testing.T) {
 	if err := Add(file.Name(), []Record{
 		{
 			Hosts: "prefix",
-			IP:    "1.1.1.1",
+			IP:    netip.MustParseAddr("1.1.1.1"),
 		},
 		{
 			Hosts: "prefixAndMore",
-			IP:    "2.2.2.2",
+			IP:    netip.MustParseAddr("2.2.2.2"),
 		},
 	}); err != nil {
 		t.Fatal(err)
@@ -183,7 +185,7 @@ func TestDeleteIgnoresPrefixedHostname(t *testing.T) {
 	if err := Delete(file.Name(), []Record{
 		{
 			Hosts: "prefix",
-			IP:    "1.1.1.1",
+			IP:    netip.MustParseAddr("1.1.1.1"),
 		},
 	}); err != nil {
 		t.Fatal(err)
@@ -235,7 +237,7 @@ func TestAdd(t *testing.T) {
 	if err := Add(file.Name(), []Record{
 		{
 			Hosts: "testhostname",
-			IP:    "2.2.2.2",
+			IP:    netip.MustParseAddr("2.2.2.2"),
 		},
 	}); err != nil {
 		t.Fatal(err)
@@ -283,7 +285,7 @@ func TestDeleteNewline(t *testing.T) {
 	rec := []Record{
 		{
 			Hosts: "prefix",
-			IP:    "2.2.2.2",
+			IP:    netip.MustParseAddr("2.2.2.2"),
 		},
 	}
 	if err := Delete(file.Name(), rec); err != nil {
@@ -306,15 +308,15 @@ func TestDelete(t *testing.T) {
 	if err := Add(file.Name(), []Record{
 		{
 			Hosts: "testhostname1",
-			IP:    "1.1.1.1",
+			IP:    netip.MustParseAddr("1.1.1.1"),
 		},
 		{
 			Hosts: "testhostname2",
-			IP:    "2.2.2.2",
+			IP:    netip.MustParseAddr("2.2.2.2"),
 		},
 		{
 			Hosts: "testhostname3",
-			IP:    "3.3.3.3",
+			IP:    netip.MustParseAddr("3.3.3.3"),
 		},
 	}); err != nil {
 		t.Fatal(err)
@@ -323,11 +325,11 @@ func TestDelete(t *testing.T) {
 	if err := Delete(file.Name(), []Record{
 		{
 			Hosts: "testhostname1",
-			IP:    "1.1.1.1",
+			IP:    netip.MustParseAddr("1.1.1.1"),
 		},
 		{
 			Hosts: "testhostname3",
-			IP:    "3.3.3.3",
+			IP:    netip.MustParseAddr("3.3.3.3"),
 		},
 	}); err != nil {
 		t.Fatal(err)
@@ -362,19 +364,20 @@ func TestConcurrentWrites(t *testing.T) {
 	if err := Add(file.Name(), []Record{
 		{
 			Hosts: "inithostname",
-			IP:    "172.17.0.1",
+			IP:    netip.MustParseAddr("172.17.0.1"),
 		},
 	}); err != nil {
 		t.Fatal(err)
 	}
 
 	group := new(errgroup.Group)
-	for i := 0; i < 10; i++ {
-		i := i
+	for i := byte(0); i < 10; i++ {
 		group.Go(func() error {
+			addr, ok := netip.AddrFromSlice([]byte{i, i, i, i})
+			assert.Assert(t, ok)
 			rec := []Record{
 				{
-					IP:    fmt.Sprintf("%d.%d.%d.%d", i, i, i, i),
+					IP:    addr,
 					Hosts: fmt.Sprintf("testhostname%d", i),
 				},
 			}
@@ -426,10 +429,12 @@ func benchDelete(b *testing.B) {
 
 	var records []Record
 	var toDelete []Record
-	for i := 0; i < 255; i++ {
+	for i := byte(0); i < 255; i++ {
+		addr, ok := netip.AddrFromSlice([]byte{i, i, i, i})
+		assert.Assert(b, ok)
 		record := Record{
 			Hosts: fmt.Sprintf("testhostname%d", i),
-			IP:    fmt.Sprintf("%d.%d.%d.%d", i, i, i, i),
+			IP:    addr,
 		}
 		records = append(records, record)
 		if i%2 == 0 {

libnetwork/libnetwork_internal_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net"
+	"net/netip"
 	"reflect"
 	"runtime"
 	"testing"
@@ -20,6 +21,7 @@ import (
 	"github.com/docker/docker/libnetwork/netutils"
 	"github.com/docker/docker/libnetwork/scope"
 	"github.com/docker/docker/libnetwork/types"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/skip"
@@ -374,7 +376,7 @@ func TestUpdateSvcRecord(t *testing.T) {
 			epName: "ep4",
 			addr4:  "172.16.0.2/24",
 			expSvcRecs: []etchosts.Record{
-				{Hosts: "id-ep4", IP: "172.16.0.2"},
+				{Hosts: "id-ep4", IP: netip.MustParseAddr("172.16.0.2")},
 			},
 		},
 		/* TODO(robmry) - add this test when the bridge driver understands v6-only
@@ -393,8 +395,8 @@ func TestUpdateSvcRecord(t *testing.T) {
 			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"},
+				{Hosts: "id-ep46", IP: netip.MustParseAddr("172.16.1.2")},
+				{Hosts: "id-ep46", IP: netip.MustParseAddr("fd60:8677:5a4c::2")},
 			},
 		},
 	}
@@ -435,7 +437,7 @@ func TestUpdateSvcRecord(t *testing.T) {
 
 			n.updateSvcRecord(context.Background(), ep, true)
 			recs := n.getSvcRecords(ep)
-			assert.Check(t, is.DeepEqual(recs, tc.expSvcRecs))
+			assert.Check(t, is.DeepEqual(recs, tc.expSvcRecs, cmpopts.EquateComparable(netip.Addr{})))
 
 			n.updateSvcRecord(context.Background(), ep, false)
 			recs = n.getSvcRecords(ep)

libnetwork/network.go

@@ -1505,14 +1505,25 @@ func (n *Network) getSvcRecords(ep *Endpoint) []etchosts.Record {
 				continue
 			}
 			if len(mapEntryList) == 0 {
-				log.G(context.TODO()).Warnf("Found empty list of IP addresses for service %s on network %s (%s)", k, n.name, n.id)
+				log.G(context.TODO()).WithFields(log.Fields{
+					"service": k,
+					"net":     n.name,
+					"nid":     n.id,
+				}).Warn("Found empty list of IP addresses")
+				continue
+			}
+			addr, err := netip.ParseAddr(mapEntryList[0].ip)
+			if err != nil {
+				log.G(context.TODO()).WithFields(log.Fields{
+					"service": k,
+					"net":     n.name,
+					"nid":     n.id,
+					"addr":    mapEntryList[0].ip,
+				}).Warn("Bad IP address")
 				continue
 			}
 
-			recs = append(recs, etchosts.Record{
-				Hosts: k,
-				IP:    mapEntryList[0].ip,
-			})
+			recs = append(recs, etchosts.Record{Hosts: k, IP: addr})
 		}
 	}
 

libnetwork/sandbox_dns_unix.go

@@ -46,7 +46,7 @@ func (sb *Sandbox) UpdateHostsEntry(regexp, ip string) error {
 // support for IPv6 can be determined and IPv6 hosts will be included/excluded
 // accordingly.
 func (sb *Sandbox) rebuildHostsFile(ctx context.Context) error {
-	var ifaceIPs []string
+	var ifaceIPs []netip.Addr
 	for _, ep := range sb.Endpoints() {
 		ifaceIPs = append(ifaceIPs, ep.getEtcHostsAddrs()...)
 	}
@@ -115,7 +115,7 @@ func (sb *Sandbox) setupResolutionFiles(ctx context.Context) error {
 	return sb.setupDNS()
 }
 
-func (sb *Sandbox) buildHostsFile(ctx context.Context, ifaceIPs []string) error {
+func (sb *Sandbox) buildHostsFile(ctx context.Context, ifaceIPs []netip.Addr) error {
 	ctx, span := otel.Tracer("").Start(ctx, "libnetwork.buildHostsFile")
 	defer span.End()
 
@@ -137,8 +137,12 @@ func (sb *Sandbox) buildHostsFile(ctx context.Context, ifaceIPs []string) error
 	}
 
 	extraContent := make([]etchosts.Record, 0, len(sb.config.extraHosts)+len(ifaceIPs))
-	for _, extraHost := range sb.config.extraHosts {
-		extraContent = append(extraContent, etchosts.Record{Hosts: extraHost.name, IP: extraHost.IP})
+	for _, host := range sb.config.extraHosts {
+		addr, err := netip.ParseAddr(host.IP)
+		if err != nil {
+			return errdefs.InvalidParameter(fmt.Errorf("could not parse extra host IP %s: %v", host.IP, err))
+		}
+		extraContent = append(extraContent, etchosts.Record{Hosts: host.name, IP: addr})
 	}
 	extraContent = append(extraContent, sb.makeHostsRecs(ifaceIPs)...)
 
@@ -154,7 +158,7 @@ func (sb *Sandbox) buildHostsFile(ctx context.Context, ifaceIPs []string) error
 	return nil
 }
 
-func (sb *Sandbox) makeHostsRecs(ifaceIPs []string) []etchosts.Record {
+func (sb *Sandbox) makeHostsRecs(ifaceIPs []netip.Addr) []etchosts.Record {
 	if len(ifaceIPs) == 0 {
 		return nil
 	}
@@ -177,23 +181,21 @@ func (sb *Sandbox) makeHostsRecs(ifaceIPs []string) []etchosts.Record {
 	return recs
 }
 
-func (sb *Sandbox) addHostsEntries(ctx context.Context, ifaceAddrs []string) {
+func (sb *Sandbox) addHostsEntries(ctx context.Context, ifaceAddrs []netip.Addr) {
 	ctx, span := otel.Tracer("").Start(ctx, "libnetwork.addHostsEntries")
 	defer span.End()
 
-	recs := sb.makeHostsRecs(ifaceAddrs)
-
 	// Assume IPv6 support, unless it's definitely disabled.
 	if en, ok := sb.IPv6Enabled(); ok && !en {
-		var filtered []etchosts.Record
-		for _, rec := range recs {
-			if addr, err := netip.ParseAddr(rec.IP); err == nil && !addr.Is6() {
-				filtered = append(filtered, rec)
+		var filtered []netip.Addr
+		for _, addr := range ifaceAddrs {
+			if !addr.Is6() {
+				filtered = append(filtered, addr)
 			}
 		}
-		recs = filtered
+		ifaceAddrs = filtered
 	}
-	if err := etchosts.Add(sb.config.hostsPath, recs); err != nil {
+	if err := etchosts.Add(sb.config.hostsPath, sb.makeHostsRecs(ifaceAddrs)); err != nil {
 		log.G(context.TODO()).Warnf("Failed adding service host entries to the running container: %v", err)
 	}
 }

libnetwork/sandbox_dns_windows.go

@@ -4,6 +4,7 @@ package libnetwork
 
 import (
 	"context"
+	"net/netip"
 
 	"github.com/docker/docker/libnetwork/etchosts"
 )
@@ -18,7 +19,7 @@ func (sb *Sandbox) restoreHostsPath() {}
 
 func (sb *Sandbox) restoreResolvConfPath() {}
 
-func (sb *Sandbox) addHostsEntries(_ context.Context, ifaceIP []string) error {
+func (sb *Sandbox) addHostsEntries(_ context.Context, ifaceIP []netip.Addr) error {
 	return nil
 }