state: cut a label so its -N suffix fits in 63 bytes

hscontrol/servertest/names_compat_test.go

@@ -31,19 +31,6 @@ import (
 // <test_id>/<step>/<node or action index>/<check>. A listed check that
 // matches fails, so a fix must delete its entry.
 var nameSkipReasons = map[string]string{
-	"names/N1/n63b/label":  labelSuffixOverflow,
-	"names/N2/n63b/label":  labelSuffixOverflow,
-	"names/N3/n63b/label":  labelSuffixOverflow,
-	"names/N4/n63b/label":  labelSuffixOverflow,
-	"names/N5/n63b/label":  labelSuffixOverflow,
-	"names/N6/n63b/label":  labelSuffixOverflow,
-	"names/N7/n63b/label":  labelSuffixOverflow,
-	"names/N8/n63b/label":  labelSuffixOverflow,
-	"names/N9a/n63b/label": labelSuffixOverflow,
-	"names/N9b/n63b/label": labelSuffixOverflow,
-	"names/N10/n63b/label": labelSuffixOverflow,
-	"names/N11/n63b/label": labelSuffixOverflow,
-
 	"names/N1/domain":     v2NameBare,
 	"names/N2/domain":     v2NameBare,
 	"names/N3/domain":     v2NameBare,
@@ -81,7 +68,6 @@ var nameSkipReasons = map[string]string{
 }
 
 const (
-	labelSuffixOverflow      = "LABEL_SUFFIX_OVERFLOW: headscale appends -N to a 63-byte label, giving a 65-byte one; SaaS cuts the label so the suffix fits in 63 bytes"
 	adminNameFollowsHostname = "ADMIN_NAME_FOLLOWS_HOSTNAME: an admin name equal to the hostname's label counts as derived from it, so a hostname change replaces it; SaaS keeps an admin name across hostname changes"
 	nameTakenText            = "NAME_TAKEN_TEXT: headscale's v2 message for a rename refused as taken is the operation, with the reason only in data; SaaS's message says the name is already taken"
 	v2NameBare               = "V2_NAME_BARE: headscale's v2 device name is the bare label; SaaS's is the FQDN under the tailnet domain"

hscontrol/state/node_store.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/juanfont/headscale/hscontrol/types"
+	"github.com/juanfont/headscale/hscontrol/util"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	"tailscale.com/net/tsaddr"
@@ -665,7 +666,8 @@ func (s *NodeStore) applyBatch(batch []work) {
 // keeps the current label.
 //
 // On collision the label is bumped as base, base-1, base-2, …, first
-// unused wins. Must be called from the [NodeStore] writer goroutine
+// unused wins; the base is cut so base-N stays a valid label, as Tailscale
+// does. Must be called from the [NodeStore] writer goroutine
 // (inside [NodeStore.applyBatch]) so the nodes map reflects all earlier
 // ops in the batch and no other writer can interleave.
 func resolveGivenName(
@@ -695,10 +697,18 @@ func resolveGivenName(
 			return candidate
 		}
 
-		candidate = base + "-" + strconv.Itoa(i)
+		candidate = bumpedLabel(base, i)
 	}
 }
 
+// bumpedLabel is base with a -n suffix, base cut so the result stays a valid
+// label, as Tailscale does.
+func bumpedLabel(base string, n int) string {
+	suffix := "-" + strconv.Itoa(n)
+
+	return base[:min(len(base), util.LabelHostnameLength-len(suffix))] + suffix
+}
+
 // snapshotFromNodes builds a Snapshot from nodes. With reusePeers the
 // previous peer-ID adjacency is carried over unchanged; with reuseRoutes
 // the previous route election is. ListPeers resolves adjacency IDs

hscontrol/state/node_store_hostname_test.go

@@ -2,6 +2,7 @@ package state
 
 import (
 	"fmt"
+	"strings"
 	"sync"
 	"testing"
 
@@ -61,6 +62,27 @@ func TestPutNodeGivenNameCollisionBumps(t *testing.T) {
 	require.Equal(t, "laptop-2", got3.GivenName(), "third registration bumps to -2")
 }
 
+func TestPutNodeBumpCutsLabelToFitSuffix(t *testing.T) {
+	store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
+
+	store.Start()
+	defer store.Stop()
+
+	long := strings.Repeat("a", 63)
+
+	got := make([]string, 0, 12)
+	for id := range types.NodeID(12) {
+		got = append(got, store.PutNode(createTestNode(id+1, 1, "alice", long)).GivenName())
+	}
+
+	require.Equal(t, long, got[0])
+	require.Equal(t, long[:61]+"-1", got[1])
+	require.Equal(t, long[:60]+"-10", got[10])
+	require.Equal(t, long[:60]+"-11", got[11])
+	require.True(t, isAutoDerivedGivenName(got[10], long),
+		"a cut label still follows its hostname")
+}
+
 // TestPutNodeEmptyGivenNameFallsBackToNode covers the SaaS rule that
 // an empty sanitised label becomes the literal "node". Subsequent
 // empty-label registrations bump as usual.

hscontrol/state/persist_test.go

@@ -1653,32 +1653,6 @@ func TestBackfillNodeIPsSurvivesConcurrentPersist(t *testing.T) {
 // differs from SaaS, keyed <test_id>/<step>/<node or action index>/<check>.
 // A listed check that matches fails, so a fix must delete its entry.
 var nameSkipReasons = map[string]string{
-	"names/N1/n63b/label":  labelSuffixOverflow,
-	"names/N2/n63b/label":  labelSuffixOverflow,
-	"names/N3/n63b/label":  labelSuffixOverflow,
-	"names/N4/n63b/label":  labelSuffixOverflow,
-	"names/N5/n63b/label":  labelSuffixOverflow,
-	"names/N6/n63b/label":  labelSuffixOverflow,
-	"names/N7/n63b/label":  labelSuffixOverflow,
-	"names/N8/n63b/label":  labelSuffixOverflow,
-	"names/N9a/n63b/label": labelSuffixOverflow,
-	"names/N9b/n63b/label": labelSuffixOverflow,
-	"names/N10/n63b/label": labelSuffixOverflow,
-	"names/N11/n63b/label": labelSuffixOverflow,
-
-	"names/N1/n63b/reopen":  labelSuffixOverflow,
-	"names/N2/n63b/reopen":  labelSuffixOverflow,
-	"names/N3/n63b/reopen":  labelSuffixOverflow,
-	"names/N4/n63b/reopen":  labelSuffixOverflow,
-	"names/N5/n63b/reopen":  labelSuffixOverflow,
-	"names/N6/n63b/reopen":  labelSuffixOverflow,
-	"names/N7/n63b/reopen":  labelSuffixOverflow,
-	"names/N8/n63b/reopen":  labelSuffixOverflow,
-	"names/N9a/n63b/reopen": labelSuffixOverflow,
-	"names/N9b/n63b/reopen": labelSuffixOverflow,
-	"names/N10/n63b/reopen": labelSuffixOverflow,
-	"names/N11/n63b/reopen": labelSuffixOverflow,
-
 	"names/N11/pdb/label":   adminNameFollowsHostname,
 	"names/N11/pdb/reopen":  adminNameFollowsHostname,
 	"names/N11b/pdb/label":  adminNameFollowsHostname,
@@ -1692,7 +1666,6 @@ var nameSkipReasons = map[string]string{
 }
 
 const (
-	labelSuffixOverflow      = "LABEL_SUFFIX_OVERFLOW: headscale appends -N to a 63-byte label, giving a 65-byte one; SaaS cuts the label so the suffix fits in 63 bytes"
 	adminNameFollowsHostname = "ADMIN_NAME_FOLLOWS_HOSTNAME: an admin name equal to the hostname's label counts as derived from it, so a hostname change replaces it; SaaS keeps an admin name across hostname changes"
 )
 

hscontrol/state/state.go

@@ -3900,23 +3900,23 @@ func (s *State) PingDB(ctx context.Context) error {
 
 // isAutoDerivedGivenName reports whether given matches what
 // dnsname.SanitizeHostname(hostname) would produce, optionally with a
-// [NodeStore] collision-bump "-N" suffix. It is used to detect whether a
-// GivenName has been admin-renamed (in which case it must not be
-// overwritten by client-side hostname changes).
+// [NodeStore] collision-bump "-N" suffix ([bumpedLabel]). It is used to
+// detect whether a GivenName has been admin-renamed (in which case it must
+// not be overwritten by client-side hostname changes).
 func isAutoDerivedGivenName(given, hostname string) bool {
 	base := dnsname.SanitizeHostname(hostname)
 	if given == base {
 		return true
 	}
 
-	suffix, ok := strings.CutPrefix(given, base+"-")
+	_, suffix, ok := strings.CutLast(given, "-")
 	if !ok {
 		return false
 	}
 
-	_, err := strconv.Atoi(suffix)
+	n, err := strconv.Atoi(suffix)
 
-	return err == nil
+	return err == nil && given == bumpedLabel(base, n)
 }
 
 // UpdateNodeFromMapRequest is the sync point where Hostinfo changes,