swarm-controller: keep retrying the forge webhook secret and registration after a startup outage
The webhook secret slot is filled in the background, on the forge token's capped unbounded backoff, when the store can't answer at start. The hook registration waits for the secret and retries on the same backoff instead of giving up after five tries. Closes #6101
This commit is contained in:
parent
431cb6e77f
commit
68b38a9d44
1 changed files with 284 additions and 114 deletions
|
|
@ -24,7 +24,9 @@ use axum::{
|
|||
};
|
||||
use hmac::{Hmac, KeyInit as _, Mac as _};
|
||||
use sha2::Sha256;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::jobq::Late;
|
||||
use crate::routes::AppState;
|
||||
|
||||
/// The legacy home of the HMAC secret, relative to the daemon's state directory.
|
||||
|
|
@ -116,6 +118,45 @@ pub(super) async fn load_or_generate_secret() -> Result<String> {
|
|||
resolve(&StoreSlot, &secret_path()).await
|
||||
}
|
||||
|
||||
/// The slot the webhook secret lands in: filled before this returns if the
|
||||
/// store answers, otherwise from a background task that reads it again until it
|
||||
/// does. The webhook endpoint answers 503 until the slot is filled, and the
|
||||
/// registration of the forge hooks waits for it.
|
||||
///
|
||||
/// Every attempt is [`load_or_generate_secret`], so a retry only reads again: a
|
||||
/// secret is minted only when the store answers that it holds none, never on a
|
||||
/// failed read.
|
||||
pub(crate) async fn load() -> Late<Arc<str>> {
|
||||
load_with(|| async { load_or_generate_secret().await.map(Arc::from) }).await
|
||||
}
|
||||
|
||||
/// [`load`], with `load` standing in for the store.
|
||||
async fn load_with<F, Fut>(mut load: F) -> Late<Arc<str>>
|
||||
where
|
||||
F: FnMut() -> Fut + Send + 'static,
|
||||
Fut: Future<Output = Result<Arc<str>>> + Send,
|
||||
{
|
||||
let slot = Late::<Arc<str>>::default();
|
||||
match load().await {
|
||||
Ok(secret) => {
|
||||
slot.get_or_init(|| secret);
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
error = %format!("{e:#}"),
|
||||
"webhook secret unavailable; swarm-wide forge webhooks are off until it is read \
|
||||
from the swarm secret store in the background"
|
||||
);
|
||||
let filling = Arc::clone(&slot);
|
||||
tokio::spawn(async move {
|
||||
crate::forge::fill_when_ready(&filling, "the webhook secret", load).await;
|
||||
tracing::info!("the webhook secret is loaded; forge webhooks are on");
|
||||
});
|
||||
}
|
||||
}
|
||||
slot
|
||||
}
|
||||
|
||||
/// What the store holds at the webhook secret's path.
|
||||
pub(super) enum Stored {
|
||||
/// The secret.
|
||||
|
|
@ -551,7 +592,11 @@ pub(super) async fn post_webhook_forge(
|
|||
// Order matters: verify BEFORE looking at the path. An unauthenticated
|
||||
// caller learning which `{kind}` values exist, from the difference
|
||||
// between 404 and 401, is a small leak — but it is free to close.
|
||||
if let Err(refusal) = verify(state.webhook_secret.as_deref(), &headers, &body) {
|
||||
if let Err(refusal) = verify(
|
||||
state.webhook_secret.get().map(AsRef::as_ref),
|
||||
&headers,
|
||||
&body,
|
||||
) {
|
||||
let message = refusal.message();
|
||||
tracing::warn!(%kind, %message, "webhook: refused delivery");
|
||||
return (refusal.status(), message).into_response();
|
||||
|
|
@ -671,13 +716,14 @@ async fn announce_knowledge_change(state: &AppState) {
|
|||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{
|
||||
DeliveryKind, Refusal, SecretSlot, Stored, load_or_generate_at, resolve, secret_path_from,
|
||||
verify,
|
||||
DeliveryKind, Refusal, SecretSlot, Stored, load_or_generate_at, load_with, resolve,
|
||||
secret_path_from, verify,
|
||||
};
|
||||
use axum::{body::Bytes, http::HeaderMap, http::StatusCode};
|
||||
use hmac::{Hmac, KeyInit as _, Mac as _};
|
||||
use sha2::Sha256;
|
||||
use std::fmt::Write as _;
|
||||
use std::sync::Arc;
|
||||
|
||||
/// Mint the header Forgejo would send. Written out rather than reusing
|
||||
/// the verifier's internals so a test cannot pass by sharing a bug with
|
||||
|
|
@ -950,6 +996,8 @@ mod tests {
|
|||
/// What `read` answers while it holds nothing.
|
||||
empty: fn() -> anyhow::Result<Stored>,
|
||||
creates: std::sync::Mutex<u32>,
|
||||
/// How many more reads fail as an unreachable store does, before it answers.
|
||||
unreachable_reads: std::sync::Mutex<u32>,
|
||||
}
|
||||
|
||||
impl FakeSlot {
|
||||
|
|
@ -958,6 +1006,7 @@ mod tests {
|
|||
held: std::sync::Mutex::new(secret.map(str::to_owned)),
|
||||
empty: || Ok(Stored::Absent),
|
||||
creates: std::sync::Mutex::new(0),
|
||||
unreachable_reads: std::sync::Mutex::new(0),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -968,6 +1017,12 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
/// Reads fail as an unreachable store does until `reads` have been made.
|
||||
fn unreachable_for(self, reads: u32) -> Self {
|
||||
*self.unreachable_reads.lock().expect("unpoisoned") = reads;
|
||||
self
|
||||
}
|
||||
|
||||
fn held(&self) -> Option<String> {
|
||||
self.held.lock().expect("unpoisoned").clone()
|
||||
}
|
||||
|
|
@ -979,6 +1034,13 @@ mod tests {
|
|||
|
||||
impl SecretSlot for FakeSlot {
|
||||
async fn read(&self) -> anyhow::Result<Stored> {
|
||||
{
|
||||
let mut left = self.unreachable_reads.lock().expect("unpoisoned");
|
||||
if *left > 0 {
|
||||
*left -= 1;
|
||||
anyhow::bail!("connection refused");
|
||||
}
|
||||
}
|
||||
match self.held() {
|
||||
Some(secret) => Ok(Stored::Value(secret)),
|
||||
None => (self.empty)(),
|
||||
|
|
@ -1107,4 +1169,60 @@ mod tests {
|
|||
assert!(!path.exists());
|
||||
assert_eq!(slot.creates(), 0);
|
||||
}
|
||||
|
||||
/// `load_with`'s source: `resolve` against `slot` and the file at
|
||||
/// `path`, as `load_or_generate_secret` does against the store.
|
||||
fn resolving(
|
||||
slot: &Arc<FakeSlot>,
|
||||
path: &std::path::Path,
|
||||
) -> impl FnMut() -> std::pin::Pin<Box<dyn Future<Output = anyhow::Result<Arc<str>>> + Send>>
|
||||
+ Send
|
||||
+ 'static {
|
||||
let (slot, path) = (Arc::clone(slot), path.to_owned());
|
||||
move || {
|
||||
let (slot, path) = (Arc::clone(&slot), path.clone());
|
||||
Box::pin(async move { resolve(&*slot, &path).await.map(Arc::from) })
|
||||
}
|
||||
}
|
||||
|
||||
// Paused time: each sleep completes at once and advances the clock.
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn a_store_down_at_start_fills_the_slot_once_it_answers() {
|
||||
let (_dir, path) = legacy_file(None);
|
||||
let store = Arc::new(FakeSlot::holding(Some(IN_STORE)).unreachable_for(3));
|
||||
let slot = load_with(resolving(&store, &path)).await;
|
||||
assert!(slot.get().is_none(), "the first read failed");
|
||||
|
||||
tokio::time::sleep(std::time::Duration::from_mins(10)).await;
|
||||
|
||||
assert_eq!(slot.get().map(|s| &**s), Some(IN_STORE));
|
||||
assert_eq!(store.creates(), 0);
|
||||
assert!(!path.exists(), "nothing was minted on disk");
|
||||
}
|
||||
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn a_store_that_stays_down_mints_nothing_however_often_it_is_read() {
|
||||
let (_dir, path) = legacy_file(None);
|
||||
let store = Arc::new(FakeSlot::holding(None).unreachable_for(u32::MAX));
|
||||
let slot = load_with(resolving(&store, &path)).await;
|
||||
tokio::time::sleep(std::time::Duration::from_hours(3)).await;
|
||||
|
||||
assert!(slot.get().is_none());
|
||||
assert_eq!(store.creates(), 0);
|
||||
assert_eq!(store.held(), None);
|
||||
assert!(!path.exists());
|
||||
assert!(
|
||||
*store.unreachable_reads.lock().expect("unpoisoned") < u32::MAX - 10,
|
||||
"the background task kept reading"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn a_store_answering_at_start_fills_the_slot_without_a_background_task() {
|
||||
let (_dir, path) = legacy_file(None);
|
||||
let store = Arc::new(FakeSlot::holding(Some(IN_STORE)));
|
||||
let slot = load_with(resolving(&store, &path)).await;
|
||||
|
||||
assert_eq!(slot.get().map(|s| &**s), Some(IN_STORE));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue