traffic: make apply and verify share one class-id assignment
apply_full_topology computed peer class ids inline while inspect_and_diff computed them through expected_peer_class_map. Two implementations of the same contract, and apply_full_topology ends by calling inspect_and_diff to verify its own work - so a divergence between them would surface as verify-after-apply failing on a topology that had just been applied correctly, with a reason pointing at class mapping rather than at the real cause. Apply now uses expected_peer_class_map, which makes it structurally impossible for the two sides to disagree. Behaviour is identical. Both paths assign class_start_id + index over peers in slice order, and discover_peers hands them over sorted lexicographically and deduplicated. Iterating the resulting BTreeMap walks the same keys in the same order, so the emitted tc commands and their class ids are unchanged; only the loop source moved. expected_peer_class_map is now the single source of truth for the class-id contract and gained the tests it never had: lexicographic ordering (10.8.1.10 before 10.8.1.2, the production contract that must not silently become numeric), a non-default class_start_id, u16 overflow, and the empty peer list. Found while splitting traffic.rs; kept out of that commit to keep the move reviewable. cargo test --workspace: 114 + 4 passed, four new. clippy -D warnings clean.
This commit is contained in:
@@ -6,7 +6,7 @@
|
||||
|
||||
use crate::{config::Config, netns::NsTransport, wg};
|
||||
|
||||
use super::inspect::inspect_and_diff;
|
||||
use super::inspect::{expected_peer_class_map, inspect_and_diff};
|
||||
|
||||
pub(super) fn teardown_owned_topology(
|
||||
transport: &dyn NsTransport,
|
||||
@@ -73,15 +73,11 @@ pub(super) fn apply_full_topology(
|
||||
],
|
||||
)?;
|
||||
|
||||
for (idx, p) in peers.iter().enumerate() {
|
||||
let idx_u16 = u16::try_from(idx)
|
||||
.map_err(|_| anyhow::anyhow!("too many peers for class id allocation"))?;
|
||||
let class_id = cfg
|
||||
.traffic
|
||||
.class_start_id
|
||||
.checked_add(idx_u16)
|
||||
.ok_or_else(|| anyhow::anyhow!("class id overflow: too many peers"))?;
|
||||
let ip = p.ipv4.to_string();
|
||||
// The same map the inspector will verify against, so apply and verify cannot disagree
|
||||
// about which class a peer belongs to.
|
||||
let class_map = expected_peer_class_map(peers, cfg.traffic.class_start_id)?;
|
||||
|
||||
for (ip, class_id) in &class_map {
|
||||
let class_egress = format!("1:{class_id}");
|
||||
let class_ingress = format!("2:{class_id}");
|
||||
let limit = cfg.traffic.peer_limit.as_str();
|
||||
|
||||
@@ -323,3 +323,45 @@ fn inspect_cmd(transport: &dyn NsTransport, args: &[&str], what: &str) -> anyhow
|
||||
.run(args)
|
||||
.map_err(|e| anyhow::anyhow!("{what} inspect failed: {e}"))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::net::Ipv4Addr;
|
||||
|
||||
fn peers(ips: &[&str]) -> Vec<wg::Peer> {
|
||||
ips.iter()
|
||||
.map(|v| wg::Peer {
|
||||
ipv4: v.parse::<Ipv4Addr>().expect("ip"),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Peers reach this function already sorted lexicographically by [`wg::discover_peers`],
|
||||
/// so `.2` follows `.10`. Assignment must not reorder them.
|
||||
#[test]
|
||||
fn class_ids_follow_lexicographic_peer_order() {
|
||||
let map = expected_peer_class_map(&peers(&["10.8.1.10", "10.8.1.2"]), 10).expect("map");
|
||||
assert_eq!(map.get("10.8.1.10").copied(), Some(10));
|
||||
assert_eq!(map.get("10.8.1.2").copied(), Some(11));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn class_ids_start_at_configured_offset() {
|
||||
let map = expected_peer_class_map(&peers(&["10.8.1.2", "10.8.1.3"]), 100).expect("map");
|
||||
assert_eq!(map.get("10.8.1.2").copied(), Some(100));
|
||||
assert_eq!(map.get("10.8.1.3").copied(), Some(101));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn class_id_overflow_is_reported() {
|
||||
let err = expected_peer_class_map(&peers(&["10.8.1.2", "10.8.1.3"]), u16::MAX)
|
||||
.expect_err("overflow");
|
||||
assert!(err.to_string().contains("class id overflow"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_peer_list_yields_empty_map() {
|
||||
assert!(expected_peer_class_map(&[], 10).expect("map").is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user