refactor(#3255): one knowledge subject, single writer and many readers
Review call: the event was addressed per hive — `$SWARM.events.<hive>.knowledge`, published in a loop over the roster, granted through a wildcard. It does not need to be. The payload is empty and the event means the same thing to every hive, so one publish to one subject delivers exactly what N publishes to N subjects did, and core NATS already fans out to whoever is subscribed. A hive that was down misses it either way and reconciles on its next periodic pull. That deletes rather than reshuffles: the roster loop, the wildcard, and the shared subject-building function whose entire purpose was keeping the grant and the publish from drifting apart. With one literal there is nothing to disagree about. The per-hive shape was justified by the callout policy's rule that an extra subject must contain the hive name. That rule governs `extra_hive_subjects` — what a HIVE may publish. This subject lives in the controller's reader grant, which the rule does not constrain, so a real rule was carried across into a decision it had no authority over. Knowledge becomes its own category rather than a leaf under a general event namespace, since a namespace shaped for events that do not exist yet is a decision made before there is anything to decide from. The empty config-PR match arm goes with it: an arm with no body claims this is where the deploy path is handled, and it is not. The deny test stays and matters more, not less: with one shared subject a forged event would reach the whole swarm where a per-hive one reached a single hive.
This commit is contained in:
parent
84125b0806
commit
9b939f4626
6 changed files with 104 additions and 156 deletions
|
|
@ -135,12 +135,7 @@ pub fn spawn(
|
||||||
// paths independent reconnect state, so one could be serving while
|
// paths independent reconnect state, so one could be serving while
|
||||||
// the other was still down. `async_nats::Client` is a handle, so the
|
// the other was still down. `async_nats::Client` is a handle, so the
|
||||||
// clone is cheap.
|
// clone is cheap.
|
||||||
tokio::spawn(drain_swarm_events(
|
tokio::spawn(drain_swarm_events(client.clone(), coord, shutdown.clone()));
|
||||||
client.clone(),
|
|
||||||
hive.clone(),
|
|
||||||
coord,
|
|
||||||
shutdown.clone(),
|
|
||||||
));
|
|
||||||
|
|
||||||
let mut health = SweepHealth::new("swarm_status_publish", "warn", FAILURES_BEFORE_BANNER);
|
let mut health = SweepHealth::new("swarm_status_publish", "warn", FAILURES_BEFORE_BANNER);
|
||||||
loop {
|
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
|
/// 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
|
/// 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.
|
/// changes, the server log is the thing that knows why — nothing here will say.
|
||||||
async fn drain_swarm_events(
|
async fn drain_swarm_events(
|
||||||
client: async_nats::Client,
|
client: async_nats::Client,
|
||||||
hive: String,
|
|
||||||
coord: std::sync::Arc<crate::coordinator::Coordinator>,
|
coord: std::sync::Arc<crate::coordinator::Coordinator>,
|
||||||
mut shutdown: tokio::sync::watch::Receiver<bool>,
|
mut shutdown: tokio::sync::watch::Receiver<bool>,
|
||||||
) {
|
) {
|
||||||
let subject = swarm_queue_client::events::knowledge(&hive);
|
// One subject for the whole swarm, so this hive's own name never enters
|
||||||
let mut sub = match client.subscribe(subject.clone()).await {
|
// 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,
|
Ok(sub) => sub,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
// Warn rather than a boot banner: the hive is fully functional
|
// Warn rather than a boot banner: the hive is fully functional
|
||||||
|
|
|
||||||
|
|
@ -367,14 +367,14 @@ pub(super) async fn post_webhook_forge(
|
||||||
"webhook: verified delivery"
|
"webhook: verified delivery"
|
||||||
);
|
);
|
||||||
|
|
||||||
match kind {
|
// Only the knowledge delivery is acted on. Deploy coordination is a
|
||||||
DeliveryKind::Knowledge => announce_knowledge_change(&state).await,
|
// separate concern with its own issue — a hive does not want to hear that a
|
||||||
// Deploy coordination is a separate concern with its own issue: a
|
// config PR was opened, it wants to be told when to rebuild from main, and
|
||||||
// hive does not want to hear that a config PR was opened, it wants
|
// that is a decision the controller makes after a merge rather than a relay
|
||||||
// to be told when to rebuild from main, and that is a decision the
|
// of this delivery. Written as a condition rather than a match arm holding
|
||||||
// controller makes after a merge rather than a relay of this
|
// an empty body, which would claim this is where that path is handled.
|
||||||
// delivery. Logged above and deliberately not forwarded.
|
if kind == DeliveryKind::Knowledge {
|
||||||
DeliveryKind::ConfigPr => {}
|
announce_knowledge_change(&state).await;
|
||||||
}
|
}
|
||||||
|
|
||||||
(StatusCode::OK, "ok").into_response()
|
(StatusCode::OK, "ok").into_response()
|
||||||
|
|
@ -416,22 +416,23 @@ async fn announce_knowledge_change(state: &AppState) {
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
let client = status.queue_client();
|
let client = status.queue_client();
|
||||||
|
let subject = swarm_queue_client::knowledge::SUBJECT;
|
||||||
|
|
||||||
for hive in state.hives.iter() {
|
// One publish, not one per hive: every subscriber gets the same empty
|
||||||
let subject = swarm_queue_client::events::knowledge(&hive.name);
|
// event, so the roster is not consulted at all. The controller does not
|
||||||
if let Err(e) = client.publish(subject.clone(), Vec::new().into()).await {
|
// need to know who the hives are in order to say the repository moved.
|
||||||
tracing::warn!(
|
if let Err(e) = client.publish(subject, Vec::new().into()).await {
|
||||||
hive = %hive.name, %subject, error = %e,
|
tracing::warn!(%subject, error = %e, "webhook: publishing the knowledge event failed");
|
||||||
"webhook: publishing the knowledge event failed"
|
return;
|
||||||
);
|
|
||||||
} else {
|
|
||||||
tracing::info!(hive = %hive.name, %subject, "webhook: knowledge event published");
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
// 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 {
|
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)]
|
#[cfg(test)]
|
||||||
|
|
|
||||||
|
|
@ -234,18 +234,19 @@ impl Policy {
|
||||||
// stays for a named/durable consumer.
|
// stays for a named/durable consumer.
|
||||||
format!("$JS.API.CONSUMER.CREATE.{stream}"),
|
format!("$JS.API.CONSUMER.CREATE.{stream}"),
|
||||||
format!("$JS.API.CONSUMER.CREATE.{stream}.>"),
|
format!("$JS.API.CONSUMER.CREATE.{stream}.>"),
|
||||||
// Swarm events. The controller is the only publisher, and it
|
// The knowledge event. One writer, many readers: the controller is
|
||||||
// publishes to *every* hive's subject, so the grant takes the
|
// the only publisher and every hive subscribes, so this is one
|
||||||
// wildcard form — from the same function the publisher calls, so a
|
// literal subject rather than a per-hive family — named from the
|
||||||
// rename cannot leave the grant naming a subject nobody uses.
|
// 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
|
// 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
|
// the symptom is unhelpful: a refused publish reaches the client as
|
||||||
// a **timeout**, so the visible failure is a hive that never hears
|
// 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
|
// about a change, with nothing in the controller's log to say a
|
||||||
// permission was the reason.
|
// permission was the reason.
|
||||||
swarm_queue_client::events::knowledge(swarm_queue_client::events::ANY_HIVE),
|
swarm_queue_client::knowledge::SUBJECT.to_owned(),
|
||||||
]);
|
]);
|
||||||
subjects
|
subjects
|
||||||
}
|
}
|
||||||
|
|
@ -308,36 +309,35 @@ mod tests {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[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");
|
let p = policy().permissions("swarm-controller").expect("a reader");
|
||||||
assert!(
|
assert!(
|
||||||
p.publish.contains(&swarm_queue_client::events::knowledge(
|
p.publish
|
||||||
swarm_queue_client::events::ANY_HIVE
|
.contains(&swarm_queue_client::knowledge::SUBJECT.to_owned()),
|
||||||
)),
|
"the controller is the only publisher of this event; without the \
|
||||||
"the controller is the only event publisher; without this its \
|
grant its publish is refused, and a refusal arrives as a timeout"
|
||||||
publish is refused, and a refusal arrives as a timeout"
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[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
|
// The controller *interprets* what a delivery means; a hive receives
|
||||||
// that verdict. A hive that could publish on this subject could tell a
|
// that verdict. A hive able to publish here could tell every other hive
|
||||||
// neighbour — or itself — that the knowledge repo changed when it did
|
// in the swarm — or itself — that the knowledge repo changed when it did
|
||||||
// not, which is an unauthenticated write into someone else's control
|
// not, which is an unauthenticated write into someone else's control
|
||||||
// path wearing an event's shape.
|
// path wearing an event's shape.
|
||||||
//
|
//
|
||||||
// Asserted on the subject ROOT rather than on one rendered subject: a
|
// One writer and many readers makes this arm matter MORE, not less: with
|
||||||
// future event leaf added to this namespace must fail this test too,
|
// a single shared subject a forged event reaches the whole swarm, where
|
||||||
// rather than passing because the test only knew about `knowledge`.
|
// a per-hive subject would have reached one.
|
||||||
let p = policy()
|
let p = policy()
|
||||||
.permissions("hive-alpha")
|
.permissions("hive-alpha")
|
||||||
.expect("a hive is admitted");
|
.expect("a hive is admitted");
|
||||||
assert!(
|
assert!(
|
||||||
!p.publish
|
!p.publish
|
||||||
.iter()
|
.iter()
|
||||||
.any(|s| s.starts_with(swarm_queue_client::events::SUBJECT_ROOT)),
|
.any(|s| s == swarm_queue_client::knowledge::SUBJECT),
|
||||||
"a hive must not publish into the swarm event namespace: {:?}",
|
"a hive must not publish the knowledge event: {:?}",
|
||||||
p.publish
|
p.publish
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -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"));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
46
swarm-queue-client/src/knowledge.rs
Normal file
46
swarm-queue-client/src/knowledge.rs
Normal file
|
|
@ -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";
|
||||||
|
|
@ -156,14 +156,18 @@ pub fn chain(error: &dyn std::error::Error) -> String {
|
||||||
/// which is the disagreement this module exists to prevent.
|
/// which is the disagreement this module exists to prevent.
|
||||||
pub mod status;
|
pub mod status;
|
||||||
|
|
||||||
/// Swarm event subjects — the names the controller publishes on and hives
|
/// The *knowledge repository changed* subject — one writer (the controller),
|
||||||
/// subscribe to.
|
/// 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
|
/// 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
|
/// is: three crates must agree on this string, and the one that agrees hardest
|
||||||
/// hardest — the auth-callout responder, which decides whether a publish is
|
/// — the auth-callout responder, which decides whether a publish is even
|
||||||
/// even permitted — speaks neither `jetstream` nor `kv`.
|
/// permitted — speaks neither `jetstream` nor `kv`.
|
||||||
pub mod events;
|
pub mod knowledge;
|
||||||
|
|
||||||
/// Only the fields this needs; authelia returns several.
|
/// Only the fields this needs; authelia returns several.
|
||||||
#[derive(serde::Deserialize)]
|
#[derive(serde::Deserialize)]
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue