feature/linuxdnsfight: move inotify watching of /etc/resolv.conf out to a feature

tsnet apps in particular never use the Linux DNS OSManagers, so they don't need
DBus, etc. I started to pull that all out into separate features so tsnet doesn't
need to bring in DBus, but hit this first.

Here you can see that tsnet (and the k8s-operator) no longer pulls in inotify.

Updates #17206

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

net/dns/direct.go

@@ -23,6 +23,7 @@ import (
 	"sync"
 	"time"
 
+	"tailscale.com/feature"
 	"tailscale.com/health"
 	"tailscale.com/net/dns/resolvconffile"
 	"tailscale.com/net/tsaddr"
@@ -415,6 +416,73 @@ func (m *directManager) GetBaseConfig() (OSConfig, error) {
 	return oscfg, nil
 }
 
+// HookWatchFile is a hook for watching file changes, for platforms that support it.
+// The function is called with a directory and filename to watch, and a callback
+// to call when the file changes. It returns an error if the watch could not be set up.
+var HookWatchFile feature.Hook[func(ctx context.Context, dir, filename string, cb func()) error]
+
+func (m *directManager) runFileWatcher() {
+	watchFile, ok := HookWatchFile.GetOk()
+	if !ok {
+		return
+	}
+	if err := watchFile(m.ctx, "/etc/", resolvConf, m.checkForFileTrample); err != nil {
+		// This is all best effort for now, so surface warnings to users.
+		m.logf("dns: inotify: %s", err)
+	}
+}
+
+var resolvTrampleWarnable = health.Register(&health.Warnable{
+	Code:     "resolv-conf-overwritten",
+	Severity: health.SeverityMedium,
+	Title:    "DNS configuration issue",
+	Text:     health.StaticMessage("System DNS config not ideal. /etc/resolv.conf overwritten. See https://tailscale.com/s/dns-fight"),
+})
+
+// checkForFileTrample checks whether /etc/resolv.conf has been trampled
+// by another program on the system. (e.g. a DHCP client)
+func (m *directManager) checkForFileTrample() {
+	m.mu.Lock()
+	want := m.wantResolvConf
+	lastWarn := m.lastWarnContents
+	m.mu.Unlock()
+
+	if want == nil {
+		return
+	}
+
+	cur, err := m.fs.ReadFile(resolvConf)
+	if err != nil {
+		m.logf("trample: read error: %v", err)
+		return
+	}
+	if bytes.Equal(cur, want) {
+		m.health.SetHealthy(resolvTrampleWarnable)
+		if lastWarn != nil {
+			m.mu.Lock()
+			m.lastWarnContents = nil
+			m.mu.Unlock()
+			m.logf("trample: resolv.conf again matches expected content")
+		}
+		return
+	}
+	if bytes.Equal(cur, lastWarn) {
+		// We already logged about this, so not worth doing it again.
+		return
+	}
+
+	m.mu.Lock()
+	m.lastWarnContents = cur
+	m.mu.Unlock()
+
+	show := cur
+	if len(show) > 1024 {
+		show = show[:1024]
+	}
+	m.logf("trample: resolv.conf changed from what we expected. did some other program interfere? current contents: %q", show)
+	m.health.SetUnhealthy(resolvTrampleWarnable, nil)
+}
+
 func (m *directManager) Close() error {
 	m.ctxClose()
 

net/dns/direct_linux.go

@@ -1,104 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-//go:build linux && !android
-
-package dns
-
-import (
-	"bytes"
-	"context"
-	"fmt"
-
-	"github.com/illarion/gonotify/v3"
-	"tailscale.com/health"
-)
-
-func (m *directManager) runFileWatcher() {
-	if err := watchFile(m.ctx, "/etc/", resolvConf, m.checkForFileTrample); err != nil {
-		// This is all best effort for now, so surface warnings to users.
-		m.logf("dns: inotify: %s", err)
-	}
-}
-
-// watchFile sets up an inotify watch for a given directory and
-// calls the callback function every time a particular file is changed.
-// The filename should be located in the provided directory.
-func watchFile(ctx context.Context, dir, filename string, cb func()) error {
-	ctx, cancel := context.WithCancel(ctx)
-	defer cancel()
-
-	const events = gonotify.IN_ATTRIB |
-		gonotify.IN_CLOSE_WRITE |
-		gonotify.IN_CREATE |
-		gonotify.IN_DELETE |
-		gonotify.IN_MODIFY |
-		gonotify.IN_MOVE
-
-	watcher, err := gonotify.NewDirWatcher(ctx, events, dir)
-	if err != nil {
-		return fmt.Errorf("NewDirWatcher: %w", err)
-	}
-
-	for {
-		select {
-		case event := <-watcher.C:
-			if event.Name == filename {
-				cb()
-			}
-		case <-ctx.Done():
-			return ctx.Err()
-		}
-	}
-}
-
-var resolvTrampleWarnable = health.Register(&health.Warnable{
-	Code:     "resolv-conf-overwritten",
-	Severity: health.SeverityMedium,
-	Title:    "Linux DNS configuration issue",
-	Text:     health.StaticMessage("Linux DNS config not ideal. /etc/resolv.conf overwritten. See https://tailscale.com/s/dns-fight"),
-})
-
-// checkForFileTrample checks whether /etc/resolv.conf has been trampled
-// by another program on the system. (e.g. a DHCP client)
-func (m *directManager) checkForFileTrample() {
-	m.mu.Lock()
-	want := m.wantResolvConf
-	lastWarn := m.lastWarnContents
-	m.mu.Unlock()
-
-	if want == nil {
-		return
-	}
-
-	cur, err := m.fs.ReadFile(resolvConf)
-	if err != nil {
-		m.logf("trample: read error: %v", err)
-		return
-	}
-	if bytes.Equal(cur, want) {
-		m.health.SetHealthy(resolvTrampleWarnable)
-		if lastWarn != nil {
-			m.mu.Lock()
-			m.lastWarnContents = nil
-			m.mu.Unlock()
-			m.logf("trample: resolv.conf again matches expected content")
-		}
-		return
-	}
-	if bytes.Equal(cur, lastWarn) {
-		// We already logged about this, so not worth doing it again.
-		return
-	}
-
-	m.mu.Lock()
-	m.lastWarnContents = cur
-	m.mu.Unlock()
-
-	show := cur
-	if len(show) > 1024 {
-		show = show[:1024]
-	}
-	m.logf("trample: resolv.conf changed from what we expected. did some other program interfere? current contents: %q", show)
-	m.health.SetUnhealthy(resolvTrampleWarnable, nil)
-}

net/dns/direct_linux_test.go

@@ -1,61 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-package dns
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"os"
-	"sync/atomic"
-	"testing"
-	"time"
-
-	"golang.org/x/sync/errgroup"
-)
-
-func TestWatchFile(t *testing.T) {
-	dir := t.TempDir()
-	filepath := dir + "/test.txt"
-
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	var callbackCalled atomic.Bool
-	callbackDone := make(chan bool)
-	callback := func() {
-		// We only send to the channel once to avoid blocking if the
-		// callback is called multiple times -- this happens occasionally
-		// if inotify sends multiple events before we cancel the context.
-		if !callbackCalled.Load() {
-			callbackDone <- true
-			callbackCalled.Store(true)
-		}
-	}
-
-	var eg errgroup.Group
-	eg.Go(func() error { return watchFile(ctx, dir, filepath, callback) })
-
-	// Keep writing until we get a callback.
-	func() {
-		for i := range 10000 {
-			if err := os.WriteFile(filepath, []byte(fmt.Sprintf("write%d", i)), 0644); err != nil {
-				t.Fatal(err)
-			}
-			select {
-			case <-callbackDone:
-				return
-			case <-time.After(10 * time.Millisecond):
-			}
-		}
-	}()
-
-	cancel()
-	if err := eg.Wait(); err != nil && !errors.Is(err, context.Canceled) {
-		t.Error(err)
-	}
-	if !callbackCalled.Load() {
-		t.Error("callback was not called")
-	}
-}

net/dns/direct_notlinux.go

@@ -1,10 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-//go:build !linux && !android && !ios
-
-package dns
-
-func (m *directManager) runFileWatcher() {
-	// Not implemented on other platforms. Maybe it could resort to polling.
-}