diff --git a/crates/tsunagi-cli/src/main.rs b/crates/tsunagi-cli/src/main.rs index 2ccb3a7..36ea64e 100644 --- a/crates/tsunagi-cli/src/main.rs +++ b/crates/tsunagi-cli/src/main.rs @@ -1140,7 +1140,15 @@ async fn status(args: StatusArgs) -> Result<(), Box> { match &observed { Observed::Agent(report) => { for network in &report.networks { - out.push(network_section(network, &report.endpoint_id)); + // Whether another configured network answers to the same + // name, which is what makes two sections look like one. + let shared = report + .networks + .iter() + .filter(|other| other.name == network.name) + .count() + > 1; + out.push(network_section(network, &report.endpoint_id, shared)); } } // Without an agent there is no live view, but the store still knows @@ -1306,23 +1314,56 @@ fn member_rows<'a>(network: &'a tsunagi::ipc::NetworkReport, own_id: &str) -> Ve } /// One network: what it is, who is in it, and what has happened since start. -fn network_section(network: &tsunagi::ipc::NetworkReport, own_id: &str) -> report::Section { +fn network_section( + network: &tsunagi::ipc::NetworkReport, + own_id: &str, + name_shared: bool, +) -> report::Section { use report::{Health, Row, Section}; - let mut section = Section::new(format!("network {}", network.name)); + // The id is in the heading, not only in a row: a name is a label a user + // chose and two networks may share one, so a heading without the id + // reads as one network that is somehow both working and empty. + let mut section = Section::new(format!( + "network {} ({})", + network.name, + short(&network.network_id, 10) + )); section.push(if network.active { - Row::new( - Health::Good, - "state", - format!("active {}", network.network_id), - ) + Row::new(Health::Good, "state", network.network_id.clone()) } else { - Row::new( - Health::Degraded, - "state", - format!("inactive {}", network.network_id), - ) + Row::new(Health::Degraded, "state", "inactive") }); + if name_shared { + section.push( + Row::new( + Health::Degraded, + "name", + format!( + "another configured network is also called `{}`", + network.name + ), + ) + .with_note( + "a network is its name *and* its secret, so these two share nothing. \ + Usually a mistyped secret; `tsunagi id secret` shows which is which.", + ), + ); + } + if let Some(conflict) = &network.range_conflict { + section.push( + Row::new( + Health::Degraded, + "range", + format!("cannot use {conflict}: another network here already does"), + ) + .with_note( + "one agent has one interface, so an address belongs to one network. \ + This one waits to adopt whatever its members settle on; give it \ + `--ipv4-range` of its own to propose one.", + ), + ); + } if let Some(overlay) = &network.overlay { section.push(Row::new( @@ -1343,10 +1384,15 @@ fn network_section(network: &tsunagi::ipc::NetworkReport, own_id: &str) -> repor let rows = member_rows(network, own_id); let online = rows.iter().filter(|row| row.online()).count(); if rows.is_empty() { + // Why there is nobody, rather than just that there is nobody: the + // two reasons want different actions. section.push(Row::new( Health::Info, "members", - "none known yet; nobody else has joined", + match (&network.range, &network.range_conflict) { + (None, Some(_)) => "none: this network has no range to allocate from", + _ => "none known yet; nobody else has joined", + }, )); } else { section.push(Row::new( @@ -2317,6 +2363,8 @@ async fn build_report( .map_or(0, |candidate| candidate.consecutive_failures), }) .collect(), + range: network.range.map(|range| range.to_string()), + range_conflict: network.range_conflict.map(|range| range.to_string()), dial_failures: network.metrics.dial_failures, handshake_failures: network.metrics.handshake_failures, control_messages: ( @@ -2580,6 +2628,8 @@ mod status_tests { failed_dials: 0, }, ], + range: Some("10.13.37.0/24".into()), + range_conflict: None, // Everything below happened while the peer was away. dial_failures: 9, handshake_failures: 0, @@ -2592,6 +2642,42 @@ mod status_tests { } } + #[test] + fn two_networks_with_one_name_are_told_apart_and_flagged() { + // The confusing case: two sections headed identically, one working + // and one empty, read as a single network that is somehow both. + let network = network_after_a_peer_returned(); + let mut out = report::Report::new(); + out.push(network_section(&network, OWN, true)); + let text = out.render(false); + + assert!( + text.contains(&format!("network LAB ({})", short(&network.network_id, 10))), + "the heading must identify the network, not just name it:\n{text}" + ); + assert!(text.contains("also called `LAB`"), "{text}"); + assert!(text.contains("mistyped secret"), "{text}"); + } + + #[test] + fn a_network_with_no_range_says_that_is_why_it_is_empty() { + // Rather than "nobody else has joined", which points at the wrong + // thing entirely: nobody can join a network with no addresses. + let mut network = network_after_a_peer_returned(); + network.peers.clear(); + network.members.clear(); + network.overlay = None; + network.range = None; + network.range_conflict = Some("10.13.37.0/24".into()); + + let mut out = report::Report::new(); + out.push(network_section(&network, OWN, false)); + let text = out.render(false); + assert!(text.contains("no range to allocate from"), "{text}"); + assert!(text.contains("another network here already does"), "{text}"); + assert!(text.contains("--ipv4-range"), "the fix is named: {text}"); + } + #[test] fn counters_from_the_past_do_not_grade_the_present() { // A peer that left and returned leaves dial failures and a packet @@ -2600,7 +2686,7 @@ mod status_tests { // network look broken. let network = network_after_a_peer_returned(); let mut out = report::Report::new(); - out.push(network_section(&network, OWN)); + out.push(network_section(&network, OWN, false)); assert_eq!(out.worst(), Health::Good, "{}", out.render(false)); let text = out.render(false); @@ -2649,7 +2735,7 @@ mod status_tests { }); let mut out = report::Report::new(); - out.push(network_section(&network, OWN)); + out.push(network_section(&network, OWN, false)); let text = out.render(false); assert_eq!(out.worst(), Health::Good, "{text}"); @@ -2665,7 +2751,7 @@ mod status_tests { network.peers[0].transport = "relay".into(); let mut out = report::Report::new(); - out.push(network_section(&network, OWN)); + out.push(network_section(&network, OWN, false)); assert_eq!(out.worst(), Health::Degraded, "{}", out.render(false)); assert!(out.render(false).contains("relay")); } @@ -2676,7 +2762,7 @@ mod status_tests { network.overlay = Some(overlay(vec![tunnel(ONLINE, None)])); let mut out = report::Report::new(); - out.push(network_section(&network, OWN)); + out.push(network_section(&network, OWN, false)); let text = out.render(false); assert_eq!(out.worst(), Health::Degraded, "{text}"); assert!(text.contains("no WireGuard handshake yet"), "{text}"); @@ -2692,7 +2778,7 @@ mod status_tests { network.handshake_failures = 4; let mut out = report::Report::new(); - out.push(network_section(&network, OWN)); + out.push(network_section(&network, OWN, false)); let text = out.render(false); assert_eq!(out.worst(), Health::Degraded, "{text}"); assert!(text.contains("same secret"), "{text}"); diff --git a/crates/tsunagi/src/agent/mod.rs b/crates/tsunagi/src/agent/mod.rs index e07acb8..eacf002 100644 --- a/crates/tsunagi/src/agent/mod.rs +++ b/crates/tsunagi/src/agent/mod.rs @@ -48,6 +48,7 @@ use crate::net::EndpointAdapter; use crate::overlay::PacketCarrier; use crate::proto::handshake; use crate::proto::message::ControlMessage; +use crate::state::Ipv4Range; use crate::storage::{CacheOutcome, Storage}; use network::{InboundSession, NetCommand, NetworkHandle, RuntimeParams}; @@ -353,18 +354,20 @@ impl Agent { /// waits to adopt whatever it settles on. One agent has one interface, so /// proposing a range it could not route would be worse than having none: /// the lowest author's range wins, and the collision would spread. - fn reserve_range(&self, network: NetworkId) -> Option { - let wanted = self.inner.config.overlay_ipv4_range?; + fn reserve_range(&self, network: NetworkId) -> (Option, Option) { + let Some(wanted) = self.inner.config.overlay_ipv4_range else { + return (None, None); + }; let reservation = crate::overlay::NetworkRoutes { range: Some(wanted), local: None, peers: Vec::new(), }; match self.inner.routes.set_network(network, reservation) { - Ok(()) => Some(wanted), + Ok(()) => (Some(wanted), None), Err(err) => { tracing::info!(%err, "not proposing a range for this network"); - None + (None, Some(wanted)) } } } @@ -456,6 +459,26 @@ impl Agent { ) -> Result { let keys = NetworkKeys::derive(name, secret); let network_id = keys.network_id(); + + // A network's identity is its name *and* its secret, so the same + // name with a different secret is a different network — and one that + // looks identical in anything that shows a name. Almost always a + // mistyped secret, so it is said out loud rather than left to be + // discovered as an empty network sitting beside a working one. + if let Ok(configured) = self.inner.storage.list_networks().await { + for other in configured { + if other.name == *name && other.network_id != network_id { + tracing::warn!( + "`{name}` is already configured with a different secret, as {}. \ + Joining with this one adds a second network under the same name; \ + they share nothing. Check the secret, or use `tsunagi id secret` \ + to see which is which.", + other.network_id + ); + } + } + } + self.inner .storage .upsert_network(network_id, name.clone(), secret.clone(), true) @@ -495,6 +518,7 @@ impl Agent { return Err(Error::NetworkAlreadyActive(network_id)); } + let (reserved, conflict) = self.reserve_range(network_id); let handle = network::spawn(RuntimeParams { keys, adapter: self.inner.adapter.clone(), @@ -510,7 +534,8 @@ impl Agent { hostname: self.inner.read_hostname(), transport: self.inner.transport.get().cloned(), device_secret: self.inner.identity.signing_key(), - ipv4_range: self.reserve_range(network_id), + ipv4_range: reserved, + range_conflict: conflict, }); networks.insert(network_id, handle); drop(networks); @@ -658,9 +683,11 @@ impl Agent { state: NetworkState::Inactive, peers: Vec::new(), candidates: Vec::new(), - // An inactive network has no runtime to ask; the roster comes - // from one. Empty, not invented. + // An inactive network has no runtime to ask; the roster and + // the range come from one. Empty, not invented. members: Vec::new(), + range: None, + range_conflict: None, metrics: NetworkMetrics::default(), }); } diff --git a/crates/tsunagi/src/agent/network.rs b/crates/tsunagi/src/agent/network.rs index c837445..7e6d32b 100644 --- a/crates/tsunagi/src/agent/network.rs +++ b/crates/tsunagi/src/agent/network.rs @@ -130,6 +130,9 @@ pub(crate) struct RuntimeParams { /// The IPv4 overlay range this agent would use, if the network has not /// already settled on another one. pub(crate) ipv4_range: Option, + /// The range it was configured with but cannot have, because another + /// network on this agent holds it. + pub(crate) range_conflict: Option, /// How data plane links are opened. `None` disables the data plane. pub(crate) transport: Option>, } @@ -1439,6 +1442,8 @@ impl Runtime { peers, candidates, members, + range: self.effective_range(), + range_conflict: self.params.range_conflict, metrics: self.metrics.clone(), } } diff --git a/crates/tsunagi/src/agent/status.rs b/crates/tsunagi/src/agent/status.rs index 3030a47..203e196 100644 --- a/crates/tsunagi/src/agent/status.rs +++ b/crates/tsunagi/src/agent/status.rs @@ -159,6 +159,15 @@ pub struct NetworkStatus { pub candidates: Vec, /// Members the signed state knows about, whether connected or not. pub members: Vec, + /// The overlay range this network uses, once it has one. + pub range: Option, + /// The range it could not have, because another network on this agent + /// already holds it. + /// + /// One agent has one interface, so an address belongs to one network. + /// This network waits to adopt whatever its members settle on instead of + /// proposing something it could not route. + pub range_conflict: Option, /// Per-network counters. pub metrics: NetworkMetrics, } diff --git a/crates/tsunagi/src/ipc/mod.rs b/crates/tsunagi/src/ipc/mod.rs index 63c19aa..29e0101 100644 --- a/crates/tsunagi/src/ipc/mod.rs +++ b/crates/tsunagi/src/ipc/mod.rs @@ -149,7 +149,12 @@ pub struct NetworkReport { pub handshake_failures: u64, /// Control messages sent and received. pub control_messages: (u64, u64), - /// The overlay, when an IP plugin is running one. + /// The overlay range this network uses, once it has one. + pub range: Option, + /// The range it could not have, because another network on this agent + /// already holds it. + pub range_conflict: Option, + /// The overlay, when a protocol is running one. pub overlay: Option, } diff --git a/crates/tsunagi/src/ipc/unix.rs b/crates/tsunagi/src/ipc/unix.rs index 97f339c..fbacc62 100644 --- a/crates/tsunagi/src/ipc/unix.rs +++ b/crates/tsunagi/src/ipc/unix.rs @@ -205,7 +205,7 @@ pub async fn set_hostname(path: impl AsRef, hostname: &str) -> Result(stream: &mut UnixStream, value: &T) -> Result<()> { let encoded = postcard::to_stdvec(value)