diff --git a/crates/integration-tests/Cargo.toml b/crates/integration-tests/Cargo.toml index ca4ef8b..1c39cd2 100644 --- a/crates/integration-tests/Cargo.toml +++ b/crates/integration-tests/Cargo.toml @@ -18,13 +18,13 @@ bgp-server.workspace = true bgp-web.workspace = true bytes.workspace = true eyre.workspace = true -tokio.workspace = true - -# Used by the examples and the tests only. -[dev-dependencies] -clap.workspace = true reqwest.workspace = true serde_json.workspace = true +tokio.workspace = true + +# Used by the examples only. +[dev-dependencies] +clap.workspace = true [lints] workspace = true diff --git a/crates/integration-tests/examples/fake_peer.rs b/crates/integration-tests/examples/fake_peer.rs index d986fa0..1fa9c5d 100644 --- a/crates/integration-tests/examples/fake_peer.rs +++ b/crates/integration-tests/examples/fake_peer.rs @@ -16,11 +16,9 @@ use std::net::Ipv4Addr; use std::time::Duration; +use bgp_integration_tests::announcement::{Announcement, withdrawal}; use bgp_integration_tests::test_peer::TestPeer; -use bgp_packet::message::{ - AsPathAttribute, CommunitiesPathAttribute, Message, NextHopPathAttribute, OriginPathAttribute, - PathAttribute, UpdateMessage, -}; +use bgp_packet::message::{Message, PathAttribute, UpdateMessage}; use bgp_primitives::ip_prefix::IpPrefix; use clap::Parser; use eyre::Result; @@ -88,8 +86,8 @@ async fn run(args: &Args, mut peer: TestPeer, addr: std::net::SocketAddr) -> Res for p in &args.announce { let prefix = IpPrefix::try_from(p.as_str())?; - let update = announce(prefix, vec![args.asn], vec![(args.asn as u16, 1)]); - peer.send(&Message::Update(update)).await?; + peer.announce(announce(prefix, vec![args.asn], vec![(args.asn as u16, 1)])) + .await?; println!("announced {prefix}"); } @@ -98,7 +96,8 @@ async fn run(args: &Args, mut peer: TestPeer, addr: std::net::SocketAddr) -> Res loop { tokio::select! { _ = ticker.tick() => peer.send(&Message::KeepAlive).await?, - msg = peer.recv() => match msg? { + // However long the server is quiet: this peer stays on the line. + msg = peer.recv_untimed() => match msg? { Message::KeepAlive => {} Message::Update(u) => { for w in &u.withdrawn_routes { @@ -121,22 +120,10 @@ async fn run(args: &Args, mut peer: TestPeer, addr: std::net::SocketAddr) -> Res } } -fn announce(prefix: IpPrefix, as_path: Vec, communities: Vec<(u16, u16)>) -> UpdateMessage { - let mut path_attributes = vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(as_path), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(10, 0, 0, 2))), - ]; - if !communities.is_empty() { - path_attributes.push(PathAttribute::Communities(CommunitiesPathAttribute( - communities, - ))); - } - UpdateMessage { - withdrawn_routes: vec![], - path_attributes, - announced_routes: vec![prefix.into()], - } +fn announce(prefix: IpPrefix, as_path: Vec, communities: Vec<(u16, u16)>) -> Announcement { + Announcement::of([prefix.into()], &as_path) + .with_next_hop(Ipv4Addr::new(10, 0, 0, 2)) + .with_communities(&communities) } async fn echo_back(peer: &mut TestPeer, u: &UpdateMessage, asn: u32) -> Result<()> { @@ -157,21 +144,13 @@ async fn echo_back(peer: &mut TestPeer, u: &UpdateMessage, asn: u32) -> Result<( } for nlri in &u.announced_routes { let prefix = nlri.prefix; - peer.send(&Message::Update(announce( - prefix, - as_path.clone(), - communities.clone(), - ))) - .await?; + peer.announce(announce(prefix, as_path.clone(), communities.clone())) + .await?; println!(" echoed {prefix} back with path {as_path:?}"); } if !u.withdrawn_routes.is_empty() { - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: u.withdrawn_routes.clone(), - path_attributes: vec![], - announced_routes: vec![], - })) - .await?; + peer.announce(withdrawal(u.withdrawn_routes.iter().cloned())) + .await?; println!( " echoed withdraw of {:?}", u.withdrawn_routes diff --git a/crates/integration-tests/src/announcement.rs b/crates/integration-tests/src/announcement.rs new file mode 100644 index 0000000..d61ade2 --- /dev/null +++ b/crates/integration-tests/src/announcement.rs @@ -0,0 +1,110 @@ +//! The UPDATEs a test peer sends, built from what a test has to say about +//! them and nothing more. + +use std::net::Ipv4Addr; + +use bgp_packet::message::{ + AsPathAttribute, CommunitiesPathAttribute, Message, NextHopPathAttribute, OriginPathAttribute, + PathAttribute, UpdateMessage, +}; +use bgp_primitives::ip_prefix::Nlri; + +use crate::prefix; + +/// An UPDATE announcing IPv4 routes as the peers of these tests do: with +/// ORIGIN IGP, the AS path given, and 192.0.2.1 as the next hop unless +/// another is set. Attributes are written in order of type code. +#[derive(Debug, Clone)] +pub struct Announcement { + routes: Vec, + path: Vec, + next_hop: Ipv4Addr, + only_to_customer: Option, + communities: Vec<(u16, u16)>, + more: Vec, +} + +/// Announces `prefixes`, written as text, with the AS path `path`. +pub fn announcement(prefixes: &[&str], path: &[u32]) -> Announcement { + Announcement::of(prefixes.iter().map(|p| prefix(p).into()), path) +} + +/// Withdraws `routes`. +pub fn withdrawal(routes: impl IntoIterator) -> UpdateMessage { + UpdateMessage { + withdrawn_routes: routes.into_iter().collect(), + path_attributes: vec![], + announced_routes: vec![], + } +} + +impl Announcement { + /// Announces `routes`, prefixes with or without path identifiers, with + /// the AS path `path`. + pub fn of(routes: impl IntoIterator, path: &[u32]) -> Self { + Self { + routes: routes.into_iter().collect(), + path: path.to_vec(), + next_hop: Ipv4Addr::new(192, 0, 2, 1), + only_to_customer: None, + communities: Vec::new(), + more: Vec::new(), + } + } + + pub fn with_next_hop(mut self, next_hop: Ipv4Addr) -> Self { + self.next_hop = next_hop; + self + } + + /// With a COMMUNITIES attribute carrying `communities`, if there are + /// any. + pub fn with_communities(mut self, communities: &[(u16, u16)]) -> Self { + self.communities = communities.to_vec(); + self + } + + /// Marked as only to go to customers, by `asn` (RFC 9234). + pub fn with_only_to_customer(mut self, asn: u32) -> Self { + self.only_to_customer = Some(asn); + self + } + + /// With `attributes` after the mandatory ones, as given. + pub fn with_attributes(mut self, attributes: impl IntoIterator) -> Self { + self.more.extend(attributes); + self + } +} + +impl From for UpdateMessage { + fn from(announcement: Announcement) -> Self { + let mut path_attributes = vec![ + PathAttribute::Origin(OriginPathAttribute::IGP), + AsPathAttribute::from_asns(announcement.path), + PathAttribute::NextHop(NextHopPathAttribute(announcement.next_hop)), + ]; + if !announcement.communities.is_empty() { + path_attributes.push(PathAttribute::Communities(CommunitiesPathAttribute( + announcement.communities, + ))); + } + path_attributes.extend( + announcement + .only_to_customer + .map(PathAttribute::OnlyToCustomer), + ); + path_attributes.extend(announcement.more); + UpdateMessage { + withdrawn_routes: vec![], + path_attributes, + announced_routes: announcement.routes, + } + } +} + +impl From for Message { + fn from(announcement: Announcement) -> Self { + Message::Update(announcement.into()) + } +} diff --git a/crates/integration-tests/src/lib.rs b/crates/integration-tests/src/lib.rs index cc5ae02..902aeab 100644 --- a/crates/integration-tests/src/lib.rs +++ b/crates/integration-tests/src/lib.rs @@ -1,7 +1,22 @@ +pub mod announcement; pub mod test_cache; pub mod test_peer; pub mod test_server; +use std::time::Duration; + +use bgp_primitives::ip_prefix::IpPrefix; +use bgp_server::address::Endpoint; +use bgp_server::config::PeerConfig; +use bgp_server::family::FamilySet; +use eyre::{Result, bail}; +use serde_json::Value; + +/// How long a test waits for something the server does on its own: a +/// route reaching the table, a session coming up, a message arriving. +/// Everything happens on the loopback, so it is generous. +pub const TIMEOUT: Duration = Duration::from_secs(5); + /// The policy a peer has with nothing configured, but for one thing: the /// peers of these tests are all at 127.0.0.1, which no route can have as /// its next hop, so their routes name another. @@ -11,3 +26,74 @@ pub fn default_policy() -> bgp_server::policy::ImportPolicy { ..Default::default() } } + +/// A prefix written as text. The tests' prefixes are literals, so a bad +/// one is a mistake in the test. +pub fn prefix(s: &str) -> IpPrefix { + IpPrefix::try_from(s).unwrap() +} + +/// A configured peer with nothing said about it but where it is and what +/// AS it is in: the default hold time, every family, the default policy of +/// these tests, and no role, limits, groups or ADD-PATH. A test says what +/// else with struct update syntax: `PeerConfig { role: Some(..), +/// ..peer_config(..) }`. +pub fn peer_config(name: &str, endpoint: impl Into, remote_asn: u32) -> PeerConfig { + PeerConfig { + name: name.into(), + endpoint: endpoint.into(), + remote_asn, + hold_time_secs: 90, + local_address: None, + families: FamilySet::supported(), + add_path: None, + import: default_policy(), + role: None, + require_role: false, + export: Default::default(), + export_limits: None, + graceful_restart: Default::default(), + groups: Vec::new(), + reflector_client: false, + passive: false, + } +} + +/// Waits until `done` holds, looking every few milliseconds, or fails +/// with `what` once [`TIMEOUT`] has passed. +pub async fn wait_until(what: &str, mut done: impl FnMut() -> bool) -> Result<()> { + let waited = tokio::time::timeout(TIMEOUT, async { + while !done() { + tokio::time::sleep(Duration::from_millis(5)).await; + } + }) + .await; + if waited.is_err() { + bail!("timed out waiting until {what}"); + } + Ok(()) +} + +/// Waits until `get` has something, and returns it; fails with `what` +/// once [`TIMEOUT`] has passed. +pub async fn wait_for(what: &str, mut get: impl FnMut() -> Option) -> Result { + let mut got = None; + wait_until(what, || { + got = get(); + got.is_some() + }) + .await?; + Ok(got.expect("the wait ended with something")) +} + +/// `GET`s `url` and reads the JSON it answers with. An error status is an +/// error. +pub async fn get_json(client: &reqwest::Client, url: &str) -> Result { + Ok(client + .get(url) + .send() + .await? + .error_for_status()? + .json() + .await?) +} diff --git a/crates/integration-tests/src/test_peer.rs b/crates/integration-tests/src/test_peer.rs index 35e8ee8..685ec1d 100644 --- a/crates/integration-tests/src/test_peer.rs +++ b/crates/integration-tests/src/test_peer.rs @@ -1,13 +1,16 @@ use std::net::{Ipv4Addr, SocketAddr}; +use std::time::Duration; -use bgp_packet::message::{BGP_HEADER_LEN, Message, UpdateMessage}; +use bgp_packet::message::{BGP_HEADER_LEN, Message, NotificationMessage, UpdateMessage}; use bgp_packet::open::{Capability, OpenMessage, OpenOption}; use bgp_primitives::constants::{Afi, AfiSafi, Safi}; use bgp_primitives::parser::ParserContext; use bytes::BytesMut; use eyre::{Result, bail, eyre}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; -use tokio::net::TcpStream; +use tokio::net::{TcpListener, TcpStream}; + +use crate::TIMEOUT; /// A minimal BGP peer for use in tests. Connects over TCP and can exchange /// framed BGP messages using the `bgp-packet` wire format. @@ -30,6 +33,13 @@ impl TestPeer { Ok(Self::from_stream(stream)) } + /// The next connection to `listener`, as the server dials it. It is + /// waited for at most [`TIMEOUT`]. + pub async fn accept(listener: &TcpListener) -> Result { + let (stream, _) = tokio::time::timeout(TIMEOUT, listener.accept()).await??; + Ok(Self::from_stream(stream)) + } + /// Wraps an already-connected stream, e.g. one accepted from a server /// that dialled us. pub fn from_stream(stream: TcpStream) -> Self { @@ -41,6 +51,11 @@ impl TestPeer { } } + /// Our end of the connection: what the server sees the peer as. + pub fn local_addr(&self) -> Result { + Ok(self.stream.local_addr()?) + } + /// Send a BGP message. pub async fn send(&mut self, msg: &Message) -> Result<()> { let send_ctx = ParserContext { @@ -53,8 +68,32 @@ impl TestPeer { Ok(()) } - /// Read exactly one BGP message. + /// Send an UPDATE: an [`Announcement`](crate::announcement::Announcement), + /// a [`withdrawal`](crate::announcement::withdrawal), or one built by + /// hand. + pub async fn announce(&mut self, update: impl Into) -> Result<()> { + self.send(&Message::Update(update.into())).await + } + + /// Read exactly one BGP message, waiting at most [`TIMEOUT`] for it. A + /// test that expects nothing to arrive says for how long with + /// [`TestPeer::recv_within`]. pub async fn recv(&mut self) -> Result { + self.recv_within(TIMEOUT).await + } + + /// Read exactly one BGP message, waiting at most `wait` for it. + pub async fn recv_within(&mut self, wait: Duration) -> Result { + match tokio::time::timeout(wait, self.recv_untimed()).await { + Ok(message) => message, + Err(_) => bail!("no message from the server within {wait:?}"), + } + } + + /// Read exactly one BGP message, however long it takes to come. For a + /// peer that is to stay on the line, like the `fake_peer` example; + /// tests want [`TestPeer::recv`]. + pub async fn recv_untimed(&mut self) -> Result { let mut header = [0u8; BGP_HEADER_LEN as usize]; self.stream.read_exact(&mut header).await?; @@ -84,6 +123,18 @@ impl TestPeer { } } + /// Read messages until a NOTIFICATION arrives, skipping KEEPALIVEs. Any + /// other message type is an error. + pub async fn expect_notification(&mut self) -> Result { + loop { + match self.recv().await? { + Message::Notification(n) => return Ok(n), + Message::KeepAlive => {} + other => bail!("expected NOTIFICATION, got {other:?}"), + } + } + } + /// Receive and unwrap as `OpenMessage`, failing if a different type arrives. pub async fn recv_open(&mut self) -> Result { match self.recv().await? { @@ -207,12 +258,23 @@ impl TestPeer { Ok(()) } - /// Returns true if the remote end has closed the connection. - pub async fn is_closed(&mut self) -> bool { - let mut buf = [0u8; 1]; - match self.stream.read(&mut buf).await { - Ok(0) | Err(_) => true, - Ok(_) => false, - } + /// Whether the server closes the connection within `wait`. Whatever it + /// still sends first, KEEPALIVEs as a rule, is read past: the end of + /// the stream, or anything unreadable, is what counts. + pub async fn closed_within(&mut self, wait: Duration) -> bool { + tokio::time::timeout(wait, async { while self.recv_untimed().await.is_ok() {} }) + .await + .is_ok() + } + + /// Whether the server closes the connection within [`TIMEOUT`] without + /// having sent anything: what a peer that is not let in sees. A server + /// that sends its OPEN first, or that leaves the connection open, is + /// answering. + pub async fn closed_unanswered(&mut self) -> bool { + matches!( + tokio::time::timeout(TIMEOUT, self.recv_untimed()).await, + Ok(Err(_)) + ) } } diff --git a/crates/integration-tests/src/test_server.rs b/crates/integration-tests/src/test_server.rs index 8fc834f..5ae0799 100644 --- a/crates/integration-tests/src/test_server.rs +++ b/crates/integration-tests/src/test_server.rs @@ -1,23 +1,28 @@ +use std::collections::BTreeSet; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::sync::Arc; use bgp_fib::Fib; use bgp_packet::message::{NlriNextHop, PathAttribute}; +use bgp_packet::open::Capability; use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::LocalRib; use bgp_rib::table::{Event, PeerIndex, Reader, RibTable}; use bgp_rpki::Rpki; -use bgp_server::config::Config; +use bgp_server::config::{Config, PeerConfig}; use bgp_server::error::ServerError; use bgp_server::refusals::RefusalLog; use bgp_server::restart::Restarts; use bgp_server::server::BgpServer; -use bgp_server::session::SessionRegistry; +use bgp_server::session::{SessionHandle, SessionRegistry}; use bgp_web::{AppState, RouterInfo}; -use eyre::Result; +use eyre::{Result, eyre}; use tokio::net::TcpListener; use tokio::task::JoinHandle; +use crate::test_peer::TestPeer; +use crate::{peer_config, wait_for, wait_until}; + /// A BGP server started on an ephemeral port for testing. /// Aborts the server task when dropped. pub struct TestServer { @@ -106,6 +111,129 @@ impl TestServer { local_asn, }) } + + /// A server in `local_asn` that takes connections from one configured + /// peer only: "r1", in `remote_asn`, at the loopback address the test + /// connects from. `customize` edits the configuration and the peer's + /// before the server starts. The server dials the peer too; where that + /// goes is returned with it, for the test to answer or to leave alone. + pub async fn with_peer( + local_asn: u32, + remote_asn: u32, + customize: impl FnOnce(&mut Config, &mut PeerConfig), + ) -> Result { + let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; + let endpoint = listener.local_addr()?; + let server = Self::start_with(local_asn, |config| { + config.accept_unconfigured_peers = false; + let mut peer = peer_config("r1", endpoint, remote_asn); + customize(config, &mut peer); + config.peers = vec![peer]; + }) + .await?; + Ok(ServerWithPeer { server, listener }) + } +} + +/// A server with one configured peer, and the listener at the peer's +/// address, which the server's dialling goes to. +pub struct ServerWithPeer { + pub server: TestServer, + pub listener: TcpListener, +} + +impl TestServer { + /// A session with the server from a peer in `asn` with `router_id`, + /// offering the capabilities of a typical dual-stack speaker. Returns + /// once the server lists the session as established. Distinct peers + /// need distinct identifiers, or the second connection is (correctly) + /// treated as a collision with the first. + pub async fn established(&self, asn: u32, router_id: Ipv4Addr) -> Result { + let mut peer = TestPeer::connect(self.addr).await?; + peer.handshake(asn, router_id, 90).await?; + self.listed(&peer).await?; + Ok(peer) + } + + /// As [`TestServer::established`], with an OPEN carrying exactly + /// `capabilities`. + pub async fn established_with( + &self, + asn: u32, + router_id: Ipv4Addr, + capabilities: Vec, + ) -> Result { + let mut peer = TestPeer::connect(self.addr).await?; + peer.handshake_with(asn, router_id, 90, capabilities) + .await?; + self.listed(&peer).await?; + Ok(peer) + } + + /// Waits until the registry lists `peer`'s session. + async fn listed(&self, peer: &TestPeer) -> Result<()> { + let from = peer.local_addr()?; + wait_until("the session is established", || { + self.registry + .list() + .iter() + .any(|session| session.peer.addr == from) + }) + .await + } + + /// Waits until the registry lists `n` established sessions. + pub async fn wait_for_sessions(&self, n: usize) -> Result<()> { + wait_until(&format!("{n} sessions are established"), || { + self.registry.list().len() == n + }) + .await + } + + /// Waits until a session is established, and returns it. + pub async fn wait_for_session(&self) -> Result { + wait_for("a session is established", || { + self.registry.list().into_iter().next() + }) + .await + } + + /// Waits until the table holds `routes` routes. + pub async fn wait_for_routes(&self, routes: usize) -> Result<()> { + wait_until(&format!("the table holds {routes} routes"), || { + self.rib.len() == routes + }) + .await + } + + /// Waits until the table holds exactly the prefixes `expected` and the + /// peer has `held` routes held aside. + pub async fn table_becomes(&self, expected: &[&str], held: u64) -> Result<()> { + let expected: BTreeSet = expected.iter().map(|p| p.to_string()).collect(); + let table = || -> BTreeSet { + self.received_routes() + .iter() + .map(|route| route.prefix.to_string()) + .collect() + }; + let held_now = || { + self.registry + .list() + .first() + .map_or(0, |session| session.counters.held()) + }; + wait_until("the table is as expected", || { + table() == expected && held_now() == held + }) + .await + .map_err(|_| { + eyre!( + "the table has {:?} with {} held; expected {expected:?} with {held} held", + table(), + held_now() + ) + }) + } } /// A route in the server's table, as a test wants to look at it. diff --git a/crates/integration-tests/tests/add_path.rs b/crates/integration-tests/tests/add_path.rs index 979735a..37bff79 100644 --- a/crates/integration-tests/tests/add_path.rs +++ b/crates/integration-tests/tests/add_path.rs @@ -4,29 +4,23 @@ use std::net::{Ipv4Addr, SocketAddr}; use std::time::Duration; +use bgp_integration_tests::announcement::{Announcement, withdrawal}; use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::TestServer; -use bgp_packet::message::{ - AsPathAttribute, Message, NextHopPathAttribute, OriginPathAttribute, PathAttribute, - UpdateMessage, -}; +use bgp_integration_tests::{peer_config, prefix, wait_until}; +use bgp_packet::message::PathAttribute; use bgp_packet::open::{AddPathEntry, AddPathSendRecv, Capability, OpenMessage}; use bgp_primitives::constants::AfiSafi; -use bgp_primitives::ip_prefix::{IpPrefix, Nlri}; +use bgp_primitives::ip_prefix::Nlri; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::config::PeerConfig; -use bgp_server::family::{FamilySet, SUPPORTED}; +use bgp_server::family::SUPPORTED; use bgp_server::policy::ImportPolicy; use eyre::Result; use serde_json::Value; const LOCAL_ASN: u32 = 65000; const PEER_ASN: u32 = 65001; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} /// A server on which connections from this machine are those of a /// configured peer, with ADD-PATH offered to it as `add_path` says. @@ -39,23 +33,11 @@ async fn server_offering( .map(|p| LocalRoute::new(prefix(p), LocalRouteAttrs::default())) .collect(); TestServer::start_with(LOCAL_ASN, |c| { + let at = SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 1); c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "route-server-client".into(), - endpoint: SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 1).into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), add_path, import: ImportPolicy::permissive(), - role: None, - require_role: false, - export: Default::default(), + ..peer_config("route-server-client", at, PEER_ASN) }]; c.originate = originate; // Keep the dialer out of the way. @@ -85,12 +67,7 @@ async fn peer_offering( let open = peer .handshake_with(PEER_ASN, Ipv4Addr::new(10, 0, 0, 1), 90, capabilities) .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; + server.wait_for_session().await?; Ok((peer, open)) } @@ -105,26 +82,8 @@ fn add_path_offered(open: &OpenMessage) -> Vec<(AfiSafi, AddPathSendRecv)> { .collect() } -fn announcement(nlri: Nlri, as_path: &[u32]) -> Message { - Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(as_path.to_vec()), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ], - announced_routes: vec![nlri], - }) -} - -async fn wait_for_routes(server: &TestServer, routes: usize) -> Result<()> { - tokio::time::timeout(TIMEOUT, async { - while server.rib.len() != routes { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; - Ok(()) +fn announcement(nlri: Nlri, as_path: &[u32]) -> Announcement { + Announcement::of([nlri], as_path) } /// The routes the server holds, as (path identifier, AS path, best). @@ -172,11 +131,11 @@ async fn test_several_routes_for_a_prefix_from_one_peer() -> Result<()> { let p = prefix("203.0.113.0/24"); let path = |path_id| Nlri { prefix: p, path_id }; - peer.send(&announcement(path(1), &[PEER_ASN, 64999, 64998])) + peer.announce(announcement(path(1), &[PEER_ASN, 64999, 64998])) .await?; - peer.send(&announcement(path(2), &[PEER_ASN, 64998])) + peer.announce(announcement(path(2), &[PEER_ASN, 64998])) .await?; - wait_for_routes(&server, 2).await?; + server.wait_for_routes(2).await?; assert_eq!( held(&server), [ @@ -209,34 +168,24 @@ async fn test_several_routes_for_a_prefix_from_one_peer() -> Result<()> { ); // Announcing a path again replaces that path only. - peer.send(&announcement(path(1), &[PEER_ASN])).await?; - tokio::time::timeout(TIMEOUT, async { - while held(&server)[0].1.len() != 1 { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; + peer.announce(announcement(path(1), &[PEER_ASN])).await?; + wait_until("the path is replaced", || held(&server)[0].1.len() == 1).await?; assert_eq!( held(&server), [(1, vec![PEER_ASN], true), (2, vec![PEER_ASN, 64998], false),] ); // And withdrawing one leaves the other. - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![path(1)], - path_attributes: vec![], - announced_routes: vec![], - })) - .await?; - wait_for_routes(&server, 1).await?; + peer.announce(withdrawal([path(1)])).await?; + server.wait_for_routes(1).await?; assert_eq!(held(&server), [(2, vec![PEER_ASN, 64998], true)]); // When the session ends, all of its routes go. - peer.send(&announcement(path(3), &[PEER_ASN, 1, 2, 3])) + peer.announce(announcement(path(3), &[PEER_ASN, 1, 2, 3])) .await?; - wait_for_routes(&server, 2).await?; + server.wait_for_routes(2).await?; drop(peer); - wait_for_routes(&server, 0).await?; + server.wait_for_routes(0).await?; Ok(()) } @@ -252,18 +201,18 @@ async fn test_add_path_is_not_used_unless_both_sides_offer_it() -> Result<()> { assert!(session.add_path_receive.is_empty() && session.add_path_send.is_empty()); // Plain prefixes both ways. - let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + let update = peer.recv_update().await?; assert_eq!(update.announced_routes, vec![prefix("10.100.0.0/16")]); - peer.send(&announcement(prefix("203.0.113.0/24").into(), &[PEER_ASN])) + peer.announce(announcement(prefix("203.0.113.0/24").into(), &[PEER_ASN])) .await?; - wait_for_routes(&server, 1).await?; + server.wait_for_routes(1).await?; assert_eq!(held(&server), [(0, vec![PEER_ASN], true)]); // And the other way round: offered by the server, not by the peer. let server = server_offering(Some(AddPathSendRecv::Both), &["10.100.0.0/16"]).await?; let (mut peer, open) = peer_offering(&server, None).await?; assert_eq!(add_path_offered(&open).len(), SUPPORTED.len()); - let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + let update = peer.recv_update().await?; assert_eq!(update.announced_routes, vec![prefix("10.100.0.0/16")]); Ok(()) } @@ -284,15 +233,15 @@ async fn test_prefixes_are_sent_with_path_identifiers_when_agreed() -> Result<() // Read as agreed: with identifiers. peer.ctx.add_path = vec![AfiSafi::IPV4_UNICAST]; - let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + let update = peer.recv_update().await?; assert_eq!(update.announced_routes.len(), 1); assert_eq!(update.announced_routes[0].prefix, prefix("10.100.0.0/16")); // The peer did not offer to send, so the server reads its prefixes // plain. - peer.send(&announcement(prefix("203.0.113.0/24").into(), &[PEER_ASN])) + peer.announce(announcement(prefix("203.0.113.0/24").into(), &[PEER_ASN])) .await?; - wait_for_routes(&server, 1).await?; + server.wait_for_routes(1).await?; assert_eq!(held(&server), [(0, vec![PEER_ASN], true)]); Ok(()) } diff --git a/crates/integration-tests/tests/admission.rs b/crates/integration-tests/tests/admission.rs index bfb1a0d..41c9003 100644 --- a/crates/integration-tests/tests/admission.rs +++ b/crates/integration-tests/tests/admission.rs @@ -5,42 +5,14 @@ use std::net::{Ipv4Addr, SocketAddr}; use std::time::Duration; use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::test_server::{ServerWithPeer, TestServer}; +use bgp_integration_tests::{TIMEOUT, peer_config}; use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; use bgp_server::policy::ImportPolicy; use eyre::Result; -use tokio::net::TcpListener; const LOCAL_ASN: u32 = 65000; const PEER_ASN: u32 = 65001; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn peer_at(endpoint: SocketAddr) -> PeerConfig { - PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import: ImportPolicy::permissive(), - role: None, - require_role: false, - export: Default::default(), - export_limits: None, - passive: false, - } -} - -/// Whether the server closed the connection without sending anything. -async fn closed_unanswered(peer: &mut TestPeer) -> bool { - matches!(tokio::time::timeout(TIMEOUT, peer.recv()).await, Ok(Err(_))) -} /// A host that is not a configured peer gets nothing: no OPEN to answer, /// no session, no routes taken from it. Any host used to be able to open a @@ -50,12 +22,16 @@ async fn test_a_host_that_is_not_a_configured_peer_is_refused() -> Result<()> { // The only peer configured is somewhere else (documentation space). let server = TestServer::start_with(LOCAL_ASN, |c| { c.accept_unconfigured_peers = false; - c.peers = vec![peer_at("192.0.2.9:179".parse().unwrap())]; + let elsewhere: SocketAddr = "192.0.2.9:179".parse().unwrap(); + c.peers = vec![PeerConfig { + import: ImportPolicy::permissive(), + ..peer_config("r1", elsewhere, PEER_ASN) + }]; }) .await?; let mut stranger = TestPeer::connect(server.addr).await?; - assert!(closed_unanswered(&mut stranger).await); + assert!(stranger.closed_unanswered().await); assert!(server.registry.list().is_empty()); Ok(()) } @@ -64,24 +40,18 @@ async fn test_a_host_that_is_not_a_configured_peer_is_refused() -> Result<()> { /// configured for that peer. #[tokio::test] async fn test_a_configured_peer_is_let_in() -> Result<()> { - // Somewhere for the server's own dialling to go; never answered. - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let endpoint = listener.local_addr()?; - let server = TestServer::start_with(LOCAL_ASN, |c| { - c.accept_unconfigured_peers = false; - c.peers = vec![peer_at(endpoint)]; + // The server's own dialling goes to the listener; it is never answered. + let ServerWithPeer { + server, + listener: _listener, + } = TestServer::with_peer(LOCAL_ASN, PEER_ASN, |_, peer| { + peer.import = ImportPolicy::permissive(); }) .await?; - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) + let _peer = server + .established(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2)) .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; assert_eq!(server.registry.list()[0].name.as_deref(), Some("r1")); Ok(()) } @@ -97,11 +67,11 @@ async fn test_one_address_may_open_only_a_few_connections() -> Result<()> { let mut held = Vec::new(); for _ in 0..4 { let mut peer = TestPeer::connect(server.addr).await?; - tokio::time::timeout(TIMEOUT, peer.recv_open()).await??; + peer.recv_open().await?; held.push(peer); } let mut one_too_many = TestPeer::connect(server.addr).await?; - assert!(closed_unanswered(&mut one_too_many).await); + assert!(one_too_many.closed_unanswered().await); // When one of them goes, there is room again. held.pop(); diff --git a/crates/integration-tests/tests/announce.rs b/crates/integration-tests/tests/announce.rs index 1f6163f..12d19e2 100644 --- a/crates/integration-tests/tests/announce.rs +++ b/crates/integration-tests/tests/announce.rs @@ -4,25 +4,21 @@ use std::net::Ipv4Addr; use std::time::Duration; +use bgp_integration_tests::announcement::announcement; use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::{RouteChange, TestServer}; +use bgp_integration_tests::{TIMEOUT, prefix}; use bgp_packet::message::{ AsPathAttribute, CommunitiesPathAttribute, LargeCommunitiesPathAttribute, LargeCommunity, LocalPrefPathAttribute, NextHopPathAttribute, OriginPathAttribute, PathAttribute, UpdateMessage, }; -use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::session::SessionEvent; use eyre::Result; const LOCAL_ASN: u32 = 65000; const PEER_ASN: u32 = 65001; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} fn route(p: &str, attrs: LocalRouteAttrs) -> LocalRoute { LocalRoute::new(prefix(p), attrs) @@ -32,27 +28,12 @@ async fn established_peer(server: &TestServer) -> Result { established_peer_with_id(server, Ipv4Addr::new(10, 0, 0, 1)).await } -/// Distinct routers need distinct BGP Identifiers, or the second connection -/// is (correctly) treated as a collision with the first. async fn established_peer_with_id(server: &TestServer, router_id: Ipv4Addr) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(PEER_ASN, router_id, 90).await?; - Ok(peer) + server.established(PEER_ASN, router_id).await } async fn next_update(peer: &mut TestPeer) -> Result { - tokio::time::timeout(TIMEOUT, peer.recv_update()).await? -} - -/// Waits until the registry reports `n` established sessions. -async fn wait_for_sessions(server: &TestServer, n: usize) -> Result<()> { - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().len() != n { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; - Ok(()) + peer.recv_update().await } // --------------------------------------------------------------------------- @@ -97,7 +78,7 @@ async fn test_registry_tracks_session_lifecycle() -> Result<()> { async fn test_announce_after_session_up() -> Result<()> { let server = TestServer::start(LOCAL_ASN).await?; let mut peer = established_peer(&server).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; server .local_rib @@ -174,7 +155,7 @@ async fn test_announce_reaches_all_sessions() -> Result<()> { let server = TestServer::start(LOCAL_ASN).await?; let mut a = established_peer_with_id(&server, Ipv4Addr::new(10, 0, 0, 1)).await?; let mut b = established_peer_with_id(&server, Ipv4Addr::new(10, 0, 0, 2)).await?; - wait_for_sessions(&server, 2).await?; + server.wait_for_sessions(2).await?; server .local_rib @@ -192,7 +173,7 @@ async fn test_announce_reaches_all_sessions() -> Result<()> { async fn test_withdraw_sends_withdrawn_route() -> Result<()> { let server = TestServer::start(LOCAL_ASN).await?; let mut peer = established_peer(&server).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; server .local_rib @@ -213,7 +194,7 @@ async fn test_withdraw_sends_withdrawn_route() -> Result<()> { async fn test_reannounce_with_prepend_and_med() -> Result<()> { let server = TestServer::start(LOCAL_ASN).await?; let mut peer = established_peer(&server).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; server .local_rib @@ -258,7 +239,7 @@ async fn test_keepalives_continue_alongside_updates() -> Result<()> { let mut peer = TestPeer::connect(server.addr).await?; peer.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 1), 3) .await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; server .local_rib @@ -289,22 +270,12 @@ async fn test_received_routes_removed_when_session_closes() -> Result<()> { let mut rib_events = server.route_changes(); let mut session_events = server.registry.subscribe(); let mut peer = established_peer(&server).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; - peer.send(&bgp_packet::message::Message::Update( - bgp_packet::message::UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(10, 0, 0, 1))), - ], - announced_routes: vec![ - prefix("203.0.113.0/24").into(), - prefix("198.51.100.0/24").into(), - ], - }, - )) + peer.announce( + announcement(&["203.0.113.0/24", "198.51.100.0/24"], &[PEER_ASN]) + .with_next_hop(Ipv4Addr::new(10, 0, 0, 1)), + ) .await?; for _ in 0..2 { let ev = tokio::time::timeout(TIMEOUT, rib_events.recv()).await??; @@ -351,7 +322,7 @@ const ROUTE_REFRESH_IPV4: &[u8] = &[ async fn test_route_refresh_resends_local_routes() -> Result<()> { let server = TestServer::start(LOCAL_ASN).await?; let mut peer = established_peer(&server).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; server .local_rib @@ -386,12 +357,11 @@ async fn test_route_refresh_resends_local_routes() -> Result<()> { #[tokio::test] async fn test_shutdown_sends_cease_to_every_peer() -> Result<()> { use bgp_packet::errors::{BgpError, CeaseSubcode}; - use bgp_packet::message::Message; let server = TestServer::start(LOCAL_ASN).await?; let mut a = established_peer_with_id(&server, Ipv4Addr::new(10, 0, 0, 1)).await?; let mut b = established_peer_with_id(&server, Ipv4Addr::new(10, 0, 0, 2)).await?; - wait_for_sessions(&server, 2).await?; + server.wait_for_sessions(2).await?; let left = server .registry @@ -401,21 +371,12 @@ async fn test_shutdown_sends_cease_to_every_peer() -> Result<()> { assert!(server.registry.is_shutting_down()); for peer in [&mut a, &mut b] { - let notif = tokio::time::timeout(TIMEOUT, async { - loop { - match peer.recv().await? { - Message::Notification(n) => return Ok::<_, eyre::Error>(n), - Message::KeepAlive => {} - other => eyre::bail!("unexpected {other:?}"), - } - } - }) - .await??; + let notif = peer.expect_notification().await?; assert!( matches!(notif.error, BgpError::Cease(CeaseSubcode::AdminShutdown)), "{notif:?}" ); - assert!(peer.is_closed().await); + assert!(peer.closed_within(TIMEOUT).await); } Ok(()) } @@ -470,7 +431,7 @@ async fn test_a_peer_in_our_own_as_gets_an_empty_path_and_a_local_pref() -> Resu // The peer reads as a router in the server's AS does: one outside it // would drop the LOCAL_PREF. peer.ctx.ibgp = true; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; assert!(server.registry.list()[0].ibgp); server diff --git a/crates/integration-tests/tests/aspa.rs b/crates/integration-tests/tests/aspa.rs index e7e5070..79f4926 100644 --- a/crates/integration-tests/tests/aspa.rs +++ b/crates/integration-tests/tests/aspa.rs @@ -6,37 +6,20 @@ use std::collections::BTreeSet; use std::net::Ipv4Addr; use std::time::Duration; -use bgp_integration_tests::default_policy; +use bgp_integration_tests::announcement::announcement; use bgp_integration_tests::test_cache::TestCache; use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; -use bgp_packet::message::{ - AsPathAttribute, Message, NextHopPathAttribute, OriginPathAttribute, PathAttribute, - UpdateMessage, -}; -use bgp_primitives::ip_prefix::IpPrefix; +use bgp_integration_tests::test_server::{ServerWithPeer, TestServer}; +use bgp_integration_tests::{default_policy, prefix, wait_until}; +use bgp_packet::message::PathAttribute; use bgp_rpki::{CacheState, RpkiConfig}; -use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; use bgp_server::policy::{ImportPolicy, PeerRole}; use eyre::Result; use serde_json::Value; -use tokio::net::TcpListener; const LOCAL_ASN: u32 = 1299; const PEER_ASN: u32 = 3333; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} - -/// What a test holds on to: the server, and the listener its dialling -/// goes to. -struct Setup { - server: TestServer, - _listener: TcpListener, -} +const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 2); /// A server that validates against `cache`, with one configured peer at /// the address the test connects from, which is `role` to it. Returns once @@ -45,108 +28,33 @@ async fn server_with( cache: &TestCache, role: Option, import: ImportPolicy, -) -> Result { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let endpoint = listener.local_addr()?; +) -> Result { let cache = cache.address(); - let server = TestServer::start_with(LOCAL_ASN, |c| { - c.accept_unconfigured_peers = false; + let setup = TestServer::with_peer(LOCAL_ASN, PEER_ASN, |c, peer| { c.rpki = Some(RpkiConfig { caches: vec![cache], retry: Some(Duration::from_millis(100)), ..RpkiConfig::default() }); - c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import, - role, - require_role: false, - export: Default::default(), - }]; + peer.import = import; + peer.role = role; }) .await?; - let rpki = server.rpki.clone().expect("validation is configured"); - tokio::time::timeout(TIMEOUT, async { - while rpki.status().state != CacheState::Synchronised { - tokio::time::sleep(Duration::from_millis(5)).await; - } + let rpki = setup.server.rpki.clone().expect("validation is configured"); + wait_until("the server has the cache's data", || { + rpki.status().state == CacheState::Synchronised }) .await?; - Ok(Setup { - server, - _listener: listener, - }) + Ok(setup) } async fn established(server: &TestServer) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; - Ok(peer) + server.established(PEER_ASN, PEER_ID).await } /// Announces `prefixes` with the AS path `path`. async fn announce(peer: &mut TestPeer, prefixes: &[&str], path: &[u32]) -> Result<()> { - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(path.to_vec()), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ], - announced_routes: prefixes.iter().map(|p| prefix(p).into()).collect(), - })) - .await -} - -/// Waits until the table holds exactly `expected` and the peer has `held` -/// routes held aside. -async fn table_becomes(server: &TestServer, expected: &[&str], held: u64) -> Result<()> { - let expected: BTreeSet = expected.iter().map(|p| p.to_string()).collect(); - let table = || -> BTreeSet { - server - .received_routes() - .iter() - .map(|route| route.prefix.to_string()) - .collect() - }; - let held_now = || { - server - .registry - .list() - .first() - .map_or(0, |session| session.counters.held()) - }; - tokio::time::timeout(TIMEOUT, async { - while table() != expected || held_now() != held { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await - .map_err(|_| { - eyre::eyre!( - "the table has {:?} with {} held; expected {expected:?} with {held} held", - table(), - held_now() - ) - }) + peer.announce(announcement(prefixes, path)).await } /// AS 15169 buys from 3356 and says so; 3356 and the peer, 3333, have no @@ -161,7 +69,8 @@ fn aspas() -> Vec<(u32, Vec)> { #[tokio::test] async fn test_a_leaked_route_is_refused_and_the_rest_are_taken() -> Result<()> { let cache = TestCache::start_with(vec![], aspas()).await; - let Setup { server, .. } = server_with(&cache, Some(PeerRole::Peer), default_policy()).await?; + let ServerWithPeer { server, .. } = + server_with(&cache, Some(PeerRole::Peer), default_policy()).await?; let (base, _http) = server.serve_http().await?; let mut peer = established(&server).await?; @@ -171,7 +80,9 @@ async fn test_a_leaked_route_is_refused_and_the_rest_are_taken() -> Result<()> { announce(&mut peer, &["193.0.0.0/21"], &[PEER_ASN]).await?; announce(&mut peer, &["194.0.0.0/24"], &[PEER_ASN, 2121]).await?; announce(&mut peer, &["8.8.8.0/24"], &[PEER_ASN, 3356, 15169]).await?; - table_becomes(&server, &["193.0.0.0/21", "194.0.0.0/24"], 1).await?; + server + .table_becomes(&["193.0.0.0/21", "194.0.0.0/24"], 1) + .await?; let client = reqwest::Client::new(); let get = |path: &str| { @@ -217,13 +128,13 @@ async fn test_a_route_that_has_crossed_a_peering_is_taken_from_a_provider_only() (None, true), ] { let cache = TestCache::start_with(vec![], aspas()).await; - let Setup { server, .. } = server_with(&cache, role, default_policy()).await?; + let ServerWithPeer { server, .. } = server_with(&cache, role, default_policy()).await?; let mut peer = established(&server).await?; announce(&mut peer, &["8.8.8.0/24"], &[PEER_ASN, 3356, 15169]).await?; let expected = if taken { - table_becomes(&server, &["8.8.8.0/24"], 0).await + server.table_becomes(&["8.8.8.0/24"], 0).await } else { - table_becomes(&server, &[], 1).await + server.table_becomes(&[], 1).await }; expected.map_err(|e| eyre::eyre!("from a peer that is {role:?}: {e}"))?; } @@ -240,14 +151,15 @@ async fn test_paths_from_a_route_server_do_not_begin_with_its_as() -> Result<()> accept_any_first_as: true, ..default_policy() }; - let Setup { server, .. } = server_with(&cache, Some(PeerRole::RouteServer), import).await?; + let ServerWithPeer { server, .. } = + server_with(&cache, Some(PeerRole::RouteServer), import).await?; let mut peer = established(&server).await?; // 3356's customer route, as 3356 gave it to the route server. announce(&mut peer, &["8.8.8.0/24"], &[3356, 15169]).await?; // The same route leaked to the route server by another member. announce(&mut peer, &["8.8.4.0/24"], &[2121, 3356, 15169]).await?; - table_becomes(&server, &["8.8.8.0/24"], 1).await + server.table_becomes(&["8.8.8.0/24"], 1).await } /// Validity changes with the ASPAs, and the routes follow: one that was @@ -256,19 +168,20 @@ async fn test_paths_from_a_route_server_do_not_begin_with_its_as() -> Result<()> #[tokio::test] async fn test_routes_follow_the_aspas() -> Result<()> { let cache = TestCache::start_with(vec![], aspas()).await; - let Setup { server, .. } = server_with(&cache, Some(PeerRole::Peer), default_policy()).await?; + let ServerWithPeer { server, .. } = + server_with(&cache, Some(PeerRole::Peer), default_policy()).await?; let mut peer = established(&server).await?; announce(&mut peer, &["194.0.0.0/24"], &[PEER_ASN, 2121]).await?; announce(&mut peer, &["8.8.8.0/24"], &[PEER_ASN, 3356, 15169]).await?; - table_becomes(&server, &["194.0.0.0/24"], 1).await?; + server.table_becomes(&["194.0.0.0/24"], 1).await?; // 2121 says that its only provider is 1101: its route by way of 3333 // is now invalid, and goes. let mut changed = aspas(); changed.push((2121, vec![1101])); cache.set_aspas(changed); - table_becomes(&server, &[], 2).await?; + server.table_becomes(&[], 2).await?; let refused = &server.refusals.peers()[0].1; assert_eq!(refused.total(), 2); assert_eq!(refused.recent()[0].prefix, prefix("194.0.0.0/24")); @@ -282,7 +195,9 @@ async fn test_routes_follow_the_aspas() -> Result<()> { (PEER_ASN, vec![0]), (2121, vec![1101, PEER_ASN]), ]); - table_becomes(&server, &["194.0.0.0/24", "8.8.8.0/24"], 0).await?; + server + .table_becomes(&["194.0.0.0/24", "8.8.8.0/24"], 0) + .await?; let paths: BTreeSet> = server .received_routes() .iter() @@ -304,9 +219,11 @@ async fn test_routes_follow_the_aspas() -> Result<()> { (3356, vec![0]), (PEER_ASN, vec![0]), ]); - table_becomes(&server, &["194.0.0.0/24"], 1).await?; + server.table_becomes(&["194.0.0.0/24"], 1).await?; cache.set_aspas(vec![]); - table_becomes(&server, &["194.0.0.0/24", "8.8.8.0/24"], 0).await + server + .table_becomes(&["194.0.0.0/24", "8.8.8.0/24"], 0) + .await } /// A peer can be exempt: its routes with invalid paths are taken, and @@ -318,12 +235,12 @@ async fn test_a_peer_can_be_exempt_from_path_verification() -> Result<()> { accept_aspa_invalid: true, ..default_policy() }; - let Setup { server, .. } = server_with(&cache, Some(PeerRole::Peer), import).await?; + let ServerWithPeer { server, .. } = server_with(&cache, Some(PeerRole::Peer), import).await?; let (base, _http) = server.serve_http().await?; let mut peer = established(&server).await?; announce(&mut peer, &["8.8.8.0/24"], &[PEER_ASN, 3356, 15169]).await?; - table_becomes(&server, &["8.8.8.0/24"], 0).await?; + server.table_becomes(&["8.8.8.0/24"], 0).await?; let routes: Value = reqwest::get(format!("{base}/api/routes/received")) .await? .json() @@ -342,16 +259,17 @@ async fn test_a_route_is_held_until_origin_and_path_are_both_in_order() -> Resul asn, }; let cache = TestCache::start_with(vec![google(13335)], aspas()).await; - let Setup { server, .. } = server_with(&cache, Some(PeerRole::Peer), default_policy()).await?; + let ServerWithPeer { server, .. } = + server_with(&cache, Some(PeerRole::Peer), default_policy()).await?; let mut peer = established(&server).await?; announce(&mut peer, &["8.8.8.0/24"], &[PEER_ASN, 3356, 15169]).await?; - table_becomes(&server, &[], 1).await?; + server.table_becomes(&[], 1).await?; // The origin is put right; the path is still a leak. cache.set(vec![google(15169)]); tokio::time::sleep(Duration::from_millis(500)).await; - table_becomes(&server, &[], 1).await?; + server.table_becomes(&[], 1).await?; cache.set_aspas(vec![(15169, vec![3356]), (3356, vec![PEER_ASN])]); - table_becomes(&server, &["8.8.8.0/24"], 0).await + server.table_becomes(&["8.8.8.0/24"], 0).await } diff --git a/crates/integration-tests/tests/basic_connection.rs b/crates/integration-tests/tests/basic_connection.rs index f11f3ca..8c8199c 100644 --- a/crates/integration-tests/tests/basic_connection.rs +++ b/crates/integration-tests/tests/basic_connection.rs @@ -4,6 +4,7 @@ use std::time::Duration; use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::RouteChange; use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::{TIMEOUT, wait_until}; use bgp_packet::errors::{BgpError, CeaseSubcode, FsmSubcode, MsgHeaderSubcode, UpdateMsgSubcode}; use bgp_packet::message::{ AsPathAttribute, Message, MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, @@ -14,8 +15,6 @@ use bgp_primitives::constants::{AfiSafi, IpVersion}; use bgp_primitives::ip_prefix::IpPrefix; use eyre::Result; -const TIMEOUT: Duration = Duration::from_secs(5); - /// The attributes every announcement of IPv4 routes must carry. An UPDATE /// without them has its routes withdrawn instead (RFC 7606). fn mandatory_attributes() -> Vec { @@ -98,20 +97,6 @@ async fn test_bad_marker_does_not_crash_server() -> Result<()> { Ok(()) } -/// Reads until a NOTIFICATION arrives, skipping KEEPALIVEs. -async fn expect_notification(peer: &mut TestPeer) -> Result { - tokio::time::timeout(TIMEOUT, async { - loop { - match peer.recv().await? { - Message::Notification(n) => return Ok(n), - Message::KeepAlive => {} - other => eyre::bail!("expected NOTIFICATION, got {other:?}"), - } - } - }) - .await? -} - /// A header with the given length and type, followed by `body`. fn framed(length: u16, msg_type: u8, body: &[u8]) -> Vec { let mut buf = vec![0xFF; 16]; @@ -170,7 +155,7 @@ async fn test_malformed_messages_are_answered_with_the_matching_notification() - peer.handshake(65001, Ipv4Addr::new(10, 0, 1, i as u8 + 1), 90) .await?; peer.send_raw(&bytes).await?; - let notification = expect_notification(&mut peer).await?; + let notification = peer.expect_notification().await?; assert_eq!( (notification.error, notification.data), (error.clone(), data.clone()), @@ -181,7 +166,7 @@ async fn test_malformed_messages_are_answered_with_the_matching_notification() - let mut peer = TestPeer::connect(server.addr).await?; let _ = peer.recv_open().await?; peer.send_raw(&bytes).await?; - let notification = expect_notification(&mut peer).await?; + let notification = peer.expect_notification().await?; assert_eq!( (notification.error, notification.data), (error, data), @@ -194,7 +179,7 @@ async fn test_malformed_messages_are_answered_with_the_matching_notification() - let mut peer = TestPeer::connect(server.addr).await?; let _ = peer.recv_open().await?; peer.send(&Message::KeepAlive).await?; - let notification = expect_notification(&mut peer).await?; + let notification = peer.expect_notification().await?; assert_eq!( notification.error, BgpError::FsmError(FsmSubcode::UnexpectedOpenSent) @@ -228,13 +213,10 @@ async fn test_message_split_across_a_keepalive_tick_is_still_one_message() -> Re tokio::time::sleep(Duration::from_millis(1300)).await; peer.send_raw(&update[10..]).await?; - tokio::time::timeout(TIMEOUT, async { - while !server.received_routes().iter().any(|e| e.prefix == prefix) { - tokio::time::sleep(Duration::from_millis(20)).await; - } + wait_until("the split UPDATE reaches the RIB", || { + server.received_routes().iter().any(|e| e.prefix == prefix) }) - .await - .map_err(|_| eyre::eyre!("the split UPDATE never reached the RIB"))?; + .await?; assert_eq!(server.registry.list().len(), 1, "session was dropped"); Ok(()) } @@ -266,7 +248,7 @@ async fn test_server_sends_keepalives() -> Result<()> { peer.handshake(65001, Ipv4Addr::new(10, 0, 0, 1), 3).await?; // We should receive a KEEPALIVE within ~1 second (hold/3). - let msg = tokio::time::timeout(Duration::from_secs(3), peer.recv()).await??; + let msg = peer.recv_within(Duration::from_secs(3)).await?; assert!( matches!(msg, Message::KeepAlive), "Expected KEEPALIVE, got {:?}", @@ -286,16 +268,7 @@ async fn test_hold_timer_expires() -> Result<()> { // Don't send any keepalives. The server will send keepalives until its own // hold timer fires and it sends a NOTIFICATION. Drain messages until we // see it (or the outer timeout fires). - let notif = tokio::time::timeout(Duration::from_secs(6), async { - loop { - match peer.recv().await? { - Message::Notification(n) => return Ok::<_, eyre::Error>(n), - Message::KeepAlive => {} // server keepalive — keep waiting - other => eyre::bail!("Unexpected message: {:?}", other), - } - } - }) - .await??; + let notif = tokio::time::timeout(Duration::from_secs(6), peer.expect_notification()).await??; assert!( matches!(notif.error, BgpError::HoldTimer), "Expected HoldTimer NOTIFICATION, got {:?}", @@ -319,8 +292,7 @@ async fn test_peer_sends_notification() -> Result<()> { .await?; // Server should close the connection cleanly. - tokio::time::sleep(Duration::from_millis(100)).await; - assert!(peer.is_closed().await); + assert!(peer.closed_within(TIMEOUT).await); Ok(()) } diff --git a/crates/integration-tests/tests/best_route.rs b/crates/integration-tests/tests/best_route.rs index 1813990..f80986e 100644 --- a/crates/integration-tests/tests/best_route.rs +++ b/crates/integration-tests/tests/best_route.rs @@ -2,66 +2,34 @@ //! peers, which of them the server chooses, and how that is shown. use std::net::Ipv4Addr; -use std::time::Duration; +use bgp_integration_tests::announcement::{Announcement, withdrawal as withdrawal_of}; use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::{RouteChange, TestServer}; -use bgp_packet::message::{ - AsPathAttribute, Message, NextHopPathAttribute, OriginPathAttribute, PathAttribute, - UpdateMessage, -}; +use bgp_integration_tests::{TIMEOUT, get_json, prefix}; +use bgp_packet::message::{PathAttribute, UpdateMessage}; use bgp_primitives::ip_prefix::{IpPrefix, Nlri}; use eyre::Result; use serde_json::Value; const LOCAL_ASN: u32 = 65000; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} /// A session from the peer numbered `number`, in AS 65000 + `number`. async fn peer(server: &TestServer, number: u8) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake( - LOCAL_ASN + u32::from(number), - Ipv4Addr::new(10, 0, 0, number), - 90, - ) - .await?; - Ok(peer) + server + .established( + LOCAL_ASN + u32::from(number), + Ipv4Addr::new(10, 0, 0, number), + ) + .await } -fn announcement(prefixes: &[IpPrefix], as_path: &[u32]) -> Message { - Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(as_path.to_vec()), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ], - announced_routes: Nlri::plain(prefixes.iter().copied()), - }) +fn announcement(prefixes: &[IpPrefix], as_path: &[u32]) -> Announcement { + Announcement::of(Nlri::plain(prefixes.iter().copied()), as_path) } -fn withdrawal(prefixes: &[IpPrefix]) -> Message { - Message::Update(UpdateMessage { - withdrawn_routes: Nlri::plain(prefixes.iter().copied()), - path_attributes: vec![], - announced_routes: vec![], - }) -} - -/// Waits until the table holds `routes` routes. -async fn wait_for_routes(server: &TestServer, routes: usize) -> Result<()> { - tokio::time::timeout(TIMEOUT, async { - while server.rib.len() != routes { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; - Ok(()) +fn withdrawal(prefixes: &[IpPrefix]) -> UpdateMessage { + withdrawal_of(Nlri::plain(prefixes.iter().copied())) } /// The AS path of the route the server holds as best for `prefix`. @@ -78,16 +46,6 @@ fn best_path(server: &TestServer, prefix: &IpPrefix) -> Option> { }) } -async fn get_json(client: &reqwest::Client, url: &str) -> Result { - Ok(client - .get(url) - .send() - .await? - .error_for_status()? - .json() - .await?) -} - /// Two peers offer the same prefix. The route with the shorter AS path is /// chosen; when it goes, the other is. #[tokio::test] @@ -99,14 +57,14 @@ async fn test_the_best_route_is_chosen_and_replaced_when_it_goes() -> Result<()> let p = prefix("203.0.113.0/24"); let mut far = peer(&server, 1).await?; - far.send(&announcement(&[p], &[65001, 64999, 64998])) + far.announce(announcement(&[p], &[65001, 64999, 64998])) .await?; - wait_for_routes(&server, 1).await?; + server.wait_for_routes(1).await?; assert_eq!(best_path(&server, &p), Some(vec![65001, 64999, 64998])); let mut near = peer(&server, 2).await?; - near.send(&announcement(&[p], &[65002, 64998])).await?; - wait_for_routes(&server, 2).await?; + near.announce(announcement(&[p], &[65002, 64998])).await?; + server.wait_for_routes(2).await?; assert_eq!(best_path(&server, &p), Some(vec![65002, 64998])); // Both routes are kept and shown; one is marked. @@ -126,11 +84,11 @@ async fn test_the_best_route_is_chosen_and_replaced_when_it_goes() -> Result<()> // The session with the best route ends: the other route takes over. drop(near); - wait_for_routes(&server, 1).await?; + server.wait_for_routes(1).await?; assert_eq!(best_path(&server, &p), Some(vec![65001, 64999, 64998])); - far.send(&withdrawal(&[p])).await?; - wait_for_routes(&server, 0).await?; + far.announce(withdrawal(&[p])).await?; + server.wait_for_routes(0).await?; assert_eq!(best_path(&server, &p), None); // A follower of the best routes is told of each change: the first @@ -157,9 +115,9 @@ async fn test_changes_of_best_route_are_on_the_event_stream() -> Result<()> { // A route that is there before the client connects is not an event. let mut first = peer(&server, 1).await?; first - .send(&announcement(&[prefix("198.51.100.0/24")], &[65001])) + .announce(announcement(&[prefix("198.51.100.0/24")], &[65001])) .await?; - wait_for_routes(&server, 1).await?; + server.wait_for_routes(1).await?; let mut stream = reqwest::Client::new() .get(format!("{base}/api/events")) @@ -167,10 +125,10 @@ async fn test_changes_of_best_route_are_on_the_event_stream() -> Result<()> { .await? .error_for_status()?; - first.send(&announcement(&[p], &[65001, 64998])).await?; - wait_for_routes(&server, 2).await?; - first.send(&withdrawal(&[p])).await?; - wait_for_routes(&server, 1).await?; + first.announce(announcement(&[p], &[65001, 64998])).await?; + server.wait_for_routes(2).await?; + first.announce(withdrawal(&[p])).await?; + server.wait_for_routes(1).await?; let mut text = String::new(); let mut events: Vec = Vec::new(); @@ -223,8 +181,8 @@ async fn test_received_routes_are_returned_up_to_a_limit() -> Result<()> { let prefixes: Vec = (0..40).map(|n| prefix(&format!("10.{n}.0.0/16"))).collect(); let mut peer = peer(&server, 1).await?; - peer.send(&announcement(&prefixes, &[65001])).await?; - wait_for_routes(&server, 40).await?; + peer.announce(announcement(&prefixes, &[65001])).await?; + server.wait_for_routes(40).await?; let count = |page: Value| page["routes"].as_array().unwrap().len(); let url = format!("{base}/api/routes/received"); diff --git a/crates/integration-tests/tests/collision.rs b/crates/integration-tests/tests/collision.rs index ccbce61..03596a7 100644 --- a/crates/integration-tests/tests/collision.rs +++ b/crates/integration-tests/tests/collision.rs @@ -3,56 +3,33 @@ //! The test server's BGP Identifier is 127.0.0.1. A test peer picks an //! identifier above or below that to steer which connection must survive. -use std::net::{Ipv4Addr, SocketAddr}; +use std::net::Ipv4Addr; use std::time::Duration; +use bgp_integration_tests::announcement::announcement; use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::test_server::{ServerWithPeer, TestServer}; +use bgp_integration_tests::{TIMEOUT, prefix, wait_until}; use bgp_packet::errors::{BgpError, CeaseSubcode, OpenMsgSubcode}; -use bgp_packet::message::{ - AsPathAttribute, Message, NextHopPathAttribute, NotificationMessage, OriginPathAttribute, - PathAttribute, UpdateMessage, -}; -use bgp_primitives::ip_prefix::IpPrefix; +use bgp_packet::message::{Message, NotificationMessage}; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::collision::Direction; -use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; use bgp_server::policy::ImportPolicy; use eyre::Result; -use tokio::net::TcpListener; const LOCAL_ASN: u32 = 65000; const PEER_ASN: u32 = 65001; -const TIMEOUT: Duration = Duration::from_secs(5); /// Above the server's 127.0.0.1. const HIGH_ID: Ipv4Addr = Ipv4Addr::new(192, 0, 2, 1); /// Below the server's 127.0.0.1. const LOW_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 1); -async fn server_dialing(listener: &TcpListener) -> Result { - let endpoint: SocketAddr = listener.local_addr()?; - TestServer::start_with(LOCAL_ASN, |c| { +/// A server that dials its peer "r1" every 200 ms until it has a session. +async fn server_dialing() -> Result { + TestServer::with_peer(LOCAL_ASN, PEER_ASN, |c, peer| { c.connect_retry = Duration::from_millis(200); - c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import: ImportPolicy::permissive(), - role: None, - require_role: false, - export: Default::default(), - }]; + peer.import = ImportPolicy::permissive(); }) .await } @@ -67,27 +44,13 @@ async fn open_only(peer: &mut TestPeer, router_id: Ipv4Addr) -> Result<()> { /// Finishes the handshake begun by [`open_only`]. async fn confirm(peer: &mut TestPeer) -> Result<()> { - match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { + match peer.recv().await? { Message::KeepAlive => {} other => eyre::bail!("expected KEEPALIVE, got {other:?}"), } peer.send(&Message::KeepAlive).await } -/// Reads until a NOTIFICATION arrives, skipping KEEPALIVEs. -async fn expect_notification(peer: &mut TestPeer) -> Result { - tokio::time::timeout(TIMEOUT, async { - loop { - match peer.recv().await? { - Message::Notification(n) => return Ok(n), - Message::KeepAlive => {} - other => eyre::bail!("expected NOTIFICATION, got {other:?}"), - } - } - }) - .await? -} - fn is_collision_cease(n: &NotificationMessage) -> bool { matches!( n.error, @@ -95,38 +58,26 @@ fn is_collision_cease(n: &NotificationMessage) -> bool { ) } -async fn wait_for_sessions(server: &TestServer, n: usize) -> Result<()> { - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().len() != n { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; - Ok(()) -} - /// Peer has the higher identifier, so the connection the *peer* initiated /// must survive: the server closes its own outbound connection. #[tokio::test] async fn test_simultaneous_open_peer_higher_keeps_inbound() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let server = server_dialing(&listener).await?; + let ServerWithPeer { server, listener } = server_dialing().await?; // Server's outbound connection reaches OpenConfirm. - let (stream, _) = tokio::time::timeout(TIMEOUT, listener.accept()).await??; - let mut dialled = TestPeer::from_stream(stream); + let mut dialled = TestPeer::accept(&listener).await?; open_only(&mut dialled, HIGH_ID).await?; // The peer now also dials the server. let mut inbound = TestPeer::connect(server.addr).await?; open_only(&mut inbound, HIGH_ID).await?; - let notif = expect_notification(&mut dialled).await?; + let notif = dialled.expect_notification().await?; assert!(is_collision_cease(¬if), "{notif:?}"); - assert!(dialled.is_closed().await); + assert!(dialled.closed_within(TIMEOUT).await); confirm(&mut inbound).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; let session = server.registry.list().remove(0); assert_eq!(session.direction, Direction::Inbound); assert_eq!(session.name.as_deref(), Some("r1")); @@ -135,11 +86,11 @@ async fn test_simultaneous_open_peer_higher_keeps_inbound() -> Result<()> { server .local_rib .announce(LocalRoute::new( - IpPrefix::try_from("10.100.1.0/24")?, + prefix("10.100.1.0/24"), LocalRouteAttrs::default(), )) .await?; - let update = tokio::time::timeout(TIMEOUT, inbound.recv_update()).await??; + let update = inbound.recv_update().await?; assert_eq!(update.announced_routes[0].to_string(), "10.100.1.0/24"); // The dialer notices the surviving session and does not open another. @@ -155,22 +106,20 @@ async fn test_simultaneous_open_peer_higher_keeps_inbound() -> Result<()> { /// must survive: the peer's inbound connection is refused. #[tokio::test] async fn test_simultaneous_open_peer_lower_keeps_outbound() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let server = server_dialing(&listener).await?; + let ServerWithPeer { server, listener } = server_dialing().await?; - let (stream, _) = tokio::time::timeout(TIMEOUT, listener.accept()).await??; - let mut dialled = TestPeer::from_stream(stream); + let mut dialled = TestPeer::accept(&listener).await?; open_only(&mut dialled, LOW_ID).await?; let mut inbound = TestPeer::connect(server.addr).await?; open_only(&mut inbound, LOW_ID).await?; - let notif = expect_notification(&mut inbound).await?; + let notif = inbound.expect_notification().await?; assert!(is_collision_cease(¬if), "{notif:?}"); - assert!(inbound.is_closed().await); + assert!(inbound.closed_within(TIMEOUT).await); confirm(&mut dialled).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; assert_eq!(server.registry.list()[0].direction, Direction::Outbound); Ok(()) } @@ -179,22 +128,20 @@ async fn test_simultaneous_open_peer_lower_keeps_outbound() -> Result<()> { /// outcome: here the inbound connection reaches OpenConfirm first. #[tokio::test] async fn test_outcome_is_independent_of_arrival_order() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let server = server_dialing(&listener).await?; + let ServerWithPeer { server, listener } = server_dialing().await?; - let (stream, _) = tokio::time::timeout(TIMEOUT, listener.accept()).await??; - let mut dialled = TestPeer::from_stream(stream); + let mut dialled = TestPeer::accept(&listener).await?; // Inbound first this time; peer is lower so outbound must still win. let mut inbound = TestPeer::connect(server.addr).await?; open_only(&mut inbound, LOW_ID).await?; open_only(&mut dialled, LOW_ID).await?; - let notif = expect_notification(&mut inbound).await?; + let notif = inbound.expect_notification().await?; assert!(is_collision_cease(¬if), "{notif:?}"); confirm(&mut dialled).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; assert_eq!(server.registry.list()[0].direction, Direction::Outbound); Ok(()) } @@ -207,11 +154,11 @@ async fn test_established_session_wins_by_default() -> Result<()> { let mut first = TestPeer::connect(server.addr).await?; first.handshake(PEER_ASN, HIGH_ID, 90).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; let mut second = TestPeer::connect(server.addr).await?; open_only(&mut second, HIGH_ID).await?; - let notif = expect_notification(&mut second).await?; + let notif = second.expect_notification().await?; assert!(is_collision_cease(¬if), "{notif:?}"); // The original is untouched. @@ -219,11 +166,11 @@ async fn test_established_session_wins_by_default() -> Result<()> { server .local_rib .announce(LocalRoute::new( - IpPrefix::try_from("10.100.1.0/24")?, + prefix("10.100.1.0/24"), LocalRouteAttrs::default(), )) .await?; - let update = tokio::time::timeout(TIMEOUT, first.recv_update()).await??; + let update = first.recv_update().await?; assert_eq!(update.announced_routes[0].to_string(), "10.100.1.0/24"); Ok(()) } @@ -238,41 +185,26 @@ async fn test_restarted_peer_replaces_stale_session_when_enabled() -> Result<()> let mut stale = TestPeer::connect(server.addr).await?; stale.handshake(PEER_ASN, HIGH_ID, 90).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; stale - .send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(10, 0, 0, 9))), - ], - announced_routes: vec![IpPrefix::try_from("203.0.113.0/24")?.into()], - })) + .announce( + announcement(&["203.0.113.0/24"], &[PEER_ASN]) + .with_next_hop(Ipv4Addr::new(10, 0, 0, 9)), + ) .await?; - tokio::time::timeout(TIMEOUT, async { - while server.received_routes().is_empty() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; + server.wait_for_routes(1).await?; let stale_addr = server.registry.list()[0].peer.addr; // "Restart": a fresh connection from the same router. let mut fresh = TestPeer::connect(server.addr).await?; fresh.handshake(PEER_ASN, HIGH_ID, 90).await?; - let notif = expect_notification(&mut stale).await?; + let notif = stale.expect_notification().await?; assert!(is_collision_cease(¬if), "{notif:?}"); - tokio::time::timeout(TIMEOUT, async { - loop { - let sessions = server.registry.list(); - if sessions.len() == 1 && sessions[0].peer.addr != stale_addr { - break; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } + wait_until("the fresh session has replaced the stale one", || { + let sessions = server.registry.list(); + sessions.len() == 1 && sessions[0].peer.addr != stale_addr }) .await?; assert!( @@ -290,7 +222,7 @@ async fn test_distinct_identifiers_do_not_collide() -> Result<()> { a.handshake(PEER_ASN, HIGH_ID, 90).await?; let mut b = TestPeer::connect(server.addr).await?; b.handshake(PEER_ASN, LOW_ID, 90).await?; - wait_for_sessions(&server, 2).await?; + server.wait_for_sessions(2).await?; Ok(()) } @@ -300,13 +232,13 @@ async fn test_identifier_is_released_when_connection_ends() -> Result<()> { let server = TestServer::start(LOCAL_ASN).await?; let mut first = TestPeer::connect(server.addr).await?; first.handshake(PEER_ASN, HIGH_ID, 90).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; drop(first); - wait_for_sessions(&server, 0).await?; + server.wait_for_sessions(0).await?; let mut second = TestPeer::connect(server.addr).await?; second.handshake(PEER_ASN, HIGH_ID, 90).await?; - wait_for_sessions(&server, 1).await?; + server.wait_for_sessions(1).await?; Ok(()) } @@ -316,7 +248,7 @@ async fn test_peer_with_our_identifier_is_refused() -> Result<()> { let server = TestServer::start(LOCAL_ASN).await?; let mut peer = TestPeer::connect(server.addr).await?; open_only(&mut peer, Ipv4Addr::new(127, 0, 0, 1)).await?; - let notif = expect_notification(&mut peer).await?; + let notif = peer.expect_notification().await?; assert!( matches!(notif.error, BgpError::OpenMsg(OpenMsgSubcode::BadBgpId)), "{notif:?}" diff --git a/crates/integration-tests/tests/debugging.rs b/crates/integration-tests/tests/debugging.rs index 7da07b3..427ae92 100644 --- a/crates/integration-tests/tests/debugging.rs +++ b/crates/integration-tests/tests/debugging.rs @@ -2,39 +2,13 @@ //! what was refused. use std::net::Ipv4Addr; -use std::time::Duration; -use bgp_integration_tests::default_policy; -use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; -use bgp_packet::message::{ - AsPathAttribute, Message, NextHopPathAttribute, OriginPathAttribute, PathAttribute, - UpdateMessage, -}; -use bgp_primitives::ip_prefix::IpPrefix; -use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; +use bgp_integration_tests::announcement::announcement; +use bgp_integration_tests::test_server::{ServerWithPeer, TestServer}; +use bgp_integration_tests::{default_policy, wait_until}; use bgp_server::policy::ImportPolicy; use eyre::Result; use serde_json::Value; -use tokio::net::TcpListener; - -const TIMEOUT: Duration = Duration::from_secs(5); - -fn announcement(prefixes: &[&str], as_path: &[u32]) -> Message { - Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(as_path.to_vec()), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ], - announced_routes: prefixes - .iter() - .map(|p| IpPrefix::try_from(*p).unwrap().into()) - .collect(), - }) -} /// The routes a query returns, as "prefix from AS, best or not". async fn routes(client: &reqwest::Client, base: &str, query: &str) -> Result> { @@ -67,24 +41,21 @@ async fn test_routes_can_be_looked_up() -> Result<()> { let (base, _http) = server.serve_http().await?; let client = reqwest::Client::new(); - let mut one = TestPeer::connect(server.addr).await?; - one.handshake(65001, Ipv4Addr::new(10, 0, 0, 1), 90).await?; - let mut two = TestPeer::connect(server.addr).await?; - two.handshake(65002, Ipv4Addr::new(10, 0, 0, 2), 90).await?; - one.send(&announcement( + let mut one = server + .established(65001, Ipv4Addr::new(10, 0, 0, 1)) + .await?; + let mut two = server + .established(65002, Ipv4Addr::new(10, 0, 0, 2)) + .await?; + one.announce(announcement( &["10.0.0.0/8", "10.1.0.0/16", "10.1.2.0/24", "192.0.2.0/24"], &[65001], )) .await?; // A longer path for one of the same prefixes: it loses. - two.send(&announcement(&["10.1.0.0/16"], &[65002, 65003])) + two.announce(announcement(&["10.1.0.0/16"], &[65002, 65003])) .await?; - tokio::time::timeout(TIMEOUT, async { - while server.rib.len() != 5 { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; + server.wait_for_routes(5).await?; // One prefix: every route for it, the best first. assert_eq!( @@ -162,31 +133,15 @@ async fn test_routes_can_be_looked_up() -> Result<()> { /// something to alert on. #[tokio::test] async fn test_refused_routes_can_be_seen_and_counted() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let endpoint = listener.local_addr()?; - let server = TestServer::start_with(1299, |c| { - c.accept_unconfigured_peers = false; - c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "refused-demo".into(), - endpoint: endpoint.into(), - remote_asn: 3333, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import: ImportPolicy { - max_prefixes: Some(100), - ..default_policy() - }, - role: None, - require_role: false, - export: Default::default(), - }]; + let ServerWithPeer { + server, + listener: _listener, + } = TestServer::with_peer(1299, 3333, |_, peer| { + peer.name = "refused-demo".into(); + peer.import = ImportPolicy { + max_prefixes: Some(100), + ..default_policy() + }; }) .await?; let (base, _http) = server.serve_http().await?; @@ -199,27 +154,23 @@ async fn test_refused_routes_can_be_seen_and_counted() -> Result<()> { let refused: Value = get("/api/routes/rejected").await?.json().await?; assert_eq!(refused, serde_json::json!([]), "nothing refused yet"); - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(3333, Ipv4Addr::new(10, 0, 0, 2), 90).await?; - peer.send(&announcement(&["193.0.0.0/21", "10.0.0.0/8"], &[3333])) + let mut peer = server.established(3333, Ipv4Addr::new(10, 0, 0, 2)).await?; + peer.announce(announcement(&["193.0.0.0/21", "10.0.0.0/8"], &[3333])) .await?; - peer.send(&announcement( + peer.announce(announcement( &["194.0.1.0/24", "194.0.2.0/24"], &[3333, 64512], )) .await?; - peer.send(&announcement(&["193.0.9.0/25"], &[3333, 2121])) + peer.announce(announcement(&["193.0.9.0/25"], &[3333, 2121])) .await?; - tokio::time::timeout(TIMEOUT, async { - while server + wait_until("four routes are refused", || { + server .registry .list() .first() .map(|s| s.counters.rejected()) - != Some(4) - { - tokio::time::sleep(Duration::from_millis(5)).await; - } + == Some(4) }) .await?; @@ -301,12 +252,7 @@ async fn test_refused_routes_can_be_seen_and_counted() -> Result<()> { // The record outlives the session; the count of routes does not. drop(peer); - tokio::time::timeout(TIMEOUT, async { - while !server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; + server.wait_for_sessions(0).await?; let refused: Value = get("/api/routes/rejected").await?.json().await?; assert_eq!(refused[0]["total"], 4); let metrics = get("/metricz").await?.text().await?; diff --git a/crates/integration-tests/tests/dialer.rs b/crates/integration-tests/tests/dialer.rs index 86b85d5..49c1400 100644 --- a/crates/integration-tests/tests/dialer.rs +++ b/crates/integration-tests/tests/dialer.rs @@ -5,89 +5,47 @@ use std::net::{Ipv4Addr, SocketAddr}; use std::time::Duration; use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::test_server::{ServerWithPeer, TestServer}; +use bgp_integration_tests::{TIMEOUT, peer_config, prefix, wait_for, wait_until}; use bgp_packet::errors::{BgpError, OpenMsgSubcode}; use bgp_packet::message::Message; -use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; use bgp_server::policy::ImportPolicy; use bgp_server::session::DialState; use eyre::Result; -use tokio::net::TcpListener; const LOCAL_ASN: u32 = 65000; const PEER_ASN: u32 = 65001; -const TIMEOUT: Duration = Duration::from_secs(5); +const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 2); -/// Starts a server configured to dial `listener`'s address as peer "r1". -async fn server_dialing(listener: &TcpListener, remote_asn: u32) -> Result { - server_with_peer(listener, remote_asn, false).await +/// A server that dials its peer "r1", in `remote_asn`, every 200 ms. +async fn server_dialing(remote_asn: u32) -> Result { + server_with_peer(remote_asn, false).await } -/// Starts a server with peer "r1" at `listener`'s address, which it dials +/// A server with peer "r1" in `remote_asn`, which it dials every 200 ms /// unless the peer is `passive`. -async fn server_with_peer( - listener: &TcpListener, - remote_asn: u32, - passive: bool, -) -> Result { - let endpoint: SocketAddr = listener.local_addr()?; - TestServer::start_with(LOCAL_ASN, |c| { +async fn server_with_peer(remote_asn: u32, passive: bool) -> Result { + TestServer::with_peer(LOCAL_ASN, remote_asn, |c, peer| { c.connect_retry = Duration::from_millis(200); - c.accept_unconfigured_peers = !passive; - c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive, - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import: ImportPolicy::permissive(), - role: None, - require_role: false, - export: Default::default(), - }]; + peer.import = ImportPolicy::permissive(); + peer.passive = passive; }) .await } -async fn accept(listener: &TcpListener) -> Result { - let (stream, _) = tokio::time::timeout(TIMEOUT, listener.accept()).await??; - Ok(TestPeer::from_stream(stream)) -} - #[tokio::test] async fn test_server_dials_configured_peer_and_establishes() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let server = server_dialing(&listener, PEER_ASN).await?; + let ServerWithPeer { server, listener } = server_dialing(PEER_ASN).await?; - let mut peer = accept(&listener).await?; + let mut peer = TestPeer::accept(&listener).await?; // Both sides send OPEN on connect; the test peer's handshake order works // whichever side opened TCP. - let open = peer - .handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; + let open = peer.handshake(PEER_ASN, PEER_ID, 90).await?; assert_eq!(open.asn, LOCAL_ASN as u16); - let mut events = server.registry.subscribe(); - // Either the Up event is still queued or the session is already listed. - let listed = tokio::time::timeout(TIMEOUT, async { - loop { - if let Some(s) = server.registry.list().first().cloned() { - return s; - } - let _ = events.recv().await; - } - }) - .await?; + let listed = server.wait_for_session().await?; assert_eq!(listed.name.as_deref(), Some("r1")); assert_eq!(listed.peer.asn, PEER_ASN); assert_eq!(listed.peer.addr, listener.local_addr()?); @@ -109,39 +67,26 @@ async fn test_server_dials_configured_peer_and_establishes() -> Result<()> { server .local_rib .announce(LocalRoute::new( - IpPrefix::try_from("10.100.1.0/24")?, + prefix("10.100.1.0/24"), LocalRouteAttrs::default(), )) .await?; - let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + let update = peer.recv_update().await?; assert_eq!(update.announced_routes[0].to_string(), "10.100.1.0/24"); Ok(()) } #[tokio::test] async fn test_server_reconnects_after_peer_closes() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let server = server_dialing(&listener, PEER_ASN).await?; + let ServerWithPeer { server, listener } = server_dialing(PEER_ASN).await?; - let mut first = accept(&listener).await?; - first - .handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; + let mut first = TestPeer::accept(&listener).await?; + first.handshake(PEER_ASN, PEER_ID, 90).await?; + server.wait_for_session().await?; drop(first); // Session goes away, then the dialer comes back. - tokio::time::timeout(TIMEOUT, async { - while !server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; + server.wait_for_sessions(0).await?; let (_, state) = server .registry .configured_peers() @@ -150,45 +95,30 @@ async fn test_server_reconnects_after_peer_closes() -> Result<()> { .unwrap(); assert!(matches!(state, DialState::Backoff { .. }), "{state:?}"); - let mut second = accept(&listener).await?; - second - .handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; + let mut second = TestPeer::accept(&listener).await?; + second.handshake(PEER_ASN, PEER_ID, 90).await?; + server.wait_for_session().await?; Ok(()) } #[tokio::test] async fn test_wrong_asn_is_refused_with_bad_peer_as() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let _server = server_dialing(&listener, PEER_ASN).await?; + let ServerWithPeer { + server: _server, + listener, + } = server_dialing(PEER_ASN).await?; - let mut peer = accept(&listener).await?; - peer.send_open(65099, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; + let mut peer = TestPeer::accept(&listener).await?; + peer.send_open(65099, PEER_ID, 90).await?; let _ = peer.recv_open().await?; - let notif = tokio::time::timeout(TIMEOUT, async { - loop { - match peer.recv().await? { - Message::Notification(n) => return Ok::<_, eyre::Error>(n), - Message::KeepAlive => {} - other => eyre::bail!("unexpected {other:?}"), - } - } - }) - .await??; + let notif = peer.expect_notification().await?; assert!( matches!(notif.error, BgpError::OpenMsg(OpenMsgSubcode::BadPeerAs)), "{notif:?}" ); // The dialer retries rather than giving up. - let _again = accept(&listener).await?; + let _again = TestPeer::accept(&listener).await?; Ok(()) } @@ -200,32 +130,19 @@ async fn test_inbound_from_configured_address_uses_config() -> Result<()> { // so the dialer never succeeds; then connect inbound from loopback. let server = TestServer::start_with(LOCAL_ASN, |c| { c.connect_retry = Duration::from_secs(60); + let at = SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 1); c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "r1".into(), - endpoint: SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 1).into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, import: ImportPolicy::permissive(), - role: None, - require_role: false, - export: Default::default(), + ..peer_config("r1", at, PEER_ASN) }]; }) .await?; // Wrong ASN inbound → refused. let mut bad = TestPeer::connect(server.addr).await?; - bad.send_open(65099, Ipv4Addr::new(10, 0, 0, 2), 90).await?; + bad.send_open(65099, PEER_ID, 90).await?; let _ = bad.recv_open().await?; - let msg = tokio::time::timeout(TIMEOUT, bad.recv()).await??; + let msg = bad.recv().await?; assert!( matches!(&msg, Message::Notification(n) if matches!(n.error, BgpError::OpenMsg(OpenMsgSubcode::BadPeerAs))), "{msg:?}" @@ -233,17 +150,8 @@ async fn test_inbound_from_configured_address_uses_config() -> Result<()> { // Right ASN inbound → established, named. let mut good = TestPeer::connect(server.addr).await?; - good.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; - let listed = tokio::time::timeout(TIMEOUT, async { - loop { - if let Some(s) = server.registry.list().first().cloned() { - return s; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; + good.handshake(PEER_ASN, PEER_ID, 90).await?; + let listed = server.wait_for_session().await?; assert_eq!(listed.name.as_deref(), Some("r1")); Ok(()) } @@ -253,64 +161,33 @@ async fn test_inbound_from_configured_address_uses_config() -> Result<()> { /// reported as the reason for backing off rather than silently ignored. #[tokio::test] async fn test_local_address_is_used_as_the_source() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let endpoint: SocketAddr = listener.local_addr()?; - let _server = TestServer::start_with(LOCAL_ASN, |c| { + let ServerWithPeer { + server: _server, + listener, + } = TestServer::with_peer(LOCAL_ASN, PEER_ASN, |c, peer| { c.connect_retry = Duration::from_millis(200); - c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: Some(Ipv4Addr::LOCALHOST.into()), - families: FamilySet::supported(), - add_path: None, - import: ImportPolicy::permissive(), - role: None, - require_role: false, - export: Default::default(), - }]; + peer.import = ImportPolicy::permissive(); + peer.local_address = Some(Ipv4Addr::LOCALHOST.into()); }) .await?; let (_stream, from) = tokio::time::timeout(TIMEOUT, listener.accept()).await??; assert_eq!(from.ip(), std::net::IpAddr::from(Ipv4Addr::LOCALHOST)); // 192.0.2.1 is documentation space; no host has it. + let endpoint = listener.local_addr()?; let server = TestServer::start_with(LOCAL_ASN, |c| { c.connect_retry = Duration::from_millis(200); c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, local_address: Some(Ipv4Addr::new(192, 0, 2, 1).into()), - families: FamilySet::supported(), - add_path: None, import: ImportPolicy::permissive(), - role: None, - require_role: false, - export: Default::default(), + ..peer_config("r1", endpoint, PEER_ASN) }]; }) .await?; - let reason = tokio::time::timeout(TIMEOUT, async { - loop { - if let Some((_, DialState::Backoff { last_error, .. })) = - server.registry.configured_peers().into_iter().next() - { - return last_error; - } - tokio::time::sleep(Duration::from_millis(20)).await; + let reason = wait_for("the dialer is backing off", || { + match server.registry.configured_peers().into_iter().next() { + Some((_, DialState::Backoff { last_error, .. })) => Some(last_error), + _ => None, } }) .await?; @@ -325,16 +202,14 @@ async fn test_local_address_is_used_as_the_source() -> Result<()> { /// such a peer was dialled every few seconds for ever. #[tokio::test] async fn test_a_peer_that_refuses_the_session_is_dialled_ever_more_slowly() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let server = server_dialing(&listener, PEER_ASN).await?; + let ServerWithPeer { server, listener } = server_dialing(PEER_ASN).await?; let mut waits = Vec::new(); for _ in 0..3 { - let mut peer = accept(&listener).await?; - peer.send_open(65099, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; + let mut peer = TestPeer::accept(&listener).await?; + peer.send_open(65099, PEER_ID, 90).await?; let _ = peer.recv_open().await?; - let refusal = tokio::time::timeout(TIMEOUT, peer.recv()).await??; + let refusal = peer.recv().await?; assert!( matches!(&refusal, Message::Notification(n) if n.error == BgpError::OpenMsg(OpenMsgSubcode::BadPeerAs)), "{refusal:?}" @@ -342,17 +217,17 @@ async fn test_a_peer_that_refuses_the_session_is_dialled_ever_more_slowly() -> R drop(peer); // How long the dialer says it will wait before trying again. - let wait = tokio::time::timeout(TIMEOUT, async { - loop { - if let Some((_, DialState::Backoff { until, last_error })) = - server.registry.configured_peers().into_iter().next() - { + let wait = wait_for("the dialer is backing off", || { + match server.registry.configured_peers().into_iter().next() { + Some((_, DialState::Backoff { until, last_error })) => { assert!(last_error.contains("AS65099"), "{last_error}"); - return until - .duration_since(std::time::SystemTime::now()) - .unwrap_or_default(); + Some( + until + .duration_since(std::time::SystemTime::now()) + .unwrap_or_default(), + ) } - tokio::time::sleep(Duration::from_millis(5)).await; + _ => None, } }) .await?; @@ -371,8 +246,7 @@ async fn test_a_peer_that_refuses_the_session_is_dialled_ever_more_slowly() -> R /// session ended. #[tokio::test] async fn test_a_passive_peer_is_not_dialled_and_comes_up_when_it_connects() -> Result<()> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let server = server_with_peer(&listener, PEER_ASN, true).await?; + let ServerWithPeer { server, listener } = server_with_peer(PEER_ASN, true).await?; let state = || server.registry.configured_peers()[0].1.clone(); // Many retry intervals pass, and nothing connects. @@ -385,29 +259,18 @@ async fn test_a_passive_peer_is_not_dialled_and_comes_up_when_it_connects() -> R assert_eq!(state(), DialState::Passive { last_error: None }); let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; - let listed = tokio::time::timeout(TIMEOUT, async { - loop { - if let Some(session) = server.registry.list().first().cloned() { - return session; - } - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; + peer.handshake(PEER_ASN, PEER_ID, 90).await?; + let listed = server.wait_for_session().await?; assert_eq!(listed.name.as_deref(), Some("r1")); drop(peer); - tokio::time::timeout(TIMEOUT, async { - while !matches!( + wait_until("the peer is shown as waiting, with how it ended", || { + matches!( state(), DialState::Passive { last_error: Some(_) } - ) { - tokio::time::sleep(Duration::from_millis(5)).await; - } + ) }) .await?; assert!(server.registry.list().is_empty()); diff --git a/crates/integration-tests/tests/export_limits.rs b/crates/integration-tests/tests/export_limits.rs index c3a9014..cf44f0c 100644 --- a/crates/integration-tests/tests/export_limits.rs +++ b/crates/integration-tests/tests/export_limits.rs @@ -7,78 +7,39 @@ use std::net::Ipv4Addr; use std::sync::Arc; use std::time::Duration; -use bgp_integration_tests::default_policy; use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::test_server::{ServerWithPeer, TestServer}; +use bgp_integration_tests::{TIMEOUT, prefix}; use bgp_packet::errors::{BgpError, CeaseSubcode}; use bgp_packet::message::Message; use bgp_packet::open::{BgpRole, Capability}; use bgp_primitives::constants::AfiSafi; -use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::LocalRoute; -use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; use bgp_server::policy::{ExportLimits, PeerRole, PrefixSet}; use eyre::{Result, bail}; -use tokio::net::TcpListener; const LOCAL_ASN: u32 = 1299; const PEER_ASN: u32 = 3333; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} - -/// What a test holds on to: the server, and the listener its dialling -/// goes to. -struct Setup { - server: TestServer, - _listener: TcpListener, -} +const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 2); /// A server that originates `originated`, with one configured peer at the /// address the test connects from, which is a peer of ours held to /// `limits`. -async fn server_with(originated: &[&str], limits: ExportLimits) -> Result { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let endpoint = listener.local_addr()?; +async fn server_with(originated: &[&str], limits: ExportLimits) -> Result { let originate = originated .iter() .map(|p| LocalRoute::new(prefix(p), Default::default())) .collect(); - let server = TestServer::start_with(LOCAL_ASN, |c| { - c.accept_unconfigured_peers = false; + TestServer::with_peer(LOCAL_ASN, PEER_ASN, |c, peer| { c.originate = originate; - c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: Some(Arc::new(limits)), - passive: false, - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import: default_policy(), - role: Some(PeerRole::Peer), - require_role: false, - export: Default::default(), - }]; - }) - .await?; - Ok(Setup { - server, - _listener: listener, + peer.export_limits = Some(Arc::new(limits)); + peer.role = Some(PeerRole::Peer); }) + .await } /// A session with the server, as its peer. async fn established(server: &TestServer) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; let capabilities = vec![ Capability::FourByteAsn { asn: PEER_ASN }, Capability::multiprotocol(AfiSafi::IPV4_UNICAST), @@ -87,9 +48,9 @@ async fn established(server: &TestServer) -> Result { role: BgpRole::Peer, }, ]; - peer.handshake_with(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90, capabilities) - .await?; - Ok(peer) + server + .established_with(PEER_ASN, PEER_ID, capabilities) + .await } /// What the peer has been told once it has been quiet for a while: the @@ -97,8 +58,7 @@ async fn established(server: &TestServer) -> Result { /// did. async fn told(peer: &mut TestPeer) -> (BTreeSet, Option<(BgpError, Vec)>) { let mut routes = BTreeSet::new(); - while let Ok(Ok(message)) = tokio::time::timeout(Duration::from_millis(500), peer.recv()).await - { + while let Ok(message) = peer.recv_within(Duration::from_millis(500)).await { match message { Message::Update(update) => { for route in &update.withdrawn_routes { @@ -134,7 +94,8 @@ async fn test_a_route_outside_the_export_limits_is_not_announced() -> Result<()> prefixes: allowing(&["193.0.0.0/21"]), max_prefixes: None, }; - let Setup { server, .. } = server_with(&["193.0.0.0/21", "198.51.100.0/24"], limits).await?; + let ServerWithPeer { server, .. } = + server_with(&["193.0.0.0/21", "198.51.100.0/24"], limits).await?; let mut peer = established(&server).await?; let (routes, ended) = told(&mut peer).await; assert_eq!(routes, BTreeSet::from(["193.0.0.0/21".to_string()])); @@ -152,7 +113,8 @@ async fn test_going_over_the_export_limit_ends_the_session_and_keeps_the_peer_do prefixes: None, max_prefixes: Some(2), }; - let Setup { server, .. } = server_with(&["193.0.0.0/21", "193.0.8.0/21"], limits).await?; + let ServerWithPeer { server, .. } = + server_with(&["193.0.0.0/21", "193.0.8.0/21"], limits).await?; let mut peer = established(&server).await?; let (routes, ended) = told(&mut peer).await; assert_eq!(routes.len(), 2); @@ -173,15 +135,12 @@ async fn test_going_over_the_export_limit_ends_the_session_and_keeps_the_peer_do text.starts_with("export limit of 2 routes reached"), "{text}" ); - assert!(peer.is_closed().await); + assert!(peer.closed_within(TIMEOUT).await); // The peer is kept down: a new connection is closed unanswered. let mut again = TestPeer::connect(server.addr).await?; assert!( - matches!( - tokio::time::timeout(TIMEOUT, again.recv()).await, - Ok(Err(_)) - ), + again.closed_unanswered().await, "a held-down peer gets no session" ); Ok(()) diff --git a/crates/integration-tests/tests/failure.rs b/crates/integration-tests/tests/failure.rs index 47fd29f..68490c5 100644 --- a/crates/integration-tests/tests/failure.rs +++ b/crates/integration-tests/tests/failure.rs @@ -6,52 +6,24 @@ //! stops with an error, for whatever supervises it to restart it. use std::net::Ipv4Addr; -use std::time::Duration; -use bgp_integration_tests::test_peer::TestPeer; +use bgp_integration_tests::TIMEOUT; use bgp_integration_tests::test_server::TestServer; use bgp_packet::errors::{BgpError, CeaseSubcode}; -use bgp_packet::message::Message; use bgp_rib::RibStopped; use bgp_server::error::ServerError; -use eyre::{Result, bail}; +use eyre::Result; const LOCAL_ASN: u32 = 65000; const PEER_ASN: u32 = 65001; -const TIMEOUT: Duration = Duration::from_secs(5); - -async fn established(server: &TestServer) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 1), 90) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; - Ok(peer) -} - -/// Reads until the server's NOTIFICATION, skipping KEEPALIVEs. -async fn farewell(peer: &mut TestPeer) -> Result { - tokio::time::timeout(TIMEOUT, async { - loop { - match peer.recv().await? { - Message::KeepAlive => {} - Message::Notification(n) => return Ok(n.error), - other => bail!("expected NOTIFICATION, got {other:?}"), - } - } - }) - .await? -} #[tokio::test] async fn test_losing_the_local_rib_stops_the_server_and_tells_the_peers() -> Result<()> { let mut server = TestServer::start(LOCAL_ASN).await?; let (base, _http) = server.serve_http().await?; - let mut peer = established(&server).await?; + let mut peer = server + .established(PEER_ASN, Ipv4Addr::new(10, 0, 0, 1)) + .await?; server.local_rib.stop_for_testing(); @@ -61,7 +33,7 @@ async fn test_losing_the_local_rib_stops_the_server_and_tells_the_peers() -> Res "{outcome:?}" ); assert_eq!( - farewell(&mut peer).await?, + peer.expect_notification().await?.error, BgpError::Cease(CeaseSubcode::AdminShutdown) ); diff --git a/crates/integration-tests/tests/graceful_restart.rs b/crates/integration-tests/tests/graceful_restart.rs index c52daee..8c5c761 100644 --- a/crates/integration-tests/tests/graceful_restart.rs +++ b/crates/integration-tests/tests/graceful_restart.rs @@ -4,20 +4,17 @@ use std::net::Ipv4Addr; use std::time::Duration; +use bgp_integration_tests::announcement::{Announcement, announcement}; use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::{TIMEOUT, wait_until as until}; use bgp_packet::errors::{BgpError, CeaseSubcode}; -use bgp_packet::message::{ - AsPathAttribute, Message, NextHopPathAttribute, NotificationMessage, OriginPathAttribute, - PathAttribute, UpdateMessage, -}; +use bgp_packet::message::{Message, NotificationMessage, UpdateMessage}; use bgp_packet::open::{Capability, GracefulRestartEntry}; use bgp_primitives::constants::{Afi, AfiSafi, Safi}; -use bgp_primitives::ip_prefix::IpPrefix; use bgp_server::restart::Leave; use eyre::{Result, bail}; -const TIMEOUT: Duration = Duration::from_secs(5); const PEER_ASN: u32 = 65001; const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 2); const PREFIXES: [&str; 3] = ["10.1.0.0/16", "10.2.0.0/16", "10.3.0.0/16"]; @@ -57,37 +54,13 @@ fn capabilities(more: impl IntoIterator) -> Vec { capabilities } -fn announcement(prefixes: &[&str]) -> Message { - Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ], - announced_routes: prefixes - .iter() - .map(|p| IpPrefix::try_from(*p).unwrap().into()) - .collect(), - }) +/// The peer's own routes. +fn own(prefixes: &[&str]) -> Announcement { + announcement(prefixes, &[PEER_ASN]) } -fn end_of_rib(family: AfiSafi) -> Message { - Message::Update(UpdateMessage::end_of_rib(family)) -} - -/// Waits until `done` holds, or fails with `what`. -async fn until(what: &str, mut done: impl FnMut() -> bool) -> Result<()> { - let waited = tokio::time::timeout(TIMEOUT, async { - while !done() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await; - if waited.is_err() { - bail!("timed out waiting until {what}"); - } - Ok(()) +fn end_of_rib(family: AfiSafi) -> UpdateMessage { + UpdateMessage::end_of_rib(family) } /// The prefixes in the server's table, and how many of its routes are @@ -109,7 +82,7 @@ async fn peer_with_routes(server: &TestServer, offer: Capability) -> Result Result<()> { let mut peer = TestPeer::connect(server.addr).await?; peer.handshake_with(PEER_ASN, PEER_ID, 90, capabilities(offer)) .await?; - peer.send(&announcement(&PREFIXES)).await?; + peer.announce(own(&PREFIXES)).await?; until("the routes are in", || server.rib.len() == 3).await?; if let Some(goodbye) = &goodbye { peer.send(&Message::Notification(goodbye.clone())).await?; @@ -310,7 +283,7 @@ async fn test_we_offer_graceful_restart_and_say_when_our_routes_are_all_sent() - // Nothing is originated: the markers are all there is. let mut markers = Vec::new(); while markers.len() < 2 { - let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + let update = peer.recv_update().await?; match update.end_of_rib_family() { Some(family) => markers.push(family), None => bail!("expected End-of-RIB, got {update:?}"), @@ -346,14 +319,14 @@ async fn test_leaving_tells_each_peer_as_graceful_restart_has_it() -> Result<()> server.registry.leave(leave, Duration::from_secs(2)).await; let mut notification = None; // Whatever came before (End-of-RIB markers) is passed over. - while let Ok(Ok(message)) = tokio::time::timeout(TIMEOUT, peer.recv()).await { + while let Ok(message) = peer.recv().await { if let Message::Notification(n) = message { notification = Some(n.error); break; } } assert_eq!(notification, told, "{leave:?} with {offer:?}"); - assert!(peer.is_closed().await); + assert!(peer.closed_within(TIMEOUT).await); } Ok(()) } diff --git a/crates/integration-tests/tests/http_api.rs b/crates/integration-tests/tests/http_api.rs index 322db21..b8a034b 100644 --- a/crates/integration-tests/tests/http_api.rs +++ b/crates/integration-tests/tests/http_api.rs @@ -3,46 +3,22 @@ use std::net::Ipv4Addr; use std::time::Duration; +use bgp_integration_tests::announcement::announcement; use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::TestServer; -use bgp_packet::message::{ - AsPathAttribute, CommunitiesPathAttribute, LargeCommunity, Message, NextHopPathAttribute, - OriginPathAttribute, PathAttribute, UpdateMessage, -}; -use bgp_primitives::ip_prefix::IpPrefix; +use bgp_integration_tests::{TIMEOUT, get_json, prefix}; +use bgp_packet::message::LargeCommunity; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use eyre::Result; use serde_json::{Value, json}; const LOCAL_ASN: u32 = 65000; const PEER_ASN: u32 = 65001; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} async fn established_peer(server: &TestServer) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 1), 90) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; - Ok(peer) -} - -async fn get_json(client: &reqwest::Client, url: &str) -> Result { - Ok(client - .get(url) - .send() - .await? - .error_for_status()? - .json() - .await?) + server + .established(PEER_ASN, Ipv4Addr::new(10, 0, 0, 1)) + .await } #[tokio::test] @@ -155,16 +131,11 @@ async fn test_received_routes_summary() -> Result<()> { let client = reqwest::Client::new(); let mut peer = established_peer(&server).await?; - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN, 65002]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(10, 0, 0, 1))), - PathAttribute::Communities(CommunitiesPathAttribute(vec![(65001, 7)])), - ], - announced_routes: vec![prefix("203.0.113.0/24").into()], - })) + peer.announce( + announcement(&["203.0.113.0/24"], &[PEER_ASN, 65002]) + .with_next_hop(Ipv4Addr::new(10, 0, 0, 1)) + .with_communities(&[(65001, 7)]), + ) .await?; let received = tokio::time::timeout(TIMEOUT, async { diff --git a/crates/integration-tests/tests/import_policy.rs b/crates/integration-tests/tests/import_policy.rs index 0b357dd..a4c939a 100644 --- a/crates/integration-tests/tests/import_policy.rs +++ b/crates/integration-tests/tests/import_policy.rs @@ -5,100 +5,51 @@ use std::collections::BTreeSet; use std::net::Ipv4Addr; use std::time::Duration; -use bgp_integration_tests::default_policy; +use bgp_integration_tests::announcement::announcement; use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::test_server::{ServerWithPeer, TestServer}; +use bgp_integration_tests::{default_policy, prefix, wait_for, wait_until}; use bgp_packet::errors::{BgpError, CeaseSubcode}; use bgp_packet::message::{ - AsPathAttribute, CommunitiesPathAttribute, LargeCommunitiesPathAttribute, LargeCommunity, - Message, NextHopPathAttribute, OriginPathAttribute, PathAttribute, UpdateMessage, + CommunitiesPathAttribute, LargeCommunitiesPathAttribute, LargeCommunity, PathAttribute, }; -use bgp_primitives::ip_prefix::IpPrefix; -use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; use bgp_server::marks::{Marks, Patterns, Scrub}; use bgp_server::policy::{ImportPolicy, ImportRule, PrefixSet}; use bgp_server::session::DialState; use eyre::Result; -use tokio::net::TcpListener; /// AS numbers as they are on the Internet: the checks refuse the private /// ones the other tests use. const LOCAL_ASN: u32 = 1299; const PEER_ASN: u32 = 3333; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} +const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 2); /// A server with one configured peer, at the address the test connects -/// from, whose routes are judged by `import`. Returns the listener that -/// keeps the server's own dialling occupied. -async fn server_with(import: ImportPolicy) -> Result<(TestServer, TcpListener)> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let endpoint = listener.local_addr()?; - let server = TestServer::start_with(LOCAL_ASN, |c| { - c.accept_unconfigured_peers = false; +/// from, whose routes are judged by `import`. The listener that keeps the +/// server's own dialling occupied comes with it. +async fn server_with(import: ImportPolicy) -> Result { + TestServer::with_peer(LOCAL_ASN, PEER_ASN, |c, peer| { c.connect_retry = Duration::from_millis(50); - c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import, - role: None, - require_role: false, - export: Default::default(), - }]; + peer.import = import; }) - .await?; - Ok((server, listener)) + .await } async fn established(server: &TestServer) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; - Ok(peer) + server.established(PEER_ASN, PEER_ID).await } /// Announces IPv4 `prefixes` with the AS path `path`. async fn announce(peer: &mut TestPeer, prefixes: &[&str], path: &[u32]) -> Result<()> { - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(path.to_vec()), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ], - announced_routes: prefixes.iter().map(|p| prefix(p).into()).collect(), - })) - .await + peer.announce(announcement(prefixes, path)).await } /// Waits until the server has dealt with `count` announced routes of the /// session's, taken or refused, and returns the prefixes in its table. async fn table_after(server: &TestServer, taken: u64, refused: u64) -> Result> { let counters = server.registry.list()[0].counters.clone(); - tokio::time::timeout(TIMEOUT, async { - while (counters.routes(), counters.rejected()) != (taken, refused) { - tokio::time::sleep(Duration::from_millis(5)).await; - } + wait_until("the routes are dealt with", || { + (counters.routes(), counters.rejected()) == (taken, refused) }) .await .map_err(|_| { @@ -126,7 +77,10 @@ fn set(prefixes: &[&str]) -> BTreeSet { /// through us. Everything a peer sent used to be taken. #[tokio::test] async fn test_routes_that_no_peer_should_announce_are_refused() -> Result<()> { - let (server, _listener) = server_with(default_policy()).await?; + let ServerWithPeer { + server, + listener: _listener, + } = server_with(default_policy()).await?; let mut peer = established(&server).await?; // One UPDATE with good and bad prefixes: the good ones are taken. @@ -164,7 +118,10 @@ async fn test_routes_that_no_peer_should_announce_are_refused() -> Result<()> { /// and the new one is not acceptable. #[tokio::test] async fn test_a_refused_route_replaces_the_one_that_was_taken() -> Result<()> { - let (server, _listener) = server_with(default_policy()).await?; + let ServerWithPeer { + server, + listener: _listener, + } = server_with(default_policy()).await?; let mut peer = established(&server).await?; announce(&mut peer, &["193.0.0.0/21", "193.0.16.0/24"], &[PEER_ASN]).await?; @@ -181,7 +138,10 @@ async fn test_a_refused_route_replaces_the_one_that_was_taken() -> Result<()> { async fn test_a_peer_is_held_to_its_prefix_list_and_origins() -> Result<()> { let mut prefixes = PrefixSet::default(); prefixes.allow(prefix("193.0.0.0/21"), 24); - let (server, _listener) = server_with(ImportPolicy { + let ServerWithPeer { + server, + listener: _listener, + } = server_with(ImportPolicy { prefixes: Some(prefixes), origin_asns: Some([PEER_ASN, 2121].into()), ..default_policy() @@ -211,7 +171,7 @@ async fn test_a_peer_is_held_to_its_prefix_list_and_origins() -> Result<()> { /// again. #[tokio::test] async fn test_a_peer_over_its_limit_is_cut_off_and_kept_down() -> Result<()> { - let (server, listener) = server_with(ImportPolicy { + let ServerWithPeer { server, listener } = server_with(ImportPolicy { max_prefixes: Some(3), hold_down: Duration::from_millis(1500), ..default_policy() @@ -219,7 +179,7 @@ async fn test_a_peer_over_its_limit_is_cut_off_and_kept_down() -> Result<()> { .await?; // The server dials the peer as well; that connection is left unanswered // until the end. - let dialled = tokio::time::timeout(TIMEOUT, listener.accept()).await??; + let dialled = TestPeer::accept(&listener).await?; let mut peer = established(&server).await?; // Refused routes do not count towards the limit, and replacing a @@ -234,58 +194,43 @@ async fn test_a_peer_over_its_limit_is_cut_off_and_kept_down() -> Result<()> { assert_eq!(table_after(&server, 3, 1).await?.len(), 3); announce(&mut peer, &["193.0.3.0/24"], &[PEER_ASN]).await?; - let notification = tokio::time::timeout(TIMEOUT, async { - loop { - if let Message::Notification(notification) = peer.recv().await? { - return Ok::<_, eyre::Report>(notification); - } - } - }) - .await??; + let notification = peer.expect_notification().await?; assert_eq!( notification.error, BgpError::Cease(CeaseSubcode::MaxPrefixes) ); drop(peer); - tokio::time::timeout(TIMEOUT, async { - while !server.received_routes().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } + wait_until("the peer's routes are gone", || { + server.received_routes().is_empty() }) .await?; // Held down: a new connection gets nothing. let mut early = TestPeer::connect(server.addr).await?; assert!( - matches!( - tokio::time::timeout(TIMEOUT, early.recv()).await, - Ok(Err(_)) - ), + early.closed_unanswered().await, "a peer that is held down is not answered" ); // Nor is it dialled: once the connection the server had open to it is // gone, the dialer waits out the hold-down and says so. drop(dialled); - let reason = tokio::time::timeout(TIMEOUT, async { - loop { - if let Some((_, DialState::Backoff { last_error, .. })) = - server.registry.configured_peers().into_iter().next() - && last_error.contains("held down") + let reason = wait_for("the dialer says the peer is held down", || { + match server.registry.configured_peers().into_iter().next() { + Some((_, DialState::Backoff { last_error, .. })) + if last_error.contains("held down") => { - return last_error; + Some(last_error) } - tokio::time::sleep(Duration::from_millis(10)).await; + _ => None, } }) .await?; assert!(reason.contains("limit of routes"), "{reason}"); // When the time is up the peer is dialled again, and taken back. - tokio::time::timeout(TIMEOUT, listener.accept()).await??; + TestPeer::accept(&listener).await?; let mut later = TestPeer::connect(server.addr).await?; - later - .handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; + later.handshake(PEER_ASN, PEER_ID, 90).await?; Ok(()) } @@ -296,7 +241,10 @@ async fn test_a_peer_over_its_limit_is_cut_off_and_kept_down() -> Result<()> { async fn test_a_peers_routes_are_marked_and_ranked_as_configured() -> Result<()> { use bgp_packet::message::{CommunitiesPathAttribute, LocalPrefPathAttribute}; use bgp_server::marks::Marks; - let (server, _listener) = server_with(ImportPolicy { + let ServerWithPeer { + server, + listener: _listener, + } = server_with(ImportPolicy { marks: Marks::parse( &["1299:*".to_string()], &["1299:1001".to_string()], @@ -308,16 +256,10 @@ async fn test_a_peers_routes_are_marked_and_ranked_as_configured() -> Result<()> }) .await?; let mut peer = established(&server).await?; - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - PathAttribute::Communities(CommunitiesPathAttribute(vec![(3333, 5), (1299, 2002)])), - ], - announced_routes: vec![prefix("193.0.0.0/21").into(), prefix("193.0.8.0/22").into()], - })) + peer.announce( + announcement(&["193.0.0.0/21", "193.0.8.0/22"], &[PEER_ASN]) + .with_communities(&[(3333, 5), (1299, 2002)]), + ) .await?; table_after(&server, 2, 0).await?; let routes = server.received_routes(); @@ -388,7 +330,10 @@ async fn test_our_communities_are_not_taken_from_other_networks() -> Result<()> }), ..default_policy() }; - let (server, _listener) = server_with(scrubbed).await?; + let ServerWithPeer { + server, + listener: _listener, + } = server_with(scrubbed).await?; let mut peer = established(&server).await?; announce_with(&mut peer, &["193.0.0.0/21"], ours_and_theirs()).await?; table_after(&server, 1, 0).await?; @@ -402,7 +347,10 @@ async fn test_our_communities_are_not_taken_from_other_networks() -> Result<()> ] ); - let (server, _listener) = server_with(default_policy()).await?; + let ServerWithPeer { + server, + listener: _listener, + } = server_with(default_policy()).await?; let mut peer = established(&server).await?; announce_with(&mut peer, &["193.0.0.0/21"], ours_and_theirs()).await?; table_after(&server, 1, 0).await?; @@ -424,18 +372,8 @@ async fn announce_with( prefixes: &[&str], more: Vec, ) -> Result<()> { - let mut path_attributes = vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ]; - path_attributes.extend(more); - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes, - announced_routes: prefixes.iter().map(|p| prefix(p).into()).collect(), - })) - .await + peer.announce(announcement(prefixes, &[PEER_ASN]).with_attributes(more)) + .await } /// Rules do to some routes what the rest of the policy does to all: by @@ -465,7 +403,10 @@ async fn test_rules_apply_to_the_routes_they_hold_for() -> Result<()> { ], ..default_policy() }; - let (server, _listener) = server_with(policy).await?; + let ServerWithPeer { + server, + listener: _listener, + } = server_with(policy).await?; let mut peer = established(&server).await?; announce_with( &mut peer, diff --git a/crates/integration-tests/tests/interop.rs b/crates/integration-tests/tests/interop.rs index 4f71ab5..7aa6b9a 100644 --- a/crates/integration-tests/tests/interop.rs +++ b/crates/integration-tests/tests/interop.rs @@ -12,8 +12,6 @@ use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use eyre::Result; -const TIMEOUT: Duration = Duration::from_secs(5); - /// The OPEN a BIRD 2 router sends for an IPv6-only session from AS210036: /// one optional parameter carrying MP(IPv6 unicast), route refresh, /// extended message, graceful restart (aware only), 4-octet AS, enhanced @@ -77,19 +75,13 @@ async fn test_bird_open_and_end_of_rib_are_accepted() -> Result<()> { peer.send_raw(BIRD_OPEN).await?; peer.recv_open().await?; - match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { + match peer.recv().await? { Message::KeepAlive => {} other => eyre::bail!("server did not accept BIRD's OPEN: {other:?}"), } peer.send(&Message::KeepAlive).await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; - let session = server.registry.list().remove(0); + let session = server.wait_for_session().await?; assert_eq!( session.peer.asn, 210036, "4-octet ASN must come from the capability, not AS_TRANS" diff --git a/crates/integration-tests/tests/kernel.rs b/crates/integration-tests/tests/kernel.rs index 80f994d..906c2e4 100644 --- a/crates/integration-tests/tests/kernel.rs +++ b/crates/integration-tests/tests/kernel.rs @@ -12,16 +12,14 @@ use std::process::Command; use std::time::{Duration, Instant}; use bgp_fib::FibConfig; +use bgp_integration_tests::announcement::{announcement, withdrawal}; +use bgp_integration_tests::prefix; use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::TestServer; -use bgp_packet::message::{ - AsPathAttribute, Message, NextHopPathAttribute, OriginPathAttribute, PathAttribute, - UpdateMessage, -}; -use bgp_primitives::ip_prefix::IpPrefix; use eyre::Result; use serde_json::Value; +/// The kernel is given more time than the loopback tests allow. const TIMEOUT: Duration = Duration::from_secs(10); const TABLE: u32 = 1100; const INTERFACE: &str = "ferrix100"; @@ -87,19 +85,8 @@ impl Drop for Interface { } async fn announce(peer: &mut TestPeer, prefixes: &[&str], next_hop: Ipv4Addr) -> Result<()> { - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![65001]), - PathAttribute::NextHop(NextHopPathAttribute(next_hop)), - ], - announced_routes: prefixes - .iter() - .map(|p| IpPrefix::try_from(*p).unwrap().into()) - .collect(), - })) - .await + peer.announce(announcement(prefixes, &[65001]).with_next_hop(next_hop)) + .await } /// A peer's routes go from the session to the kernel, and out of it again @@ -174,12 +161,8 @@ async fn test_routes_from_a_session_reach_the_kernel() -> Result<()> { ); // One is withdrawn. - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![IpPrefix::try_from("198.18.0.0/15").unwrap().into()], - path_attributes: vec![], - announced_routes: vec![], - })) - .await?; + peer.announce(withdrawal([prefix("198.18.0.0/15").into()])) + .await?; kernel_becomes(&["203.0.113.0/24 via 10.100.0.2 dev ferrix100 proto bgp metric 20"]); // The session ends: packets must not go on following its routes. diff --git a/crates/integration-tests/tests/negotiation.rs b/crates/integration-tests/tests/negotiation.rs index 6f61a39..af4f565 100644 --- a/crates/integration-tests/tests/negotiation.rs +++ b/crates/integration-tests/tests/negotiation.rs @@ -4,16 +4,16 @@ use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr}; use std::time::Duration; +use bgp_integration_tests::announcement::announcement; use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::test_server::{ReceivedRoute, TestServer}; +use bgp_integration_tests::{peer_config, prefix, wait_for}; use bgp_packet::errors::{BgpError, OpenMsgSubcode}; use bgp_packet::message::{ - AsPathAttribute, Message, MpReachNlriPathAttribute, NextHopPathAttribute, NlriNextHop, - OriginPathAttribute, PathAttribute, UpdateMessage, + AsPathAttribute, Message, MpReachNlriPathAttribute, NlriNextHop, PathAttribute, UpdateMessage, }; use bgp_packet::open::Capability; use bgp_primitives::constants::{Afi, AfiSafi, Safi}; -use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::config::PeerConfig; use bgp_server::family::{FamilySet, SUPPORTED}; @@ -23,11 +23,6 @@ use eyre::{Result, bail}; const LOCAL_ASN: u32 = 65000; const PEER_ASN: u32 = 65001; const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 1); -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} fn only(family: AfiSafi) -> FamilySet { [family].into_iter().collect() @@ -37,35 +32,26 @@ fn four_octet() -> Capability { Capability::FourByteAsn { asn: PEER_ASN } } -async fn wait_for_session(server: &TestServer) -> Result { - Ok(tokio::time::timeout(TIMEOUT, async { - loop { - if let Some(session) = server.registry.list().into_iter().next() { - return session; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } +/// Waits until the server has a route, and returns what it has. +async fn some_routes(server: &TestServer) -> Result> { + wait_for("a route is in the table", || { + let routes = server.received_routes(); + (!routes.is_empty()).then_some(routes) }) - .await?) + .await } /// An UPDATE announcing one IPv4 route in the body and one IPv6 route in /// MP_REACH_NLRI. fn dual_stack_update() -> UpdateMessage { - UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - PathAttribute::MpReachNlri(MpReachNlriPathAttribute { - family: AfiSafi::IPV6_UNICAST, - next_hop: NlriNextHop::Ipv6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1)), - prefixes: vec![prefix("2001:db8:1::/48").into()], - }), - ], - announced_routes: vec![prefix("192.0.2.0/24").into()], - } + let ipv6 = PathAttribute::MpReachNlri(MpReachNlriPathAttribute { + family: AfiSafi::IPV6_UNICAST, + next_hop: NlriNextHop::Ipv6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1)), + prefixes: vec![prefix("2001:db8:1::/48").into()], + }); + announcement(&["192.0.2.0/24"], &[PEER_ASN]) + .with_attributes([ipv6]) + .into() } /// The session carries the families both OPENs offered, and the server's @@ -93,7 +79,7 @@ async fn test_families_are_the_ones_both_sides_offered() -> Result<()> { "the server's OPEN offers {family}" ); } - let session = wait_for_session(&server).await?; + let session = server.wait_for_session().await?; assert_eq!(session.families, only(AfiSafi::IPV4_UNICAST)); Ok(()) } @@ -114,20 +100,11 @@ async fn test_routes_of_a_family_not_negotiated_are_not_accepted() -> Result<()> ], ) .await?; - wait_for_session(&server).await?; + server.wait_for_session().await?; - peer.send(&Message::Update(dual_stack_update())).await?; + peer.announce(dual_stack_update()).await?; - let routes = tokio::time::timeout(TIMEOUT, async { - loop { - let routes = server.received_routes(); - if !routes.is_empty() { - return routes; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; + let routes = some_routes(&server).await?; // One UPDATE is applied as a whole, so the IPv6 route is not on its way. assert_eq!(routes.len(), 1); assert_eq!(routes[0].prefix, prefix("192.0.2.0/24")); @@ -159,7 +136,7 @@ async fn test_routes_of_a_family_not_negotiated_are_not_sent() -> Result<()> { ], ) .await?; - let session = wait_for_session(&server).await?; + let session = server.wait_for_session().await?; assert_eq!(session.families, only(AfiSafi::IPV6_UNICAST)); // A second peer that takes IPv4 is sent the route. The advertiser @@ -168,7 +145,7 @@ async fn test_routes_of_a_family_not_negotiated_are_not_sent() -> Result<()> { let mut dual = TestPeer::connect(server.addr).await?; dual.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) .await?; - let update = tokio::time::timeout(TIMEOUT, dual.recv_update()).await??; + let update = dual.recv_update().await?; assert_eq!(update.announced_routes, vec![prefix("10.100.0.0/16")]); let nothing = tokio::time::timeout(Duration::from_millis(200), v6_only.recv_update()).await; @@ -182,23 +159,11 @@ async fn test_routes_of_a_family_not_negotiated_are_not_sent() -> Result<()> { async fn test_no_family_in_common_refuses_the_session() -> Result<()> { let server = TestServer::start_with(LOCAL_ASN, |c| { // Inbound connections from this address inherit the peer's settings. + let at = SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 1); c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "v6-only".into(), - endpoint: SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 1).into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, families: only(AfiSafi::IPV6_UNICAST), - add_path: None, import: ImportPolicy::permissive(), - role: None, - require_role: false, - export: Default::default(), + ..peer_config("v6-only", at, PEER_ASN) }]; // Keep the dialer out of the way. c.connect_retry = Duration::from_secs(3600); @@ -223,7 +188,7 @@ async fn test_no_family_in_common_refuses_the_session() -> Result<()> { .collect(); assert_eq!(offered, [&Capability::multiprotocol(AfiSafi::IPV6_UNICAST)]); - match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { + match peer.recv().await? { Message::Notification(n) => { assert_eq!( n.error, @@ -256,7 +221,7 @@ async fn test_a_peer_offering_an_unsupported_family_is_still_accepted() -> Resul ) .await?; - let session = wait_for_session(&server).await?; + let session = server.wait_for_session().await?; assert_eq!(session.families, only(AfiSafi::IPV4_UNICAST)); assert!( session.four_octet_asn, @@ -281,13 +246,13 @@ async fn test_a_peer_without_capabilities_gets_two_octet_as_paths() -> Result<() let mut peer = TestPeer::connect(server.addr).await?; peer.handshake_with(PEER_ASN, PEER_ID, 90, vec![]).await?; - let session = wait_for_session(&server).await?; + let session = server.wait_for_session().await?; assert!(!session.four_octet_asn); assert_eq!(session.families, only(AfiSafi::IPV4_UNICAST)); // The test peer now decodes AS_PATH two octets wide, as such a speaker // would; an UPDATE encoded four octets wide would not parse. - let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + let update = peer.recv_update().await?; assert_eq!(update.announced_routes, vec![prefix("10.100.0.0/16")]); assert!( update @@ -302,26 +267,9 @@ async fn test_a_peer_without_capabilities_gets_two_octet_as_paths() -> Result<() assert_eq!(as4_path, Some(vec![WIDE_ASN])); // And its own two-octet paths are read correctly. - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN, 64512]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ], - announced_routes: vec![prefix("192.0.2.0/24").into()], - })) - .await?; - let routes = tokio::time::timeout(TIMEOUT, async { - loop { - let routes = server.received_routes(); - if !routes.is_empty() { - return routes; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; + peer.announce(announcement(&["192.0.2.0/24"], &[PEER_ASN, 64512])) + .await?; + let routes = some_routes(&server).await?; assert!( routes[0] .attributes diff --git a/crates/integration-tests/tests/roles.rs b/crates/integration-tests/tests/roles.rs index 14bba9e..497fd9a 100644 --- a/crates/integration-tests/tests/roles.rs +++ b/crates/integration-tests/tests/roles.rs @@ -3,74 +3,36 @@ //! marked as only to go to customers. use std::net::{Ipv4Addr, Ipv6Addr}; -use std::time::Duration; -use bgp_integration_tests::default_policy; +use bgp_integration_tests::announcement::announcement; use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::test_server::{ServerWithPeer, TestServer}; +use bgp_integration_tests::{prefix, wait_until}; use bgp_packet::errors::{BgpError, OpenMsgSubcode}; use bgp_packet::message::{ - AsPathAttribute, Message, MpReachNlriPathAttribute, NextHopPathAttribute, NlriNextHop, - OriginPathAttribute, PathAttribute, UpdateMessage, + AsPathAttribute, Message, MpReachNlriPathAttribute, NlriNextHop, OriginPathAttribute, + PathAttribute, UpdateMessage, }; use bgp_packet::open::{BgpRole, Capability}; use bgp_primitives::constants::AfiSafi; -use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::LocalRoute; -use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; use bgp_server::policy::PeerRole; use eyre::{Result, bail}; use serde_json::Value; -use tokio::net::TcpListener; const LOCAL_ASN: u32 = 1299; const PEER_ASN: u32 = 3333; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} - -/// What a test holds on to: the server, and the listener its dialling -/// goes to. -struct Setup { - server: TestServer, - _listener: TcpListener, -} +const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 2); /// A server with one configured peer at the address the test connects /// from, which is `role` to it, and which originates 193.0.0.0/21. -async fn server_with(role: Option, require_role: bool) -> Result { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let endpoint = listener.local_addr()?; - let server = TestServer::start_with(LOCAL_ASN, |c| { - c.accept_unconfigured_peers = false; +async fn server_with(role: Option, require_role: bool) -> Result { + TestServer::with_peer(LOCAL_ASN, PEER_ASN, |c, peer| { c.originate = vec![LocalRoute::new(prefix("193.0.0.0/21"), Default::default())]; - c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import: default_policy(), - role, - require_role, - export: Default::default(), - }]; - }) - .await?; - Ok(Setup { - server, - _listener: listener, + peer.role = role; + peer.require_role = require_role; }) + .await } /// The capabilities of an ordinary peer, and with `role` what it says it @@ -88,16 +50,9 @@ fn capabilities(role: Option) -> Vec { /// A session with the server, on which the peer says it is `says`. async fn established(server: &TestServer, says: Option) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake_with(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90, capabilities(says)) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; - Ok(peer) + server + .established_with(PEER_ASN, PEER_ID, capabilities(says)) + .await } /// Announces `prefixes` with the AS path `path` and, with `marked_by`, an @@ -108,30 +63,17 @@ async fn announce( path: &[u32], marked_by: Option, ) -> Result<()> { - let mut path_attributes = vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(path.to_vec()), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ]; - path_attributes.extend(marked_by.map(PathAttribute::OnlyToCustomer)); - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes, - announced_routes: prefixes.iter().map(|p| prefix(p).into()).collect(), - })) - .await + let mut announcement = announcement(prefixes, path); + if let Some(asn) = marked_by { + announcement = announcement.with_only_to_customer(asn); + } + peer.announce(announcement).await } /// Waits until the table holds `count` routes, and returns the mark each /// has, by prefix. async fn marks(server: &TestServer, count: usize) -> Result)>> { - tokio::time::timeout(TIMEOUT, async { - while server.received_routes().len() != count { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await - .map_err(|_| { + server.wait_for_routes(count).await.map_err(|_| { eyre::eyre!( "expected {count} routes, the table has {:?}", server.received_routes() @@ -167,26 +109,16 @@ fn announced_role(open: &bgp_packet::open::OpenMessage) -> Option { /// shown as one whose peer confirmed its role. #[tokio::test] async fn test_roles_that_fit_together_are_confirmed() -> Result<()> { - let Setup { server, .. } = server_with(Some(PeerRole::Customer), false).await?; + let ServerWithPeer { server, .. } = server_with(Some(PeerRole::Customer), false).await?; let (base, _http) = server.serve_http().await?; let mut peer = TestPeer::connect(server.addr).await?; let open = peer - .handshake_with( - PEER_ASN, - Ipv4Addr::new(10, 0, 0, 2), - 90, - capabilities(Some(BgpRole::Customer)), - ) + .handshake_with(PEER_ASN, PEER_ID, 90, capabilities(Some(BgpRole::Customer))) .await?; // The peer is our customer: we are its provider. assert_eq!(announced_role(&open), Some(BgpRole::Provider)); - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; + server.wait_for_session().await?; let peers: Value = reqwest::get(format!("{base}/api/peers")) .await? .json() @@ -201,18 +133,13 @@ async fn test_roles_that_fit_together_are_confirmed() -> Result<()> { /// configured as is refused, with the error RFC 9234 gives for it. #[tokio::test] async fn test_a_peer_that_says_it_is_something_else_is_refused() -> Result<()> { - let Setup { server, .. } = server_with(Some(PeerRole::Customer), false).await?; + let ServerWithPeer { server, .. } = server_with(Some(PeerRole::Customer), false).await?; let mut peer = TestPeer::connect(server.addr).await?; // It thinks it is our peer; we have it down as a customer. - peer.send_open_with( - PEER_ASN, - Ipv4Addr::new(10, 0, 0, 2), - 90, - capabilities(Some(BgpRole::Peer)), - ) - .await?; + peer.send_open_with(PEER_ASN, PEER_ID, 90, capabilities(Some(BgpRole::Peer))) + .await?; peer.recv_open().await?; - match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { + match peer.recv().await? { Message::Notification(n) => { assert_eq!(n.error, BgpError::OpenMsg(OpenMsgSubcode::RoleMismatch)); // Subcode 11 on the wire. @@ -228,7 +155,7 @@ async fn test_a_peer_that_says_it_is_something_else_is_refused() -> Result<()> { /// our word for what it is, unless the configuration requires it to say. #[tokio::test] async fn test_a_peer_that_does_not_say_is_refused_only_where_required() -> Result<()> { - let Setup { server, .. } = server_with(Some(PeerRole::Customer), false).await?; + let ServerWithPeer { server, .. } = server_with(Some(PeerRole::Customer), false).await?; let (base, _http) = server.serve_http().await?; let _peer = established(&server, None).await?; let peers: Value = reqwest::get(format!("{base}/api/peers")) @@ -238,12 +165,12 @@ async fn test_a_peer_that_does_not_say_is_refused_only_where_required() -> Resul assert_eq!(peers[0]["role"], "customer"); assert_eq!(peers[0]["role_confirmed"], false); - let Setup { server, .. } = server_with(Some(PeerRole::Customer), true).await?; + let ServerWithPeer { server, .. } = server_with(Some(PeerRole::Customer), true).await?; let mut peer = TestPeer::connect(server.addr).await?; - peer.send_open_with(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90, capabilities(None)) + peer.send_open_with(PEER_ASN, PEER_ID, 90, capabilities(None)) .await?; peer.recv_open().await?; - match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { + match peer.recv().await? { Message::Notification(n) => { assert_eq!(n.error, BgpError::OpenMsg(OpenMsgSubcode::RoleMismatch)); } @@ -257,7 +184,7 @@ async fn test_a_peer_that_does_not_say_is_refused_only_where_required() -> Resul /// whatever else the peer's policy takes. #[tokio::test] async fn test_a_marked_route_from_a_customer_is_a_leak() -> Result<()> { - let Setup { server, .. } = server_with(Some(PeerRole::Customer), false).await?; + let ServerWithPeer { server, .. } = server_with(Some(PeerRole::Customer), false).await?; let mut peer = established(&server, Some(BgpRole::Customer)).await?; // The customer's own route, and one its other provider gave it. @@ -275,12 +202,7 @@ async fn test_a_marked_route_from_a_customer_is_a_leak() -> Result<()> { marks(&server, 1).await?, [("194.0.0.0/24".to_string(), None)] ); - tokio::time::timeout(TIMEOUT, async { - while server.refusals.peers().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; + wait_until("a route is refused", || !server.refusals.peers().is_empty()).await?; let refused = &server.refusals.peers()[0].1; assert_eq!(refused.total(), 1); assert_eq!(refused.recent()[0].prefix, prefix("8.8.8.0/24")); @@ -296,7 +218,7 @@ async fn test_a_marked_route_from_a_customer_is_a_leak() -> Result<()> { /// the route from above or from another peer. #[tokio::test] async fn test_a_route_marked_by_someone_else_is_a_leak_from_a_peer() -> Result<()> { - let Setup { server, .. } = server_with(Some(PeerRole::Peer), false).await?; + let ServerWithPeer { server, .. } = server_with(Some(PeerRole::Peer), false).await?; let mut peer = established(&server, Some(BgpRole::Peer)).await?; announce(&mut peer, &["194.0.0.0/24"], &[PEER_ASN], Some(PEER_ASN)).await?; @@ -326,14 +248,13 @@ async fn test_a_route_marked_by_someone_else_is_a_leak_from_a_peer() -> Result<( #[tokio::test] async fn test_routes_from_a_provider_are_marked_on_the_way_in() -> Result<()> { for four_octet in [true, false] { - let Setup { server, .. } = server_with(Some(PeerRole::Provider), false).await?; + let ServerWithPeer { server, .. } = server_with(Some(PeerRole::Provider), false).await?; let mut peer = TestPeer::connect(server.addr).await?; let mut offered = capabilities(Some(BgpRole::Provider)); if !four_octet { offered.retain(|capability| !matches!(capability, Capability::FourByteAsn { .. })); } - peer.handshake_with(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90, offered) - .await?; + peer.handshake_with(PEER_ASN, PEER_ID, 90, offered).await?; announce(&mut peer, &["194.0.0.0/24"], &[PEER_ASN, 2121], None).await?; // One that someone further up has marked already keeps that mark. @@ -387,9 +308,9 @@ async fn test_our_routes_are_marked_on_the_way_down_or_across() -> Result<()> { (PeerRole::Provider, BgpRole::Provider, false), (PeerRole::RouteServer, BgpRole::RouteServer, false), ] { - let Setup { server, .. } = server_with(Some(peer_is), false).await?; + let ServerWithPeer { server, .. } = server_with(Some(peer_is), false).await?; let mut peer = established(&server, Some(says)).await?; - let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + let update = peer.recv_update().await?; assert_eq!(update.announced_routes, [prefix("193.0.0.0/21")]); let mark = update .path_attributes @@ -401,9 +322,9 @@ async fn test_our_routes_are_marked_on_the_way_down_or_across() -> Result<()> { assert_eq!(mark, marked.then_some(LOCAL_ASN), "to our {peer_is:?}"); } // Nothing is said to a peer whose role is not known. - let Setup { server, .. } = server_with(None, false).await?; + let ServerWithPeer { server, .. } = server_with(None, false).await?; let mut peer = established(&server, None).await?; - let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + let update = peer.recv_update().await?; assert!( !update .path_attributes diff --git a/crates/integration-tests/tests/rpki.rs b/crates/integration-tests/tests/rpki.rs index 33db5d2..4ef188d 100644 --- a/crates/integration-tests/tests/rpki.rs +++ b/crates/integration-tests/tests/rpki.rs @@ -2,34 +2,23 @@ //! cache, a peer that announces routes, and the validation data changing //! under both. -use std::collections::BTreeSet; use std::net::Ipv4Addr; use std::time::Duration; -use bgp_integration_tests::default_policy; +use bgp_integration_tests::announcement::{announcement, withdrawal}; use bgp_integration_tests::test_cache::TestCache; use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; -use bgp_packet::message::{ - AsPathAttribute, Message, NextHopPathAttribute, OriginPathAttribute, PathAttribute, - UpdateMessage, -}; -use bgp_primitives::ip_prefix::IpPrefix; +use bgp_integration_tests::test_server::{ServerWithPeer, TestServer}; +use bgp_integration_tests::{default_policy, prefix, wait_until}; +use bgp_packet::message::{AsPathAttribute, PathAttribute}; use bgp_rpki::{CacheState, RpkiConfig, Vrp}; -use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; use bgp_server::policy::ImportPolicy; use eyre::Result; use serde_json::Value; -use tokio::net::TcpListener; const LOCAL_ASN: u32 = 1299; const PEER_ASN: u32 = 3333; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} +const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 2); fn vrp(p: &str, max_length: u8, asn: u32) -> Vrp { Vrp { @@ -39,13 +28,6 @@ fn vrp(p: &str, max_length: u8, asn: u32) -> Vrp { } } -/// What a test holds on to: the server, and the listener its dialling -/// goes to. -struct Setup { - server: TestServer, - _listener: TcpListener, -} - /// A server that validates against `cache`, with one configured peer at /// the address the test connects from. Returns once the server has the /// cache's data. @@ -53,12 +35,9 @@ async fn server_with( cache: &TestCache, expire: Option, import: ImportPolicy, -) -> Result { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let endpoint = listener.local_addr()?; +) -> Result { let cache = cache.address(); - let server = TestServer::start_with(LOCAL_ASN, |c| { - c.accept_unconfigured_peers = false; + let setup = TestServer::with_peer(LOCAL_ASN, PEER_ASN, |c, peer| { c.rpki = Some(RpkiConfig { caches: vec![cache], // Data never expires sooner than a refresh and a retry take. @@ -66,50 +45,19 @@ async fn server_with( retry: Some(Duration::from_millis(100)), expire, }); - c.peers = vec![PeerConfig { - graceful_restart: Default::default(), - groups: Vec::new(), - export_limits: None, - passive: false, - reflector_client: false, - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import, - role: None, - require_role: false, - export: Default::default(), - }]; + peer.import = import; }) .await?; - let rpki = server.rpki.clone().expect("validation is configured"); - tokio::time::timeout(TIMEOUT, async { - while rpki.status().state != CacheState::Synchronised { - tokio::time::sleep(Duration::from_millis(5)).await; - } + let rpki = setup.server.rpki.clone().expect("validation is configured"); + wait_until("the server has the cache's data", || { + rpki.status().state == CacheState::Synchronised }) .await?; - Ok(Setup { - server, - _listener: listener, - }) + Ok(setup) } async fn established(server: &TestServer) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; - Ok(peer) + server.established(PEER_ASN, PEER_ID).await } /// Announces `prefixes` as originated by `origin`, reached through the @@ -120,53 +68,7 @@ async fn announce(peer: &mut TestPeer, prefixes: &[&str], origin: u32) -> Result } else { vec![PEER_ASN, origin] }; - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(path), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ], - announced_routes: prefixes.iter().map(|p| prefix(p).into()).collect(), - })) - .await -} - -fn set(prefixes: &[&str]) -> BTreeSet { - prefixes.iter().map(|p| p.to_string()).collect() -} - -/// Waits until the table holds exactly `expected` and the peer has `held` -/// routes held aside. -async fn table_becomes(server: &TestServer, expected: &[&str], held: u64) -> Result<()> { - let expected = set(expected); - let table = || -> BTreeSet { - server - .received_routes() - .iter() - .map(|route| route.prefix.to_string()) - .collect() - }; - let held_now = || { - server - .registry - .list() - .first() - .map_or(0, |session| session.counters.held()) - }; - tokio::time::timeout(TIMEOUT, async { - while table() != expected || held_now() != held { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await - .map_err(|_| { - eyre::eyre!( - "the table has {:?} with {} held; expected {expected:?} with {held} held", - table(), - held_now() - ) - }) + peer.announce(announcement(prefixes, &path)).await } /// A route whose origin is not the one the holder of the addresses @@ -175,7 +77,7 @@ async fn table_becomes(server: &TestServer, expected: &[&str], held: u64) -> Res #[tokio::test] async fn test_an_invalid_route_is_refused_and_the_rest_are_taken() -> Result<()> { let cache = TestCache::start(vec![vrp("193.0.0.0/21", 24, PEER_ASN)]).await; - let Setup { server, .. } = server_with(&cache, None, default_policy()).await?; + let ServerWithPeer { server, .. } = server_with(&cache, None, default_policy()).await?; let (base, _http) = server.serve_http().await?; let mut peer = established(&server).await?; @@ -184,7 +86,9 @@ async fn test_an_invalid_route_is_refused_and_the_rest_are_taken() -> Result<()> // AS, more specific than it may announce. announce(&mut peer, &["193.0.4.0/24"], 15169).await?; announce(&mut peer, &["193.0.6.0/25"], PEER_ASN).await?; - table_becomes(&server, &["193.0.0.0/21", "194.0.0.0/24"], 1).await?; + server + .table_becomes(&["193.0.0.0/21", "194.0.0.0/24"], 1) + .await?; let client = reqwest::Client::new(); let get = |path: &str| { @@ -230,18 +134,18 @@ async fn test_an_invalid_route_is_refused_and_the_rest_are_taken() -> Result<()> async fn test_routes_follow_the_validation_data() -> Result<()> { let ripe = vrp("193.0.0.0/21", 24, PEER_ASN); let cache = TestCache::start(vec![ripe]).await; - let Setup { server, .. } = server_with(&cache, None, default_policy()).await?; + let ServerWithPeer { server, .. } = server_with(&cache, None, default_policy()).await?; let mut peer = established(&server).await?; // Nothing is known about 8.8.8.0/24, so it is taken. 193.0.4.0/24 is // covered, and not for this origin. announce(&mut peer, &["8.8.8.0/24", "193.0.4.0/24"], 15169).await?; - table_becomes(&server, &["8.8.8.0/24"], 1).await?; + server.table_becomes(&["8.8.8.0/24"], 1).await?; // The holder of 8.8.8.0/24 authorises another AS: the route in the // table is now invalid, and goes. cache.set(vec![ripe, vrp("8.8.8.0/24", 24, 13335)]); - table_becomes(&server, &[], 2).await?; + server.table_becomes(&[], 2).await?; let refused = &server.refusals.peers()[0].1; assert_eq!(refused.total(), 2); assert_eq!(refused.recent()[0].prefix, prefix("8.8.8.0/24")); @@ -254,7 +158,9 @@ async fn test_routes_follow_the_validation_data() -> Result<()> { vrp("8.8.8.0/24", 24, 15169), vrp("193.0.4.0/24", 24, 15169), ]); - table_becomes(&server, &["8.8.8.0/24", "193.0.4.0/24"], 0).await?; + server + .table_becomes(&["8.8.8.0/24", "193.0.4.0/24"], 0) + .await?; for route in server.received_routes() { assert!( route @@ -273,27 +179,23 @@ async fn test_routes_follow_the_validation_data() -> Result<()> { async fn test_a_held_route_the_peer_withdrew_does_not_come_back() -> Result<()> { let ripe = vrp("193.0.0.0/21", 24, PEER_ASN); let cache = TestCache::start(vec![ripe]).await; - let Setup { server, .. } = server_with(&cache, None, default_policy()).await?; + let ServerWithPeer { server, .. } = server_with(&cache, None, default_policy()).await?; let mut peer = established(&server).await?; announce(&mut peer, &["193.0.4.0/24", "193.0.5.0/24"], 15169).await?; - table_becomes(&server, &[], 2).await?; + server.table_becomes(&[], 2).await?; // One is withdrawn, the other announced again with the right origin. - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![prefix("193.0.4.0/24").into()], - path_attributes: vec![], - announced_routes: vec![], - })) - .await?; + peer.announce(withdrawal([prefix("193.0.4.0/24").into()])) + .await?; announce(&mut peer, &["193.0.5.0/24"], PEER_ASN).await?; - table_becomes(&server, &["193.0.5.0/24"], 0).await?; + server.table_becomes(&["193.0.5.0/24"], 0).await?; // Now the old origin would be valid for both. Neither old route is // there to be brought back. cache.set(vec![ripe, vrp("193.0.4.0/23", 24, 15169)]); tokio::time::sleep(Duration::from_millis(500)).await; - table_becomes(&server, &["193.0.5.0/24"], 0).await?; + server.table_becomes(&["193.0.5.0/24"], 0).await?; assert!( server.received_routes()[0] .attributes @@ -312,12 +214,12 @@ async fn test_a_peer_can_be_exempt_from_validation() -> Result<()> { accept_rpki_invalid: true, ..default_policy() }; - let Setup { server, .. } = server_with(&cache, None, import).await?; + let ServerWithPeer { server, .. } = server_with(&cache, None, import).await?; let (base, _http) = server.serve_http().await?; let mut peer = established(&server).await?; announce(&mut peer, &["193.0.4.0/24"], 15169).await?; - table_becomes(&server, &["193.0.4.0/24"], 0).await?; + server.table_becomes(&["193.0.4.0/24"], 0).await?; let routes: Value = reqwest::get(format!("{base}/api/routes/received")) .await? .json() @@ -333,14 +235,14 @@ async fn test_a_peer_can_be_exempt_from_validation() -> Result<()> { #[tokio::test] async fn test_held_routes_are_taken_when_the_validation_data_expires() -> Result<()> { let cache = TestCache::start(vec![vrp("193.0.0.0/21", 24, PEER_ASN)]).await; - let Setup { server, .. } = + let ServerWithPeer { server, .. } = server_with(&cache, Some(Duration::from_millis(1500)), default_policy()).await?; let mut peer = established(&server).await?; announce(&mut peer, &["193.0.4.0/24"], 15169).await?; - table_becomes(&server, &[], 1).await?; + server.table_becomes(&[], 1).await?; drop(cache); - table_becomes(&server, &["193.0.4.0/24"], 0).await?; + server.table_becomes(&["193.0.4.0/24"], 0).await?; assert!(server.rpki.as_ref().unwrap().current().vrps.is_empty()); Ok(()) } @@ -356,37 +258,21 @@ async fn test_an_unused_as4_path_does_not_make_a_route_valid() -> Result<()> { use bgp_packet::message::AsPathSegment; const HOLDER: u32 = 15169; let cache = TestCache::start(vec![vrp("193.0.0.0/21", 24, HOLDER)]).await; - let Setup { server, .. } = server_with(&cache, None, default_policy()).await?; + let ServerWithPeer { server, .. } = server_with(&cache, None, default_policy()).await?; // No capabilities: AS numbers are two octets wide on this session. - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake_with(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90, vec![]) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; + let mut peer = server.established_with(PEER_ASN, PEER_ID, vec![]).await?; assert!(!server.registry.list()[0].four_octet_asn); - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes: vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - PathAttribute::As4Path(AsPathAttribute { - segments: vec![AsPathSegment { - ordered: true, - path: vec![3356, HOLDER], - }], - }), - ], - announced_routes: vec![prefix("193.0.4.0/24").into()], - })) - .await?; - table_becomes(&server, &[], 1).await?; + let as4_path = PathAttribute::As4Path(AsPathAttribute { + segments: vec![AsPathSegment { + ordered: true, + path: vec![3356, HOLDER], + }], + }); + peer.announce(announcement(&["193.0.4.0/24"], &[PEER_ASN]).with_attributes([as4_path])) + .await?; + server.table_becomes(&[], 1).await?; // The data changes, and every held route is looked at again. cache.set(vec![ @@ -394,14 +280,12 @@ async fn test_an_unused_as4_path_does_not_make_a_route_valid() -> Result<()> { vrp("198.51.100.0/24", 24, 64500), ]); let rpki = server.rpki.clone().unwrap(); - tokio::time::timeout(TIMEOUT, async { - while rpki.current().vrps.len() != 2 { - tokio::time::sleep(Duration::from_millis(5)).await; - } + wait_until("the server has the new data", || { + rpki.current().vrps.len() == 2 }) .await?; // Time enough for the look, and then some. tokio::time::sleep(Duration::from_millis(600)).await; - table_becomes(&server, &[], 1).await?; + server.table_becomes(&[], 1).await?; Ok(()) } diff --git a/crates/integration-tests/tests/transit.rs b/crates/integration-tests/tests/transit.rs index 487d186..5e0f82f 100644 --- a/crates/integration-tests/tests/transit.rs +++ b/crates/integration-tests/tests/transit.rs @@ -6,25 +6,21 @@ use std::net::Ipv4Addr; use std::sync::Arc; use std::time::Duration; +use bgp_integration_tests::announcement::{announcement, withdrawal}; use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::TestServer; +use bgp_integration_tests::{TIMEOUT, prefix}; use bgp_packet::message::{ - AsPathAttribute, CommunitiesPathAttribute, Message, NextHopPathAttribute, OriginPathAttribute, - PathAttribute, RouteRefreshMessage, UpdateMessage, + CommunitiesPathAttribute, Message, NextHopPathAttribute, PathAttribute, RouteRefreshMessage, + UpdateMessage, }; use bgp_primitives::constants::AfiSafi; -use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::marks::{Control, Marks}; use bgp_server::policy::ExportPolicy; use eyre::Result; const LOCAL_ASN: u32 = 64601; -const TIMEOUT: Duration = Duration::from_secs(5); - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} /// A server that passes on to every peer what it learned from the others. async fn transit(customize: impl FnOnce(&mut bgp_server::config::Config)) -> Result { @@ -40,17 +36,9 @@ async fn transit(customize: impl FnOnce(&mut bgp_server::config::Config)) -> Res /// The peer with AS `64700 + n`, its session up. async fn peer(server: &TestServer, n: u8) -> Result { - let before = server.registry.list().len(); - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(64700 + u32::from(n), Ipv4Addr::new(10, 0, 0, n), 90) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().len() != before + 1 { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; - Ok(peer) + server + .established(64700 + u32::from(n), Ipv4Addr::new(10, 0, 0, n)) + .await } async fn announce( @@ -59,31 +47,13 @@ async fn announce( path: &[u32], communities: &[(u16, u16)], ) -> Result<()> { - let mut path_attributes = vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(path.to_vec()), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ]; - if !communities.is_empty() { - path_attributes.push(PathAttribute::Communities(CommunitiesPathAttribute( - communities.to_vec(), - ))); - } - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes, - announced_routes: prefixes.iter().map(|p| prefix(p).into()).collect(), - })) - .await + peer.announce(announcement(prefixes, path).with_communities(communities)) + .await } async fn withdraw(peer: &mut TestPeer, prefixes: &[&str]) -> Result<()> { - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: prefixes.iter().map(|p| prefix(p).into()).collect(), - path_attributes: vec![], - announced_routes: vec![], - })) - .await + peer.announce(withdrawal(prefixes.iter().map(|p| prefix(p).into()))) + .await } /// What a peer has been told so far, as the prefixes it would now have @@ -180,7 +150,7 @@ async fn test_routes_from_one_peer_reach_the_others() -> Result<()> { // The next hop is ourselves: the address of our end of the session. announce(&mut one, &["10.4.0.0/16"], &[64701], &[]).await?; - let update = tokio::time::timeout(TIMEOUT, two.recv_update()).await??; + let update = two.recv_update().await?; assert!( update .path_attributes @@ -220,7 +190,7 @@ async fn test_a_peer_is_kept_at_the_best_route() -> Result<()> { // A better route for it comes: that is the one told now. announce(&mut near, &["10.1.0.0/16"], &[64701, 65001], &[]).await?; - let update = tokio::time::timeout(TIMEOUT, watcher.recv_update()).await??; + let update = watcher.recv_update().await?; told.take(&update); assert_eq!(told.routes["10.1.0.0/16"], [LOCAL_ASN, 64701, 65001]); assert!( @@ -230,7 +200,7 @@ async fn test_a_peer_is_kept_at_the_best_route() -> Result<()> { // And goes again: back to the other, with no gap. withdraw(&mut near, &["10.1.0.0/16"]).await?; - let update = tokio::time::timeout(TIMEOUT, watcher.recv_update()).await??; + let update = watcher.recv_update().await?; told.take(&update); assert_eq!(told.routes["10.1.0.0/16"], [LOCAL_ASN, 64702, 65000, 65001]); @@ -239,7 +209,7 @@ async fn test_a_peer_is_kept_at_the_best_route() -> Result<()> { // told of. let mut told_far = Told::default(); told_becomes(&mut far, &mut told_far, &["10.1.0.0/16"]).await?; - let gone = tokio::time::timeout(TIMEOUT, far.recv_update()).await??; + let gone = far.recv_update().await?; assert_eq!(gone.withdrawn_routes, vec![prefix("10.1.0.0/16")]); told_nothing(&mut far).await; @@ -251,7 +221,7 @@ async fn test_a_peer_is_kept_at_the_best_route() -> Result<()> { AfiSafi::IPV4_UNICAST, ))) .await?; - let again = tokio::time::timeout(TIMEOUT, late.recv_update()).await??; + let again = late.recv_update().await?; assert_eq!(again.announced_routes, vec![prefix("10.1.0.0/16")]); Ok(()) } @@ -349,7 +319,7 @@ async fn test_communities_say_who_is_told() -> Result<()> { &[(0, 64702), (3356, 3)], ) .await?; - let update = tokio::time::timeout(TIMEOUT, three.recv_update()).await??; + let update = three.recv_update().await?; assert!( update .path_attributes @@ -378,12 +348,7 @@ async fn test_nothing_learned_is_passed_on_unless_asked_for() -> Result<()> { let mut told = Told::default(); told_becomes(&mut two, &mut told, &["10.99.0.0/16"]).await?; announce(&mut one, &["10.1.0.0/16"], &[64701], &[]).await?; - tokio::time::timeout(TIMEOUT, async { - while server.rib.len() != 1 { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; + server.wait_for_routes(1).await?; told_nothing(&mut two).await; Ok(()) } diff --git a/crates/integration-tests/tests/update_errors.rs b/crates/integration-tests/tests/update_errors.rs index 46dfe99..c317123 100644 --- a/crates/integration-tests/tests/update_errors.rs +++ b/crates/integration-tests/tests/update_errors.rs @@ -2,19 +2,18 @@ //! cost the routes in the UPDATE, not the session. use std::net::Ipv4Addr; -use std::time::Duration; use bgp_integration_tests::test_peer::TestPeer; use bgp_integration_tests::test_server::{ReceivedRoute, TestServer}; +use bgp_integration_tests::{prefix, wait_for}; use bgp_packet::errors::{BgpError, UpdateMsgSubcode}; -use bgp_packet::message::{LocalPrefPathAttribute, Message, PathAttribute}; +use bgp_packet::message::{LocalPrefPathAttribute, PathAttribute}; use bgp_primitives::ip_prefix::IpPrefix; use bgp_server::metrics::UPDATE_ERRORS; -use eyre::{Result, bail}; +use eyre::Result; const LOCAL_ASN: u32 = 65000; const PEER_ASN: u32 = 65001; -const TIMEOUT: Duration = Duration::from_secs(5); const WELL_KNOWN: u8 = 0x40; const OPTIONAL_TRANSITIVE: u8 = 0xc0; @@ -58,31 +57,24 @@ fn update(attributes: &[Vec], nlri: &[u8]) -> Vec { const ROUTE_A: [u8; 3] = [16, 10, 1]; const ROUTE_B: [u8; 3] = [16, 10, 2]; -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} - async fn established(server: &TestServer, asn: u32) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(asn, Ipv4Addr::new(10, 0, 0, 1), 90).await?; - Ok(peer) + server.established(asn, Ipv4Addr::new(10, 0, 0, 1)).await } /// Waits until the server's Adj-RIB-In holds exactly `prefixes`. async fn wait_for_routes(server: &TestServer, prefixes: &[&str]) -> Result> { let mut expected: Vec = prefixes.iter().map(|p| prefix(p)).collect(); expected.sort_by_key(|p| p.to_string()); - Ok(tokio::time::timeout(TIMEOUT, async { - loop { - let mut routes = server.received_routes(); - routes.sort_by_key(|r| r.prefix.to_string()); - if routes.iter().map(|r| &r.prefix).eq(expected.iter()) { - return routes; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } + wait_for(&format!("the table holds {prefixes:?}"), || { + let mut routes = server.received_routes(); + routes.sort_by_key(|r| r.prefix.to_string()); + routes + .iter() + .map(|r| &r.prefix) + .eq(expected.iter()) + .then_some(routes) }) - .await?) + .await } /// The situation RFC 7606 exists for: a route is in the table, and then an @@ -205,24 +197,11 @@ async fn test_an_unreadable_prefix_still_resets_the_session() -> Result<()> { )) .await?; - loop { - match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { - Message::KeepAlive => {} - Message::Notification(n) => { - assert_eq!( - n.error, - BgpError::UpdateMsg(UpdateMsgSubcode::InvalidNetworkField) - ); - break; - } - other => bail!("expected NOTIFICATION, got {other:?}"), - } - } - tokio::time::timeout(TIMEOUT, async { - while !server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; + let notification = peer.expect_notification().await?; + assert_eq!( + notification.error, + BgpError::UpdateMsg(UpdateMsgSubcode::InvalidNetworkField) + ); + server.wait_for_sessions(0).await?; Ok(()) }