populate serving from primary routes (#2489)

* populate serving from primary routes

Depends on #2464
Fixes #2480

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* also exit

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* fix route update outside of connection

there was a bug where routes would not be updated if
they changed while a node was connected and it was not part of an
autoapprove.

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* update expected test output, cli only shows service node

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

---------

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

cmd/headscale/cli/nodes.go

@@ -729,7 +729,7 @@ func nodeRoutesToPtables(
 		"Hostname",
 		"Approved",
 		"Available",
-		"Serving",
+		"Serving (Primary)",
 	}
 	tableData := pterm.TableData{tableHeader}
 

hscontrol/grpcv1.go

@@ -27,6 +27,7 @@ import (
 	v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
 	"github.com/juanfont/headscale/hscontrol/db"
 	"github.com/juanfont/headscale/hscontrol/policy"
+	"github.com/juanfont/headscale/hscontrol/routes"
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/util"
 )
@@ -349,7 +350,7 @@ func (api headscaleV1APIServer) SetApprovedRoutes(
 		}
 	}
 	tsaddr.SortPrefixes(routes)
-	slices.Compact(routes)
+	routes = slices.Compact(routes)
 
 	node, err := db.Write(api.h.db.DB, func(tx *gorm.DB) (*types.Node, error) {
 		err := db.SetApprovedRoutes(tx, types.NodeID(request.GetNodeId()), routes)
@@ -371,7 +372,10 @@ func (api headscaleV1APIServer) SetApprovedRoutes(
 		api.h.nodeNotifier.NotifyWithIgnore(ctx, types.UpdatePeerChanged(node.ID), node.ID)
 	}
 
-	return &v1.SetApprovedRoutesResponse{Node: node.Proto()}, nil
+	proto := node.Proto()
+	proto.SubnetRoutes = util.PrefixesToString(api.h.primaryRoutes.PrimaryRoutes(node.ID))
+
+	return &v1.SetApprovedRoutesResponse{Node: proto}, nil
 }
 
 func validateTag(tag string) error {
@@ -497,7 +501,7 @@ func (api headscaleV1APIServer) ListNodes(
 			return nil, err
 		}
 
-		response := nodesToProto(api.h.polMan, isLikelyConnected, nodes)
+		response := nodesToProto(api.h.polMan, isLikelyConnected, api.h.primaryRoutes, nodes)
 		return &v1.ListNodesResponse{Nodes: response}, nil
 	}
 
@@ -510,11 +514,11 @@ func (api headscaleV1APIServer) ListNodes(
 		return nodes[i].ID < nodes[j].ID
 	})
 
-	response := nodesToProto(api.h.polMan, isLikelyConnected, nodes)
+	response := nodesToProto(api.h.polMan, isLikelyConnected, api.h.primaryRoutes, nodes)
 	return &v1.ListNodesResponse{Nodes: response}, nil
 }
 
