refactor(#4006): one wanted-state bucket per hive, so a watch can be scoped
A hive reads its own declaration today and that scopes cleanly: DIRECT.GET carries the key in the subject, so the grant can name it. A *watch* cannot be scoped that way — a consumer's filter travels in the request payload, so $JS.API.CONSUMER.CREATE.<stream> grants the whole stream. With every hive in one bucket, letting a hive watch its own declaration would let it read every other hive's. One bucket per hive (hive-wanted-<hive>) makes the stream a hive may hold exactly as wide as what it is allowed to see, which is what #4006's live-watch needs. That watch is a separate change; this only moves the boundary. mara's calls, both on #4006: one stream per hive rather than teaching the auth responder a hive roster, and a wildcard for the controller — "its okay if swarm controller can theoretically override hive". A bucket name is a single subject token with no prefix matching, so no wildcard narrower than * covers N per-hive buckets; the controller's grant is account-wide by consequence, and documented as chosen rather than left to look accidental. The reader arm of #4005's key-layout guard asserted the opposite of that ruling, so it is replaced rather than deleted: the hive arm survives as no_hive_may_write_another_role_s_agent_status (with a positive control), and the_readers_grant_is_deliberately_account_wide pins the decision and names the ruling, so the width reads as chosen to whoever finds it next. Two pre-existing negative assertions were silently defanged by the rename -- they matched hive-wanted.beta and $KV.hive-wanted.alpha, strings nothing produces any more, and kept passing. Both now match current names. swarm-controller resolves the store per hive per call instead of caching one in a OnceCell: there is no single handle that serves N buckets, and declarations change on operator action rather than per tick.
This commit is contained in:
parent
8c94e340b8
commit
78637ded0c
6 changed files with 111 additions and 66 deletions
|
|
@ -63,7 +63,7 @@ pub async fn pull(coord: &Arc<Coordinator>) -> Result<()> {
|
||||||
// A hung node is worse than a failed one: the dashboard shows it running.
|
// A hung node is worse than a failed one: the dashboard shows it running.
|
||||||
swarm_queue_client::ensure_connected(&client)?;
|
swarm_queue_client::ensure_connected(&client)?;
|
||||||
|
|
||||||
let Some(store) = swarm_queue_client::wanted::open_read_only(&client).await else {
|
let Some(store) = swarm_queue_client::wanted::open_read_only(&client, &hive).await else {
|
||||||
tracing::info!(%hive, "wanted state: no bucket yet; nothing has been declared");
|
tracing::info!(%hive, "wanted state: no bucket yet; nothing has been declared");
|
||||||
return Ok(());
|
return Ok(());
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -57,29 +57,31 @@ fn apply(current: Option<&[u8]>, agent: &str, state: AgentState) -> Result<(Hive
|
||||||
/// Writes the wanted-state bucket, and reads it back.
|
/// Writes the wanted-state bucket, and reads it back.
|
||||||
pub struct WantedWriter {
|
pub struct WantedWriter {
|
||||||
client: async_nats::Client,
|
client: async_nats::Client,
|
||||||
store: tokio::sync::OnceCell<async_nats::jetstream::kv::Store>,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl WantedWriter {
|
impl WantedWriter {
|
||||||
#[must_use]
|
#[must_use]
|
||||||
pub fn new(client: async_nats::Client) -> Self {
|
pub fn new(client: async_nats::Client) -> Self {
|
||||||
Self {
|
Self { client }
|
||||||
client,
|
|
||||||
store: tokio::sync::OnceCell::new(),
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The bucket handle, created on first use if nothing has made it yet.
|
/// One hive's bucket handle, created on first use if nothing has made it
|
||||||
|
/// yet.
|
||||||
///
|
///
|
||||||
/// Creation lives in [`swarm_queue_client::wanted`] because a bucket is
|
/// Creation lives in [`swarm_queue_client::wanted`] because a bucket is
|
||||||
/// described identically by everyone who may create it. Only the
|
/// described identically by everyone who may create it. Only the
|
||||||
/// controller creates this one; a hive opens it read-only.
|
/// controller creates these; a hive opens its own read-only.
|
||||||
|
///
|
||||||
|
/// Resolved per call rather than cached: there is one bucket per hive, so
|
||||||
|
/// a single cached handle cannot serve them, and the declarations this
|
||||||
|
/// writes change on operator action rather than on a loop — the extra
|
||||||
|
/// lookup is per *declaration*, not per tick. A cache here would be a map
|
||||||
|
/// whose invalidation nobody needs yet.
|
||||||
async fn store(
|
async fn store(
|
||||||
&self,
|
&self,
|
||||||
) -> std::result::Result<&async_nats::jetstream::kv::Store, swarm_queue_client::Error> {
|
hive: &str,
|
||||||
self.store
|
) -> std::result::Result<async_nats::jetstream::kv::Store, swarm_queue_client::Error> {
|
||||||
.get_or_try_init(|| swarm_queue_client::wanted::open_or_create(&self.client))
|
swarm_queue_client::wanted::open_or_create(&self.client, hive).await
|
||||||
.await
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The declaration currently published for `hive`, or `None`.
|
/// The declaration currently published for `hive`, or `None`.
|
||||||
|
|
@ -87,7 +89,7 @@ impl WantedWriter {
|
||||||
// An unconnected client does not fail a JetStream request, it hangs
|
// An unconnected client does not fail a JetStream request, it hangs
|
||||||
// on it — see `swarm_queue_client::ensure_connected`.
|
// on it — see `swarm_queue_client::ensure_connected`.
|
||||||
swarm_queue_client::ensure_connected(&self.client)?;
|
swarm_queue_client::ensure_connected(&self.client)?;
|
||||||
let store = self.store().await?;
|
let store = self.store(hive).await?;
|
||||||
let Some(entry) = store
|
let Some(entry) = store
|
||||||
.entry(hive)
|
.entry(hive)
|
||||||
.await
|
.await
|
||||||
|
|
@ -110,7 +112,7 @@ impl WantedWriter {
|
||||||
/// fact — the conflict has to be caught here.
|
/// fact — the conflict has to be caught here.
|
||||||
pub async fn set(&self, hive: &str, agent: &str, state: AgentState) -> Result<HiveWanted> {
|
pub async fn set(&self, hive: &str, agent: &str, state: AgentState) -> Result<HiveWanted> {
|
||||||
swarm_queue_client::ensure_connected(&self.client)?;
|
swarm_queue_client::ensure_connected(&self.client)?;
|
||||||
let store = self.store().await?;
|
let store = self.store(hive).await?;
|
||||||
|
|
||||||
for _ in 0..MAX_ATTEMPTS {
|
for _ in 0..MAX_ATTEMPTS {
|
||||||
let entry = store
|
let entry = store
|
||||||
|
|
|
||||||
|
|
@ -138,14 +138,14 @@ impl Policy {
|
||||||
format!("KV_{}", self.bucket)
|
format!("KV_{}", self.bucket)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The stream backing the wanted-state bucket.
|
/// The stream backing one hive's wanted-state bucket.
|
||||||
///
|
///
|
||||||
/// Derived from the crate constant rather than from a second configurable
|
/// Derived from the crate's own name function rather than from a
|
||||||
/// bucket name: `wanted::BUCKET` is a `const` precisely so writer and
|
/// configurable bucket name: `wanted::bucket` is what the writer and the
|
||||||
/// reader cannot disagree about it, and a flag here would reintroduce the
|
/// reader both call, so a flag here would let two deployments disagree
|
||||||
/// disagreement one layer out.
|
/// about a name they must share.
|
||||||
fn wanted_stream() -> String {
|
fn wanted_stream(hive: &str) -> String {
|
||||||
format!("KV_{}", swarm_queue_client::wanted::BUCKET)
|
format!("KV_{}", swarm_queue_client::wanted::bucket(hive))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The stream backing the per-agent status bucket.
|
/// The stream backing the per-agent status bucket.
|
||||||
|
|
@ -256,19 +256,22 @@ impl Policy {
|
||||||
// pointing at a subject nobody publishes to. Not `extra_hive_subjects`:
|
// pointing at a subject nobody publishes to. Not `extra_hive_subjects`:
|
||||||
// that is for streams this crate does not know about.
|
// that is for streams this crate does not know about.
|
||||||
subjects.push(swarm_queue_client::notices::subject(hive));
|
subjects.push(swarm_queue_client::notices::subject(hive));
|
||||||
// Reading the wanted-state bucket, and **only this hive's key**.
|
// Reading the wanted-state bucket, which is **this hive's own**.
|
||||||
//
|
//
|
||||||
// A KV read is a publish: `store.get` is a request, and the direct-get
|
// One bucket per hive rather than one keyed by hive, so that a *watch*
|
||||||
// subject carries the key, so the grant scopes to one hive rather than
|
// can be granted without widening the read: a consumer's filter travels
|
||||||
// to the bucket. `$KV.<wanted>.<hive>` is deliberately absent — the
|
// in the request payload, so `CONSUMER.CREATE` grants a whole stream and
|
||||||
// controller declares this and a hive converges to it, so a hive that
|
// cannot be scoped to a key the way `DIRECT.GET` can.
|
||||||
// could write its own key could declare its own desired state.
|
//
|
||||||
|
// `$KV.<bucket>.<hive>` stays deliberately absent — the controller
|
||||||
|
// declares this and a hive converges to it, so a hive that could write
|
||||||
|
// its own key could declare its own desired state.
|
||||||
|
let wanted = Self::wanted_stream(hive);
|
||||||
subjects.extend([
|
subjects.extend([
|
||||||
format!("$JS.API.STREAM.INFO.{}", Self::wanted_stream()),
|
format!("$JS.API.STREAM.INFO.{wanted}"),
|
||||||
format!(
|
format!(
|
||||||
"$JS.API.DIRECT.GET.{}.$KV.{}.{hive}",
|
"$JS.API.DIRECT.GET.{wanted}.$KV.{}.{hive}",
|
||||||
Self::wanted_stream(),
|
swarm_queue_client::wanted::bucket(hive)
|
||||||
swarm_queue_client::wanted::BUCKET
|
|
||||||
),
|
),
|
||||||
]);
|
]);
|
||||||
// Publishing this hive's agents into the per-agent status bucket, and
|
// Publishing this hive's agents into the per-agent status bucket, and
|
||||||
|
|
@ -349,13 +352,25 @@ impl Policy {
|
||||||
// admission). Same failure mode as the knowledge event above: a
|
// admission). Same failure mode as the knowledge event above: a
|
||||||
// refused publish reaches the client as a timeout.
|
// refused publish reaches the client as a timeout.
|
||||||
swarm_queue_client::DEPLOY_SUBJECT_WILDCARD.to_owned(),
|
swarm_queue_client::DEPLOY_SUBJECT_WILDCARD.to_owned(),
|
||||||
// The wanted-state bucket, which the controller creates and writes
|
// The wanted-state buckets — one per hive, all created and written
|
||||||
// for every hive. `.>` here against one key per hive above: this is
|
// by this single client.
|
||||||
// the single writer, so scoping it per hive would be a list of
|
//
|
||||||
// hives to keep, which `hive_name`'s doc argues against.
|
// Wildcards rather than a name per hive: a bucket name is one
|
||||||
format!("$JS.API.STREAM.INFO.{}", Self::wanted_stream()),
|
// subject token and subjects have no prefix matching, so
|
||||||
format!("$JS.API.STREAM.CREATE.{}", Self::wanted_stream()),
|
// `KV_hive-wanted-*` is not expressible. The choice is an exact
|
||||||
format!("$KV.{}.>", swarm_queue_client::wanted::BUCKET),
|
// name per hive — which needs a hive roster in this responder,
|
||||||
|
// which `hive_name`'s doc argues against — or `*`.
|
||||||
|
//
|
||||||
|
// ⚠️ `*` therefore reaches every stream in the account, including
|
||||||
|
// the buckets hives publish themselves, so this client *can*
|
||||||
|
// overwrite a hive's self-reported status. That is an accepted
|
||||||
|
// operator decision, not an oversight: the controller is the
|
||||||
|
// swarm's writer of record, and a swarm that cannot trust it has a
|
||||||
|
// larger problem than this grant.
|
||||||
|
"$JS.API.STREAM.INFO.*".to_owned(),
|
||||||
|
"$JS.API.STREAM.CREATE.*".to_owned(),
|
||||||
|
"$JS.API.DIRECT.GET.*.>".to_owned(),
|
||||||
|
"$KV.*.>".to_owned(),
|
||||||
]);
|
]);
|
||||||
// The per-agent status bucket, read-side only. Same five subjects as
|
// The per-agent status bucket, read-side only. Same five subjects as
|
||||||
// the hive-status block above and for the same measured reasons —
|
// the hive-status block above and for the same measured reasons —
|
||||||
|
|
@ -414,23 +429,38 @@ mod tests {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn the_reader_gets_no_write_subject_for_the_agent_status_bucket() {
|
fn no_hive_may_write_another_role_s_agent_status() {
|
||||||
// The controller reads this bucket and never writes it. A `$KV.`
|
// A hive publishes its *own* agents' status under
|
||||||
// subject here would let a reader forge any agent's status on any
|
// `$KV.agent-status.<hive>.*` and must reach no further. This is the
|
||||||
// hive — the containment `hive_subjects` establishes is only worth
|
// arm of the old reader-side guard that still holds.
|
||||||
// anything if the other role cannot bypass it.
|
|
||||||
let bucket = swarm_queue_client::agent_status::BUCKET;
|
let bucket = swarm_queue_client::agent_status::BUCKET;
|
||||||
let prefix = format!("$KV.{bucket}.");
|
let subjects = policy().hive_subjects("alpha");
|
||||||
|
for forbidden in [
|
||||||
|
format!("$KV.{bucket}.>"),
|
||||||
|
format!("$KV.{bucket}.*"),
|
||||||
|
format!("$KV.{bucket}.beta.*"),
|
||||||
|
] {
|
||||||
|
assert!(!subjects.contains(&forbidden), "hive alpha got {forbidden}");
|
||||||
|
}
|
||||||
|
// Control: the containment being asserted is not vacuous — the hive
|
||||||
|
// does hold its own slice.
|
||||||
|
assert!(subjects.contains(&format!("$KV.{bucket}.alpha.*")));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_readers_grant_is_deliberately_account_wide() {
|
||||||
|
// ⚠️ This pins a DECISION, not a safety property, and it replaced a
|
||||||
|
// guard that asserted the opposite — the operator's call was that the
|
||||||
|
// controller being able to override a hive is acceptable.
|
||||||
|
//
|
||||||
|
// A bucket name is one subject token and subjects have no prefix
|
||||||
|
// matching, so per-hive wanted-state buckets cannot be covered by any
|
||||||
|
// wildcard narrower than `*`. Scoping the controller would mean a hive
|
||||||
|
// roster inside this responder. If that trade is ever revisited, this
|
||||||
|
// test is the thing to change — it exists so the width reads as chosen.
|
||||||
let subjects = policy().reader_subjects();
|
let subjects = policy().reader_subjects();
|
||||||
assert!(
|
assert!(subjects.contains(&"$KV.*.>".to_owned()));
|
||||||
!subjects.iter().any(|s| s.starts_with(&prefix)),
|
assert!(subjects.contains(&"$JS.API.STREAM.INFO.*".to_owned()));
|
||||||
"the reader was granted a {prefix}* subject"
|
|
||||||
);
|
|
||||||
// Control: the prefix test does fire on a bucket the reader
|
|
||||||
// legitimately writes, so the pass above is about agent-status rather
|
|
||||||
// than about a matcher that never matches anything.
|
|
||||||
let wanted = format!("$KV.{}.", swarm_queue_client::wanted::BUCKET);
|
|
||||||
assert!(subjects.iter().any(|s| s.starts_with(&wanted)));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|
@ -736,8 +766,9 @@ mod tests {
|
||||||
.permissions("hive-alpha")
|
.permissions("hive-alpha")
|
||||||
.expect("a hive is admitted");
|
.expect("a hive is admitted");
|
||||||
assert!(
|
assert!(
|
||||||
p.publish
|
p.publish.contains(
|
||||||
.contains(&"$JS.API.DIRECT.GET.KV_hive-wanted.$KV.hive-wanted.alpha".to_owned()),
|
&"$JS.API.DIRECT.GET.KV_hive-wanted-alpha.$KV.hive-wanted-alpha.alpha".to_owned()
|
||||||
|
),
|
||||||
"got: {:?}",
|
"got: {:?}",
|
||||||
p.publish
|
p.publish
|
||||||
);
|
);
|
||||||
|
|
@ -749,7 +780,12 @@ mod tests {
|
||||||
let p = policy()
|
let p = policy()
|
||||||
.permissions("hive-alpha")
|
.permissions("hive-alpha")
|
||||||
.expect("a hive is admitted");
|
.expect("a hive is admitted");
|
||||||
assert!(!p.publish.iter().any(|s| s.contains("hive-wanted.beta")));
|
// ⚠️ Matched against the CURRENT bucket name. The per-hive split moved
|
||||||
|
// this from `hive-wanted.beta` to `hive-wanted-beta`, and the old
|
||||||
|
// spelling kept passing against a subject set that no longer contains
|
||||||
|
// it — a pass that had stopped meaning anything.
|
||||||
|
assert!(!p.publish.iter().any(|s| s.contains("hive-wanted-beta")));
|
||||||
|
assert!(!p.publish.iter().any(|s| s.contains("beta")));
|
||||||
assert!(
|
assert!(
|
||||||
!p.publish
|
!p.publish
|
||||||
.iter()
|
.iter()
|
||||||
|
|
@ -764,8 +800,12 @@ mod tests {
|
||||||
let p = policy()
|
let p = policy()
|
||||||
.permissions("hive-alpha")
|
.permissions("hive-alpha")
|
||||||
.expect("a hive is admitted");
|
.expect("a hive is admitted");
|
||||||
assert!(!p.publish.iter().any(|s| s == "$KV.hive-wanted.alpha"));
|
// Current names, for the reason the sibling test above records: the
|
||||||
assert!(!p.publish.iter().any(|s| s == "$KV.hive-wanted.>"));
|
// pre-split strings `$KV.hive-wanted.{alpha,>}` still "passed" after
|
||||||
|
// the rename because nothing produces them any more.
|
||||||
|
assert!(!p.publish.iter().any(|s| s == "$KV.hive-wanted-alpha.alpha"));
|
||||||
|
assert!(!p.publish.iter().any(|s| s == "$KV.hive-wanted-alpha.>"));
|
||||||
|
assert!(!p.publish.iter().any(|s| s.starts_with("$KV.hive-wanted")));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|
@ -775,11 +815,11 @@ mod tests {
|
||||||
let p = policy()
|
let p = policy()
|
||||||
.permissions("swarm-controller")
|
.permissions("swarm-controller")
|
||||||
.expect("a reader is admitted");
|
.expect("a reader is admitted");
|
||||||
assert!(p.publish.contains(&"$KV.hive-wanted.>".to_owned()));
|
// Account-wide since the per-hive split: a bucket name is one subject
|
||||||
assert!(
|
// token, so no wildcard narrower than `*` covers N per-hive buckets.
|
||||||
p.publish
|
// See `the_readers_grant_is_deliberately_account_wide`.
|
||||||
.contains(&"$JS.API.STREAM.CREATE.KV_hive-wanted".to_owned())
|
assert!(p.publish.contains(&"$KV.*.>".to_owned()));
|
||||||
);
|
assert!(p.publish.contains(&"$JS.API.STREAM.CREATE.*".to_owned()));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|
|
||||||
|
|
@ -125,7 +125,7 @@ pub async fn open_or_create(
|
||||||
})
|
})
|
||||||
.await
|
.await
|
||||||
.map_err(|source| Error::CreateBucket {
|
.map_err(|source| Error::CreateBucket {
|
||||||
bucket: BUCKET,
|
bucket: BUCKET.to_owned(),
|
||||||
source,
|
source,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -67,7 +67,7 @@ pub async fn open_or_create(
|
||||||
})
|
})
|
||||||
.await
|
.await
|
||||||
.map_err(|source| Error::CreateBucket {
|
.map_err(|source| Error::CreateBucket {
|
||||||
bucket: BUCKET,
|
bucket: BUCKET.to_owned(),
|
||||||
source,
|
source,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -170,7 +170,10 @@ mod tests {
|
||||||
// the grant scoping in `swarm-nats-auth` names this string, so a bucket
|
// the grant scoping in `swarm-nats-auth` names this string, so a bucket
|
||||||
// the client refuses to open would surface as a permissions problem
|
// the client refuses to open would surface as a permissions problem
|
||||||
// rather than as the naming problem it is.
|
// rather than as the naming problem it is.
|
||||||
let legal = |s: &str| s.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-');
|
let legal = |s: &str| {
|
||||||
|
s.chars()
|
||||||
|
.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
|
||||||
|
};
|
||||||
assert!(legal(&bucket("alpha")));
|
assert!(legal(&bucket("alpha")));
|
||||||
assert!(legal(&bucket("a-hive-with-hyphens")));
|
assert!(legal(&bucket("a-hive-with-hyphens")));
|
||||||
assert!(legal(&bucket("h9")));
|
assert!(legal(&bucket("h9")));
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue