Refactor Route IDs (#1891)

client/internal/routemanager/client.go

@@ -33,7 +33,7 @@ type clientNetwork struct {
 	stop                context.CancelFunc
 	statusRecorder      *peer.Status
 	wgInterface         *iface.WGIface
-	routes              map[string]*route.Route
+	routes              map[route.ID]*route.Route
 	routeUpdate         chan routesUpdate
 	peerStateUpdate     chan struct{}
 	routePeersNotifiers map[string]chan struct{}
@@ -50,7 +50,7 @@ func newClientNetworkWatcher(ctx context.Context, wgInterface *iface.WGIface, st
 		stop:                cancel,
 		statusRecorder:      statusRecorder,
 		wgInterface:         wgInterface,
-		routes:              make(map[string]*route.Route),
+		routes:              make(map[route.ID]*route.Route),
 		routePeersNotifiers: make(map[string]chan struct{}),
 		routeUpdate:         make(chan routesUpdate),
 		peerStateUpdate:     make(chan struct{}),
@@ -59,8 +59,8 @@ func newClientNetworkWatcher(ctx context.Context, wgInterface *iface.WGIface, st
 	return client
 }
 
-func (c *clientNetwork) getRouterPeerStatuses() map[string]routerPeerStatus {
-	routePeerStatuses := make(map[string]routerPeerStatus)
+func (c *clientNetwork) getRouterPeerStatuses() map[route.ID]routerPeerStatus {
+	routePeerStatuses := make(map[route.ID]routerPeerStatus)
 	for _, r := range c.routes {
 		peerStatus, err := c.statusRecorder.GetPeer(r.Peer)
 		if err != nil {
@@ -90,12 +90,12 @@ func (c *clientNetwork) getRouterPeerStatuses() map[string]routerPeerStatus {
 // * Latency: Routes with lower latency are prioritized.
 //
 // It returns the ID of the selected optimal route.
-func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[string]routerPeerStatus) string {
-	chosen := ""
+func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[route.ID]routerPeerStatus) route.ID {
+	chosen := route.ID("")
 	chosenScore := float64(0)
 	currScore := float64(0)
 
-	currID := ""
+	currID := route.ID("")
 	if c.chosenRoute != nil {
 		currID = c.chosenRoute.ID
 	}
@@ -295,7 +295,7 @@ func (c *clientNetwork) sendUpdateToClientNetworkWatcher(update routesUpdate) {
 }
 
 func (c *clientNetwork) handleUpdate(update routesUpdate) {
-	updateMap := make(map[string]*route.Route)
+	updateMap := make(map[route.ID]*route.Route)
 
 	for _, r := range update.routes {
 		updateMap[r.ID] = r

client/internal/routemanager/client_test.go

@@ -12,21 +12,21 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 
 	testCases := []struct {
 		name            string
-		statuses        map[string]routerPeerStatus
-		expectedRouteID string
-		currentRoute    string
-		existingRoutes  map[string]*route.Route
+		statuses        map[route.ID]routerPeerStatus
+		expectedRouteID route.ID
+		currentRoute    route.ID
+		existingRoutes  map[route.ID]*route.Route
 	}{
 		{
 			name: "one route",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					relayed:   false,
 					direct:    true,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -38,14 +38,14 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "one connected routes with relayed and direct",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					relayed:   true,
 					direct:    true,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -57,14 +57,14 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "one connected routes with relayed and no direct",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					relayed:   true,
 					direct:    false,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -76,14 +76,14 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "no connected peers",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: false,
 					relayed:   false,
 					direct:    false,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -95,7 +95,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "multiple connected peers with different metrics",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					relayed:   false,
@@ -107,7 +107,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 					direct:    true,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: 9000,
@@ -124,7 +124,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "multiple connected peers with one relayed",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					relayed:   false,
@@ -136,7 +136,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 					direct:    true,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -153,7 +153,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "multiple connected peers with one direct",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					relayed:   false,
@@ -165,7 +165,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 					direct:    false,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -182,7 +182,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "multiple connected peers with different latencies",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					latency:   300 * time.Millisecond,
@@ -192,7 +192,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 					latency:   10 * time.Millisecond,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -209,7 +209,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "should ignore routes with latency 0",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					latency:   0 * time.Millisecond,
@@ -219,7 +219,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 					latency:   10 * time.Millisecond,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -236,7 +236,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "current route with similar score and similar but slightly worse latency should not change",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					relayed:   false,
@@ -250,7 +250,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 					latency:   10 * time.Millisecond,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -267,7 +267,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "current route with bad score should be changed to route with better score",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					relayed:   false,
@@ -281,7 +281,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 					latency:   10 * time.Millisecond,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,
@@ -298,7 +298,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 		},
 		{
 			name: "current chosen route doesn't exist anymore",
-			statuses: map[string]routerPeerStatus{
+			statuses: map[route.ID]routerPeerStatus{
 				"route1": {
 					connected: true,
 					relayed:   false,
@@ -312,7 +312,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) {
 					latency:   10 * time.Millisecond,
 				},
 			},
-			existingRoutes: map[string]*route.Route{
+			existingRoutes: map[route.ID]*route.Route{
 				"route1": {
 					ID:     "route1",
 					Metric: route.MaxMetric,

client/internal/routemanager/manager.go

@@ -29,8 +29,8 @@ var defaultv6 = netip.PrefixFrom(netip.IPv6Unspecified(), 0)
 // Manager is a route manager interface
 type Manager interface {
 	Init() (peer.BeforeAddPeerHookFunc, peer.AfterRemovePeerHookFunc, error)
-	UpdateRoutes(updateSerial uint64, newRoutes []*route.Route) (map[string]*route.Route, map[string][]*route.Route, error)
-	TriggerSelection(map[string][]*route.Route)
+	UpdateRoutes(updateSerial uint64, newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap, error)
+	TriggerSelection(route.HAMap)
 	GetRouteSelector() *routeselector.RouteSelector
 	SetRouteChangeListener(listener listener.NetworkChangeListener)
 	InitialRouteRange() []string
@@ -43,7 +43,7 @@ type DefaultManager struct {
 	ctx            context.Context
 	stop           context.CancelFunc
 	mux            sync.Mutex
-	clientNetworks map[string]*clientNetwork
+	clientNetworks map[route.HAUniqueID]*clientNetwork
 	routeSelector  *routeselector.RouteSelector
 	serverRouter   serverRouter
 	statusRecorder *peer.Status
@@ -57,7 +57,7 @@ func NewManager(ctx context.Context, pubKey string, wgInterface *iface.WGIface,
 	dm := &DefaultManager{
 		ctx:            mCTX,
 		stop:           cancel,
-		clientNetworks: make(map[string]*clientNetwork),
+		clientNetworks: make(map[route.HAUniqueID]*clientNetwork),
 		routeSelector:  routeselector.NewRouteSelector(),
 		statusRecorder: statusRecorder,
 		wgInterface:    wgInterface,
@@ -122,7 +122,7 @@ func (m *DefaultManager) Stop() {
 }
 
 // UpdateRoutes compares received routes with existing routes and removes, updates or adds them to the client and server maps
-func (m *DefaultManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Route) (map[string]*route.Route, map[string][]*route.Route, error) {
+func (m *DefaultManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap, error) {
 	select {
 	case <-m.ctx.Done():
 		log.Infof("not updating routes as context is closed")
@@ -164,12 +164,12 @@ func (m *DefaultManager) GetRouteSelector() *routeselector.RouteSelector {
 }
 
 // GetClientRoutes returns the client routes
-func (m *DefaultManager) GetClientRoutes() map[string]*clientNetwork {
+func (m *DefaultManager) GetClientRoutes() map[route.HAUniqueID]*clientNetwork {
 	return m.clientNetworks
 }
 
 // TriggerSelection triggers the selection of routes, stopping deselected watchers and starting newly selected ones
-func (m *DefaultManager) TriggerSelection(networks map[string][]*route.Route) {
+func (m *DefaultManager) TriggerSelection(networks route.HAMap) {
 	m.mux.Lock()
 	defer m.mux.Unlock()
 
@@ -190,7 +190,7 @@ func (m *DefaultManager) TriggerSelection(networks map[string][]*route.Route) {
 }
 
 // stopObsoleteClients stops the client network watcher for the networks that are not in the new list
-func (m *DefaultManager) stopObsoleteClients(networks map[string][]*route.Route) {
+func (m *DefaultManager) stopObsoleteClients(networks route.HAMap) {
 	for id, client := range m.clientNetworks {
 		if _, ok := networks[id]; !ok {
 			log.Debugf("Stopping client network watcher, %s", id)
@@ -200,7 +200,7 @@ func (m *DefaultManager) stopObsoleteClients(networks map[string][]*route.Route)
 	}
 }
 
-func (m *DefaultManager) updateClientNetworks(updateSerial uint64, networks map[string][]*route.Route) {
+func (m *DefaultManager) updateClientNetworks(updateSerial uint64, networks route.HAMap) {
 	// removing routes that do not exist as per the update from the Management service.
 	m.stopObsoleteClients(networks)
 
@@ -219,15 +219,15 @@ func (m *DefaultManager) updateClientNetworks(updateSerial uint64, networks map[
 	}
 }
 
-func (m *DefaultManager) classifyRoutes(newRoutes []*route.Route) (map[string]*route.Route, map[string][]*route.Route) {
-	newClientRoutesIDMap := make(map[string][]*route.Route)
-	newServerRoutesMap := make(map[string]*route.Route)
-	ownNetworkIDs := make(map[string]bool)
+func (m *DefaultManager) classifyRoutes(newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap) {
+	newClientRoutesIDMap := make(route.HAMap)
+	newServerRoutesMap := make(map[route.ID]*route.Route)
+	ownNetworkIDs := make(map[route.HAUniqueID]bool)
 
 	for _, newRoute := range newRoutes {
-		networkID := route.GetHAUniqueID(newRoute)
+		haID := route.GetHAUniqueID(newRoute)
 		if newRoute.Peer == m.pubKey {
-			ownNetworkIDs[networkID] = true
+			ownNetworkIDs[haID] = true
 			// only linux is supported for now
 			if runtime.GOOS != "linux" {
 				log.Warnf("received a route to manage, but agent doesn't support router mode on %s OS", runtime.GOOS)
@@ -238,12 +238,12 @@ func (m *DefaultManager) classifyRoutes(newRoutes []*route.Route) (map[string]*r
 	}
 
 	for _, newRoute := range newRoutes {
-		networkID := route.GetHAUniqueID(newRoute)
-		if !ownNetworkIDs[networkID] {
+		haID := route.GetHAUniqueID(newRoute)
+		if !ownNetworkIDs[haID] {
 			if !isPrefixSupported(newRoute.Network) {
 				continue
 			}
-			newClientRoutesIDMap[networkID] = append(newClientRoutesIDMap[networkID], newRoute)
+			newClientRoutesIDMap[haID] = append(newClientRoutesIDMap[haID], newRoute)
 		}
 	}
 

client/internal/routemanager/mock.go

@@ -14,8 +14,8 @@ import (
 
 // MockManager is the mock instance of a route manager
 type MockManager struct {
-	UpdateRoutesFunc     func(updateSerial uint64, newRoutes []*route.Route) (map[string]*route.Route, map[string][]*route.Route, error)
-	TriggerSelectionFunc func(map[string][]*route.Route)
+	UpdateRoutesFunc     func(updateSerial uint64, newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap, error)
+	TriggerSelectionFunc func(haMap route.HAMap)
 	GetRouteSelectorFunc func() *routeselector.RouteSelector
 	StopFunc             func()
 }
@@ -30,14 +30,14 @@ func (m *MockManager) InitialRouteRange() []string {
 }
 
 // UpdateRoutes mock implementation of UpdateRoutes from Manager interface
-func (m *MockManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Route) (map[string]*route.Route, map[string][]*route.Route, error) {
+func (m *MockManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap, error) {
 	if m.UpdateRoutesFunc != nil {
 		return m.UpdateRoutesFunc(updateSerial, newRoutes)
 	}
 	return nil, nil, fmt.Errorf("method UpdateRoutes is not implemented")
 }
 
-func (m *MockManager) TriggerSelection(networks map[string][]*route.Route) {
+func (m *MockManager) TriggerSelection(networks route.HAMap) {
 	if m.TriggerSelectionFunc != nil {
 		m.TriggerSelectionFunc(networks)
 	}

client/internal/routemanager/notifier.go

@@ -36,7 +36,7 @@ func (n *notifier) setInitialClientRoutes(clientRoutes []*route.Route) {
 	n.initialRouteRangers = nets
 }
 
-func (n *notifier) onNewRoutes(idMap map[string][]*route.Route) {
+func (n *notifier) onNewRoutes(idMap route.HAMap) {
 	newNets := make([]string, 0)
 	for _, routes := range idMap {
 		for _, r := range routes {

client/internal/routemanager/server.go

@@ -3,7 +3,7 @@ package routemanager
 import "github.com/netbirdio/netbird/route"
 
 type serverRouter interface {
-	updateRoutes(map[string]*route.Route) error
+	updateRoutes(map[route.ID]*route.Route) error
 	removeFromServerNetwork(*route.Route) error
 	cleanUp()
 }

client/internal/routemanager/server_nonandroid.go

@@ -19,7 +19,7 @@ import (
 type defaultServerRouter struct {
 	mux            sync.Mutex
 	ctx            context.Context
-	routes         map[string]*route.Route
+	routes         map[route.ID]*route.Route
 	firewall       firewall.Manager
 	wgInterface    *iface.WGIface
 	statusRecorder *peer.Status
@@ -28,15 +28,15 @@ type defaultServerRouter struct {
 func newServerRouter(ctx context.Context, wgInterface *iface.WGIface, firewall firewall.Manager, statusRecorder *peer.Status) (serverRouter, error) {
 	return &defaultServerRouter{
 		ctx:            ctx,
-		routes:         make(map[string]*route.Route),
+		routes:         make(map[route.ID]*route.Route),
 		firewall:       firewall,
 		wgInterface:    wgInterface,
 		statusRecorder: statusRecorder,
 	}, nil
 }
 
-func (m *defaultServerRouter) updateRoutes(routesMap map[string]*route.Route) error {
-	serverRoutesToRemove := make([]string, 0)
+func (m *defaultServerRouter) updateRoutes(routesMap map[route.ID]*route.Route) error {
+	serverRoutesToRemove := make([]route.ID, 0)
 
 	for routeID := range m.routes {
 		update, found := routesMap[routeID]
@@ -168,7 +168,7 @@ func routeToRouterPair(source string, route *route.Route) (firewall.RouterPair,
 		return firewall.RouterPair{}, err
 	}
 	return firewall.RouterPair{
-		ID:          route.ID,
+		ID:          string(route.ID),
 		Source:      parsed.String(),
 		Destination: route.Network.Masked().String(),
 		Masquerade:  route.Masquerade,