From 22659234c48b876ff3e9f611ce67d8ed5c1cbf1e Mon Sep 17 00:00:00 2001 From: atlas Date: Sat, 15 Aug 2026 22:26:21 +0200 Subject: [PATCH] refactor(swarm-queue-client): share the hive-status bucket's name and shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bucket has two ends in two crates: a hive writes its own key, the controller reads every key. `swarm-controller` declared the name as a private const with a doc comment arguing that "reader and writer must name the same bucket" — an argument the writer, in another crate, could not obey. The name is the mild half. Both ends do get-or-create, because either may come up first on a fresh swarm and neither can assume the other has run. Two `Config`s that drift means whichever end created the bucket wins and the other's `get_key_value` succeeds against a bucket it did not ask for: no error, no log, just a retention policy nobody chose. Sharing the constructor gives that race one outcome. Behind a default-off `kv` feature, so the crate's other consumer — the auth-callout responder, which speaks the connect and nothing else — still pulls neither `jetstream` nor `kv`. That was the actual reason the feature was excluded when this crate was extracted; the flag preserves it. The surface is deliberately narrow: one bucket's name and creation config, not a general KV facade. --- swarm-controller/Cargo.toml | 6 ++- swarm-controller/src/status.rs | 41 ++++---------------- swarm-queue-client/Cargo.toml | 20 ++++++++-- swarm-queue-client/README.md | 23 ++++++++++-- swarm-queue-client/src/lib.rs | 6 +++ swarm-queue-client/src/status.rs | 64 ++++++++++++++++++++++++++++++++ 6 files changed, 119 insertions(+), 41 deletions(-) create mode 100644 swarm-queue-client/src/status.rs diff --git a/swarm-controller/Cargo.toml b/swarm-controller/Cargo.toml index c5012025..8448176c 100644 --- a/swarm-controller/Cargo.toml +++ b/swarm-controller/Cargo.toml @@ -24,7 +24,11 @@ serde_json.workspace = true # every other participant - a hive publishing its own status runs the same # code with a different client id. Two copies of credential handling is one # token-refresh fix that has to be found twice. -swarm-queue-client.workspace = true +# +# `kv` for the same reason one level in: the status bucket's name and +# creation config are shared with the hive that writes it, so this end does +# not get to declare them privately. +swarm-queue-client = { workspace = true, features = ["kv"] } tokio.workspace = true tracing.workspace = true tracing-subscriber.workspace = true diff --git a/swarm-controller/src/status.rs b/swarm-controller/src/status.rs index b7d322aa..2911752c 100644 --- a/swarm-controller/src/status.rs +++ b/swarm-controller/src/status.rs @@ -38,13 +38,6 @@ use utoipa::ToSchema; use crate::HiveEntry; -/// The KV bucket hives publish their snapshots into. -/// -/// A constant and not an option: reader and writer must name the same -/// bucket, and an option is a way for two deployments to disagree about -/// which one that is. Nothing about a bucket name is site-specific. -pub const BUCKET: &str = "hive-status"; - /// Default age past which a snapshot is reported stale. /// /// A threshold is a statement about how often hives offer, and that @@ -155,34 +148,16 @@ impl StatusReader { /// The bucket handle, created on first use if nothing has made it yet. /// - /// Whichever side arrives first creates it, and both sides want the - /// same shape, so this is a race with one outcome. `history: 1` is - /// the shape: the aggregate reads *the last thing each hive said*, - /// and retaining more would be storage bought for a query nobody - /// makes. + /// The name and the creation config come from + /// [`swarm_queue_client::status`] rather than from here: the hive that + /// writes this bucket opens it with the same call, and a bucket both + /// ends may create is one both ends have to describe identically. + /// + /// Whichever side arrives first creates it, and both sides ask for the + /// same shape, so this is a race with one outcome. async fn store(&self) -> Result<&async_nats::jetstream::kv::Store> { self.store - .get_or_try_init(|| async { - let js = async_nats::jetstream::new(self.client.clone()); - match js.get_key_value(BUCKET).await { - Ok(store) => Ok(store), - Err(e) => { - tracing::info!( - bucket = BUCKET, - reason = %e, - "status bucket not available, creating it" - ); - js.create_key_value(async_nats::jetstream::kv::Config { - bucket: BUCKET.to_owned(), - description: "Last status snapshot offered by each hive".to_owned(), - history: 1, - ..Default::default() - }) - .await - .with_context(|| format!("creating the {BUCKET} bucket")) - } - } - }) + .get_or_try_init(|| swarm_queue_client::status::open_or_create(&self.client)) .await } diff --git a/swarm-queue-client/Cargo.toml b/swarm-queue-client/Cargo.toml index 32a2d0e5..0c25927d 100644 --- a/swarm-queue-client/Cargo.toml +++ b/swarm-queue-client/Cargo.toml @@ -4,11 +4,23 @@ version.workspace = true readme = "README.md" edition.workspace = true +[features] +# OFF by default, and that default is the point: the auth-callout responder +# consumes this crate for the connect alone and speaks neither `jetstream` +# nor `kv`. A consumer that needs the status bucket says so in its own +# Cargo.toml, so the requirement stays visible where it is incurred. +# +# What is behind the flag is deliberately narrow - the *name and shape* of +# one bucket two crates open from opposite ends (`src/status.rs`), not a +# general "KV support" surface. The crate's job still ends at a connected +# client; the exception exists because an agreement between two crates has +# to live in one of them, and neither end of that bucket is senior to the +# other. +kv = ["async-nats/kv"] + [dependencies] -# No `kv`/`jetstream` feature here on purpose: this crate's job ends at a -# connected client. What a consumer does with it - KV for the controller and -# the hive, plain messaging for anything later - is the consumer's business, -# and its Cargo.toml is where that requirement should be visible. +# Bare (no `kv`/`jetstream`) unless a consumer opts into the `kv` feature +# above - the connect itself needs none of them. async-nats.workspace = true reqwest.workspace = true serde.workspace = true diff --git a/swarm-queue-client/README.md b/swarm-queue-client/README.md index 79230794..07a75129 100644 --- a/swarm-queue-client/README.md +++ b/swarm-queue-client/README.md @@ -50,6 +50,23 @@ effect actually did. ## What this crate does not do -It ends at a connected client. No `jetstream`/`kv` feature is enabled here — -what a consumer does with the connection is its own business, and its -`Cargo.toml` is where that requirement should be visible. +It ends at a connected client. `jetstream`/`kv` are **off by default** — what a +consumer does with the connection is its own business, and its `Cargo.toml` is +where that requirement should be visible. The auth-callout responder speaks the +connect and nothing else, and pays for nothing else. + +## The one exception: the `kv` feature + +`kv` adds `status`, which holds the name and the creation config of the +`hive-status` bucket — nothing more. + +It is here because that bucket has **two ends in two crates**: a hive writes its +own key, the controller reads every key. The name being a repeated literal is +the mild half of the problem; the sharp half is that either end may arrive first +on a fresh swarm, so both create the bucket if it is missing. Two `Config`s that +drift means whichever end created it wins and the other opens a bucket it did +not ask for — no error, no log, just a retention policy nobody chose. + +An agreement between two crates has to live in one of them, and neither end of +this bucket is senior to the other. Behind a default-off feature, the consumer +that needs none of it still pays nothing. diff --git a/swarm-queue-client/src/lib.rs b/swarm-queue-client/src/lib.rs index 94a4af0d..16f6f51a 100644 --- a/swarm-queue-client/src/lib.rs +++ b/swarm-queue-client/src/lib.rs @@ -112,6 +112,12 @@ pub fn chain(error: &dyn std::error::Error) -> String { rendered } +/// The hive-status KV bucket, shared by the hive that writes it and the +/// controller that reads it. Behind the `kv` feature — see the module doc +/// for why a bucket name and its config belong to neither end alone. +#[cfg(feature = "kv")] +pub mod status; + /// Only the one field this needs; authelia returns several. #[derive(serde::Deserialize)] struct TokenResponse { diff --git a/swarm-queue-client/src/status.rs b/swarm-queue-client/src/status.rs new file mode 100644 index 00000000..0c528cf0 --- /dev/null +++ b/swarm-queue-client/src/status.rs @@ -0,0 +1,64 @@ +//! The hive-status KV bucket: its name, and the shape it is created with. +//! +//! Two processes touch this bucket from opposite ends — a hive writes its +//! own key, the swarm controller reads every key — and they live in +//! different crates. That is the whole reason this module exists rather +//! than a `const` on each side: **the two ends must agree, and a literal +//! repeated across crates is an agreement nothing checks.** +//! +//! The name is the obvious half. The sharper half is the *config*: both +//! ends open the bucket with [`open_or_create`], because either may +//! arrive first on a fresh swarm and neither can assume the other has +//! run. If the two ends passed different `Config`s, whichever created it +//! would win and the other's `get_key_value` would succeed against a +//! bucket it did not ask for — no error, no log, just a retention policy +//! nobody chose. Sharing the constructor makes the race have one outcome +//! instead of two. +//! +//! Feature-gated (`kv`) so the crate's other consumer, the auth-callout +//! responder, still pulls neither `jetstream` nor `kv`: it speaks the +//! connect and nothing else. + +use anyhow::{Context, Result}; + +/// The KV bucket hives publish their status snapshots into, one key per +/// hive keyed by `hiveName`. +/// +/// A constant and not an option: reader and writer must name the same +/// bucket, and an option is a way for two deployments to disagree about +/// which one that is. Nothing about a bucket name is site-specific. +pub const BUCKET: &str = "hive-status"; + +/// Open the status bucket, creating it if nothing has yet. +/// +/// `history: 1` is the shape: every consumer reads *the last thing each +/// hive said*, and retaining more would be storage bought for a query +/// nobody makes. +/// +/// Creating rather than requiring a provisioning step is deliberate — the +/// controller and the hives come up in no particular order, and a bucket +/// that must pre-exist turns "the swarm was deployed in the wrong order" +/// into a permanent, silent absence of data. +pub async fn open_or_create( + client: &async_nats::Client, +) -> Result { + let js = async_nats::jetstream::new(client.clone()); + match js.get_key_value(BUCKET).await { + Ok(store) => Ok(store), + Err(e) => { + tracing::info!( + bucket = BUCKET, + reason = %e, + "status bucket not available, creating it" + ); + js.create_key_value(async_nats::jetstream::kv::Config { + bucket: BUCKET.to_owned(), + description: "Last status snapshot offered by each hive".to_owned(), + history: 1, + ..Default::default() + }) + .await + .with_context(|| format!("creating the {BUCKET} bucket")) + } + } +}