From 5c7bc2f6753b71b2fb0163ee0c49b1ab44ae07b7 Mon Sep 17 00:00:00 2001 From: Jasper den Hertog Date: Thu, 21 Dec 2023 12:10:35 +0100 Subject: [PATCH] NewType wrapper for human readable communities - remove `human_serde` feature - HumanReadableCommunity type with Serializer impl - regular derive(Serialize) on Community - generic communities() and all_communities() on Update --- Cargo.toml | 2 +- src/bgp/aspath.rs | 10 ++-- src/bgp/communities.rs | 92 +++++++++++++++++++++++++++---- src/bgp/message/update.rs | 60 +++++++++++--------- src/bgp/message/update_builder.rs | 6 +- 5 files changed, 125 insertions(+), 45 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 35a41e04..232dcd9e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,4 +35,4 @@ default = ["octseq"] bgp = ["bytes", "log", "octseq", "const-str"] bmp = ["bgp", "bytes", "chrono", "log", "octseq"] std = [] -human_serde = [] + diff --git a/src/bgp/aspath.rs b/src/bgp/aspath.rs index 9401fda5..95a7247d 100644 --- a/src/bgp/aspath.rs +++ b/src/bgp/aspath.rs @@ -15,10 +15,10 @@ use std::{error, fmt}; use crate::asn::{Asn, LargeAsnError}; -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] use serde::ser::SerializeSeq; -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] use serde::{Serialize, Serializer}; use octseq::builder::{infallible, EmptyBuilder, FromBuilder, OctetsBuilder}; @@ -29,7 +29,7 @@ use crate::util::parser::ParseError; pub type OwnedHop = Hop>; -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] pub trait SerializeForOperators: Serialize { fn serialize_for_operator( &self, @@ -420,7 +420,7 @@ impl fmt::Display for HopPath { } } -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] impl Serialize for HopPath { fn serialize(&self, serializer: S) -> Result where @@ -434,7 +434,7 @@ impl Serialize for HopPath { } } -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] impl SerializeForOperators for HopPath { fn serialize_for_operator( &self, diff --git a/src/bgp/communities.rs b/src/bgp/communities.rs index 462053c9..25c3b723 100644 --- a/src/bgp/communities.rs +++ b/src/bgp/communities.rs @@ -95,10 +95,10 @@ use std::str::FromStr; use crate::asn::{Asn, Asn16, ParseAsnError}; -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] use serde::{Serialize, Serializer}; -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] pub trait SerializeForOperators: Serialize { fn serialize_for_operator( &self, @@ -108,10 +108,11 @@ pub trait SerializeForOperators: Serialize { S: Serializer; } -//--- Community -------------------------------------------------------------- +//------------ Community ----------------------------------------------------- /// Standard and Extended/Large Communities variants. #[derive(Copy, Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, )] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] pub enum Community { Standard(StandardCommunity), Extended(ExtendedCommunity), @@ -228,14 +229,83 @@ impl Display for Community { } } -#[cfg(feature = "human_serde")] -impl Serialize for Community { +//------------ HumanReadableCommunity ---------------------------------------0 + +/// Wrapper around Community with a special Serde Serializer implementation +/// that produces better human-readable output and leaves out some details. +/// Example output JSON can be found in the Rotonda docs repo: +/// https://github.com/NLnetLabs/rotonda-doc/ +/// +/// the communities() and all_communities() on the Update struct will need to +/// have this type included when calling these methods, like so: +/// `my_update.communities::`. +#[derive(Copy, Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, )] +pub struct HumanReadableCommunity(pub Community); + +impl From for HumanReadableCommunity { + fn from(value: Community) -> Self { + HumanReadableCommunity(value) + } +} + +impl From<[u8; 4]> for HumanReadableCommunity { + fn from(raw: [u8; 4]) -> HumanReadableCommunity { + HumanReadableCommunity(Community::Standard(StandardCommunity(raw))) + } +} + +impl From for HumanReadableCommunity { + fn from(value: StandardCommunity) -> Self { + HumanReadableCommunity(Community::Standard(value)) + } +} + +impl From for HumanReadableCommunity { + fn from(wk: Wellknown) -> Self { + HumanReadableCommunity(Community::Standard(wk.into())) + } +} + +impl From for HumanReadableCommunity { + fn from(value: ExtendedCommunity) -> Self { + HumanReadableCommunity(Community::Extended(value)) + } +} + +impl From for HumanReadableCommunity { + fn from(value: LargeCommunity) -> Self { + HumanReadableCommunity(Community::Large(value)) + } +} + +impl From for HumanReadableCommunity { + fn from(value: Ipv6ExtendedCommunity) -> Self { + HumanReadableCommunity(Community::Ipv6Extended(value)) + } +} + +impl FromStr for HumanReadableCommunity { + type Err = ParseError; + + fn from_str(s: &str) -> Result { + Community::from_str(s).map(Self) + } +} + +impl Display for HumanReadableCommunity { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + Display::fmt(&self.0, f) + } +} + +#[cfg(feature = "serde")] +impl Serialize for HumanReadableCommunity { fn serialize(&self, serializer: S) -> Result where S: Serializer, { if serializer.is_human_readable() { - match &self { + match &self.0 { Community::Standard(c) => { c.serialize_for_operator(serializer) } @@ -538,7 +608,7 @@ impl Display for StandardCommunity { } -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] impl SerializeForOperators for StandardCommunity { fn serialize_for_operator( &self, @@ -1055,7 +1125,7 @@ impl Display for ExtendedCommunity { // Serialize -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] impl SerializeForOperators for ExtendedCommunity { fn serialize_for_operator( &self, @@ -1445,7 +1515,7 @@ impl Display for LargeCommunity { // Serialize -#[cfg(feature = "human_serde")] +#[cfg(feature = "serde")] impl SerializeForOperators for LargeCommunity { fn serialize_for_operator( &self, @@ -1513,8 +1583,8 @@ impl From for ParseError { } // Types used only by our own human_serialize feature to structure the -// serialized output differently than is done by the default Serializer. -#[cfg(feature = "human_serde")] +// serialized output differently than is done by derive(Serialize). +#[cfg(feature = "serde")] mod ser { #[derive(serde::Serialize)] #[serde(rename = "value")] diff --git a/src/bgp/message/update.rs b/src/bgp/message/update.rs index b8aa8dd0..5e5fa477 100644 --- a/src/bgp/message/update.rs +++ b/src/bgp/message/update.rs @@ -21,14 +21,14 @@ use crate::bgp::message::nlri::{self, }; use core::ops::Range; +use std::marker::PhantomData; use std::net::Ipv4Addr; use crate::util::parser::ParseError; use crate::bgp::communities::{ - Community, ExtendedCommunity, Ipv6ExtendedCommunity, - LargeCommunity + LargeCommunity, }; /// BGP UPDATE message, variant of the [`Message`] enum. @@ -747,8 +747,8 @@ impl UpdateMessage { //--- Communities -------------------------------------------------------- /// Returns an iterator over Standard Communities (RFC1997), if any. - pub fn communities(&self) - -> Result>>, ParseError> + pub fn communities>(&self) + -> Result, T>>, ParseError> { if let Some(WireformatPathAttribute::Communities(epa)) = self.path_attributes()?.get(PathAttributeType::Communities) { let mut p = epa.value_into_parser(); @@ -799,22 +799,26 @@ impl UpdateMessage { /// Returns an optional `Vec` containing all conventional, Extended and /// Large communities, if any, or None if none of the three appear in the /// path attributes of this message. - pub fn all_communities(&self) -> Result>, ParseError> { - let mut res = Vec::::new(); + pub fn all_communities(&self) -> Result>, ParseError> + where T: From<[u8; 4]> + + From + + From + + From { + let mut res: Vec = Vec::new(); if let Some(c) = self.communities()? { res.append(&mut c.collect::>()); } if let Some(c) = self.ext_communities()? { - res.append(&mut c.map(Community::Extended).collect::>()); + res.append(&mut c.map(|c| c.into()).collect::>()); } if let Some(c) = self.ipv6_ext_communities()? { res.append( - &mut c.map(Community::Ipv6Extended).collect::>() + &mut c.map(|c| c.into()).collect::>() ); } if let Some(c) = self.large_communities()? { - res.append(&mut c.map(Community::Large).collect::>()); + res.append(&mut c.map(|c| c.into()).collect::>()); } if res.is_empty() { @@ -823,7 +827,6 @@ impl UpdateMessage { Ok(Some(res)) } } - } @@ -1124,19 +1127,23 @@ pub enum FourOctetAsn { /// Iterator for BGP UPDATE Communities. /// -/// Returns values of enum [`Community`], wrapping [`StandardCommunity`], -/// [`ExtendedCommunity`], [`LargeCommunity`] and well-known communities. -pub struct CommunityIter { +/// Returns values of enum or struct [`T`], where T wraps or has variants +/// [`StandardCommunity`], [`ExtendedCommunity`], [`LargeCommunity`] or a +/// well-known community. T are most notably `Community` or +/// `HumanReadableCommunity`. In most cases you will need to explicitly state +/// the type of T. +pub struct CommunityIter { slice: Octs, pos: usize, + _t: PhantomData } -impl CommunityIter { +impl> CommunityIter { fn new(slice: Octs) -> Self { - CommunityIter { slice, pos: 0 } + CommunityIter { slice, pos: 0, _t: PhantomData } } - fn get_community(&mut self) -> Community { + fn get_community(&mut self) -> T { let mut buf = [0u8; 4]; buf[..].copy_from_slice(&self.slice.as_ref()[self.pos..self.pos+4]); self.pos += 4; @@ -1144,10 +1151,10 @@ impl CommunityIter { } } -impl Iterator for CommunityIter { - type Item = Community; +impl> Iterator for CommunityIter { + type Item = T; - fn next(&mut self) -> Option { + fn next(&mut self) -> Option { if self.pos == self.slice.as_ref().len() { return None } @@ -1961,6 +1968,7 @@ mod tests { #[test] fn pa_communities() { + use crate::bgp::communities::Community; // BGP UPDATE with 9 path attributes for 1 NLRI with Path Id, // includes both normal communities and extended communities. let buf = vec![ @@ -1996,11 +2004,11 @@ mod tests { )) ); - assert!(upd.communities().unwrap().is_some()); - for c in upd.communities().unwrap().unwrap() { + assert!(upd.communities::().unwrap().is_some()); + for c in upd.communities::().unwrap().unwrap() { println!("{:?}", c); } - assert!(upd.communities().unwrap().unwrap() + assert!(upd.communities::().unwrap().unwrap() .eq([ StandardCommunity::new(42.into(), Tag::new(518)).into(), Wellknown::NoExport.into(), @@ -2081,6 +2089,8 @@ mod tests { #[test] fn chained_community_iters() { + use crate::bgp::communities::Community; + let buf = vec![ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, @@ -2105,10 +2115,10 @@ mod tests { let upd: UpdateMessage<_> = Message::from_octets(&buf, Some(sc)) .unwrap().try_into().unwrap(); - for c in upd.all_communities().unwrap().unwrap() { + for c in upd.all_communities::().unwrap().unwrap() { println!("{}", c); } - assert!(upd.all_communities().unwrap().unwrap() + assert!(upd.all_communities::().unwrap().unwrap() .eq(&[ StandardCommunity::new(42.into(), Tag::new(518)).into(), Wellknown::NoExport.into(), @@ -2255,7 +2265,7 @@ mod tests { // the MP_REACH_NLRI currently ends up as a ::Invalid path attribute // variant, so the call to .mp_announcements() yields a Ok(None) and thus - // the second unwrap fails. Therefor, ignore for now: + // the second unwrap fails. Therefore, ignore for now: #[ignore = "need to rethink this one because of API change"] #[test] fn unknown_afi_safi_announcements() { diff --git a/src/bgp/message/update_builder.rs b/src/bgp/message/update_builder.rs index 853d23d6..496e81e6 100644 --- a/src/bgp/message/update_builder.rs +++ b/src/bgp/message/update_builder.rs @@ -1444,7 +1444,7 @@ mod tests { use octseq::Parser; - use crate::addr::Prefix; + use crate::{addr::Prefix, bgp::communities::Community}; use crate::asn::Asn; //use crate::bgp::communities::Wellknown; use crate::bgp::message::nlri::BasicNlri; @@ -1743,7 +1743,7 @@ mod tests { a_cnt += pdu.announcements().unwrap().count(); assert!(pdu.local_pref().unwrap().is_some()); assert!(pdu.multi_exit_disc().unwrap().is_some()); - assert_eq!(pdu.communities().unwrap().unwrap().count(), 300); + assert_eq!(pdu.communities::().unwrap().unwrap().count(), 300); } assert_eq!(a_cnt, prefixes_num.try_into().unwrap()); @@ -2187,7 +2187,7 @@ mod tests { assert!(prev_type_code < type_code); prev_type_code = type_code; } - assert_eq!(pdu.communities().unwrap().unwrap().count(), 2); + assert_eq!(pdu.communities::().unwrap().unwrap().count(), 2); } // TODO also do fn check(raw: Bytes)