policy/v2, servertest: add via source loss captures

Repeated via destinations, a user via source, and an empty one.
A rename that empties the via source must reach open streams as v67.

hscontrol/policy/v2/tailscale_grants_compat_test.go

@@ -169,8 +169,15 @@ var grantSkipReasons = map[string]string{
 	"via-grant-v60": "FILTER_ADDRESS_FORM: same addresses, different wire form and rule order",
 	"via-grant-v61": "FILTER_ADDRESS_FORM: same addresses, different wire form",
 	"via-grant-v62": "FILTER_ADDRESS_FORM: same addresses, different wire form",
+
+	// Checks keyed <test_id>/<node>/filter: a listed check that matches
+	// fails, so a fix must delete its entry.
+	"via-grant-v63/blastoise/filter": viaDstAddressForm,
+	"via-grant-v63/squirtle/filter":  viaDstAddressForm,
 }
 
+const viaDstAddressForm = "FILTER_ADDRESS_FORM: SaaS writes a single-address via destination as a bare address, headscale as a /32 prefix"
+
 // TestGrantsCompat is a data-driven test that loads all GRANT-*.json
 // test files captured from Tailscale SaaS and compares headscale's grants
 // engine output against the real Tailscale behavior.
@@ -298,49 +305,41 @@ func testGrantSuccess(
 	pm, err := NewPolicyManager(policyJSON, users, nodes.ViewSlice())
 	require.NoError(t, err, "%s: policy manager should load the policy", tf.TestID)
 
+	// EquateEmpty treats nil and empty slices as equal since Tailscale's
+	// JSON null -> nil, headscale may return empty slice.
+	opts := append(cmpOptions(), cmpopts.EquateEmpty())
+
 	for nodeName, capture := range tf.Captures {
 		t.Run(nodeName, func(t *testing.T) {
 			node := findGrantsNode(nodes, nodeName)
 			require.NotNilf(t, node,
 				"golden node %s not found in test setup", nodeName)
 
-			// Compile headscale filter rules for this node
-			gotRules := pol.compileFilterRulesForNode(
-				users,
-				node.View(),
-				nodes.ViewSlice(),
-			)
-
-			gotRules = policyutil.ReduceFilterRules(node.View(), gotRules)
-
 			wantRules := capture.PacketFilterRules
 
-			// Compare headscale output against Tailscale expected output.
-			// The diff labels show (-tailscale +headscale) to make clear
-			// which side produced which output.
-			// EquateEmpty treats nil and empty slices as equal since
-			// Tailscale's JSON null -> nil, headscale may return empty slice.
-			opts := append(cmpOptions(), cmpopts.EquateEmpty())
-			if diff := cmp.Diff(wantRules, gotRules, opts...); diff != "" {
-				t.Errorf(
-					"%s/%s: filter rules mismatch (-tailscale +headscale):\n%s",
-					tf.TestID,
-					nodeName,
-					diff,
+			compatCheck(t, grantSkipReasons, tf.TestID+"/"+nodeName+"/filter", func() string {
+				gotRules := policyutil.ReduceFilterRules(
+					node.View(),
+					pol.compileFilterRulesForNode(users, node.View(), nodes.ViewSlice()),
 				)
-			}
 
-			pmRules, err := pm.FilterForNode(node.View())
-			require.NoError(t, err)
+				pmRules, err := pm.FilterForNode(node.View())
+				if err != nil {
+					return err.Error()
+				}
 
-			if diff := cmp.Diff(wantRules, pmRules, opts...); diff != "" {
-				t.Errorf(
-					"%s/%s: policy manager filter rules mismatch (-tailscale +headscale):\n%s",
-					tf.TestID,
-					nodeName,
-					diff,
-				)
-			}
+				var diffs []string
+
+				if diff := cmp.Diff(wantRules, gotRules, opts...); diff != "" {
+					diffs = append(diffs, "filter rules mismatch (-tailscale +headscale):\n"+diff)
+				}
+
+				if diff := cmp.Diff(wantRules, pmRules, opts...); diff != "" {
+					diffs = append(diffs, "policy manager filter rules mismatch (-tailscale +headscale):\n"+diff)
+				}
+
+				return strings.Join(diffs, "\n")
+			})
 		})
 	}
 }

hscontrol/servertest/via_compat_test.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/juanfont/headscale/hscontrol/servertest"
 	"github.com/juanfont/headscale/hscontrol/state"
+	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/types/testcapture"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -69,6 +70,9 @@ var viaCompatTests = []struct {
 	{"via-grant-v60", "user-owned exit node outside the via tag; members have an autogroup:self rule"},
 	{"via-grant-v61", "via exit steering plus a member capability grant to a tagged host"},
 	{"via-grant-v62", "via exit steering plus a member rule to autogroup:internet on one port"},
+	{"via-grant-v63", "repeated and nested via destinations, and a second via grant on one port"},
+	{"via-grant-v66", "a user source steers a subnet via tag:router"},
+	{"via-grant-v67", "an empty group as the only via source"},
 }
 
 // routesMapCompatTests lists routes captures replayed through the mapper.
@@ -96,6 +100,11 @@ var routesMapCompatTests = []struct {
 var viaCompatSkipReasons = map[string]string{
 	"tagged-self": "TAGGED_SELF_PROFILE: SaaS gives a tagged node a profile of its own, headscale the shared tagged-devices one",
 
+	// TestViaGrantSourceLossCompat, keyed <test_id>/rename/<node>/netmap.
+	"via-grant-v67/rename/squirtle/netmap":  viaSourceLossNotSent,
+	"via-grant-v67/rename/blastoise/netmap": viaSourceLossNotSent,
+	"via-grant-v67/rename/ivysaur/netmap":   viaSourceLossNotSent,
+
 	// Adjacent member addresses merge into one prefix, and the reduced
 	// filter keeps all of it; SaaS lists only the node's own address.
 	"routes-f2-subnet-as-acl-source/ivysaur/packet-filter":  "FILTER_DST_MERGED: own filter names a neighbour's address",
@@ -215,8 +224,18 @@ var viaCompatSkipReasons = map[string]string{
 	"via-grant-v50/fearow/self-routes":    selfRoutesReduced,
 	"via-grant-v50/spearow/self-routes":   selfRoutesReduced,
 	"via-grant-v50/squirtle/self-routes":  selfRoutesReduced,
+	"via-grant-v63/blastoise/self-routes": selfRoutesReduced,
+	"via-grant-v63/fearow/self-routes":    selfRoutesReduced,
+	"via-grant-v63/spearow/self-routes":   selfRoutesReduced,
+	"via-grant-v63/squirtle/self-routes":  selfRoutesReduced,
+	"via-grant-v66/blastoise/self-routes": selfRoutesReduced,
+	"via-grant-v66/squirtle/self-routes":  selfRoutesReduced,
+	"via-grant-v67/blastoise/self-routes": selfRoutesReduced,
+	"via-grant-v67/squirtle/self-routes":  selfRoutesReduced,
 }
 
+const viaSourceLossNotSent = "VIA_SOURCE_LOSS_NOT_SENT: a user change that leaves a via grant without sources does not reach open streams"
+
 const selfRoutesReduced = "SELF_ROUTES_REDUCED: headscale sends a router only the primary routes its policy lets it reach"
 
 // TestRoutesMapCompat replays routesMapCompatTests through
@@ -286,6 +305,89 @@ func TestViaGrantMapCompat(t *testing.T) {
 	}
 }
 
+// TestViaGrantSourceLossCompat renames the only user a via grant names, so
+// the grant's source resolves to nothing, and checks that the open streams
+// of the routers and the user's node reach what SaaS sends for the same
+// grant with an empty source, without reconnecting.
+//
+// Test data source: ../policy/v2/testdata/grant_results/via-grant-v66.hujson
+// and via-grant-v67.hujson, shared with TestGrantsCompat and
+// TestViaGrantMapCompat.
+func TestViaGrantSourceLossCompat(t *testing.T) {
+	t.Parallel()
+
+	read := func(id string) *testcapture.Capture {
+		c, err := testcapture.Read(filepath.Join(
+			"..", "policy", "v2", "testdata", "grant_results", id+".hujson",
+		))
+		require.NoError(t, err, "failed to read %s", id)
+
+		return c
+	}
+
+	before, after := read("via-grant-v66"), read("via-grant-v67")
+
+	srv, clients := startViaMapCompat(t, before)
+	compareViaMapCompat(t, before, clients)
+
+	user, err := srv.State().GetUserByName("thor")
+	require.NoError(t, err)
+
+	_, userChange, err := srv.State().RenameUser(types.UserID(user.ID), "thor-renamed")
+	require.NoError(t, err)
+	srv.App.Change(userChange)
+
+	saasAddrs := saasAddrsByNode(after, clients)
+
+	for _, name := range []string{"squirtle", "blastoise", "ivysaur"} {
+		t.Run("rename/"+name, func(t *testing.T) {
+			t.Parallel()
+
+			key := after.TestID + "/rename/" + name + "/netmap"
+			want := after.Captures[name]
+
+			// Profiles are left out: the rename changes the user's login
+			// by construction.
+			check := func(tt require.TestingT) {
+				nm := clients[name].Netmap()
+				require.NotNil(tt, nm, "netmap is nil")
+
+				comparePeers(tt, nm, want, clients)
+				comparePacketFilter(tt, nm, want, clients, saasAddrs, key+"/packet-filter", viaCompatSkipReasons)
+			}
+
+			reason, listed := viaCompatSkipReasons[key]
+			if !listed {
+				requireNetmapHolds(t, check)
+
+				return
+			}
+
+			// Long enough for a change in flight to arrive.
+			assert.Never(t, func() bool { return passesOnce(check) }, 5*time.Second, 100*time.Millisecond,
+				"netmap matches SaaS; delete viaCompatSkipReasons[%q]", key)
+			t.Skip(reason)
+		})
+	}
+}
+
+// passesOnce runs check once and reports whether it passed. It runs in its
+// own goroutine because a failing require stops the goroutine it runs in.
+func passesOnce(check func(require.TestingT)) bool {
+	rec := &failRecorder{CollectT: new(assert.CollectT)}
+	done := make(chan struct{})
+
+	go func() {
+		defer close(done)
+
+		check(rec)
+	}()
+
+	<-done
+
+	return !rec.failed
+}
+
 func runViaMapCompat(t *testing.T, c *testcapture.Capture) {
 	t.Helper()