From 4c84cc9e4bd919296d8bc5bac2e91c523ffaf61c Mon Sep 17 00:00:00 2001 From: tsunagi Date: Mon, 21 Sep 2026 20:24:11 +0100 Subject: [PATCH] Tell two networks of the same name apart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A status report with two sections both headed "network LAB" reads as one network that is somehow working and empty at once. It was two networks: the same name with different secrets, which is two different networks that share nothing, because a network's identity is its name *and* its secret. Three fixes for the one confusion. The heading now carries the network id, so the sections are plainly different things. A name is a label the user chose; the id is the identity. Joining a name that is already configured with another secret says so, at the moment it happens, because that is almost always a mistyped secret and until now it silently produced an empty network sitting beside a working one. `status` flags it too, for the case where it already happened. And the second network's emptiness now says why. It had no address because the only configured range was already taken by the first — one agent has one interface, so an address belongs to one network — and "nobody else has joined" pointed at the wrong thing entirely. It now names the range it cannot have, the reason, and the flag that gives it one of its own. Nothing was wrong with the connectivity: the working network's tunnel was up and its ping was answering throughout. Co-Authored-By: Claude Opus 5 (1M context) --- crates/tsunagi-cli/src/main.rs | 124 +++++++++++++++++++++++----- crates/tsunagi/src/agent/mod.rs | 41 +++++++-- crates/tsunagi/src/agent/network.rs | 5 ++ crates/tsunagi/src/agent/status.rs | 9 ++ crates/tsunagi/src/ipc/mod.rs | 7 +- crates/tsunagi/src/ipc/unix.rs | 2 +- 6 files changed, 160 insertions(+), 28 deletions(-) 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)