[client] UI refactor (#6069)

Refactor UI --------- Co-authored-by: Eduard Gert <kontakt@eduardgert.de> Co-authored-by: braginini <bangvalo@gmail.com> Co-authored-by: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: riccardom <riccardomanfrin@gmail.com>
2026-09-21 22:29:08 +02:00 · 2026-06-19 09:59:28 +02:00
parent 679c7182a4
commit 8b7ce337d8
394 changed files with 46607 additions and 6687 deletions
@@ -0,0 +1,191 @@
+package routemanager
+
+import (
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/client/internal/routeselector"
+	"github.com/netbirdio/netbird/route"
+)
+
+func newExitNodeTestManager() *DefaultManager {
+	return &DefaultManager{routeSelector: routeselector.NewRouteSelector()}
+}
+
+func exitRoute(netID, peer string, skipAutoApply bool) *route.Route {
+	return &route.Route{
+		NetID:         route.NetID(netID),
+		Network:       netip.MustParsePrefix("0.0.0.0/0"),
+		Peer:          peer,
+		SkipAutoApply: skipAutoApply,
+	}
+}
+
+func TestPickPreferredExitNode(t *testing.T) {
+	tests := []struct {
+		name string
+		info exitNodeInfo
+		want route.NetID
+	}{
+		{
+			name: "persisted user selection wins over management",
+			info: exitNodeInfo{
+				allIDs:               []route.NetID{"a", "b", "c"},
+				userSelected:         []route.NetID{"b"},
+				selectedByManagement: []route.NetID{"a"},
+			},
+			want: "b",
+		},
+		{
+			name: "multiple user-selected self-heal to deterministic min",
+			info: exitNodeInfo{
+				allIDs:       []route.NetID{"a", "b", "c"},
+				userSelected: []route.NetID{"c", "a"},
+			},
+			want: "a",
+		},
+		{
+			name: "explicit opt-out keeps none",
+			info: exitNodeInfo{
+				allIDs:         []route.NetID{"a", "b"},
+				userDeselected: []route.NetID{"a", "b"},
+			},
+			want: "",
+		},
+		{
+			name: "fresh defaults to management auto-apply pick",
+			info: exitNodeInfo{
+				allIDs:               []route.NetID{"a", "b", "c"},
+				selectedByManagement: []route.NetID{"b"},
+			},
+			want: "b",
+		},
+		{
+			name: "no user pick and no management auto-apply selects none",
+			info: exitNodeInfo{
+				allIDs: []route.NetID{"c", "a", "b"},
+			},
+			want: "",
+		},
+		{
+			name: "user-deselect does not block a management auto-apply sibling",
+			info: exitNodeInfo{
+				allIDs:               []route.NetID{"a", "b"},
+				userDeselected:       []route.NetID{"a"},
+				selectedByManagement: []route.NetID{"b"},
+			},
+			want: "b",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, pickPreferredExitNode(tt.info), "preferred exit node")
+		})
+	}
+}
+
+func TestEnforceSingleExitNode(t *testing.T) {
+	m := newExitNodeTestManager()
+	all := []route.NetID{"a", "b", "c"}
+
+	m.enforceSingleExitNode("b", all)
+	assert.False(t, m.routeSelector.IsSelected("a"), "a should be deselected")
+	assert.True(t, m.routeSelector.IsSelected("b"), "b should be the only selected exit node")
+	assert.False(t, m.routeSelector.IsSelected("c"), "c should be deselected")
+
+	// Switching the preferred node moves the single selection.
+	m.enforceSingleExitNode("c", all)
+	assert.False(t, m.routeSelector.IsSelected("a"), "a stays deselected")
+	assert.False(t, m.routeSelector.IsSelected("b"), "b should now be deselected")
+	assert.True(t, m.routeSelector.IsSelected("c"), "c should now be selected")
+
+	// Empty preferred turns every exit node off.
+	m.enforceSingleExitNode("", all)
+	for _, id := range all {
+		assert.False(t, m.routeSelector.IsSelected(id), "no exit node should be selected")
+	}
+}
+
+func TestEnforceSingleExitNode_RespectsDeselectAll(t *testing.T) {
+	m := newExitNodeTestManager()
+	m.routeSelector.DeselectAllRoutes()
+
+	m.enforceSingleExitNode("b", []route.NetID{"a", "b"})
+
+	assert.True(t, m.routeSelector.IsDeselectAll(), "global deselect-all must stay in effect")
+	assert.False(t, m.routeSelector.IsSelected("b"), "no exit node should be forced on while deselect-all is set")
+}
+
+func TestUpdateRouteSelectorFromManagement_FreshSelectsOne(t *testing.T) {
+	m := newExitNodeTestManager()
+	routes := route.HAMap{
+		"exitA|0.0.0.0/0":    {exitRoute("exitA", "p1", false)},
+		"exitB|0.0.0.0/0":    {exitRoute("exitB", "p2", false)},
+		"lan|192.168.1.0/24": {{NetID: "lan", Network: netip.MustParsePrefix("192.168.1.0/24"), Peer: "p3"}},
+		"exitC|0.0.0.0/0":    {exitRoute("exitC", "p4", false)},
+	}
+
+	m.updateRouteSelectorFromManagement(routes)
+
+	// Exactly one exit node (the deterministic first) is selected.
+	assert.True(t, m.routeSelector.IsSelected("exitA"), "exitA is the deterministic default")
+	assert.False(t, m.routeSelector.IsSelected("exitB"), "exitB must not also be selected")
+	assert.False(t, m.routeSelector.IsSelected("exitC"), "exitC must not also be selected")
+	// Non-exit routes are left at their default-on state.
+	assert.True(t, m.routeSelector.IsSelected("lan"), "non-exit route selection is untouched")
+}
+
+func TestUpdateRouteSelectorFromManagement_HonorsPersistedPick(t *testing.T) {
+	m := newExitNodeTestManager()
+	routes := route.HAMap{
+		"exitA|0.0.0.0/0": {exitRoute("exitA", "p1", false)},
+		"exitB|0.0.0.0/0": {exitRoute("exitB", "p2", false)},
+	}
+	all := []route.NetID{"exitA", "exitB"}
+
+	// Simulate the state the runtime select path leaves behind: exactly one
+	// exit node explicitly selected, its sibling deselected.
+	require.NoError(t, m.routeSelector.SelectRoutes([]route.NetID{"exitB"}, true, all))
+	require.NoError(t, m.routeSelector.DeselectRoutes([]route.NetID{"exitA"}, all))
+
+	m.updateRouteSelectorFromManagement(routes)
+
+	assert.True(t, m.routeSelector.IsSelected("exitB"), "persisted pick must stay selected")
+	assert.False(t, m.routeSelector.IsSelected("exitA"), "the other exit node stays deselected")
+}
+
+func TestUpdateRouteSelectorFromManagement_OptOutKeepsNone(t *testing.T) {
+	m := newExitNodeTestManager()
+	routes := route.HAMap{
+		"exitA|0.0.0.0/0": {exitRoute("exitA", "p1", false)},
+		"exitB|0.0.0.0/0": {exitRoute("exitB", "p2", false)},
+	}
+	all := []route.NetID{"exitA", "exitB"}
+
+	// User deselected exit nodes and selected none.
+	require.NoError(t, m.routeSelector.DeselectRoutes(all, all))
+
+	m.updateRouteSelectorFromManagement(routes)
+
+	assert.False(t, m.routeSelector.IsSelected("exitA"), "opt-out keeps exitA off")
+	assert.False(t, m.routeSelector.IsSelected("exitB"), "opt-out keeps exitB off")
+}
+
+func TestUpdateRouteSelectorFromManagement_NoAutoApplySelectsNone(t *testing.T) {
+	m := newExitNodeTestManager()
+	// SkipAutoApply=true: management offers the exit nodes but doesn't request
+	// auto-activation, so none should be selected until the user picks one.
+	routes := route.HAMap{
+		"exitA|0.0.0.0/0": {exitRoute("exitA", "p1", true)},
+		"exitB|0.0.0.0/0": {exitRoute("exitB", "p2", true)},
+	}
+
+	m.updateRouteSelectorFromManagement(routes)
+
+	assert.False(t, m.routeSelector.IsSelected("exitA"), "no auto-apply keeps exitA off")
+	assert.False(t, m.routeSelector.IsSelected("exitB"), "no auto-apply keeps exitB off")
+}
@@ -442,6 +442,11 @@ func (m *DefaultManager) UpdateRoutes(

 		m.updateClientNetworks(updateSerial, filteredClientRoutes)
 		m.notifier.OnNewRoutes(filteredClientRoutes)
+		// A new network map can add or drop route/exit-node candidates without
+		// touching any peer's chosen-route state, so the peer status alone
+		// wouldn't notify SubscribeStatus subscribers. Bump the revision so the
+		// UI re-fetches ListNetworks.
+		m.statusRecorder.BumpNetworksRevision()
 	}
 	m.clientRoutes = clientRoutes

