diff --git a/hive-c0re/src/swarm_status.rs b/hive-c0re/src/swarm_status.rs index f4b66698..072b5fdd 100644 --- a/hive-c0re/src/swarm_status.rs +++ b/hive-c0re/src/swarm_status.rs @@ -135,12 +135,7 @@ pub fn spawn( // paths independent reconnect state, so one could be serving while // the other was still down. `async_nats::Client` is a handle, so the // clone is cheap. - tokio::spawn(drain_swarm_events( - client.clone(), - hive.clone(), - coord, - shutdown.clone(), - )); + tokio::spawn(drain_swarm_events(client.clone(), coord, shutdown.clone())); let mut health = SweepHealth::new("swarm_status_publish", "warn", FAILURES_BEFORE_BANNER); loop { @@ -179,7 +174,7 @@ pub fn spawn( }); } -/// Listen on this hive's swarm-event subject and act on what arrives. +/// Listen on the swarm's knowledge-event subject and act on what arrives. /// /// The controller decides *what a forge delivery means* and addresses the /// result here; this end does not know a forge exists. Today the one event is @@ -211,12 +206,14 @@ pub fn spawn( /// changes, the server log is the thing that knows why — nothing here will say. async fn drain_swarm_events( client: async_nats::Client, - hive: String, coord: std::sync::Arc, mut shutdown: tokio::sync::watch::Receiver, ) { - let subject = swarm_queue_client::events::knowledge(&hive); - let mut sub = match client.subscribe(subject.clone()).await { + // One subject for the whole swarm, so this hive's own name never enters + // it: the controller publishes once and core NATS fans out to whoever is + // subscribed. + let subject = swarm_queue_client::knowledge::SUBJECT; + let mut sub = match client.subscribe(subject).await { Ok(sub) => sub, Err(e) => { // Warn rather than a boot banner: the hive is fully functional diff --git a/swarm-controller/src/webhook.rs b/swarm-controller/src/webhook.rs index 6a2435b6..740cf358 100644 --- a/swarm-controller/src/webhook.rs +++ b/swarm-controller/src/webhook.rs @@ -367,14 +367,14 @@ pub(super) async fn post_webhook_forge( "webhook: verified delivery" ); - match kind { - DeliveryKind::Knowledge => announce_knowledge_change(&state).await, - // Deploy coordination is a separate concern with its own issue: a - // hive does not want to hear that a config PR was opened, it wants - // to be told when to rebuild from main, and that is a decision the - // controller makes after a merge rather than a relay of this - // delivery. Logged above and deliberately not forwarded. - DeliveryKind::ConfigPr => {} + // Only the knowledge delivery is acted on. Deploy coordination is a + // separate concern with its own issue — a hive does not want to hear that a + // config PR was opened, it wants to be told when to rebuild from main, and + // that is a decision the controller makes after a merge rather than a relay + // of this delivery. Written as a condition rather than a match arm holding + // an empty body, which would claim this is where that path is handled. + if kind == DeliveryKind::Knowledge { + announce_knowledge_change(&state).await; } (StatusCode::OK, "ok").into_response() @@ -416,22 +416,23 @@ async fn announce_knowledge_change(state: &AppState) { return; }; let client = status.queue_client(); + let subject = swarm_queue_client::knowledge::SUBJECT; - for hive in state.hives.iter() { - let subject = swarm_queue_client::events::knowledge(&hive.name); - if let Err(e) = client.publish(subject.clone(), Vec::new().into()).await { - tracing::warn!( - hive = %hive.name, %subject, error = %e, - "webhook: publishing the knowledge event failed" - ); - } else { - tracing::info!(hive = %hive.name, %subject, "webhook: knowledge event published"); - } + // One publish, not one per hive: every subscriber gets the same empty + // event, so the roster is not consulted at all. The controller does not + // need to know who the hives are in order to say the repository moved. + if let Err(e) = client.publish(subject, Vec::new().into()).await { + tracing::warn!(%subject, error = %e, "webhook: publishing the knowledge event failed"); + return; } - + // Logged after the flush rather than after the publish: `publish` only + // hands the message to the client's write buffer, so a line printed there + // would claim delivery this end cannot yet know about. if let Err(e) = client.flush().await { - tracing::warn!(error = %e, "webhook: flushing knowledge events failed"); + tracing::warn!(%subject, error = %e, "webhook: flushing the knowledge event failed"); + return; } + tracing::info!(%subject, "webhook: knowledge event published"); } #[cfg(test)] diff --git a/swarm-nats-auth/src/policy.rs b/swarm-nats-auth/src/policy.rs index 411f0668..23e21911 100644 --- a/swarm-nats-auth/src/policy.rs +++ b/swarm-nats-auth/src/policy.rs @@ -234,18 +234,19 @@ impl Policy { // stays for a named/durable consumer. format!("$JS.API.CONSUMER.CREATE.{stream}"), format!("$JS.API.CONSUMER.CREATE.{stream}.>"), - // Swarm events. The controller is the only publisher, and it - // publishes to *every* hive's subject, so the grant takes the - // wildcard form — from the same function the publisher calls, so a - // rename cannot leave the grant naming a subject nobody uses. + // The knowledge event. One writer, many readers: the controller is + // the only publisher and every hive subscribes, so this is one + // literal subject rather than a per-hive family — named from the + // crate the publisher and the subscriber also name, so a rename + // cannot leave the grant pointing at a subject nobody uses. // // This is the reader's only non-JetStream subject, and without it - // the controller cannot emit an event at all. Worth stating because + // the controller cannot emit the event at all. Worth stating because // the symptom is unhelpful: a refused publish reaches the client as // a **timeout**, so the visible failure is a hive that never hears // about a change, with nothing in the controller's log to say a // permission was the reason. - swarm_queue_client::events::knowledge(swarm_queue_client::events::ANY_HIVE), + swarm_queue_client::knowledge::SUBJECT.to_owned(), ]); subjects } @@ -308,36 +309,35 @@ mod tests { } #[test] - fn a_reader_may_publish_swarm_events_for_every_hive() { + fn a_reader_may_publish_the_knowledge_event() { let p = policy().permissions("swarm-controller").expect("a reader"); assert!( - p.publish.contains(&swarm_queue_client::events::knowledge( - swarm_queue_client::events::ANY_HIVE - )), - "the controller is the only event publisher; without this its \ - publish is refused, and a refusal arrives as a timeout" + p.publish + .contains(&swarm_queue_client::knowledge::SUBJECT.to_owned()), + "the controller is the only publisher of this event; without the \ + grant its publish is refused, and a refusal arrives as a timeout" ); } #[test] - fn a_hive_may_not_publish_a_swarm_event_to_anyone_including_itself() { + fn a_hive_may_not_publish_the_knowledge_event_to_anyone_including_itself() { // The controller *interprets* what a delivery means; a hive receives - // that verdict. A hive that could publish on this subject could tell a - // neighbour — or itself — that the knowledge repo changed when it did + // that verdict. A hive able to publish here could tell every other hive + // in the swarm — or itself — that the knowledge repo changed when it did // not, which is an unauthenticated write into someone else's control // path wearing an event's shape. // - // Asserted on the subject ROOT rather than on one rendered subject: a - // future event leaf added to this namespace must fail this test too, - // rather than passing because the test only knew about `knowledge`. + // One writer and many readers makes this arm matter MORE, not less: with + // a single shared subject a forged event reaches the whole swarm, where + // a per-hive subject would have reached one. let p = policy() .permissions("hive-alpha") .expect("a hive is admitted"); assert!( !p.publish .iter() - .any(|s| s.starts_with(swarm_queue_client::events::SUBJECT_ROOT)), - "a hive must not publish into the swarm event namespace: {:?}", + .any(|s| s == swarm_queue_client::knowledge::SUBJECT), + "a hive must not publish the knowledge event: {:?}", p.publish ); } diff --git a/swarm-queue-client/src/events.rs b/swarm-queue-client/src/events.rs deleted file mode 100644 index 883b0b94..00000000 --- a/swarm-queue-client/src/events.rs +++ /dev/null @@ -1,100 +0,0 @@ -//! Swarm event subjects: the names the controller publishes on and hives -//! subscribe to. -//! -//! Same reason [`crate::status`] exists rather than a `const` on each side — -//! **the ends must agree, and a literal repeated across crates is an agreement -//! nothing checks.** Here there are three of them: the swarm controller -//! publishes, a hive subscribes, and the auth-callout responder derives the -//! subject the controller is *permitted* to publish to. A copied literal in the -//! third would produce the worst failure of the set: a grant that looks right, -//! a publish that is refused, and — because a NATS denial reaches the client as -//! a timeout rather than an error — no message saying so. -//! -//! Unconditional, with no feature gate and no NATS types, for the same reason -//! the bucket *name* in [`crate::status`] is not gated: the responder names -//! this subject without ever publishing to it, and speaks neither `jetstream` -//! nor `kv`. A gate here would make that consumer choose between a stack it -//! does not use and a copied literal. -//! -//! # Why a per-hive subject rather than one shared one -//! -//! The callout policy refuses any extra hive subject with no `{hive}` in it, -//! because such a template expands to the same subject for every hive and so -//! grants each of them the others'. A per-hive event subject satisfies that by -//! construction. It is also what makes the event addressable: the controller -//! decides *which* hives need to know, rather than every hive filtering a -//! shared firehose. - -/// The root of the swarm event namespace. -/// -/// `$SWARM` rather than a bare name: the `$` prefix is NATS' convention for -/// system-ish subjects and keeps these clear of anything an application might -/// choose for itself. -pub const SUBJECT_ROOT: &str = "$SWARM.events"; - -/// The leaf naming the *knowledge repository changed* event. -/// -/// Semantic, not transport-shaped: it says what happened, not that a forge -/// webhook arrived. The controller interprets a delivery and decides this is -/// what it means; a hive that receives it does not need to know a forge exists. -const KNOWLEDGE_LEAF: &str = "knowledge"; - -/// The wildcard standing for "any hive", for a grant that must cover all of -/// them. -/// -/// Exported so the one caller that needs it — the callout responder, building -/// the controller's publish grant — can pass it to [`knowledge`] instead of -/// assembling a wildcard subject itself. That is the whole point: the grant and -/// the published subject come out of **the same function**, so they cannot -/// drift into disagreement the way two literals would. -pub const ANY_HIVE: &str = "*"; - -/// The subject carrying *the knowledge repository changed* for `hive`. -/// -/// Pass [`ANY_HIVE`] to get the wildcard form used by a grant. -#[must_use] -pub fn knowledge(hive: &str) -> String { - format!("{SUBJECT_ROOT}.{hive}.{KNOWLEDGE_LEAF}") -} - -#[cfg(test)] -mod tests { - use super::{ANY_HIVE, SUBJECT_ROOT, knowledge}; - - /// The concrete and wildcard forms must differ in exactly the hive token. - /// - /// Written as a structural comparison rather than by asserting two - /// literals, because two literals is the failure this module exists to - /// prevent: a test that spells the expected subject out by hand passes - /// happily when both it and the code are wrong in the same way. - #[test] - fn the_grant_form_and_the_published_form_differ_only_in_the_hive() { - let concrete = knowledge("alpha"); - let wildcard = knowledge(ANY_HIVE); - - assert_eq!( - concrete.replacen("alpha", ANY_HIVE, 1), - wildcard, - "substituting the hive token must turn one form into the other" - ); - } - - /// A NATS wildcard matches one token, so the hive must occupy exactly one. - /// A hive name with a dot in it would silently widen the grant. - #[test] - fn the_hive_occupies_exactly_one_subject_token() { - let root_tokens = SUBJECT_ROOT.split('.').count(); - assert_eq!( - knowledge("alpha").split('.').count(), - root_tokens + 2, - "root + hive + leaf; anything else means the hive is not one token" - ); - } - - /// The event is addressed per hive — one hive's subject must never be - /// another's. Cheap, and it is the property the callout policy relies on. - #[test] - fn two_hives_get_different_subjects() { - assert_ne!(knowledge("alpha"), knowledge("beta")); - } -} diff --git a/swarm-queue-client/src/knowledge.rs b/swarm-queue-client/src/knowledge.rs new file mode 100644 index 00000000..a62e2977 --- /dev/null +++ b/swarm-queue-client/src/knowledge.rs @@ -0,0 +1,46 @@ +//! The subject carrying *the knowledge repository changed*. +//! +//! One writer, many readers: the swarm controller publishes, every hive +//! subscribes. The controller does not need to know who the hives are to tell +//! them the repository moved — core NATS fans one publish out to whoever is +//! listening. +//! +//! # Why one subject rather than one per hive +//! +//! The payload is empty and means the same thing to every hive, so per-hive +//! addressing delivers exactly what one subject does, having first made the +//! publisher enumerate the roster and the grant carry a wildcard. Delivery is +//! at-most-once either way, and a hive that was down reconciles on its next +//! periodic pull — a missed event costs latency, not correctness. +//! +//! ⚠️ The per-hive shape was justified by the callout policy's rule that an +//! extra subject must contain `{hive}`. That rule governs what a *hive* may +//! publish; this subject lives in the controller's reader grant, which it does +//! not constrain. The argument came from the wrong half of the permission model. +//! +//! # Why the constant lives here +//! +//! Three consumers name it: the controller publishing, the callout responder +//! granting, and hive-c0re subscribing. A copied literal in the third gives the +//! worst failure of the set — a grant that looks right, a publish that is +//! refused, and, because a NATS denial reaches the client as a timeout, nothing +//! saying so. Unconditional and NATS-type-free for the same reason +//! [`crate::status`]'s bucket name is: the responder speaks neither `jetstream` +//! nor `kv`. + +/// The subject the swarm controller publishes on when the hive-wide knowledge +/// repository has changed. +/// +/// `$SWARM` rather than a bare name: the `$` prefix is NATS' convention for +/// system-ish subjects, keeping it clear of anything an application might +/// choose for itself. +/// +/// Semantic, not transport-shaped — it says what happened, not that a forge +/// webhook arrived. The controller interprets a delivery and decides this is +/// what it meant; a hive receiving it does not need to know a forge exists. +/// +/// No unit test here: a single constant has no structure to assert, and a test +/// comparing it to a second spelling is the failure this module exists to +/// prevent. The property worth testing is *who may publish it*, which lives +/// with the policy that decides — see `swarm-nats-auth`. +pub const SUBJECT: &str = "$SWARM.knowledge"; diff --git a/swarm-queue-client/src/lib.rs b/swarm-queue-client/src/lib.rs index 0a0fc7ba..0213373e 100644 --- a/swarm-queue-client/src/lib.rs +++ b/swarm-queue-client/src/lib.rs @@ -156,14 +156,18 @@ pub fn chain(error: &dyn std::error::Error) -> String { /// which is the disagreement this module exists to prevent. pub mod status; -/// Swarm event subjects — the names the controller publishes on and hives -/// subscribe to. +/// The *knowledge repository changed* subject — one writer (the controller), +/// many readers (the hives). +/// +/// Its own category rather than a leaf under a general event namespace: a +/// namespace built for events that do not exist yet is a shape decided before +/// there is anything to decide it from. /// /// Unconditional and NATS-type-free for the same reason the bucket name above -/// is: three crates must agree on these strings, and the one that agrees -/// hardest — the auth-callout responder, which decides whether a publish is -/// even permitted — speaks neither `jetstream` nor `kv`. -pub mod events; +/// is: three crates must agree on this string, and the one that agrees hardest +/// — the auth-callout responder, which decides whether a publish is even +/// permitted — speaks neither `jetstream` nor `kv`. +pub mod knowledge; /// Only the fields this needs; authelia returns several. #[derive(serde::Deserialize)]