diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 478b9426..d307afd9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -303,6 +303,9 @@ jobs: exit 1 fi echo "FIPS_TEST_ADDRLESS_IFACE=fips-probe0" >> "$GITHUB_ENV" + # Declare that this runner has fixtures, so a test that depends on + # one fails when the fixture is missing instead of skipping silently. + echo "FIPS_TEST_REQUIRE_FIXTURES=1" >> "$GITHUB_ENV" - name: Install Rust toolchain uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 @@ -443,6 +446,9 @@ jobs: exit 1 fi echo "FIPS_TEST_ADDRLESS_IFACE=fips-probe0" >> "$GITHUB_ENV" + # Declare that this runner has fixtures, so a test that depends on + # one fails when the fixture is missing instead of skipping silently. + echo "FIPS_TEST_REQUIRE_FIXTURES=1" >> "$GITHUB_ENV" - name: Install Rust toolchain uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 diff --git a/src/transport/ethernet/mod.rs b/src/transport/ethernet/mod.rs index b962133f..84348e2f 100644 --- a/src/transport/ethernet/mod.rs +++ b/src/transport/ethernet/mod.rs @@ -1651,10 +1651,13 @@ mod tests { io::interface_present(loopback), "loopback must be present for this test to mean anything" ); - if PacketSocket::open(loopback, 0x2121).is_ok() { - // This host can open the socket, so there is no fault to observe. - return; - } + // Whether the socket opens splits the test into two halves, and both + // assert. Returning early on the privileged host — which is what this + // used to do — made the test vacuous as root and on any developer + // machine with a group-readable /dev/bpf*, so the fail-fast path it is + // named for went unchecked exactly where someone was most likely to be + // running it. + let can_open = PacketSocket::open(loopback, 0x2121).is_ok(); let config = EthernetConfig { interface: loopback.to_string(), @@ -1672,6 +1675,29 @@ mod tests { let (tx, _rx) = super::super::packet_channel(8); let mut eth = EthernetTransport::new(TransportId::new(9), None, config, tx); + if can_open { + // The privileged half. A present, bindable interface binds inline + // and reports itself bound before `start_async` returns — which is + // the ordinary case on a booted router, and which no other unit + // test reaches: every other one here uses an interface that does + // not exist, so the bind-success path has no unit coverage at all + // without this branch. + eth.start_async() + .await + .expect("a present, bindable interface must start"); + assert_eq!( + eth.presence(), + Presence::Present, + "a bind that succeeded must leave the transport present" + ); + assert!( + eth.binding.socket().is_some(), + "a present transport must hold its socket" + ); + eth.stop_async().await.expect("stop"); + return; + } + let err = eth .start_async() .await @@ -1853,7 +1879,18 @@ mod tests { /// perfectly well-addressed interface without anyone noticing. #[test] fn an_interface_with_no_addresses_is_still_present() { + // A silent skip is how this test spent its life green without ever + // running: no fixture, early return, pass. It still has to skip on a + // developer machine that has no address-less interface, so the guard + // is the runner declaring that it *does* — if the fixture step is + // removed or renamed, this fails instead of quietly covering nothing. let Ok(iface) = std::env::var("FIPS_TEST_ADDRLESS_IFACE") else { + assert!( + std::env::var_os("FIPS_TEST_REQUIRE_FIXTURES").is_none(), + "this runner sets FIPS_TEST_REQUIRE_FIXTURES but not \ + FIPS_TEST_ADDRLESS_IFACE: the fixture step did not run, and \ + the musl/glibc getifaddrs contract this pins went unchecked" + ); return; }; assert!( @@ -1864,7 +1901,14 @@ mod tests { ); // It has no carrier either — a dummy device is up but not running — // which pins that presence and carrier really are separate reads. - let _ = io::interface_carrier(&iface); + // Asserted rather than discarded: if these two ever collapsed into the + // same read, a carrier-less bridge would report absent and the whole + // IFF_UP-not-IFF_RUNNING decision would be silently undone. + assert!( + !io::interface_carrier(&iface), + "{iface} is up with no carrier, so presence and carrier must \ + disagree here — if they agree, they are the same read" + ); // And it resolves to an index, which is what a bind would attach to. assert!(io::interface_index(&iface).is_some()); } @@ -1999,6 +2043,90 @@ mod tests { assert!(eth.binding.socket().is_none()); } + #[tokio::test] + async fn a_restarted_transport_starts_its_binder_again() { + // `start_async` clears the stop flag so a restart is not immediately + // undone by the previous run's shutdown. Nothing tested the second + // start at all: `the_binder_stops_with_the_transport` only asserts a + // second *stop* errors, so a transport that could never be restarted + // would have passed everything here. + let (mut eth, _rx) = absent_transport(false); + + eth.start_async().await.expect("first start"); + eth.stop_async().await.expect("stop"); + assert!(eth.shutdown.load(Ordering::SeqCst), "stop raises the flag"); + + eth.start_async() + .await + .expect("a stopped transport restarts"); + assert!( + !eth.shutdown.load(Ordering::SeqCst), + "the restart must clear the previous run's stop, or the new binder \ + tears its own binding down on its first pass" + ); + assert_eq!(eth.state(), TransportState::Up); + + eth.stop_async().await.expect("stop again"); + } + + #[tokio::test] + async fn the_absence_deadline_is_measured_from_the_start() { + // `PresenceState::new` stamps the episode clock at construction, but + // construction and `start_async` need not be adjacent — config load and + // supervisor staging sit between them. Without the restamp a transport + // staged for longer than the window reports sustained absence on its + // very first binder tick, having given the interface no bring-up window + // at all, which is the one thing the window exists to provide. + let (mut eth, _rx) = absent_transport(false); + + // Stand in for a slow bring-up by ageing the clock past the deadline. + std::thread::sleep(Duration::from_millis(20)); + let staged_for = eth.presence.since(); + + eth.start_async().await.expect("start"); + assert!( + eth.presence.since() < staged_for, + "the episode clock must restart at start, not run from whenever \ + the object happened to be constructed" + ); + + eth.stop_async().await.expect("stop"); + } + + #[tokio::test] + async fn a_refused_presence_edge_is_delivered_once_there_is_room() { + // The retry slot, which nothing followed through. The existing test + // fills the channel and asserts the binder keeps running, then drops + // the receiver — so a slot that captured the edge and never re-sent it + // would pass, and health would sit on a stale level forever. + let (mut eth, _rx) = absent_transport(false); + let (tx, mut presence_rx) = tokio::sync::mpsc::channel(1); + eth.set_presence_tx(tx.clone()); + + // Occupy the only slot, so the start edge is refused on its way out. + tx.try_send(TransportPresence { + transport_id: TransportId::new(99), + present: true, + health_relevant: true, + }) + .expect("the one slot"); + + eth.start_async().await.expect("start"); + + // Drain the squatter. The binder now has room on its next pass. + let squatter = presence_rx.recv().await.expect("squatter"); + assert_eq!(squatter.transport_id, TransportId::new(99)); + + let edge = tokio::time::timeout(Duration::from_secs(5), presence_rx.recv()) + .await + .expect("the refused edge must be retried, not dropped") + .expect("channel open"); + assert_eq!(edge.transport_id, TransportId::new(1)); + assert!(!edge.present, "the retried edge is the absence it refused"); + + eth.stop_async().await.expect("stop"); + } + #[tokio::test] async fn a_full_presence_channel_does_not_block_the_binder() { // The health channel must not be able to deadlock the machine whose diff --git a/src/transport/ethernet/presence.rs b/src/transport/ethernet/presence.rs index c79c738c..644ad4c1 100644 --- a/src/transport/ethernet/presence.rs +++ b/src/transport/ethernet/presence.rs @@ -768,5 +768,72 @@ mod tests { assert!(AbsencePolicy::Optional.is_optional()); assert!(!AbsencePolicy::Required.is_optional()); assert_eq!(AbsencePolicy::Required.as_str(), "required"); + // Both labels, not just one. `show_transports` renders this string and + // fipstop's severity split keys on it, so a swapped pair would paint + // every expected interface as the tolerated kind and vice versa — + // while a test that checks only `Required` stays green through it. + assert_eq!(AbsencePolicy::Optional.as_str(), "optional"); + } + + #[test] + fn a_first_bind_is_not_a_hardware_change() { + // The boundary the flush hangs off. `record_bind` returns "different + // hardware", and on the very first bind there is no previous MAC to + // differ from — so it must answer false, or every clean start would + // drop a neighbour cache it had just built and log a hardware swap + // that never happened. + let state = PresenceState::new(); + assert!( + !state.record_bind([1, 2, 3, 4, 5, 6]), + "the first bind has nothing to differ from" + ); + assert_eq!(state.binds(), 1); + assert_eq!(state.presence(), Presence::Present); + } + + #[test] + fn a_rebind_on_new_hardware_reports_the_change_once() { + // And it reports the change once, not on every subsequent bind: the + // caller drops its cached neighbours on a `true`, so a sticky answer + // would flush the cache on every rebind forever. + let state = PresenceState::new(); + state.record_bind([1, 2, 3, 4, 5, 6]); + + assert!( + state.record_bind([0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff]), + "a name returning on a different MAC is different hardware" + ); + assert!( + !state.record_bind([0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff]), + "the same hardware rebinding is not a change" + ); + assert_eq!(state.binds(), 3); + } + + #[test] + fn every_presence_label_is_distinct_and_round_trips() { + // The labels are the operator-facing vocabulary — `show_transports` + // emits them and the fipstop State column renders them — and + // `Presence::as_str` had no test at all, so `binding` in particular was + // never observed by anything. + use std::collections::HashSet; + let all = [Presence::Absent, Presence::Binding, Presence::Present]; + let labels: HashSet<&str> = all.iter().map(|p| p.as_str()).collect(); + assert_eq!(labels.len(), 3, "each phase needs its own label"); + assert!(labels.contains("binding")); + + for phase in all { + assert_eq!( + Presence::from_u8(phase.as_u8()), + phase, + "{phase} must survive the atomic round trip the state uses" + ); + assert_eq!(phase.to_string(), phase.as_str(), "Display must agree"); + } + + // Anything outside the enum reads as absent rather than panicking: the + // byte comes out of an AtomicU8 that a torn write could leave at any + // value, and the safe answer there is "not bound". + assert_eq!(Presence::from_u8(99), Presence::Absent); } }