@@ -582,6 +587,10 @@ func (m *DefaultManager) TriggerSelection(networks route.HAMap) {
 	if err := m.stateManager.UpdateState((*SelectorState)(m.routeSelector)); err != nil {
 		log.Errorf("failed to update state: %v", err)
 	}
+
+	// A selection change flips Network.selected without altering the candidate
+	// set, so bump the revision to push the new state to the UI.
+	m.statusRecorder.BumpNetworksRevision()
 }

 // stopObsoleteClients stops the client network watcher for the networks that are not in the new list
@@ -701,7 +710,13 @@ func resolveURLsToIPs(urls []string) []net.IP {
 	return ips
 }

-// updateRouteSelectorFromManagement updates the route selector based on the isSelected status from the management server
+// updateRouteSelectorFromManagement reconciles exit-node selection on every
+// network map: it keeps at most one exit node selected — the user's persisted
+// pick, else whatever management marks for auto-apply (SkipAutoApply=false),
+// else none. We never auto-activate an exit node the map doesn't request; it
+// stays off until the user picks it. Exit nodes are mutually exclusive, but the
+// RouteSelector stores routes with default-on semantics, so without this every
+// available exit node would report selected at once.
 func (m *DefaultManager) updateRouteSelectorFromManagement(clientRoutes route.HAMap) {
 	m.mirrorV6ExitPairSelections(clientRoutes)

@@ -712,13 +727,14 @@ func (m *DefaultManager) updateRouteSelectorFromManagement(clientRoutes route.HA
 		return
 	}

-	exitNodeInfo := m.collectExitNodeInfo(clientRoutes)
-	if len(exitNodeInfo.allIDs) == 0 {
+	info := m.collectExitNodeInfo(clientRoutes)
+	if len(info.allIDs) == 0 {
 		return
 	}

-	m.updateExitNodeSelections(exitNodeInfo)
-	m.logExitNodeUpdate(exitNodeInfo)
+	preferred := pickPreferredExitNode(info)
+	m.enforceSingleExitNode(preferred, info.allIDs)
+	m.logExitNodeUpdate(info, preferred)
 }

 // mirrorV6ExitPairSelections keeps every synthesized "-v6" exit route's selection
@@ -746,6 +762,10 @@ type exitNodeInfo struct {
 	userDeselected       []route.NetID
 }

+// collectExitNodeInfo categorises the available exit nodes by their persisted
+// selection state. It keys on the base (v4) NetID and skips the synthesized
+// "-v6" partner, which inherits its base's selection through the RouteSelector
+// — counting it separately would double-count the pair.
 func (m *DefaultManager) collectExitNodeInfo(clientRoutes route.HAMap) exitNodeInfo {
 	var info exitNodeInfo

@@ -755,6 +775,9 @@ func (m *DefaultManager) collectExitNodeInfo(clientRoutes route.HAMap) exitNodeI
 		}

 		netID := haID.NetID()
+		if strings.HasSuffix(string(netID), route.V6ExitSuffix) {
+			continue
+		}
 		info.allIDs = append(info.allIDs, netID)

 		if m.routeSelector.HasUserSelectionForRoute(netID) {
@@ -791,45 +814,69 @@ func (m *DefaultManager) checkManagementSelection(routes []*route.Route, netID r
 	}
 }

-func (m *DefaultManager) updateExitNodeSelections(info exitNodeInfo) {
-	routesToDeselect := m.getRoutesToDeselect(info.allIDs)
-	m.deselectExitNodes(routesToDeselect)
-	m.selectExitNodesByManagement(info.selectedByManagement, info.allIDs)
+// pickPreferredExitNode chooses the single exit node to keep selected. In order:
+//   - a persisted user selection wins (deterministic if several survive from
+//     legacy state, so the set self-heals down to one);
+//   - otherwise activate only what management marks for auto-apply
+//     (SkipAutoApply=false); the lexicographically first if it marks several.
+//
+// Returns "" when neither holds — we never force an arbitrary exit node on. A
+// route the map doesn't auto-apply stays off until the user selects it.
+// info.userDeselected is informational only: an explicit deselect simply keeps
+// that route out of both lists above, so it can't be picked.
+func pickPreferredExitNode(info exitNodeInfo) route.NetID {
+	if len(info.userSelected) > 0 {
+		return minNetID(info.userSelected)
+	}
+	if len(info.selectedByManagement) > 0 {
+		return minNetID(info.selectedByManagement)
+	}
+	return ""
 }

-func (m *DefaultManager) getRoutesToDeselect(allIDs []route.NetID) []route.NetID {
-	var routesToDeselect []route.NetID
-	for _, netID := range allIDs {
-		if !m.routeSelector.HasUserSelectionForRoute(netID) {
-			routesToDeselect = append(routesToDeselect, netID)
+// enforceSingleExitNode makes preferred the only selected exit node: every other
+// available exit node is deselected and preferred (if any) is selected, without
+// disturbing non-exit route selections. A global deselect-all is left untouched
+// so the user's "all off" stays in effect.
+func (m *DefaultManager) enforceSingleExitNode(preferred route.NetID, allIDs []route.NetID) {
+	if m.routeSelector.IsDeselectAll() {
+		return
+	}
+
+	others := make([]route.NetID, 0, len(allIDs))
+	for _, id := range allIDs {
+		if id != preferred {
+			others = append(others, id)
 		}
 	}
-	return routesToDeselect
-}
-
-func (m *DefaultManager) deselectExitNodes(routesToDeselect []route.NetID) {
-	if len(routesToDeselect) == 0 {
-		return
+	if len(others) > 0 {
+		if err := m.routeSelector.DeselectRoutes(others, allIDs); err != nil {
+			log.Warnf("deselect other exit nodes: %v", err)
+		}
 	}
-
-	err := m.routeSelector.DeselectRoutes(routesToDeselect, routesToDeselect)
-	if err != nil {
-		log.Warnf("Failed to deselect exit nodes: %v", err)
+	if preferred != "" {
+		if err := m.routeSelector.SelectRoutes([]route.NetID{preferred}, true, allIDs); err != nil {
+			log.Warnf("select preferred exit node %q: %v", preferred, err)
+		}
 	}
 }

-func (m *DefaultManager) selectExitNodesByManagement(selectedByManagement []route.NetID, allIDs []route.NetID) {
-	if len(selectedByManagement) == 0 {
-		return
-	}
-
-	err := m.routeSelector.SelectRoutes(selectedByManagement, true, allIDs)
-	if err != nil {
-		log.Warnf("Failed to select exit nodes: %v", err)
-	}
+func (m *DefaultManager) logExitNodeUpdate(info exitNodeInfo, preferred route.NetID) {
+	log.Debugf("Exit node selection: %d available, preferred=%q (%d user-selected, %d user-deselected, %d management-selected)",
+		len(info.allIDs), preferred, len(info.userSelected), len(info.userDeselected), len(info.selectedByManagement))
 }

-func (m *DefaultManager) logExitNodeUpdate(info exitNodeInfo) {
-	log.Debugf("Updated route selector: %d exit nodes available, %d selected by management, %d user-selected, %d user-deselected",
-		len(info.allIDs), len(info.selectedByManagement), len(info.userSelected), len(info.userDeselected))
+// minNetID returns the lexicographically smallest NetID, for a deterministic
+// default pick that stays stable across restarts.
+func minNetID(ids []route.NetID) route.NetID {
+	if len(ids) == 0 {
+		return ""
+	}
+	best := ids[0]
+	for _, id := range ids[1:] {
+		if id < best {
+			best = id
+		}
+	}
+	return best
 }