node: pull keys, bans and policy from the control plane in one loop - #453
Conversation
Every fresh join on hub.sam-mesh.dev has failed since 2026-09-19 19:45Z with "failed to verify router biscuit: no valid key found", which is what the cold-path probe measures and why testnet-health has been red. The control plane rotated its signing key 24h after the rc.1 rollout. The routers still present biscuits signed by the retiring key, which is legitimately in its 48h grace period, but the enroll response carries only the newest key and the node dials the router with just that one before main.go's post-enrollment /keys catch-up runs. bananas never sees this because it is redeployed several times a day and its rotation ticker never reaches 24h. Reproduced locally against hub (hub /keys advertises two keys, bananas one). The deeper gap is that a running node learned a rotated key, a ban or a policy change only from a gossip event published once with no replay (and the shipped control plane never publishes them, google#317). Routers already poll /keys and /info as a safety net; nodes had three separate partial paths: SyncMeshConfig at start, a policy loop, and the event handlers. Enrollment now fetches the full key set right after the response and before the router handshake, on both the OIDC and bootstrap paths. At runtime one loop pulls /keys, /info and /policies together every --control-plane-sync-interval (5m, matching the router), reconciling the ban set the same way the router does so a ban or an unban reaches a running node without an event. POLICY_UPDATE now just brings the next pull forward. --policy-sync-interval is replaced by the new flag. Regression test: an enroll response carrying only the current key against a router still signed by the grace-period key, which fails on the parent commit with the exact error seen on hub.
There was a problem hiding this comment.
Code Review
This pull request consolidates the synchronization of control plane data (signing keys, ban sets, router addresses, and mesh policy) into a single unified loop running on a 5-minute interval, replacing the separate policy sync loop. It also ensures that a newly enrolled node fetches the full set of trusted keys from the control plane before the router handshake, preventing failures when routers use keys still in their grace period. The feedback recommends adding defensive nil checks for the control plane sync trigger channel to prevent potential hangs if the node is constructed manually without initializing this channel.
| func (n *SamNode) triggerControlPlaneSync() { | ||
| select { | ||
| case n.controlPlaneSyncTrigger <- struct{}{}: | ||
| default: | ||
| } | ||
| } |
There was a problem hiding this comment.
To prevent potential indefinite blocks or hangs when SamNode is constructed manually (e.g., in tests or external integrations) without initializing the trigger channel, it is highly recommended to add a defensive nil check for n.controlPlaneSyncTrigger before attempting to send to it.
| func (n *SamNode) triggerControlPlaneSync() { | |
| select { | |
| case n.controlPlaneSyncTrigger <- struct{}{}: | |
| default: | |
| } | |
| } | |
| func (n *SamNode) triggerControlPlaneSync() { | |
| if n.controlPlaneSyncTrigger == nil { | |
| return | |
| } | |
| select { | |
| case n.controlPlaneSyncTrigger <- struct{}{}: | |
| default: | |
| } | |
| } |
| } | ||
| go func() { | ||
| timer := time.NewTimer(2 * time.Second) | ||
| defer timer.Stop() |
There was a problem hiding this comment.
Similarly, we should defensively check if n.controlPlaneSyncTrigger is nil in startControlPlaneSyncLoop to avoid blocking indefinitely on the select case case <-n.controlPlaneSyncTrigger: if the channel was not initialized.
func (n *SamNode) startControlPlaneSyncLoop(ctx context.Context, interval time.Duration) {
if interval <= 0 || n.Store == nil || n.controlPlaneSyncTrigger == nil {
return
}Five minutes was the router's cadence, not a number chosen for nodes, and a mesh can be a handful of laptops or a million devices. The one constraint on the interval is the control plane's --key-grace-period (1h by default): a successor key can only be adopted while the key it replaces still vouches for it, and one attempt per window is a race. Fifteen minutes gives four attempts per default window and a fifth of the load; an operator who raises the grace period can raise this too, and the flag now says so. Every periodic pull is stretched by up to a tenth of the interval, and the jitter applied to an event-triggered pull now defaults to the same tenth instead of a fixed ten seconds, so a policy update on a large mesh is spread over minutes rather than landing on the control plane at once.
What
Fixes the red testnet-health on hub: every fresh join has failed since 2026-09-19 19:45Z, which is what the cold-path probe measures. Also closes the gap that let it happen, by giving nodes the same pull-based safety net routers already have, in one place instead of three — and makes that place the only place node and router read from the control plane.
Why
Reproduced locally with
sam-node run --join --control-plane https://hub.sam-mesh.dev:hub/keysadvertises two signing keys,bananas/keysone: hub's control plane rotated its key 24h after the rc.1 rollout (bananas is redeployed several times a day, so its 24h rotation ticker never fires). The routers still present biscuits signed by the retiring key, legitimately inside its 48h grace period, but the enroll response carries only the newest key andEnroll()dials the router with just that one —main.go's post-enrollment/keyscatch-up ran too late.The underlying problem is wider than enrollment: a running node learned a rotated key, a ban or a policy change only from a gossip event that is published once with no replay — and the shipped control plane never publishes them (#317, fixed separately in #454). Routers poll
/keysand/infoperiodically as a safety net; nodes hadSyncMeshConfigat start, a separate policy loop, and the event handlers.How (by commit)
node: pull keys, bans and policy from the control plane in one loop/keysright after the response and before the router handshake, on both the OIDC and bootstrap paths (adoptEnrolledKeys).internal/node/controlplane_sync.go):SyncControlPlanepulls/keys,/infoand/policiestogether; each part is attempted even if another fails./inforeconciles the ban set on the node the way the router already does.POLICY_UPDATEjust triggers the unified pull;BANNED/KEY_ROTATIONstill apply immediately.--policy-sync-intervalwith--control-plane-sync-interval.node: sync every 15m by default, spread the fleet's pulls--key-grace-period(1h default) — a successor key can only be adopted while its predecessor still vouches for it, so one attempt per window is a race; four is the trade against load on large meshes. Flag help and docs say to keep it well below the grace period and raise it on large meshes.node, router: one client for the control plane's pull endpointsinternal/controlplane/client: the singleGET /keys(verified) andGET /infoimplementation, plus the HTTP client with the per-hop plaintext check. Depends onapi/only, so it pulls in none of the control plane server. Node and router both use it; the router's/keysread gains the 1 MiB body cap.SyncMeshConfig(the store-level copy of the startup pull) is gone:sam-node runand the mobile FFI build the node from the stored config and callSyncControlPlanebeforeStart, which adopts the control plane's current router addresses in memory until the host exists.Options.BannedPeerIDs, which only carriedSyncMeshConfig's ban set, goes with it.Tests
TestEnrollTrustsRouterSignedByGraceKey(regression): enroll response carries only the current key, mock router signed by the grace-period key. Fails on the parent commit with the exact hub error.TestSyncControlPlane,TestSyncControlPlaneBeforeStart(reachable and unreachable control plane),TestSyncControlPlaneRefusesUntrustedKeySet,TestSyncTrustedKeys,TestReconcileBannedPeers(incl. non-canonical peer ID encoding),TestControlPlaneSyncLoop,TestGaterEnforcesSeededBans.internal/controlplane/client:/infodecode and path,/keysadoption and rejection (stranger-signed, unsigned), status/decode/oversize errors, and the plaintext transport policy read per request.TestSyncKeysRequiresTrustedSignatureand friends pass through the shared client.Sending to CI for
make test/ integration / e2e — this laptop is short on resources for the full suites.Related
173.255.113.127:4501is unreachable from the outside (dial timeout); unrelated to this change.