From eeaac680bef26585c8cc1569f949e26d0d6aba80 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Sep 2026 13:18:01 +0000 Subject: [PATCH] mapper: drop DNSConfig from policy responses It forced every client into a full netmap rebuild; the resolver race it guarded against was a client bug (tailscale/tailscale#19749). --- CHANGELOG.md | 1 + hscontrol/mapper/batcher_test.go | 25 +++++++++++++++++++++++++ hscontrol/mapper/mapper.go | 11 ++++------- hscontrol/servertest/nodeattrs_test.go | 1 + 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 154fd7f2f..58086a0fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,7 @@ clients, and how to run the same setup without Nix. - Fix `headscale users destroy`/`rename` reporting "multiple users match query" when no user matches; an ambiguous match now lists the matching users [#3476](https://github.com/juanfont/headscale/pull/3476) - Deleting a user that still owns nodes now lists the nodes (ID and hostname) that must be deleted first [#3475](https://github.com/juanfont/headscale/pull/3475) - Fix deleted nodes, and peers hidden by a policy change, staying listed in the Tailscale Android app; removed peers are now sent as their own incremental map update [#3492](https://github.com/juanfont/headscale/pull/3492) +- Policy changes no longer resend DNS configuration to every node, sparing clients a full netmap rebuild; a node gets its DNS configuration when its own NextDNS nodeAttrs, tags or hostname change, which also fixes NextDNS device metadata going stale after a hostname change [#3492](https://github.com/juanfont/headscale/pull/3492) - Headscale now requires Go 1.27 to build - `headscale preauthkeys create --user` accepts a user name as well as an ID - `derp.paths` files may be Tailscale JSON or HuJSON DERP maps as well as YAML diff --git a/hscontrol/mapper/batcher_test.go b/hscontrol/mapper/batcher_test.go index 746661f54..daf298064 100644 --- a/hscontrol/mapper/batcher_test.go +++ b/hscontrol/mapper/batcher_test.go @@ -2446,3 +2446,28 @@ func TestHandleNodeChangeRetryAfterRemoval(t *testing.T) { assert.Empty(t, sent[1].PeersRemoved) assert.NotEmpty(t, sent[1].PacketFilters) } + +// TestDNSConfigOnlyWithSelfRefresh checks policy responses leave DNSConfig +// out, which clients read as unchanged, while self refreshes carry it: a +// node's DNS config derives from its own CapMap and Hostinfo, and a +// DNSConfig forces clients into a full netmap rebuild. +func TestDNSConfigOnlyWithSelfRefresh(t *testing.T) { + testData, cleanup := setupBatcherWithTestData(t, NewBatcherAndMapper, 1, 2, normalBufferSize) + defer cleanup() + + testData.Config.TailcfgDNSConfig = &tailcfg.DNSConfig{ + Proxied: true, + Domains: []string{"headscale.test"}, + } + + self := testData.Nodes[0].n.ID + mc := newMockNodeConnection(self) + + require.NoError(t, handleNodeChange(mc, testData.Batcher.mapper, change.PolicyChange())) + require.NoError(t, handleNodeChange(mc, testData.Batcher.mapper, change.SelfUpdate(self))) + + sent := mc.getSent() + require.Len(t, sent, 2) + assert.Nil(t, sent[0].DNSConfig, "policy response must not carry DNSConfig") + assert.NotNil(t, sent[1].DNSConfig, "self refresh must carry DNSConfig") +} diff --git a/hscontrol/mapper/mapper.go b/hscontrol/mapper/mapper.go index b136681ae..53af3de19 100644 --- a/hscontrol/mapper/mapper.go +++ b/hscontrol/mapper/mapper.go @@ -309,14 +309,12 @@ func (m *mapper) selfMapResponse( // - PeersChanged for remaining peers (their AllowedIPs may have changed due to policy) // - Updated PacketFilters // - Updated SSHPolicy (SSH rules may reference users/groups that changed) -// - DNSConfig so the client's resolver state stays anchored even when a -// policy-triggered wgengine reconfigure races a netmon LinkChange (the -// LinkChange handler reapplies dns.Manager.Set with the engine's -// lastDNSConfig; if that snapshot is stale, the OS resolver loses the -// MagicDNS reverse-DNS routes and Nameservers and curl-by-FQDN stops -// resolving for the rest of the policy window). // - Optionally, the node's own self info (when includeSelf is true) // +// DNSConfig is left out: it forces clients into a full netmap rebuild, and +// the node's DNS config inputs arrive with its own [change.SelfUpdate], see +// [state.State.DrainSelfRefreshes]. +// // This avoids the issue where an empty Peers slice is interpreted by Tailscale // clients as "no change" rather than "no peers". // When includeSelf is true, the node's self info is included so that a node @@ -331,7 +329,6 @@ func (m *mapper) policyChangeResponse( builder := m.NewMapResponseBuilder(nodeID). WithDebugType(policyResponseDebug). WithCapabilityVersion(capVer). - WithDNSConfig(). WithPacketFilters(). WithSSHPolicy() diff --git a/hscontrol/servertest/nodeattrs_test.go b/hscontrol/servertest/nodeattrs_test.go index c84eec1f5..50d4e8540 100644 --- a/hscontrol/servertest/nodeattrs_test.go +++ b/hscontrol/servertest/nodeattrs_test.go @@ -477,6 +477,7 @@ func firstResolver(nm *netmap.NetworkMap) string { // TestNodeAttrsNextDNS checks a node's DNS config follows each of its // inputs: the NextDNS profile from nodeAttrs, whether reached through a // policy reload or a tag change, and the device metadata from its Hostinfo. +// Policy responses do not carry DNSConfig, so each must arrive on its own. func TestNodeAttrsNextDNS(t *testing.T) { t.Parallel()