From baca8309f16b123218c47f81037961629d2787a3 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Sep 2026 09:19:47 +0000 Subject: [PATCH] servertest: reproduce removed peers missing from IPN bus deltas Delta-only IPN bus watchers (the Android app since 1.100) miss removals riding a full-rebuild response: deleted, policy-hidden, or while offline. Updates tailscale/tailscale#15660 --- hscontrol/servertest/client.go | 70 ++++++++++++++++++++-- hscontrol/servertest/issues_test.go | 91 +++++++++++++++++++++++++++++ hscontrol/servertest/server.go | 15 +++++ 3 files changed, 171 insertions(+), 5 deletions(-) diff --git a/hscontrol/servertest/client.go b/hscontrol/servertest/client.go index 67232dd33..9db32b5ce 100644 --- a/hscontrol/servertest/client.go +++ b/hscontrol/servertest/client.go @@ -3,6 +3,7 @@ package servertest import ( "context" "fmt" + "slices" "sync" "testing" "time" @@ -16,7 +17,9 @@ import ( "tailscale.com/types/key" "tailscale.com/types/netmap" "tailscale.com/types/persist" + "tailscale.com/types/views" "tailscale.com/util/eventbus" + "tailscale.com/wgengine/filter" ) // TestClient wraps a Tailscale [controlclient.Direct] connected to a @@ -42,6 +45,10 @@ type TestClient struct { netmap *netmap.NetworkMap history []*netmap.NetworkMap + // deltaUpdates and deltaRemoved: see [WithDeltaUpdates]. + deltaUpdates bool + deltaRemoved []tailcfg.NodeID + // updates is a buffered channel that receives a signal // each time a new [netmap.NetworkMap] arrives. updates chan *netmap.NetworkMap @@ -55,10 +62,19 @@ type TestClient struct { type ClientOption func(*clientConfig) type clientConfig struct { - ephemeral bool - hostname string - tags []string - user *types.User + ephemeral bool + hostname string + tags []string + user *types.User + deltaUpdates bool +} + +// WithDeltaUpdates makes the client accept incremental map updates the way +// a real tailscaled does, and record the peer removals it could apply that +// way ([TestClient.DeltaRemovedPeers]). Only those removals reach IPN bus +// watchers opted out of full netmaps, such as the Android app. +func WithDeltaUpdates() ClientOption { + return func(c *clientConfig) { c.deltaUpdates = true } } // WithEphemeral makes the client register as an ephemeral node. @@ -156,6 +172,8 @@ func NewClient(tb testing.TB, server *TestServer, name string, opts ...ClientOpt bus: bus, dialer: dialer, tracker: tracker, + + deltaUpdates: cc.deltaUpdates, } tb.Cleanup(func() { @@ -205,7 +223,12 @@ func (c *TestClient) startPollLoop() { go func() { defer close(c.pollDone) - _ = c.direct.PollNetMap(c.pollCtx, c) + var updater controlclient.NetmapUpdater = c + if c.deltaUpdates { + updater = deltaRecorder{c} + } + + _ = c.direct.PollNetMap(c.pollCtx, updater) }() } @@ -241,6 +264,43 @@ func (c *TestClient) UpdateFullNetmap(nm *netmap.NetworkMap) { } } +// deltaRecorder is the [controlclient.NetmapUpdater] of a [WithDeltaUpdates] +// client. Like tailscaled it takes packet filter and user profile updates +// narrowly, which lets [controlclient.Direct] try the delta path. It +// records removals and declines the mutations, so the full netmap is +// still delivered and the client's peer state stays exact. +type deltaRecorder struct{ *TestClient } + +func (d deltaRecorder) UpdatePacketFilter(views.Slice[tailcfg.FilterRule], []filter.Match) bool { + return true +} + +func (d deltaRecorder) UpdateUserProfiles(map[tailcfg.UserID]tailcfg.UserProfileView) bool { + return true +} + +func (d deltaRecorder) UpdateNetmapDelta(muts []netmap.NodeMutation) bool { + d.mu.Lock() + defer d.mu.Unlock() + + for _, m := range muts { + if _, ok := m.(netmap.NodeMutationRemove); ok { + d.deltaRemoved = append(d.deltaRemoved, m.NodeIDBeingMutated()) + } + } + + return false +} + +// DeltaRemovedPeers returns the peers whose removal arrived in a response +// the client could apply incrementally; see [WithDeltaUpdates]. +func (c *TestClient) DeltaRemovedPeers() []tailcfg.NodeID { + c.mu.RLock() + defer c.mu.RUnlock() + + return slices.Clone(c.deltaRemoved) +} + // cleanup releases all resources. func (c *TestClient) cleanup() { if c.pollCancel != nil { diff --git a/hscontrol/servertest/issues_test.go b/hscontrol/servertest/issues_test.go index 329466f50..7b0a10901 100644 --- a/hscontrol/servertest/issues_test.go +++ b/hscontrol/servertest/issues_test.go @@ -995,6 +995,97 @@ func TestIssuesIdentity(t *testing.T) { }) } +// TestPeerRemovedAsDelta checks a peer leaving a client's view reaches it +// in a response it can apply as a delta. Bundled with DNSConfig or +// SSHPolicy, the removal forces a full netmap rebuild, which never tells IPN +// bus watchers opted out of full netmaps (the Android app since 1.100) that +// the peer is gone, so they keep showing it. +func TestPeerRemovedAsDelta(t *testing.T) { + t.Parallel() + + // MagicDNS puts DNSConfig in policy responses, making them full rebuilds. + setup := func(t *testing.T) (*servertest.TestServer, *servertest.TestClient, types.NodeID) { + t.Helper() + + srv := servertest.NewServer(t, servertest.WithMagicDNS("delta.example.com")) + user1 := srv.CreateUser(t, "delta-user1") + user2 := srv.CreateUser(t, "delta-user2") + + c1 := servertest.NewClient(t, srv, "delta-node1", + servertest.WithUser(user1), servertest.WithDeltaUpdates()) + servertest.NewClient(t, srv, "delta-node2", servertest.WithUser(user2)) + + c1.WaitForPeers(t, 1, 15*time.Second) + + return srv, c1, findNodeID(t, srv, "delta-node2") + } + + assertRemovedAsDelta := func(t *testing.T, c1 *servertest.TestClient, id types.NodeID) { + t.Helper() + + assert.EventuallyWithT(t, func(c *assert.CollectT) { + assert.Contains(c, c1.DeltaRemovedPeers(), id.NodeID()) + }, 10*time.Second, 50*time.Millisecond, "peer removal never applied as a delta") + } + + t.Run("node_deleted", func(t *testing.T) { + t.Parallel() + + srv, c1, nodeID2 := setup(t) + + node2, ok := srv.State().GetNodeByID(nodeID2) + require.True(t, ok) + + changes, err := srv.State().DeleteNode(node2) + require.NoError(t, err) + srv.App.Change(changes) + + assertRemovedAsDelta(t, c1, nodeID2) + }) + + // The new stream's initial map is a full rebuild, so a removal missed + // while disconnected must still follow as a delta. + t.Run("deleted_while_disconnected", func(t *testing.T) { + t.Parallel() + + srv, c1, nodeID2 := setup(t) + + node2, ok := srv.State().GetNodeByID(nodeID2) + require.True(t, ok) + + c1.Disconnect(t) + + changes, err := srv.State().DeleteNode(node2) + require.NoError(t, err) + srv.App.Change(changes) + + c1.Reconnect(t) + + assertRemovedAsDelta(t, c1, nodeID2) + }) + + t.Run("hidden_by_policy", func(t *testing.T) { + t.Parallel() + + srv, c1, nodeID2 := setup(t) + + changed, err := srv.State().SetPolicy([]byte(`{ + "acls": [ + {"action": "accept", "src": ["delta-user1@"], "dst": ["delta-user1@:*"]}, + {"action": "accept", "src": ["delta-user2@"], "dst": ["delta-user2@:*"]} + ] + }`)) + require.NoError(t, err) + require.True(t, changed) + + changes, err := srv.State().ReloadPolicy() + require.NoError(t, err) + srv.App.Change(changes...) + + assertRemovedAsDelta(t, c1, nodeID2) + }) +} + func findNodeID(tb testing.TB, srv *servertest.TestServer, hostname string) types.NodeID { tb.Helper() diff --git a/hscontrol/servertest/server.go b/hscontrol/servertest/server.go index c80314fdb..65e351583 100644 --- a/hscontrol/servertest/server.go +++ b/hscontrol/servertest/server.go @@ -45,6 +45,7 @@ type serverConfig struct { batcherWorkers int taildropEnabled bool realListener bool + magicDNSDomain string } func defaultServerConfig() *serverConfig { @@ -101,6 +102,12 @@ func WithTaildropEnabled(enabled bool) ServerOption { return func(c *serverConfig) { c.taildropEnabled = enabled } } +// WithMagicDNS enables MagicDNS under domain, so map responses carry a +// [tailcfg.DNSConfig]. +func WithMagicDNS(domain string) ServerOption { + return func(c *serverConfig) { c.magicDNSDomain = domain } +} + // NewServer creates and starts a Headscale test server. // The server is fully functional and accepts real Tailscale control // protocol connections over Noise. @@ -147,6 +154,14 @@ func NewServer(tb testing.TB, opts ...ServerOption) *TestServer { }, } + if sc.magicDNSDomain != "" { + cfg.BaseDomain = sc.magicDNSDomain + cfg.TailcfgDNSConfig = &tailcfg.DNSConfig{ + Proxied: true, + Domains: []string{sc.magicDNSDomain}, + } + } + app, err := hscontrol.NewHeadscale(&cfg) if err != nil { tb.Fatalf("servertest: NewHeadscale: %v", err)