diff --git a/net/dns/config.go b/net/dns/config.go index 22caf6ef5..6c170f19b 100644 --- a/net/dns/config.go +++ b/net/dns/config.go @@ -7,6 +7,7 @@ package dns import ( "bufio" "fmt" + "maps" "net/netip" "reflect" "slices" @@ -190,15 +191,21 @@ func sameResolverNames(a, b []*dnstype.Resolver) bool { return true } +// Clone makes a shallow clone of c. +// +// The returned Config still references slices and maps from c. +// +// TODO(bradfitz): use cmd/{viewer,cloner} for these and make the +// caller use views instead. func (c *Config) Clone() *Config { if c == nil { return nil } return &Config{ DefaultResolvers: slices.Clone(c.DefaultResolvers), - Routes: make(map[dnsname.FQDN][]*dnstype.Resolver, len(c.Routes)), + Routes: maps.Clone(c.Routes), SearchDomains: slices.Clone(c.SearchDomains), - Hosts: make(map[dnsname.FQDN][]netip.Addr, len(c.Hosts)), + Hosts: maps.Clone(c.Hosts), OnlyIPv6: c.OnlyIPv6, } } diff --git a/net/dns/config_test.go b/net/dns/config_test.go new file mode 100644 index 000000000..684dea6bc --- /dev/null +++ b/net/dns/config_test.go @@ -0,0 +1,66 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package dns + +import ( + "net/netip" + "reflect" + "testing" + + "tailscale.com/types/dnstype" + "tailscale.com/util/dnsname" +) + +func TestConfigClone(t *testing.T) { + tests := []struct { + name string + conf *Config + }{ + { + name: "nil", + conf: nil, + }, + { + name: "empty", + conf: &Config{}, + }, + { + name: "full", + conf: &Config{ + DefaultResolvers: []*dnstype.Resolver{ + { + Addr: "abc", + BootstrapResolution: []netip.Addr{netip.MustParseAddr("1.2.3.4")}, + UseWithExitNode: true, + }, + }, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{ + "foo.bar.": { + { + Addr: "abc", + BootstrapResolution: []netip.Addr{netip.MustParseAddr("1.2.3.4")}, + UseWithExitNode: true, + }, + }, + }, + SearchDomains: []dnsname.FQDN{"bar.baz."}, + Hosts: map[dnsname.FQDN][]netip.Addr{ + "host.bar.": {netip.MustParseAddr("5.6.7.8")}, + }, + OnlyIPv6: true, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.conf.Clone() + if !reflect.DeepEqual(got, tt.conf) { + t.Error("Cloned result is not reflect.DeepEqual") + } + if !got.Equal(tt.conf) { + t.Error("Cloned result is not Equal") + } + }) + } +} diff --git a/util/checkchange/checkchange.go b/util/checkchange/checkchange.go index 4d18730f1..8ba64720d 100644 --- a/util/checkchange/checkchange.go +++ b/util/checkchange/checkchange.go @@ -17,7 +17,7 @@ type EqualCloner[T any] interface { // // It only modifies *old if they are different. old must be non-nil. func Update[T EqualCloner[T]](old *T, new T) (changed bool) { - if new.Equal(*old) { + if (*old).Equal(new) { return false } *old = new.Clone() diff --git a/wgengine/userspace.go b/wgengine/userspace.go index fa2379288..9f42dae2a 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -965,8 +965,9 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, isSubnetRouterChanged := isSubnetRouter != e.lastIsSubnetRouter engineChanged := checkchange.Update(&e.lastEngineFull, cfg) - dnsChanged := checkchange.Update(&e.lastDNSConfig, dnsCfg) + dnsChanged := buildfeatures.HasDNS && checkchange.Update(&e.lastDNSConfig, dnsCfg) routerChanged := checkchange.Update(&e.lastRouter, routerCfg) + listenPortChanged := listenPort != e.magicConn.LocalPort() peerMTUChanged := peerMTUEnable != e.magicConn.PeerMTUEnabled() if !engineChanged && !routerChanged && !dnsChanged && !listenPortChanged && !isSubnetRouterChanged && !peerMTUChanged { @@ -987,7 +988,9 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, // instead have ipnlocal populate a map of DNS IP => linkName and // put that in the *dns.Config instead, and plumb it down to the // dns.Manager. Maybe also with isLocalAddr above. - e.isDNSIPOverTailscale.Store(ipset.NewContainsIPFunc(views.SliceOf(dnsIPsOverTailscale(dnsCfg, routerCfg)))) + if buildfeatures.HasDNS { + e.isDNSIPOverTailscale.Store(ipset.NewContainsIPFunc(views.SliceOf(dnsIPsOverTailscale(dnsCfg, routerCfg)))) + } // See if any peers have changed disco keys, which means they've restarted. // If so, we need to update the wireguard-go/device.Device in two phases: @@ -1063,7 +1066,18 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, if err != nil { return err } + } + // We've historically re-set DNS even after just a router change. While + // refactoring in tailscale/tailscale#17448 and and + // tailscale/tailscale#17499, I'm erring on the side of keeping that + // historical quirk for now (2025-10-08), lest it's load bearing in + // unexpected ways + // + // TODO(bradfitz): try to do the "configuring DNS" part below only if + // dnsChanged, not routerChanged. The "resolver.ShouldUseRoutes" part + // probably needs to keep happening for both. + if buildfeatures.HasDNS && (routerChanged || dnsChanged) { if resolver.ShouldUseRoutes(e.controlKnobs) { e.logf("wgengine: Reconfig: user dialer") e.dialer.SetRoutes(routerCfg.Routes, routerCfg.LocalRoutes) @@ -1075,7 +1089,7 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, // DNS managers refuse to apply settings if the device has no // assigned address. e.logf("wgengine: Reconfig: configuring DNS") - err = e.dns.Set(*dnsCfg) + err := e.dns.Set(*dnsCfg) e.health.SetDNSHealth(err) if err != nil { return err @@ -1097,7 +1111,7 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, } } - if isSubnetRouterChanged && e.birdClient != nil { + if buildfeatures.HasBird && isSubnetRouterChanged && e.birdClient != nil { e.logf("wgengine: Reconfig: configuring BIRD") var err error if isSubnetRouter {