minor fixups for config

config/src/converters/k8s/status/bgp.rs

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Open Network Fabric Authors
 
-//! Converters for internal BGP status -> K8s GatewayAgentStatusStateBgp CRD.
+//! Converters for internal BGP status -> K8s `GatewayAgentStatusStateBgp` CRD.
 
 use std::collections::BTreeMap;
 

config/src/converters/k8s/status/dataplane_status.rs

@@ -136,7 +136,7 @@ mod tests {
             let last_applied_time = time_gen.generate(d)?;
             let status = d
                 .produce::<Option<LegalValue<DataplaneStatus>>>()?
-                .map(|v| v.take());
+                .map(LegalValue::take);
             let last_collected_time_raw = time_gen.generate(d)?;
             let last_collected_time = status.as_ref().map(|_| last_collected_time_raw);
             let last_heartbeat_raw = time_gen.generate(d)?;

config/src/display.rs

@@ -333,7 +333,7 @@ impl Display for GwConfigMeta {
         let error = self
             .error
             .as_ref()
-            .map_or("none".to_string(), |e| e.to_string());
+            .map_or("none".to_string(), std::string::ToString::to_string);
 
         let is_rollback = if self.is_rollback { "(rollback)" } else { "" };
 

config/src/external/communities.rs

@@ -16,6 +16,10 @@ impl PriorityCommunityTable {
         Self::default()
     }
     /// Insert a community
+    ///
+    /// # Errors
+    ///
+    /// Returns [`ConfigError::DuplicateCommunity`] if the community already exists in the table.
     pub fn insert(&mut self, order: usize, community: &str) -> Result<(), ConfigError> {
         if self.0.iter().any(|(_, comm)| comm == community) {
             return Err(ConfigError::DuplicateCommunity(community.to_string()));

config/src/external/gwgroup.rs

@@ -77,6 +77,12 @@ impl GwGroup {
         //N.B. we reverse the operands since want the most preferred first
         self.members.sort_by(|m1, m2| m2.cmp(m1));
     }
+    /// Add a member to the gateway group.
+    ///
+    /// # Errors
+    ///
+    /// Returns [`ConfigError::DuplicateMember`] if a member with the same name already exists.
+    /// Returns [`ConfigError::DuplicateMemberAddress`] if a member with the same IP address already exists.
     pub fn add_member(&mut self, member: GwGroupMember) -> Result<(), ConfigError> {
         if self.get_member_by_name(&member.name).is_some() {
             return Err(ConfigError::DuplicateMember(member.name.clone()));
@@ -126,6 +132,11 @@ impl GwGroupTable {
     pub fn new() -> Self {
         Self::default()
     }
+    /// Add a gateway group to the table.
+    ///
+    /// # Errors
+    ///
+    /// Returns [`ConfigError::DuplicateGroup`] if a group with the same name already exists.
     pub fn add_group(&mut self, group: GwGroup) -> Result<(), ConfigError> {
         if self.0.contains_key(group.name()) {
             return Err(ConfigError::DuplicateGroup(group.name().to_owned()));
@@ -332,7 +343,7 @@ mod test {
     fn test_bgp_community_setup() {
         let comtable = sample_community_table();
         let mut gwtable = build_sample_gw_groups();
-        gwtable.iter_mut().for_each(|group| group.sort_members());
+        gwtable.iter_mut().for_each(GwGroup::sort_members);
 
         println!("{gwtable}");
         println!("{comtable}");

config/src/external/mod.rs

@@ -93,6 +93,11 @@ impl ExternalConfig {
         }
         Ok(())
     }
+    /// Validate the external configuration.
+    ///
+    /// # Errors
+    ///
+    /// Returns a [`ConfigError`] if validation fails.
     pub fn validate(&mut self) -> ConfigResult {
         self.device.validate()?;
         self.underlay.validate()?;

config/src/external/overlay/mod.rs

@@ -39,6 +39,10 @@ impl Overlay {
     }
 
     /// 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 {
         debug!("Validating VPC peerings...");
         for peering in self.peering_table.values() {
@@ -60,6 +64,10 @@ impl Overlay {
     }
 
     /// Top most validation function for `Overlay` configuration
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the overlay configuration is invalid.
     pub fn validate(&mut self) -> ConfigResult {
         debug!("Validating overlay configuration...");
 

config/src/external/overlay/validation_tests.rs

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Open Network Fabric Authors
 
-//! Validation tests for VpcExpose / VpcPeering / Overlay
+//! Validation tests for `VpcExpose` / `VpcPeering` / Overlay
 //!
 //! These tests cover the expected semantics and restrictions for Expose objects in VPC peerings.
 //!

config/src/external/overlay/vpcpeering.rs

@@ -83,11 +83,11 @@ pub struct VpcExpose {
     pub nat: Option<VpcExposeNat>,
 }
 impl VpcExpose {
-    // Make the [`VpcExpose`] use stateless NAT.
-    //
-    // # Errors
-    //
-    // Returns an error if the [`VpcExpose`] already has a different NAT mode.
+    /// Make the [`VpcExpose`] use stateless NAT.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the [`VpcExpose`] already has a different NAT mode.
     pub fn make_stateless_nat(mut self) -> Result<Self, ConfigError> {
         match self.nat.as_mut() {
             Some(nat) if nat.is_stateless() => Ok(self),
@@ -103,12 +103,12 @@ impl VpcExpose {
         }
     }
 
-    // Make the [`VpcExpose`] use stateful NAT, with the given idle timeout, if provided.
-    // If the [`VpcExpose`] is already in stateful mode, the idle timeout is overwritten.
-    //
-    // # Errors
-    //
-    // Returns an error if the [`VpcExpose`] already has a different NAT mode.
+    /// Make the [`VpcExpose`] use stateful NAT, with the given idle timeout, if provided.
+    /// If the [`VpcExpose`] is already in stateful mode, the idle timeout is overwritten.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the [`VpcExpose`] already has a different NAT mode.
     pub fn make_stateful_nat(
         mut self,
         idle_timeout: Option<Duration>,
@@ -132,15 +132,15 @@ impl VpcExpose {
         }
     }
 
-    // Make the [`VpcExpose`] use port forwarding, with the given idle timeout, if provided, and the
-    // given L4 protocol, if provided.
-    //
-    // If the [`VpcExpose`] is already in port forwarding mode, the idle timeout and L4 protocol are
-    // overwritten.
-    //
-    // # Errors
-    //
-    // Returns an error if the [`VpcExpose`] already has a different NAT mode.
+    /// Make the [`VpcExpose`] use port forwarding, with the given idle timeout, if provided, and the
+    /// given L4 protocol, if provided.
+    ///
+    /// If the [`VpcExpose`] is already in port forwarding mode, the idle timeout and L4 protocol are
+    /// overwritten.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the [`VpcExpose`] already has a different NAT mode.
     pub fn make_port_forwarding(
         mut self,
         idle_timeout: Option<Duration>,
@@ -223,13 +223,23 @@ impl VpcExpose {
         self.nots.insert(prefix);
         self
     }
+    /// Add a prefix to the NAT `as` range.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the expose has no NAT configuration.
     pub fn as_range(mut self, prefix: PrefixWithOptionalPorts) -> Result<Self, ConfigError> {
         let nat = self.nat.as_mut().ok_or(ConfigError::MissingParameter(
             "'as' block requires NAT configuration for the expose",
         ))?;
         nat.as_range.insert(prefix);
         Ok(self)
     }
+    /// Add a prefix to the NAT `not as` exclusion set.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the expose has no NAT configuration.
     pub fn not_as(mut self, prefix: PrefixWithOptionalPorts) -> Result<Self, ConfigError> {
         let nat = self.nat.as_mut().ok_or(ConfigError::MissingParameter(
             "'not' prefix for 'as' block requires NAT configuration for the expose",
@@ -249,9 +259,15 @@ impl VpcExpose {
             // only V4 atm
             vec![Prefix::root_v4()]
         } else if let Some(nat) = self.nat.as_ref() {
-            nat.as_range.iter().map(|p| p.prefix()).collect::<Vec<_>>()
+            nat.as_range
+                .iter()
+                .map(PrefixWithOptionalPorts::prefix)
+                .collect::<Vec<_>>()
         } else {
-            self.ips.iter().map(|p| p.prefix()).collect::<Vec<_>>()
+            self.ips
+                .iter()
+                .map(PrefixWithOptionalPorts::prefix)
+                .collect::<Vec<_>>()
         }
     }
 
@@ -361,7 +377,11 @@ impl VpcExpose {
         Ok(())
     }
 
-    /// Validate the [`VpcExpose`]:
+    /// Validate the [`VpcExpose`].
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the expose configuration is invalid.
     #[allow(clippy::too_many_lines)]
     pub fn validate(&self) -> ConfigResult {
         // Check default exposes and prefixes
@@ -442,7 +462,7 @@ impl VpcExpose {
         fn prefixes_size(prefixes: &PrefixPortsSet) -> PrefixWithPortsSize {
             prefixes
                 .iter()
-                .map(|p| p.size())
+                .map(PrefixWithOptionalPorts::size)
                 .sum::<PrefixWithPortsSize>()
         }
         let zero_size = ppsize_zero();
@@ -528,7 +548,7 @@ impl VpcManifest {
     }
     #[must_use]
     pub fn has_host_prefixes(&self) -> bool {
-        self.exposes.iter().any(|expose| expose.has_host_prefixes())
+        self.exposes.iter().any(VpcExpose::has_host_prefixes)
     }
     fn validate_expose_collisions(&self) -> ConfigResult {
         // Check that prefixes in each expose don't overlap with prefixes in other exposes
@@ -602,6 +622,11 @@ impl VpcManifest {
     pub fn add_exposes(&mut self, exposes: impl IntoIterator<Item = VpcExpose>) {
         self.exposes.extend(exposes);
     }
+    /// Validate the [`VpcManifest`].
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the manifest configuration is invalid.
     pub fn validate(&self) -> ConfigResult {
         if self.name.is_empty() {
             return Err(ConfigError::MissingIdentifier("Manifest name"));
@@ -695,7 +720,7 @@ impl VpcPeering {
         }
     }
 
-    /// Create a VpcPeering mapped to a group called "default".
+    /// Create a `VpcPeering` mapped to a group called "default".
     /// This should only be used for tests
     #[must_use]
     pub fn with_default_group(name: &str, left: VpcManifest, right: VpcManifest) -> Self {
@@ -744,7 +769,11 @@ impl VpcPeeringTable {
         self.0.is_empty()
     }
 
-    /// Add a [`VpcPeering`] to a [`VpcPeeringTable`]
+    /// Add a [`VpcPeering`] to a [`VpcPeeringTable`].
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the peering name is missing or a duplicate peering exists.
     pub fn add(&mut self, peering: VpcPeering) -> ConfigResult {
         if peering.name.is_empty() {
             return Err(ConfigError::MissingIdentifier("Peering name"));

config/src/external/underlay/mod.rs

@@ -72,14 +72,19 @@ impl Underlay {
         }
     }
 
+    /// Validate the underlay configuration.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if any interface is invalid or VTEP configuration is wrong.
     pub fn validate(&mut self) -> ConfigResult {
         debug!("Validating underlay configuration...");
 
         // validate interfaces
         self.vrf
             .interfaces
             .values()
-            .try_for_each(|iface| iface.validate())?;
+            .try_for_each(InterfaceConfig::validate)?;
 
         // set vtep information if a vtep interface has been specified in the config
         self.vtep = self.get_vtep_info()?;

config/src/gwconfig.rs

@@ -109,9 +109,11 @@ impl GwConfig {
         self.external.genid
     }
 
-    //////////////////////////////////////////////////////////////////
     /// Validate a [`GwConfig`]. We only validate the external.
-    //////////////////////////////////////////////////////////////////
+    ///
+    /// # Errors
+    ///
+    /// Returns a [`ConfigError`] if the external configuration fails validation.
     pub fn validate(&mut self) -> ConfigResult {
         debug!("Validating external config with genid {} ..", self.genid());
         self.external.validate()

config/src/internal/device/mod.rs

@@ -22,6 +22,11 @@ impl DeviceConfig {
     pub fn set_tracing(&mut self, tracing: TracingConfig) {
         self.tracing = Some(tracing);
     }
+    /// Validate the device configuration.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the device configuration is invalid.
     pub fn validate(&self) -> ConfigResult {
         debug!("Validating device configuration..");
         if let Some(tracing) = &self.tracing {

config/src/internal/device/tracecfg.rs

@@ -35,9 +35,14 @@ impl TracingConfig {
     pub fn add_tag(&mut self, tag: &str, level: LevelFilter) {
         let _ = self.tags.insert(tag.to_string(), level);
     }
+    /// Validate the tracing configuration.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if any configured trace tag is unknown.
     pub fn validate(&self) -> ConfigResult {
         debug!("Validating tracing configuration..");
-        let tags: Vec<&str> = self.tags.keys().map(|k| k.as_str()).collect();
+        let tags: Vec<&str> = self.tags.keys().map(String::as_str).collect();
         Ok(get_trace_ctl().check_tags(&tags)?)
     }
 }

config/src/internal/interfaces/interface.rs

@@ -156,6 +156,11 @@ impl InterfaceConfig {
         matches!(self.iftype, InterfaceType::Vtep(_))
     }
 
+    /// Validate the interface configuration.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the interface name is empty.
     pub fn validate(&self) -> ConfigResult {
         // name is mandatory
         if self.name.is_empty() {

config/src/internal/mod.rs

@@ -68,6 +68,11 @@ impl InternalConfig {
     pub fn get_vtep(&self) -> &Option<VtepConfig> {
         &self.vtep
     }
+    /// Add a VRF configuration to the internal config.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the VRF has duplicate fields (name, table ID, or VNI).
     pub fn add_vrf_config(&mut self, vrf_cfg: VrfConfig) -> ConfigResult {
         self.vrfs.add_vrf_config(vrf_cfg)
     }