From 9c5607138a850cd2eeac099aa22ae71c15699c38 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 12 Mar 2026 11:15:04 +0000 Subject: [PATCH] logtail,control/controlclient,ipn/ipnlocal: add tests for per-instance DisableLogTail Add test coverage for the per-instance DisableLogTail feature: - TestEnableDisableRoundTrip: verify logtail.Enable/Disable toggle the global atomic bool correctly through multiple round-trips. - TestHandleDebugMessageDisableLogTail: verify handleDebugMessage fires the OnDisableLogTail callback, does NOT call envknob.SetNoLogsNoSupport, handles nil callback without panic, and does not fire the callback when DisableLogTail is false. - TestNoLogsNoSupportCombinesSources: verify LocalBackend.NoLogsNoSupport returns true when either the envknob or noLogsFromControl is set. - TestFlowLogsConflictCheck: verify that when logging is disabled and a netmap with CapabilityDataPlaneAuditLogs arrives, WantRunning is set to false with the correct error message (different for control-disabled vs user-disabled). - TestStartLockedClearsControlDisableLogTail: verify that Start clears noLogsFromControl and re-enables logtail. - TestProfileSwitchHeadscaleToTailscale: verify the full lifecycle of switching from a control server that disabled logging to one that requires flow logs. - TestGlobalNoLogsPreventReEnable: verify that when the global envknob is set, Start does NOT re-enable logtail, and the conflict check fires with the user-explicit error message. --- control/controlclient/direct_test.go | 82 ++++++ ipn/ipnlocal/local_test.go | 369 +++++++++++++++++++++++++++ logtail/logtail_test.go | 31 +++ 3 files changed, 482 insertions(+) diff --git a/control/controlclient/direct_test.go b/control/controlclient/direct_test.go index d10b346ae..90500e9cf 100644 --- a/control/controlclient/direct_test.go +++ b/control/controlclient/direct_test.go @@ -4,15 +4,19 @@ package controlclient import ( + "context" "encoding/json" "net/http" "net/http/httptest" "net/netip" + "sync/atomic" "testing" "time" + "tailscale.com/envknob" "tailscale.com/hostinfo" "tailscale.com/ipn/ipnstate" + "tailscale.com/logtail" "tailscale.com/net/netmon" "tailscale.com/net/tsdial" "tailscale.com/tailcfg" @@ -181,3 +185,81 @@ func TestTsmpPing(t *testing.T) { t.Fatal(err) } } + +func TestHandleDebugMessageDisableLogTail(t *testing.T) { + // This test mutates package-level logtail state and must not run in + // parallel with tests that depend on logtail being enabled. + t.Cleanup(func() { logtail.Enable() }) + + t.Run("callback_fires", func(t *testing.T) { + logtail.Enable() // reset from any prior subtest + + var called atomic.Bool + c := &Direct{ + logf: t.Logf, + onDisableLogTail: func() { called.Store(true) }, + } + + err := c.handleDebugMessage(context.Background(), &tailcfg.Debug{DisableLogTail: true}) + if err != nil { + t.Fatalf("handleDebugMessage: %v", err) + } + + if !called.Load() { + t.Error("onDisableLogTail callback was not called") + } + }) + + t.Run("envknob_not_set", func(t *testing.T) { + logtail.Enable() // reset + t.Setenv("TS_NO_LOGS_NO_SUPPORT", "") + + c := &Direct{ + logf: t.Logf, + onDisableLogTail: func() {}, + } + + err := c.handleDebugMessage(context.Background(), &tailcfg.Debug{DisableLogTail: true}) + if err != nil { + t.Fatalf("handleDebugMessage: %v", err) + } + + if envknob.NoLogsNoSupport() { + t.Error("envknob.NoLogsNoSupport() should be false; handleDebugMessage must not call envknob.SetNoLogsNoSupport()") + } + }) + + t.Run("nil_callback_no_panic", func(t *testing.T) { + logtail.Enable() // reset + + c := &Direct{ + logf: t.Logf, + onDisableLogTail: nil, + } + + err := c.handleDebugMessage(context.Background(), &tailcfg.Debug{DisableLogTail: true}) + if err != nil { + t.Fatalf("handleDebugMessage: %v", err) + } + // No panic means success. + }) + + t.Run("false_does_not_fire", func(t *testing.T) { + logtail.Enable() // reset + + var called atomic.Bool + c := &Direct{ + logf: t.Logf, + onDisableLogTail: func() { called.Store(true) }, + } + + err := c.handleDebugMessage(context.Background(), &tailcfg.Debug{DisableLogTail: false}) + if err != nil { + t.Fatalf("handleDebugMessage: %v", err) + } + + if called.Load() { + t.Error("onDisableLogTail should not be called when DisableLogTail is false") + } + }) +} diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index b9d8da046..cba2f57ba 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -43,6 +43,7 @@ "tailscale.com/ipn/ipnauth" "tailscale.com/ipn/ipnlocal/netmapcache" "tailscale.com/ipn/store/mem" + "tailscale.com/logtail" "tailscale.com/net/netcheck" "tailscale.com/net/netmon" "tailscale.com/net/tsaddr" @@ -3683,6 +3684,374 @@ func TestApplySysPolicy(t *testing.T) { } } +func TestNoLogsNoSupportCombinesSources(t *testing.T) { + tests := []struct { + name string + envknob bool + controlDisabled bool + want bool + }{ + {name: "neither", envknob: false, controlDisabled: false, want: false}, + {name: "control_only", envknob: false, controlDisabled: true, want: true}, + {name: "envknob_only", envknob: true, controlDisabled: false, want: true}, + {name: "both", envknob: true, controlDisabled: true, want: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envknob { + t.Setenv("TS_NO_LOGS_NO_SUPPORT", "true") + } else { + t.Setenv("TS_NO_LOGS_NO_SUPPORT", "") + } + + b := newTestLocalBackend(t) + b.noLogsFromControl.Store(tt.controlDisabled) + + if got := b.NoLogsNoSupport(); got != tt.want { + t.Errorf("NoLogsNoSupport() = %v, want %v (envknob=%v, control=%v)", + got, tt.want, tt.envknob, tt.controlDisabled) + } + }) + } +} + +// withCapMap is a peerOptFunc that sets the given capabilities on a node. +func withCapMap(caps tailcfg.NodeCapMap) peerOptFunc { + return func(n *tailcfg.Node) { + for k, v := range caps { + mak.Set(&n.CapMap, k, v) + } + } +} + +// buildNetmapWithFlowLogs constructs a netmap whose self node advertises +// CapabilityDataPlaneAuditLogs, with AllCaps properly populated. +func buildNetmapWithFlowLogs(selfID tailcfg.NodeID) *netmap.NetworkMap { + self := makePeer(selfID, + withName("self"), + withAddresses(netip.MustParsePrefix("100.64.1.1/32")), + withCapMap(tailcfg.NodeCapMap{ + tailcfg.CapabilityDataPlaneAuditLogs: nil, + }), + ) + nm := buildNetmapWithPeers(self) + nm.AllCaps = set.SetOf([]tailcfg.NodeCapability{tailcfg.CapabilityDataPlaneAuditLogs}) + return nm +} + +func TestFlowLogsConflictCheck(t *testing.T) { + // This test mutates package-level logtail state. + t.Cleanup(func() { logtail.Enable() }) + + t.Run("control_disabled_logs", func(t *testing.T) { + t.Setenv("TS_NO_LOGS_NO_SUPPORT", "") + + var cc *mockControl + lb := newLocalBackendWithTestControl(t, false, func(tb testing.TB, opts controlclient.Options) controlclient.Client { + cc = newClient(tb, opts) + return cc + }) + + // Capture error notifications. + gotErrMsg := make(chan string, 1) + lb.SetNotifyCallback(func(n ipn.Notify) { + if n.ErrMessage != nil { + select { + case gotErrMsg <- *n.ErrMessage: + default: + } + } + }) + + mustDo(t)(lb.Start(ipn.Options{})) + mustDo2(t)(lb.EditPrefs(&ipn.MaskedPrefs{ + Prefs: ipn.Prefs{WantRunning: true}, + WantRunningSet: true, + })) + + // Authenticate with a plain netmap (no flow logs cap). + plainNM := buildNetmapWithPeers( + makePeer(1, withName("self"), withAddresses(netip.MustParsePrefix("100.64.1.1/32"))), + ) + cc.authenticated(plainNM) + waitForGoroutinesToStop(lb) + + // Simulate control plane having disabled logs (Headscale). + lb.noLogsFromControl.Store(true) + + // Now send a netmap that requires flow logs. + flowNM := buildNetmapWithFlowLogs(1) + cc.send(sendOpt{loginFinished: true, nm: flowNM}) + + if err := tstest.WaitFor(5*time.Second, func() error { + if lb.Prefs().WantRunning() { + return fmt.Errorf("WantRunning is still true; expected false after flow-logs conflict") + } + return nil + }); err != nil { + t.Fatal(err) + } + + // Verify the error notification mentions "control server". + select { + case msg := <-gotErrMsg: + if !strings.Contains(msg, "disabled by the control server") { + t.Errorf("error message %q does not mention control server", msg) + } + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for error notification") + } + }) + + t.Run("envknob_disabled_logs", func(t *testing.T) { + t.Setenv("TS_NO_LOGS_NO_SUPPORT", "true") + + var cc *mockControl + lb := newLocalBackendWithTestControl(t, false, func(tb testing.TB, opts controlclient.Options) controlclient.Client { + cc = newClient(tb, opts) + return cc + }) + + // Capture error notifications. + gotErrMsg := make(chan string, 1) + lb.SetNotifyCallback(func(n ipn.Notify) { + if n.ErrMessage != nil { + select { + case gotErrMsg <- *n.ErrMessage: + default: + } + } + }) + + mustDo(t)(lb.Start(ipn.Options{})) + mustDo2(t)(lb.EditPrefs(&ipn.MaskedPrefs{ + Prefs: ipn.Prefs{WantRunning: true}, + WantRunningSet: true, + })) + + // Authenticate with a netmap requiring flow logs. + flowNM := buildNetmapWithFlowLogs(1) + cc.authenticated(flowNM) + + if err := tstest.WaitFor(5*time.Second, func() error { + if lb.Prefs().WantRunning() { + return fmt.Errorf("WantRunning is still true; expected false after flow-logs conflict") + } + return nil + }); err != nil { + t.Fatal(err) + } + + // Verify the error notification mentions the CLI flag. + select { + case msg := <-gotErrMsg: + if !strings.Contains(msg, "no-logs-no-support") { + t.Errorf("error message %q does not mention --no-logs-no-support", msg) + } + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for error notification") + } + }) + + t.Run("no_conflict_without_cap", func(t *testing.T) { + t.Setenv("TS_NO_LOGS_NO_SUPPORT", "") + + var cc *mockControl + lb := newLocalBackendWithTestControl(t, false, func(tb testing.TB, opts controlclient.Options) controlclient.Client { + cc = newClient(tb, opts) + return cc + }) + + mustDo(t)(lb.Start(ipn.Options{})) + mustDo2(t)(lb.EditPrefs(&ipn.MaskedPrefs{ + Prefs: ipn.Prefs{WantRunning: true}, + WantRunningSet: true, + })) + + lb.noLogsFromControl.Store(true) + + // Netmap without CapabilityDataPlaneAuditLogs → no conflict. + plainNM := buildNetmapWithPeers( + makePeer(1, withName("self"), withAddresses(netip.MustParsePrefix("100.64.1.1/32"))), + ) + cc.authenticated(plainNM) + waitForGoroutinesToStop(lb) + + if !lb.Prefs().WantRunning() { + t.Error("WantRunning should still be true; no flow-logs conflict expected") + } + }) +} + +func TestStartLockedClearsControlDisableLogTail(t *testing.T) { + // This test mutates package-level logtail state. + t.Cleanup(func() { logtail.Enable() }) + t.Setenv("TS_NO_LOGS_NO_SUPPORT", "") + + lb := newLocalBackendWithTestControl(t, false, func(tb testing.TB, opts controlclient.Options) controlclient.Client { + return newClient(tb, opts) + }) + + // Simulate a previous control server having disabled logs. + lb.noLogsFromControl.Store(true) + logtail.Disable() + + mustDo(t)(lb.Start(ipn.Options{})) + + if err := tstest.WaitFor(2*time.Second, func() error { + if lb.noLogsFromControl.Load() { + return fmt.Errorf("noLogsFromControl should be cleared after Start") + } + return nil + }); err != nil { + t.Fatal(err) + } + + if err := tstest.WaitFor(2*time.Second, func() error { + if lb.NoLogsNoSupport() { + return fmt.Errorf("NoLogsNoSupport() should be false after Start re-enabled logging") + } + return nil + }); err != nil { + t.Fatal(err) + } +} + +func TestProfileSwitchHeadscaleToTailscale(t *testing.T) { + // This test mutates package-level logtail state. + t.Cleanup(func() { logtail.Enable() }) + t.Setenv("TS_NO_LOGS_NO_SUPPORT", "") + + var cc *mockControl + lb := newLocalBackendWithTestControl(t, false, func(tb testing.TB, opts controlclient.Options) controlclient.Client { + cc = newClient(tb, opts) + return cc + }) + + // Step 1: Start and connect (simulating Headscale). + mustDo(t)(lb.Start(ipn.Options{})) + mustDo2(t)(lb.EditPrefs(&ipn.MaskedPrefs{ + Prefs: ipn.Prefs{WantRunning: true}, + WantRunningSet: true, + })) + plainNM := buildNetmapWithPeers( + makePeer(1, withName("self"), withAddresses(netip.MustParsePrefix("100.64.1.1/32"))), + ) + cc.authenticated(plainNM) + waitForGoroutinesToStop(lb) + + // Simulate Headscale having sent DisableLogTail. + lb.noLogsFromControl.Store(true) + logtail.Disable() + + // Step 2: Re-start the backend (simulating a profile switch to a Tailscale + // control server). startLocked should clear the per-instance flag and + // re-enable logtail. + mustDo(t)(lb.Start(ipn.Options{})) + mustDo2(t)(lb.EditPrefs(&ipn.MaskedPrefs{ + Prefs: ipn.Prefs{WantRunning: true}, + WantRunningSet: true, + })) + + if err := tstest.WaitFor(2*time.Second, func() error { + if lb.noLogsFromControl.Load() { + return fmt.Errorf("noLogsFromControl should be cleared after profile switch") + } + return nil + }); err != nil { + t.Fatal(err) + } + + // Step 3: Authenticate with a netmap that requires flow logs. + // This should succeed because noLogsFromControl was cleared. + flowNM := buildNetmapWithFlowLogs(1) + cc.authenticated(flowNM) + waitForGoroutinesToStop(lb) + + if err := tstest.WaitFor(2*time.Second, func() error { + if !lb.Prefs().WantRunning() { + return fmt.Errorf("WantRunning should be true; profile switch should have cleared no-logs state") + } + return nil + }); err != nil { + t.Fatal(err) + } +} + +func TestGlobalNoLogsPreventReEnable(t *testing.T) { + // This test mutates package-level logtail state. + t.Cleanup(func() { logtail.Enable() }) + t.Setenv("TS_NO_LOGS_NO_SUPPORT", "true") + + var cc *mockControl + lb := newLocalBackendWithTestControl(t, false, func(tb testing.TB, opts controlclient.Options) controlclient.Client { + cc = newClient(tb, opts) + return cc + }) + + // Capture error notifications. + gotErrMsg := make(chan string, 1) + lb.SetNotifyCallback(func(n ipn.Notify) { + if n.ErrMessage != nil { + select { + case gotErrMsg <- *n.ErrMessage: + default: + } + } + }) + + // Simulate a previous control server having disabled logs. + lb.noLogsFromControl.Store(true) + logtail.Disable() + + // Start should clear the per-instance flag but NOT re-enable logtail + // because the global envknob is set. + mustDo(t)(lb.Start(ipn.Options{})) + + if err := tstest.WaitFor(2*time.Second, func() error { + // The per-instance flag is always cleared on Start. + if lb.noLogsFromControl.Load() { + return fmt.Errorf("noLogsFromControl should be cleared after Start") + } + return nil + }); err != nil { + t.Fatal(err) + } + + // But NoLogsNoSupport should still be true because of the envknob. + if !lb.NoLogsNoSupport() { + t.Fatal("NoLogsNoSupport() should still be true (envknob is set)") + } + + // Authenticate with a netmap requiring flow logs → conflict. + mustDo2(t)(lb.EditPrefs(&ipn.MaskedPrefs{ + Prefs: ipn.Prefs{WantRunning: true}, + WantRunningSet: true, + })) + + flowNM := buildNetmapWithFlowLogs(1) + cc.authenticated(flowNM) + + if err := tstest.WaitFor(5*time.Second, func() error { + if lb.Prefs().WantRunning() { + return fmt.Errorf("WantRunning should be false; global no-logs should conflict with flow logs") + } + return nil + }); err != nil { + t.Fatal(err) + } + + // Verify the error notification mentions the CLI flag. + select { + case msg := <-gotErrMsg: + if !strings.Contains(msg, "no-logs-no-support") { + t.Errorf("error message %q does not mention --no-logs-no-support", msg) + } + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for error notification") + } +} + func TestPreferencePolicyInfo(t *testing.T) { tests := []struct { name string diff --git a/logtail/logtail_test.go b/logtail/logtail_test.go index 19e1eeb7a..452c6c5fb 100644 --- a/logtail/logtail_test.go +++ b/logtail/logtail_test.go @@ -470,6 +470,37 @@ func BenchmarkWriteText(b *testing.B) { } } +func TestEnableDisableRoundTrip(t *testing.T) { + // This test mutates package-level logtail state and must not run + // in parallel with tests that depend on logtailDisabled. + t.Cleanup(func() { logtailDisabled.Store(false) }) + + // Initially not disabled. + if logtailDisabled.Load() { + t.Fatal("logtailDisabled should be false at start of test") + } + + Disable() + if !logtailDisabled.Load() { + t.Fatal("logtailDisabled should be true after Disable()") + } + + Enable() + if logtailDisabled.Load() { + t.Fatal("logtailDisabled should be false after Enable()") + } + + // Verify a second round-trip works. + Disable() + if !logtailDisabled.Load() { + t.Fatal("logtailDisabled should be true after second Disable()") + } + Enable() + if logtailDisabled.Load() { + t.Fatal("logtailDisabled should be false after second Enable()") + } +} + func BenchmarkWriteJSON(b *testing.B) { var lg Logger lg.clock = tstime.StdClock{}