From 91239327100db0bc588530d5a44172add767f195 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 8 Oct 2025 18:16:15 -0700 Subject: [PATCH] net/dns, wgengine: use viewer/cloner for Config Per earlier TODO. Updates #17506 Change-Id: I21fe851c4bcced98fcee844cb428ca9c2f6b0588 Signed-off-by: Brad Fitzpatrick --- net/dns/config.go | 22 +------ net/dns/config_test.go | 66 -------------------- net/dns/dns_clone.go | 74 ++++++++++++++++++++++ net/dns/dns_view.go | 138 +++++++++++++++++++++++++++++++++++++++++ wgengine/userspace.go | 13 ++-- 5 files changed, 222 insertions(+), 91 deletions(-) delete mode 100644 net/dns/config_test.go create mode 100644 net/dns/dns_clone.go create mode 100644 net/dns/dns_view.go diff --git a/net/dns/config.go b/net/dns/config.go index 6c170f19b..2425b304d 100644 --- a/net/dns/config.go +++ b/net/dns/config.go @@ -1,13 +1,14 @@ // Copyright (c) Tailscale Inc & AUTHORS // SPDX-License-Identifier: BSD-3-Clause +//go:generate go run tailscale.com/cmd/viewer --type=Config --clonefunc + // Package dns contains code to configure and manage DNS settings. package dns import ( "bufio" "fmt" - "maps" "net/netip" "reflect" "slices" @@ -191,25 +192,6 @@ 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: maps.Clone(c.Routes), - SearchDomains: slices.Clone(c.SearchDomains), - Hosts: maps.Clone(c.Hosts), - OnlyIPv6: c.OnlyIPv6, - } -} - func (c *Config) Equal(o *Config) bool { if c == nil || o == nil { return c == o diff --git a/net/dns/config_test.go b/net/dns/config_test.go deleted file mode 100644 index 684dea6bc..000000000 --- a/net/dns/config_test.go +++ /dev/null @@ -1,66 +0,0 @@ -// 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/net/dns/dns_clone.go b/net/dns/dns_clone.go new file mode 100644 index 000000000..807bfce23 --- /dev/null +++ b/net/dns/dns_clone.go @@ -0,0 +1,74 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +// Code generated by tailscale.com/cmd/cloner; DO NOT EDIT. + +package dns + +import ( + "net/netip" + + "tailscale.com/types/dnstype" + "tailscale.com/util/dnsname" +) + +// Clone makes a deep copy of Config. +// The result aliases no memory with the original. +func (src *Config) Clone() *Config { + if src == nil { + return nil + } + dst := new(Config) + *dst = *src + if src.DefaultResolvers != nil { + dst.DefaultResolvers = make([]*dnstype.Resolver, len(src.DefaultResolvers)) + for i := range dst.DefaultResolvers { + if src.DefaultResolvers[i] == nil { + dst.DefaultResolvers[i] = nil + } else { + dst.DefaultResolvers[i] = src.DefaultResolvers[i].Clone() + } + } + } + if dst.Routes != nil { + dst.Routes = map[dnsname.FQDN][]*dnstype.Resolver{} + for k := range src.Routes { + dst.Routes[k] = append([]*dnstype.Resolver{}, src.Routes[k]...) + } + } + dst.SearchDomains = append(src.SearchDomains[:0:0], src.SearchDomains...) + if dst.Hosts != nil { + dst.Hosts = map[dnsname.FQDN][]netip.Addr{} + for k := range src.Hosts { + dst.Hosts[k] = append([]netip.Addr{}, src.Hosts[k]...) + } + } + return dst +} + +// A compilation failure here means this code must be regenerated, with the command at the top of this file. +var _ConfigCloneNeedsRegeneration = Config(struct { + DefaultResolvers []*dnstype.Resolver + Routes map[dnsname.FQDN][]*dnstype.Resolver + SearchDomains []dnsname.FQDN + Hosts map[dnsname.FQDN][]netip.Addr + OnlyIPv6 bool +}{}) + +// Clone duplicates src into dst and reports whether it succeeded. +// To succeed, must be of types <*T, *T> or <*T, **T>, +// where T is one of Config. +func Clone(dst, src any) bool { + switch src := src.(type) { + case *Config: + switch dst := dst.(type) { + case *Config: + *dst = *src.Clone() + return true + case **Config: + *dst = src.Clone() + return true + } + } + return false +} diff --git a/net/dns/dns_view.go b/net/dns/dns_view.go new file mode 100644 index 000000000..c7ce376cb --- /dev/null +++ b/net/dns/dns_view.go @@ -0,0 +1,138 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +// Code generated by tailscale/cmd/viewer; DO NOT EDIT. + +package dns + +import ( + jsonv1 "encoding/json" + "errors" + "net/netip" + + jsonv2 "github.com/go-json-experiment/json" + "github.com/go-json-experiment/json/jsontext" + "tailscale.com/types/dnstype" + "tailscale.com/types/views" + "tailscale.com/util/dnsname" +) + +//go:generate go run tailscale.com/cmd/cloner -clonefunc=true -type=Config + +// View returns a read-only view of Config. +func (p *Config) View() ConfigView { + return ConfigView{ж: p} +} + +// ConfigView provides a read-only view over Config. +// +// Its methods should only be called if `Valid()` returns true. +type ConfigView struct { + // ж is the underlying mutable value, named with a hard-to-type + // character that looks pointy like a pointer. + // It is named distinctively to make you think of how dangerous it is to escape + // to callers. You must not let callers be able to mutate it. + ж *Config +} + +// Valid reports whether v's underlying value is non-nil. +func (v ConfigView) Valid() bool { return v.ж != nil } + +// AsStruct returns a clone of the underlying value which aliases no memory with +// the original. +func (v ConfigView) AsStruct() *Config { + if v.ж == nil { + return nil + } + return v.ж.Clone() +} + +// MarshalJSON implements [jsonv1.Marshaler]. +func (v ConfigView) MarshalJSON() ([]byte, error) { + return jsonv1.Marshal(v.ж) +} + +// MarshalJSONTo implements [jsonv2.MarshalerTo]. +func (v ConfigView) MarshalJSONTo(enc *jsontext.Encoder) error { + return jsonv2.MarshalEncode(enc, v.ж) +} + +// UnmarshalJSON implements [jsonv1.Unmarshaler]. +func (v *ConfigView) UnmarshalJSON(b []byte) error { + if v.ж != nil { + return errors.New("already initialized") + } + if len(b) == 0 { + return nil + } + var x Config + if err := jsonv1.Unmarshal(b, &x); err != nil { + return err + } + v.ж = &x + return nil +} + +// UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom]. +func (v *ConfigView) UnmarshalJSONFrom(dec *jsontext.Decoder) error { + if v.ж != nil { + return errors.New("already initialized") + } + var x Config + if err := jsonv2.UnmarshalDecode(dec, &x); err != nil { + return err + } + v.ж = &x + return nil +} + +// DefaultResolvers are the DNS resolvers to use for DNS names +// which aren't covered by more specific per-domain routes below. +// If empty, the OS's default resolvers (the ones that predate +// Tailscale altering the configuration) are used. +func (v ConfigView) DefaultResolvers() views.SliceView[*dnstype.Resolver, dnstype.ResolverView] { + return views.SliceOfViews[*dnstype.Resolver, dnstype.ResolverView](v.ж.DefaultResolvers) +} + +// Routes maps a DNS suffix to the resolvers that should be used +// for queries that fall within that suffix. +// If a query doesn't match any entry in Routes, the +// DefaultResolvers are used. +// A Routes entry with no resolvers means the route should be +// authoritatively answered using the contents of Hosts. +func (v ConfigView) Routes() views.MapFn[dnsname.FQDN, []*dnstype.Resolver, views.SliceView[*dnstype.Resolver, dnstype.ResolverView]] { + return views.MapFnOf(v.ж.Routes, func(t []*dnstype.Resolver) views.SliceView[*dnstype.Resolver, dnstype.ResolverView] { + return views.SliceOfViews[*dnstype.Resolver, dnstype.ResolverView](t) + }) +} + +// SearchDomains are DNS suffixes to try when expanding +// single-label queries. +func (v ConfigView) SearchDomains() views.Slice[dnsname.FQDN] { + return views.SliceOf(v.ж.SearchDomains) +} + +// Hosts maps DNS FQDNs to their IPs, which can be a mix of IPv4 +// and IPv6. +// Queries matching entries in Hosts are resolved locally by +// 100.100.100.100 without leaving the machine. +// Adding an entry to Hosts merely creates the record. If you want +// it to resolve, you also need to add appropriate routes to +// Routes. +func (v ConfigView) Hosts() views.MapSlice[dnsname.FQDN, netip.Addr] { + return views.MapSliceOf(v.ж.Hosts) +} + +// OnlyIPv6, if true, uses the IPv6 service IP (for MagicDNS) +// instead of the IPv4 version (100.100.100.100). +func (v ConfigView) OnlyIPv6() bool { return v.ж.OnlyIPv6 } +func (v ConfigView) Equal(v2 ConfigView) bool { return v.ж.Equal(v2.ж) } + +// A compilation failure here means this code must be regenerated, with the command at the top of this file. +var _ConfigViewNeedsRegeneration = Config(struct { + DefaultResolvers []*dnstype.Resolver + Routes map[dnsname.FQDN][]*dnstype.Resolver + SearchDomains []dnsname.FQDN + Hosts map[dnsname.FQDN][]netip.Addr + OnlyIPv6 bool +}{}) diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 9f42dae2a..d1ca21f4d 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -132,8 +132,8 @@ type userspaceEngine struct { lastRouter *router.Config lastEngineFull *wgcfg.Config // of full wireguard config, not trimmed lastEngineInputs *maybeReconfigInputs - lastDNSConfig *dns.Config - lastIsSubnetRouter bool // was the node a primary subnet router in the last run. + lastDNSConfig dns.ConfigView // or invalid if none + lastIsSubnetRouter bool // was the node a primary subnet router in the last run. recvActivityAt map[key.NodePublic]mono.Time trimmedNodes map[key.NodePublic]bool // set of node keys of peers currently excluded from wireguard config sentActivityAt map[netip.Addr]*mono.Time // value is accessed atomically @@ -965,8 +965,11 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, isSubnetRouterChanged := isSubnetRouter != e.lastIsSubnetRouter engineChanged := checkchange.Update(&e.lastEngineFull, cfg) - dnsChanged := buildfeatures.HasDNS && checkchange.Update(&e.lastDNSConfig, dnsCfg) routerChanged := checkchange.Update(&e.lastRouter, routerCfg) + dnsChanged := buildfeatures.HasDNS && !e.lastDNSConfig.Equal(dnsCfg.View()) + if dnsChanged { + e.lastDNSConfig = dnsCfg.View() + } listenPortChanged := listenPort != e.magicConn.LocalPort() peerMTUChanged := peerMTUEnable != e.magicConn.PeerMTUEnabled() @@ -1322,8 +1325,8 @@ func (e *userspaceEngine) linkChange(delta *netmon.ChangeDelta) { e.wgLock.Lock() dnsCfg := e.lastDNSConfig e.wgLock.Unlock() - if dnsCfg != nil { - if err := e.dns.Set(*dnsCfg); err != nil { + if dnsCfg.Valid() { + if err := e.dns.Set(*dnsCfg.AsStruct()); err != nil { e.logf("wgengine: error setting DNS config after major link change: %v", err) } else if err := e.reconfigureVPNIfNecessary(); err != nil { e.logf("wgengine: error reconfiguring VPN after major link change: %v", err)