-func nodesToProto(polMan policy.PolicyManager, isLikelyConnected *xsync.MapOf[types.NodeID, bool], nodes types.Nodes) []*v1.Node {
+func nodesToProto(polMan policy.PolicyManager, isLikelyConnected *xsync.MapOf[types.NodeID, bool], pr *routes.PrimaryRoutes, nodes types.Nodes) []*v1.Node {
 	response := make([]*v1.Node, len(nodes))
 	for index, node := range nodes {
 		resp := node.Proto()
@@ -532,6 +536,7 @@ func nodesToProto(polMan policy.PolicyManager, isLikelyConnected *xsync.MapOf[ty
 			}
 		}
 		resp.ValidTags = lo.Uniq(append(tags, node.ForcedTags...))
+		resp.SubnetRoutes = util.PrefixesToString(append(pr.PrimaryRoutes(node.ID), node.ExitRoutes()...))
 		response[index] = resp
 	}
 

hscontrol/poll.go

@@ -458,10 +458,14 @@ func (m *mapSession) handleEndpointUpdate() {
 		// TODO(kradalby): I am not sure if we need this?
 		nodesChangedHook(m.h.db, m.h.polMan, m.h.nodeNotifier)
 
-		// Approve routes if they are auto-approved by the policy.
+		// Approve any route that has been defined in policy as
-		// If any of them are approved, report them to the primary route tracker
+		// auto approved. Any change here is not important as any
-		// and send updates accordingly.
+		// actual state change will be detected when the route manager
-		if policy.AutoApproveRoutes(m.h.polMan, m.node) {
+		// is updated.
+		policy.AutoApproveRoutes(m.h.polMan, m.node)
+
+		// Update the routes of the given node in the route manager to
+		// see if an update needs to be sent.
 		if m.h.primaryRoutes.SetRoutes(m.node.ID, m.node.SubnetRoutes()...) {
 			ctx := types.NotifyCtx(m.ctx, "poll-primary-change", m.node.Hostname)
 			m.h.nodeNotifier.NotifyAll(ctx, types.UpdateFull())
@@ -481,8 +485,6 @@ func (m *mapSession) handleEndpointUpdate() {
 		}
 	}
 
-	}
-
 	// Check if there has been a change to Hostname and update them
 	// in the database. Then send a Changed update
 	// (containing the whole node object) to peers to inform about
@@ -506,8 +508,6 @@ func (m *mapSession) handleEndpointUpdate() {
 
 	m.w.WriteHeader(http.StatusOK)
 	mapResponseEndpointUpdates.WithLabelValues("ok").Inc()
-
-	return
 }
 
 func (m *mapSession) handleReadOnlyRequest() {
@@ -532,8 +532,6 @@ func (m *mapSession) handleReadOnlyRequest() {
 
 	m.w.WriteHeader(http.StatusOK)
 	mapResponseReadOnly.WithLabelValues("ok").Inc()
-
-	return
 }
 
 func logTracePeerChange(hostname string, hostinfoChange bool, change *tailcfg.PeerChange) {

hscontrol/types/node.go

@@ -247,13 +247,7 @@ func (node *Node) IPsAsString() []string {
 }
 
 func (node *Node) InIPSet(set *netipx.IPSet) bool {
-	for _, nodeAddr := range node.IPs() {
+	return slices.ContainsFunc(node.IPs(), set.Contains)
-		if set.Contains(nodeAddr) {
-			return true
-		}
-	}
-
-	return false
 }
 
 // AppendToIPSet adds the individual ips in NodeAddresses to a
@@ -334,9 +328,12 @@ func (node *Node) Proto() *v1.Node {
 		GivenName:   node.GivenName,
 		User:        node.User.Proto(),
 		ForcedTags:  node.ForcedTags,
+
+		// Only ApprovedRoutes and AvailableRoutes is set here. SubnetRoutes has
+		// to be populated manually with PrimaryRoute, to ensure it includes the
+		// routes that are actively served from the node.
 		ApprovedRoutes:  util.PrefixesToString(node.ApprovedRoutes),
 		AvailableRoutes: util.PrefixesToString(node.AnnouncedRoutes()),
-		SubnetRoutes:    util.PrefixesToString(node.SubnetRoutes()),
 
 		RegisterMethod: node.RegisterMethodToV1Enum(),
 

integration/route_test.go

@@ -375,7 +375,7 @@ func TestHASubnetRouterFailover(t *testing.T) {
 	assert.Len(t, nodes, 6)
 
 	assertNodeRouteCount(t, nodes[0], 1, 1, 1)
-	assertNodeRouteCount(t, nodes[1], 1, 1, 1)
+	assertNodeRouteCount(t, nodes[1], 1, 1, 0)
 	assertNodeRouteCount(t, nodes[2], 1, 0, 0)
 
 	// Verify that the client has routes from the primary machine
@@ -431,8 +431,8 @@ func TestHASubnetRouterFailover(t *testing.T) {
 	assert.Len(t, nodes, 6)
 
 	assertNodeRouteCount(t, nodes[0], 1, 1, 1)
-	assertNodeRouteCount(t, nodes[1], 1, 1, 1)
+	assertNodeRouteCount(t, nodes[1], 1, 1, 0)
-	assertNodeRouteCount(t, nodes[2], 1, 1, 1)
+	assertNodeRouteCount(t, nodes[2], 1, 1, 0)
 
 	// Verify that the client has routes from the primary machine
 	srs1 = subRouter1.MustStatus()
@@ -645,7 +645,7 @@ func TestHASubnetRouterFailover(t *testing.T) {
 	assert.Len(t, nodes, 6)
 
 	assertNodeRouteCount(t, nodes[0], 1, 1, 1)
-	assertNodeRouteCount(t, nodes[1], 1, 1, 1)
+	assertNodeRouteCount(t, nodes[1], 1, 1, 0)
 	assertNodeRouteCount(t, nodes[2], 1, 0, 0)
 
 	// Verify that the route is announced from subnet router 1
@@ -737,7 +737,7 @@ func TestHASubnetRouterFailover(t *testing.T) {
 	require.NoError(t, err)
 	assert.Len(t, nodes, 6)
 
-	assertNodeRouteCount(t, nodes[0], 1, 1, 1)
+	assertNodeRouteCount(t, nodes[0], 1, 1, 0)
 	assertNodeRouteCount(t, nodes[1], 1, 1, 1)
 	assertNodeRouteCount(t, nodes[2], 1, 0, 0)
 
@@ -838,7 +838,7 @@ func TestEnableDisableAutoApprovedRoute(t *testing.T) {
 	command = []string{
 		"tailscale",
 		"set",
-		"--advertise-routes=",
+		`--advertise-routes=`,
 	}
 	_, _, err = subRouter1.Execute(command)
 	require.NoErrorf(t, err, "failed to remove advertised route: %s", err)