net/dns: some post-review cleanups.

Signed-off-by: David Anderson <danderson@tailscale.com>

net/dns/direct.go

@@ -164,8 +164,7 @@ func (m directManager) SupportsSplitDNS() bool {
 }
 
 func (m directManager) GetBaseConfig() (OSConfig, error) {
-	// TODO
+	return OSConfig{}, ErrGetBaseConfigNotSupported
-	return OSConfig{}, nil
 }
 
 func (m directManager) Close() error {

net/dns/manager.go

@@ -71,11 +71,6 @@ func forceSplitDNSForTesting(cfg *Config) {
 func (m *Manager) Set(cfg Config) error {
 	m.logf("Set: %+v", cfg)
 
-	if false {
-		// Temporary, for danderson to test things.
-		forceSplitDNSForTesting(&cfg)
-	}
-
 	rcfg, ocfg, err := m.compileConfig(cfg)
 	if err != nil {
 		return err
@@ -168,8 +163,6 @@ func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
 
 	// If the OS can't do native split-dns, read out the underlying
 	// resolver config and blend it into our config.
-	// TODO: for now, use quad-8 as the upstream until more plumbing
-	// is done.
 	if m.os.SupportsSplitDNS() {
 		ocfg.MatchDomains = cfg.matchDomains()
 	} else {
@@ -187,7 +180,10 @@ func (m *Manager) compileConfig(cfg Config) (resolver.Config, OSConfig, error) {
 func addFQDNDots(domains []string) []string {
 	ret := make([]string, 0, len(domains))
 	for _, dom := range domains {
-		ret = append(ret, strings.TrimSuffix(dom, ".")+".")
+		if !strings.HasSuffix(dom, ".") {
+			dom = dom + "."
+		}
+		ret = append(ret, dom)
 	}
 	return ret
 }
@@ -198,6 +194,7 @@ func addFQDNDots(domains []string) []string {
 // https://github.com/tailscale/tailscale/issues/1666 tracks making
 // that not true, if we ever want to.
 func toIPsOnly(ipps []netaddr.IPPort) (ret []netaddr.IP) {
+	ret = make([]netaddr.IP, 0, len(ipps))
 	for _, ipp := range ipps {
 		ret = append(ret, ipp.IP)
 	}
@@ -205,6 +202,7 @@ func toIPsOnly(ipps []netaddr.IPPort) (ret []netaddr.IP) {
 }
 
 func toIPPorts(ips []netaddr.IP) (ret []netaddr.IPPort) {
+	ret = make([]netaddr.IPPort, 0, len(ips))
 	for _, ip := range ips {
 		ret = append(ret, netaddr.IPPort{IP: ip, Port: 53})
 	}

net/dns/manager_windows.go

@@ -301,9 +301,6 @@ func (m windowsManager) Close() error {
 }
 
 func (m windowsManager) GetBaseConfig() (OSConfig, error) {
-	if m.nrptWorks {
-		return OSConfig{}, errors.New("GetBaseConfig not supported")
-	}
 	resolvers, err := m.getBasePrimaryResolver()
 	if err != nil {
 		return OSConfig{}, err

net/dns/nm.go

@@ -203,8 +203,7 @@ func (m nmManager) SetDNS(config OSConfig) error {
 func (m nmManager) SupportsSplitDNS() bool { return false }
 
 func (m nmManager) GetBaseConfig() (OSConfig, error) {
-	// TODO
+	return OSConfig{}, ErrGetBaseConfigNotSupported
-	return OSConfig{}, nil
 }
 
 func (m nmManager) Close() error {

net/dns/noop.go

@@ -6,10 +6,12 @@
 
 type noopManager struct{}
 
-func (m noopManager) SetDNS(OSConfig) error            { return nil }
+func (m noopManager) SetDNS(OSConfig) error  { return nil }
-func (m noopManager) SupportsSplitDNS() bool           { return false }
+func (m noopManager) SupportsSplitDNS() bool { return false }
-func (m noopManager) Close() error                     { return nil }
+func (m noopManager) Close() error           { return nil }
-func (m noopManager) GetBaseConfig() (OSConfig, error) { return OSConfig{}, nil }
+func (m noopManager) GetBaseConfig() (OSConfig, error) {
+	return OSConfig{}, ErrGetBaseConfigNotSupported
+}
 
 func NewNoopManager() noopManager {
 	return noopManager{}

net/dns/osconfig.go

@@ -4,7 +4,11 @@
 
 package dns
 
-import "inet.af/netaddr"
+import (
+	"errors"
+
+	"inet.af/netaddr"
+)
 
 // An OSConfigurator applies DNS settings to the operating system.
 type OSConfigurator interface {
@@ -23,6 +27,9 @@ type OSConfigurator interface {
 	// GetBaseConfig must return the tailscale-free base config even
 	// after SetDNS has been called to set a Tailscale configuration.
 	// Only works when SupportsSplitDNS=false.
+
+	// Implementations that don't support getting the base config must
+	// return ErrGetBaseConfigNotSupported.
 	GetBaseConfig() (OSConfig, error)
 	// Close removes Tailscale-related DNS configuration from the OS.
 	Close() error
@@ -43,3 +50,8 @@ type OSConfig struct {
 	// report SupportsSplitDNS()=true.
 	MatchDomains []string
 }
+
+// ErrGetBaseConfigNotSupported is the error
+// OSConfigurator.GetBaseConfig returns when the OSConfigurator
+// doesn't support reading the underlying configuration out of the OS.
+var ErrGetBaseConfigNotSupported = errors.New("getting OS base config is not supported")

net/dns/resolvconf.go

@@ -143,8 +143,7 @@ func (m resolvconfManager) SupportsSplitDNS() bool {
 }
 
 func (m resolvconfManager) GetBaseConfig() (OSConfig, error) {
-	// TODO
+	return OSConfig{}, ErrGetBaseConfigNotSupported
-	return OSConfig{}, nil
 }
 
 func (m resolvconfManager) Close() error {

net/dns/resolved.go

@@ -158,8 +158,7 @@ func (m resolvedManager) SupportsSplitDNS() bool {
 }
 
 func (m resolvedManager) GetBaseConfig() (OSConfig, error) {
-	// TODO
+	return OSConfig{}, ErrGetBaseConfigNotSupported
-	return OSConfig{}, nil
 }
 
 func (m resolvedManager) Close() error {

wgengine/router/callback.go

@@ -50,8 +50,7 @@ func (r *CallbackRouter) SupportsSplitDNS() bool {
 }
 
 func (r *CallbackRouter) GetBaseConfig() (dns.OSConfig, error) {
-	// TODO
+	return dns.OSConfig{}, dns.ErrGetBaseConfigNotSupported
-	return dns.OSConfig{}, nil
 }
 
 func (r *CallbackRouter) Close() error {