dashboard: fail purge-tombstone closed when the container list is unreadable
post_purge_tombstone's live-container guard used lifecycle::list().await .unwrap_or_default(), so a failed list() (hive-priv socket unreachable, restarting, ...) read as 'no live containers' and let the purge proceed — deleting agent_state_dir/applied_dir for an agent whose container may still be running. Factor the decision into check_not_live(), a pure helper that refuses on both 'still listed' and 'list unreadable', with unit tests covering both plus the allowed case. Closes #4671
This commit is contained in:
parent
11cd2038c9
commit
7dd52207de
1 changed files with 69 additions and 9 deletions
|
|
@ -115,6 +115,31 @@ pub(crate) async fn emit_tombstones_snapshot(coord: &Arc<Coordinator>) {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Decide whether `post_purge_tombstone` may proceed, given the outcome of
|
||||||
|
/// `lifecycle::list()`. `Ok` only when the list was read and names no live
|
||||||
|
/// container for `name`; an unreadable list proves nothing about whether a
|
||||||
|
/// container is live, so it refuses too rather than reading absence into it.
|
||||||
|
fn check_not_live(
|
||||||
|
name: &hive_types::Ident,
|
||||||
|
listed: anyhow::Result<Vec<String>>,
|
||||||
|
) -> Result<(), String> {
|
||||||
|
match listed {
|
||||||
|
Ok(live)
|
||||||
|
if live.iter().any(|c| {
|
||||||
|
c == &format!("{}{name}", lifecycle::AGENT_PREFIX) || c.as_str() == name.as_str()
|
||||||
|
}) =>
|
||||||
|
{
|
||||||
|
Err(format!(
|
||||||
|
"refusing to purge {name}: container still exists — use DESTR0Y first"
|
||||||
|
))
|
||||||
|
}
|
||||||
|
Ok(_) => Ok(()),
|
||||||
|
Err(e) => Err(format!(
|
||||||
|
"refusing to purge {name}: cannot confirm no live container exists: {e:#}"
|
||||||
|
)),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Wipe a tombstoned agent's
|
/// Wipe a tombstoned agent's
|
||||||
/// retained state dir + applied config dir entirely.
|
/// retained state dir + applied config dir entirely.
|
||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
|
|
@ -148,15 +173,10 @@ pub(super) async fn post_purge_tombstone(
|
||||||
};
|
};
|
||||||
// Sanity: refuse to purge if a live container still exists with this
|
// Sanity: refuse to purge if a live container still exists with this
|
||||||
// name. The dashboard already filters tombstones to non-live names,
|
// name. The dashboard already filters tombstones to non-live names,
|
||||||
// but the operator could send a stale POST.
|
// but the operator could send a stale POST. An unreadable list is not
|
||||||
let live = lifecycle::list().await.unwrap_or_default();
|
// proof of absence — see `check_not_live`.
|
||||||
if live
|
if let Err(reason) = check_not_live(&name, lifecycle::list().await) {
|
||||||
.iter()
|
return error_response(&reason);
|
||||||
.any(|c| c == &format!("{}{name}", lifecycle::AGENT_PREFIX) || c.as_str() == name.as_str())
|
|
||||||
{
|
|
||||||
return error_response(&format!(
|
|
||||||
"refusing to purge {name}: container still exists — use DESTR0Y first"
|
|
||||||
));
|
|
||||||
}
|
}
|
||||||
let mut errors = Vec::new();
|
let mut errors = Vec::new();
|
||||||
for dir in [
|
for dir in [
|
||||||
|
|
@ -184,3 +204,43 @@ pub(super) async fn post_purge_tombstone(
|
||||||
error_response(&format!("purge {name} partial: {}", errors.join(", ")))
|
error_response(&format!("purge {name} partial: {}", errors.join(", ")))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
fn ident(s: &str) -> hive_types::Ident {
|
||||||
|
hive_types::Ident::parse(s).unwrap()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A live container by that name in the list refuses the purge.
|
||||||
|
#[test]
|
||||||
|
fn live_container_in_list_refuses() {
|
||||||
|
let name = ident("doomed");
|
||||||
|
let err = check_not_live(&name, Ok(vec!["h-doomed".to_owned()]))
|
||||||
|
.expect_err("a listed container must refuse the purge");
|
||||||
|
assert!(err.contains("still exists"), "{err}");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// No matching name in a readable list allows the purge.
|
||||||
|
#[test]
|
||||||
|
fn absent_from_list_allows() {
|
||||||
|
let name = ident("doomed");
|
||||||
|
check_not_live(&name, Ok(vec!["h-other".to_owned()]))
|
||||||
|
.expect("an absent container must allow the purge");
|
||||||
|
// Control: the same name present in the list still refuses, so the
|
||||||
|
// pass above comes from the absence, not from the helper accepting
|
||||||
|
// anything.
|
||||||
|
assert!(check_not_live(&name, Ok(vec!["h-doomed".to_owned()])).is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// An unreadable list proves nothing, so it must refuse — this is the
|
||||||
|
/// case that regresses to a silent delete under `unwrap_or_default()`.
|
||||||
|
#[test]
|
||||||
|
fn unreadable_list_refuses() {
|
||||||
|
let name = ident("doomed");
|
||||||
|
let err = check_not_live(&name, Err(anyhow::anyhow!("connect to hive-priv socket")))
|
||||||
|
.expect_err("an unreadable list must not read as an absent container");
|
||||||
|
assert!(err.contains("cannot confirm"), "{err}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue