net/dns, wgengine: use viewer/cloner for Config

Per earlier TODO.

Updates #17506

Change-Id: I21fe851c4bcced98fcee844cb428ca9c2f6b0588
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

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

net/dns/config_test.go

@@ -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")
-			}
-		})
-	}
-}

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, <src, dst> 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
+}

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
+}{})

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)