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 {