From c6723fa103835de68886f8ac0e1c12a43e118e88 Mon Sep 17 00:00:00 2001 From: Igor Serganov <41109559+iSerganov@users.noreply.github.com> Date: Mon, 2 Nov 2026 12:28:28 -0700 Subject: [PATCH] state: keep collision-bumped GivenName within the DNS label limit (#3635) `dnsname.SanitizeHostname` truncates a hostname to 63 characters. When that label was already taken, resolveGivenName appended "-N" to the full base, producing a 65+ character GivenName. Such a name is not a valid DNS label, so the node cannot be rendered into its own netmap or any peer's, and on PostgreSQL the varchar(63) given_name column rejects the write. Trim the base so base plus suffix fits in a DNS label and strip a hyphen left at the cut. isAutoDerivedGivenName now compares against the same bump, so a trimmed name still follows hostname changes instead of being treated as an admin rename. --- hscontrol/state/node_store.go | 2 +- hscontrol/state/node_store_hostname_test.go | 64 +++++++++++++++++++++ hscontrol/state/state.go | 12 +++- 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/hscontrol/state/node_store.go b/hscontrol/state/node_store.go index 13c165d96..979c2156b 100644 --- a/hscontrol/state/node_store.go +++ b/hscontrol/state/node_store.go @@ -763,7 +763,7 @@ func resolveGivenName( func bumpedLabel(base string, n int) string { suffix := "-" + strconv.Itoa(n) - return base[:min(len(base), util.LabelHostnameLength-len(suffix))] + suffix + return strings.TrimRight(base[:min(len(base), util.LabelHostnameLength-len(suffix))], "-") + suffix } // snapshotFromNodes builds a Snapshot from nodes. With reusePeers the diff --git a/hscontrol/state/node_store_hostname_test.go b/hscontrol/state/node_store_hostname_test.go index f9a01d88d..d8e8b8b68 100644 --- a/hscontrol/state/node_store_hostname_test.go +++ b/hscontrol/state/node_store_hostname_test.go @@ -7,9 +7,11 @@ import ( "testing" "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "tailscale.com/tailcfg" + "tailscale.com/util/dnsname" ) func TestNewNodeStoreBumpsDuplicateNames(t *testing.T) { @@ -308,3 +310,65 @@ func TestSetGivenNameIdempotent(t *testing.T) { require.NoError(t, err) require.Equal(t, "laptop", view.GivenName()) } + +func TestPutNodeGivenNameBumpFitsDNSLabel(t *testing.T) { + maxLabel := strings.Repeat("a", util.LabelHostnameLength) + + tests := []struct { + name string + hostname string + holders int + want string + }{ + {name: "short base keeps full base", hostname: "laptop", holders: 1, want: "laptop-1"}, + {name: "max length base trimmed for one digit suffix", hostname: maxLabel, holders: 1, want: strings.Repeat("a", 61) + "-1"}, + {name: "max length base trimmed for two digit suffix", hostname: maxLabel, holders: 10, want: strings.Repeat("a", 60) + "-10"}, + {name: "hyphen left at cut point is stripped", hostname: strings.Repeat("a", 60) + "-bb", holders: 1, want: strings.Repeat("a", 60) + "-1"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout) + + store.Start() + defer store.Stop() + + for id := range tt.holders { + store.PutNode(createTestNode(types.NodeID(id+1), 1, "alice", tt.hostname)) //nolint:gosec // test ids + } + + got := store.PutNode(createTestNode(types.NodeID(tt.holders+1), 1, "alice", tt.hostname)).GivenName() //nolint:gosec // test ids + + require.Equal(t, tt.want, got) + require.NoError(t, dnsname.ValidLabel(got)) + }) + } +} + +func TestIsAutoDerivedGivenName(t *testing.T) { + maxLabel := strings.Repeat("a", util.LabelHostnameLength) + + tests := []struct { + name string + given string + hostname string + want bool + }{ + {name: "sanitised hostname", given: "laptop", hostname: "laptop", want: true}, + {name: "short bump", given: "laptop-1", hostname: "laptop", want: true}, + {name: "signed suffix is not a generated name", given: "laptop-+1", hostname: "laptop", want: false}, + {name: "trimmed max length bump", given: strings.Repeat("a", 61) + "-1", hostname: maxLabel, want: true}, + {name: "hyphen trimmed at cut point", given: strings.Repeat("a", 60) + "-1", hostname: strings.Repeat("a", 60) + "-bb", want: true}, + {name: "legacy hyphen kept at cut point", given: strings.Repeat("a", 60) + "--1", hostname: strings.Repeat("a", 60) + "-bb", want: true}, + {name: "zero padded suffix is not a generated name", given: "laptop-01", hostname: "laptop", want: false}, + {name: "admin name with repeated hyphen", given: "laptop--1", hostname: "laptop", want: false}, + {name: "trimmed prefix with mismatched suffix length", given: strings.Repeat("a", 61) + "-10", hostname: maxLabel, want: false}, + {name: "admin rename", given: "workstation", hostname: "laptop", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, isAutoDerivedGivenName(tt.given, tt.hostname)) + }) + } +} diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 57ef214e6..5c386d097 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -4125,8 +4125,18 @@ func isAutoDerivedGivenName(given, hostname string) bool { } n, err := strconv.Atoi(suffix) + if err != nil { + return false + } - return err == nil && given == bumpedLabel(base, n) + if given == bumpedLabel(base, n) { + return true + } + + // Keep following labels generated before cut-point hyphens were trimmed. + suffix = "-" + strconv.Itoa(n) + + return given == base[:min(len(base), util.LabelHostnameLength-len(suffix))]+suffix } func dnsMetadataChanged(previous, reported *tailcfg.Hostinfo) bool {