derp: keep node order across map refreshes

Clients close their home relay when its order changes. SaaS's never moved.

hscontrol/derp/derp.go

@@ -147,38 +147,45 @@ func shuffleDERPMap(dm *tailcfg.DERPMap) {
 
 	slices.Sort(ids)
 
+	// A new generator per call gives the same input the same order. Clients
+	// close their home relay when its node order changes, so a refresh that
+	// finds no change must not reorder anything.
+	rng := rand.New(rand.NewSource(derpShuffleSeed())) //nolint:gosec // weak random is fine for DERP scrambling
+
 	for _, id := range ids {
 		region := dm.Regions[id]
 		if len(region.Nodes) == 0 {
 			continue
 		}
 
-		derpRandom().Shuffle(len(region.Nodes), reflect.Swapper(region.Nodes))
+		rng.Shuffle(len(region.Nodes), reflect.Swapper(region.Nodes))
 	}
 }
 
 var crc64Table = crc64.MakeTable(crc64.ISO)
 
 var (
-	derpRandomInst *rand.Rand
-	derpRandomMu   sync.Mutex
+	derpSeed    int64
+	derpSeedSet bool
+	derpSeedMu  sync.Mutex
 )
 
-func derpRandom() *rand.Rand {
-	derpRandomMu.Lock()
-	defer derpRandomMu.Unlock()
+func derpShuffleSeed() int64 {
+	derpSeedMu.Lock()
+	defer derpSeedMu.Unlock()
 
-	if derpRandomInst == nil {
+	if !derpSeedSet {
 		seed := cmp.Or(viper.GetString("dns.base_domain"), time.Now().String())
-		derpRandomInst = rand.New(rand.NewSource(int64(crc64.Checksum([]byte(seed), crc64Table)))) //nolint:gosec // weak random is fine for DERP scrambling
+		derpSeed = int64(crc64.Checksum([]byte(seed), crc64Table)) //nolint:gosec // only a seed, overflow is harmless
+		derpSeedSet = true
 	}
 
-	return derpRandomInst
+	return derpSeed
 }
 
 func resetDerpRandomForTesting() {
-	derpRandomMu.Lock()
-	defer derpRandomMu.Unlock()
+	derpSeedMu.Lock()
+	defer derpSeedMu.Unlock()
 
-	derpRandomInst = nil
+	derpSeedSet = false
 }

hscontrol/derp/derp_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/require"
 	"tailscale.com/tailcfg"
@@ -262,6 +263,39 @@ func TestShuffleDERPMapDeterministic(t *testing.T) {
 	}
 }
 
+// TestGetDERPMapKeepsOrderAcrossRefreshes: clients close their home relay when
+// its node order changes, so a refresh of the same input must keep the order.
+func TestGetDERPMapKeepsOrderAcrossRefreshes(t *testing.T) {
+	viper.Set("dns.base_domain", "stable.example.com")
+
+	defer viper.Reset()
+
+	resetDerpRandomForTesting()
+
+	source := &tailcfg.DERPMap{Regions: map[tailcfg.DERPRegionID]*tailcfg.DERPRegion{}}
+
+	for id := range tailcfg.DERPRegionID(5) {
+		region := &tailcfg.DERPRegion{RegionID: id + 1}
+		for _, name := range []string{"a", "b", "c", "d", "e"} {
+			region.Nodes = append(region.Nodes, &tailcfg.DERPNode{Name: name, RegionID: id + 1})
+		}
+
+		source.Regions[id+1] = region
+	}
+
+	first, err := GetDERPMap(types.DERPConfig{DERPMap: source})
+	require.NoError(t, err)
+
+	for range 5 {
+		next, err := GetDERPMap(types.DERPConfig{DERPMap: source})
+		require.NoError(t, err)
+
+		if diff := cmp.Diff(first, next); diff != "" {
+			t.Fatalf("refresh changed the DERP map (-first +next):\n%s", diff)
+		}
+	}
+}
+
 func TestShuffleDERPMapEdgeCases(t *testing.T) {
 	tests := []struct {
 		name    string