From c2a09d272110c5839d5792b6d7669d2a07de1c00 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Tue, 5 Nov 2024 16:30:26 +0000 Subject: [PATCH 1/7] Don't update /etc/hosts separately for each initial network Signed-off-by: Rob Murray --- libnetwork/endpoint.go | 4 +-- libnetwork/etchosts/etchosts.go | 4 +-- libnetwork/sandbox_dns_unix.go | 58 +++++++++++++++---------------- libnetwork/sandbox_dns_windows.go | 2 +- 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/libnetwork/endpoint.go b/libnetwork/endpoint.go index f47d281547..032f26787d 100644 --- a/libnetwork/endpoint.go +++ b/libnetwork/endpoint.go @@ -551,9 +551,7 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint } } - if err := sb.updateHostsFile(ctx, ep.getEtcHostsAddrs()); err != nil { - return err - } + sb.addHostsEntries(ctx, ep.getEtcHostsAddrs()) if err := sb.updateDNS(n.enableIPv6); err != nil { return err } diff --git a/libnetwork/etchosts/etchosts.go b/libnetwork/etchosts/etchosts.go index 945451820a..69370135ec 100644 --- a/libnetwork/etchosts/etchosts.go +++ b/libnetwork/etchosts/etchosts.go @@ -106,12 +106,12 @@ func build(path string, contents ...[]Record) error { // Add adds an arbitrary number of Records to an already existing /etc/hosts file func Add(path string, recs []Record) error { - defer pathLock(path)() - if len(recs) == 0 { return nil } + defer pathLock(path)() + b, err := mergeRecords(path, recs) if err != nil { return err diff --git a/libnetwork/sandbox_dns_unix.go b/libnetwork/sandbox_dns_unix.go index 95467e0243..b5abc10739 100644 --- a/libnetwork/sandbox_dns_unix.go +++ b/libnetwork/sandbox_dns_unix.go @@ -46,13 +46,12 @@ 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 { - if err := sb.buildHostsFile(); err != nil { - return errdefs.System(err) - } + var ifaceIPs []string for _, ep := range sb.Endpoints() { - if err := sb.updateHostsFile(ctx, ep.getEtcHostsAddrs()); err != nil { - return errdefs.System(err) - } + ifaceIPs = append(ifaceIPs, ep.getEtcHostsAddrs()...) + } + if err := sb.buildHostsFile(ctx, ifaceIPs); err != nil { + return errdefs.System(err) } return nil } @@ -109,14 +108,17 @@ func (sb *Sandbox) setupResolutionFiles(ctx context.Context) error { if err := createBasePath(dir); err != nil { return err } - if err := sb.buildHostsFile(); err != nil { + if err := sb.buildHostsFile(ctx, nil); err != nil { return err } return sb.setupDNS() } -func (sb *Sandbox) buildHostsFile() error { +func (sb *Sandbox) buildHostsFile(ctx context.Context, ifaceIPs []string) error { + ctx, span := otel.Tracer("").Start(ctx, "libnetwork.buildHostsFile") + defer span.End() + sb.restoreHostsPath() dir, _ := filepath.Split(sb.config.hostsPath) @@ -134,10 +136,11 @@ func (sb *Sandbox) buildHostsFile() error { return nil } - extraContent := make([]etchosts.Record, 0, len(sb.config.extraHosts)) + 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}) } + extraContent = append(extraContent, sb.makeHostsRecs(ifaceIPs)...) // Assume IPv6 support, unless it's definitely disabled. buildf := etchosts.Build @@ -151,40 +154,35 @@ func (sb *Sandbox) buildHostsFile() error { return nil } -func (sb *Sandbox) updateHostsFile(ctx context.Context, ifaceIPs []string) error { - ctx, span := otel.Tracer("").Start(ctx, "libnetwork.updateHostsFile") - defer span.End() - +func (sb *Sandbox) makeHostsRecs(ifaceIPs []string) []etchosts.Record { if len(ifaceIPs) == 0 { return nil } - if sb.config.originHostsPath != "" { - return nil - } - // User might have provided a FQDN in hostname or split it across hostname // and domainname. We want the FQDN and the bare hostname. - fqdn := sb.config.hostName + hosts := sb.config.hostName if sb.config.domainName != "" { - fqdn += "." + sb.config.domainName - } - hosts := fqdn - - if hostName, _, ok := strings.Cut(fqdn, "."); ok { - hosts += " " + hostName + hosts += "." + sb.config.domainName } - var extraContent []etchosts.Record + if hn, _, ok := strings.Cut(hosts, "."); ok { + hosts += " " + hn + } + + var recs []etchosts.Record for _, ip := range ifaceIPs { - extraContent = append(extraContent, etchosts.Record{Hosts: hosts, IP: ip}) + recs = append(recs, etchosts.Record{Hosts: hosts, IP: ip}) } - - sb.addHostsEntries(extraContent) - return nil + return recs } -func (sb *Sandbox) addHostsEntries(recs []etchosts.Record) { +func (sb *Sandbox) addHostsEntries(ctx context.Context, ifaceAddrs []string) { + 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 diff --git a/libnetwork/sandbox_dns_windows.go b/libnetwork/sandbox_dns_windows.go index d3dc8b9595..e71b543956 100644 --- a/libnetwork/sandbox_dns_windows.go +++ b/libnetwork/sandbox_dns_windows.go @@ -18,7 +18,7 @@ func (sb *Sandbox) restoreHostsPath() {} func (sb *Sandbox) restoreResolvConfPath() {} -func (sb *Sandbox) updateHostsFile(_ context.Context, ifaceIP []string) error { +func (sb *Sandbox) addHostsEntries(_ context.Context, ifaceIP []string) error { return nil } From 80e4631998c9e9d359eedebbf6360869695dfd9c Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Tue, 5 Nov 2024 17:59:28 +0000 Subject: [PATCH 2/7] 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 Signed-off-by: Rob Murray --- integration/networking/etchosts_test.go | 4 +- libnetwork/docs/vagrant.md | 4 +- libnetwork/endpoint.go | 13 ++++-- libnetwork/endpoint_unix_test.go | 4 +- libnetwork/etchosts/etchosts.go | 17 ++++---- libnetwork/etchosts/etchosts_test.go | 55 ++++++++++++++----------- libnetwork/libnetwork_internal_test.go | 10 +++-- libnetwork/network.go | 21 +++++++--- libnetwork/sandbox_dns_unix.go | 30 +++++++------- libnetwork/sandbox_dns_windows.go | 3 +- 10 files changed, 93 insertions(+), 68 deletions(-) diff --git a/integration/networking/etchosts_test.go b/integration/networking/etchosts_test.go index 89e0219430..60651cc640 100644 --- a/integration/networking/etchosts_test.go +++ b/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 `, diff --git a/libnetwork/docs/vagrant.md b/libnetwork/docs/vagrant.md index 0d2d3daa09..1bf2844b4b 100644 --- a/libnetwork/docs/vagrant.md +++ b/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 diff --git a/libnetwork/endpoint.go b/libnetwork/endpoint.go index 032f26787d..31997c0d71 100644 --- a/libnetwork/endpoint.go +++ b/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 } diff --git a/libnetwork/endpoint_unix_test.go b/libnetwork/endpoint_unix_test.go index 080b5a2ad3..a5549e3bb6 100644 --- a/libnetwork/endpoint_unix_test.go +++ b/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 diff --git a/libnetwork/etchosts/etchosts.go b/libnetwork/etchosts/etchosts.go index 69370135ec..ead0ea5e42 100644 --- a/libnetwork/etchosts/etchosts.go +++ b/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) } } diff --git a/libnetwork/etchosts/etchosts_test.go b/libnetwork/etchosts/etchosts_test.go index 9f7f061ec3..e73d684174 100644 --- a/libnetwork/etchosts/etchosts_test.go +++ b/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 { diff --git a/libnetwork/libnetwork_internal_test.go b/libnetwork/libnetwork_internal_test.go index 8f800c6679..8c02edaa5e 100644 --- a/libnetwork/libnetwork_internal_test.go +++ b/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) diff --git a/libnetwork/network.go b/libnetwork/network.go index 28ab6faa8b..a8adb9d7d0 100644 --- a/libnetwork/network.go +++ b/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}) } } diff --git a/libnetwork/sandbox_dns_unix.go b/libnetwork/sandbox_dns_unix.go index b5abc10739..460d6706af 100644 --- a/libnetwork/sandbox_dns_unix.go +++ b/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) } } diff --git a/libnetwork/sandbox_dns_windows.go b/libnetwork/sandbox_dns_windows.go index e71b543956..3c4f20cd75 100644 --- a/libnetwork/sandbox_dns_windows.go +++ b/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 } From 28d029cf9f4ed3cc97b92bddded52886c5126edf Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 7 Nov 2024 01:55:02 +0100 Subject: [PATCH 3/7] libnetwork/etchosts: don't panic on invalid regex This regex is constructed using user-input, which could technically produce an invalid regex. Given that we have an error-return to our availability, let's return any error we get, instead of panicking. Signed-off-by: Sebastiaan van Stijn --- libnetwork/etchosts/etchosts.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libnetwork/etchosts/etchosts.go b/libnetwork/etchosts/etchosts.go index ead0ea5e42..32c0849eb0 100644 --- a/libnetwork/etchosts/etchosts.go +++ b/libnetwork/etchosts/etchosts.go @@ -193,12 +193,15 @@ loop: // IP is new IP address // hostname is hostname to search for to replace IP func Update(path, IP, hostname string) error { + re, err := regexp.Compile(fmt.Sprintf(`(\S*)(\t%s)(\s|\.)`, regexp.QuoteMeta(hostname))) + if err != nil { + return err + } defer pathLock(path)() old, err := os.ReadFile(path) if err != nil { return err } - re := regexp.MustCompile(fmt.Sprintf("(\\S*)(\\t%s)(\\s|\\.)", regexp.QuoteMeta(hostname))) return os.WriteFile(path, re.ReplaceAll(old, []byte(IP+"$2"+"$3")), 0o644) } From 7d98e45a6eb865f2116245fff8c0244a15ef1ed8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 7 Nov 2024 02:03:54 +0100 Subject: [PATCH 4/7] libnetwork/etchosts: Add: combine with "mergeRecords()" The `mergeRecords` function wasn't actually _merging_ anything, but only appended records to the existing `/etc/hosts` content. However, doing so was split across two functions; `Add` and `mergeRecords()`; - `Add()` obtains a lock for the given path - then calls `mergeRecords` which reads the file-content and appends the new records to the content. - Closes the file and returns the new content - Then `Add` does a `os.WriteFile` to ... the same file Given that we're appending, we won't have to read the file's content, and we can append to the file itself. Signed-off-by: Sebastiaan van Stijn --- libnetwork/etchosts/etchosts.go | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/libnetwork/etchosts/etchosts.go b/libnetwork/etchosts/etchosts.go index 32c0849eb0..f6f39f5fc4 100644 --- a/libnetwork/etchosts/etchosts.go +++ b/libnetwork/etchosts/etchosts.go @@ -111,34 +111,20 @@ func Add(path string, recs []Record) error { defer pathLock(path)() - b, err := mergeRecords(path, recs) - if err != nil { - return err - } - - return os.WriteFile(path, b, 0o644) -} - -func mergeRecords(path string, recs []Record) ([]byte, error) { - f, err := os.Open(path) - if err != nil { - return nil, err - } - defer f.Close() - content := bytes.NewBuffer(nil) - - if _, err := content.ReadFrom(f); err != nil { - return nil, err - } - for _, r := range recs { if _, err := r.WriteTo(content); err != nil { - return nil, err + return err } } - return content.Bytes(), nil + f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0o644) + if err != nil { + return err + } + _, err = f.Write(content.Bytes()) + _ = f.Close() + return err } // Delete deletes an arbitrary number of Records already existing in /etc/hosts file From 6a5ab42f2828674915ccb5447e1d5720f22bca6d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 7 Nov 2024 02:18:21 +0100 Subject: [PATCH 5/7] libnetwork/etchosts: Delete: truncate file instead of close and write We already have the filehandle open, so we could just truncate, and overwrite the content. Signed-off-by: Sebastiaan van Stijn --- libnetwork/etchosts/etchosts.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/libnetwork/etchosts/etchosts.go b/libnetwork/etchosts/etchosts.go index f6f39f5fc4..5a3eab646a 100644 --- a/libnetwork/etchosts/etchosts.go +++ b/libnetwork/etchosts/etchosts.go @@ -133,19 +133,19 @@ func Add(path string, recs []Record) error { // 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)() - if len(recs) == 0 { return nil } - old, err := os.Open(path) + defer pathLock(path)() + f, err := os.OpenFile(path, os.O_RDWR, 0o644) if err != nil { return err } + defer f.Close() var buf bytes.Buffer - s := bufio.NewScanner(old) + s := bufio.NewScanner(f) eol := []byte{'\n'} loop: for s.Scan() { @@ -167,11 +167,14 @@ loop: buf.Write(b) buf.Write(eol) } - old.Close() if err := s.Err(); err != nil { return err } - return os.WriteFile(path, buf.Bytes(), 0o644) + if err := f.Truncate(0); err != nil { + return err + } + _, err = f.WriteAt(buf.Bytes(), 0) + return err } // Update all IP addresses where hostname matches. From 7c1e41a06d3b20a3a4ee0231cf466c90e855b159 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 7 Nov 2024 02:21:50 +0100 Subject: [PATCH 6/7] libnetwork: Sandbox.buildHostsFile: remove intermediate var Call the respective (`etchosts.BuildNoIPv6` or `etchosts.Build`) functions directly instead of using the intermediate `buildf` variable. Signed-off-by: Sebastiaan van Stijn --- libnetwork/sandbox_dns_unix.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/libnetwork/sandbox_dns_unix.go b/libnetwork/sandbox_dns_unix.go index 460d6706af..fe0450b07b 100644 --- a/libnetwork/sandbox_dns_unix.go +++ b/libnetwork/sandbox_dns_unix.go @@ -147,15 +147,10 @@ func (sb *Sandbox) buildHostsFile(ctx context.Context, ifaceIPs []netip.Addr) er extraContent = append(extraContent, sb.makeHostsRecs(ifaceIPs)...) // Assume IPv6 support, unless it's definitely disabled. - buildf := etchosts.Build if en, ok := sb.IPv6Enabled(); ok && !en { - buildf = etchosts.BuildNoIPv6 + return etchosts.BuildNoIPv6(sb.config.hostsPath, extraContent) } - if err := buildf(sb.config.hostsPath, extraContent); err != nil { - return err - } - - return nil + return etchosts.Build(sb.config.hostsPath, extraContent) } func (sb *Sandbox) makeHostsRecs(ifaceIPs []netip.Addr) []etchosts.Record { From 16f6fd1a959e7db3a46c88f4eed954b5323536d8 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Thu, 7 Nov 2024 11:18:06 +0000 Subject: [PATCH 7/7] Add a comment explaining host-networking hosts file generation Signed-off-by: Rob Murray --- libnetwork/sandbox_dns_unix.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libnetwork/sandbox_dns_unix.go b/libnetwork/sandbox_dns_unix.go index fe0450b07b..bf3b6f6d8e 100644 --- a/libnetwork/sandbox_dns_unix.go +++ b/libnetwork/sandbox_dns_unix.go @@ -126,7 +126,11 @@ func (sb *Sandbox) buildHostsFile(ctx context.Context, ifaceIPs []netip.Addr) er return err } - // This is for the host mode networking + // This is for the host mode networking. If extra hosts are supplied, even though + // it's host-networking, the container's hosts file is not based on the host's - + // so that it's possible to override a hostname that's in the host's hosts file. + // See analysis of how this came about in: + // https://github.com/moby/moby/pull/48823#issuecomment-2461777129 if sb.config.useDefaultSandBox && len(sb.config.extraHosts) == 0 { // We are working under the assumption that the origin file option had been properly expressed by the upper layer // if not here we are going to error out