diff --git a/client/internal/engine.go b/client/internal/engine.go index d73beb400..d7a8de4d9 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -1709,14 +1709,14 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap, maxBatch int, // remaining peer batches they are skipped. if firstPass { e.applyWholesale(networkMap, serial) + + doneOffline := e.phase("offline_peers") + e.updateOfflinePeers(networkMap.GetOfflinePeers()) + doneOffline() } log.Debugf("got peers update from Management Service, total peers to connect to = %d", len(networkMap.GetRemotePeers())) - doneOffline := e.phase("offline_peers") - e.updateOfflinePeers(networkMap.GetOfflinePeers()) - doneOffline() - // Filter out own peer from the remote peers list localPubKey := e.config.WgPrivateKey.PublicKey().String() remotePeers := make([]*mgmProto.RemotePeerConfig, 0, len(networkMap.GetRemotePeers())) @@ -1756,18 +1756,18 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap, maxBatch int, e.statusRecorder.FinishPeerListModifications() - e.updatePeerSSHHostKeys(remotePeers) - - if err := e.updateSSHClientConfig(remotePeers); err != nil { - log.Warnf("failed to update SSH client config: %v", err) - } - - e.updateSSHServerAuth(networkMap.GetSshAuth()) - - // Set the exclude list only once peers have fully converged (this pass added - // the last batch). It needs all target peers present in the store, and - // ExcludePeer has replace-semantics — a partial set mid-convergence would be wrong. + // The SSH and lazy-exclude steps walk the whole map and need every target peer + // in the store, so they run once, on the pass that added the last batch. The + // exclude list also has replace-semantics: a partial set would be wrong. if !needMore { + e.updatePeerSSHHostKeys(remotePeers) + + if err := e.updateSSHClientConfig(remotePeers); err != nil { + log.Warnf("failed to update SSH client config: %v", err) + } + + e.updateSSHServerAuth(networkMap.GetSshAuth()) + doneLazy := e.phase("lazy_exclude") excludedLazyPeers := e.toExcludedLazyPeers(remotePeers) e.connMgr.SetExcludeList(e.ctx, excludedLazyPeers) diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 7394de106..ac2db189b 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -790,6 +790,44 @@ func TestEngine_UpdateNetworkMap(t *testing.T) { require.False(t, more, "pass 2 should converge") assert.Equal(t, before+1, countMapEvents(), "the converging pass should publish exactly one event") }) + + // The SSH steps walk the whole map and need every peer in the store, so the + // draining passes skip them and the converging pass records every host key. + t.Run("chunked apply records SSH host keys once peers converge", func(t *testing.T) { + sshPeer := func(key, ip string) *mgmtProto.RemotePeerConfig { + return &mgmtProto.RemotePeerConfig{ + WgPubKey: key, + AllowedIps: []string{ip}, + SshConfig: &mgmtProto.SSHConfig{SshPubKey: []byte("ssh-ed25519 " + key)}, + } + } + peers := []*mgmtProto.RemotePeerConfig{ + sshPeer("SSHf3Ma6z6mdLbriAJbqhX7+nM/B71lgw2+91q3LfhU=", "100.64.0.20/32"), + sshPeer("TSHf3Ma6z6mdLbriAJbqhX7+nM/B71lgw2+91q3LfhU=", "100.64.0.21/32"), + } + hostKey := func(p *mgmtProto.RemotePeerConfig) []byte { + state, err := engine.statusRecorder.GetPeer(p.GetWgPubKey()) + if err != nil { + return nil + } + return state.SSHHostKey + } + nm := &mgmtProto.NetworkMap{Serial: 9, RemotePeers: peers} + + more, err := engine.updateNetworkMap(nm, 1, true) + require.NoError(t, err) + require.True(t, more, "the first pass should leave batches to drain") + assert.Nil(t, hostKey(peers[0]), "a draining pass should not record SSH host keys") + + for i := 0; more && i < 10; i++ { + more, err = engine.updateNetworkMap(nm, 1, false) + require.NoError(t, err) + } + require.False(t, more, "the map should converge") + for _, p := range peers { + assert.Equal(t, p.GetSshConfig().GetSshPubKey(), hostKey(p), "the converging pass should record the host key of %s", p.GetWgPubKey()) + } + }) } func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {