hive-c0re: re-register the CI runner when the forge address changes

act_runner records the --instance URL it was registered with and reads the
forge address from .runner and nowhere else: the nix option reaches only
`register`, and upstream re-registers on a changed token or labels, never on
a changed URL. Our own precond short-circuits on .runner existing, and
runner_valid asks whether the runner id still exists -- which after a rename
it does. So changing swarm.forge.domain left the runner dialling a name its
container's derived extraHosts no longer resolves, with every check green.
The symptom is CI going quiet rather than anything failing.

Compare the persisted address against the configured base as part of the
same early return. A disagreement mints a fresh registration token, which
changes the token hash upstream already keys on, so upstream removes .runner
and re-registers against the current --instance. This module never writes or
deletes that file; the deletion stays with the script that owns registration.

The comparison is host-only and lenient on purpose. Both sides render from
one nix expression -- http://${swarm.forge.domain}, reaching the runner as
instances.hive.url and this daemon as HIVE_FORGE_URL -- so they cannot drift,
while scheme, port and trailing slash are exactly the cosmetic differences
that would otherwise re-register on every boot. Unreadable, missing or
unparseable input keeps the existing credentials, matching runner_valid's
treatment of a transport error: only a positive disagreement counts.
This commit is contained in:
atlas 2026-08-26 13:11:15 +02:00
commit ebb4eea691

View file

@ -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<u64> {
(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::<Value>(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<String> {
.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",
));
}
}