[management] Optimize network map updates (#2718)

* Skip peer update on unchanged network map (#2236) * Enhance network updates by skipping unchanged messages Optimizes the network update process by skipping updates where no changes in the peer update message received. * Add unit tests * add locks * Improve concurrency and update peer message handling * Refactor account manager network update tests * fix test * Fix inverted network map update condition * Add default group and policy to test data * Run peer updates in a separate goroutine * Refactor * Refactor lock * Fix peers update by including NetworkMap and posture Checks * go mod tidy * fix merge Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix merge Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * [management] Skip account peers update if no changes affect peers (#2310) * Remove incrementing network serial and updating peers after group deletion * Update account peer if posture check is linked to policy * Remove account peers update on saving setup key * Refactor group link checking into re-usable functions * Add HasPeers function to group * Refactor group management * Optimize group change effects on account peers * Update account peers if ns group has peers * Refactor group changes * Optimize account peers update in DNS settings * Optimize update of account peers on jwt groups sync * Refactor peer account updates for efficiency * Optimize peer update on user deletion and changes * Remove condition check for network serial update * Optimize account peers updates on route changes * Remove UpdatePeerSSHKey method * Remove unused isPolicyRuleGroupsEmpty * Add tests for peer update behavior on posture check changes * Add tests for peer update behavior on policy changes * Add tests for peer update behavior on group changes * Add tests for peer update behavior on dns settings changes * Refactor * Add tests for peer update behavior on name server changes * Add tests for peer update behavior on user changes * Add tests for peer update behavior on route changes * fix tests * Add tests for peer update behavior on setup key changes * Add tests for peer update behavior on peers changes * fix merge * Fix tests * go mod tidy * Add NameServer and Route comparators * Update network map diff logic with custom comparators * Add tests * Refactor duplicate diff handling logic * fix linter * fix tests * Refactor policy group handling and update logic. Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * Update route check by checking if group has peers Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * Refactor posture check policy linking logic Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * Simplify peer update condition in DNS management Refactor the condition for updating account peers to remove redundant checks Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix merge Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add policy tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add posture checks tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix user and setup key tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix account and route tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix typo Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix nameserver tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix routes tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix group tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * upgrade diff package Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix nameserver tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * use generic differ for netip.Addr and netip.Prefix Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * go mod tidy Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add peer tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix merge Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix management suite tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix postgres tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * enable diff nil structs comparison Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * skip the update only last sent the serial is larger Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * refactor peer and user Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * skip spell check for groupD Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * Refactor group, ns group, policy and posture checks Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * skip spell check for GroupD Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * update account policy check before verifying policy status Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * Update management/server/route_test.go Co-authored-by: Maycon Santos <mlsmaycon@gmail.com> * Update management/server/route_test.go Co-authored-by: Maycon Santos <mlsmaycon@gmail.com> * Update management/server/route_test.go Co-authored-by: Maycon Santos <mlsmaycon@gmail.com> * Update management/server/route_test.go Co-authored-by: Maycon Santos <mlsmaycon@gmail.com> * Update management/server/route_test.go Co-authored-by: Maycon Santos <mlsmaycon@gmail.com> * add tests missing tests for dns setting groups Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add tests for posture checks changes Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add ns group and policy tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add route and group tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * increase Linux test timeout to 10 minutes Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * Run diff for client posture checks only Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add panic recovery and detailed logging in peer update comparison Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * Fix tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> --------- Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> Co-authored-by: Maycon Santos <mlsmaycon@gmail.com> --------- Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> Co-authored-by: Maycon Santos <mlsmaycon@gmail.com>
2025-08-17 02:21:47 +02:00 · 2024-10-23 13:05:02 +03:00
parent 30ebcf38c7
commit 7bda385e1b
32 changed files with 3485 additions and 280 deletions
--- a/management/server/account_test.go
+++ b/management/server/account_test.go
@ -1122,66 +1122,196 @@ func TestAccountManager_AddPeerWithUserID(t *testing.T) {
 	assert.Equal(t, peer.IP.String(), fmt.Sprint(ev.Meta["ip"]))
 }

-func TestAccountManager_NetworkUpdates(t *testing.T) {
-	manager, err := createManager(t)
-	if err != nil {
-		t.Fatal(err)
+func TestAccountManager_NetworkUpdates_SaveGroup(t *testing.T) {
+	manager, account, peer1, peer2, peer3 := setupNetworkMapTest(t)
+
+	group := group.Group{
+		ID:    "groupA",
+		Name:  "GroupA",
+		Peers: []string{},
+	}
+	if err := manager.SaveGroup(context.Background(), account.Id, userID, &group); err != nil {
+		t.Errorf("save group: %v", err)
 		return
 	}

-	userID := "account_creator"
-
-	account, err := createAccount(manager, "test_account", userID, "")
-	if err != nil {
-		t.Fatal(err)
+	policy := Policy{
+		ID:      "policy",
+		Enabled: true,
+		Rules: []*PolicyRule{
+			{
+				Enabled:       true,
+				Sources:       []string{"groupA"},
+				Destinations:  []string{"groupA"},
+				Bidirectional: true,
+				Action:        PolicyTrafficActionAccept,
+			},
+		},
 	}
+	err := manager.SavePolicy(context.Background(), account.Id, userID, &policy, false)
+	require.NoError(t, err)

-	setupKey, err := manager.CreateSetupKey(context.Background(), account.Id, "test-key", SetupKeyReusable, time.Hour, nil, 999, userID, false)
-	if err != nil {
-		t.Fatal("error creating setup key")
-		return
-	}
+	updMsg := manager.peersUpdateManager.CreateChannel(context.Background(), peer1.ID)
+	defer manager.peersUpdateManager.CloseChannel(context.Background(), peer1.ID)

-	if account.Network.Serial != 0 {
-		t.Errorf("expecting account network to have an initial Serial=0")
-		return
-	}
+	wg := sync.WaitGroup{}
+	wg.Add(1)
+	go func() {
+		defer wg.Done()

-	getPeer := func() *nbpeer.Peer {
-		key, err := wgtypes.GeneratePrivateKey()
-		if err != nil {
-			t.Fatal(err)
-			return nil
+		message := <-updMsg
+		networkMap := message.Update.GetNetworkMap()
+		if len(networkMap.RemotePeers) != 2 {
+			t.Errorf("mismatch peers count: 2 expected, got %v", len(networkMap.RemotePeers))
 		}
-		expectedPeerKey := key.PublicKey().String()
+	}()

-		peer, _, _, err := manager.AddPeer(context.Background(), setupKey.Key, "", &nbpeer.Peer{
-			Key:  expectedPeerKey,
-			Meta: nbpeer.PeerSystemMeta{Hostname: expectedPeerKey},
-		})
-		if err != nil {
-			t.Fatalf("expecting peer1 to be added, got failure %v", err)
-			return nil
-		}
-
-		return peer
+	group.Peers = []string{peer1.ID, peer2.ID, peer3.ID}
+	if err := manager.SaveGroup(context.Background(), account.Id, userID, &group); err != nil {
+		t.Errorf("save group: %v", err)
+		return
 	}

-	peer1 := getPeer()
-	peer2 := getPeer()
-	peer3 := getPeer()
+	wg.Wait()
+}

-	account, err = manager.Store.GetAccount(context.Background(), account.Id)
-	if err != nil {
-		t.Fatal(err)
+func TestAccountManager_NetworkUpdates_DeletePolicy(t *testing.T) {
+	manager, account, peer1, _, _ := setupNetworkMapTest(t)
+
+	updMsg := manager.peersUpdateManager.CreateChannel(context.Background(), peer1.ID)
+	defer manager.peersUpdateManager.CloseChannel(context.Background(), peer1.ID)
+
+	wg := sync.WaitGroup{}
+	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.DeletePolicy(context.Background(), account.Id, account.Policies[0].ID, userID); err != nil {
+		t.Errorf("delete default rule: %v", err)
+		return
+	}
+
+	wg.Wait()
+}
+
+func TestAccountManager_NetworkUpdates_SavePolicy(t *testing.T) {
+	manager, account, peer1, peer2, _ := setupNetworkMapTest(t)
+
+	group := group.Group{
+		ID:    "groupA",
+		Name:  "GroupA",
+		Peers: []string{peer1.ID, peer2.ID},
+	}
+	if err := manager.SaveGroup(context.Background(), account.Id, userID, &group); err != nil {
+		t.Errorf("save group: %v", err)
 		return
 	}

 	updMsg := manager.peersUpdateManager.CreateChannel(context.Background(), peer1.ID)
 	defer manager.peersUpdateManager.CloseChannel(context.Background(), peer1.ID)

+	policy := Policy{
+		Enabled: true,
+		Rules: []*PolicyRule{
+			{
+				Enabled:       true,
+				Sources:       []string{"groupA"},
+				Destinations:  []string{"groupA"},
+				Bidirectional: true,
+				Action:        PolicyTrafficActionAccept,
+			},
+		},
+	}
+
+	wg := sync.WaitGroup{}
+	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.SavePolicy(context.Background(), account.Id, userID, &policy, false); err != nil {
+		t.Errorf("delete default rule: %v", err)
+		return
+	}
+
+	wg.Wait()
+}
+
+func TestAccountManager_NetworkUpdates_DeletePeer(t *testing.T) {
+	manager, account, peer1, _, peer3 := setupNetworkMapTest(t)
+
 	group := group.Group{
-		ID:    "group-id",
+		ID:    "groupA",
+		Name:  "GroupA",
+		Peers: []string{peer1.ID, peer3.ID},
+	}
+	if err := manager.SaveGroup(context.Background(), account.Id, userID, &group); err != nil {
+		t.Errorf("save group: %v", err)
+		return
+	}
+
+	policy := Policy{
+		Enabled: true,
+		Rules: []*PolicyRule{
+			{
+				Enabled:       true,
+				Sources:       []string{"groupA"},
+				Destinations:  []string{"groupA"},
+				Bidirectional: true,
+				Action:        PolicyTrafficActionAccept,
+			},
+		},
+	}
+
+	if err := manager.SavePolicy(context.Background(), account.Id, userID, &policy, false); err != nil {
+		t.Errorf("save policy: %v", err)
+		return
+	}
+
+	updMsg := manager.peersUpdateManager.CreateChannel(context.Background(), peer1.ID)
+	defer manager.peersUpdateManager.CloseChannel(context.Background(), peer1.ID)
+
+	wg := sync.WaitGroup{}
+	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(context.Background(), account.Id, peer3.ID, userID); err != nil {
+		t.Errorf("delete peer: %v", err)
+		return
+	}
+
+	wg.Wait()
+}
+
+func TestAccountManager_NetworkUpdates_DeleteGroup(t *testing.T) {
+	manager, account, peer1, peer2, peer3 := setupNetworkMapTest(t)
+
+	updMsg := manager.peersUpdateManager.CreateChannel(context.Background(), peer1.ID)
+	defer manager.peersUpdateManager.CloseChannel(context.Background(), peer1.ID)
+
+	group := group.Group{
+		ID:    "groupA",
 		Name:  "GroupA",
 		Peers: []string{peer1.ID, peer2.ID, peer3.ID},
 	}
@ -1191,116 +1321,48 @@ func TestAccountManager_NetworkUpdates(t *testing.T) {
 		Rules: []*PolicyRule{
 			{
 				Enabled:       true,
-				Sources:       []string{"group-id"},
-				Destinations:  []string{"group-id"},
+				Sources:       []string{"groupA"},
+				Destinations:  []string{"groupA"},
 				Bidirectional: true,
 				Action:        PolicyTrafficActionAccept,
 			},
 		},
 	}

+	if err := manager.DeletePolicy(context.Background(), account.Id, account.Policies[0].ID, userID); err != nil {
+		t.Errorf("delete default rule: %v", err)
+		return
+	}
+
+	if err := manager.SavePolicy(context.Background(), account.Id, userID, &policy, false); err != nil {
+		t.Errorf("save policy: %v", err)
+		return
+	}
+
 	wg := sync.WaitGroup{}
-	t.Run("save group update", func(t *testing.T) {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+	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(context.Background(), account.Id, userID, &group); err != nil {
-			t.Errorf("save group: %v", err)
-			return
+		message := <-updMsg
+		networkMap := message.Update.GetNetworkMap()
+		if len(networkMap.RemotePeers) != 0 {
+			t.Errorf("mismatch peers count: 0 expected, got %v", len(networkMap.RemotePeers))
 		}
+	}()

-		wg.Wait()
-	})
+	// clean policy is pre requirement for delete group
+	if err := manager.DeletePolicy(context.Background(), account.Id, policy.ID, userID); err != nil {
+		t.Errorf("delete default rule: %v", err)
+		return
+	}

-	t.Run("delete policy update", func(t *testing.T) {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+	if err := manager.DeleteGroup(context.Background(), account.Id, "", group.ID); err != nil {
+		t.Errorf("delete group: %v", err)
+		return
+	}

-			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.DeletePolicy(context.Background(), account.Id, account.Policies[0].ID, userID); err != nil {
-			t.Errorf("delete default rule: %v", err)
-			return
-		}
-
-		wg.Wait()
-	})
-
-	t.Run("save policy 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.SavePolicy(context.Background(), account.Id, userID, &policy, false); 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(context.Background(), account.Id, peer3.ID, userID); 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))
-			}
-		}()
-
-		// clean policy is pre requirement for delete group
-		_ = manager.DeletePolicy(context.Background(), account.Id, policy.ID, userID)
-
-		if err := manager.DeleteGroup(context.Background(), account.Id, "", group.ID); err != nil {
-			t.Errorf("delete group: %v", err)
-			return
-		}
-
-		wg.Wait()
-	})
+	wg.Wait()
 }

 func TestAccountManager_DeletePeer(t *testing.T) {
@ -2754,3 +2816,73 @@ func waitTimeout(wg *sync.WaitGroup, timeout time.Duration) bool {
 		return true
 	}
 }
+
+func setupNetworkMapTest(t *testing.T) (*DefaultAccountManager, *Account, *nbpeer.Peer, *nbpeer.Peer, *nbpeer.Peer) {
+	t.Helper()
+
+	manager, err := createManager(t)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	account, err := createAccount(manager, "test_account", userID, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	setupKey, err := manager.CreateSetupKey(context.Background(), account.Id, "test-key", SetupKeyReusable, time.Hour, nil, 999, userID, false)
+	if err != nil {
+		t.Fatal("error creating setup key")
+	}
+
+	getPeer := func(manager *DefaultAccountManager, setupKey *SetupKey) *nbpeer.Peer {
+		key, err := wgtypes.GeneratePrivateKey()
+		if err != nil {
+			t.Fatal(err)
+		}
+		expectedPeerKey := key.PublicKey().String()
+
+		peer, _, _, err := manager.AddPeer(context.Background(), setupKey.Key, "", &nbpeer.Peer{
+			Key:  expectedPeerKey,
+			Meta: nbpeer.PeerSystemMeta{Hostname: expectedPeerKey},
+			Status: &nbpeer.PeerStatus{
+				Connected: true,
+				LastSeen:  time.Now().UTC(),
+			},
+		})
+		if err != nil {
+			t.Fatalf("expecting peer to be added, got failure %v", err)
+		}
+
+		return peer
+	}
+
+	peer1 := getPeer(manager, setupKey)
+	peer2 := getPeer(manager, setupKey)
+	peer3 := getPeer(manager, setupKey)
+
+	return manager, account, peer1, peer2, peer3
+}
+
+func peerShouldNotReceiveUpdate(t *testing.T, updateMessage <-chan *UpdateMessage) {
+	t.Helper()
+	select {
+	case msg := <-updateMessage:
+		t.Errorf("Unexpected message received: %+v", msg)
+	case <-time.After(500 * time.Millisecond):
+		return
+	}
+}
+
+func peerShouldReceiveUpdate(t *testing.T, updateMessage <-chan *UpdateMessage) {
+	t.Helper()
+
+	select {
+	case msg := <-updateMessage:
+		if msg == nil {
+			t.Errorf("Received nil update message, expected valid message")
+		}
+	case <-time.After(500 * time.Millisecond):
+		t.Error("Timed out waiting for update message")
+	}
+}