[pve-devel] [PATCH proxmox-ve-rs v2 13/15] ve-config: add validation for section-config

[Gabriel Goller <g.goller@proxmox.com>]

Our section-config is nested 3 times (fabric -> node -> interfaces), but
as only one indentation level (two with propertyStrings) are possible in
section-config configuration files, we need to add some validation to
ensure that the config is valid. In the future, more stuff to be
validated can be added here, but currently we check:

 * if the router-id is unique
 * if the node refers to a existing fabric
 * if the router-ids are in the specified loopback prefix

Our Section-Config is structured like this:

fabric: test
    fabric-option: this

node: test_pve0
    node-option: that

The key to the node section is called the `NodeId` and consist of two
parts: `test`, which is the fabric, and `pve0`, which is the nodename.
The validation checks if the `test` fabric exists.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
Co-authored-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-ve-config/src/sdn/fabric/mod.rs       | 33 +++++++++
 .../src/sdn/fabric/openfabric/mod.rs          |  1 +
 .../src/sdn/fabric/openfabric/validation.rs   | 58 ++++++++++++++++
 proxmox-ve-config/src/sdn/fabric/ospf/mod.rs  |  1 +
 .../src/sdn/fabric/ospf/validation.rs         | 68 +++++++++++++++++++
 proxmox-ve-config/src/sdn/mod.rs              |  1 +
 6 files changed, 162 insertions(+)
 create mode 100644 proxmox-ve-config/src/sdn/fabric/openfabric/validation.rs
 create mode 100644 proxmox-ve-config/src/sdn/fabric/ospf/validation.rs

diff --git a/proxmox-ve-config/src/sdn/fabric/mod.rs b/proxmox-ve-config/src/sdn/fabric/mod.rs
index 5b24a3be8c17..45795b0e51b0 100644
--- a/proxmox-ve-config/src/sdn/fabric/mod.rs
+++ b/proxmox-ve-config/src/sdn/fabric/mod.rs
@@ -109,3 +109,36 @@ impl std::str::FromStr for SectionType {
         })
     }
 }
+
+#[derive(Debug, Clone)]
+pub struct Valid<T>(SectionConfigData<T>);
+
+impl<T> Deref for Valid<T> {
+    type Target = SectionConfigData<T>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+pub trait Validate<T> {
+    fn validate(data: SectionConfigData<T>) -> Result<Valid<T>, anyhow::Error>;
+    fn validate_as_ref(data: &SectionConfigData<T>) -> Result<(), anyhow::Error>;
+}
+
+impl<T> Valid<T> {
+    pub fn into_inner(self) -> SectionConfigData<T> {
+        self.0
+    }
+}
+
+impl<T> Valid<T>
+where
+    T: ApiSectionDataEntry + DeserializeOwned + Validate<T>,
+{
+    pub fn parse_section_config(filename: &str, data: &str) -> Result<Valid<T>, anyhow::Error> {
+        let config = T::parse_section_config(filename, data)?;
+        T::validate(config)
+    }
+}
+
diff --git a/proxmox-ve-config/src/sdn/fabric/openfabric/mod.rs b/proxmox-ve-config/src/sdn/fabric/openfabric/mod.rs
index 6e7f4e947a0e..2aca197e095b 100644
--- a/proxmox-ve-config/src/sdn/fabric/openfabric/mod.rs
+++ b/proxmox-ve-config/src/sdn/fabric/openfabric/mod.rs
@@ -1,5 +1,6 @@
 #[cfg(feature = "frr")]
 pub mod frr;
+pub mod validation;
 
 use proxmox_network_types::{
     debian::Hostname,
diff --git a/proxmox-ve-config/src/sdn/fabric/openfabric/validation.rs b/proxmox-ve-config/src/sdn/fabric/openfabric/validation.rs
new file mode 100644
index 000000000000..bf0c02620e63
--- /dev/null
+++ b/proxmox-ve-config/src/sdn/fabric/openfabric/validation.rs
@@ -0,0 +1,58 @@
+use anyhow::{anyhow, bail};
+use std::collections::{HashMap, HashSet};
+
+use proxmox_section_config::typed::SectionConfigData;
+
+use crate::sdn::fabric::{Valid, Validate};
+
+use super::OpenFabricSectionConfig;
+
+impl Validate<OpenFabricSectionConfig> for OpenFabricSectionConfig {
+    /// This function will validate the SectionConfigData<T> and return a Valid<SectionConfigData<T>>
+    /// The validation checks if the every node is part of an existing fabric. This is necessary as
+    /// with the current SectionConfigData format, we don't get this guarantee.
+    fn validate(
+        data: SectionConfigData<OpenFabricSectionConfig>,
+    ) -> Result<Valid<OpenFabricSectionConfig>, anyhow::Error> {
+        Self::validate_as_ref(&data)?;
+        Ok(Valid(data))
+    }
+
+    fn validate_as_ref(
+        data: &SectionConfigData<OpenFabricSectionConfig>,
+    ) -> Result<(), anyhow::Error> {
+        let mut fabrics = HashMap::new();
+        let mut nodes = Vec::new();
+
+        for (_, section) in data {
+            match section {
+                OpenFabricSectionConfig::Node(node) => {
+                    nodes.push(node);
+                }
+                OpenFabricSectionConfig::Fabric(fabric) => {
+                    fabrics.insert(&fabric.fabric_id, fabric);
+                }
+            }
+        }
+
+        let mut router_ids = HashSet::new();
+
+        for node in nodes {
+            let fabric = fabrics
+                .get(&node.fabric_id)
+                .ok_or_else(|| anyhow!("Validation Error: Missing fabric configuration"))?;
+
+            if !router_ids.insert(node.router_id) {
+                bail!("Validation Error: Duplicate loopback ip");
+            }
+
+            if !fabric.loopback_prefix.contains_address(&node.router_id) {
+                bail!(
+                    "Validation Error: Loopback IP of node is not contained in Loopback IP prefix"
+                );
+            }
+        }
+
+        Ok(())
+    }
+}
diff --git a/proxmox-ve-config/src/sdn/fabric/ospf/mod.rs b/proxmox-ve-config/src/sdn/fabric/ospf/mod.rs
index ece251645ee8..b1bec62cfa1c 100644
--- a/proxmox-ve-config/src/sdn/fabric/ospf/mod.rs
+++ b/proxmox-ve-config/src/sdn/fabric/ospf/mod.rs
@@ -1,5 +1,6 @@
 #[cfg(feature = "frr")]
 pub mod frr;
+pub mod validation;
 
 use proxmox_network_types::debian::Hostname;
 use proxmox_network_types::ip_address::Ipv4Cidr;
diff --git a/proxmox-ve-config/src/sdn/fabric/ospf/validation.rs b/proxmox-ve-config/src/sdn/fabric/ospf/validation.rs
new file mode 100644
index 000000000000..539515485dac
--- /dev/null
+++ b/proxmox-ve-config/src/sdn/fabric/ospf/validation.rs
@@ -0,0 +1,68 @@
+use anyhow::{anyhow, bail};
+use std::collections::{HashMap, HashSet};
+
+use proxmox_section_config::typed::SectionConfigData;
+
+use crate::sdn::fabric::{Valid, Validate};
+
+use super::OspfSectionConfig;
+
+impl Validate<OspfSectionConfig> for OspfSectionConfig {
+    /// This function will validate the SectionConfigData<T> and return a Valid<SectionConfigData<T>>
+    /// The validation checks if the every node is part of an existing fabric. This is necessary as
+    /// with the current SectionConfigData format, we don't get this guarantee.
+    fn validate(
+        data: SectionConfigData<OspfSectionConfig>,
+    ) -> Result<Valid<OspfSectionConfig>, anyhow::Error> {
+        Self::validate_as_ref(&data)?;
+        Ok(Valid(data))
+    }
+
+    fn validate_as_ref(data: &SectionConfigData<OspfSectionConfig>) -> Result<(), anyhow::Error> {
+        let mut fabrics = HashMap::new();
+        let mut nodes = Vec::new();
+
+        for (_, section) in data {
+            match section {
+                OspfSectionConfig::Node(node) => {
+                    nodes.push(node);
+                }
+                OspfSectionConfig::Fabric(fabric) => {
+                    fabrics.insert(&fabric.fabric_id, fabric);
+                }
+            }
+        }
+
+        let mut router_ids = HashSet::new();
+
+        for node in nodes {
+            let fabric = fabrics
+                .get(&node.fabric_id)
+                .ok_or_else(|| anyhow!("Validation Error: Missing fabric configuration"))?;
+
+            if !router_ids.insert(node.router_id) {
+                bail!("Validation Error: Duplicate loopback ip");
+            }
+
+            if !fabric.loopback_prefix.contains_address(&node.router_id) {
+                bail!(
+                    "Validation Error: Loopback IP of node is not contained in Loopback IP prefix"
+                );
+            }
+
+            for interface in &node.interfaces {
+                match (interface.ip.is_some(), interface.unnumbered) {
+                    (true, Some(true)) => {
+                        bail!("Validation Error: Interface cannot be both unnumbered and have an IP address");
+                    }
+                    (false, None | Some(false)) => {
+                        bail!("Validation Error: Interface \"{}\" on node \"{}\" must either be unnumbered or have an IP address", interface.name, node.node_id);
+                    }
+                    _ => (),
+                }
+            }
+        }
+
+        Ok(())
+    }
+}
diff --git a/proxmox-ve-config/src/sdn/mod.rs b/proxmox-ve-config/src/sdn/mod.rs
index cde6fed88f26..7a46db3d85bb 100644
--- a/proxmox-ve-config/src/sdn/mod.rs
+++ b/proxmox-ve-config/src/sdn/mod.rs
@@ -1,4 +1,5 @@
 pub mod config;
+pub mod fabric;
 pub mod ipam;
 
 use std::{error::Error, fmt::Display, str::FromStr};
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel