diff --git a/client/internal/routemanager/notifier/notifier_android.go b/client/internal/routemanager/notifier/notifier_android.go index 5fa329310..24cbb94db 100644 --- a/client/internal/routemanager/notifier/notifier_android.go +++ b/client/internal/routemanager/notifier/notifier_android.go @@ -4,8 +4,6 @@ package notifier import ( "net/netip" - "slices" - "sort" "sync" "github.com/netbirdio/netbird/client/internal/listener" @@ -75,19 +73,3 @@ func (n *Notifier) notifyLocked() { func (n *Notifier) Close() { // unused } - -func routesToStrings(routes []*route.Route) []string { - nets := make([]string, 0, len(routes)) - for _, r := range routes { - nets = append(nets, r.NetString()) - } - return nets -} - -func hasRouteDiff(a []*route.Route, b []*route.Route) bool { - as := routesToStrings(a) - bs := routesToStrings(b) - sort.Strings(as) - sort.Strings(bs) - return !slices.Equal(as, bs) -} diff --git a/client/internal/routemanager/notifier/route_diff.go b/client/internal/routemanager/notifier/route_diff.go new file mode 100644 index 000000000..52abddf36 --- /dev/null +++ b/client/internal/routemanager/notifier/route_diff.go @@ -0,0 +1,27 @@ +package notifier + +import ( + "slices" + "sort" + + "github.com/netbirdio/netbird/route" +) + +// routePrefixes returns the distinct prefixes a route set covers, sorted. +// Duplicates are dropped deliberately: an HA group hands us one route per +// peer serving the same prefix, and the platform is given the prefix, not the +// candidates. Counting them would report a change every time a peer joins or +// leaves a group, and on Android each report renews the TUN. +func routePrefixes(routes []*route.Route) []string { + nets := make([]string, 0, len(routes)) + for _, r := range routes { + nets = append(nets, r.NetString()) + } + sort.Strings(nets) + return slices.Compact(nets) +} + +// hasRouteDiff reports whether the prefixes the two route sets cover differ. +func hasRouteDiff(a []*route.Route, b []*route.Route) bool { + return !slices.Equal(routePrefixes(a), routePrefixes(b)) +} diff --git a/client/internal/routemanager/notifier/route_diff_test.go b/client/internal/routemanager/notifier/route_diff_test.go new file mode 100644 index 000000000..80df69d9d --- /dev/null +++ b/client/internal/routemanager/notifier/route_diff_test.go @@ -0,0 +1,88 @@ +package notifier + +import ( + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/route" +) + +func routeFor(id route.ID, prefix string) *route.Route { + return &route.Route{ + ID: id, + NetID: "net", + Network: netip.MustParsePrefix(prefix), + } +} + +// TestHasRouteDiff_IgnoresHACandidateCount is the reason the comparison +// deduplicates. Every notification renews the TUN, and a renewed TUN +// invalidates the sockets the embedded servers are listening on, so a peer +// joining or leaving an HA group must not count as a route change when the +// prefixes the TUN carries are identical. +func TestHasRouteDiff_IgnoresHACandidateCount(t *testing.T) { + onePeer := []*route.Route{routeFor("a", "10.0.0.0/24")} + twoPeers := []*route.Route{ + routeFor("a", "10.0.0.0/24"), + routeFor("b", "10.0.0.0/24"), + } + + assert.False(t, hasRouteDiff(onePeer, twoPeers), + "a second peer serving the same prefix is not a route change") + assert.False(t, hasRouteDiff(twoPeers, onePeer), + "losing one of two peers serving the same prefix is not a route change") +} + +func TestHasRouteDiff_ReportsRealChanges(t *testing.T) { + tests := []struct { + name string + a []*route.Route + b []*route.Route + want bool + }{ + { + name: "added prefix", + a: []*route.Route{routeFor("a", "10.0.0.0/24")}, + b: []*route.Route{routeFor("a", "10.0.0.0/24"), routeFor("b", "10.0.1.0/24")}, + want: true, + }, + { + name: "removed prefix", + a: []*route.Route{routeFor("a", "10.0.0.0/24"), routeFor("b", "10.0.1.0/24")}, + b: []*route.Route{routeFor("a", "10.0.0.0/24")}, + want: true, + }, + { + name: "replaced prefix", + a: []*route.Route{routeFor("a", "10.0.0.0/24")}, + b: []*route.Route{routeFor("a", "10.0.1.0/24")}, + want: true, + }, + { + name: "same prefix, different order", + a: []*route.Route{routeFor("a", "10.0.1.0/24"), routeFor("b", "10.0.0.0/24")}, + b: []*route.Route{routeFor("b", "10.0.0.0/24"), routeFor("a", "10.0.1.0/24")}, + want: false, + }, + { + name: "all routes gone", + a: []*route.Route{routeFor("a", "10.0.0.0/24")}, + b: nil, + want: true, + }, + { + name: "both empty", + a: nil, + b: nil, + want: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, hasRouteDiff(tc.a, tc.b), + "route diff for %s", tc.name) + }) + } +}