fix(orchestrator): generalize launch-port fallback + archival-bitcoin dependency gating
Master-plan backlog §10b/§10c: replace two per-app-hardcoded lookups with generic, manifest-driven behavior so future apps are covered automatically instead of needing a code edit. - extract_lan_address (docker_packages.rs) now skips container-side ports that are known non-HTTP (SSH, FTP, common DB ports) instead of blindly taking podman's first-listed port. Fixes the whole class of bug the gitea SSH-before-web static override was a one-off patch for. - requires_unpruned_bitcoin (dependencies.rs) now checks the app's own manifest for a `bitcoin:archival` dependency declaration first, falling back to the old hardcoded id list. electrumx and mempool manifests now declare it explicitly as the proof case. 869/869 Rust tests green, catalog drift clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
46dae75a0f
commit
306b6356ee
@ -22,6 +22,7 @@ app:
|
||||
- app_id: bitcoin-knots
|
||||
version: ">=26.0"
|
||||
- storage: 50Gi
|
||||
- bitcoin:archival
|
||||
|
||||
resources:
|
||||
cpu_limit: 0
|
||||
|
||||
@ -13,6 +13,7 @@ app:
|
||||
- app_id: bitcoin-core
|
||||
version: ">=24.0"
|
||||
- storage: 20Gi
|
||||
- bitcoin:archival
|
||||
|
||||
resources:
|
||||
cpu_limit: 2
|
||||
|
||||
@ -1,6 +1,8 @@
|
||||
use super::config::get_containers_for_app;
|
||||
use super::runtime::manifest_apps_dirs;
|
||||
use crate::data_model::{PackageDataEntry, PackageState};
|
||||
use anyhow::{Context, Result};
|
||||
use archipelago_container::{AppManifest, Dependency};
|
||||
use std::collections::HashMap;
|
||||
use tracing::info;
|
||||
|
||||
@ -11,7 +13,38 @@ const BITCOIN_NAMES: &[&str] = &["bitcoin-knots", "bitcoin-core", "bitcoin"];
|
||||
const ELECTRUM_NAMES: &[&str] = &["electrumx", "mempool-electrs", "electrs"];
|
||||
const ARCHIVAL_BITCOIN_DISK_GB: u64 = 1000;
|
||||
|
||||
/// The manifest string dependency that declares "needs an archival
|
||||
/// (unpruned + txindex) Bitcoin node" — see `manifest_declares_archival_bitcoin`.
|
||||
const ARCHIVAL_BITCOIN_DEPENDENCY: &str = "bitcoin:archival";
|
||||
|
||||
/// Whether `package_id`'s own on-disk manifest declares
|
||||
/// `dependencies: [bitcoin:archival]`. Manifest-driven alternative to the
|
||||
/// hardcoded id list below — a new app just declares the dependency instead
|
||||
/// of needing a code change here.
|
||||
fn manifest_declares_archival_bitcoin(package_id: &str) -> bool {
|
||||
for apps_dir in manifest_apps_dirs() {
|
||||
let path = apps_dir.join(package_id).join("manifest.yml");
|
||||
let Ok(contents) = std::fs::read_to_string(&path) else {
|
||||
continue;
|
||||
};
|
||||
let Ok(manifest) = AppManifest::parse(&contents) else {
|
||||
continue;
|
||||
};
|
||||
return dependency_list_declares_archival_bitcoin(&manifest.app.dependencies);
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
fn dependency_list_declares_archival_bitcoin(deps: &[Dependency]) -> bool {
|
||||
deps.iter()
|
||||
.any(|dep| matches!(dep, Dependency::Simple(s) if s == ARCHIVAL_BITCOIN_DEPENDENCY))
|
||||
}
|
||||
|
||||
fn requires_unpruned_bitcoin(package_id: &str) -> bool {
|
||||
if manifest_declares_archival_bitcoin(package_id) {
|
||||
return true;
|
||||
}
|
||||
// Fallback for apps not yet migrated to the manifest declaration above.
|
||||
matches!(
|
||||
package_id,
|
||||
"electrumx" | "mempool-electrs" | "electrs" | "mempool" | "mempool-web"
|
||||
@ -473,7 +506,11 @@ pub(super) fn configure_fedimint_lnd(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{order_present_containers, requires_unpruned_bitcoin, startup_order};
|
||||
use super::{
|
||||
dependency_list_declares_archival_bitcoin, manifest_declares_archival_bitcoin,
|
||||
order_present_containers, requires_unpruned_bitcoin, startup_order,
|
||||
};
|
||||
use archipelago_container::Dependency;
|
||||
|
||||
#[test]
|
||||
fn order_present_containers_never_injects_phantom_stack_members() {
|
||||
@ -547,4 +584,44 @@ mod tests {
|
||||
assert!(!requires_unpruned_bitcoin(package_id), "{package_id}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dependency_matcher_finds_the_archival_marker_among_other_deps() {
|
||||
let deps = vec![
|
||||
Dependency::App {
|
||||
app_id: "bitcoin-knots".to_string(),
|
||||
version: Some(">=26.0".to_string()),
|
||||
},
|
||||
Dependency::Storage {
|
||||
storage: "50Gi".to_string(),
|
||||
},
|
||||
Dependency::Simple("bitcoin:archival".to_string()),
|
||||
];
|
||||
assert!(dependency_list_declares_archival_bitcoin(&deps));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dependency_matcher_false_when_marker_absent() {
|
||||
let deps = vec![Dependency::App {
|
||||
app_id: "bitcoin-knots".to_string(),
|
||||
version: Some(">=26.0".to_string()),
|
||||
}];
|
||||
assert!(!dependency_list_declares_archival_bitcoin(&deps));
|
||||
assert!(!dependency_list_declares_archival_bitcoin(&[]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn manifest_declared_archival_bitcoin_covers_a_new_app_without_a_code_change() {
|
||||
// electrumx and mempool declare `dependencies: [..., bitcoin:archival]`
|
||||
// on disk (apps/electrumx/manifest.yml, apps/mempool/manifest.yml) —
|
||||
// this is the manifest-driven path working end-to-end, not the
|
||||
// hardcoded id list. A future app only needs this manifest line, no
|
||||
// edit to `requires_unpruned_bitcoin`.
|
||||
assert!(manifest_declares_archival_bitcoin("electrumx"));
|
||||
assert!(manifest_declares_archival_bitcoin("mempool"));
|
||||
// An app whose manifest exists but never declares the marker.
|
||||
assert!(!manifest_declares_archival_bitcoin("bitcoin-knots"));
|
||||
// An id with no manifest on disk at all.
|
||||
assert!(!manifest_declares_archival_bitcoin("does-not-exist"));
|
||||
}
|
||||
}
|
||||
|
||||
@ -1603,7 +1603,7 @@ fn manifest_host_ports(container_name: &str) -> Vec<u16> {
|
||||
Vec::new()
|
||||
}
|
||||
|
||||
fn manifest_apps_dirs() -> Vec<std::path::PathBuf> {
|
||||
pub(super) fn manifest_apps_dirs() -> Vec<std::path::PathBuf> {
|
||||
let mut dirs = Vec::new();
|
||||
if let Ok(manifest_dir) = std::env::var("CARGO_MANIFEST_DIR") {
|
||||
dirs.push(Path::new(&manifest_dir).join("../../apps"));
|
||||
|
||||
@ -677,18 +677,43 @@ pub async fn read_tor_address(app_id: &str) -> Option<String> {
|
||||
.filter(|s| s.ends_with(".onion") && !s.is_empty())
|
||||
}
|
||||
|
||||
/// Container-side ports that are essentially never a web UI, even when
|
||||
/// published alongside one — e.g. gitea publishes SSH (`2222->22`) before its
|
||||
/// web port (`3001->3000`), and podman's port list order isn't guaranteed to
|
||||
/// put the UI port first. Skipping these lets launch-URL guessing work for
|
||||
/// any future multi-port app without a per-app static override.
|
||||
const NON_HTTP_CONTAINER_PORTS: &[&str] = &["22", "21", "3306", "5432", "6379", "27017"];
|
||||
|
||||
fn extract_lan_address(ports: &[String]) -> Option<String> {
|
||||
let mut first_candidate = None;
|
||||
for port_str in ports {
|
||||
// Parse port strings like "0.0.0.0:18443->18443/tcp" or "0.0.0.0:18443-18444->18443-18444/tcp"
|
||||
if let Some(public_part) = port_str.split("->").next() {
|
||||
if let Some(port_part) = public_part.split(':').nth(1) {
|
||||
// Extract just the first port if it's a range (e.g., "18443-18444" -> "18443")
|
||||
let single_port = port_part.split('-').next().unwrap_or(port_part);
|
||||
return Some(format!("http://localhost:{}", single_port));
|
||||
}
|
||||
let Some(public_part) = port_str.split("->").next() else {
|
||||
continue;
|
||||
};
|
||||
let Some(port_part) = public_part.split(':').nth(1) else {
|
||||
continue;
|
||||
};
|
||||
// Extract just the first port if it's a range (e.g., "18443-18444" -> "18443")
|
||||
let host_port = port_part.split('-').next().unwrap_or(port_part);
|
||||
let candidate = format!("http://localhost:{}", host_port);
|
||||
if first_candidate.is_none() {
|
||||
first_candidate = Some(candidate.clone());
|
||||
}
|
||||
|
||||
let container_port = port_str
|
||||
.split("->")
|
||||
.nth(1)
|
||||
.and_then(|s| s.split('/').next())
|
||||
.map(|s| s.split('-').next().unwrap_or(s));
|
||||
if container_port.is_some_and(|p| NON_HTTP_CONTAINER_PORTS.contains(&p)) {
|
||||
continue;
|
||||
}
|
||||
return Some(candidate);
|
||||
}
|
||||
None
|
||||
// Nothing looked HTTP-like — fall back to whatever was published first
|
||||
// rather than reporting no launch URL at all.
|
||||
first_candidate
|
||||
}
|
||||
|
||||
/// netbird's dashboard launch URL: HTTPS on 8087 (the proxy terminates TLS —
|
||||
@ -858,3 +883,54 @@ mod launch_url_port_tests {
|
||||
assert_eq!(launch_url_port("http://localhost/"), None);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod extract_lan_address_tests {
|
||||
use super::extract_lan_address;
|
||||
|
||||
#[test]
|
||||
fn skips_ssh_port_when_web_port_is_published() {
|
||||
// gitea: SSH published before the web port, in podman's list order.
|
||||
let ports = vec![
|
||||
"0.0.0.0:2222->22/tcp".to_string(),
|
||||
"0.0.0.0:3001->3000/tcp".to_string(),
|
||||
];
|
||||
assert_eq!(
|
||||
extract_lan_address(&ports).as_deref(),
|
||||
Some("http://localhost:3001")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn falls_back_to_first_port_when_nothing_looks_like_http() {
|
||||
let ports = vec!["0.0.0.0:2222->22/tcp".to_string()];
|
||||
assert_eq!(
|
||||
extract_lan_address(&ports).as_deref(),
|
||||
Some("http://localhost:2222")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_http_port_still_resolves() {
|
||||
let ports = vec!["0.0.0.0:8096->8096/tcp".to_string()];
|
||||
assert_eq!(
|
||||
extract_lan_address(&ports).as_deref(),
|
||||
Some("http://localhost:8096")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn handles_port_ranges() {
|
||||
let ports = vec!["0.0.0.0:18443-18444->18443-18444/tcp".to_string()];
|
||||
assert_eq!(
|
||||
extract_lan_address(&ports).as_deref(),
|
||||
Some("http://localhost:18443")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_ports_returns_none() {
|
||||
let ports: Vec<String> = vec![];
|
||||
assert_eq!(extract_lan_address(&ports), None);
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user