Allow also nm only (empty config) syncs to be squashed into target state

[changing behavior!] This makes the debug bundle contain the SUPERSET of the map with nm updates and config updates

client/internal/mapsync.go

@@ -6,7 +6,6 @@ import (
 	"time"
 
 	log "github.com/sirupsen/logrus"
-	"google.golang.org/protobuf/proto"
 
 	mgmProto "github.com/netbirdio/netbird/shared/management/proto"
 )
@@ -110,22 +109,31 @@ func (m *mapStateManager) SetTarget(update *mgmProto.SyncResponse) error {
 // rest of the manager (generation tracking, convergence, signaling) is unaffected
 // because it already treats target as "the complete desired state, whatever it is".
 func (m *mapStateManager) mergeTarget(prev, update *mgmProto.SyncResponse) *mgmProto.SyncResponse {
-	// Plain replace unless a config-only update (no map) arrives while the pending
-	// target still has an unconverged map (targetGen > appliedGen). In that case graft
-	// the pending map (and its checks) onto the incoming config update, so the next pass
-	// keeps converging the map instead of settling on a nil-map target and stranding the
-	// remaining peer batches until another full map arrives.
-	if update == nil || update.GetNetworkMap() != nil || prev == nil || prev.GetNetworkMap() == nil || m.targetGen == m.appliedGen {
+	// Nothing pending to preserve (no prev, or prev already fully applied): plain replace.
+	if prev == nil || update == nil || m.targetGen == m.appliedGen {
 		return update
 	}
 
-	merged, ok := proto.Clone(update).(*mgmProto.SyncResponse)
-	if !ok {
-		return update
+	// prev still has unapplied state (targetGen > appliedGen). In the sync protocol a
+	// nil component means "no change", so if `update` omits a component that prev
+	// carried, carry prev's forward — otherwise coalescing an update that superseded a
+	// not-yet-applied one would silently drop the map or config it uniquely brought.
+	// A present component in `update` is newer and wins. Management may send map-only
+	// updates (nil config) and config-only updates (nil map); both are handled here.
+	// A nil component in `update` means "no change", so fill it in from prev — otherwise
+	// coalescing an update that superseded a not-yet-applied one would drop the map or
+	// config it uniquely carried. A present component in `update` is newer and wins.
+	// We mutate `update` in place: it is a fresh per-message allocation from the sync
+	// stream (see receiveUpdatesEvents — not reused), and persisting this squashed target
+	// is correct, since it is the current full (superset) desired state.
+	if update.GetNetworkMap() == nil && prev.GetNetworkMap() != nil {
+		update.NetworkMap = prev.GetNetworkMap()
+		update.Checks = prev.Checks // checks travel with the map
 	}
-	merged.NetworkMap = prev.GetNetworkMap()
-	merged.Checks = prev.Checks
-	return merged
+	if update.GetNetbirdConfig() == nil && prev.GetNetbirdConfig() != nil {
+		update.NetbirdConfig = prev.GetNetbirdConfig()
+	}
+	return update
 }
 
 // run drives convergence until ctx is done. It is meant to run in its own goroutine.

client/internal/mapsync_test.go

@@ -12,32 +12,43 @@ import (
 	mgmProto "github.com/netbirdio/netbird/shared/management/proto"
 )
 
-// a config-only update arriving while a full map is still converging must keep the
-// pending map (so its remaining peer batches still apply); once converged or when the
-// pending target has no map, it replaces as usual.
-func TestMapStateManager_MergeTargetPreservesPendingMap(t *testing.T) {
+// mergeTarget fills components missing from the incoming update with the pending
+// (not-yet-applied) prev's, in place, so a coalesced/superseded update does not drop
+// the map or config it uniquely carried.
+func TestMapStateManager_MergeTargetPreservesPendingState(t *testing.T) {
 	m := newMapStateManager(nil, nil, nil)
 
-	fullMap := &mgmProto.SyncResponse{NetworkMap: &mgmProto.NetworkMap{Serial: 5}}
-	configOnly := &mgmProto.SyncResponse{NetbirdConfig: &mgmProto.NetbirdConfig{}}
-
-	// still converging the full map (targetGen > appliedGen): graft the map onto the
-	// incoming config-only update instead of dropping it
+	// config-only update while a full map is still converging (targetGen > appliedGen):
+	// the pending map (+ checks) is filled into the update in place
 	m.targetGen, m.appliedGen = 5, 4
-	merged := m.mergeTarget(fullMap, configOnly)
-	require.NotNil(t, merged.GetNetworkMap(), "pending map must be preserved")
-	require.EqualValues(t, 5, merged.GetNetworkMap().GetSerial())
-	require.NotNil(t, merged.GetNetbirdConfig(), "new config must be carried")
-	require.NotSame(t, configOnly, merged, "must not mutate the received update in place")
+	prev := &mgmProto.SyncResponse{NetworkMap: &mgmProto.NetworkMap{Serial: 5}}
+	update := &mgmProto.SyncResponse{NetbirdConfig: &mgmProto.NetbirdConfig{}}
+	merged := m.mergeTarget(prev, update)
+	require.Same(t, update, merged, "merges in place, returns the update")
+	require.EqualValues(t, 5, merged.GetNetworkMap().GetSerial(), "pending map preserved")
+	require.NotNil(t, merged.GetNetbirdConfig(), "new config kept")
 
-	// already converged (targetGen == appliedGen): nothing pending -> plain replace
+	// symmetric: map-only update while a config-only update is pending -> keep the config
+	m.targetGen, m.appliedGen = 5, 4
+	prev = &mgmProto.SyncResponse{NetbirdConfig: &mgmProto.NetbirdConfig{}}
+	update = &mgmProto.SyncResponse{NetworkMap: &mgmProto.NetworkMap{Serial: 7}}
+	merged = m.mergeTarget(prev, update)
+	require.EqualValues(t, 7, merged.GetNetworkMap().GetSerial(), "new map kept")
+	require.NotNil(t, merged.GetNetbirdConfig(), "pending config preserved")
+
+	// prev already applied (targetGen == appliedGen): plain replace, no fill-in
 	m.targetGen, m.appliedGen = 5, 5
-	require.Same(t, configOnly, m.mergeTarget(fullMap, configOnly))
+	prev = &mgmProto.SyncResponse{NetworkMap: &mgmProto.NetworkMap{Serial: 5}}
+	update = &mgmProto.SyncResponse{NetbirdConfig: &mgmProto.NetbirdConfig{}}
+	merged = m.mergeTarget(prev, update)
+	require.Same(t, update, merged)
+	require.Nil(t, merged.GetNetworkMap(), "no map grafted when prev already applied")
 
-	// a full map always replaces
-	newFull := &mgmProto.SyncResponse{NetworkMap: &mgmProto.NetworkMap{Serial: 6}}
+	// nothing to carry (update has a map, prev has no config): plain replace
 	m.targetGen, m.appliedGen = 5, 4
-	require.Same(t, newFull, m.mergeTarget(fullMap, newFull))
+	prev = &mgmProto.SyncResponse{NetworkMap: &mgmProto.NetworkMap{Serial: 5}}
+	update = &mgmProto.SyncResponse{NetworkMap: &mgmProto.NetworkMap{Serial: 6}}
+	require.Same(t, update, m.mergeTarget(prev, update))
 }
 
 // converges over the bounded passes (apply returns more until the 3rd pass),