diff --git a/management/server/peer.go b/management/server/peer.go index 589cf9abf..bb3ac5ff6 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -1066,6 +1066,12 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSy metaDiffAffectsPosture := posture.AffectsPosture(ctx, &metaDiff, resPostureChecks) if requiresPeerUpdate(ctx, isStatusChanged, sync.UpdateAccountPeers, ipv6CapabilityChanged, metaDiffAffectsPosture, metaDiff.VersionChanged(), metaDiff.HostnameChanged()) { + // The maps pushed below carry changed content (the peer's version, + // hostname, capabilities, or validation state). The serial versions the + // distributed map, so it must advance with the content. + if err = am.Store.IncrementNetworkSerial(ctx, accountID); err != nil { + return nil, nil, nil, 0, fmt.Errorf("increment network serial: %w", err) + } changedPeerIDs := []string{peer.ID} affectedPeerIDs := am.syncPeerAffectedPeers(ctx, accountID, peer.ID, nmap, peerNotValid, metaDiffAffectsPosture) if err = am.networkMapController.OnPeersUpdated(ctx, accountID, changedPeerIDs, affectedPeerIDs); err != nil { @@ -1236,6 +1242,11 @@ func (am *DefaultAccountManager) LoginPeer(ctx context.Context, login types.Peer } if shouldUpdatePeers { + // The maps pushed below carry changed peer content. The serial versions + // the distributed map, so it must advance with the content. + if err = am.Store.IncrementNetworkSerial(ctx, accountID); err != nil { + return nil, nil, nil, false, fmt.Errorf("increment network serial: %w", err) + } changedPeerIDs := []string{peer.ID} affectedPeerIDs := am.resolveAffectedPeersForPeerChanges(ctx, am.Store, accountID, changedPeerIDs) if err = am.networkMapController.OnPeersUpdated(ctx, accountID, changedPeerIDs, affectedPeerIDs); err != nil { diff --git a/management/server/peer_test.go b/management/server/peer_test.go index 80d270e98..1b79449f8 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -16,11 +16,11 @@ import ( "testing" "time" - "go.uber.org/mock/gomock" "github.com/rs/xid" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "golang.org/x/exp/maps" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" @@ -2828,6 +2828,46 @@ func TestSyncPeer_IPv6CapabilityChangePropagates(t *testing.T) { }) } +// TestSyncPeer_PeerUpdateBumpsNetworkSerial ensures that a sync which changes +// map-relevant peer content (agent version, hostname, capabilities, validation +// state) advances the network serial before other peers receive the recomputed +// map. The serial versions the distributed map, so its content must not change +// under an unchanged serial. +func TestSyncPeer_PeerUpdateBumpsNetworkSerial(t *testing.T) { + manager, _, account, _, peer2, _ := setupNetworkMapTest(t) + + network, err := manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id) + require.NoError(t, err) + serialBefore := network.CurrentSerial() + + t.Run("no bump when nothing changed", func(t *testing.T) { + _, _, _, _, err := manager.SyncPeer(context.Background(), types.PeerSync{ + WireGuardPubKey: peer2.Key, + Meta: peer2.Meta, + }, peer2.AccountID) + require.NoError(t, err) + + network, err := manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id) + require.NoError(t, err) + assert.Equal(t, serialBefore, network.CurrentSerial(), "an unchanged sync should not advance the serial") + }) + + t.Run("bump when the agent version changes", func(t *testing.T) { + newMeta := peer2.Meta + newMeta.WtVersion = "0.99.99" + + _, _, _, _, err := manager.SyncPeer(context.Background(), types.PeerSync{ + WireGuardPubKey: peer2.Key, + Meta: newMeta, + }, peer2.AccountID) + require.NoError(t, err) + + network, err := manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id) + require.NoError(t, err) + assert.Greater(t, network.CurrentSerial(), serialBefore, "a map-relevant meta change should advance the serial") + }) +} + func TestUpdatePeer_DnsLabelCollisionWithFQDN(t *testing.T) { manager, _, err := createManager(t) require.NoError(t, err, "unable to create account manager")