swarm-controller: wire the config-PR webhook, not just the poll
This commit is contained in:
parent
66c494697f
commit
f3e42c93b4
3 changed files with 163 additions and 25 deletions
|
|
@ -1,4 +1,5 @@
|
||||||
//! Swarm-level config-PR status, polled independently of any one hive.
|
//! Swarm-level config-PR status, kept current by both a webhook nudge and a
|
||||||
|
//! periodic poll.
|
||||||
//!
|
//!
|
||||||
//! `hive-c0re::forge::config_pr_poll` already scans `agent-configs/*` for
|
//! `hive-c0re::forge::config_pr_poll` already scans `agent-configs/*` for
|
||||||
//! open PRs — but it does that once per hive, to queue that hive's own
|
//! open PRs — but it does that once per hive, to queue that hive's own
|
||||||
|
|
@ -7,20 +8,56 @@
|
||||||
//! have an open config PR" that does not depend on which hive currently
|
//! have an open config PR" that does not depend on which hive currently
|
||||||
//! hosts X being reachable.
|
//! hosts X being reachable.
|
||||||
//!
|
//!
|
||||||
//! This mirrors `hive-c0re`'s poll shape (period, idempotent full rescan)
|
//! Both paths write the same [`ConfigPrCache`]:
|
||||||
//! rather than consuming the swarm-wide `pull_request` webhook
|
//!
|
||||||
//! (`crate::webhook::DeliveryKind::ConfigPr`) that already lands here: that
|
//! - [`spawn`] — a periodic full rescan, mirroring `hive-c0re`'s own poll
|
||||||
//! delivery is unparsed today, and building a reliable event path means
|
//! shape. The backstop: catches anything a missed delivery loses, and is
|
||||||
//! building this poll as a missed-delivery backstop anyway (the same reason
|
//! what populates the cache before the first delivery ever arrives.
|
||||||
//! `hive-c0re`'s own poller exists). Shipping the backstop alone first
|
//! - [`ConfigPrCache::apply_webhook_delivery`] — called from
|
||||||
//! avoids paying for both at once; wiring the webhook as a low-latency nudge
|
//! `crate::webhook::post_webhook_forge` on a verified `ConfigPr` delivery.
|
||||||
//! on top is a cheap follow-up once this cache is the source of truth.
|
//! The low-latency path: a PR opening or closing shows up immediately
|
||||||
|
//! instead of waiting up to [`POLL_INTERVAL`].
|
||||||
|
//!
|
||||||
|
//! Per mara's review call: ship both from the start rather than the poll
|
||||||
|
//! alone — the eventual swarm-level replacement for `hive-c0re`'s own
|
||||||
|
//! poll+webhook pair needs both anyway, so building only half here would be
|
||||||
|
//! work redone rather than work reused.
|
||||||
|
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::sync::{Arc, Mutex};
|
use std::sync::{Arc, Mutex};
|
||||||
|
|
||||||
|
use serde::Deserialize;
|
||||||
|
|
||||||
use crate::forge::{Client, ConfigPrStatus};
|
use crate::forge::{Client, ConfigPrStatus};
|
||||||
|
|
||||||
|
/// The handful of fields this cache needs out of a Forgejo `pull_request`
|
||||||
|
/// webhook payload — not a full typed mirror of the event (Forgejo's own
|
||||||
|
/// schema has dozens more), just enough to know which agent, which PR, and
|
||||||
|
/// whether it's still open.
|
||||||
|
#[derive(Deserialize)]
|
||||||
|
pub struct ConfigPrWebhookPayload {
|
||||||
|
pull_request: WebhookPullRequest,
|
||||||
|
repository: WebhookRepository,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Deserialize)]
|
||||||
|
struct WebhookPullRequest {
|
||||||
|
number: u64,
|
||||||
|
/// Forgejo sends `"open"` or `"closed"` here — merged and
|
||||||
|
/// closed-without-merging are indistinguishable at this field, but this
|
||||||
|
/// cache only ever answers "is there an open PR," so the distinction
|
||||||
|
/// doesn't matter to it.
|
||||||
|
state: String,
|
||||||
|
html_url: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Deserialize)]
|
||||||
|
struct WebhookRepository {
|
||||||
|
/// The config repo's name IS the agent's name — same convention
|
||||||
|
/// `Client::list_open_config_prs` relies on.
|
||||||
|
name: String,
|
||||||
|
}
|
||||||
|
|
||||||
/// How often to rescan `agent-configs/*`. Matches the interval named in
|
/// How often to rescan `agent-configs/*`. Matches the interval named in
|
||||||
/// `hive-c0re::forge::config_pr_poll`'s own doc comment — same org, same
|
/// `hive-c0re::forge::config_pr_poll`'s own doc comment — same org, same
|
||||||
/// staleness tolerance, no reason for the two to disagree.
|
/// staleness tolerance, no reason for the two to disagree.
|
||||||
|
|
@ -62,6 +99,53 @@ impl ConfigPrCache {
|
||||||
.lock()
|
.lock()
|
||||||
.unwrap_or_else(std::sync::PoisonError::into_inner) = scan;
|
.unwrap_or_else(std::sync::PoisonError::into_inner) = scan;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Apply one verified `ConfigPr` webhook delivery's raw body to the
|
||||||
|
/// cache — the low-latency counterpart to [`spawn`]'s periodic rescan.
|
||||||
|
///
|
||||||
|
/// A parse failure is logged and dropped, not propagated: the caller
|
||||||
|
/// (`crate::webhook::post_webhook_forge`) already returned 200 to
|
||||||
|
/// Forgejo (HMAC verification, not payload parsing, is what a retry
|
||||||
|
/// could fix), and the next poll tick reconciles whatever this delivery
|
||||||
|
/// would have changed — same "poll as backstop" property [`spawn`]'s
|
||||||
|
/// doc comment describes, just exercised on the failure path instead of
|
||||||
|
/// the steady-state one.
|
||||||
|
///
|
||||||
|
/// An `open` PR is upserted unconditionally. A `closed` one is removed
|
||||||
|
/// only if the cached entry's PR number still matches — guards against
|
||||||
|
/// an out-of-order delivery (a stale `closed` for PR #1 arriving after a
|
||||||
|
/// newer `opened` for PR #2 on the same repo) wiping out a genuinely
|
||||||
|
/// current entry.
|
||||||
|
pub fn apply_webhook_delivery(&self, body: &[u8]) {
|
||||||
|
let payload: ConfigPrWebhookPayload = match serde_json::from_slice(body) {
|
||||||
|
Ok(p) => p,
|
||||||
|
Err(e) => {
|
||||||
|
tracing::warn!(
|
||||||
|
error = %e,
|
||||||
|
"config-pr webhook: payload did not parse, cache unchanged (next poll reconciles)"
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
let mut map = self
|
||||||
|
.0
|
||||||
|
.lock()
|
||||||
|
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||||
|
if payload.pull_request.state == "open" {
|
||||||
|
map.insert(
|
||||||
|
payload.repository.name,
|
||||||
|
ConfigPrStatus {
|
||||||
|
pr_number: payload.pull_request.number,
|
||||||
|
html_url: payload.pull_request.html_url,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
} else if map
|
||||||
|
.get(&payload.repository.name)
|
||||||
|
.is_some_and(|cached| cached.pr_number == payload.pull_request.number)
|
||||||
|
{
|
||||||
|
map.remove(&payload.repository.name);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Build an empty cache and spawn the periodic scan that keeps it current.
|
/// Build an empty cache and spawn the periodic scan that keeps it current.
|
||||||
|
|
@ -89,3 +173,56 @@ pub fn spawn(client: Arc<Client>) -> Arc<ConfigPrCache> {
|
||||||
});
|
});
|
||||||
cache
|
cache
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::ConfigPrCache;
|
||||||
|
|
||||||
|
fn payload(agent: &str, number: u64, state: &str) -> Vec<u8> {
|
||||||
|
serde_json::json!({
|
||||||
|
"pull_request": { "number": number, "state": state, "html_url": "https://forge.example/pr" },
|
||||||
|
"repository": { "name": agent },
|
||||||
|
})
|
||||||
|
.to_string()
|
||||||
|
.into_bytes()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_open_delivery_upserts_the_entry() {
|
||||||
|
let cache = ConfigPrCache::new();
|
||||||
|
cache.apply_webhook_delivery(&payload("damocles", 5, "open"));
|
||||||
|
let status = cache.get("damocles").expect("entry inserted");
|
||||||
|
assert_eq!(status.pr_number, 5);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_closed_delivery_removes_a_matching_entry() {
|
||||||
|
let cache = ConfigPrCache::new();
|
||||||
|
cache.apply_webhook_delivery(&payload("damocles", 5, "open"));
|
||||||
|
cache.apply_webhook_delivery(&payload("damocles", 5, "closed"));
|
||||||
|
assert!(cache.get("damocles").is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_stale_closed_delivery_does_not_clobber_a_newer_open_pr() {
|
||||||
|
let cache = ConfigPrCache::new();
|
||||||
|
// PR #5 opened, then closed, then a genuinely new PR #6 opens.
|
||||||
|
cache.apply_webhook_delivery(&payload("damocles", 5, "open"));
|
||||||
|
cache.apply_webhook_delivery(&payload("damocles", 6, "open"));
|
||||||
|
// The #5 close event arrives late, after #6 already replaced it.
|
||||||
|
cache.apply_webhook_delivery(&payload("damocles", 5, "closed"));
|
||||||
|
let status = cache.get("damocles").expect("PR #6 must survive");
|
||||||
|
assert_eq!(status.pr_number, 6);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_malformed_payload_leaves_the_cache_unchanged() {
|
||||||
|
let cache = ConfigPrCache::new();
|
||||||
|
cache.apply_webhook_delivery(&payload("damocles", 5, "open"));
|
||||||
|
cache.apply_webhook_delivery(b"not json");
|
||||||
|
let status = cache
|
||||||
|
.get("damocles")
|
||||||
|
.expect("prior entry must survive a bad delivery");
|
||||||
|
assert_eq!(status.pr_number, 5);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -770,10 +770,11 @@ async fn get_jobq_rollup(State(state): State<AppState>) -> Json<Vec<hive_jobq_wi
|
||||||
Json(hive_jobq_wire::state_rollup(graph, roots))
|
Json(hive_jobq_wire::state_rollup(graph, roots))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// `agent`'s open config PR, from the last successful swarm-level scan
|
/// `agent`'s open config PR, from [`config_pr::ConfigPrCache`] — kept
|
||||||
/// ([`config_pr::spawn`]) — not a live forge read, so this answers even when
|
/// current by a webhook nudge (near-real-time) backstopped by a periodic
|
||||||
/// the forge itself is momentarily unreachable, at the cost of being up to
|
/// scan (worst case: up to one [`config_pr::POLL_INTERVAL`] stale if a
|
||||||
/// one [`config_pr::POLL_INTERVAL`] stale.
|
/// delivery was ever missed). Not a live forge read either way, so this
|
||||||
|
/// answers even when the forge itself is momentarily unreachable.
|
||||||
///
|
///
|
||||||
/// `200` with a `null` body means "no open PR" (or "no scan has completed
|
/// `200` with a `null` body means "no open PR" (or "no scan has completed
|
||||||
/// yet") — not distinguished, for the same reason [`ConfigPrCache::get`]
|
/// yet") — not distinguished, for the same reason [`ConfigPrCache::get`]
|
||||||
|
|
|
||||||
|
|
@ -311,10 +311,11 @@ pub(super) fn verify(
|
||||||
|
|
||||||
/// POST `/webhook/forge/{kind}` — a swarm-wide Forgejo delivery.
|
/// POST `/webhook/forge/{kind}` — a swarm-wide Forgejo delivery.
|
||||||
///
|
///
|
||||||
/// Verifies the HMAC over the raw body. Nothing consumes the delivery yet;
|
/// Verifies the HMAC over the raw body, then acts per `kind`: a `Knowledge`
|
||||||
/// once the swarm→hive channel lands, this daemon **parses** it and emits a
|
/// delivery announces the change to every hive (see
|
||||||
/// semantic message (*knowledge repo changed*, *deploy agent X at rev Y*) to
|
/// [`announce_knowledge_change`]); a `ConfigPr` delivery updates
|
||||||
/// the hives that need it — see the module docs.
|
/// `crate::config_pr::ConfigPrCache` immediately (see that module's doc
|
||||||
|
/// comment for why both this and a periodic poll write the same cache).
|
||||||
///
|
///
|
||||||
/// Returns 200 on an accepted delivery so Forgejo does not retry. A refused
|
/// Returns 200 on an accepted delivery so Forgejo does not retry. A refused
|
||||||
/// one answers 401 (bad signature) or 503 (this daemon has no secret), and
|
/// one answers 401 (bad signature) or 503 (this daemon has no secret), and
|
||||||
|
|
@ -367,14 +368,13 @@ pub(super) async fn post_webhook_forge(
|
||||||
"webhook: verified delivery"
|
"webhook: verified delivery"
|
||||||
);
|
);
|
||||||
|
|
||||||
// Only the knowledge delivery is acted on. Deploy coordination is a
|
match kind {
|
||||||
// separate concern with its own issue — a hive does not want to hear that a
|
DeliveryKind::Knowledge => announce_knowledge_change(&state).await,
|
||||||
// config PR was opened, it wants to be told when to rebuild from main, and
|
DeliveryKind::ConfigPr => {
|
||||||
// that is a decision the controller makes after a merge rather than a relay
|
if let Some(cache) = state.config_prs.as_ref() {
|
||||||
// of this delivery. Written as a condition rather than a match arm holding
|
cache.apply_webhook_delivery(&body);
|
||||||
// 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()
|
(StatusCode::OK, "ok").into_response()
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue