net/dnsfallback: more explicitly pass through logf function

Redoes the approach from #5550 and #7539 to explicitly pass in the logf
function, instead of having global state that can be overridden.

Signed-off-by: Mihai Parparita <mihai@tailscale.com>

cmd/tailscaled/tailscaled.go

@@ -534,7 +534,7 @@ func getLocalBackend(ctx context.Context, logf logger.Logf, logID logid.PublicID
 		lb.SetLogFlusher(logPol.Logtail.StartFlush)
 	}
 	if root := lb.TailscaleVarRoot(); root != "" {
-		dnsfallback.SetCachePath(filepath.Join(root, "derpmap.cached.json"))
+		dnsfallback.SetCachePath(filepath.Join(root, "derpmap.cached.json"), logf)
 	}
 	lb.SetDecompressor(func() (controlclient.Decompressor, error) {
 		return smallzstd.NewDecoder(nil)

control/controlclient/direct.go

@@ -211,7 +211,7 @@ func NewDirect(opts Options) (*Direct, error) {
 		dnsCache := &dnscache.Resolver{
 			Forward:          dnscache.Get().Forward, // use default cache's forwarder
 			UseLastGood:      true,
-			LookupIPFallback: dnsfallback.Lookup,
+			LookupIPFallback: dnsfallback.Lookup(opts.Logf),
 			Logf:             opts.Logf,
 		}
 		tr := http.DefaultTransport.(*http.Transport).Clone()

control/controlhttp/client.go

@@ -393,7 +393,7 @@ func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, addr netip.Addr,
 	} else {
 		dns = &dnscache.Resolver{
 			Forward:          dnscache.Get().Forward,
-			LookupIPFallback: dnsfallback.Lookup,
+			LookupIPFallback: dnsfallback.Lookup(a.logf),
 			UseLastGood:      true,
 			Logf:             a.Logf, // not a.logf method; we want to propagate nil-ness
 		}

ipn/ipnlocal/local.go

@@ -1077,7 +1077,7 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 		b.e.SetDERPMap(st.NetMap.DERPMap)
 
 		// Update our cached DERP map
-		dnsfallback.UpdateCache(st.NetMap.DERPMap)
+		dnsfallback.UpdateCache(st.NetMap.DERPMap, b.logf)
 
 		b.send(ipn.Notify{NetMap: st.NetMap})
 	}

logpolicy/logpolicy.go

@@ -711,7 +711,7 @@ func DialContext(ctx context.Context, netw, addr string) (net.Conn, error) {
 	dnsCache := &dnscache.Resolver{
 		Forward:          dnscache.Get().Forward, // use default cache's forwarder
 		UseLastGood:      true,
-		LookupIPFallback: dnsfallback.Lookup,
+		LookupIPFallback: dnsfallback.Lookup(log.Printf),
 	}
 	dialer := dnscache.Dialer(nd.DialContext, dnsCache)
 	c, err = dialer(ctx, netw, addr)

net/dnsfallback/dnsfallback.go

@@ -13,7 +13,6 @@
 	"encoding/json"
 	"errors"
 	"fmt"
-	"log"
 	"net"
 	"net/http"
 	"net/netip"
@@ -27,13 +26,18 @@
 	"tailscale.com/net/netns"
 	"tailscale.com/net/tlsdial"
 	"tailscale.com/net/tshttpproxy"
-	"tailscale.com/syncs"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/slicesx"
 )
 
-func Lookup(ctx context.Context, host string) ([]netip.Addr, error) {
+func Lookup(logf logger.Logf) func(ctx context.Context, host string) ([]netip.Addr, error) {
+	return func(ctx context.Context, host string) ([]netip.Addr, error) {
+		return lookup(ctx, host, logf)
+	}
+}
+
+func lookup(ctx context.Context, host string, logf logger.Logf) ([]netip.Addr, error) {
 	if ip, err := netip.ParseAddr(host); err == nil && ip.IsValid() {
 		return []netip.Addr{ip}, nil
 	}
@@ -81,7 +85,7 @@ type nameIP struct {
 		logf("trying bootstrapDNS(%q, %q) for %q ...", cand.dnsName, cand.ip, host)
 		ctx, cancel := context.WithTimeout(ctx, 3*time.Second)
 		defer cancel()
-		dm, err := bootstrapDNSMap(ctx, cand.dnsName, cand.ip, host)
+		dm, err := bootstrapDNSMap(ctx, cand.dnsName, cand.ip, host, logf)
 		if err != nil {
 			logf("bootstrapDNS(%q, %q) for %q error: %v", cand.dnsName, cand.ip, host, err)
 			continue
@@ -100,7 +104,7 @@ type nameIP struct {
 
 // serverName and serverIP of are, say, "derpN.tailscale.com".
 // queryName is the name being sought (e.g. "controlplane.tailscale.com"), passed as hint.
-func bootstrapDNSMap(ctx context.Context, serverName string, serverIP netip.Addr, queryName string) (dnsMap, error) {
+func bootstrapDNSMap(ctx context.Context, serverName string, serverIP netip.Addr, queryName string, logf logger.Logf) (dnsMap, error) {
 	dialer := netns.NewDialer(logf)
 	tr := http.DefaultTransport.(*http.Transport).Clone()
 	tr.Proxy = tshttpproxy.ProxyFromEnvironment
@@ -194,7 +198,7 @@ func getStaticDERPMap() *tailcfg.DERPMap {
 // UpdateCache stores the DERP map cache back to disk.
 //
 // The caller must not mutate 'c' after calling this function.
-func UpdateCache(c *tailcfg.DERPMap) {
+func UpdateCache(c *tailcfg.DERPMap, logf logger.Logf) {
 	// Don't do anything if nothing changed.
 	curr := cachedDERPMap.Load()
 	if reflect.DeepEqual(curr, c) {
@@ -227,7 +231,7 @@ func UpdateCache(c *tailcfg.DERPMap) {
 //
 // This function should be called before any calls to UpdateCache, as it is not
 // concurrency-safe.
-func SetCachePath(path string) {
+func SetCachePath(path string, logf logger.Logf) {
 	cachePath = path
 
 	f, err := os.Open(path)
@@ -246,23 +250,3 @@ func SetCachePath(path string) {
 	cachedDERPMap.Store(dm)
 	logf("[v2] dnsfallback: SetCachePath loaded cached DERP map")
 }
-
-// logfunc stores the logging function to use for this package.
-var logfunc syncs.AtomicValue[logger.Logf]
-
-// SetLogger sets the logging function that this package will use, and returns
-// the old value (which may be nil).
-//
-// If this function is never called, or if this function is called with a nil
-// value, 'log.Printf' will be used to print logs.
-func SetLogger(log logger.Logf) (old logger.Logf) {
-	return logfunc.Swap(log)
-}
-
-func logf(format string, args ...any) {
-	if lf := logfunc.Load(); lf != nil {
-		lf(format, args...)
-	} else {
-		log.Printf(format, args...)
-	}
-}

net/dnsfallback/dnsfallback_test.go

@@ -24,11 +24,6 @@ func TestGetDERPMap(t *testing.T) {
 }
 
 func TestCache(t *testing.T) {
-	oldlog := logfunc.Load()
-	SetLogger(t.Logf)
-	t.Cleanup(func() {
-		SetLogger(oldlog)
-	})
 	cacheFile := filepath.Join(t.TempDir(), "cache.json")
 
 	// Write initial cache value
@@ -73,7 +68,7 @@ func TestCache(t *testing.T) {
 	cachedDERPMap.Store(nil)
 
 	// Load the cache
-	SetCachePath(cacheFile)
+	SetCachePath(cacheFile, t.Logf)
 	if cm := cachedDERPMap.Load(); !reflect.DeepEqual(initialCache, cm) {
 		t.Fatalf("cached map was %+v; want %+v", cm, initialCache)
 	}
@@ -105,11 +100,6 @@ func TestCache(t *testing.T) {
 }
 
 func TestCacheUnchanged(t *testing.T) {
-	oldlog := logfunc.Load()
-	SetLogger(t.Logf)
-	t.Cleanup(func() {
-		SetLogger(oldlog)
-	})
 	cacheFile := filepath.Join(t.TempDir(), "cache.json")
 
 	// Write initial cache value
@@ -140,7 +130,7 @@ func TestCacheUnchanged(t *testing.T) {
 	cachedDERPMap.Store(nil)
 
 	// Load the cache
-	SetCachePath(cacheFile)
+	SetCachePath(cacheFile, t.Logf)
 	if cm := cachedDERPMap.Load(); !reflect.DeepEqual(initialCache, cm) {
 		t.Fatalf("cached map was %+v; want %+v", cm, initialCache)
 	}
@@ -152,7 +142,7 @@ func TestCacheUnchanged(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	UpdateCache(initialCache)
+	UpdateCache(initialCache, t.Logf)
 	if _, err := os.Stat(cacheFile); !os.IsNotExist(err) {
 		t.Fatalf("got err=%v; expected to not find cache file", err)
 	}
@@ -173,7 +163,7 @@ func TestCacheUnchanged(t *testing.T) {
 	clonedNode.IPv4 = "1.2.3.5"
 	updatedCache.Regions[99].Nodes = append(updatedCache.Regions[99].Nodes, &clonedNode)
 
-	UpdateCache(updatedCache)
+	UpdateCache(updatedCache, t.Logf)
 	if st, err := os.Stat(cacheFile); err != nil {
 		t.Fatalf("could not stat cache file; err=%v", err)
 	} else if !st.Mode().IsRegular() || st.Size() == 0 {

tsnet/tsnet.go

@@ -41,7 +41,6 @@
 	"tailscale.com/logpolicy"
 	"tailscale.com/logtail"
 	"tailscale.com/logtail/filch"
-	"tailscale.com/net/dnsfallback"
 	"tailscale.com/net/memnet"
 	"tailscale.com/net/proxymux"
 	"tailscale.com/net/socks5"
@@ -651,25 +650,6 @@ func (s *Server) logf(format string, a ...interface{}) {
 	log.Printf(format, a...)
 }
 
-// ReplaceGlobalLoggers will replace any Tailscale-specific package-global
-// loggers with this Server's logger. It returns a function that, when called,
-// will undo any changes made.
-//
-// Note that calling this function from multiple Servers will result in the
-// last call taking all logs; logs are not duplicated.
-func (s *Server) ReplaceGlobalLoggers() (undo func()) {
-	var undos []func()
-
-	oldDnsFallback := dnsfallback.SetLogger(s.logf)
-	undos = append(undos, func() { dnsfallback.SetLogger(oldDnsFallback) })
-
-	return func() {
-		for _, fn := range undos {
-			fn()
-		}
-	}
-}
-
 // printAuthURLLoop loops once every few seconds while the server is still running and
 // is in NeedsLogin state, printing out the auth URL.
 func (s *Server) printAuthURLLoop() {