Filter routes to sync from same HA group (#618)

An additional check and filter for routes that are part
 of the same HA group where the peer is a routing peer

client/internal/routemanager/client.go

@@ -53,10 +53,6 @@ func newClientNetworkWatcher(ctx context.Context, wgInterface *iface.WGIface, st
 	return client
 }
 
-func getHANetworkID(input *route.Route) string {
-	return input.NetID + "-" + input.Network.String()
-}
-
 func (c *clientNetwork) getRouterPeerStatuses() map[string]routerPeerStatus {
 	routePeerStatuses := make(map[string]routerPeerStatus)
 	for _, r := range c.routes {

client/internal/routemanager/manager.go

@@ -151,7 +151,7 @@ func (m *DefaultManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Ro
 		ownNetworkIDs := make(map[string]bool)
 
 		for _, newRoute := range newRoutes {
-			networkID := getHANetworkID(newRoute)
+			networkID := route.GetHAUniqueID(newRoute)
 			if newRoute.Peer == m.pubKey {
 				ownNetworkIDs[networkID] = true
 				// only linux is supported for now
@@ -164,7 +164,7 @@ func (m *DefaultManager) UpdateRoutes(updateSerial uint64, newRoutes []*route.Ro
 		}
 
 		for _, newRoute := range newRoutes {
-			networkID := getHANetworkID(newRoute)
+			networkID := route.GetHAUniqueID(newRoute)
 			if !ownNetworkIDs[networkID] {
 				// if prefix is too small, lets assume is a possible default route which is not yet supported
 				// we skip this route management

management/server/account.go

@@ -140,17 +140,35 @@ type UserInfo struct {
 // getRoutesToSync returns the enabled routes for the peer ID and the routes
 // from the ACL peers that have distribution groups associated with the peer ID
 func (a *Account) getRoutesToSync(peerID string, aclPeers []*Peer) []*route.Route {
-	routes := a.getEnabledRoutesByPeer(peerID)
+	routes, peerDisabledRoutes := a.getEnabledAndDisabledRoutesByPeer(peerID)
+	peerRoutesMembership := make(lookupMap)
+	for _, r := range append(routes, peerDisabledRoutes...) {
+		peerRoutesMembership[route.GetHAUniqueID(r)] = struct{}{}
+	}
+
 	groupListMap := a.getPeerGroups(peerID)
 	for _, peer := range aclPeers {
-		activeRoutes := a.getEnabledRoutesByPeer(peer.Key)
-		filteredRoutes := a.filterRoutesByGroups(activeRoutes, groupListMap)
+		activeRoutes, _ := a.getEnabledAndDisabledRoutesByPeer(peer.Key)
+		groupFilteredRoutes := a.filterRoutesByGroups(activeRoutes, groupListMap)
+		filteredRoutes := a.filterRoutesFromPeersOfSameHAGroup(groupFilteredRoutes, peerRoutesMembership)
 		routes = append(routes, filteredRoutes...)
 	}
 
 	return routes
 }
 
+// filterRoutesByHAMembership filters and returns a list of routes that don't share the same HA route membership
+func (a *Account) filterRoutesFromPeersOfSameHAGroup(routes []*route.Route, peerMemberships lookupMap) []*route.Route {
+	var filteredRoutes []*route.Route
+	for _, r := range routes {
+		_, found := peerMemberships[route.GetHAUniqueID(r)]
+		if !found {
+			filteredRoutes = append(filteredRoutes, r)
+		}
+	}
+	return filteredRoutes
+}
+
 // filterRoutesByGroups returns a list with routes that have distribution groups in the group's map
 func (a *Account) filterRoutesByGroups(routes []*route.Route, groupListMap lookupMap) []*route.Route {
 	var filteredRoutes []*route.Route
@@ -166,17 +184,21 @@ func (a *Account) filterRoutesByGroups(routes []*route.Route, groupListMap looku
 	return filteredRoutes
 }
 
-// getEnabledRoutesByPeer returns a list of routes of a given peer
-func (a *Account) getEnabledRoutesByPeer(peerPubKey string) []*route.Route {
+// getEnabledAndDisabledRoutesByPeer returns the enabled and disabled lists of routes that belong to a peer
+func (a *Account) getEnabledAndDisabledRoutesByPeer(peerPubKey string) ([]*route.Route, []*route.Route) {
 	//TODO Peer.ID migration: we will need to replace search by Peer.ID here
-	var routes []*route.Route
+	var enabledRoutes []*route.Route
+	var disabledRoutes []*route.Route
 	for _, r := range a.Routes {
-		if r.Peer == peerPubKey && r.Enabled {
-			routes = append(routes, r)
-			continue
+		if r.Peer == peerPubKey {
+			if r.Enabled {
+				enabledRoutes = append(enabledRoutes, r)
+				continue
+			}
+			disabledRoutes = append(disabledRoutes, r)
 		}
 	}
-	return routes
+	return enabledRoutes, disabledRoutes
 }
 
 // GetRoutesByPrefix return list of routes by account and route prefix

management/server/account_test.go

@@ -1080,6 +1080,10 @@ func TestAccount_GetRoutesToSync(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	_, prefix2, err := route.ParseNetwork("192.168.0.0/24")
+	if err != nil {
+		t.Fatal(err)
+	}
 	account := &Account{
 		Peers: map[string]*Peer{
 			"peer-1": {Key: "peer-1"}, "peer-2": {Key: "peer-2"}, "peer-3": {Key: "peer-1"},
@@ -1100,6 +1104,18 @@ func TestAccount_GetRoutesToSync(t *testing.T) {
 			},
 			"route-2": {
 				ID:          "route-2",
+				Network:     prefix2,
+				NetID:       "network-2",
+				Description: "network-2",
+				Peer:        "peer-2",
+				NetworkType: 0,
+				Masquerade:  false,
+				Metric:      999,
+				Enabled:     true,
+				Groups:      []string{"group1"},
+			},
+			"route-3": {
+				ID:          "route-3",
 				Network:     prefix,
 				NetID:       "network-1",
 				Description: "network-1",
@@ -1120,8 +1136,8 @@ func TestAccount_GetRoutesToSync(t *testing.T) {
 	for _, r := range routes {
 		routeIDs[r.ID] = struct{}{}
 	}
-	assert.Contains(t, routeIDs, "route-1")
 	assert.Contains(t, routeIDs, "route-2")
+	assert.Contains(t, routeIDs, "route-3")
 
 	emptyRoutes := account.getRoutesToSync("peer-3", []*Peer{{Key: "peer-1"}, {Key: "peer-2"}})
 

route/route.go

@@ -145,3 +145,8 @@ func compareGroupsList(list, other []string) bool {
 
 	return true
 }
+
+// GetHAUniqueID returns a highly available route ID by combining Network ID and Network range address
+func GetHAUniqueID(input *Route) string {
+	return input.NetID + "-" + input.Network.String()
+}