From 1fd1571aa06437fe78da46e5ba428f677c665e74 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Thu, 9 Jul 2026 16:00:29 +0200 Subject: [PATCH 01/12] feat(config): vpcid map is specific of a VpcTable The vpcid map is a concept specific to a vpc table. Make it private, and construct it from a vpc table instead of an overlay. Signed-off-by: Fredi Raspall --- config/src/external/overlay/mod.rs | 46 ++++++++-------------------- config/src/external/overlay/tests.rs | 9 +----- config/src/external/overlay/vpc.rs | 29 +++++++++++------- 3 files changed, 32 insertions(+), 52 deletions(-) diff --git a/config/src/external/overlay/mod.rs b/config/src/external/overlay/mod.rs index 1f02f7d075..cb9976a944 100644 --- a/config/src/external/overlay/mod.rs +++ b/config/src/external/overlay/mod.rs @@ -10,7 +10,7 @@ pub mod vpcpeering; use crate::{ConfigError, ConfigResult}; use tracing::{debug, error}; -use vpc::{ValidatedVpcTable, VpcIdMap, VpcTable}; +use vpc::{ValidatedVpcTable, VpcTable}; use vpcpeering::{VpcManifest, VpcPeeringTable}; #[derive(Clone, Debug, Default)] @@ -27,6 +27,7 @@ impl Overlay { peering_table, } } + /// Check if a `Vpc` referred in a peering exists fn check_peering_vpc(&self, peering: &str, manifest: &VpcManifest) -> ConfigResult { self.vpc_table.get_vpc(&manifest.name).ok_or_else(|| { @@ -36,12 +37,8 @@ impl Overlay { Ok(()) } - /// Validate all peerings, checking if the VPCs they refer to exist in vpc table - /// - /// # Errors - /// - /// Returns an error if a peering references a VPC that does not exist in the VPC table. - pub fn validate_peerings(&self) -> ConfigResult { + /// Validate all peerings: check if the VPCs they refer to exist in vpc table + fn validate_peering_vpcs(&self) -> ConfigResult { debug!("Validating VPC peerings..."); for peering in self.peering_table.values() { self.check_peering_vpc(&peering.name, &peering.left)?; @@ -50,17 +47,6 @@ impl Overlay { Ok(()) } - /// Build a `VpcIdMap`. We have already checked that all VPC Ids are distinct - #[must_use] - pub(crate) fn vpcid_map(&self) -> VpcIdMap { - let id_map: VpcIdMap = self - .vpc_table - .values() - .map(|vpc| (vpc.name.clone(), vpc.id.clone())) - .collect(); - id_map - } - /// Validate the overlay configuration, returning a `ValidatedOverlay` if successful. /// /// # Errors @@ -69,28 +55,22 @@ impl Overlay { pub fn validate(&self) -> Result { debug!("Validating overlay configuration..."); - self.validate_peerings()?; + // validate peerings: + self.validate_peering_vpcs()?; // Collect peerings for every VPC and validate the table - let validated_vpc_table = self.collect_peerings().validate()?; - let validated_overlay = ValidatedOverlay { - vpc_table: validated_vpc_table, - }; + let vpc_table = self + .vpc_table + .collect_peerings(&self.peering_table) + .validate()?; + + let validated_overlay = ValidatedOverlay { vpc_table }; let peering_table = &self.peering_table; debug!("Overlay configuration is VALID:\n{validated_overlay}\n{peering_table}"); Ok(validated_overlay) } - /// Collect peerings from the peering table for every VPC. - /// - /// Should only be called in `validate`, or in tests. - pub(crate) fn collect_peerings(&self) -> VpcTable { - let id_map = self.vpcid_map(); - self.vpc_table - .collect_peerings(&self.peering_table, &id_map) - } - /// FOR TESTS ONLY. Fake validation for the overlay. /// /// # Safety @@ -100,7 +80,7 @@ impl Overlay { #[allow(unsafe_code)] #[must_use] pub unsafe fn fake_validated_overlay_for_tests(&self) -> ValidatedOverlay { - let vpc_table = self.collect_peerings(); + let vpc_table = self.vpc_table.collect_peerings(&self.peering_table); let fake_valid_vpc_table = unsafe { vpc_table.fake_validated_vpc_table_for_tests() }; ValidatedOverlay { vpc_table: fake_valid_vpc_table, diff --git a/config/src/external/overlay/tests.rs b/config/src/external/overlay/tests.rs index 85ce792b2c..5dce3e013e 100644 --- a/config/src/external/overlay/tests.rs +++ b/config/src/external/overlay/tests.rs @@ -10,7 +10,6 @@ pub mod test { use crate::external::ConfigError; use crate::external::overlay::Overlay; - use crate::external::overlay::VpcIdMap; use crate::external::overlay::vpc::{Vpc, VpcTable}; use crate::external::overlay::vpcpeering::VpcExpose; use crate::external::overlay::vpcpeering::VpcManifest; @@ -688,14 +687,8 @@ pub mod test { /* display peering table */ println!("{peering_table}"); - /* collect ids */ - let id_map: VpcIdMap = vpc_table - .values() - .map(|vpc| (vpc.name.clone(), vpc.id.clone())) - .collect(); - /* collect the peerings for each VPC */ - vpc_table.collect_peerings(&peering_table, &id_map); + vpc_table = vpc_table.collect_peerings(&peering_table); /* display VPC table */ println!("{}", vpc_table.as_summary()); diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index 89a30a15dd..71754d2ee1 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -187,7 +187,7 @@ impl TryFrom<&str> for VpcId { } } -pub(crate) type VpcIdMap = BTreeMap; +type VpcIdMap = BTreeMap; /// Representation of a VPC from the RPC #[derive(Clone, Debug, PartialEq)] @@ -266,16 +266,15 @@ impl Vpc { .map(Peering::validate) .collect::>()?; - let valid_vpc_candidate = ValidatedVpc { + let validated_vpc = ValidatedVpc { name: self.name.clone(), id: self.id.clone(), vni: self.vni, interfaces: self.interfaces.clone(), peerings: validated_peerings, }; - - valid_vpc_candidate.check_overlap_and_default()?; - Ok(valid_vpc_candidate) + validated_vpc.check_overlap_and_default()?; + Ok(validated_vpc) } /// FOR TESTS ONLY. Fake validation for the VPC peering manifests. @@ -468,6 +467,17 @@ impl VpcTable { Ok(()) } + /// Build a `VpcIdMap` + #[must_use] + fn vpcid_map(&self) -> VpcIdMap { + debug!("Building a VPC Id map..."); + let id_map: VpcIdMap = self + .values() + .map(|vpc| (vpc.name.clone(), vpc.id.clone())) + .collect(); + id_map + } + /// Get a [`Vpc`] from the vpc table by name #[must_use] pub fn get_vpc(&self, vpc_name: &str) -> Option<&Vpc> { @@ -490,16 +500,13 @@ impl VpcTable { } /// Collect peerings for all [`Vpc`]s in this [`VpcTable`] - pub(crate) fn collect_peerings( - &self, - peering_table: &VpcPeeringTable, - idmap: &VpcIdMap, - ) -> VpcTable { + pub(crate) fn collect_peerings(&self, peering_table: &VpcPeeringTable) -> VpcTable { + let idmap = self.vpcid_map(); debug!("Collecting peerings for all VPCs.."); let mut new_table = self.clone(); new_table .values_mut() - .for_each(|vpc| vpc.set_peerings(peering_table, idmap)); + .for_each(|vpc| vpc.set_peerings(peering_table, &idmap)); new_table } From 571769f2fb9f560a1085d24e38dbfbaf358593f0 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Thu, 9 Jul 2026 17:05:47 +0200 Subject: [PATCH 02/12] feat(config,deps): extend vpcmap and peerings Extend the VpcId map with Vnis and let peerings include the Vni of the remote VPC. Signed-off-by: Fredi Raspall --- config/src/display.rs | 12 +++--- config/src/external/overlay/vpc.rs | 47 +++++++++++++++++------ config/src/external/overlay/vpcpeering.rs | 2 +- config/src/utils/collapse.rs | 1 + flow-filter/src/setup.rs | 3 ++ nat/src/masquerade/apalloc/test_alloc.rs | 11 +++--- nat/src/static_nat/setup/mod.rs | 6 ++- nat/src/static_nat/test.rs | 28 ++++++++------ 8 files changed, 74 insertions(+), 36 deletions(-) diff --git a/config/src/display.rs b/config/src/display.rs index 5b5a1d3ce2..c976bf4c87 100644 --- a/config/src/display.rs +++ b/config/src/display.rs @@ -18,6 +18,7 @@ use crate::external::overlay::vpcpeering::{ use crate::external::overlay::vpcpeering::{VpcManifest, VpcPeering, VpcPeeringTable}; use crate::external::overlay::{Overlay, ValidatedOverlay}; use chrono::{DateTime, Utc}; +use net::vxlan::Vni; use common::cliprovider::Heading; const SEP: &str = " "; @@ -134,10 +135,11 @@ fn fmt_remote_manifest( f: &mut std::fmt::Formatter<'_>, manifest: &VpcManifest, remote_id: &VpcId, + remote_vni: Vni, ) -> std::fmt::Result { writeln!( f, - " remote VPC is {} (id:{}):", + " remote VPC is {} (id:{}), vni: {remote_vni}:", manifest.name, remote_id )?; for e in &manifest.exposes { @@ -152,7 +154,7 @@ impl Display for Peering { writeln!(f, " gwgroup: {}", &self.gwgroup)?; fmt_local_manifest(f, &self.local)?; writeln!(f)?; - fmt_remote_manifest(f, &self.remote, &self.remote_id)?; + fmt_remote_manifest(f, &self.remote, &self.remote_id, self.remote_vni)?; writeln!(f) } } @@ -172,12 +174,12 @@ fn fmt_remote_validated_manifest( f: &mut std::fmt::Formatter<'_>, manifest: &ValidatedManifest, remote_id: &VpcId, + remote_vni: Vni, ) -> std::fmt::Result { writeln!( f, - " remote VPC is {} (id:{}):", + " remote VPC is {} (id:{remote_id}) vni:{remote_vni} :", manifest.name(), - remote_id )?; for e in manifest.valexp() { e.fmt(f)?; @@ -191,7 +193,7 @@ impl Display for ValidatedPeering { writeln!(f, " gwgroup: {}", self.gwgroup())?; fmt_local_validated_manifest(f, self.local())?; writeln!(f)?; - fmt_remote_validated_manifest(f, self.remote(), self.remote_id())?; + fmt_remote_validated_manifest(f, self.remote(), self.remote_id(), self.remote_vni())?; writeln!(f) } } diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index 71754d2ee1..5d5658d807 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -31,6 +31,7 @@ pub struct Peering { pub local: VpcManifest, /* local manifest */ pub remote: VpcManifest, /* remote manifest */ pub remote_id: VpcId, /* Id of peer */ + pub remote_vni: Vni, /* Vni of peer -- should be vpc discriminant in future */ pub gwgroup: String, /* gateway group serving this peering */ } @@ -52,6 +53,7 @@ impl Peering { local: self.local.validate()?, remote: self.remote.validate()?, remote_id: self.remote_id.clone(), + remote_vni: self.remote_vni, gwgroup: self.gwgroup.clone(), }; valid_peering_candidate.validate_nat_combinations()?; @@ -79,6 +81,7 @@ impl Peering { local: fake_local, remote: fake_remote, remote_id: self.remote_id.clone(), + remote_vni: self.remote_vni, gwgroup: self.gwgroup.clone(), } } @@ -90,6 +93,7 @@ pub struct ValidatedPeering { local: ValidatedManifest, /* local manifest */ remote: ValidatedManifest, /* remote manifest */ remote_id: VpcId, /* Id of peer */ + remote_vni: Vni, /* Vni of peer -- should be vpc discriminant in future */ gwgroup: String, /* gateway group serving this peering */ } @@ -114,6 +118,11 @@ impl ValidatedPeering { &self.remote_id } + #[must_use] + pub fn remote_vni(&self) -> Vni { + self.remote_vni + } + #[must_use] pub fn gwgroup(&self) -> &String { &self.gwgroup @@ -187,7 +196,19 @@ impl TryFrom<&str> for VpcId { } } -type VpcIdMap = BTreeMap; +struct VpcSummary { + vpcid: VpcId, + vni: Vni, +} +impl From<&Vpc> for VpcSummary { + fn from(vpc: &Vpc) -> Self { + Self { + vpcid: vpc.id.clone(), + vni: vpc.vni, + } + } +} +type VpcMap = BTreeMap; /// Representation of a VPC from the RPC #[derive(Clone, Debug, PartialEq)] @@ -196,7 +217,7 @@ pub struct Vpc { pub id: VpcId, /* internal Id, unique*/ pub vni: Vni, /* mandatory */ pub interfaces: InterfaceConfigTable, /* user-defined interfaces in this VPC */ - pub peerings: Vec, /* peerings of this VPC - NOT set via gRPC */ + pub peerings: Vec, /* peerings of this VPC (collected) */ } impl Vpc { pub fn new(name: &str, id: &str, vni: u32) -> Result { @@ -211,18 +232,19 @@ impl Vpc { } /// Collect all peerings from the [`VpcPeeringTable`] table this vpc participates in - fn set_peerings(&mut self, peering_table: &VpcPeeringTable, idmap: &VpcIdMap) { + fn set_peerings(&mut self, peering_table: &VpcPeeringTable, idmap: &VpcMap) { debug!("Collecting peerings for vpc '{}'...", self.name); self.peerings = peering_table .peerings_vpc(&self.name) .map(|p| { let (local, remote) = p.get_peering_manifests(&self.name); - let remote_id = idmap.get(&remote.name).unwrap_or_else(|| unreachable!()); + let remote_vpc = idmap.get(&remote.name).unwrap_or_else(|| unreachable!()); Peering { name: p.name.clone(), local: local.clone(), remote: remote.clone(), - remote_id: remote_id.clone(), + remote_id: remote_vpc.vpcid.clone(), + remote_vni: remote_vpc.vni, gwgroup: p.gwgroup.clone(), } }) @@ -301,6 +323,7 @@ impl Vpc { local: fake_local, remote: fake_remote, remote_id: peering.remote_id.clone(), + remote_vni: peering.remote_vni, gwgroup: peering.gwgroup.clone(), } }) @@ -467,13 +490,13 @@ impl VpcTable { Ok(()) } - /// Build a `VpcIdMap` + /// Build a `VpcMap` #[must_use] - fn vpcid_map(&self) -> VpcIdMap { - debug!("Building a VPC Id map..."); - let id_map: VpcIdMap = self + fn vpc_map(&self) -> VpcMap { + debug!("Building a VPC map..."); + let id_map: VpcMap = self .values() - .map(|vpc| (vpc.name.clone(), vpc.id.clone())) + .map(|vpc| (vpc.name.clone(), VpcSummary::from(vpc))) .collect(); id_map } @@ -501,12 +524,12 @@ impl VpcTable { /// Collect peerings for all [`Vpc`]s in this [`VpcTable`] pub(crate) fn collect_peerings(&self, peering_table: &VpcPeeringTable) -> VpcTable { - let idmap = self.vpcid_map(); + let vpc_map = self.vpc_map(); debug!("Collecting peerings for all VPCs.."); let mut new_table = self.clone(); new_table .values_mut() - .for_each(|vpc| vpc.set_peerings(peering_table, &idmap)); + .for_each(|vpc| vpc.set_peerings(peering_table, &vpc_map)); new_table } diff --git a/config/src/external/overlay/vpcpeering.rs b/config/src/external/overlay/vpcpeering.rs index a43f254da6..5a13989ba2 100644 --- a/config/src/external/overlay/vpcpeering.rs +++ b/config/src/external/overlay/vpcpeering.rs @@ -839,7 +839,7 @@ impl VpcPeering { } /// Given a peering fetch the manifests, orderly depending on the provided vpc name #[must_use] - pub fn get_peering_manifests(&self, vpc: &str) -> (&VpcManifest, &VpcManifest) { + pub(crate) fn get_peering_manifests(&self, vpc: &str) -> (&VpcManifest, &VpcManifest) { if self.left.name == vpc { (&self.left, &self.right) } else { diff --git a/config/src/utils/collapse.rs b/config/src/utils/collapse.rs index f339c9c1e4..88cebda4bc 100644 --- a/config/src/utils/collapse.rs +++ b/config/src/utils/collapse.rs @@ -236,6 +236,7 @@ mod tests { local: manifest, remote: manifest_empty.clone(), remote_id: "12345".try_into().expect("Failed to create VPC ID"), + remote_vni: 100.try_into().unwrap(), gwgroup: "default".into(), }; diff --git a/flow-filter/src/setup.rs b/flow-filter/src/setup.rs index 15f57a1e2b..8d58b2a838 100644 --- a/flow-filter/src/setup.rs +++ b/flow-filter/src/setup.rs @@ -1232,6 +1232,7 @@ mod tests { vec![VpcExpose::empty().ip("20.0.0.0/24".into())], ), remote_id: "VPC02".try_into().unwrap(), + remote_vni: vpc2.vni, gwgroup: "default".into(), }); @@ -1290,6 +1291,7 @@ mod tests { vec![VpcExpose::empty().ip("20.0.0.0/24".into())], ), remote_id: "VPC02".try_into().unwrap(), + remote_vni: vpc2.vni, gwgroup: "default".into(), }); @@ -1304,6 +1306,7 @@ mod tests { vec![VpcExpose::empty().ip("20.0.0.0/25".into())], ), remote_id: "VPC03".try_into().unwrap(), + remote_vni: vpc3.vni, gwgroup: "default".into(), }); diff --git a/nat/src/masquerade/apalloc/test_alloc.rs b/nat/src/masquerade/apalloc/test_alloc.rs index 52c7f41300..b7c129e2f3 100644 --- a/nat/src/masquerade/apalloc/test_alloc.rs +++ b/nat/src/masquerade/apalloc/test_alloc.rs @@ -93,12 +93,17 @@ mod context { let manifest2 = VpcManifest::with_exposes("VPC-2", vec![expose3, expose4]); + // VPC-1 and VPC-2 + let mut vpc1 = Vpc::new("VPC-1", "67890", vni1().as_u32()).unwrap(); + let mut vpc2 = Vpc::new("VPC-2", "12345", vni2().as_u32()).unwrap(); + // Peerings let peering1 = Peering { name: "test_peering1".into(), local: manifest1.clone(), remote: manifest2.clone(), remote_id: "12345".try_into().unwrap(), + remote_vni: vpc2.vni, gwgroup: "default".into(), }; let peering2 = Peering { @@ -106,15 +111,11 @@ mod context { local: manifest2, remote: manifest1, remote_id: "67890".try_into().unwrap(), + remote_vni: vpc1.vni, gwgroup: "default".into(), }; - // VPC-1 - let mut vpc1 = Vpc::new("VPC-1", "67890", vni1().as_u32()).unwrap(); vpc1.peerings.push(peering1.clone()); - - // VPC-2 - let mut vpc2 = Vpc::new("VPC-2", "12345", vni2().as_u32()).unwrap(); vpc2.peerings.push(peering2.clone()); // VPC table diff --git a/nat/src/static_nat/setup/mod.rs b/nat/src/static_nat/setup/mod.rs index a21b6b72e5..9fbd12c5e2 100644 --- a/nat/src/static_nat/setup/mod.rs +++ b/nat/src/static_nat/setup/mod.rs @@ -182,16 +182,18 @@ mod tests { manifest2.add_expose(expose3); manifest2.add_expose(expose4); + let src_vni = Vni::new_checked(100).unwrap(); + let dst_vni = Vni::new_checked(200).unwrap(); + let peering: Peering = Peering { name: "test_peering".into(), local: manifest1, remote: manifest2, remote_id: "12345".try_into().expect("Failed to create VPC ID"), + remote_vni: dst_vni, gwgroup: "default".into(), }; - let src_vni = Vni::new_checked(100).unwrap(); - let dst_vni = Vni::new_checked(200).unwrap(); let mut vpctable = VpcTable::new(); let mut src_vpc = Vpc::new("VPC", "12345", src_vni.as_u32()).unwrap(); src_vpc.peerings.push(peering.clone()); diff --git a/nat/src/static_nat/test.rs b/nat/src/static_nat/test.rs index 67a3f69cf9..148027da5d 100644 --- a/nat/src/static_nat/test.rs +++ b/nat/src/static_nat/test.rs @@ -218,11 +218,23 @@ fn build_context() -> NatTables { let manifest2 = VpcManifest::with_exposes("VPC-2", vec![expose3, expose4]); + // This code is extremely convoluted + let mut vpctable = VpcTable::new(); + + // vpc-1 + let vni1 = Vni::new_checked(100).unwrap(); + let mut vpc1 = Vpc::new("VPC-1", "67890", vni1.as_u32()).unwrap(); + + // vpc-2 + let vni2 = Vni::new_checked(200).unwrap(); + let mut vpc2 = Vpc::new("VPC-2", "12345", vni2.as_u32()).unwrap(); + let peering1 = Peering { name: "test_peering1".into(), local: manifest1.clone(), remote: manifest2.clone(), remote_id: "12345".try_into().expect("Failed to create VPC ID"), + remote_vni: vpc2.vni, gwgroup: "default".into(), }; let peering2 = Peering { @@ -230,22 +242,16 @@ fn build_context() -> NatTables { local: manifest2, remote: manifest1, remote_id: "67890".try_into().expect("Failed to create VPC ID"), + remote_vni: vpc1.vni, gwgroup: "default".into(), }; - // This code is extremely convoluted - let mut vpctable = VpcTable::new(); - - // vpc-1 - let vni1 = Vni::new_checked(100).unwrap(); - let mut vpc1 = Vpc::new("VPC-1", "67890", vni1.as_u32()).unwrap(); + // Add peerings to vpcs vpc1.peerings.push(peering1.clone()); - vpctable.add(vpc1).unwrap(); - - // vpc-2 - let vni2 = Vni::new_checked(200).unwrap(); - let mut vpc2 = Vpc::new("VPC-2", "12345", vni2.as_u32()).unwrap(); vpc2.peerings.push(peering2.clone()); + + // add vpcs to table + vpctable.add(vpc1).unwrap(); vpctable.add(vpc2).unwrap(); let mut nat_table = NatTables::new(); From 43899ff51a3125f23920e5be74525788e310aeea Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 12:18:19 +0200 Subject: [PATCH 03/12] feat(lpm): add PrefixPortsSet builders for all-prefixes The builders will be used to represent default peering exposes. Signed-off-by: Fredi Raspall --- lpm/src/prefix/with_ports.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lpm/src/prefix/with_ports.rs b/lpm/src/prefix/with_ports.rs index 6dccb5ab5c..3869c339a9 100644 --- a/lpm/src/prefix/with_ports.rs +++ b/lpm/src/prefix/with_ports.rs @@ -61,7 +61,7 @@ pub trait IpRangeWithPorts { } /// A structure containing a prefix and a port range. -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] #[cfg_attr(any(test, feature = "bolero"), derive(bolero::TypeGenerator))] pub struct PrefixWithPorts { prefix: Prefix, @@ -158,6 +158,22 @@ impl PrefixPortsSet { Self::default() } + #[must_use] + pub fn root_v4() -> Self { + let mut empty = Self::new(); + let root_v4 = PrefixWithOptionalPorts::new(Prefix::root_v4(), None); + empty.insert(root_v4); + empty + } + + #[must_use] + pub fn root_v6() -> Self { + let mut empty = Self::new(); + let root_v6 = PrefixWithOptionalPorts::new(Prefix::root_v6(), None); + empty.insert(root_v6); + empty + } + /// Given two [`PrefixPortsSet`] objects, returns the set containing the intersection of /// prefixes and ports. This new set may contain overlapping prefixes (within the set), if any /// of the two initial sets contains overlapping prefixes (within that set) that also overlap @@ -239,7 +255,7 @@ impl std::ops::DerefMut for PrefixPortsSet { /// type would be semantically equivalent. To avoid this, the constructor /// of this type guarantees that `max_range` is mapped to None, without /// needing to call an additional simplify method. -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct PrefixWithOptionalPorts { prefix: Prefix, ports: Option, @@ -356,7 +372,7 @@ where } /// A port range, with a start and an end port (both included in the range). -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct PortRange { start: u16, end: u16, From 4ff204a700e9be27738474ab71e39e01c732213c Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 12:19:58 +0200 Subject: [PATCH 04/12] feat(config): add vpcrouting module The vpc routing module defines a vpc routing table which is an an alternate representation of the peerings of a given VPC. This representation is easier to validate and closer to what some NFs require to build their internal state. The validation of the vpc routing table includes now the constraint that remote destinations of a VPC that are allowed to overlap must be mapped to the same gateway group for failover to work correctly. This restriction is not one imposed by a single gateway but a fabric-wide one. Signed-off-by: Fredi Raspall --- config/src/external/overlay/mod.rs | 1 + config/src/external/overlay/vpcrouting.rs | 164 ++++++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 config/src/external/overlay/vpcrouting.rs diff --git a/config/src/external/overlay/mod.rs b/config/src/external/overlay/mod.rs index cb9976a944..e4bed68f7d 100644 --- a/config/src/external/overlay/mod.rs +++ b/config/src/external/overlay/mod.rs @@ -7,6 +7,7 @@ pub mod tests; pub mod validation_tests; pub mod vpc; pub mod vpcpeering; +pub mod vpcrouting; use crate::{ConfigError, ConfigResult}; use tracing::{debug, error}; diff --git a/config/src/external/overlay/vpcrouting.rs b/config/src/external/overlay/vpcrouting.rs new file mode 100644 index 0000000000..c9aef71d07 --- /dev/null +++ b/config/src/external/overlay/vpcrouting.rs @@ -0,0 +1,164 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Dataplane configuration model: vpc routing + +use crate::ConfigError; +use crate::external::ValidatedPeering; +use crate::external::overlay::vpcpeering::ValidatedExpose; +use lpm::prefix::{IpRangeWithPorts, PrefixPortsSet, PrefixWithOptionalPorts}; +use net::vxlan::Vni; +use ordermap::OrderMap; + +/// A type indicating the action required by an exposed destination +#[derive(Hash, PartialEq, Eq, Clone, Copy, Debug)] +pub enum ExposeAction { + Masquerade, + PortForwarding, + StaticNat, + Forward, + Default, +} +impl From<&ValidatedExpose> for ExposeAction { + fn from(expose: &ValidatedExpose) -> Self { + if expose.has_masquerade() { + return ExposeAction::Masquerade; + } else if expose.has_port_forwarding() { + return ExposeAction::PortForwarding; + } else if expose.has_static_nat() { + return ExposeAction::StaticNat; + } else if expose.is_default() { + return ExposeAction::Default; + } + ExposeAction::Forward + } +} + +/// A type representing a route to a remote vpc +#[derive(Debug)] +pub struct VpcRoute { + dst: PrefixWithOptionalPorts, // destination(s) this route applies to + dstvpc: String, // destination VPC + dstvni: Vni, // data path discriminant towards destination + rem_action: ExposeAction, // action required by remote VPC + gwgroup: String, // gateway group handling the peering this route corresponds to +} + +/// A type representing a set of routes to the same destination. +/// This type is currently not public +#[derive(Debug)] +struct VpcRouteSet(Vec); +impl VpcRouteSet { + #[must_use] + fn new() -> Self { + Self(Vec::new()) + } + fn iter(&self) -> impl Iterator { + self.0.iter() + } +} + +impl VpcRoute { + #[must_use] + fn is_default(&self) -> bool { + self.rem_action == ExposeAction::Default + } + #[must_use] + fn is_masquerade(&self) -> bool { + self.rem_action == ExposeAction::Masquerade + } + /// Tell if a route is allowed to overlap with some other route. + fn can_overlap(&self, other: &VpcRoute) -> bool { + if self.dstvpc == other.dstvpc { + return true; + } + if self.is_masquerade() && other.is_masquerade() { + return true; + } + // both can't be default at the same time + self.is_default() ^ other.is_default() + } +} + +/// A table of `VpcRoutes`. +/// +/// For any destination `PrefixWithOptionalPorts`, this table can keep +/// a collection of `VpcRoute`s in the form of a `VpcRouteSet`. Routes to +/// the same destination are kept together in a `VpcRouteSet`. +#[derive(Debug)] +pub struct VpcRouteTable { + table: OrderMap, +} + +impl VpcRouteTable { + #[must_use] + fn new() -> Self { + Self { + table: OrderMap::default(), + } + } + pub fn iter(&self) -> impl Iterator { + self.table.values().flat_map(VpcRouteSet::iter) + } + + #[must_use] + /// Build a `VpcRouteTable` from the set of `ValidatedPeering` of a VPC + pub fn build(peerings: &Vec) -> Self { + let mut rt = VpcRouteTable::new(); + for peering in peerings { + for expose in peering.remote().valexp() { + let destinations = if expose.is_default() { + &PrefixPortsSet::root_v4() + } else { + expose.public_ips() + }; + // build a route for each of the destinations of the remote expose + for prefix in destinations { + let route = VpcRoute { + dstvpc: peering.remote().name().to_string(), + gwgroup: peering.gwgroup().clone(), + dst: *prefix, + rem_action: ExposeAction::from(expose), + dstvni: peering.remote_vni(), + }; + let set = rt.table.entry(*prefix).or_insert(VpcRouteSet::new()); + set.0.push(route); + } + } + } + rt + } + + /// Consume and validate a `VpcRouteTable` + /// + /// # Errors + /// + /// This method returns `ConfigError` if the `VpcRouteTable` fails to meet the validation rules: + /// 1) a vpc can have one default route at the most + /// 2) destinations cannot overlap except if they are masqueraded or a default + /// 3) overlapping destinations, when allowed, must use the same gateway group + /// + pub fn validate(self) -> Result { + let all_routes: Vec<&VpcRoute> = self.table.values().flat_map(VpcRouteSet::iter).collect(); + for (i, &route) in all_routes.iter().enumerate() { + for &other in &all_routes[i + 1..] { + if route.is_default() && other.is_default() { + return Err(ConfigError::Forbidden( + "Multiple default destinations exposed to VPC", + )); + } + if route.dst.overlaps(&other.dst) { + if !route.can_overlap(other) { + return Err(ConfigError::OverlappingPrefixes(route.dst, other.dst)); + } + if route.gwgroup != other.gwgroup { + return Err(ConfigError::Forbidden( + "Overlapping exposes cannot use distinct groups", + )); + } + } + } + } + Ok(self) + } +} From 59f2dd0220c80e0fd23111cd778616598a1cee24 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 13:20:39 +0200 Subject: [PATCH 05/12] feat(config): build vpc routing table and validate it Build a vpc routing table from the set of validated peerings of a VPC and validate it, replacing the previous validation function. Signed-off-by: Fredi Raspall --- config/src/external/overlay/vpc.rs | 63 ++---------------------------- 1 file changed, 3 insertions(+), 60 deletions(-) diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index 5d5658d807..88d2e124c7 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -5,7 +5,6 @@ #![allow(clippy::missing_errors_doc)] -use lpm::prefix::IpRangeWithPorts; use net::vxlan::Vni; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -16,6 +15,7 @@ use crate::external::overlay::VpcManifest; use crate::external::overlay::VpcPeeringTable; use crate::external::overlay::vpcpeering::ValidatedManifest; use crate::external::overlay::vpcpeering::VpcExposeNatConfig; +use crate::external::overlay::vpcrouting::VpcRouteTable; use crate::internal::interfaces::interface::InterfaceConfigTable; use crate::{ConfigError, ConfigResult}; @@ -288,6 +288,8 @@ impl Vpc { .map(Peering::validate) .collect::>()?; + VpcRouteTable::build(&validated_peerings).validate()?; + let validated_vpc = ValidatedVpc { name: self.name.clone(), id: self.id.clone(), @@ -295,7 +297,6 @@ impl Vpc { interfaces: self.interfaces.clone(), peerings: validated_peerings, }; - validated_vpc.check_overlap_and_default()?; Ok(validated_vpc) } @@ -390,64 +391,6 @@ impl ValidatedVpc { .any(|e| e.has_port_forwarding() || e.has_masquerade()) }) } - - /// Check that prefixes exposed to a given VPC do not overlap. Exceptions: - /// - /// - overlap is allowed between a prefix and a default expose (it overlaps by design) - /// - overlap is allowed between prefixes from different exposes if both their exposes use - /// masquerade (we can fall back to the flow table to disambiguate the destination VPC) - /// - /// Also check that at most one default expose is exposed to the VPC. - fn check_overlap_and_default(&self) -> ConfigResult { - // FIXME: Find a less expensive approach to find overlapping prefixes - for (i, current_peering) in self.peerings().iter().enumerate() { - // Check we don't have non-default, overlapping prefixes exposed to the VPC - for other_peering in &self.peerings()[i + 1..] { - for current_expose in current_peering.remote().valexp() { - for other_expose in other_peering.remote().valexp() { - if current_expose.has_masquerade() && other_expose.has_masquerade() { - // Overlap is allowed if both expose blocks use masquerade - continue; - } - match (current_expose.is_default(), other_expose.is_default()) { - (true, true) => { - // We support at most one default destination exposed to any VPC - error!( - "Multiple 'default' destinations exposed to VPC {}", - self.name() - ); - return Err(ConfigError::Forbidden( - "Multiple 'default' destinations exposed to VPC", - )); - } - (true, false) | (false, true) => { - // Overlap is allowed between a prefix and a default expose - continue; - } - (false, false) => { /* keep processing */ } - } - for current_prefix in current_expose.public_ips() { - for other_prefix in other_expose.public_ips() { - if current_prefix.overlaps(other_prefix) { - error!( - "Prefixes exposed to VPC {} overlap: {} and {}", - self.name(), - current_prefix, - other_prefix - ); - return Err(ConfigError::OverlappingPrefixes( - *current_prefix, - *other_prefix, - )); - } - } - } - } - } - } - } - Ok(()) - } } #[derive(Clone, Debug, Default)] From f395cd1fee7c5f53f77a6b33aa09968c4326e306 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 14:17:24 +0200 Subject: [PATCH 06/12] feat(config): Add VpcRoute getters Signed-off-by: Fredi Raspall --- config/src/external/overlay/vpcrouting.rs | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/config/src/external/overlay/vpcrouting.rs b/config/src/external/overlay/vpcrouting.rs index c9aef71d07..3f7ed19ac7 100644 --- a/config/src/external/overlay/vpcrouting.rs +++ b/config/src/external/overlay/vpcrouting.rs @@ -80,6 +80,30 @@ impl VpcRoute { } } +// pub getters +impl VpcRoute { + #[must_use] + pub fn destination(&self) -> PrefixWithOptionalPorts { + self.dst + } + #[must_use] + pub fn dst_vpc(&self) -> &str { + &self.dstvpc + } + #[must_use] + pub fn remote_action(&self) -> ExposeAction { + self.rem_action + } + #[must_use] + pub fn dst_vni(&self) -> Vni { + self.dstvni + } + #[must_use] + pub fn gw_group(&self) -> &str { + &self.gwgroup + } +} + /// A table of `VpcRoutes`. /// /// For any destination `PrefixWithOptionalPorts`, this table can keep From 082d703a574d163aec4e1f12e4239a106eab38c8 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 14:23:05 +0200 Subject: [PATCH 07/12] feat(config): impl Display for Vpc routing table Signed-off-by: Fredi Raspall --- config/src/display.rs | 66 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/config/src/display.rs b/config/src/display.rs index c976bf4c87..e95d7ba770 100644 --- a/config/src/display.rs +++ b/config/src/display.rs @@ -16,7 +16,9 @@ use crate::external::overlay::vpcpeering::{ VpcExposePortForwarding, VpcExposeStaticNat, }; use crate::external::overlay::vpcpeering::{VpcManifest, VpcPeering, VpcPeeringTable}; +use crate::external::overlay::vpcrouting::{ExposeAction, VpcRoute, VpcRouteTable}; use crate::external::overlay::{Overlay, ValidatedOverlay}; + use chrono::{DateTime, Utc}; use net::vxlan::Vni; @@ -347,6 +349,18 @@ impl Display for ValidatedVpcSummary<'_> { } } +pub struct VpcTableRoutingTables<'a>(&'a ValidatedVpcTable); +impl Display for VpcTableRoutingTables<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + for vpc in self.0.values() { + Heading(vpc.name().to_string()).fmt(f)?; + vpc.route_table().fmt(f)?; + writeln!(f)?; + } + Ok(()) + } +} + pub struct VpcTableSummary<'a>(&'a VpcTable); impl Display for VpcTableSummary<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -480,6 +494,58 @@ impl Display for ValidatedOverlay { } } +/* ===== VPC routing tables =====*/ + +macro_rules! VPC_RT_TBL_FMT { + () => { + "{:<30} {:<10} {:<12} {:<13} {:<16}" + }; +} +fn fmt_vpc_rt_table_heading(f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + writeln!( + f, + VPC_RT_TBL_FMT!(), + "destinations", "remote-VPC", "remote-vni", "remote-action", "gw-group" + ) +} + +impl Display for ExposeAction { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.pad(match self { + ExposeAction::Default => "default", + ExposeAction::Forward => "forward", + ExposeAction::StaticNat => "static-nat", + ExposeAction::PortForwarding => "port-forward", + ExposeAction::Masquerade => "masqueraded", + }) + } +} + +impl Display for VpcRoute { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let dst = self.destination().to_string(); + writeln!( + f, + VPC_RT_TBL_FMT!(), + dst, + self.dst_vpc(), + self.dst_vni(), + self.remote_action(), + self.gw_group() + ) + } +} + +impl Display for VpcRouteTable { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + fmt_vpc_rt_table_heading(f)?; + for route_set in self.iter() { + route_set.fmt(f)?; + } + Ok(()) + } +} + /* ===== Configuration history ===== */ macro_rules! CONFIGDB_TBL_FMT { From 4a304ea6ce9edd54cbaaa4b6de0bfc32218396f2 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 14:31:38 +0200 Subject: [PATCH 08/12] feat(config): remove unnecessary derived impls Remove unnecessary derived impls as these impose unnecessary requirements on embedded types. Signed-off-by: Fredi Raspall --- config/src/external/overlay/mod.rs | 2 +- config/src/external/overlay/tests.rs | 12 +++++++----- config/src/external/overlay/vpc.rs | 6 +++--- config/src/internal/interfaces/interface.rs | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/config/src/external/overlay/mod.rs b/config/src/external/overlay/mod.rs index e4bed68f7d..1a84dd0db0 100644 --- a/config/src/external/overlay/mod.rs +++ b/config/src/external/overlay/mod.rs @@ -89,7 +89,7 @@ impl Overlay { } } -#[derive(Clone, Debug, Default)] +#[derive(Debug, Default)] pub struct ValidatedOverlay { vpc_table: ValidatedVpcTable, } diff --git a/config/src/external/overlay/tests.rs b/config/src/external/overlay/tests.rs index 5dce3e013e..de09bb26dc 100644 --- a/config/src/external/overlay/tests.rs +++ b/config/src/external/overlay/tests.rs @@ -65,7 +65,7 @@ pub mod test { /* invalid vni should be rejected */ let vpc1 = Vpc::new("VPC-1", "AAAAA", 0); - assert_eq!(vpc1, Err(ConfigError::InvalidVpcVni(0))); + assert!(vpc1.is_err_and(|e| matches!(e, ConfigError::InvalidVpcVni(0)))); /* add vpc with valid vni 3000 */ let vpc1 = Vpc::new("VPC-1", "AAAAA", 3000).expect("Should succeed"); @@ -90,12 +90,14 @@ pub mod test { ); /* vpc with bad Id should not build */ - let bad = Vpc::new("VPC-2", "AAA", 9000); - assert_eq!(bad, Err(ConfigError::BadVpcId("AAA".to_string()))); + let bad_id = "AAA".to_string(); + let bad = Vpc::new("VPC-2", &bad_id, 9000); + assert!(bad.is_err_and(|e| matches!(e, ConfigError::BadVpcId(_bad_id)))); /* vpc with bad Id should not build */ - let bad = Vpc::new("VPC-2", "!1234", 9000); - assert_eq!(bad, Err(ConfigError::BadVpcId("!1234".to_string()))); + let bad_id = "!1234".to_string(); + let bad = Vpc::new("VPC-2", &bad_id, 9000); + assert!(bad.is_err_and(|e| matches!(e, ConfigError::BadVpcId(_bad_id)))); } #[test] diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index 88d2e124c7..79daa47bde 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -211,7 +211,7 @@ impl From<&Vpc> for VpcSummary { type VpcMap = BTreeMap; /// Representation of a VPC from the RPC -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug)] pub struct Vpc { pub name: String, /* name of vpc, used as key */ pub id: VpcId, /* internal Id, unique*/ @@ -340,7 +340,7 @@ impl Vpc { } } -#[derive(Clone, Debug, PartialEq)] +#[derive(Debug)] pub struct ValidatedVpc { name: String, /* name of vpc, used as key */ id: VpcId, /* internal Id, unique*/ @@ -515,7 +515,7 @@ impl VpcTable { } } -#[derive(Clone, Debug, Default)] +#[derive(Debug, Default)] pub struct ValidatedVpcTable { vpcs: BTreeMap, ids: BTreeMap, // name of vpc diff --git a/config/src/internal/interfaces/interface.rs b/config/src/internal/interfaces/interface.rs index b16a57c5a5..78e3b8c653 100644 --- a/config/src/internal/interfaces/interface.rs +++ b/config/src/internal/interfaces/interface.rs @@ -65,7 +65,7 @@ pub struct InterfaceConfig { pub pci: Option, } -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, Default)] /// An interface configuration table pub struct InterfaceConfigTable(BTreeMap); From 73f2127d29c545a86ff6ed6aa0902367b8f23035 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 14:59:21 +0200 Subject: [PATCH 09/12] feat(config): store vpc routing table in ValidatedVpc Store the vpc routing table in a ValidatedVpc for future use. Signed-off-by: Fredi Raspall --- config/src/external/overlay/vpc.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index 79daa47bde..0b1d5a86c1 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -288,7 +288,7 @@ impl Vpc { .map(Peering::validate) .collect::>()?; - VpcRouteTable::build(&validated_peerings).validate()?; + let rt = VpcRouteTable::build(&validated_peerings).validate()?; let validated_vpc = ValidatedVpc { name: self.name.clone(), @@ -296,6 +296,7 @@ impl Vpc { vni: self.vni, interfaces: self.interfaces.clone(), peerings: validated_peerings, + rt, }; Ok(validated_vpc) } @@ -330,12 +331,15 @@ impl Vpc { }) .collect::>(); + let not_validated_rt = VpcRouteTable::build(&fake_validated_peerings); + ValidatedVpc { name: self.name.clone(), id: self.id.clone(), vni: self.vni, interfaces: self.interfaces.clone(), peerings: fake_validated_peerings, + rt: not_validated_rt, } } } @@ -347,6 +351,7 @@ pub struct ValidatedVpc { vni: Vni, /* mandatory */ interfaces: InterfaceConfigTable, /* user-defined interfaces in this VPC */ peerings: Vec, /* peerings of this VPC - NOT set via gRPC */ + rt: VpcRouteTable, } impl ValidatedVpc { From f9127e352ef7c6c5012708c403a58acfa63ccead Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 15:05:30 +0200 Subject: [PATCH 10/12] feat(config): remove unused ConfigError variants Signed-off-by: Fredi Raspall --- config/src/errors.rs | 8 -------- config/src/external/overlay/tests.rs | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/config/src/errors.rs b/config/src/errors.rs index c438c87b2b..322ac41cbf 100644 --- a/config/src/errors.rs +++ b/config/src/errors.rs @@ -42,8 +42,6 @@ pub enum ConfigError { InvalidVpcVni(u32), #[error("Config with id {0} not found")] NoSuchConfig(GenId), - #[error("A config with id {0} already exists")] - ConfigAlreadyExists(GenId), #[error("Failure applying config: {0}")] FailureApply(String), #[error("Forbidden: {0}")] @@ -58,20 +56,14 @@ pub enum ConfigError { MissingIdentifier(&'static str), #[error("Missing mandatory parameter: {0}")] MissingParameter(&'static str), - #[error("Incomplete: {0}")] - Incomplete(String), #[error("Multiple instances of {0} found, expected {1}")] TooManyInstances(&'static str, usize), #[error("Internal error: {0}")] InternalFailure(String), - #[error("MTU out of range [68, 65535]: {0}")] - BadMtu(u32), // Peering and VpcExpose validation #[error("All prefixes are excluded in VpcExpose: {0}")] ExcludedAllPrefixes(Box), - #[error("Exclusion prefix {0} not contained within existing allowed prefix")] - OutOfRangeExclusionPrefix(PrefixWithOptionalPorts), #[error("VPC prefixes overlap: {0} and {1}")] OverlappingPrefixes(PrefixWithOptionalPorts, PrefixWithOptionalPorts), #[error("Inconsistent IP version in VpcExpose: {0}")] diff --git a/config/src/external/overlay/tests.rs b/config/src/external/overlay/tests.rs index de09bb26dc..0266039d9c 100644 --- a/config/src/external/overlay/tests.rs +++ b/config/src/external/overlay/tests.rs @@ -1171,7 +1171,7 @@ pub mod test { let overlay = Overlay::new(vpc_table, peering_table); assert!(overlay.validate().is_err_and( - |e| e == ConfigError::Forbidden("Multiple 'default' destinations exposed to VPC") + |e| e == ConfigError::Forbidden("Multiple default destinations exposed to VPC") )); } From 5818ca61e937ff40259980625648607be42ebf85 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 16:31:37 +0200 Subject: [PATCH 11/12] feat(cli): show vpc routing tables in cli Signed-off-by: Fredi Raspall --- cli/bin/cmdtree_dp.rs | 5 +++++ cli/src/cliproto.rs | 1 + config/src/display.rs | 6 +++++- config/src/external/overlay/vpc.rs | 5 +++++ routing/src/cli/handler.rs | 2 ++ 5 files changed, 18 insertions(+), 1 deletion(-) diff --git a/cli/bin/cmdtree_dp.rs b/cli/bin/cmdtree_dp.rs index af3ac88481..9350d8cb2c 100644 --- a/cli/bin/cmdtree_dp.rs +++ b/cli/bin/cmdtree_dp.rs @@ -81,6 +81,11 @@ fn cmd_show_vpc() -> Node { root += Node::new("peerings") .desc("Show the peerings of each vpc") .action(CliAction::ShowVpcPeerings); + + root += Node::new("routing") + .desc("Show the routing table of each vpc") + .action(CliAction::ShowVpcRouting); + root } fn cmd_show_ip() -> Node { diff --git a/cli/src/cliproto.rs b/cli/src/cliproto.rs index 1d463fbfcd..7ddfd5befa 100644 --- a/cli/src/cliproto.rs +++ b/cli/src/cliproto.rs @@ -286,6 +286,7 @@ pub enum CliAction { // config: vpcs & peerings ShowVpc, ShowVpcPeerings, + ShowVpcRouting, // router: Eventlog RouterEventLog, diff --git a/config/src/display.rs b/config/src/display.rs index e95d7ba770..73cd508997 100644 --- a/config/src/display.rs +++ b/config/src/display.rs @@ -425,6 +425,10 @@ impl ValidatedVpcTable { pub fn as_peerings(&self) -> ValidatedVpcTablePeerings<'_> { ValidatedVpcTablePeerings(self) } + #[must_use] + pub fn as_route_tables(&self) -> VpcTableRoutingTables<'_> { + VpcTableRoutingTables(self) + } } impl Vpc { @@ -498,7 +502,7 @@ impl Display for ValidatedOverlay { macro_rules! VPC_RT_TBL_FMT { () => { - "{:<30} {:<10} {:<12} {:<13} {:<16}" + " {:<30} {:<10} {:<12} {:<13} {:<16}" }; } fn fmt_vpc_rt_table_heading(f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index 0b1d5a86c1..ad7cc4fde5 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -380,6 +380,11 @@ impl ValidatedVpc { &self.peerings } + #[must_use] + pub fn route_table(&self) -> &VpcRouteTable { + &self.rt + } + /// Tell how many peerings this VPC has #[must_use] pub fn num_peerings(&self) -> usize { diff --git a/routing/src/cli/handler.rs b/routing/src/cli/handler.rs index 71d5f38403..dbba5f992d 100644 --- a/routing/src/cli/handler.rs +++ b/routing/src/cli/handler.rs @@ -392,6 +392,7 @@ fn show_config(request: CliRequest, config: Option<&Arc>) -> let contents = match request.action { CliAction::ShowVpc => vpc_table.as_summary().to_string(), CliAction::ShowVpcPeerings => vpc_table.as_peerings().to_string(), + CliAction::ShowVpcRouting => vpc_table.as_route_tables().to_string(), CliAction::ShowGatewayGroups => config.external().gwgroups().to_string(), CliAction::ShowGatewayCommunities => config.external().communities().to_string(), CliAction::ShowConfigInternal => { @@ -446,6 +447,7 @@ fn do_handle_cli_request( CliAction::ShowTech => show_tech(request, db, rio, sources), CliAction::ShowVpc | CliAction::ShowVpcPeerings + | CliAction::ShowVpcRouting | CliAction::ShowGatewayCommunities | CliAction::ShowGatewayGroups | CliAction::ShowConfigInternal => show_config(request, rio.gwconfig.as_ref()), From 5c59bf38b11eb6e62233dfe1a3ab746616d8fc74 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 10 Jul 2026 18:12:20 +0200 Subject: [PATCH 12/12] refactor(config,deps): unify validated types back Revert back to non-validated types Signed-off-by: Fredi Raspall --- config/src/display.rs | 189 +----------------- config/src/external/mod.rs | 115 ++++------- config/src/external/overlay/mod.rs | 35 ++-- config/src/external/overlay/tests.rs | 80 ++++---- .../src/external/overlay/validation_tests.rs | 140 ++++++------- config/src/external/overlay/vpc.rs | 159 +++++---------- config/src/external/overlay/vpcpeering.rs | 184 +++++++---------- config/src/external/overlay/vpcrouting.rs | 26 +-- config/src/external/underlay/mod.rs | 12 +- config/src/gwconfig.rs | 32 +-- config/src/lib.rs | 2 +- config/src/utils/overlap.rs | 14 +- flow-filter/src/setup.rs | 145 ++++++-------- flow-filter/src/tests.rs | 36 +++- mgmt/src/processor/confbuild/internal.rs | 32 +-- mgmt/src/processor/confbuild/namegen.rs | 4 +- mgmt/src/processor/confbuild/router.rs | 4 +- mgmt/src/processor/gwconfigdb.rs | 12 +- mgmt/src/processor/k8s_client.rs | 2 +- mgmt/src/processor/mgmt_client.rs | 6 +- mgmt/src/processor/proc.rs | 29 ++- mgmt/src/tests/mgmt.rs | 3 +- nat/src/masquerade/allocator_writer.rs | 18 +- nat/src/masquerade/apalloc/setup.rs | 84 ++++---- nat/src/masquerade/apalloc/test_alloc.rs | 7 +- nat/src/masquerade/test.rs | 60 +++--- nat/src/portfw/portfwtable/access.rs | 7 +- nat/src/portfw/portfwtable/setup.rs | 19 +- nat/src/static_nat/setup/mod.rs | 16 +- nat/src/static_nat/test.rs | 20 +- nat/src/test.rs | 16 +- routing/src/cli/handler.rs | 4 +- routing/src/router/ctl.rs | 8 +- routing/src/router/rio.rs | 4 +- validator/src/main.rs | 4 +- 35 files changed, 590 insertions(+), 938 deletions(-) diff --git a/config/src/display.rs b/config/src/display.rs index 73cd508997..5acac97bac 100644 --- a/config/src/display.rs +++ b/config/src/display.rs @@ -8,16 +8,13 @@ use std::fmt::Display; use crate::GwConfigMeta; -use crate::external::overlay::vpc::{ - Peering, ValidatedPeering, ValidatedVpc, ValidatedVpcTable, Vpc, VpcId, VpcTable, -}; +use crate::external::overlay::Overlay; +use crate::external::overlay::vpc::{Peering, Vpc, VpcId, VpcTable}; use crate::external::overlay::vpcpeering::{ - ValidatedExpose, ValidatedManifest, VpcExpose, VpcExposeMasquerade, VpcExposeNatConfig, - VpcExposePortForwarding, VpcExposeStaticNat, + VpcExpose, VpcExposeMasquerade, VpcExposeNatConfig, VpcExposePortForwarding, VpcExposeStaticNat, }; use crate::external::overlay::vpcpeering::{VpcManifest, VpcPeering, VpcPeeringTable}; use crate::external::overlay::vpcrouting::{ExposeAction, VpcRoute, VpcRouteTable}; -use crate::external::overlay::{Overlay, ValidatedOverlay}; use chrono::{DateTime, Utc}; use net::vxlan::Vni; @@ -99,32 +96,6 @@ impl Display for VpcExpose { } } -impl Display for ValidatedExpose { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let mut carriage = false; - if self.is_default() { - write!(f, "{SEP} prefixes: default")?; - } - if !self.ips().is_empty() { - write!(f, "{SEP} prefixes:")?; - self.ips().iter().for_each(|x| { - let _ = write!(f, " {x}"); - }); - } - - writeln!(f)?; - - if let Some(nat) = self.nat() { - write!(f, "{SEP} as:")?; - self.as_range_or_empty().iter().for_each(|pfx| { - let _ = write!(f, " {pfx} proto: {:?} NAT:{}", nat.proto, nat.config); - }); - carriage = true; - } - if carriage { writeln!(f) } else { Ok(()) } - } -} - // Vpc manifest is common to VpcPeering and Peering fn fmt_local_manifest(f: &mut std::fmt::Formatter<'_>, manifest: &VpcManifest) -> std::fmt::Result { writeln!(f, " local:")?; @@ -161,45 +132,6 @@ impl Display for Peering { } } -// Vpc manifest is common to VpcPeering and Peering -fn fmt_local_validated_manifest( - f: &mut std::fmt::Formatter<'_>, - manifest: &ValidatedManifest, -) -> std::fmt::Result { - writeln!(f, " local:")?; - for e in manifest.valexp() { - e.fmt(f)?; - } - Ok(()) -} -fn fmt_remote_validated_manifest( - f: &mut std::fmt::Formatter<'_>, - manifest: &ValidatedManifest, - remote_id: &VpcId, - remote_vni: Vni, -) -> std::fmt::Result { - writeln!( - f, - " remote VPC is {} (id:{remote_id}) vni:{remote_vni} :", - manifest.name(), - )?; - for e in manifest.valexp() { - e.fmt(f)?; - } - Ok(()) -} - -impl Display for ValidatedPeering { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, " ■ {}:", self.name())?; - writeln!(f, " gwgroup: {}", self.gwgroup())?; - fmt_local_validated_manifest(f, self.local())?; - writeln!(f)?; - fmt_remote_validated_manifest(f, self.remote(), self.remote_id(), self.remote_vni())?; - writeln!(f) - } -} - /* ========= VPCs and peerings =========*/ macro_rules! VPC_TBL_FMT { @@ -286,70 +218,7 @@ impl Display for VpcSummary<'_> { } } -pub struct ValidatedVpcDetailed<'a>(pub &'a ValidatedVpc); -impl Display for ValidatedVpcDetailed<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let vpc = self.0; - Heading(format!( - "Peerings of VPC:{} Id:{} vni :{} ({})", - vpc.name(), - vpc.id(), - vpc.vni(), - vpc.peerings().len() - )) - .fmt(f)?; - for peering in vpc.peerings() { - peering.fmt(f)?; - } - Ok(()) - } -} - -pub struct ValidatedVpcSummary<'a>(&'a ValidatedVpc); -impl Display for ValidatedVpcSummary<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let vpc = self.0; - - // VPC that has no peerings - if vpc.peerings().is_empty() { - writeln!( - f, - "{}", - format_args!(VPC_TBL_FMT!(), &vpc.name(), vpc.id(), vpc.vni(), "", "", "") - )?; - } else { - // VPC that has peerings - for (num, peering) in vpc.peerings().iter().enumerate() { - let (name, id, vni, num_peers) = if num == 0 { - ( - vpc.name(), - vpc.id().to_string(), - vpc.vni().to_string(), - vpc.peerings().len().to_string(), - ) - } else { - ("", "".to_string(), "".to_string(), "".to_string()) - }; - writeln!( - f, - "{}", - format_args!( - VPC_TBL_FMT!(), - name, - id, - vni, - num_peers, - peering.remote().name(), - peering.name() - ) - )?; - } - } - Ok(()) - } -} - -pub struct VpcTableRoutingTables<'a>(&'a ValidatedVpcTable); +pub struct VpcTableRoutingTables<'a>(&'a VpcTable); impl Display for VpcTableRoutingTables<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { for vpc in self.0.values() { @@ -373,18 +242,6 @@ impl Display for VpcTableSummary<'_> { } } -pub struct ValidatedVpcTableSummary<'a>(&'a ValidatedVpcTable); -impl Display for ValidatedVpcTableSummary<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - Heading(format!("VPCs ({})", self.0.len())).fmt(f)?; - fmt_vpc_table_heading(f)?; - for vpc in self.0.values() { - vpc.as_summary().fmt(f)?; - } - Ok(()) - } -} - pub struct VpcTablePeerings<'a>(&'a VpcTable); impl Display for VpcTablePeerings<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -395,16 +252,6 @@ impl Display for VpcTablePeerings<'_> { } } -pub struct ValidatedVpcTablePeerings<'a>(&'a ValidatedVpcTable); -impl Display for ValidatedVpcTablePeerings<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for vpc in self.0.values() { - vpc.as_detailed().fmt(f)?; - } - Ok(()) - } -} - impl VpcTable { #[must_use] pub fn as_summary(&self) -> VpcTableSummary<'_> { @@ -414,17 +261,6 @@ impl VpcTable { pub fn as_peerings(&self) -> VpcTablePeerings<'_> { VpcTablePeerings(self) } -} - -impl ValidatedVpcTable { - #[must_use] - pub fn as_summary(&self) -> ValidatedVpcTableSummary<'_> { - ValidatedVpcTableSummary(self) - } - #[must_use] - pub fn as_peerings(&self) -> ValidatedVpcTablePeerings<'_> { - ValidatedVpcTablePeerings(self) - } #[must_use] pub fn as_route_tables(&self) -> VpcTableRoutingTables<'_> { VpcTableRoutingTables(self) @@ -442,17 +278,6 @@ impl Vpc { } } -impl ValidatedVpc { - #[must_use] - pub fn as_summary(&self) -> ValidatedVpcSummary<'_> { - ValidatedVpcSummary(self) - } - #[must_use] - pub fn as_detailed(&self) -> ValidatedVpcDetailed<'_> { - ValidatedVpcDetailed(self) - } -} - /* ===== VPC peerings as received via API =====*/ fn fmt_peering_manifest( @@ -492,12 +317,6 @@ impl Display for Overlay { } } -impl Display for ValidatedOverlay { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.vpc_table().as_summary().fmt(f) - } -} - /* ===== VPC routing tables =====*/ macro_rules! VPC_RT_TBL_FMT { diff --git a/config/src/external/mod.rs b/config/src/external/mod.rs index 7fa78c5996..15a03249b2 100644 --- a/config/src/external/mod.rs +++ b/config/src/external/mod.rs @@ -8,14 +8,13 @@ pub mod gwgroup; pub mod overlay; pub mod underlay; -use crate::ValidatedGwConfig; -use crate::external::overlay::vpc::ValidatedPeering; +use crate::external::overlay::vpc::Peering; use crate::internal::device::DeviceConfig; use crate::{ConfigError, ConfigResult}; use communities::PriorityCommunityTable; use derive_builder::Builder; use gwgroup::GwGroupTable; -use overlay::{Overlay, ValidatedOverlay}; +use overlay::Overlay; use std::collections::HashSet; use std::num::NonZero; use tracing::debug; @@ -46,11 +45,11 @@ impl ExternalConfig { Self { gwname: gwname.to_owned(), genid: Self::BLANK_GENID, - device: DeviceConfig::new(), + device: DeviceConfig::default(), underlay: Underlay::default(), overlay: Overlay::default(), - gwgroups: GwGroupTable::new(), - communities: PriorityCommunityTable::new(), + gwgroups: GwGroupTable::default(), + communities: PriorityCommunityTable::default(), flow_table_capacity: None, } } @@ -74,16 +73,24 @@ impl ExternalConfig { Ok(()) } - fn check_peering_gwgroups_exist<'a>( - &self, - peerings: impl Iterator, - ) -> ConfigResult { + fn check_peering_gwgroups_exist(&self) -> ConfigResult { // collect all distinct group names across all peerings - let groups: HashSet<_> = peerings - .into_iter() - .map(ValidatedPeering::gwgroup) + // Note: this would be faster using the overlay peering table, but + // we extract the peerings from the vpcs themselves + let groups: HashSet<_> = self + .overlay + .vpc_table() + .peerings() + .map(Peering::gwgroup) .collect(); - + /* + let groups: HashSet<_> = self + .overlay + .peering_table + .values() + .map(|p| p.gwgroup.clone()) + .collect(); + */ // check that they are present in the group table for group_name in groups { self.gwgroups @@ -93,40 +100,31 @@ impl ExternalConfig { Ok(()) } - /// Validate the external configuration. - /// This method consumes `ExternalConfig` and outputs a `ValidatedGwConfig` on success. + /// Validate and enrich the external configuration in place (validating the underlay and + /// overlay and collecting peerings into each VPC). + /// + /// To obtain the runtime [`GwConfig`], validate then wrap: `cfg.validate()?; GwConfig::new(cfg)`. /// /// # Errors /// /// Returns a [`ConfigError`] if validation fails. - pub fn validate(mut self) -> Result { + pub fn validate(&mut self) -> ConfigResult { debug!("Validating external config with genid {} ..", self.genid); self.device.validate()?; self.validate_gw_groups()?; - let underlay = self.underlay.validate()?; - let overlay = self.overlay.validate()?; - let peerings = overlay.vpc_table().peerings(); - self.check_peering_gwgroups_exist(peerings)?; + self.underlay.validate()?; + self.overlay.validate()?; + self.check_peering_gwgroups_exist()?; // if there are vpcs configured, there MUST be a vtep configured - if !overlay.vpc_table().is_empty() && underlay.vtep.is_none() { + if !self.overlay.vpc_table().is_empty() && self.underlay.vtep.is_none() { return Err(ConfigError::MissingParameter( "Vtep interface configuration", )); } - let validated_external = ValidatedExternalConfig { - gwname: self.gwname, - genid: self.genid, - device: self.device, - underlay, - overlay, - gwgroups: self.gwgroups, - communities: self.communities, - flow_table_capacity: self.flow_table_capacity, - }; - debug!("Community table:\n{}", validated_external.communities()); - debug!("Gateway-groups are:\n{}", validated_external.gwgroups); - Ok(ValidatedGwConfig::new(validated_external)) + debug!("Community table:\n{}", self.communities); + debug!("Gateway-groups are:\n{}", self.gwgroups); + Ok(()) } /// FOR TESTS ONLY. Fake validation for the external config. @@ -141,48 +139,11 @@ impl ExternalConfig { #[cfg(feature = "testing")] #[allow(unsafe_code)] #[must_use] - pub unsafe fn fake_validated_external_for_tests(self) -> ValidatedExternalConfig { + pub unsafe fn fake_validated_external_for_tests(mut self) -> ExternalConfig { #[allow(clippy::unwrap_used)] - let validated_underlay = self.underlay.validate().unwrap(); - let fake_valid_overlay = unsafe { self.overlay.fake_validated_overlay_for_tests() }; - ValidatedExternalConfig { - gwname: self.gwname, - genid: self.genid, - device: self.device, - underlay: validated_underlay, - overlay: fake_valid_overlay, - gwgroups: self.gwgroups, - communities: self.communities, - flow_table_capacity: self.flow_table_capacity, - } - } -} - -#[derive(Debug)] -pub struct ValidatedExternalConfig { - gwname: String, /* name of gateway */ - genid: GenId, /* configuration generation id (version) */ - device: DeviceConfig, /* goes as-is into the internal config */ - underlay: Underlay, /* goes as-is into the internal config */ - overlay: ValidatedOverlay, /* VPCs and peerings -- get highly developed in internal config */ - gwgroups: GwGroupTable, /* gateway group table */ - communities: PriorityCommunityTable, /* priority-to-community table */ - flow_table_capacity: Option>, /* optional hard cap of flow table */ -} - -impl ValidatedExternalConfig { - #[must_use] - pub(crate) fn blank() -> Self { - Self { - gwname: String::new(), - genid: ExternalConfig::BLANK_GENID, - device: DeviceConfig::new(), - underlay: Underlay::default(), - overlay: ValidatedOverlay::default(), - gwgroups: GwGroupTable::new(), - communities: PriorityCommunityTable::new(), - flow_table_capacity: None, - } + self.underlay.validate().unwrap(); + self.overlay = unsafe { self.overlay.fake_validated_overlay_for_tests() }; + self } #[must_use] @@ -206,7 +167,7 @@ impl ValidatedExternalConfig { } #[must_use] - pub fn overlay(&self) -> &ValidatedOverlay { + pub fn overlay(&self) -> &Overlay { &self.overlay } diff --git a/config/src/external/overlay/mod.rs b/config/src/external/overlay/mod.rs index 1a84dd0db0..ccce591b46 100644 --- a/config/src/external/overlay/mod.rs +++ b/config/src/external/overlay/mod.rs @@ -11,7 +11,7 @@ pub mod vpcrouting; use crate::{ConfigError, ConfigResult}; use tracing::{debug, error}; -use vpc::{ValidatedVpcTable, VpcTable}; +use vpc::VpcTable; use vpcpeering::{VpcManifest, VpcPeeringTable}; #[derive(Clone, Debug, Default)] @@ -48,28 +48,25 @@ impl Overlay { Ok(()) } - /// Validate the overlay configuration, returning a `ValidatedOverlay` if successful. + /// Validate the overlay configuration, returning it with the VPC table validated and the + /// peerings collected into each VPC. /// /// # Errors /// /// Returns an error if the overlay configuration is invalid. - pub fn validate(&self) -> Result { + pub fn validate(&mut self) -> ConfigResult { debug!("Validating overlay configuration..."); // validate peerings: self.validate_peering_vpcs()?; // Collect peerings for every VPC and validate the table - let vpc_table = self - .vpc_table - .collect_peerings(&self.peering_table) - .validate()?; + let mut vpc_table = self.vpc_table.collect_peerings(&self.peering_table); + vpc_table.validate()?; + self.vpc_table = vpc_table; - let validated_overlay = ValidatedOverlay { vpc_table }; - - let peering_table = &self.peering_table; - debug!("Overlay configuration is VALID:\n{validated_overlay}\n{peering_table}"); - Ok(validated_overlay) + debug!("Overlay configuration is VALID:\n{self}"); + Ok(()) } /// FOR TESTS ONLY. Fake validation for the overlay. @@ -80,23 +77,17 @@ impl Overlay { #[cfg(feature = "testing")] #[allow(unsafe_code)] #[must_use] - pub unsafe fn fake_validated_overlay_for_tests(&self) -> ValidatedOverlay { + pub unsafe fn fake_validated_overlay_for_tests(&self) -> Overlay { let vpc_table = self.vpc_table.collect_peerings(&self.peering_table); let fake_valid_vpc_table = unsafe { vpc_table.fake_validated_vpc_table_for_tests() }; - ValidatedOverlay { + Overlay { vpc_table: fake_valid_vpc_table, + peering_table: self.peering_table.clone(), } } -} - -#[derive(Debug, Default)] -pub struct ValidatedOverlay { - vpc_table: ValidatedVpcTable, -} -impl ValidatedOverlay { #[must_use] - pub fn vpc_table(&self) -> &ValidatedVpcTable { + pub fn vpc_table(&self) -> &VpcTable { &self.vpc_table } } diff --git a/config/src/external/overlay/tests.rs b/config/src/external/overlay/tests.rs index 0266039d9c..518af7d915 100644 --- a/config/src/external/overlay/tests.rs +++ b/config/src/external/overlay/tests.rs @@ -104,25 +104,25 @@ pub mod test { fn test_expose_validate() { let expose = VpcExpose::empty(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::Forbidden( "Non-default expose cannot have empty 'ips' list" )) ); let expose = VpcExpose::empty().ip("10.0.0.0/16".into()); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Empty ips but non-empty nots - Currently not supported /* let expose = VpcExpose::empty().not("10.0.1.0/24".into()); - assert_eq!(expose.validate(), Ok(())); + assert_eq!(expose.clone().validate(), Ok(())); */ // Empty as_range but non-empty not_as - Currently not supported /* let expose = VpcExpose::empty().not_as("2.0.1.0/24".into()); - assert_eq!(expose.validate(), Ok(())); + assert_eq!(expose.clone().validate(), Ok(())); */ let expose = VpcExpose::empty() @@ -131,7 +131,7 @@ pub mod test { .ip("10.0.0.0/16".into()) .as_range("2.0.0.0/16".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); let expose = VpcExpose::empty() .make_static_nat() @@ -142,7 +142,7 @@ pub mod test { .unwrap() .not_as("2.0.0.0/24".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); let expose = VpcExpose::empty() .make_static_nat() @@ -150,7 +150,7 @@ pub mod test { .ip("1::/64".into()) .as_range("2::/64".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Overlapping prefixes let expose = VpcExpose::empty() @@ -162,7 +162,7 @@ pub mod test { .unwrap() .as_range("2.0.0.0/16".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Out-of-range exclusion prefix let expose = VpcExpose::empty() @@ -174,7 +174,7 @@ pub mod test { .unwrap() .not_as("2.0.1.0/24".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Incorrect: mixed IP versions let expose = VpcExpose::empty() @@ -187,7 +187,7 @@ pub mod test { .as_range("2::/64".into()) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::InconsistentIpVersion(Box::new(expose.clone()))) ); @@ -199,7 +199,7 @@ pub mod test { .as_range("1::/112".into()) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::InconsistentIpVersion(Box::new(expose.clone()))) ); @@ -214,7 +214,7 @@ pub mod test { .not_as("2::/120".into()) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::InconsistentIpVersion(Box::new(expose.clone()))) ); @@ -232,7 +232,7 @@ pub mod test { .not_as("2.0.128.0/17".into()) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::ExcludedAllPrefixes(Box::new(expose.clone()))) ); @@ -245,7 +245,7 @@ pub mod test { .as_range("2.0.0.0/24".into()) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::MismatchedPrefixSizes( ppsize_from((65536 - 256u32) * (u32::from(u16::MAX) + 1)), ppsize_from(256u32 * (u32::from(u16::MAX) + 1)), @@ -374,7 +374,7 @@ pub mod test { // Build overlay object and validate it let overlay = Overlay::new(vpc_table, peering_table); assert_eq!( - overlay.validate().map(|_| ()), + overlay.clone().validate(), Err(ConfigError::IncompatibleNatModes("Peering-1".to_owned())) ); } @@ -421,7 +421,7 @@ pub mod test { // Build overlay object and validate it let overlay = Overlay::new(vpc_table, peering_table); - assert!(overlay.validate().is_ok()); + assert!(overlay.clone().validate().is_ok()); } #[test] @@ -441,7 +441,7 @@ pub mod test { peering_table.add(peering).expect("Should succeed"); /* build overlay object and validate it */ - let overlay = Overlay::new(vpc_table, peering_table); + let mut overlay = Overlay::new(vpc_table, peering_table); assert!( overlay .validate() @@ -476,7 +476,7 @@ pub mod test { peering_table.add(peering2).expect("Should succeed"); /* build overlay object and validate it */ - let overlay = Overlay::new(vpc_table, peering_table); + let mut overlay = Overlay::new(vpc_table, peering_table); assert!( overlay .validate() @@ -506,7 +506,7 @@ pub mod test { /* build overlay object and validate it */ let overlay = Overlay::new(vpc_table, peering_table); - assert!(overlay.validate().is_ok()); + assert!(overlay.clone().validate().is_ok()); } #[test] @@ -707,7 +707,7 @@ pub mod test { "10.0.0.0/16".into(), Some(PortRange::new(1, 65535).unwrap()), )); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); let expose = VpcExpose::empty() .make_static_nat() @@ -721,7 +721,7 @@ pub mod test { Some(PortRange::new(8001, 9000).unwrap()), )) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); let expose = VpcExpose::empty() .make_static_nat() @@ -744,7 +744,7 @@ pub mod test { Some(PortRange::new(8001, 8200).unwrap()), )) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); let expose = VpcExpose::empty() .make_static_nat() @@ -758,7 +758,7 @@ pub mod test { Some(PortRange::new(8001, 9000).unwrap()), )) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Overlapping prefix, but distinct port ranges let expose = VpcExpose::empty() @@ -770,7 +770,7 @@ pub mod test { "10.0.0.0/17".into(), Some(PortRange::new(8001, 9500).unwrap()), )); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Overlapping prefixes let expose = VpcExpose::empty() @@ -794,7 +794,7 @@ pub mod test { Some(PortRange::new(8001, 8500).unwrap()), )) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Out-of-range exclusion prefix (IPs) let expose = VpcExpose::empty() @@ -810,7 +810,7 @@ pub mod test { "10.0.0.0/15".into(), Some(PortRange::new(5001, 5500).unwrap()), )); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Out-of-range exclusion prefix (port range) let expose = VpcExpose::empty() @@ -822,7 +822,7 @@ pub mod test { "10.0.0.0/24".into(), Some(PortRange::new(7001, 8000).unwrap()), )); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Out-of-range exclusion prefix (port range, albeit with overlap) let expose = VpcExpose::empty() @@ -834,7 +834,7 @@ pub mod test { "10.0.0.0/24".into(), Some(PortRange::new(5001, 8000).unwrap()), )); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Incorrect: mixed IP versions let expose = VpcExpose::empty() @@ -859,7 +859,7 @@ pub mod test { )) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::InconsistentIpVersion(Box::new(expose.clone()))) ); @@ -877,7 +877,7 @@ pub mod test { )) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::InconsistentIpVersion(Box::new(expose.clone()))) ); @@ -900,7 +900,7 @@ pub mod test { Some(PortRange::new(5001, 6000).unwrap()), )); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::ExcludedAllPrefixes(Box::new(expose.clone()))) ); @@ -922,7 +922,7 @@ pub mod test { )) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::MismatchedPrefixSizes( ppsize_from(65536u32 * 1000 - 256u32 * 500), ppsize_from(256u32 * 1000), @@ -940,7 +940,7 @@ pub mod test { .as_range(PrefixWithOptionalPorts::new("2.0.0.0/24".into(), None)) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::Forbidden( "Port ranges are not supported with masquerade", )) @@ -957,7 +957,7 @@ pub mod test { )) .unwrap(); assert_eq!( - expose.validate(), + expose.clone().validate(), Err(ConfigError::Forbidden( "Port ranges are not supported with masquerade", )) @@ -1018,7 +1018,7 @@ pub mod test { .unwrap(); let overlay = Overlay::new(vpc_table, peering_table); - assert!(overlay.validate().is_err_and(|e| e + assert!(overlay.clone().validate().is_err_and(|e| e == ConfigError::OverlappingPrefixes( PrefixWithOptionalPorts::new( "5.0.0.0/24".into(), @@ -1080,7 +1080,7 @@ pub mod test { .unwrap(); let overlay = Overlay::new(vpc_table, peering_table); - assert!(overlay.validate().is_ok()); + assert!(overlay.clone().validate().is_ok()); } #[test] @@ -1125,7 +1125,7 @@ pub mod test { .unwrap(); let overlay = Overlay::new(vpc_table, peering_table); - assert!(overlay.validate().is_ok()); + assert!(overlay.clone().validate().is_ok()); } #[test] @@ -1170,7 +1170,7 @@ pub mod test { .unwrap(); let overlay = Overlay::new(vpc_table, peering_table); - assert!(overlay.validate().is_err_and( + assert!(overlay.clone().validate().is_err_and( |e| e == ConfigError::Forbidden("Multiple default destinations exposed to VPC") )); } @@ -1216,14 +1216,14 @@ pub mod test { .unwrap(); let overlay = Overlay::new(vpc_table, peering_table); - assert!(overlay.validate().is_err_and( + assert!(overlay.clone().validate().is_err_and( |e| e == ConfigError::Forbidden("Manifest cannot have multiple default exposes",) )); } #[test] fn test_manifest_must_have_exposes() { - let manifest = VpcManifest::new("some-vpc"); + let mut manifest = VpcManifest::new("some-vpc"); assert!( manifest .validate() diff --git a/config/src/external/overlay/validation_tests.rs b/config/src/external/overlay/validation_tests.rs index 31835d08d0..380d390d17 100644 --- a/config/src/external/overlay/validation_tests.rs +++ b/config/src/external/overlay/validation_tests.rs @@ -38,7 +38,7 @@ mod test { peering_table.add(peering).unwrap(); let overlay = Overlay::new(vpc_table, peering_table); - overlay.validate().map(|_| ()) + overlay.clone().validate() } // Helper: build an Overlay from three VPCs and two peerings, then validate it @@ -59,7 +59,7 @@ mod test { peering_table.add(peering2).unwrap(); let overlay: Overlay = Overlay::new(vpc_table, peering_table); - overlay.validate().map(|_| ()) + overlay.clone().validate() } // ================================================================================== @@ -72,7 +72,7 @@ mod test { #[test] fn test_empty_expose_rejected() { let expose = VpcExpose::empty(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::Forbidden(_))), "{result:?}" @@ -83,7 +83,7 @@ mod test { #[test] fn test_empty_ips_with_nonempty_nots_rejected() { let expose = VpcExpose::empty().not("10.0.1.0/24".into()); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::Forbidden(_))), "{result:?}" @@ -99,7 +99,7 @@ mod test { .ip("10.0.0.0/16".into()) .not_as("2.0.1.0/24".into()) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::Forbidden(_))), "{result:?}" @@ -113,7 +113,7 @@ mod test { .make_static_nat() .unwrap() .ip("10.0.0.0/24".into()); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::Forbidden(_))), "{result:?}" @@ -127,7 +127,7 @@ mod test { #[ignore = "TODO: validation for reserved IPs not yet implemented"] fn test_reserved_ipv4_zero_rejected() { let expose = VpcExpose::empty().ip("0.0.0.0/32".into()); - assert!(expose.validate().is_err()); + assert!(expose.clone().validate().is_err()); } // Reserved IP ::/128 in ips should be rejected @@ -135,7 +135,7 @@ mod test { #[ignore = "TODO: validation for reserved IPs not yet implemented"] fn test_reserved_ipv6_zero_rejected() { let expose = VpcExpose::empty().ip("::/128".into()); - assert!(expose.validate().is_err()); + assert!(expose.clone().validate().is_err()); } // Reserved IP 255.255.255.255/32 in as_range should be rejected @@ -146,7 +146,7 @@ mod test { .ip("10.0.0.1/32".into()) .as_range("255.255.255.255/32".into()) .unwrap(); - assert!(expose.validate().is_err()); + assert!(expose.clone().validate().is_err()); } // Multicast prefix 224.0.0.0/4 in ips should be rejected @@ -154,7 +154,7 @@ mod test { #[ignore = "TODO: validation for multicast prefixes not yet implemented"] fn test_multicast_prefix_rejected() { let expose = VpcExpose::empty().ip("224.0.0.0/4".into()); - assert!(expose.validate().is_err()); + assert!(expose.clone().validate().is_err()); } // Loopback prefix 127.0.0.0/8 in ips should be rejected @@ -162,14 +162,14 @@ mod test { #[ignore = "TODO: validation for loopback prefixes not yet implemented"] fn test_loopback_prefix_rejected() { let expose = VpcExpose::empty().ip("127.0.0.0/8".into()); - assert!(expose.validate().is_err()); + assert!(expose.clone().validate().is_err()); } // Port 0 in port range should be rejected #[test] fn test_port_zero_rejected() { let expose = VpcExpose::empty().ip(prefix_with_ports("10.0.0.0/24", 0, 80)); - assert!(expose.validate().is_err()); + assert!(expose.clone().validate().is_err()); } // --- 0.0.0.0/0 and ::/0 prefixes --- @@ -181,14 +181,14 @@ mod test { #[test] fn test_root_v4_in_ips_passes() { let expose = VpcExpose::empty().ip("0.0.0.0/0".into()); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Root prefix ::/0 in ips is legal (IPv6 variant) #[test] fn test_root_v6_in_ips_passes() { let expose = VpcExpose::empty().ip("::/0".into()); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Root prefix 0.0.0.0/0 in as_range is legal @@ -200,7 +200,7 @@ mod test { .ip("10.0.0.0/8".into()) .as_range("0.0.0.0/0".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Root prefix 0.0.0.0/0 in nots is rejected - not illegal per-se, but excludes all available @@ -210,7 +210,7 @@ mod test { let expose = VpcExpose::empty() .ip("10.0.0.0/8".into()) .not("0.0.0.0/0".into()); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::ExcludedAllPrefixes(_))), "{result:?}" @@ -228,7 +228,7 @@ mod test { .unwrap() .not_as("0.0.0.0/0".into()) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::ExcludedAllPrefixes(_))), "{result:?}" @@ -243,7 +243,7 @@ mod test { let expose = VpcExpose::empty() .ip("10.0.0.0/16".into()) .ip("1::/64".into()); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::InconsistentIpVersion(_))), "{result:?}" @@ -260,7 +260,7 @@ mod test { .ip("10.0.0.0/16".into()) .as_range("1::/112".into()) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::InconsistentIpVersion(_))), "{result:?}" @@ -276,7 +276,7 @@ mod test { let expose = VpcExpose::empty() .ip("10.0.0.0/16".into()) .ip("10.0.0.0/17".into()); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Overlapping prefixes within as_range are allowed, should be merged internally @@ -291,7 +291,7 @@ mod test { .unwrap() .as_range("10.0.0.0/17".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // TODO: Can we merge the two overlapping prefixes? } @@ -303,7 +303,7 @@ mod test { .ip("10.0.0.0/8".into()) .not("10.0.0.0/16".into()) .not("10.0.0.0/17".into()); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Overlapping prefixes within not_as are allowed, should be merged internally @@ -320,7 +320,7 @@ mod test { .unwrap() .not_as("10.0.0.0/17".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Overlapping prefixes in ips with distinct port ranges passes @@ -329,7 +329,7 @@ mod test { let expose = VpcExpose::empty() .ip(prefix_with_ports("10.0.0.0/24", 80, 80)) .ip(prefix_with_ports("10.0.0.0/24", 443, 443)); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Overlapping prefixes in ips with overlapping port ranges passes @@ -338,7 +338,7 @@ mod test { let expose = VpcExpose::empty() .ip(prefix_with_ports("10.0.0.0/24", 80, 80)) .ip(prefix_with_ports("10.0.0.0/24", 80, 80)); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // --- Exclusion prefixes --- @@ -349,7 +349,7 @@ mod test { let expose = VpcExpose::empty() .ip("10.0.0.0/16".into()) .not("8.0.0.0/24".into()); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Out-of-range exclusion prefix for as_range is legal (but we should warn about it) @@ -363,7 +363,7 @@ mod test { .unwrap() .not_as("8.0.0.0/24".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Exclusion prefix for ips with partial overlap (not fully contained) is valid (but we should @@ -382,7 +382,7 @@ mod test { .ip("20.0.0.0/16".into()) .ip("10.0.0.0/16".into()) .not("10.0.0.0/8".into()); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Exclusion prefix for ips with partial overlap (not fully contained), when using port ranges, @@ -399,7 +399,7 @@ mod test { "10.0.0.0/16".into(), Some(PortRange::new(1500, 2500).unwrap()), )); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Exclusion prefix for as_range with partial overlap (not fully contained) is valid (but we @@ -424,7 +424,7 @@ mod test { .unwrap() .not_as("10.0.0.0/8".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Exclusion prefix for as_range with partial overlap (not fully contained) is valid (but we @@ -446,7 +446,7 @@ mod test { Some(PortRange::new(1500, 2500).unwrap()), )) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Excluding all prefixes in ips is rejected @@ -456,7 +456,7 @@ mod test { .ip("10.0.0.0/16".into()) .not("10.0.0.0/17".into()) .not("10.0.128.0/17".into()); - let result = expose.validate(); + let result = expose.clone().validate(); assert_eq!( result, Err(ConfigError::ExcludedAllPrefixes(Box::new(expose.clone()))), @@ -477,7 +477,7 @@ mod test { .unwrap() .not_as("10.0.128.0/17".into()) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert_eq!( result, Err(ConfigError::ExcludedAllPrefixes(Box::new(expose.clone()))), @@ -497,7 +497,7 @@ mod test { .not("10.0.1.0/24".into()) .as_range("2.0.0.0/24".into()) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert_eq!( result, Err(ConfigError::MismatchedPrefixSizes( @@ -519,7 +519,7 @@ mod test { .ip(prefix_with_ports("10.0.0.2/32", 80, 80)) .as_range(prefix_with_ports("2.0.0.1/32", 8080, 8080)) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::Forbidden(_))), "{result:?}" @@ -536,7 +536,7 @@ mod test { .not(prefix_with_ports("10.0.0.1/32", 80, 80)) .as_range(prefix_with_ports("2.0.0.0/31", 8080, 8080)) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::Forbidden(_))), "{result:?}", @@ -552,7 +552,7 @@ mod test { .ip(prefix_with_ports("10.0.0.0/24", 80, 80)) .as_range(prefix_with_ports("2.0.0.0/25", 8080, 8080)) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::MismatchedPrefixSizes(_, _))), "{result:?}", @@ -571,7 +571,7 @@ mod test { // ranges are normalized to `None`. So a check on start would be skipped without the validate assert!(expose.ips.iter().all(|p| p.ports().is_none())); - let result = expose.validate(); + let result = expose.clone().validate(); assert!(result.is_err_and(|e| matches!(e, ConfigError::Forbidden(_)))); } @@ -584,7 +584,7 @@ mod test { .ip(prefix_with_ports("10.0.0.0/24", 80, 80)) .as_range("2.0.0.0/24".into()) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!( matches!(result, Err(ConfigError::Forbidden(_))), "{result:?}" @@ -595,7 +595,7 @@ mod test { #[test] fn test_default_expose_with_ips_rejected() { let expose = VpcExpose::empty().set_default().ip("10.0.0.0/16".into()); - let result = expose.validate(); + let result = expose.clone().validate(); assert!(matches!(result, Err(ConfigError::Invalid(_))), "{result:?}"); let expose = VpcExpose::empty() @@ -604,11 +604,11 @@ mod test { .unwrap() .as_range("10.0.0.0/16".into()) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!(matches!(result, Err(ConfigError::Invalid(_))), "{result:?}"); let expose = VpcExpose::empty().set_default().not("10.0.0.0/16".into()); - let result = expose.validate(); + let result = expose.clone().validate(); assert!(matches!(result, Err(ConfigError::Invalid(_))), "{result:?}"); let expose = VpcExpose::empty() @@ -617,7 +617,7 @@ mod test { .unwrap() .not_as("10.0.0.0/16".into()) .unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!(matches!(result, Err(ConfigError::Invalid(_))), "{result:?}"); } @@ -627,7 +627,7 @@ mod test { let expose = VpcExpose::empty() .ip("10.0.0.0/16".into()) .ip("10.1.0.0/16".into()); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // Valid expose with ips + as_range + nots + not_as passes @@ -642,7 +642,7 @@ mod test { .unwrap() .not_as("2.0.1.0/24".into()) .unwrap(); - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); } // ================================================================================== @@ -655,7 +655,7 @@ mod test { let mut manifest = VpcManifest::new("VPC-1"); manifest.add_expose(VpcExpose::empty().ip("10.0.0.0/16".into())); manifest.add_expose(VpcExpose::empty().ip("10.1.0.0/16".into())); - assert!(manifest.validate().is_ok()); + assert!(manifest.clone().validate().is_ok()); } // Two no-NAT exposes with overlapping ips rejected @@ -664,7 +664,7 @@ mod test { let mut manifest = VpcManifest::new("VPC-1"); manifest.add_expose(VpcExpose::empty().ip("10.0.0.0/16".into())); manifest.add_expose(VpcExpose::empty().ip("10.0.1.0/24".into())); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -684,7 +684,7 @@ mod test { .as_range("2.0.0.0/16".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -704,7 +704,7 @@ mod test { .as_range("2.0.0.0/16".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -724,7 +724,7 @@ mod test { .as_range("2.0.0.0/16".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -744,7 +744,7 @@ mod test { .as_range("2.0.0.0/16".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -764,7 +764,7 @@ mod test { .as_range(prefix_with_ports("2.0.0.1/32", 8080, 8080)) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -784,7 +784,7 @@ mod test { .as_range(prefix_with_ports("2.0.0.1/32", 8080, 8080)) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -811,7 +811,7 @@ mod test { .as_range("2.1.0.0/16".into()) .unwrap(), ); - assert!(manifest.validate().is_ok()); + assert!(manifest.clone().validate().is_ok()); } // Two static NAT exposes with overlapping ips rejected @@ -834,7 +834,7 @@ mod test { .as_range("3.0.0.0/16".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -861,7 +861,7 @@ mod test { .as_range("2.0.0.0/16".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -888,7 +888,7 @@ mod test { .as_range("3.0.0.0/16".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -915,7 +915,7 @@ mod test { .as_range("2.0.0.0/16".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -942,7 +942,7 @@ mod test { .as_range(prefix_with_ports("3.0.0.1/32", 8080, 8080)) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -969,7 +969,7 @@ mod test { .as_range(prefix_with_ports("2.0.0.1/32", 8080, 8080)) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -996,7 +996,7 @@ mod test { .as_range("3.0.0.0/24".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -1023,7 +1023,7 @@ mod test { .as_range("2.0.1.0/24".into()) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -1050,7 +1050,7 @@ mod test { .as_range(prefix_with_ports("3.0.0.1/32", 8080, 8080)) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -1077,7 +1077,7 @@ mod test { .as_range(prefix_with_ports("2.0.0.1/32", 8080, 8080)) .unwrap(), ); - let result = manifest.validate(); + let result = manifest.clone().validate(); assert!( matches!(result, Err(ConfigError::OverlappingPrefixes(_, _))), "{result:?}", @@ -1104,7 +1104,7 @@ mod test { .as_range(prefix_with_ports("2.0.0.1/32", 9090, 9090)) .unwrap(), ); - assert!(manifest.validate().is_ok()); + assert!(manifest.clone().validate().is_ok()); } // Masquerade + port forwarding overlap where masquerade contains port forwarding passes @@ -1129,7 +1129,7 @@ mod test { .as_range(prefix_with_ports("2.0.0.1/32", 8080, 8080)) .unwrap(), ); - assert!(manifest.validate().is_ok()); + assert!(manifest.clone().validate().is_ok()); } // Masquerade + port forwarding partial overlap passes @@ -1154,7 +1154,7 @@ mod test { .as_range(prefix_with_ports("3.0.0.0/24", 8080, 8080)) .unwrap(), ); - assert!(manifest.validate().is_ok()); + assert!(manifest.clone().validate().is_ok()); } // ================================================================================== @@ -1424,7 +1424,7 @@ mod test { peering_table.add(peering2).unwrap(); let overlay = Overlay::new(vpc_table, peering_table); - let result = overlay.validate(); + let result = overlay.clone().validate(); assert!( matches!(result, Err(ConfigError::DuplicateVpcPeerings(_))), "{result:?}", @@ -1571,12 +1571,12 @@ mod test { // A default expose cannot have nat field set at all let expose = VpcExpose::empty().set_default(); // Verify default alone is valid - assert!(expose.validate().is_ok()); + assert!(expose.clone().validate().is_ok()); // Default with NAT should fail let expose = VpcExpose::empty().set_default().make_static_nat().unwrap(); - let result = expose.validate(); + let result = expose.clone().validate(); assert!(matches!(result, Err(ConfigError::Invalid(_))), "{result:?}"); } diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index ad7cc4fde5..53eb6acc0f 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -13,7 +13,6 @@ use tracing::{debug, error, warn}; use crate::external::overlay::VpcManifest; use crate::external::overlay::VpcPeeringTable; -use crate::external::overlay::vpcpeering::ValidatedManifest; use crate::external::overlay::vpcpeering::VpcExposeNatConfig; use crate::external::overlay::vpcrouting::VpcRouteTable; use crate::internal::interfaces::interface::InterfaceConfigTable; @@ -36,7 +35,12 @@ pub struct Peering { } impl Peering { - pub fn validate(&self) -> Result { + /// Validate this [`Peering`] (and its manifests) in place. + /// + /// # Errors + /// + /// Returns an error if the peering configuration is invalid. + pub fn validate(&mut self) -> ConfigResult { debug!( "Validating manifest of VPC {} in peering {}", self.local.name, self.name @@ -48,17 +52,9 @@ impl Peering { )); } - let valid_peering_candidate = ValidatedPeering { - name: self.name.clone(), - local: self.local.validate()?, - remote: self.remote.validate()?, - remote_id: self.remote_id.clone(), - remote_vni: self.remote_vni, - gwgroup: self.gwgroup.clone(), - }; - valid_peering_candidate.validate_nat_combinations()?; - - Ok(valid_peering_candidate) + self.local.validate()?; + self.remote.validate()?; + self.validate_nat_combinations() } /// FOR TESTS ONLY. Fake validation for a VPC peering. @@ -69,14 +65,14 @@ impl Peering { #[cfg(feature = "testing")] #[allow(unsafe_code)] #[must_use] - pub unsafe fn fake_validated_peering_for_tests(&self) -> ValidatedPeering { + pub unsafe fn fake_validated_peering_for_tests(&self) -> Peering { let (fake_local, fake_remote) = unsafe { ( self.local.fake_valid_manifest_for_tests(), self.remote.fake_valid_manifest_for_tests(), ) }; - ValidatedPeering { + Peering { name: self.name.clone(), local: fake_local, remote: fake_remote, @@ -85,31 +81,19 @@ impl Peering { gwgroup: self.gwgroup.clone(), } } -} - -#[derive(Clone, Debug, PartialEq)] -pub struct ValidatedPeering { - name: String, /* name of peering */ - local: ValidatedManifest, /* local manifest */ - remote: ValidatedManifest, /* remote manifest */ - remote_id: VpcId, /* Id of peer */ - remote_vni: Vni, /* Vni of peer -- should be vpc discriminant in future */ - gwgroup: String, /* gateway group serving this peering */ -} -impl ValidatedPeering { #[must_use] pub fn name(&self) -> &str { &self.name } #[must_use] - pub fn local(&self) -> &ValidatedManifest { + pub fn local(&self) -> &VpcManifest { &self.local } #[must_use] - pub fn remote(&self) -> &ValidatedManifest { + pub fn remote(&self) -> &VpcManifest { &self.remote } @@ -218,6 +202,8 @@ pub struct Vpc { pub vni: Vni, /* mandatory */ pub interfaces: InterfaceConfigTable, /* user-defined interfaces in this VPC */ pub peerings: Vec, /* peerings of this VPC (collected) */ + /// Route table towards remote VPCs. Empty until `validate()` builds it. + rt: VpcRouteTable, } impl Vpc { pub fn new(name: &str, id: &str, vni: u32) -> Result { @@ -228,6 +214,7 @@ impl Vpc { vni, interfaces: InterfaceConfigTable::new(), peerings: vec![], + rt: VpcRouteTable::default(), }) } @@ -272,33 +259,29 @@ impl Vpc { Ok(()) } - /// Validate a [`Vpc`] and produce a [`ValidatedVpc`] if it passes validation. + /// Validate a [`Vpc`], returning the enriched (peerings validated, route table built) VPC. /// /// # Errors /// /// Returns an error if the VPC configuration is invalid. - pub fn validate(&self) -> Result { + /// Validate this [`Vpc`] in place, validating its peerings and building its route table. + /// + /// # Errors + /// + /// Returns an error if the VPC configuration is invalid. + pub fn validate(&mut self) -> ConfigResult { debug!("Validating config for VPC {}...", self.name); self.check_peering_count()?; debug!("Checking peerings of VPC {}...", self.name); - let validated_peerings: Vec = self - .peerings - .iter() - .map(Peering::validate) - .collect::>()?; + for peering in &mut self.peerings { + peering.validate()?; + } - let rt = VpcRouteTable::build(&validated_peerings).validate()?; + self.rt = VpcRouteTable::build(&self.peerings); + self.rt.validate()?; - let validated_vpc = ValidatedVpc { - name: self.name.clone(), - id: self.id.clone(), - vni: self.vni, - interfaces: self.interfaces.clone(), - peerings: validated_peerings, - rt, - }; - Ok(validated_vpc) + Ok(()) } /// FOR TESTS ONLY. Fake validation for the VPC peering manifests. @@ -309,7 +292,7 @@ impl Vpc { #[cfg(feature = "testing")] #[allow(unsafe_code)] #[must_use] - pub unsafe fn fake_validated_vpc_for_tests(&self) -> ValidatedVpc { + pub unsafe fn fake_validated_vpc_for_tests(&self) -> Vpc { let fake_validated_peerings = self .peerings .iter() @@ -320,7 +303,7 @@ impl Vpc { peering.remote.fake_valid_manifest_for_tests(), ) }; - ValidatedPeering { + Peering { name: peering.name.clone(), local: fake_local, remote: fake_remote, @@ -333,7 +316,7 @@ impl Vpc { let not_validated_rt = VpcRouteTable::build(&fake_validated_peerings); - ValidatedVpc { + Vpc { name: self.name.clone(), id: self.id.clone(), vni: self.vni, @@ -342,19 +325,7 @@ impl Vpc { rt: not_validated_rt, } } -} - -#[derive(Debug)] -pub struct ValidatedVpc { - name: String, /* name of vpc, used as key */ - id: VpcId, /* internal Id, unique*/ - vni: Vni, /* mandatory */ - interfaces: InterfaceConfigTable, /* user-defined interfaces in this VPC */ - peerings: Vec, /* peerings of this VPC - NOT set via gRPC */ - rt: VpcRouteTable, -} -impl ValidatedVpc { #[must_use] pub fn name(&self) -> &str { &self.name @@ -376,7 +347,7 @@ impl ValidatedVpc { } #[must_use] - pub fn peerings(&self) -> &[ValidatedPeering] { + pub fn peerings(&self) -> &[Peering] { &self.peerings } @@ -393,7 +364,7 @@ impl ValidatedVpc { /// Provide an iterator over all peerings that have either masquerade or port-forwarding /// exposes locally. - pub fn local_stateful_nat_peerings(&self) -> impl Iterator { + pub fn local_stateful_nat_peerings(&self) -> impl Iterator { self.peerings().iter().filter(|p| { p.local() .valexp() @@ -486,21 +457,16 @@ impl VpcTable { new_table } - /// Validate the [`VpcTable`] and produce a [`ValidatedVpcTable`] if it passes validation. + /// Validate the [`VpcTable`], returning it with every [`Vpc`] validated and enriched. /// /// # Errors /// /// Returns an error if any [`Vpc`] fails validation. - pub fn validate(&self) -> Result { - let validated_vpcs = self - .vpcs - .iter() - .map(|(name, vpc)| vpc.validate().map(|vpc| (name.clone(), vpc))) - .collect::>()?; - Ok(ValidatedVpcTable { - vpcs: validated_vpcs, - ids: self.ids.clone(), - }) + pub fn validate(&mut self) -> ConfigResult { + for vpc in self.vpcs.values_mut() { + vpc.validate()?; + } + Ok(()) } /// FOR TESTS ONLY. Fake validation for the VPC table. @@ -511,47 +477,21 @@ impl VpcTable { #[cfg(feature = "testing")] #[allow(unsafe_code)] #[must_use] - pub(crate) unsafe fn fake_validated_vpc_table_for_tests(&self) -> ValidatedVpcTable { - let fake_validated_vpcs = unsafe { + pub(crate) unsafe fn fake_validated_vpc_table_for_tests(&self) -> VpcTable { + let vpcs = unsafe { self.vpcs .iter() .map(|(name, vpc)| (name.clone(), vpc.fake_validated_vpc_for_tests())) .collect() }; - ValidatedVpcTable { - vpcs: fake_validated_vpcs, + VpcTable { + vpcs, + vnis: self.vnis.clone(), ids: self.ids.clone(), } } -} - -#[derive(Debug, Default)] -pub struct ValidatedVpcTable { - vpcs: BTreeMap, - ids: BTreeMap, // name of vpc -} - -impl ValidatedVpcTable { - #[must_use] - pub fn len(&self) -> usize { - self.vpcs.len() - } - #[must_use] - pub fn is_empty(&self) -> bool { - self.vpcs.is_empty() - } - - pub fn values(&self) -> impl Iterator { - self.vpcs.values() - } - - #[must_use] - pub fn get_vpc(&self, vpc_name: &str) -> Option<&ValidatedVpc> { - self.vpcs.get(vpc_name) - } - - fn get_vpc_by_vpcid(&self, vpcid: &VpcId) -> Option<&ValidatedVpc> { + fn get_vpc_by_vpcid(&self, vpcid: &VpcId) -> Option<&Vpc> { match self.ids.get(vpcid) { Some(name) => self.vpcs.get(name), None => None, @@ -559,14 +499,9 @@ impl ValidatedVpcTable { } #[must_use] - pub fn get_remote_vni(&self, peering: &ValidatedPeering) -> Vni { + pub fn get_remote_vni(&self, peering: &Peering) -> Vni { self.get_vpc_by_vpcid(peering.remote_id()) .unwrap_or_else(|| unreachable!()) .vni } - - /// Iterate over all of the [`Peering`]s of all [`Vpc`]s immutably - pub fn peerings(&self) -> impl Iterator { - self.vpcs.values().flat_map(|vpc| vpc.peerings.iter()) - } } diff --git a/config/src/external/overlay/vpcpeering.rs b/config/src/external/overlay/vpcpeering.rs index 5a13989ba2..8d8a1154a6 100644 --- a/config/src/external/overlay/vpcpeering.rs +++ b/config/src/external/overlay/vpcpeering.rs @@ -167,7 +167,8 @@ impl VpcExpose { Ok(self) } - fn as_range_or_empty(&self) -> &PrefixPortsSet { + #[must_use] + pub fn as_range_or_empty(&self) -> &PrefixPortsSet { self.nat.as_ref().map_or(empty_set(), |nat| &nat.as_range) } @@ -270,8 +271,14 @@ impl VpcExpose { /// # Errors /// /// Returns an error if the expose configuration is invalid. + /// Validate and collapse this [`VpcExpose`] in place (exclusion prefixes are folded into the + /// allowed prefixes). + /// + /// # Errors + /// + /// Returns an error if the expose configuration is invalid. #[allow(clippy::too_many_lines)] - pub fn validate(&self) -> Result { + pub fn validate(&mut self) -> ConfigResult { // Check default exposes and prefixes self.validate_default_expose()?; @@ -342,38 +349,39 @@ impl VpcExpose { } } - // Apply exclusion prefixes - let mut clone = self.clone(); - collapse_prefixes(&mut clone); - merge_overlapping_prefixes(&mut clone.ips); - merge_contiguous_prefixes(&mut clone.ips); - if let Some(nat) = &mut clone.nat { + // Capture pre-collapse state needed by the post-collapse checks below: the original expose + // for error reporting, and whether the user specified any exclusion prefixes (which + // `collapse_prefixes` is about to fold away and clear). + let original = self.clone(); + let had_exclusions = !self.nots.is_empty() || !self.not_as_or_empty().is_empty(); + + // Apply exclusion prefixes. `collapse_prefixes` folds `nots`/`not_as` into `ips`/`as_range` + // and clears the exclusion sets, so `self` becomes exclusion-prefix-free. + collapse_prefixes(self); + merge_overlapping_prefixes(&mut self.ips); + merge_contiguous_prefixes(&mut self.ips); + if let Some(nat) = &mut self.nat { merge_overlapping_prefixes(&mut nat.as_range); merge_contiguous_prefixes(&mut nat.as_range); } - let collapsed_expose = ValidatedExpose { - default: clone.default, - ips: clone.ips, - nat: clone.nat, - }; // Ensure we don't exclude all of the allowed prefixes - if collapsed_expose.ips().is_empty() && !collapsed_expose.is_default() { - return Err(ConfigError::ExcludedAllPrefixes(Box::new(self.clone()))); + if self.ips.is_empty() && !self.default { + return Err(ConfigError::ExcludedAllPrefixes(Box::new(original))); } - if collapsed_expose.nat().is_some() && collapsed_expose.as_range_or_empty().is_empty() { - return Err(ConfigError::ExcludedAllPrefixes(Box::new(self.clone()))); + if self.nat.is_some() && self.as_range_or_empty().is_empty() { + return Err(ConfigError::ExcludedAllPrefixes(Box::new(original))); } - let ips_sizes = collapsed_expose.ips().total_prefixes_size(); - let as_range_sizes = collapsed_expose.as_range_or_empty().total_prefixes_size(); + let ips_sizes = self.ips.total_prefixes_size(); + let as_range_sizes = self.as_range_or_empty().total_prefixes_size(); // For static NAT, ensure that, if the list of publicly-exposed addresses is not empty, then // we have the same number of addresses on each side. // // Note: We shouldn't have subtraction overflows because we check that exclusion prefixes // size was smaller than allowed prefixes size already. - if collapsed_expose.has_static_nat() && ips_sizes != as_range_sizes { + if self.has_static_nat() && ips_sizes != as_range_sizes { return Err(ConfigError::MismatchedPrefixSizes( ips_sizes, as_range_sizes, @@ -386,14 +394,13 @@ impl VpcExpose { // - we have a single prefix on each side (private and public addresses) // - we have the same number of addresses on each side // - the list of associated port ranges also has the same size on each side - if collapsed_expose.has_port_forwarding() { - if !self.nots.is_empty() || !self.not_as_or_empty().is_empty() { + if self.has_port_forwarding() { + if had_exclusions { return Err(ConfigError::Forbidden( "Port forwarding does not support exclusion prefixes", )); } - if collapsed_expose.ips().len() != 1 || collapsed_expose.as_range_or_empty().len() != 1 - { + if self.ips.len() != 1 || self.as_range_or_empty().len() != 1 { return Err(ConfigError::Forbidden( "Port forwarding requires a single prefix on each side", )); @@ -406,7 +413,7 @@ impl VpcExpose { } // For port forwarding, ensure that a port range is always present. Lack of port range would imply // all ports, which is not allowed since port 0 is forbidden in the implementation - for prefixes in [collapsed_expose.ips(), collapsed_expose.as_range_or_empty()] { + for prefixes in [self.ips(), self.as_range_or_empty()] { if prefixes.iter().any(|p| p.ports().is_none()) { return Err(ConfigError::Forbidden( "Port forwarding requires a port range on each prefix", @@ -416,42 +423,32 @@ impl VpcExpose { } // For masquerade, we don't support port ranges - if collapsed_expose.has_masquerade() - && (collapsed_expose.ips().iter().any(|p| p.ports().is_some()) - || collapsed_expose - .as_range_or_empty() - .iter() - .any(|p| p.ports().is_some())) + if self.has_masquerade() + && (self.ips.iter().any(|p| p.ports().is_some()) + || self.as_range_or_empty().iter().any(|p| p.ports().is_some())) { return Err(ConfigError::Forbidden( "Port ranges are not supported with masquerade", )); } - Ok(collapsed_expose) + Ok(()) } - /// FOR TESTS ONLY + /// FOR TESTS ONLY. Produces an enriched-but-not-collapsed expose (exclusion prefixes are + /// dropped without being applied), bypassing real validation. #[cfg(feature = "testing")] #[must_use] #[allow(unsafe_code)] - unsafe fn fake_validated_expose(&self) -> ValidatedExpose { - ValidatedExpose { + unsafe fn fake_validated_expose(&self) -> VpcExpose { + VpcExpose { default: self.default, ips: self.ips.clone(), + nots: PrefixPortsSet::new(), nat: self.nat.clone(), } } -} - -#[derive(Clone, Debug, Default, PartialEq)] -pub struct ValidatedExpose { - default: bool, - ips: PrefixPortsSet, - nat: Option, -} -impl ValidatedExpose { #[must_use] pub fn is_default(&self) -> bool { self.default @@ -462,26 +459,6 @@ impl ValidatedExpose { &self.ips } - #[must_use] - pub fn as_range_or_empty(&self) -> &PrefixPortsSet { - self.nat.as_ref().map_or(empty_set(), |nat| &nat.as_range) - } - - // If the as_range list is empty, then there's no NAT required for the expose, meaning that the - // public IPs are those from the "ips" list. This method returns the current list of public IPs - // for the VpcExpose. - #[must_use] - pub fn public_ips(&self) -> &PrefixPortsSet { - let Some(nat) = self.nat.as_ref() else { - return &self.ips; - }; - if nat.as_range.is_empty() { - &self.ips - } else { - &nat.as_range - } - } - /// The prefixes of an expose to be advertised to a remote peer #[must_use] pub fn adv_prefixes(&self) -> Vec { @@ -563,11 +540,6 @@ impl ValidatedExpose { self.nat.as_ref() } - #[must_use] - pub fn nat_config(&self) -> Option<&VpcExposeNatConfig> { - self.nat.as_ref().map(|nat| &nat.config) - } - #[must_use] pub fn nat_proto(&self) -> Option<&L4Protocol> { self.nat.as_ref().map(|nat| &nat.proto) @@ -623,7 +595,12 @@ impl VpcManifest { /// # Errors /// /// Returns an error if the manifest configuration is invalid. - pub fn validate(&self) -> Result { + /// Validate and collapse this [`VpcManifest`] (and its exposes) in place. + /// + /// # Errors + /// + /// Returns an error if the manifest configuration is invalid. + pub fn validate(&mut self) -> ConfigResult { if self.name.is_empty() { return Err(ConfigError::MissingIdentifier("Manifest name")); } @@ -636,16 +613,12 @@ impl VpcManifest { )); } - let mut valid_manifest_candidate = ValidatedManifest { - name: self.name.clone(), - valexp: Vec::new(), - }; - for expose in &self.exposes { - valid_manifest_candidate.valexp.push(expose.validate()?); + // Validate and collapse each expose in place. + for expose in &mut self.exposes { + expose.validate()?; } - valid_manifest_candidate.validate_expose_collisions()?; - Ok(valid_manifest_candidate) + self.validate_expose_collisions() } #[must_use] @@ -661,78 +634,65 @@ impl VpcManifest { #[cfg(feature = "testing")] #[allow(unsafe_code)] #[must_use] - pub unsafe fn fake_valid_manifest_for_tests(&self) -> ValidatedManifest { - let mut fake_valid_manifest = ValidatedManifest { + pub unsafe fn fake_valid_manifest_for_tests(&self) -> VpcManifest { + let exposes = self + .exposes + .iter() + .map(|expose| unsafe { expose.fake_validated_expose() }) + .collect(); + VpcManifest { name: self.name.clone(), - valexp: Vec::new(), - }; - for expose in &self.exposes { - let fake_valid_expose = unsafe { expose.fake_validated_expose() }; - fake_valid_manifest.valexp.push(fake_valid_expose); + exposes, } - fake_valid_manifest } -} - -#[derive(Clone, Debug, PartialEq)] -pub struct ValidatedManifest { - name: String, /* key: name of vpc */ - // Validated, exclusion-prefixes-free view of exposes. - valexp: Vec, -} -impl ValidatedManifest { #[must_use] pub fn name(&self) -> &str { &self.name } + /// Validated, exclusion-prefixes-free view of the exposes. #[must_use] - pub fn valexp(&self) -> &[ValidatedExpose] { - &self.valexp - } - - #[must_use] - pub fn default_expose(&self) -> Option<&ValidatedExpose> { - self.valexp().iter().find(|expose| expose.is_default()) + pub fn valexp(&self) -> &[VpcExpose] { + &self.exposes } - fn filter_exposes(&self, predicate: F) -> impl Iterator + fn filter_exposes(&self, predicate: F) -> impl Iterator where - F: FnMut(&&ValidatedExpose) -> bool, + F: FnMut(&&VpcExpose) -> bool, { self.valexp().iter().filter(predicate) } - pub fn static_nat_exposes(&self) -> impl Iterator { + pub fn static_nat_exposes(&self) -> impl Iterator { self.filter_exposes(|expose| expose.has_static_nat()) } - pub fn masquerade_exposes_44(&self) -> impl Iterator { + pub fn masquerade_exposes_44(&self) -> impl Iterator { self.filter_exposes(|expose| expose.has_masquerade() && expose.is_44()) } - pub fn masquerade_exposes_66(&self) -> impl Iterator { + pub fn masquerade_exposes_66(&self) -> impl Iterator { self.filter_exposes(|expose| expose.has_masquerade() && expose.is_66()) } - pub fn port_forwarding_exposes(&self) -> impl Iterator { + pub fn port_forwarding_exposes(&self) -> impl Iterator { self.filter_exposes(|expose| expose.has_port_forwarding()) } - pub fn port_forwarding_exposes_44(&self) -> impl Iterator { + pub fn port_forwarding_exposes_44(&self) -> impl Iterator { self.filter_exposes(|expose| expose.has_port_forwarding() && expose.is_44()) } - pub fn port_forwarding_exposes_66(&self) -> impl Iterator { + pub fn port_forwarding_exposes_66(&self) -> impl Iterator { self.filter_exposes(|expose| expose.has_port_forwarding() && expose.is_66()) } fn validate_expose_collisions(&self) -> ConfigResult { // Check that prefixes in each expose don't overlap with prefixes in other exposes - for (index, expose_left) in self.valexp.iter().enumerate() { + for (index, expose_left) in self.exposes.iter().enumerate() { // Loop over the remaining exposes in the list - for expose_right in self.valexp.iter().skip(index + 1) { + for expose_right in self.exposes.iter().skip(index + 1) { #[allow(clippy::unnested_or_patterns)] match (&expose_left.nat_config(), &expose_right.nat_config()) { // Overlap allowed diff --git a/config/src/external/overlay/vpcrouting.rs b/config/src/external/overlay/vpcrouting.rs index 3f7ed19ac7..49eace9876 100644 --- a/config/src/external/overlay/vpcrouting.rs +++ b/config/src/external/overlay/vpcrouting.rs @@ -3,9 +3,9 @@ //! Dataplane configuration model: vpc routing -use crate::ConfigError; -use crate::external::ValidatedPeering; -use crate::external::overlay::vpcpeering::ValidatedExpose; +use crate::external::overlay::vpc::Peering; +use crate::external::overlay::vpcpeering::VpcExpose; +use crate::{ConfigError, ConfigResult}; use lpm::prefix::{IpRangeWithPorts, PrefixPortsSet, PrefixWithOptionalPorts}; use net::vxlan::Vni; use ordermap::OrderMap; @@ -19,8 +19,8 @@ pub enum ExposeAction { Forward, Default, } -impl From<&ValidatedExpose> for ExposeAction { - fn from(expose: &ValidatedExpose) -> Self { +impl From<&VpcExpose> for ExposeAction { + fn from(expose: &VpcExpose) -> Self { if expose.has_masquerade() { return ExposeAction::Masquerade; } else if expose.has_port_forwarding() { @@ -35,7 +35,7 @@ impl From<&ValidatedExpose> for ExposeAction { } /// A type representing a route to a remote vpc -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct VpcRoute { dst: PrefixWithOptionalPorts, // destination(s) this route applies to dstvpc: String, // destination VPC @@ -46,7 +46,7 @@ pub struct VpcRoute { /// A type representing a set of routes to the same destination. /// This type is currently not public -#[derive(Debug)] +#[derive(Clone, Debug)] struct VpcRouteSet(Vec); impl VpcRouteSet { #[must_use] @@ -109,7 +109,7 @@ impl VpcRoute { /// For any destination `PrefixWithOptionalPorts`, this table can keep /// a collection of `VpcRoute`s in the form of a `VpcRouteSet`. Routes to /// the same destination are kept together in a `VpcRouteSet`. -#[derive(Debug)] +#[derive(Clone, Debug, Default)] pub struct VpcRouteTable { table: OrderMap, } @@ -126,8 +126,8 @@ impl VpcRouteTable { } #[must_use] - /// Build a `VpcRouteTable` from the set of `ValidatedPeering` of a VPC - pub fn build(peerings: &Vec) -> Self { + /// Build a `VpcRouteTable` from the (validated) peerings of a VPC + pub fn build(peerings: &[Peering]) -> Self { let mut rt = VpcRouteTable::new(); for peering in peerings { for expose in peering.remote().valexp() { @@ -153,7 +153,7 @@ impl VpcRouteTable { rt } - /// Consume and validate a `VpcRouteTable` + /// Validate a `VpcRouteTable` /// /// # Errors /// @@ -162,7 +162,7 @@ impl VpcRouteTable { /// 2) destinations cannot overlap except if they are masqueraded or a default /// 3) overlapping destinations, when allowed, must use the same gateway group /// - pub fn validate(self) -> Result { + pub fn validate(&self) -> ConfigResult { let all_routes: Vec<&VpcRoute> = self.table.values().flat_map(VpcRouteSet::iter).collect(); for (i, &route) in all_routes.iter().enumerate() { for &other in &all_routes[i + 1..] { @@ -183,6 +183,6 @@ impl VpcRouteTable { } } } - Ok(self) + Ok(()) } } diff --git a/config/src/external/underlay/mod.rs b/config/src/external/underlay/mod.rs index 080c1f5204..e4941f1fe8 100644 --- a/config/src/external/underlay/mod.rs +++ b/config/src/external/underlay/mod.rs @@ -3,10 +3,10 @@ //! Underlay configuration -use crate::ConfigError; use crate::internal::interfaces::interface::{InterfaceConfig, InterfaceType}; use crate::internal::routing::evpn::VtepConfig; use crate::internal::routing::vrf::VrfConfig; +use crate::{ConfigError, ConfigResult}; use net::eth::mac::SourceMac; use net::ipv4::UnicastIpv4Addr; @@ -77,7 +77,7 @@ impl Underlay { /// # Errors /// /// Returns an error if any interface is invalid or VTEP configuration is wrong. - pub fn validate(&self) -> Result { + pub fn validate(&mut self) -> ConfigResult { debug!("Validating underlay configuration..."); // validate interfaces @@ -86,10 +86,8 @@ impl Underlay { .values() .try_for_each(InterfaceConfig::validate)?; - Ok(Self { - vrf: self.vrf.clone(), - // set vtep information if a vtep interface has been specified in the config - vtep: self.get_vtep_info()?, - }) + // set vtep information if a vtep interface has been specified in the config + self.vtep = self.get_vtep_info()?; + Ok(()) } } diff --git a/config/src/gwconfig.rs b/config/src/gwconfig.rs index 5061a4e91e..25e476d979 100644 --- a/config/src/gwconfig.rs +++ b/config/src/gwconfig.rs @@ -4,7 +4,7 @@ //! Top-level configuration object for the dataplane use crate::errors::{ConfigError, ConfigResult}; -use crate::external::{GenId, ValidatedExternalConfig}; +use crate::external::{ExternalConfig, GenId}; use crate::internal::InternalConfig; use concurrency::slot::Slot; use concurrency::sync::Arc; @@ -61,28 +61,36 @@ impl GwConfigMeta { } #[derive(Debug)] -pub struct ValidatedGwConfig { +pub struct GwConfig { meta: Slot, - external: ValidatedExternalConfig, + external: ExternalConfig, internal: Option, } -impl ValidatedGwConfig { +impl GwConfig { #[must_use] - pub(crate) fn new(external: ValidatedExternalConfig) -> Self { + pub(crate) fn new(external: ExternalConfig) -> Self { Self { meta: Slot::new(Arc::from(GwConfigMeta::new(external.genid()))), external, internal: None, } } - #[must_use] + /// Build an empty [`GwConfig`] from an empty [`ExternalConfig`]. + /// An empty [`ExternalConfig`] should always be valid. A unit test verifies this invariant. pub fn blank() -> Self { - // The blank config has no overlay, peerings, or VPCs, so it trivially passes validation. - // A unit test verifies this invariant. - let external = ValidatedExternalConfig::blank(); - Self::new(external) + Self::from_external(ExternalConfig::new("")).unwrap_or_else(|_| unreachable!()) + } + + /// Consume an [`ExternalConfig`] to obtain a [`GwConfig`], which is validated by definition. + /// This is the only way to obtain a non-blank [`GwConfig`]. + /// + /// # Errors + /// This method returns `ConfigError` if the external config fails to validate + pub fn from_external(mut external: ExternalConfig) -> Result { + external.validate()?; + Ok(Self::new(external)) } #[must_use] @@ -91,7 +99,7 @@ impl ValidatedGwConfig { } #[must_use] - pub fn external(&self) -> &ValidatedExternalConfig { + pub fn external(&self) -> &ExternalConfig { &self.external } @@ -116,7 +124,7 @@ mod tests { #[test] fn test_blank_config_is_valid() { - let _ = ExternalConfig::new("") + ExternalConfig::new("") .validate() .expect("Failed to validate blank config"); } diff --git a/config/src/lib.rs b/config/src/lib.rs index 4bc447dcd0..833755edfc 100644 --- a/config/src/lib.rs +++ b/config/src/lib.rs @@ -28,7 +28,7 @@ pub mod utils; pub use display::ConfigSummary; pub use errors::{ConfigError, ConfigResult, stringify}; pub use external::{ExternalConfig, GenId}; -pub use gwconfig::{GwConfigMeta, ValidatedGwConfig}; +pub use gwconfig::{GwConfig, GwConfigMeta}; pub use internal::InternalConfig; pub use internal::device::DeviceConfig; diff --git a/config/src/utils/overlap.rs b/config/src/utils/overlap.rs index cbf2af1d54..60d200eaea 100644 --- a/config/src/utils/overlap.rs +++ b/config/src/utils/overlap.rs @@ -2,12 +2,12 @@ // Copyright Open Network Fabric Authors use crate::ConfigError; -use crate::external::overlay::vpcpeering::ValidatedExpose; +use crate::external::overlay::vpcpeering::VpcExpose; use lpm::prefix::{IpRangeWithPorts, PrefixPortsSet}; pub fn check_private_prefixes_dont_overlap( - expose_left: &ValidatedExpose, - expose_right: &ValidatedExpose, + expose_left: &VpcExpose, + expose_right: &VpcExpose, ) -> Result<(), ConfigError> { if port_forwarding_with_distinct_l4_protocols(expose_left, expose_right) { return Ok(()); @@ -22,8 +22,8 @@ pub fn check_private_prefixes_dont_overlap( // - expose_left.as_range / expose_right.ips // - expose_left.ips / expose_right.ips pub fn check_public_prefixes_dont_overlap( - expose_left: &ValidatedExpose, - expose_right: &ValidatedExpose, + expose_left: &VpcExpose, + expose_right: &VpcExpose, ) -> Result<(), ConfigError> { if port_forwarding_with_distinct_l4_protocols(expose_left, expose_right) { return Ok(()); @@ -34,8 +34,8 @@ pub fn check_public_prefixes_dont_overlap( // If the two expose blocks have port forwarding set up, one for TCP and one for UDP, then there is // no overlap. fn port_forwarding_with_distinct_l4_protocols( - expose_left: &ValidatedExpose, - expose_right: &ValidatedExpose, + expose_left: &VpcExpose, + expose_right: &VpcExpose, ) -> bool { expose_left.has_port_forwarding() && expose_right.has_port_forwarding() diff --git a/flow-filter/src/setup.rs b/flow-filter/src/setup.rs index 8d58b2a838..4a0802fb29 100644 --- a/flow-filter/src/setup.rs +++ b/flow-filter/src/setup.rs @@ -4,11 +4,9 @@ use crate::FlowFilterTable; use crate::tables::{FlowFilterSubtable, NatRequirement, RemoteData, VpcdLookupResult}; use config::ConfigError; -#[cfg(test)] use config::external::overlay::Overlay; -use config::external::overlay::ValidatedOverlay; -use config::external::overlay::vpc::{ValidatedPeering, ValidatedVpc}; -use config::external::overlay::vpcpeering::{ValidatedExpose, ValidatedManifest}; +use config::external::overlay::vpc::{Peering, Vpc}; +use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest}; use lpm::prefix::{IpRangeWithPorts, PrefixPortsSet, PrefixWithOptionalPorts}; use net::packet::VpcDiscriminant; use std::collections::{BTreeMap, BTreeSet, HashSet}; @@ -19,7 +17,7 @@ trace_target!("flow-filter-setup", LevelFilter::INFO, &[]); impl FlowFilterTable { /// Build a [`FlowFilterTable`] from an overlay - pub fn build_from_overlay(overlay: &ValidatedOverlay) -> Result { + pub fn build_from_overlay(overlay: &Overlay) -> Result { let mut table = FlowFilterTable::new(); for vpc in overlay.vpc_table().values() { @@ -33,9 +31,9 @@ impl FlowFilterTable { fn add_peering( &mut self, - overlay: &ValidatedOverlay, - vpc: &ValidatedVpc, - peering: &ValidatedPeering, + overlay: &Overlay, + vpc: &Vpc, + peering: &Peering, ) -> Result<(), ConfigError> { let local_vpcd = VpcDiscriminant::VNI(vpc.vni()); let dst_vpcd = VpcDiscriminant::VNI(overlay.vpc_table().get_remote_vni(peering)); @@ -73,9 +71,9 @@ type PrefixWithData = ( ); fn get_prefixes_for_processing( - overlay: &ValidatedOverlay, - vpc: &ValidatedVpc, - peering: &ValidatedPeering, + overlay: &Overlay, + vpc: &Vpc, + peering: &Peering, dst_vpcd: VpcDiscriminant, skip_ports: bool, ) -> (Vec, Vec) { @@ -112,8 +110,8 @@ impl FlowFilterSubtable { dst_vpcd: VpcDiscriminant, local_prefixes: Vec, remote_prefixes: Vec, - local_default_expose: Option<&ValidatedExpose>, - remote_default_expose: Option<&ValidatedExpose>, + local_default_expose: Option<&VpcExpose>, + remote_default_expose: Option<&VpcExpose>, ) -> Result<(), ConfigError> { // Handle local default expose (for all remote prefixes) if let Some(local_default_expose) = local_default_expose { @@ -264,9 +262,9 @@ impl FlowFilterSubtable { // - between prefixes from remote manifest and prefixes from remote manifests for other peerings // - between prefixes from local manifest and prefixes from local manifests for other peerings fn get_manifests_overlap( - overlay: &ValidatedOverlay, - vpc: &ValidatedVpc, - peering: &ValidatedPeering, + overlay: &Overlay, + vpc: &Vpc, + peering: &Peering, dst_vpcd: VpcDiscriminant, skip_ports: bool, ) -> ( @@ -331,11 +329,11 @@ where // // Exclude the "default"-destination expose blocks from overlap calculation. fn get_manifest_ips_overlap( - manifest_left: &ValidatedManifest, - manifest_right: &ValidatedManifest, + manifest_left: &VpcManifest, + manifest_right: &VpcManifest, dst_vpcd_left: VpcDiscriminant, dst_vpcd_right: VpcDiscriminant, - get_ips: fn(&ValidatedExpose) -> &BTreeSet, + get_ips: fn(&VpcExpose) -> &BTreeSet, compare_to_self: bool, skip_ports: bool, ) -> BTreeMap> { @@ -435,9 +433,9 @@ fn consolidate_overlap_list( // with VPC C's 10.0.0.0/25) // - For VPC C: [10.0.0.0/25] fn get_split_prefixes_for_manifest( - manifest: &ValidatedManifest, + manifest: &VpcManifest, vpcd: &VpcDiscriminant, - get_ips: fn(&ValidatedExpose) -> &PrefixPortsSet, + get_ips: fn(&VpcExpose) -> &PrefixPortsSet, overlaps: BTreeMap>, skip_ports: bool, ) -> Vec { @@ -505,7 +503,7 @@ fn split_overlapping( split_prefixes } -fn get_nat_requirement(expose: &ValidatedExpose) -> Option { +fn get_nat_requirement(expose: &VpcExpose) -> Option { expose.nat().map(NatRequirement::from_nat) } @@ -675,19 +673,17 @@ mod tests { let vpcd1 = vpcd(100); let vpcd2 = vpcd(200); - let manifest1 = VpcManifest::with_exposes( + let mut manifest1 = VpcManifest::with_exposes( "manifest1", vec![VpcExpose::empty().ip("10.0.0.0/24".into())], - ) - .validate() - .unwrap(); + ); + manifest1.validate().unwrap(); - let manifest2 = VpcManifest::with_exposes( + let mut manifest2 = VpcManifest::with_exposes( "manifest2", vec![VpcExpose::empty().ip("20.0.0.0/24".into())], - ) - .validate() - .unwrap(); + ); + manifest2.validate().unwrap(); let overlap = get_manifest_ips_overlap( &manifest1, @@ -708,19 +704,17 @@ mod tests { let vpcd1 = vpcd(100); let vpcd2 = vpcd(200); - let manifest1 = VpcManifest::with_exposes( + let mut manifest1 = VpcManifest::with_exposes( "manifest1", vec![VpcExpose::empty().ip("10.0.0.0/24".into())], - ) - .validate() - .unwrap(); + ); + manifest1.validate().unwrap(); - let manifest2 = VpcManifest::with_exposes( + let mut manifest2 = VpcManifest::with_exposes( "manifest2", vec![VpcExpose::empty().ip("10.0.0.0/25".into())], - ) - .validate() - .unwrap(); + ); + manifest2.validate().unwrap(); let overlap = get_manifest_ips_overlap( &manifest1, @@ -747,22 +741,20 @@ mod tests { let vpcd1 = vpcd(100); let vpcd2 = vpcd(200); - let manifest1 = VpcManifest::with_exposes( + let mut manifest1 = VpcManifest::with_exposes( "manifest1", vec![VpcExpose::empty().ip("10.0.0.0/24".into())], - ) - .validate() - .unwrap(); + ); + manifest1.validate().unwrap(); - let manifest2 = VpcManifest::with_exposes( + let mut manifest2 = VpcManifest::with_exposes( "manifest2", vec![VpcExpose::empty().ip(PrefixWithOptionalPorts::new( "10.0.0.0/25".into(), Some(PortRange::new(100, 200).unwrap()), ))], - ) - .validate() - .unwrap(); + ); + manifest2.validate().unwrap(); let overlap = get_manifest_ips_overlap( &manifest1, @@ -793,18 +785,17 @@ mod tests { let vpcd1 = vpcd(100); let vpcd2 = vpcd(200); - let manifest1 = VpcManifest::with_exposes( + let mut manifest1 = VpcManifest::with_exposes( "manifest1", vec![ VpcExpose::empty() .ip("10.0.0.0/24".into()) .ip("20.0.0.128/25".into()), ], - ) - .validate() - .unwrap(); + ); + manifest1.validate().unwrap(); - let manifest2 = VpcManifest::with_exposes( + let mut manifest2 = VpcManifest::with_exposes( "manifest2", vec![ VpcExpose::empty() @@ -815,9 +806,8 @@ mod tests { .unwrap(), VpcExpose::empty().ip("20.0.0.0/24".into()), ], - ) - .validate() - .unwrap(); + ); + manifest2.validate().unwrap(); let overlap = get_manifest_ips_overlap( &manifest1, @@ -1066,12 +1056,11 @@ mod tests { fn test_get_split_prefixes_for_manifest_no_overlap() { let vpcd = vpcd(100); - let manifest = VpcManifest::with_exposes( + let mut manifest = VpcManifest::with_exposes( "manifest", vec![VpcExpose::empty().ip("10.0.0.0/24".into())], - ) - .validate() - .unwrap(); + ); + manifest.validate().unwrap(); let overlaps = BTreeMap::new(); @@ -1099,12 +1088,11 @@ mod tests { fn test_get_split_prefixes_for_manifest_with_overlap() { let vpcd = vpcd(100); - let manifest = VpcManifest::with_exposes( + let mut manifest = VpcManifest::with_exposes( "manifest", vec![VpcExpose::empty().ip("10.0.0.0/24".into())], - ) - .validate() - .unwrap(); + ); + manifest.validate().unwrap(); let mut overlaps = BTreeMap::new(); // The overlap covers part of the manifest's prefix @@ -1158,12 +1146,11 @@ mod tests { let remote_data_c = RemoteData::new(vpcd(300), None, None); // VPC A exposes 10.0.0.0/24 - let manifest = VpcManifest::with_exposes( + let mut manifest = VpcManifest::with_exposes( "manifest_a", vec![VpcExpose::empty().ip("10.0.0.0/24".into())], - ) - .validate() - .unwrap(); + ); + manifest.validate().unwrap(); let mut overlaps = BTreeMap::new(); // Overlap 1: 10.0.0.0/25 is shared between A and B @@ -1239,14 +1226,14 @@ mod tests { vpc_table.add(vpc1.clone()).unwrap(); vpc_table.add(vpc2).unwrap(); - let overlay = Overlay { + let mut overlay = Overlay { vpc_table, peering_table: VpcPeeringTable::new(), - } - .validate() - .unwrap(); + }; + overlay.validate().unwrap(); - let vpc1 = vpc1.validate().unwrap(); + let mut vpc1 = vpc1; + vpc1.validate().unwrap(); let mut table = FlowFilterTable::new(); table .add_peering(&overlay, &vpc1, &vpc1.peerings()[0]) @@ -1314,14 +1301,13 @@ mod tests { vpc_table.add(vpc2).unwrap(); vpc_table.add(vpc3).unwrap(); - let overlay = Overlay { + let mut overlay = Overlay { vpc_table, peering_table: VpcPeeringTable::new(), - } - .validate() - .unwrap(); + }; + overlay.validate().unwrap(); - // vpc1 is not valid, we have to fake its transformation into a ValidatedVpc for this test + // vpc1 is not valid, we have to fake its transformation into a Vpc for this test assert!(matches!( vpc1.clone().validate(), Err(ConfigError::OverlappingPrefixes(_, _)) @@ -1389,12 +1375,11 @@ mod tests { )) .unwrap(); - let overlay = Overlay { + let mut overlay = Overlay { vpc_table, peering_table, - } - .validate() - .unwrap(); + }; + overlay.validate().unwrap(); let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); diff --git a/flow-filter/src/tests.rs b/flow-filter/src/tests.rs index 2d67f2f895..5c787ef72e 100644 --- a/flow-filter/src/tests.rs +++ b/flow-filter/src/tests.rs @@ -446,7 +446,7 @@ fn test_flow_filter_table_overlap_cases() { // doesn't matter for the test. let overlay = Overlay::new(vpc_table, peering_table); assert!(matches!( - overlay.validate(), + overlay.clone().validate(), Err(ConfigError::OverlappingPrefixes(_, _)) )); let overlay = unsafe { overlay.fake_validated_overlay_for_tests() }; @@ -672,7 +672,8 @@ fn test_flow_filter_table_from_overlay() { )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); let mut writer = FlowFilterTableWriter::new(); writer.update_flow_filter_table(table); @@ -756,7 +757,8 @@ fn test_flow_filter_table_check_send_from_default() { )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); let mut writer = FlowFilterTableWriter::new(); writer.update_flow_filter_table(table); @@ -801,7 +803,10 @@ fn test_flow_filter_table_check_default_to_default() { // We don't validate because overlapping prefixes actually make the config invalid; but it // doesn't matter for the test. let overlay = Overlay::new(vpc_table, peering_table); - assert!(matches!(overlay.validate(), Err(ConfigError::Forbidden(_)))); + assert!(matches!( + overlay.clone().validate(), + Err(ConfigError::Forbidden(_)) + )); let overlay = unsafe { overlay.fake_validated_overlay_for_tests() }; let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); @@ -884,7 +889,10 @@ fn test_flow_filter_table_check_nat_requirements() { // We don't validate because overlapping prefixes actually make the config invalid; but it // doesn't matter for the test. let overlay = Overlay::new(vpc_table, peering_table); - assert!(matches!(overlay.validate(), Err(ConfigError::Forbidden(_)))); + assert!(matches!( + overlay.clone().validate(), + Err(ConfigError::Forbidden(_)) + )); let overlay = unsafe { overlay.fake_validated_overlay_for_tests() }; let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); @@ -998,7 +1006,8 @@ fn test_flow_filter_table_check_masquerade_plus_port_forwarding() { )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); let mut writer = FlowFilterTableWriter::new(); writer.update_flow_filter_table(table); @@ -1202,7 +1211,8 @@ fn test_flow_filter_protocol_aware_port_forwarding() { )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); let mut writer = FlowFilterTableWriter::new(); writer.update_flow_filter_table(table); @@ -1329,7 +1339,8 @@ fn test_flow_filter_protocol_any_port_forwarding() { )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); let mut writer = FlowFilterTableWriter::new(); writer.update_flow_filter_table(table); @@ -1485,7 +1496,8 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); let mut writer = FlowFilterTableWriter::new(); writer.update_flow_filter_table(table); @@ -1712,7 +1724,8 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); let mut writer = FlowFilterTableWriter::new(); writer.update_flow_filter_table(table); @@ -1829,7 +1842,8 @@ fn test_flow_filter_table_from_overlay_masquerade_port_forwarding_private_ips_ov )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); let table = FlowFilterTable::build_from_overlay(&overlay).unwrap(); let mut writer = FlowFilterTableWriter::new(); writer.update_flow_filter_table(table); diff --git a/mgmt/src/processor/confbuild/internal.rs b/mgmt/src/processor/confbuild/internal.rs index 05fd43198c..14a3d117b3 100644 --- a/mgmt/src/processor/confbuild/internal.rs +++ b/mgmt/src/processor/confbuild/internal.rs @@ -10,9 +10,9 @@ const IMPORT_VRFS: bool = false; use config::external::communities::PriorityCommunityTable; use config::external::gwgroup::GwGroupTable; -use config::external::overlay::ValidatedOverlay; -use config::external::overlay::vpc::{ValidatedPeering, ValidatedVpc}; -use config::external::overlay::vpcpeering::ValidatedManifest; +use config::external::overlay::Overlay; +use config::external::overlay::vpc::{Peering, Vpc}; +use config::external::overlay::vpcpeering::VpcManifest; use config::{ConfigError, ConfigResult}; use lpm::prefix::Prefix; @@ -33,12 +33,12 @@ use config::internal::routing::routemap::{ }; use config::internal::routing::statics::StaticRoute; use config::internal::routing::vrf::VrfConfig; -use config::{InternalConfig, ValidatedGwConfig}; +use config::{GwConfig, InternalConfig}; /// Populate a prefix list to import routes into a vpc vrf fn vpc_import_prefix_list_for_peer( - vpc: &ValidatedVpc, - rmanifest: &ValidatedManifest, + vpc: &Vpc, + rmanifest: &VpcManifest, ) -> Result { let mut plist = PrefixList::new( &vpc.import_plist_peer(rmanifest.name()), @@ -66,7 +66,7 @@ fn vpc_import_prefix_list_for_peer( /// Build AF l2vpn EVPN config for a VPC VRF #[must_use] -fn vpc_bgp_af_l2vpn_evpn(vpc: &ValidatedVpc) -> AfL2vpnEvpn { +fn vpc_bgp_af_l2vpn_evpn(vpc: &Vpc) -> AfL2vpnEvpn { AfL2vpnEvpn::new() .set_adv_all_vni(false) .set_adv_default_gw(false) @@ -103,7 +103,7 @@ struct VpcRoutingConfigIpv4 { } impl VpcRoutingConfigIpv4 { #[must_use] - fn new(vpc: &ValidatedVpc) -> Self { + fn new(vpc: &Vpc) -> Self { Self { import_rmap: RouteMap::new(&vpc.import_rmap_ipv4()), import_plists: Vec::with_capacity(vpc.num_peerings()), @@ -116,8 +116,8 @@ impl VpcRoutingConfigIpv4 { } fn build_routing_config_peer( &mut self, - vpc: &ValidatedVpc, - peer: &ValidatedPeering, + vpc: &Vpc, + peer: &Peering, community: Community, ) -> ConfigResult { /* remote manifest */ @@ -181,7 +181,7 @@ impl VpcRoutingConfigIpv4 { fn build_routing_config( &mut self, gwname: &str, - vpc: &ValidatedVpc, + vpc: &Vpc, grouptable: &GwGroupTable, commtable: &PriorityCommunityTable, ) -> ConfigResult { @@ -197,7 +197,7 @@ impl VpcRoutingConfigIpv4 { } /// Build BGP config for a VPC VRF (bmp is applied elsewhere) -fn vpc_vrf_bgp_config(vpc: &ValidatedVpc, asn: u32, router_id: Option) -> BgpConfig { +fn vpc_vrf_bgp_config(vpc: &Vpc, asn: u32, router_id: Option) -> BgpConfig { let mut bgp = BgpConfig::new(asn).set_vrf_name(vpc.vrf_name()); if let Some(router_id) = router_id { bgp.set_router_id(router_id); @@ -207,7 +207,7 @@ fn vpc_vrf_bgp_config(vpc: &ValidatedVpc, asn: u32, router_id: Option) } /// Build VRF config for a VPC -fn vpc_vrf_config(vpc: &ValidatedVpc) -> Result { +fn vpc_vrf_config(vpc: &Vpc) -> Result { debug!("Building VRF config for vpc '{}'", vpc.name()); /* build vrf config */ let mut vrf_cfg = VrfConfig::new(&vpc.vrf_name(), Some(vpc.vni()), false) @@ -247,7 +247,7 @@ fn vpc_bgp_af_ipv4_unicast(vpc_rconf: &VpcRoutingConfigIpv4) -> AfIpv4Ucast { } fn build_vpc_internal_config( - vpc: &ValidatedVpc, + vpc: &Vpc, asn: u32, router_id: Option, internal: &mut InternalConfig, @@ -288,7 +288,7 @@ fn build_vpc_internal_config( } fn build_internal_overlay_config( - overlay: &ValidatedOverlay, + overlay: &Overlay, asn: u32, router_id: Option, internal: &mut InternalConfig, @@ -338,7 +338,7 @@ fn configure_bgp_peers(vrf: &mut VrfConfig, internal: &mut InternalConfig) { /// Public entry — build with BMP (global options injected into default VRF and import views) pub fn build_internal_config( - config: &ValidatedGwConfig, + config: &GwConfig, bmp: Option, ) -> Result { let genid = config.genid(); diff --git a/mgmt/src/processor/confbuild/namegen.rs b/mgmt/src/processor/confbuild/namegen.rs index 700c15392a..bd7525c3e1 100644 --- a/mgmt/src/processor/confbuild/namegen.rs +++ b/mgmt/src/processor/confbuild/namegen.rs @@ -11,7 +11,7 @@ #![allow(unused)] -use config::external::overlay::vpc::{ValidatedVpc, VpcId}; +use config::external::overlay::vpc::{Vpc, VpcId}; use net::interface::InterfaceName; //////////////////////////////////////////////////////////////////////// @@ -53,7 +53,7 @@ pub(crate) trait VpcConfigNames { fn adv_rmap(&self) -> String; } -impl VpcConfigNames for ValidatedVpc { +impl VpcConfigNames for Vpc { fn vrf_name(&self) -> String { self.id().vrf_name().to_string() } diff --git a/mgmt/src/processor/confbuild/router.rs b/mgmt/src/processor/confbuild/router.rs index 3e7d3ce5f5..7168a4d8ec 100644 --- a/mgmt/src/processor/confbuild/router.rs +++ b/mgmt/src/processor/confbuild/router.rs @@ -16,7 +16,7 @@ use tracing::{debug, error}; use config::internal::interfaces::interface::InterfaceConfig; use config::internal::routing::vrf::VrfConfig; -use config::{ConfigError, InternalConfig, ValidatedGwConfig}; +use config::{ConfigError, GwConfig, InternalConfig}; use net::eth::mac::{Mac, SourceMac}; use net::interface::{Interface, InterfaceIndex, InterfaceName, Mtu}; @@ -183,7 +183,7 @@ fn generate_router_interfaces_config( } pub(crate) fn generate_router_config( kernel_vrfs: &HashMap, - config: Arc, + config: Arc, ) -> Result { let genid = config.genid(); debug!("Generating router config for genid {genid}..."); diff --git a/mgmt/src/processor/gwconfigdb.rs b/mgmt/src/processor/gwconfigdb.rs index 3f73f48b5d..180ca138cc 100644 --- a/mgmt/src/processor/gwconfigdb.rs +++ b/mgmt/src/processor/gwconfigdb.rs @@ -5,20 +5,20 @@ use crate::processor::confbuild::internal::build_internal_config; use concurrency::sync::Arc; -use config::{ConfigSummary, GenId, GwConfigMeta, ValidatedGwConfig}; +use config::{ConfigSummary, GenId, GwConfig, GwConfigMeta}; use tracing::{debug, info}; /// Configuration database, keeps a set of [`GwConfig`]s keyed by generation id [`GenId`] pub(crate) struct GwConfigDatabase { - applied: Arc, /* Currently applied config or blank */ - history: Vec, /* event history */ + applied: Arc, /* Currently applied config or blank */ + history: Vec, /* event history */ } impl GwConfigDatabase { #[must_use] pub fn new() -> Self { debug!("Building config database..."); - let mut blank = ValidatedGwConfig::blank(); + let mut blank = GwConfig::blank(); let internal = build_internal_config(&blank, None).unwrap_or_else(|_| unreachable!()); blank.set_internal_config(internal); GwConfigDatabase { @@ -43,7 +43,7 @@ impl GwConfigDatabase { } /// Store the given config - pub fn store(&mut self, config: Arc) { + pub fn store(&mut self, config: Arc) { info!("Storing config for generation '{}' in db", config.genid()); self.applied = config; } @@ -56,7 +56,7 @@ impl GwConfigDatabase { /// Get a refcounted reference to the applied `GwConfig` #[must_use] - pub fn get_current_config(&self) -> Arc { + pub fn get_current_config(&self) -> Arc { self.applied.clone() } } diff --git a/mgmt/src/processor/k8s_client.rs b/mgmt/src/processor/k8s_client.rs index 1a34603ecf..edaca99c9b 100644 --- a/mgmt/src/processor/k8s_client.rs +++ b/mgmt/src/processor/k8s_client.rs @@ -173,7 +173,7 @@ impl K8sClient { let callback = Arc::from(callback); // infinite loop - watch_gateway_agent_crd(&k8s_client2.hostname, callback.clone()).await; + watch_gateway_agent_crd(&k8s_client2.hostname, callback.clone()).await } pub async fn k8s_start_status_update(&self, status_update_interval: &std::time::Duration) { diff --git a/mgmt/src/processor/mgmt_client.rs b/mgmt/src/processor/mgmt_client.rs index 206599399d..a997bf88dc 100644 --- a/mgmt/src/processor/mgmt_client.rs +++ b/mgmt/src/processor/mgmt_client.rs @@ -7,7 +7,7 @@ use config::ConfigError; use config::ConfigResult; use config::GenId; use config::internal::status::DataplaneStatus; -use config::{ExternalConfig, ValidatedGwConfig}; +use config::{ExternalConfig, GwConfig}; use concurrency::sync::Arc; use tokio::sync::mpsc::Sender; @@ -31,7 +31,7 @@ pub(crate) enum ConfigRequest { #[derive(Debug)] pub(crate) enum ConfigResponse { ApplyConfig(ConfigResult), - GetCurrentConfig(Arc), + GetCurrentConfig(Arc), GetGeneration(GenId), GetDataplaneStatus(Box), } @@ -96,7 +96,7 @@ impl ConfigClient { /// # Errors /// This method returns `ConfigProcessorError` if the request could not be sent or the response /// could not be received. - pub async fn get_current_config(&self) -> Result, ConfigProcessorError> { + pub async fn get_current_config(&self) -> Result, ConfigProcessorError> { let (req, rx) = ConfigChannelRequest::new(ConfigRequest::GetCurrentConfig); self.tx.send(req).await?; let gwconfig = match rx.await? { diff --git a/mgmt/src/processor/proc.rs b/mgmt/src/processor/proc.rs index cb1436c7e0..f4fabc1e79 100644 --- a/mgmt/src/processor/proc.rs +++ b/mgmt/src/processor/proc.rs @@ -4,19 +4,19 @@ //! Configuration processor use concurrency::sync::Arc; -use config::external::overlay::ValidatedOverlay; +use config::external::overlay::Overlay; use flow_entry::flow_table::FlowTable; use std::collections::{HashMap, HashSet}; use tokio::sync::RwLock; use tokio::sync::mpsc; -use config::external::overlay::vpc::ValidatedVpcTable; +use config::external::overlay::vpc::VpcTable; use config::internal::device::tracecfg::TracingConfig; use config::internal::status::{ DataplaneStatus, FrrStatus, VpcCounters, VpcPeeringCounters, VpcStatus, }; use config::{ConfigError, ConfigResult, stringify}; -use config::{DeviceConfig, ExternalConfig, GenId, InternalConfig, ValidatedGwConfig}; +use config::{DeviceConfig, ExternalConfig, GenId, GwConfig, InternalConfig}; use crate::processor::confbuild::internal::build_internal_config; use crate::processor::confbuild::router::generate_router_config; @@ -145,7 +145,7 @@ impl ConfigProcessor { /// Main entry point for new configurations pub(crate) async fn process_incoming_config(&mut self, config: ExternalConfig) -> ConfigResult { - let mut validated_config = config.validate()?; + let mut validated_config = GwConfig::from_external(config)?; let internal = build_internal_config(&validated_config, self.proc_params.bmp_options.clone())?; validated_config.set_internal_config(internal); @@ -154,7 +154,7 @@ impl ConfigProcessor { async fn update_history( &mut self, - config: &ValidatedGwConfig, + config: &GwConfig, result: &ConfigResult, is_rollback: bool, ) { @@ -174,7 +174,7 @@ impl ConfigProcessor { } /// Apply a configuration. On success, store it. On failure, roll-back. Update the history in either case. - async fn apply(&mut self, config: ValidatedGwConfig) -> ConfigResult { + async fn apply(&mut self, config: GwConfig) -> ConfigResult { let config = Arc::from(config); let result = self.apply_gw_config(config.clone()).await; self.update_history(&config, &result, false).await; @@ -454,7 +454,7 @@ impl VpcManager { /// Build router config and apply it over the router control channel async fn apply_router_config( kernel_vrfs: &HashMap, - config: Arc, + config: Arc, router_ctl: &mut RouterCtlSender, ) -> ConfigResult { let genid = config.genid(); @@ -476,7 +476,7 @@ async fn apply_router_config( /// /// Returns the list of `(VpcDiscriminant, name)` so the caller can seed the stats store. fn update_stats_vpc_mappings( - config: &ValidatedGwConfig, + config: &GwConfig, vpcmapw: &mut VpcMapWriter, ) -> Vec<(VpcDiscriminant, String)> { // create a mapping table from the vpc table in the config @@ -499,10 +499,7 @@ fn update_stats_vpc_mappings( } /// Update the Nat tables for static NAT -fn apply_static_nat_config( - vpc_table: &ValidatedVpcTable, - nattablesw: &mut NatTablesWriter, -) -> ConfigResult { +fn apply_static_nat_config(vpc_table: &VpcTable, nattablesw: &mut NatTablesWriter) -> ConfigResult { let nat_table = build_nat_configuration(vpc_table)?; nattablesw.update_nat_tables(nat_table); debug!("Successfully updated the static NAT configuration"); @@ -512,7 +509,7 @@ fn apply_static_nat_config( /// Update the config for masquerade. /// This is now infallible. Validation should ensure it is. fn apply_masquerade_config( - vpc_table: &ValidatedVpcTable, + vpc_table: &VpcTable, flow_table: &FlowTable, natallocatorw: &mut NatAllocatorWriter, genid: GenId, @@ -523,7 +520,7 @@ fn apply_masquerade_config( } fn apply_flow_filtering_config( - overlay: &ValidatedOverlay, + overlay: &Overlay, flowfilterw: &mut FlowFilterTableWriter, ) -> ConfigResult { let flow_filter_table = FlowFilterTable::build_from_overlay(overlay)?; @@ -533,7 +530,7 @@ fn apply_flow_filtering_config( } fn apply_port_forwarding_config( - vpc_table: &ValidatedVpcTable, + vpc_table: &VpcTable, portfw_w: &mut PortFwTableWriter, ) -> ConfigResult { let ruleset = build_port_forwarding_configuration(vpc_table)?; @@ -568,7 +565,7 @@ fn apply_device_config(device: &DeviceConfig) -> ConfigResult { impl ConfigProcessor { /// Main method to apply a config - async fn apply_gw_config(&mut self, config: Arc) -> Result<(), ConfigError> { + async fn apply_gw_config(&mut self, config: Arc) -> Result<(), ConfigError> { let genid = config.genid(); debug!("Applying config with genid '{genid}'..."); diff --git a/mgmt/src/tests/mgmt.rs b/mgmt/src/tests/mgmt.rs index 778c2ffdca..cebb22b2c3 100644 --- a/mgmt/src/tests/mgmt.rs +++ b/mgmt/src/tests/mgmt.rs @@ -28,6 +28,7 @@ pub mod test { use config::external::underlay::Underlay; use config::ExternalConfig; + use config::GwConfig; use config::internal::device::DeviceConfig; use config::internal::interfaces::interface::{ IfEthConfig, IfVtepConfig, InterfaceConfig, InterfaceType, @@ -393,7 +394,7 @@ pub mod test { /* Not really a test but a tool to check generated FRR configs given a gateway config */ let external = sample_external_config(); let peering_table = external.overlay.peering_table.clone(); - let validated_config = external.validate().expect("Config validation failed"); + let validated_config = GwConfig::from_external(external).expect("Config validation failed"); if false { let vpc_table = validated_config.external().overlay().vpc_table(); println!("\n{}\n{peering_table}", vpc_table.as_summary()); diff --git a/nat/src/masquerade/allocator_writer.rs b/nat/src/masquerade/allocator_writer.rs index e8305303a8..37663de5dc 100644 --- a/nat/src/masquerade/allocator_writer.rs +++ b/nat/src/masquerade/allocator_writer.rs @@ -5,8 +5,8 @@ use crate::masquerade::apalloc::NatAllocator; use concurrency::slot::SlotOption; use concurrency::sync::Arc; use config::GenId; -use config::external::overlay::vpc::{ValidatedPeering, ValidatedVpcTable}; -use config::external::overlay::vpcpeering::ValidatedExpose; +use config::external::overlay::vpc::{Peering, VpcTable}; +use config::external::overlay::vpcpeering::VpcExpose; use flow_entry::flow_table::FlowTable; use net::packet::VpcDiscriminant; use tracing::debug; @@ -19,7 +19,7 @@ use crate::masquerade::flows::upgrade_all_masquerading_flows; pub(crate) struct MasqueradePeering { pub(crate) src_vpcd: VpcDiscriminant, pub(crate) dst_vpcd: VpcDiscriminant, - pub(crate) peering: ValidatedPeering, + pub(crate) peering: Peering, } #[derive(Debug, Default, Clone)] pub struct MasqueradeConfig { @@ -36,7 +36,7 @@ impl PartialEq for MasqueradeConfig { impl MasqueradeConfig { #[must_use] - pub fn new(vpc_table: &ValidatedVpcTable, genid: GenId) -> Self { + pub fn new(vpc_table: &VpcTable, genid: GenId) -> Self { let mut peerings = Vec::new(); for vpc in vpc_table.values() { for peering in vpc.local_stateful_nat_peerings() { @@ -75,12 +75,10 @@ impl MasqueradeConfig { } pub(crate) fn has_masquerading_peerings(&self) -> bool { - self.peerings.iter().map(|p| &p.peering).any(|p| { - p.local() - .valexp() - .iter() - .any(ValidatedExpose::has_masquerade) - }) + self.peerings + .iter() + .map(|p| &p.peering) + .any(|p| p.local().valexp().iter().any(VpcExpose::has_masquerade)) } pub(crate) fn get_peering( diff --git a/nat/src/masquerade/apalloc/setup.rs b/nat/src/masquerade/apalloc/setup.rs index 7c587a7345..1cb3c61d6c 100644 --- a/nat/src/masquerade/apalloc/setup.rs +++ b/nat/src/masquerade/apalloc/setup.rs @@ -6,8 +6,8 @@ use super::alloc::{IpAllocator, NatPool, PoolBitmap}; use super::{NatAllocator, PoolTable, PoolTableKey}; use crate::masquerade::natip::NatIp; use crate::ranges::IpRange; -use config::external::overlay::vpc::ValidatedPeering; -use config::external::overlay::vpcpeering::{ValidatedExpose, ValidatedManifest}; +use config::external::overlay::vpc::Peering; +use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest}; use lpm::prefix::range_map::DisjointRangesBTreeMap; use lpm::prefix::{ IpPrefix, L4Protocol, PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts, @@ -21,16 +21,12 @@ use tracing::error; const DEFAULT_MASQUERADE_IDLE_TIMEOUT: Duration = Duration::from_mins(2); impl NatAllocator { - pub(crate) fn add_peering_addresses( - &mut self, - peering: &ValidatedPeering, - dst_vpc_id: VpcDiscriminant, - ) { + pub(crate) fn add_peering_addresses(&mut self, peering: &Peering, dst_vpc_id: VpcDiscriminant) { build_nat_pool_generic( peering.local(), dst_vpc_id, - ValidatedManifest::masquerade_exposes_44, - ValidatedManifest::port_forwarding_exposes_44, + VpcManifest::masquerade_exposes_44, + VpcManifest::port_forwarding_exposes_44, &mut self.pools_src44, NextHeader::ICMP, self.randomize, @@ -39,8 +35,8 @@ impl NatAllocator { build_nat_pool_generic( peering.local(), dst_vpc_id, - ValidatedManifest::masquerade_exposes_66, - ValidatedManifest::port_forwarding_exposes_66, + VpcManifest::masquerade_exposes_66, + VpcManifest::port_forwarding_exposes_66, &mut self.pools_src66, NextHeader::ICMP6, self.randomize, @@ -50,7 +46,7 @@ impl NatAllocator { #[allow(clippy::too_many_arguments)] fn build_nat_pool_generic<'a, I: NatIpWithBitmap, J: NatIpWithBitmap, F, FIter, P, PIter>( - manifest: &'a ValidatedManifest, + manifest: &'a VpcManifest, dst_vpc_id: VpcDiscriminant, // A filter to select relevant exposes: those with masquerade, for the relevant IP version exposes_filter: F, @@ -60,12 +56,12 @@ fn build_nat_pool_generic<'a, I: NatIpWithBitmap, J: NatIpWithBitmap, F, FIter, icmp_proto: NextHeader, randomize: bool, ) where - F: FnOnce(&'a ValidatedManifest) -> FIter, - FIter: Iterator, - P: FnOnce(&'a ValidatedManifest) -> PIter, - PIter: Iterator, + F: FnOnce(&'a VpcManifest) -> FIter, + FIter: Iterator, + P: FnOnce(&'a VpcManifest) -> PIter, + PIter: Iterator, { - let port_forwarding_exposes: Vec<&'a ValidatedExpose> = + let port_forwarding_exposes: Vec<&'a VpcExpose> = port_forwarding_exposes_filter(manifest).collect(); exposes_filter(manifest).for_each(|expose| { @@ -120,8 +116,8 @@ struct ReserveSets { } fn find_masquerade_portfw_overlap<'a>( - port_forwarding_exposes: &Vec<&'a ValidatedExpose>, - expose: &'a ValidatedExpose, + port_forwarding_exposes: &Vec<&'a VpcExpose>, + expose: &'a VpcExpose, ) -> ReserveSets { let expose_nat = expose.nat().unwrap_or_else(|| unreachable!()); let mut reserve_sets = ReserveSets::default(); @@ -308,31 +304,28 @@ mod tests { #[test] fn find_masquerade_portfw_overlap_multiple_pf_exposes() { - let expose = VpcExpose::empty() + let mut expose = VpcExpose::empty() .make_masquerade(None) .unwrap() .ip("10.0.0.0/16".into()) .ip("172.16.0.0/16".into()) .as_range("192.168.0.0/16".into()) - .unwrap() - .validate() .unwrap(); - let pf_expose1 = VpcExpose::empty() + expose.validate().unwrap(); + let mut pf_expose1 = VpcExpose::empty() .make_port_forwarding(None, None) .unwrap() .ip(prefix_with_ports("10.0.1.0/24", 8080, 8090)) .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() .unwrap(); - let pf_expose2 = VpcExpose::empty() + pf_expose1.validate().unwrap(); + let mut pf_expose2 = VpcExpose::empty() .make_port_forwarding(None, None) .unwrap() .ip(prefix_with_ports("172.16.5.0/24", 8080, 8090)) .as_range(prefix_with_ports("192.168.2.0/24", 8080, 8090)) - .unwrap() - .validate() .unwrap(); + pf_expose2.validate().unwrap(); let pf_exposes_vec = vec![&pf_expose1, &pf_expose2]; let result = find_masquerade_portfw_overlap(&pf_exposes_vec, &expose); assert_eq!( @@ -352,22 +345,20 @@ mod tests { #[test] fn find_masquerade_portfw_overlap_with_ports() { - let expose = VpcExpose::empty() + let mut expose = VpcExpose::empty() .make_masquerade(None) .unwrap() .ip("10.0.0.0/24".into()) .as_range("192.168.0.0/24".into()) - .unwrap() - .validate() .unwrap(); - let pf_expose = VpcExpose::empty() + expose.validate().unwrap(); + let mut pf_expose = VpcExpose::empty() .make_port_forwarding(None, None) .unwrap() .ip(prefix_with_ports("10.0.0.0/24", 8080, 8090)) .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() .unwrap(); + pf_expose.validate().unwrap(); let pf_exposes_vec = vec![&pf_expose]; let result = find_masquerade_portfw_overlap(&pf_exposes_vec, &expose); assert_eq!( @@ -381,22 +372,20 @@ mod tests { #[test] fn find_masquerade_portfw_overlap_with_ports_tcp() { - let expose = VpcExpose::empty() + let mut expose = VpcExpose::empty() .make_masquerade(None) .unwrap() .ip("10.0.0.0/24".into()) .as_range("192.168.0.0/24".into()) - .unwrap() - .validate() .unwrap(); - let pf_expose = VpcExpose::empty() + expose.validate().unwrap(); + let mut pf_expose = VpcExpose::empty() .make_port_forwarding(None, Some(L4Protocol::Tcp)) // TCP only .unwrap() .ip(prefix_with_ports("10.0.0.0/24", 8080, 8090)) .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() .unwrap(); + pf_expose.validate().unwrap(); let pf_exposes_vec = vec![&pf_expose]; let result = find_masquerade_portfw_overlap(&pf_exposes_vec, &expose); assert_eq!( @@ -411,30 +400,27 @@ mod tests { #[test] fn find_masquerade_portfw_overlap_duplicates_collapsed() { // Two port-forwarding exposes with the same prefix should produce one entry - let expose = VpcExpose::empty() + let mut expose = VpcExpose::empty() .make_masquerade(None) .unwrap() .ip("10.0.0.0/16".into()) .as_range("192.168.0.0/24".into()) - .unwrap() - .validate() .unwrap(); - let pf_expose1 = VpcExpose::empty() + expose.validate().unwrap(); + let mut pf_expose1 = VpcExpose::empty() .make_port_forwarding(None, None) .unwrap() .ip(prefix_with_ports("10.0.1.0/24", 8080, 8090)) .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() .unwrap(); - let pf_expose2 = VpcExpose::empty() + pf_expose1.validate().unwrap(); + let mut pf_expose2 = VpcExpose::empty() .make_port_forwarding(None, None) .unwrap() .ip(prefix_with_ports("10.0.1.0/24", 8080, 8090)) .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() .unwrap(); + pf_expose2.validate().unwrap(); let pf_exposes_vec = vec![&pf_expose1, &pf_expose2]; let result = find_masquerade_portfw_overlap(&pf_exposes_vec, &expose); assert_eq!( diff --git a/nat/src/masquerade/apalloc/test_alloc.rs b/nat/src/masquerade/apalloc/test_alloc.rs index b7c129e2f3..a4d6626072 100644 --- a/nat/src/masquerade/apalloc/test_alloc.rs +++ b/nat/src/masquerade/apalloc/test_alloc.rs @@ -11,7 +11,7 @@ mod context { use crate::masquerade::allocator_writer::MasqueradeConfig; use crate::masquerade::apalloc::alloc::IpAllocator; use crate::masquerade::apalloc::{NatAllocator, PoolTable, PoolTableKey}; - use config::external::overlay::vpc::{Peering, ValidatedVpcTable, Vpc, VpcTable}; + use config::external::overlay::vpc::{Peering, Vpc, VpcTable}; use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest}; use net::ip::NextHeader; use net::packet::VpcDiscriminant; @@ -71,7 +71,7 @@ mod context { .unwrap() } - fn build_context() -> ValidatedVpcTable { + fn build_context() -> VpcTable { // Exposes and manifests let expose1 = VpcExpose::empty() .make_masquerade(None) @@ -123,7 +123,8 @@ mod context { vpctable.add(vpc1).unwrap(); vpctable.add(vpc2).unwrap(); - vpctable.validate().unwrap() + vpctable.validate().unwrap(); + vpctable } pub fn build_allocator() -> NatAllocator { diff --git a/nat/src/masquerade/test.rs b/nat/src/masquerade/test.rs index 9b1f850f72..67d68d9602 100644 --- a/nat/src/masquerade/test.rs +++ b/nat/src/masquerade/test.rs @@ -107,7 +107,8 @@ fn test_setup( genid: GenId, overlay: &Overlay, ) -> (Arc, DynPipeline, NatAllocatorWriter) { - let overlay = overlay.validate().unwrap(); + let mut overlay = overlay.clone(); + overlay.validate().unwrap(); // build the configuration for the nat allocator let nat_config = MasqueradeConfig::new(overlay.vpc_table(), genid); @@ -375,15 +376,14 @@ fn flow_lookup(flow_table: &FlowTable, packet: &mut Packet #[cfg_attr(not(miri), traced_test)] #[allow(clippy::too_many_lines)] async fn test_full_config() { - let config = build_gwconfig_from_overlay(build_overlay_4vpcs()) - .validate() - .unwrap(); + let mut config = build_gwconfig_from_overlay(build_overlay_4vpcs()); + config.validate().unwrap(); let flow_table = FlowTable::new(16); // Check that we can validate the allocator let (mut nat, mut allocator) = Masquerade::new_with_defaults(); - let nat_config = MasqueradeConfig::new(config.external().overlay().vpc_table(), 1); + let nat_config = MasqueradeConfig::new(config.overlay().vpc_table(), 1); allocator.update_nat_allocator(nat_config, &flow_table); // No NAT @@ -455,10 +455,9 @@ async fn test_full_config() { assert_eq!(idle_timeout, ONE_MINUTE); // Update config and allocator - let new_config = build_gwconfig_from_overlay(build_overlay_2vpcs()) - .validate() - .unwrap(); - let nat_config = MasqueradeConfig::new(new_config.external().overlay().vpc_table(), 2); + let mut new_config = build_gwconfig_from_overlay(build_overlay_2vpcs()); + new_config.validate().unwrap(); + let nat_config = MasqueradeConfig::new(new_config.overlay().vpc_table(), 2); allocator.update_nat_allocator(nat_config, &flow_table); // Check existing connection @@ -545,13 +544,12 @@ fn build_overlay_2vpcs_no_nat() -> Overlay { #[test] #[cfg_attr(not(miri), traced_test)] fn test_full_config_no_nat() { - let config = build_gwconfig_from_overlay(build_overlay_2vpcs_no_nat()) - .validate() - .unwrap(); + let mut config = build_gwconfig_from_overlay(build_overlay_2vpcs_no_nat()); + config.validate().unwrap(); // Check that we can validate the allocator let (_, mut allocator) = Masquerade::new_with_defaults(); - let nat_config = MasqueradeConfig::new(config.external().overlay().vpc_table(), 1); + let nat_config = MasqueradeConfig::new(config.overlay().vpc_table(), 1); allocator.update_nat_allocator(nat_config, &FlowTable::new(16)); } @@ -616,13 +614,12 @@ fn check_packet_icmp_echo_new( #[tokio::test] #[cfg_attr(not(emulated), traced_test)] async fn test_icmp_echo_nat() { - let config = build_gwconfig_from_overlay(build_overlay_2vpcs()) - .validate() - .unwrap(); + let mut config = build_gwconfig_from_overlay(build_overlay_2vpcs()); + config.validate().unwrap(); // Check that we can validate the allocator let (mut nat, mut allocator) = Masquerade::new_with_defaults(); - let nat_config = MasqueradeConfig::new(config.external().overlay().vpc_table(), 1); + let nat_config = MasqueradeConfig::new(config.overlay().vpc_table(), 1); allocator.update_nat_allocator(nat_config, &FlowTable::new(16)); // No NAT @@ -931,13 +928,12 @@ fn build_overlay_2vpcs_with_default() -> Overlay { #[tokio::test] async fn test_default_expose() { - let config = build_gwconfig_from_overlay(build_overlay_2vpcs_with_default()) - .validate() - .unwrap(); + let mut config = build_gwconfig_from_overlay(build_overlay_2vpcs_with_default()); + config.validate().unwrap(); // Check that we can validate the allocator let (mut nat, mut allocator) = Masquerade::new_with_defaults(); - let nat_config = MasqueradeConfig::new(config.external().overlay().vpc_table(), 1); + let nat_config = MasqueradeConfig::new(config.overlay().vpc_table(), 1); allocator.update_nat_allocator(nat_config, &FlowTable::new(16)); // Using the expose with a prefix @@ -1136,13 +1132,12 @@ async fn test_full_config_unidirectional_nat_overlapping_destination() { let _ = tctl.setup_from_string("vpc-routing=debug,flow-lookup=debug,masquerade=debug"); } - let config = - build_gwconfig_from_overlay(build_overlay_3vpcs_unidirectional_nat_overlapping_addr()) - .validate() - .unwrap(); + let mut config = + build_gwconfig_from_overlay(build_overlay_3vpcs_unidirectional_nat_overlapping_addr()); + config.validate().unwrap(); // Build VPC discriminant lookup stage - let vpcd_tables = FlowFilterTable::build_from_overlay(config.external().overlay()).unwrap(); + let vpcd_tables = FlowFilterTable::build_from_overlay(config.overlay()).unwrap(); let mut vpcdtablesw = FlowFilterTableWriter::new(); vpcdtablesw.update_flow_filter_table(vpcd_tables); let mut vpcdlookup = FlowFilter::new("vpcd-lookup", vpcdtablesw.get_reader()); @@ -1154,7 +1149,7 @@ async fn test_full_config_unidirectional_nat_overlapping_destination() { // Build NAT stage let (mut nat, mut allocator) = Masquerade::new_with_defaults(); - let nat_config = MasqueradeConfig::new(config.external().overlay().vpc_table(), 1); + let nat_config = MasqueradeConfig::new(config.overlay().vpc_table(), 1); // Check that we can validate the allocator allocator.update_nat_allocator(nat_config, &FlowTable::new(16)); @@ -1221,8 +1216,7 @@ async fn test_full_config_unidirectional_nat_overlapping_destination() { // Build a new NAT stage let mut allocator = NatAllocatorWriter::new(); let mut nat = Masquerade::new("masquerade", flow_table.clone(), allocator.get_reader()); - let nat_config = - MasqueradeConfig::new(config.external().overlay().vpc_table(), 2).set_randomize(false); + let nat_config = MasqueradeConfig::new(config.overlay().vpc_table(), 2).set_randomize(false); // Check that we can validate the allocator // @@ -1420,7 +1414,7 @@ fn build_overlay_2vpcs_unidirectional_nat_overlapping_exposes() -> Overlay { #[cfg_attr(not(emulated), traced_test)] #[allow(clippy::too_many_lines)] async fn test_full_config_unidirectional_nat_overlapping_exposes_for_single_peering() { - let config = + let mut config = build_gwconfig_from_overlay(build_overlay_2vpcs_unidirectional_nat_overlapping_exposes()); // Validation fails - We currently forbid multiple peerings between any pair of VPCs. We // could probably allow them for masquerade, but we still need the restriction for static NAT. @@ -1831,7 +1825,8 @@ async fn test_masquerade_reconfig_keep_flow() { assert_eq!(flow_genid(&out).unwrap(), genid); // update the NAT allocator with an identical config - let overlay = build_overlay_2vpcs().validate().unwrap(); + let mut overlay = build_overlay_2vpcs(); + overlay.validate().unwrap(); let nat_config = MasqueradeConfig::new(overlay.vpc_table(), genid + 1); allocw.update_nat_allocator(nat_config, &flow_table); @@ -1867,7 +1862,8 @@ async fn test_masquerade_reconfig_drop_flow() { assert_eq!(flow_genid(&out).unwrap(), genid); // update the NAT allocator with an identical config - let overlay = build_overlay_2vpcs_modified().validate().unwrap(); + let mut overlay = build_overlay_2vpcs_modified(); + overlay.validate().unwrap(); let nat_config = MasqueradeConfig::new(overlay.vpc_table(), genid + 1); allocw.update_nat_allocator(nat_config, &flow_table); diff --git a/nat/src/portfw/portfwtable/access.rs b/nat/src/portfw/portfwtable/access.rs index 400e28a701..0c383339ae 100644 --- a/nat/src/portfw/portfwtable/access.rs +++ b/nat/src/portfw/portfwtable/access.rs @@ -7,7 +7,7 @@ use super::super::build_port_forwarding_configuration; use super::PortFwTableError; use super::objects::{PortFwEntry, PortFwTable}; -use config::external::overlay::vpc::ValidatedVpcTable; +use config::external::overlay::vpc::VpcTable; use left_right::{Absorb, ReadGuard, ReadHandle, ReadHandleFactory, WriteHandle}; #[allow(unused)] @@ -59,10 +59,7 @@ impl PortFwTableWriter { self.0.publish(); // intended Ok(()) } - pub fn update_from_vpc_table( - &mut self, - vpc_table: &ValidatedVpcTable, - ) -> Result<(), PortFwTableError> { + pub fn update_from_vpc_table(&mut self, vpc_table: &VpcTable) -> Result<(), PortFwTableError> { let ruleset = build_port_forwarding_configuration(vpc_table) .map_err(|e| PortFwTableError::Unsupported(e.to_string()))?; self.update_table(&ruleset) diff --git a/nat/src/portfw/portfwtable/setup.rs b/nat/src/portfw/portfwtable/setup.rs index 812e14b329..ba9bc24e47 100644 --- a/nat/src/portfw/portfwtable/setup.rs +++ b/nat/src/portfw/portfwtable/setup.rs @@ -6,18 +6,18 @@ use crate::portfw::{PortFwEntry, PortFwKey, PortFwTableError}; use config::ConfigError; -use config::external::overlay::vpc::{ValidatedPeering, ValidatedVpc, ValidatedVpcTable}; -use config::external::overlay::vpcpeering::ValidatedExpose; +use config::external::overlay::vpc::{Peering, Vpc, VpcTable}; +use config::external::overlay::vpcpeering::VpcExpose; use lpm::prefix::L4Protocol; use net::ip::NextHeader; use net::packet::VpcDiscriminant; -fn port_fw_proto(expose: &ValidatedExpose) -> L4Protocol { +fn port_fw_proto(expose: &VpcExpose) -> L4Protocol { expose.nat().unwrap_or_else(|| unreachable!()).proto } fn expose_to_portfw_rule( - expose: &ValidatedExpose, + expose: &VpcExpose, proto: NextHeader, src_vpc: VpcDiscriminant, dst_vpc: VpcDiscriminant, @@ -59,9 +59,9 @@ fn expose_to_portfw_rule( ) } fn vpc_port_fw_peering( - vpc_table: &ValidatedVpcTable, + vpc_table: &VpcTable, dst_vpc: VpcDiscriminant, - peering: &ValidatedPeering, + peering: &Peering, ) -> Result, PortFwTableError> { let mut rules = vec![]; for expose in peering.local().port_forwarding_exposes() { @@ -87,10 +87,7 @@ fn vpc_port_fw_peering( } Ok(rules) } -fn vpc_port_fw( - vpc_table: &ValidatedVpcTable, - vpc: &ValidatedVpc, -) -> Result, PortFwTableError> { +fn vpc_port_fw(vpc_table: &VpcTable, vpc: &Vpc) -> Result, PortFwTableError> { let mut collected = vec![]; let dst_vpc = VpcDiscriminant::from_vni(vpc.vni()); for peering in vpc.peerings() { @@ -101,7 +98,7 @@ fn vpc_port_fw( } pub fn build_port_forwarding_configuration( - vpc_table: &ValidatedVpcTable, + vpc_table: &VpcTable, ) -> Result, ConfigError> { let mut ruleset = vec![]; for vpc in vpc_table.values() { diff --git a/nat/src/static_nat/setup/mod.rs b/nat/src/static_nat/setup/mod.rs index 9fbd12c5e2..b30dece371 100644 --- a/nat/src/static_nat/setup/mod.rs +++ b/nat/src/static_nat/setup/mod.rs @@ -13,8 +13,8 @@ pub mod tables; use tables::{NatTableValue, NatTables, PerVniTable}; use config::ConfigError; -use config::external::overlay::vpc::{ValidatedPeering, ValidatedVpcTable}; -use config::external::overlay::vpcpeering::ValidatedExpose; +use config::external::overlay::vpc::{Peering, VpcTable}; +use config::external::overlay::vpcpeering::VpcExpose; use lpm::prefix::{Prefix, PrefixWithOptionalPorts}; use net::vxlan::Vni; use std::collections::BTreeSet; @@ -35,13 +35,13 @@ pub(crate) fn generate_nat_values<'a>( } fn generate_public_values( - expose: &ValidatedExpose, + expose: &VpcExpose, ) -> impl Iterator> { generate_nat_values(expose.ips(), expose.as_range_or_empty()) } fn generate_private_values( - expose: &ValidatedExpose, + expose: &VpcExpose, ) -> impl Iterator> { generate_nat_values(expose.as_range_or_empty(), expose.ips()) } @@ -54,7 +54,7 @@ impl PerVniTable { /// Returns an error if some lists of prefixes contain duplicates pub(crate) fn add_peering( &mut self, - peering: &ValidatedPeering, + peering: &Peering, dst_vni: Vni, ) -> Result<(), NatPeeringError> { peering @@ -103,7 +103,7 @@ impl PerVniTable { /// # Errors /// /// Returns [`ConfigError::FailureApply`] if the configuration for some NAT peering cannot be built. -pub fn build_nat_configuration(vpc_table: &ValidatedVpcTable) -> Result { +pub fn build_nat_configuration(vpc_table: &VpcTable) -> Result { let mut nat_tables = NatTables::new(); for vpc in vpc_table.values() { let mut table = PerVniTable::new(); @@ -199,9 +199,11 @@ mod tests { src_vpc.peerings.push(peering.clone()); vpctable.add(src_vpc).unwrap(); + let mut peering_v = peering.clone(); + peering_v.validate().unwrap(); let mut vni_table = PerVniTable::new(); vni_table - .add_peering(&peering.validate().unwrap(), dst_vni) + .add_peering(&peering_v, dst_vni) .expect("Failed to build NAT tables"); } } diff --git a/nat/src/static_nat/test.rs b/nat/src/static_nat/test.rs index 148027da5d..7055758bc2 100644 --- a/nat/src/static_nat/test.rs +++ b/nat/src/static_nat/test.rs @@ -17,7 +17,7 @@ use config::internal::interfaces::interface::InterfaceConfig; use config::internal::interfaces::interface::{IfVtepConfig, InterfaceType}; use config::internal::routing::bgp::BgpConfig; use config::internal::routing::vrf::VrfConfig; -use config::{ExternalConfig, ValidatedGwConfig}; +use config::{ExternalConfig, GwConfig}; use crate::StaticNat; use crate::static_nat::setup::build_nat_configuration; @@ -256,14 +256,18 @@ fn build_context() -> NatTables { let mut nat_table = NatTables::new(); + let mut peering1_v = peering1.clone(); + peering1_v.validate().unwrap(); let mut vni_table1 = PerVniTable::new(); vni_table1 - .add_peering(&peering1.validate().unwrap(), vni2) + .add_peering(&peering1_v, vni2) .expect("Failed to build NAT tables"); + let mut peering2_v = peering2.clone(); + peering2_v.validate().unwrap(); let mut vni_table2 = PerVniTable::new(); vni_table2 - .add_peering(&peering2.validate().unwrap(), vni1) + .add_peering(&peering2_v, vni1) .expect("Failed to build NAT tables"); nat_table.add_table(vni_table1, vni1); @@ -372,7 +376,7 @@ fn test_nat_icmp_error_msg_static_44() { } #[allow(clippy::too_many_lines)] -fn build_sample_config() -> ValidatedGwConfig { +fn build_sample_config() -> GwConfig { fn add_expose(manifest: &mut VpcManifest, expose: VpcExpose) { manifest.add_expose(expose); } @@ -555,7 +559,7 @@ fn build_sample_config() -> ValidatedGwConfig { let overlay = Overlay::new(vpc_table, peering_table); - build_gwconfig_from_overlay(overlay).validate().unwrap() + GwConfig::from_external(build_gwconfig_from_overlay(overlay)).unwrap() } // Use the provided overlay with some default configuration to build a valid `ExternalConfig`. This @@ -785,7 +789,7 @@ fn test_full_config() { fn build_gwconfig_from_exposes( exposes_left: Vec, exposes_right: Vec, -) -> ValidatedGwConfig { +) -> GwConfig { fn add_expose(manifest: &mut VpcManifest, expose: VpcExpose) { manifest.add_expose(expose); } @@ -810,9 +814,7 @@ fn build_gwconfig_from_exposes( let overlay = Overlay::new(vpc_table, peering_table); - build_gwconfig_from_overlay(overlay.clone()) - .validate() - .unwrap() + GwConfig::from_external(build_gwconfig_from_overlay(overlay.clone())).unwrap() } fn check_packet_with_ports( diff --git a/nat/src/test.rs b/nat/src/test.rs index e4157fc910..e6bf93e9aa 100644 --- a/nat/src/test.rs +++ b/nat/src/test.rs @@ -11,9 +11,9 @@ use crate::portfw::{PortForwarder, PortFwTableWriter}; use crate::static_nat::NatTablesWriter; use crate::static_nat::setup::build_nat_configuration; use concurrency::sync::Arc; +use config::external::overlay::Overlay; use config::external::overlay::vpc::{Vpc, VpcTable}; use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest, VpcPeering, VpcPeeringTable}; -use config::external::overlay::{Overlay, ValidatedOverlay}; use flow_entry::flow_table::FlowLookup; use flow_entry::flow_table::FlowTable; use flow_filter::{FlowFilter, FlowFilterTable, FlowFilterTableWriter}; @@ -65,7 +65,7 @@ fn build_packet( } fn setup_masq_pipeline( - overlay: &ValidatedOverlay, + overlay: &Overlay, ) -> ( DynPipeline, Arc, @@ -185,7 +185,8 @@ async fn test_nat_combination_static_masquerade() { "default".into(), )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); // Build pipeline // @@ -295,7 +296,8 @@ async fn test_nat_combination_static_portfw() { "default".into(), )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); // Build pipeline // @@ -416,7 +418,8 @@ async fn test_nat_combination_static_masq_icmp_error() { "default".into(), )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); // Build pipeline // @@ -544,7 +547,8 @@ async fn test_nat_combination_static_portfwd_icmp_error() { "default".into(), )) .unwrap(); - let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + let mut overlay = Overlay::new(vpc_table, peering_table); + overlay.validate().unwrap(); // Build pipeline // diff --git a/routing/src/cli/handler.rs b/routing/src/cli/handler.rs index dbba5f992d..7d8c77bd75 100644 --- a/routing/src/cli/handler.rs +++ b/routing/src/cli/handler.rs @@ -23,7 +23,7 @@ use crate::routingdb::RoutingDb; use chrono::Local; use cli::cliproto::{CliAction, CliError, CliRequest, CliResponse, RequestArgs, RouteProtocol}; use concurrency::sync::Arc; -use config::{ConfigSummary, GwConfigMeta, ValidatedGwConfig}; +use config::{ConfigSummary, GwConfig, GwConfigMeta}; use lpm::prefix::{Ipv4Prefix, Ipv6Prefix}; use net::vxlan::Vni; use std::os::unix::net::SocketAddr; @@ -384,7 +384,7 @@ fn show_provider( CliResponse::from_request_ok(request, data) } -fn show_config(request: CliRequest, config: Option<&Arc>) -> CliResponse { +fn show_config(request: CliRequest, config: Option<&Arc>) -> CliResponse { let Some(config) = config else { return CliResponse::from_request_ok(request, "No configuration is applied".to_string()); }; diff --git a/routing/src/router/ctl.rs b/routing/src/router/ctl.rs index 4c144386e2..d4bb6e2773 100644 --- a/routing/src/router/ctl.rs +++ b/routing/src/router/ctl.rs @@ -4,7 +4,7 @@ //! Control channel for the router use concurrency::sync::Arc; -use config::{GwConfigMeta, ValidatedGwConfig}; +use config::{GwConfig, GwConfigMeta}; use interface_manager::monitor::EthEvent; use mio::{Interest, Waker}; use tokio::sync::mpsc::Sender; @@ -56,7 +56,7 @@ pub(crate) enum RouterCtlMsg { GuardedUnlock, Configure(RouterConfig, RouterCtlReplyTx), GetFrrAppliedConfig(RouterCtlReplyTx), - Config(Arc), + Config(Arc), ConfigHistory(Arc>), IfEvent(EthEvent), } @@ -154,7 +154,7 @@ impl RouterCtlSender { }; Ok(frr_cfg) } - pub async fn send_config(&mut self, config: Arc) -> Result<(), RouterError> { + pub async fn send_config(&mut self, config: Arc) -> Result<(), RouterError> { let msg = RouterCtlMsg::Config(config); self.send_and_wake(msg).await } @@ -242,7 +242,7 @@ fn handle_get_frr_applied_config(rio: &Rio, reply_to: RouterCtlReplyTx) { }); } -fn handle_config(rio: &mut Rio, config: Arc) { +fn handle_config(rio: &mut Rio, config: Arc) { rio.gwconfig = Some(config); } fn handle_config_history(rio: &mut Rio, history: Arc>) { diff --git a/routing/src/router/rio.rs b/routing/src/router/rio.rs index e5120eb85c..16d17bc8f6 100644 --- a/routing/src/router/rio.rs +++ b/routing/src/router/rio.rs @@ -20,7 +20,7 @@ use crate::routingdb::RoutingDb; use bytes::BytesMut; use cli::IoCache; use cli::cliproto::{CLI_RX_BUFF_SIZE, CliRequest}; -use config::{GwConfigMeta, ValidatedGwConfig}; +use config::{GwConfig, GwConfigMeta}; use dplane_rpc::socks::RpcCachedSock; use inotify::{EventMask, Inotify, WatchMask}; use lifecycle::{CancellationToken, Subsystem}; @@ -152,7 +152,7 @@ pub(crate) struct Rio { pub(crate) waker: Arc, pub(crate) cpistats: CpiStats, stale_timeout: Option, - pub(crate) gwconfig: Option>, + pub(crate) gwconfig: Option>, pub(crate) cfg_history: Arc>, pub(crate) cli_cache: IoCache, pub(crate) inotify: Inotify, diff --git a/validator/src/main.rs b/validator/src/main.rs index 61515e07ca..1a7706cb23 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -123,12 +123,12 @@ fn deserialize(ga_input: &str) -> Result { /// Main validation function fn validate(gwagent_json: &str) -> Result<(), ValidateError> { let crd = deserialize(gwagent_json)?; - let external = ExternalConfig::try_from(&crd).map_err(|e| match e { + let mut external = ExternalConfig::try_from(&crd).map_err(|e| match e { FromK8sConversionError::K8sInfra(e) => ValidateError::MetadataError(e.to_string()), _ => ValidateError::ConversionError(e.to_string()), })?; - let _ = external.validate().map_err(|e| { + external.validate().map_err(|e| { let mut config = ConfigErrors::default(); config.errors.push(e.to_string()); ValidateError::Configuration(config)