diff --git a/hive-c0re/src/forge/ci_runner.rs b/hive-c0re/src/forge/ci_runner.rs index a5b1b8d7..f4e04ea4 100644 --- a/hive-c0re/src/forge/ci_runner.rs +++ b/hive-c0re/src/forge/ci_runner.rs @@ -12,6 +12,7 @@ use anyhow::{Context as _, Result}; use serde_json::Value; +use url::Url; use super::forge_http_base; @@ -37,8 +38,10 @@ pub(super) async fn ensure_ci_runner_registered(core_token: &str) { if !ci_enabled() { return; } - // Existing creds still valid on the forge → nothing to do. + // Existing creds still valid on the forge, AND pointed at the forge + // address we are configured for → nothing to do. if let Some(id) = existing_runner_id() + && runner_address_current() && runner_valid(core_token, id).await { return; @@ -78,6 +81,51 @@ fn existing_runner_id() -> Option { (id != 0).then_some(id) } +/// Whether the persisted `.runner` still names the forge host we are +/// configured for. +/// +/// `act_runner` records the `--instance` URL it was registered with and the +/// daemon reads the address from there and nowhere else — the nix option only +/// reaches `register`, and upstream re-registers on a changed token or labels, +/// never on a changed URL. So a forge rename leaves a runner dialling a name +/// its container's `extraHosts` no longer resolves, with every existing check +/// still green: the runner id is untouched, so `runner_valid` says yes. +/// +/// Fail-safe in the same direction as [`runner_valid`]: anything we cannot +/// read or parse returns `true` (keep the existing creds). Only a *positive* +/// disagreement between two parseable hosts is treated as stale, because the +/// cost of a false stale is re-registering on every boot. +fn runner_address_current() -> bool { + let Ok(raw) = std::fs::read_to_string(RUNNER_FILE) else { + return true; + }; + runner_address_matches(&raw, forge_http_base()) +} + +/// Host-only comparison of `.runner`'s `address` against the configured base. +/// +/// Split from [`runner_address_current`] so it is testable without the +/// filesystem. Compares **hosts**, not whole URLs: both strings come from the +/// same nix expression (`http://${swarm.forge.domain}`, reaching the runner as +/// `instances.hive.url` and this daemon as `HIVE_FORGE_URL`), but scheme, +/// port and trailing slash are exactly the kind of cosmetic difference that +/// would otherwise re-register the runner on every boot. +fn runner_address_matches(raw: &str, configured: &str) -> bool { + let Some(address) = serde_json::from_str::(raw) + .ok() + .and_then(|j| j.get("address")?.as_str().map(str::to_owned)) + else { + return true; + }; + match (Url::parse(&address), Url::parse(configured)) { + (Ok(a), Ok(b)) => match (a.host_str(), b.host_str()) { + (Some(a), Some(b)) => a.eq_ignore_ascii_case(b), + _ => true, + }, + _ => true, + } +} + /// `GET /admin/runners/{id}` — `true` iff the runner still exists on the forge /// (HTTP 200). A 404 (deleted from the admin panel) or any other status means /// re-registration is needed. A transport error (forge unreachable) is treated @@ -126,3 +174,57 @@ async fn fetch_registration_token(core_token: &str) -> Result { .map(str::to_owned) .context("registration-token response missing 'token' field") } + +#[cfg(test)] +mod tests { + use super::runner_address_matches; + + fn runner(address: &str) -> String { + format!(r#"{{"id":7,"uuid":"u","name":"hive","token":"t","address":"{address}"}}"#) + } + + /// The case the bug is about: a forge rename leaves the old host behind. + #[test] + fn differing_host_is_stale() { + assert!(!runner_address_matches( + &runner("http://forge.pr1ma.darkest.space"), + "http://forge.constellation.darkest.space", + )); + } + + /// Control for the arm above — without it, a helper that always returned + /// `false` would pass `differing_host_is_stale` and re-register forever. + #[test] + fn same_host_is_current() { + assert!(runner_address_matches( + &runner("http://forge.example"), + "http://forge.example", + )); + } + + /// Cosmetic differences must NOT count: both sides are rendered from one + /// nix expression, but a port or trailing slash appearing on one side is + /// not a rename. + #[test] + fn port_scheme_and_trailing_slash_do_not_count() { + assert!(runner_address_matches( + &runner("http://forge.example:80/"), + "https://FORGE.example", + )); + } + + /// Fail-safe: anything unreadable keeps the existing credentials, so a + /// malformed or older `.runner` never costs a working runner. + #[test] + fn unparseable_input_keeps_creds() { + assert!(runner_address_matches("not json", "http://forge.example")); + assert!(runner_address_matches( + r#"{"id":7}"#, + "http://forge.example" + )); + assert!(runner_address_matches( + &runner("://nonsense"), + "http://forge.example", + )); + } +}