Skip to content

Commit

Permalink
fix(acl): update each peer's network when rule,group or peer changed (n…
Browse files Browse the repository at this point in the history 
…etbirdio#333)

* fix(acl): update each peer's network when rule,group or peer changed

* fix(ACL): update network test

* fix(acl): cleanup indexes before update them

* fix(acl): clean up rules indexes only for account
  • Loading branch information
@gigovich
gigovich authored Jun 4, 2022
1 parent 7cf588d commit 507f997
Show file tree
Hide file tree
Showing 6 changed files with 357 additions and 89 deletions.
185 changes: 184 additions & 1 deletion management/server/account_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package server

import (
"net"
"sync"
"testing"

"github.com/netbirdio/netbird/management/server/jwtclaims"
Expand Down Expand Up @@ -513,6 +514,189 @@ func TestAccountManager_AddPeerWithUserID(t *testing.T) {
}
}

func TestAccountManager_NetworkUpdates(t *testing.T) {
manager, err := createManager(t)
if err != nil {
t.Fatal(err)
return
}

account, err := manager.AddAccount("test_account", "account_creator", "")
if err != nil {
t.Fatal(err)
}

var setupKey *SetupKey
for _, key := range account.SetupKeys {
setupKey = key
if setupKey.Type == SetupKeyReusable {
break
}
}

if setupKey == nil {
t.Errorf("expecting account to have a default setup key")
return
}

if account.Network.Serial != 0 {
t.Errorf("expecting account network to have an initial Serial=0")
return
}

getPeer := func() *Peer {
key, err := wgtypes.GeneratePrivateKey()
if err != nil {
t.Fatal(err)
return nil
}
expectedPeerKey := key.PublicKey().String()

peer, err := manager.AddPeer(setupKey.Key, "", &Peer{
Key: expectedPeerKey,
Meta: PeerSystemMeta{},
Name: expectedPeerKey,
})
if err != nil {
t.Fatalf("expecting peer1 to be added, got failure %v", err)
return nil
}

return peer
}

peer1 := getPeer()
peer2 := getPeer()
peer3 := getPeer()

account, err = manager.GetAccountById(account.Id)
if err != nil {
t.Fatal(err)
return
}

updMsg := manager.peersUpdateManager.CreateChannel(peer1.Key)
defer manager.peersUpdateManager.CloseChannel(peer1.Key)

group := Group{
ID: "group-id",
Name: "GroupA",
Peers: []string{peer1.Key, peer2.Key, peer3.Key},
}

rule := Rule{
Source: []string{"group-id"},
Destination: []string{"group-id"},
Flow: TrafficFlowBidirect,
}

wg := sync.WaitGroup{}
t.Run("save group update", func(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()

message := <-updMsg
networkMap := message.Update.GetNetworkMap()
if len(networkMap.RemotePeers) != 2 {
t.Errorf("mismatch peers count: 2 expected, got %v", len(networkMap.RemotePeers))
}
}()

if err := manager.SaveGroup(account.Id, &group); err != nil {
t.Errorf("save group: %v", err)
return
}

wg.Wait()
})

t.Run("delete rule update", func(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()

message := <-updMsg
networkMap := message.Update.GetNetworkMap()
if len(networkMap.RemotePeers) != 0 {
t.Errorf("mismatch peers count: 0 expected, got %v", len(networkMap.RemotePeers))
}
}()

var defaultRule *Rule
for _, r := range account.Rules {
defaultRule = r
}

if err := manager.DeleteRule(account.Id, defaultRule.ID); err != nil {
t.Errorf("delete default rule: %v", err)
return
}

wg.Wait()
})

t.Run("save rule update", func(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()

message := <-updMsg
networkMap := message.Update.GetNetworkMap()
if len(networkMap.RemotePeers) != 2 {
t.Errorf("mismatch peers count: 2 expected, got %v", len(networkMap.RemotePeers))
}
}()

if err := manager.SaveRule(account.Id, &rule); err != nil {
t.Errorf("delete default rule: %v", err)
return
}

wg.Wait()
})

t.Run("delete peer update", func(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()

message := <-updMsg
networkMap := message.Update.GetNetworkMap()
if len(networkMap.RemotePeers) != 1 {
t.Errorf("mismatch peers count: 1 expected, got %v", len(networkMap.RemotePeers))
}
}()

if _, err := manager.DeletePeer(account.Id, peer3.Key); err != nil {
t.Errorf("delete peer: %v", err)
return
}

wg.Wait()
})

t.Run("delete group update", func(t *testing.T) {
wg.Add(1)
go func() {
defer wg.Done()

message := <-updMsg
networkMap := message.Update.GetNetworkMap()
if len(networkMap.RemotePeers) != 0 {
t.Errorf("mismatch peers count: 0 expected, got %v", len(networkMap.RemotePeers))
}
}()

if err := manager.DeleteGroup(account.Id, group.ID); err != nil {
t.Errorf("delete group rule: %v", err)
return
}

wg.Wait()
})
}

func TestAccountManager_DeletePeer(t *testing.T) {
manager, err := createManager(t)
if err != nil {
Expand Down Expand Up @@ -664,7 +848,6 @@ func TestAccountManager_UpdatePeerMeta(t *testing.T) {
}

assert.Equal(t, newMeta, p.Meta)

}

func createManager(t *testing.T) (*DefaultAccountManager, error) {
Expand Down
35 changes: 33 additions & 2 deletions management/server/file_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ func (s *FileStore) DeletePeer(accountId string, peerKey string) (*Peer, error)

delete(account.Peers, peerKey)
delete(s.PeerKeyId2AccountId, peerKey)
delete(s.PeerKeyId2DstRulesId, peerKey)
delete(s.PeerKeyId2SrcRulesId, peerKey)

// cleanup groups
var peers []string
Expand Down Expand Up @@ -240,9 +242,34 @@ func (s *FileStore) SaveAccount(account *Account) error {
s.PeerKeyId2AccountId[peer.Key] = account.Id
}

// remove all peers related to account from rules indexes
cleanIDs := make([]string, 0)
for key := range s.PeerKeyId2SrcRulesId {
if accountID, ok := s.PeerKeyId2AccountId[key]; ok && accountID == account.Id {
cleanIDs = append(cleanIDs, key)
}
}
for _, key := range cleanIDs {
delete(s.PeerKeyId2SrcRulesId, key)
}
cleanIDs = cleanIDs[:0]
for key := range s.PeerKeyId2DstRulesId {
if accountID, ok := s.PeerKeyId2AccountId[key]; ok && accountID == account.Id {
cleanIDs = append(cleanIDs, key)
}
}
for _, key := range cleanIDs {
delete(s.PeerKeyId2DstRulesId, key)
}

// rebuild rule indexes
for _, rule := range account.Rules {
for _, gid := range rule.Source {
for _, pid := range account.Groups[gid].Peers {
g, ok := account.Groups[gid]
if !ok {
break
}
for _, pid := range g.Peers {
rules := s.PeerKeyId2SrcRulesId[pid]
if rules == nil {
rules = map[string]struct{}{}
Expand All @@ -252,7 +279,11 @@ func (s *FileStore) SaveAccount(account *Account) error {
}
}
for _, gid := range rule.Destination {
for _, pid := range account.Groups[gid].Peers {
g, ok := account.Groups[gid]
if !ok {
break
}
for _, pid := range g.Peers {
rules := s.PeerKeyId2DstRulesId[pid]
if rules == nil {
rules = map[string]struct{}{}
Expand Down
29 changes: 24 additions & 5 deletions management/server/group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,13 @@ func (am *DefaultAccountManager) SaveGroup(accountID string, group *Group) error
}

account.Groups[group.ID] = group
return am.Store.SaveAccount(account)

account.Network.IncSerial()
if err = am.Store.SaveAccount(account); err != nil {
return err
}

return am.updateAccountPeers(account)
}

// DeleteGroup object of the peers
Expand All @@ -69,7 +75,12 @@ func (am *DefaultAccountManager) DeleteGroup(accountID, groupID string) error {

delete(account.Groups, groupID)

return am.Store.SaveAccount(account)
account.Network.IncSerial()
if err = am.Store.SaveAccount(account); err != nil {
return err
}

return am.updateAccountPeers(account)
}

// ListGroups objects of the peers
Expand Down Expand Up @@ -116,7 +127,12 @@ func (am *DefaultAccountManager) GroupAddPeer(accountID, groupID, peerKey string
group.Peers = append(group.Peers, peerKey)
}

return am.Store.SaveAccount(account)
account.Network.IncSerial()
if err = am.Store.SaveAccount(account); err != nil {
return err
}

return am.updateAccountPeers(account)
}

// GroupDeletePeer removes peer from the group
Expand All @@ -134,14 +150,17 @@ func (am *DefaultAccountManager) GroupDeletePeer(accountID, groupID, peerKey str
return status.Errorf(codes.NotFound, "group with ID %s not found", groupID)
}

account.Network.IncSerial()
for i, itemID := range group.Peers {
if itemID == peerKey {
group.Peers = append(group.Peers[:i], group.Peers[i+1:]...)
return am.Store.SaveAccount(account)
if err := am.Store.SaveAccount(account); err != nil {
return status.Errorf(codes.Internal, "can't save account")
}
}
}

return nil
return am.updateAccountPeers(account)
}

// GroupListPeers returns list of the peers from the group
Expand Down
Loading

0 comments on commit 507f997

Please sign in to comment.