From 0a416198cb02bd71483818d3876218bd7569c35f Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Mon, 18 Sep 2023 13:22:30 +0200 Subject: [PATCH] Ignore non Linux peers in the network map update --- management/server/account.go | 73 +++++++++++++----------- management/server/http/routes_handler.go | 2 - management/server/route_test.go | 33 ++++++++++- 3 files changed, 73 insertions(+), 35 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 883cbe012d1..02e1a0dcfb2 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -253,6 +253,21 @@ func (a *Account) filterRoutesByGroups(routes []*route.Route, groupListMap looku func (a *Account) getEnabledAndDisabledRoutesByPeer(peerID string) ([]*route.Route, []*route.Route) { var enabledRoutes []*route.Route var disabledRoutes []*route.Route + + takeRoute := func(r *route.Route, id string) { + peer := a.GetPeer(peerID) + if peer == nil { + log.Errorf("route %s has peer %s that doesn't exist under account %s", r.ID, peerID, a.Id) + return + } + + if r.Enabled { + enabledRoutes = append(enabledRoutes, r) + return + } + disabledRoutes = append(disabledRoutes, r) + } + for _, r := range a.Routes { if r.PeersGroup != "" { group := a.GetGroup(r.PeersGroup) @@ -261,36 +276,14 @@ func (a *Account) getEnabledAndDisabledRoutesByPeer(peerID string) ([]*route.Rou continue } for _, id := range group.Peers { - peer := a.GetPeer(id) - if peer == nil { - log.Errorf("route %s has peers group %s which has %s that doesn't exist under account %s", r.ID, r.PeersGroup, id, a.Id) - continue + if id == peerID { + takeRoute(r, id) + break } - rCopy := r.Copy() - rCopy.Peer = peer.Key - if r.Enabled { - enabledRoutes = append(enabledRoutes, rCopy) - continue - } - disabledRoutes = append(disabledRoutes, rCopy) - continue } } if r.Peer == peerID { - // We need to set Peer.Key instead of Peer.ID because this object will be sent to agents as part of a network map. - // Ideally we should have a separate field for that, but fine for now. - peer := a.GetPeer(peerID) - if peer == nil { - log.Errorf("route %s has peer %s that doesn't exist under account %s", r.ID, peerID, a.Id) - continue - } - raut := r.Copy() - raut.Peer = peer.Key - if r.Enabled { - enabledRoutes = append(enabledRoutes, raut) - continue - } - disabledRoutes = append(disabledRoutes, raut) + takeRoute(r, peerID) } } return enabledRoutes, disabledRoutes @@ -341,20 +334,36 @@ func (a *Account) GetPeerNetworkMap(peerID, dnsDomain string) *NetworkMap { // Please mind, that the returned route.Route objects will contain Peer.Key instead of Peer.ID. routes := a.getRoutesToSync(peerID, peersToConnect) - // TODO(yury): each route can contain peers group. We should unfold them to peers + takePeer := func(id string) (*Peer, bool) { + peer := a.GetPeer(id) + if peer == nil || peer.Meta.GoOS != "linux" { + return nil, false + } + return peer, true + } + + // We need to set Peer.Key instead of Peer.ID because this object will be sent to agents as part of a network map. + // Ideally we should have a separate field for that, but fine for now. var routesUpdate []*route.Route - seenPeer := make(map[string]bool) for _, r := range routes { - if r.PeersGroup == "" { - routesUpdate = append(routesUpdate, r) + if r.Peer != "" { + peer, valid := takePeer(r.Peer) + if !valid { + continue + } + rCopy := r.Copy() + rCopy.Peer = peer.Key + routesUpdate = append(routesUpdate, rCopy) continue } + seenPeer := make(map[string]bool) if group := a.GetGroup(r.PeersGroup); group != nil { for _, peerId := range group.Peers { - peer := a.GetPeer(peerId) // BROKEN!!! - if peer == nil { + peer, valid := takePeer(peerId) + if !valid { continue } + if _, ok := seenPeer[peer.Key]; !ok { rCopy := r.Copy() rCopy.Peer = peer.Key diff --git a/management/server/http/routes_handler.go b/management/server/http/routes_handler.go index b6be76fd58a..50cfc27f600 100644 --- a/management/server/http/routes_handler.go +++ b/management/server/http/routes_handler.go @@ -97,8 +97,6 @@ func (h *RoutesHandler) CreateRoute(w http.ResponseWriter, r *http.Request) { return } - // TODO(yury): check that peers are Linux - newRoute, err := h.accountManager.CreateRoute( account.Id, newPrefix.String(), peerId, peersGroupId, req.Description, req.NetworkId, req.Masquerade, req.Metric, req.Groups, req.Enabled, user.Id, diff --git a/management/server/route_test.go b/management/server/route_test.go index e49f7090a1c..6fb603578d4 100644 --- a/management/server/route_test.go +++ b/management/server/route_test.go @@ -14,8 +14,10 @@ import ( const ( peer1Key = "BhRPtynAAYRDy08+q4HTMsos8fs4plTP4NOSh7C1ry8=" peer2Key = "/yF0+vCfv+mRR5k0dca0TrGdO/oiNeAI58gToZm5NyI=" + peer3Key = "ayF0+vCfv+mRR5k0dca0TrGdO/oiNeAI58gToZm5NaF=" peer1ID = "peer-1-id" peer2ID = "peer-2-id" + peer3ID = "peer-3-id" routeGroup1 = "routeGroup1" routeGroup2 = "routeGroup2" routeGroupHA = "routeGroupHA" @@ -1188,6 +1190,31 @@ func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, er } account.Peers[peer2.ID] = peer2 + ips = account.getTakenIPs() + peer3IP, err := AllocatePeerIP(account.Network.Net, ips) + if err != nil { + return nil, err + } + + peer3 := &Peer{ + IP: peer3IP, + ID: peer3ID, + Key: peer3Key, + Name: "test-host3@netbird.io", + UserID: userID, + Meta: PeerSystemMeta{ + Hostname: "test-host3@netbird.io", + GoOS: "darwin", + Kernel: "Darwin", + Core: "13.4.1", + Platform: "arm64", + OS: "darwin", + WtVersion: "development", + UIVersion: "development", + }, + } + account.Peers[peer3.ID] = peer3 + err = am.Store.SaveAccount(account) if err != nil { return nil, err @@ -1204,6 +1231,10 @@ func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, er if err != nil { return nil, err } + err = am.GroupAddPeer(accountID, groupAll.ID, peer3ID) + if err != nil { + return nil, err + } newGroup := []*Group{ { @@ -1219,7 +1250,7 @@ func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, er { ID: routeGroupHA, Name: routeGroupHA, - Peers: []string{peer1.ID, peer2.ID}, + Peers: []string{peer1.ID, peer2.ID, peer3.ID}, // we have one non Linux peer, see peer3 }, }