From 58a0c6ef64c84b107fffcf8ca9c4af14db9b774c Mon Sep 17 00:00:00 2001 From: archipelago Date: Tue, 6 Oct 2026 23:54:20 -0400 Subject: [PATCH] Repair managed nginx listeners and verify reload acceptance --- core/archipelago/src/bootstrap.rs | 195 +++++++++++++++++++--- docs/https-app-gate-followup-20261006.md | 16 ++ tests/regression/nginx-listener-reload.py | 76 +++++++++ 3 files changed, 268 insertions(+), 19 deletions(-) create mode 100644 tests/regression/nginx-listener-reload.py diff --git a/core/archipelago/src/bootstrap.rs b/core/archipelago/src/bootstrap.rs index b81121f7..7e3f1839 100644 --- a/core/archipelago/src/bootstrap.rs +++ b/core/archipelago/src/bootstrap.rs @@ -302,6 +302,13 @@ pub async fn ensure_doctor_installed() { Ok(_) => debug!("Doctor artifacts already in sync"), Err(e) => warn!("Doctor bootstrap failed (non-fatal): {:#}", e), } + // Resolve known TLS bind conflicts before installing routes: nginx may + // otherwise acknowledge the reload signal while keeping its old workers. + match run_nginx_listener_repair().await { + Ok(true) => info!("nginx HTTPS listeners retargeted to this host's current addresses"), + Ok(false) => debug!("nginx listeners already match this host's addresses"), + Err(e) => warn!("nginx listener repair failed (non-fatal): {:#}", e), + } match run_nginx().await { Ok(true) => info!("Patched nginx config to proxy missing backend endpoints"), Ok(false) => debug!("Nginx backend endpoint proxy blocks already present"), @@ -326,11 +333,6 @@ pub async fn ensure_doctor_installed() { Ok(false) => debug!("Console welcome banner already current (or not an ISO node)"), Err(e) => warn!("Welcome banner sync failed (non-fatal): {:#}", e), } - match run_nginx_listener_repair().await { - Ok(true) => info!("nginx HTTPS listeners retargeted to this host's current addresses"), - Ok(false) => debug!("nginx listeners already match this host's addresses"), - Err(e) => warn!("nginx listener repair failed (non-fatal): {:#}", e), - } match run_ha_rpc_proxy_bind_repair().await { Ok(true) => { info!("HA bitcoind RPC forwarder rebound dynamically — survives network moves now") @@ -1031,9 +1033,11 @@ async fn podman_stdout(args: &[&str]) -> String { /// Both observed on archi-dev-box, 2026-08-15: nginx dead since boot with /// `bind() to 192.168.63.240:443 failed (99: Cannot assign requested /// address)`, and the dashboard simply unreachable. -const NGINX_SITES: [&str; 2] = [ +const NGINX_SITES: [&str; 4] = [ "/etc/nginx/sites-available/archipelago-http", "/etc/nginx/sites-available/archipelago", + "/etc/nginx/sites-enabled/archipelago-http", + "/etc/nginx/sites-enabled/archipelago", ]; const NGINX_RESTART_DROPIN: &str = "/etc/systemd/system/nginx.service.d/10-archipelago-restart.conf"; @@ -1070,6 +1074,105 @@ fn is_cgnat(addr: &str) -> bool { /// new text when it differs. Lines for absent addresses are dropped and one /// line per present address is kept, preserving the file's indentation. fn retarget_https_listeners(text: &str, present: &[String]) -> Option { + if present.is_empty() { + return None; + } + retarget_managed_https_wildcards(text, present) + .or_else(|| retarget_pinned_https_listeners(text, present)) +} + +/// Only the shipped node-CA dashboard profile may replace wildcard TLS binds. +/// Public/custom vhosts, certificate paths and listener options are untouched. +fn retarget_managed_https_wildcards(text: &str, present: &[String]) -> Option { + if present.is_empty() { + return None; + } + // This migration owns the shipped top-level layout only. Do not combine an + // operator's differently nested/indented server with the managed profile. + if text + .lines() + .any(|line| line.trim() == "server {" && line != "server {") + { + return None; + } + let wildcard = |line: &str| { + matches!( + line.trim(), + "listen 443 ssl default_server;" | "listen [::]:443 ssl default_server;" + ) + }; + let mut out = String::new(); + let mut changed = false; + for block in text.split_inclusive("\nserver {") { + // A top-level server boundary ends the preceding segment. Exact + // directives avoid claiming ownership of arbitrary TLS configuration. + let lines: Vec<_> = block.lines().map(str::trim).collect(); + let recognized = lines.contains(&"server_name _;") + && lines.contains(&"root /opt/archipelago/web-ui;") + && lines.contains(&"ssl_certificate /etc/archipelago/ssl/archipelago.crt;") + && lines.contains(&"ssl_certificate_key /etc/archipelago/ssl/archipelago.key;") + && lines.iter().any(|line| wildcard(line)) + && !lines.iter().any(|line| { + line.starts_with("listen ") + && (line.contains(":443") || line.starts_with("listen 443")) + && !wildcard(line) + }); + if !recognized { + out.push_str(block); + continue; + } + let mut wrote = false; + for line in block.split_inclusive('\n') { + if wildcard(line) { + if !wrote { + let indent = &line[..line.len() - line.trim_start().len()]; + for address in present { + out.push_str(&format!("{indent}listen {address}:443 ssl;\n")); + } + wrote = true; + } + } else { + out.push_str(line); + } + } + changed = true; + } + changed.then_some(out) +} + +/// `systemctl reload` returns before nginx tries its new binds. A new worker +/// generation proves the master accepted the reload; a successful signal alone +/// does not. Keep old connections alive and bound this check to ten seconds. +async fn reload_nginx_checked() -> Result<()> { + let script = r#"set -eu +master="$(systemctl show -p MainPID --value nginx)" +case "$master" in ''|*[!0-9]*|0|1) exit 1 ;; esac +workers() { ps --ppid "$master" -o pid=,args= | awk '$2 == "nginx:" && $3 == "worker" && $4 == "process" && $5 != "is" {print $1}' | sort -n | tr '\n' ' '; } +before="$(workers)" +test -n "$before" +nginx -t +systemctl reload nginx +attempt=0 +while [ "$attempt" -lt 10 ]; do + after="$(workers)" + for worker in $after; do + case " $before " in *" $worker "*) ;; *) exit 0 ;; esac + done + attempt=$((attempt + 1)) + sleep 1 +done +echo 'nginx reload did not produce a new worker generation' >&2 +exit 1 +"#; + let status = host_sudo(&["sh", "-c", script]).await?; + anyhow::ensure!( + status.success(), + "nginx did not accept the reloaded configuration" + ); + Ok(()) +} + +fn retarget_pinned_https_listeners(text: &str, present: &[String]) -> Option { let listen_of = |l: &str| -> Option { let t = l.trim(); let rest = t.strip_prefix("listen ")?.strip_suffix(":443 ssl;")?; @@ -1117,8 +1220,18 @@ async fn run_nginx_listener_repair() -> Result { return Ok(false); // no network yet; a later boot pass will do it } let mut changed = false; + let mut seen = std::collections::HashSet::new(); + let repair_id = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_nanos(); for site in NGINX_SITES { - let Ok(text) = tokio::fs::read_to_string(site).await else { + let Ok(target) = tokio::fs::canonicalize(site).await else { + continue; + }; + if !seen.insert(target.clone()) { + continue; + } + let Ok(text) = tokio::fs::read_to_string(&target).await else { continue; }; let Some(healed) = retarget_https_listeners(&text, &present) else { @@ -1134,9 +1247,16 @@ async fn run_nginx_listener_repair() -> Result { // Install behind `nginx -t`, and roll back if the test fails — a bad // config here would take the dashboard down, which is the very // failure this repair exists to prevent. + // Backups must never be written in sites-enabled: nginx includes every + // file there, and a backup would introduce duplicate server directives. + let backup = format!( + "/var/lib/archipelago/support/nginx-listeners-{repair_id}-{}.conf", + seen.len() + ); + let target = format!("'{}'", target.to_string_lossy().replace('\'', "'\\''")); let script = format!( - "set -eu\ncp {site} {site}.bak-listeners\ninstall -m 0644 {staged} {site}\n\ - if ! nginx -t 2>/dev/null; then cp {site}.bak-listeners {site}; exit 3; fi\nexit 0\n" + "set -eu\ninstall -d -m 0700 /var/lib/archipelago/support\ncp -- {target} {backup}\nchmod 0600 {backup}\ninstall -m 0644 {staged} {target}\n\ + if ! nginx -t 2>/dev/null; then install -m 0644 {backup} {target}; exit 3; fi\nexit 0\n" ); let status = host_sudo(&["sh", "-lc", &script]).await?; match status.code() { @@ -1156,13 +1276,14 @@ async fn run_nginx_listener_repair() -> Result { cat > {dropin} <<'EOF'\n[Service]\nRestart=on-failure\nRestartSec=5\n\ [Unit]\nStartLimitIntervalSec=300\nStartLimitBurst=10\nEOF\n\ systemctl daemon-reload\n\ - if ! systemctl is-active --quiet nginx; then systemctl reset-failed nginx 2>/dev/null || true; systemctl start nginx 2>/dev/null || true; \ - elif [ \"${{RELOAD:-1}}\" = 1 ]; then systemctl reload nginx 2>/dev/null || true; fi\nexit 0\n", + if ! systemctl is-active --quiet nginx; then systemctl reset-failed nginx 2>/dev/null || true; systemctl start nginx; fi\nexit 0\n", dropin = NGINX_RESTART_DROPIN ); - host_sudo(&["sh", "-lc", &script]) + let status = host_sudo(&["sh", "-lc", &script]) .await .context("nginx restart policy + start")?; + anyhow::ensure!(status.success(), "nginx restart policy or start failed"); + reload_nginx_checked().await?; Ok(changed) } @@ -1715,8 +1836,12 @@ async fn run_nginx() -> Result { if patched_paths.iter().any(|p| p == &canonical) { continue; } + // Rewrite the target, preserving the enabled symlink itself. + let target = canonical + .to_str() + .context("nginx config path is not UTF-8")?; + changed |= patch_nginx_conf(target).await?; patched_paths.push(canonical); - changed |= patch_nginx_conf(path).await?; } Ok(changed) } @@ -2052,9 +2177,9 @@ async fn patch_nginx_conf(path: &str) -> Result { // Reload nginx so the new block takes effect immediately. Reload (not // restart) keeps in-flight connections alive. - if let Err(e) = host_sudo(&["systemctl", "reload", "nginx"]).await { - warn!("nginx reload failed (non-fatal): {:#}", e); - } + // Retain the backup if the master rejects the reload (for example because + // Tailscale owns a wildcard bind). A later listener repair may resolve it. + reload_nginx_checked().await?; let _ = host_sudo(&["rm", "-f", &backup]).await; Ok(true) } @@ -2068,17 +2193,24 @@ mod tests { assert_eq!(fixed.matches("location /api/rental-playback/ {").count(), 2); assert_eq!(super::heal_rental_playback_route(&fixed), fixed); assert_eq!(fixed.matches("proxy_cache off;").count(), 2); - assert_eq!(fixed.matches("proxy_set_header Range $http_range;").count(), 2); + assert_eq!( + fixed.matches("proxy_set_header Range $http_range;").count(), + 2 + ); let partial = fixed.replacen(super::NGINX_RENTAL_PLAYBACK_BLOCK, "", 1); assert_eq!(super::heal_rental_playback_route(&partial), fixed); } - #[test] fn catalog_routes_upgrade_both_vhosts_without_changing_access_guards() { let old = "server { if ($guard) { return 404; } location /api/app-catalog { proxy_pass http://127.0.0.1:5678; } }\nserver { location /api/app-catalog { proxy_set_header Cookie $http_cookie; } }"; let fixed = super::heal_node_catalog_route(old); - assert_eq!(fixed.matches("location ~ ^/api/(?:app-catalog|node-app-catalog)$ {").count(), 2); + assert_eq!( + fixed + .matches("location ~ ^/api/(?:app-catalog|node-app-catalog)$ {") + .count(), + 2 + ); assert!(fixed.contains("if ($guard) { return 404; }")); assert!(fixed.contains("proxy_set_header Cookie $http_cookie;")); assert_eq!(super::heal_node_catalog_route(&fixed), fixed); @@ -2235,6 +2367,31 @@ mod tests { assert!(retarget_https_listeners(&healed, &present).is_none()); } + #[test] + fn managed_wildcard_tls_migration_preserves_other_vhosts() { + let profile = "server {\n listen 443 ssl default_server;\n listen [::]:443 ssl default_server;\n server_name _;\n ssl_certificate /etc/archipelago/ssl/archipelago.crt;\n ssl_certificate_key /etc/archipelago/ssl/archipelago.key;\n root /opt/archipelago/web-ui;\n}\n"; + let custom = "server {\n listen 443 ssl default_server;\n server_name public.example;\n ssl_certificate /custom.crt;\n}\n"; + let present = vec!["192.168.1.50".into(), "10.44.0.1".into()]; + let original = format!("{custom}{profile}{custom}"); + let healed = retarget_managed_https_wildcards(&original, &present).unwrap(); + assert!(healed.starts_with(custom)); + assert!(healed.ends_with(custom)); + assert_eq!(healed.matches("listen 192.168.1.50:443 ssl;").count(), 1); + assert_eq!(healed.matches("listen 10.44.0.1:443 ssl;").count(), 1); + assert_eq!(healed.matches("listen 443 ssl default_server;").count(), 2); + assert!(!healed.contains("listen [::]:443")); + assert!(retarget_managed_https_wildcards(&healed, &present).is_none()); + assert!(retarget_managed_https_wildcards(profile, &[]).is_none()); + for changed in [ + profile.replace("archipelago.crt", "custom.crt"), + profile.replace("server_name _;", "server_name public.example;"), + profile.replace("listen 443 ssl default_server;", "listen 443 ssl http2;"), + profile.replace("server {", " server {"), + ] { + assert!(retarget_managed_https_wildcards(&changed, &present).is_none()); + } + } + #[test] fn wildcard_only_configs_and_cgnat_are_left_alone() { // No address-pinned listener → nothing to heal (the ISO's own config). diff --git a/docs/https-app-gate-followup-20261006.md b/docs/https-app-gate-followup-20261006.md index 17bde711..16ab1c93 100644 --- a/docs/https-app-gate-followup-20261006.md +++ b/docs/https-app-gate-followup-20261006.md @@ -134,3 +134,19 @@ behavior rather than treating a successful reload command as proof that nginx accepted the new configuration. The existing per-address retarget helper ignores wildcard-only configs. This live repair is not a claim that the general migration or IPv6 HTTPS/companion trust acceptance is complete. + +Source follow-up drafted after that rollout (not deployed): the bootstrap now +recognizes only the shipped wildcard node-CA dashboard profile for conversion to +present non-tailnet IPv4 listeners. Custom certificate/name/listener profiles are +left out of that new migration. Both available and enabled paths are visited, +canonical targets are deduplicated, symlinks retained, and listener backups are +placed outside nginx include directories. Listener repair precedes route repair. + +Reload acceptance now checks that the nginx service master produced a new active +worker generation; sending a successful reload signal alone no longer counts. +Five isolated shell-fixture regressions pass, exercising accepted reload, unchanged +workers, shutting-down workers, rejected configuration and failed signalling. +These run the exact embedded shell against fake service commands; they do not +reload a real node. Added Rust profile-migration tests and the integrated backend +compile remain pending the shared qualification slot. The live repaired nodes +still run the previously qualified 49703d7e binary. diff --git a/tests/regression/nginx-listener-reload.py b/tests/regression/nginx-listener-reload.py new file mode 100644 index 00000000..6eb885f9 --- /dev/null +++ b/tests/regression/nginx-listener-reload.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python3 +"""Exercise the exact bootstrap reload shell with isolated fake service commands. + +No actual nginx/systemd command is invoked. A successful reload signal must not +count as acceptance if the master leaves its old workers running. +""" +from pathlib import Path +import os +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[2] +SOURCE = ROOT / 'core/archipelago/src/bootstrap.rs' + + +class ReloadAcceptance(unittest.TestCase): + def run_reload(self, mode): + source = SOURCE.read_text() + function = source.split('async fn reload_nginx_checked()', 1)[1] + script = function.split('r#"', 1)[1].split('"#;', 1)[0] + with tempfile.TemporaryDirectory(prefix='nginx-reload-fixture-') as directory: + root = Path(directory) + commands = { + 'systemctl': '''#!/bin/sh +if [ "$1" = show ]; then echo 23; exit 0; fi +[ "$1" = reload ] || exit 90 +[ "$RELOAD_CASE" != signal_failure ] || exit 1 +touch "$FIXTURE_ROOT/reloaded" +''', + 'nginx': '#!/bin/sh\n[ "$RELOAD_CASE" != invalid_config ]\n', + 'ps': '''#!/bin/sh +[ "$1" = --ppid ] && [ "$2" = 23 ] || exit 91 +if [ "$RELOAD_CASE" = accepted ] && [ -f "$FIXTURE_ROOT/reloaded" ]; then + printf '102 nginx: worker process\n103 nginx: worker process\n' +elif [ "$RELOAD_CASE" = shutting_down ] && [ -f "$FIXTURE_ROOT/reloaded" ]; then + printf '100 nginx: worker process\n101 nginx: worker process\n102 nginx: worker process is shutting down\n' +else + printf '100 nginx: worker process\n101 nginx: worker process\n' +fi +''', + 'sleep': '#!/bin/sh\nexit 0\n', + } + for name, body in commands.items(): + path = root / name + path.write_text(body) + path.chmod(0o700) + env = {'PATH': f'{root}:/usr/bin:/bin', 'FIXTURE_ROOT': str(root), + 'RELOAD_CASE': mode, 'LC_ALL': 'C'} + result = subprocess.run(['/bin/sh', '-c', script], env=env, + capture_output=True, text=True, timeout=5) + signalled = (root / 'reloaded').exists() + return result.returncode, signalled + + def test_new_workers_accept_reload(self): + self.assertEqual(self.run_reload('accepted'), (0, True)) + + def test_successful_signal_without_new_workers_is_failure(self): + code, signalled = self.run_reload('bind_failure') + self.assertNotEqual(code, 0) + self.assertTrue(signalled) + + def test_shutting_down_workers_do_not_prove_acceptance(self): + self.assertNotEqual(self.run_reload('shutting_down')[0], 0) + + def test_invalid_config_never_signals(self): + code, signalled = self.run_reload('invalid_config') + self.assertNotEqual(code, 0) + self.assertFalse(signalled) + + def test_failed_signal_is_failure(self): + self.assertNotEqual(self.run_reload('signal_failure')[0], 0) + + +if __name__ == '__main__': + unittest.main()