feat(vpcpeering): Implement validation for VpcManifest

We already have validation for VpcExpose, VpcPeering, and expose or
peering addition to lists and tables; we're only missing some basic
checks for the VpcManifest objects.

There's not much to do this time, because most of the work consists in
validating the VpcExpose objects and preventing prefix overlap between
them, which we have implemented in previous commits. Make sure the name
is not empty, make sure the lists of VpcExpose objects are not empty,
and use the resulting validation function for each VpcManifest when
validating a VpcPeering object.

Add relevant enum variants to ApiError.

Fix some tests that would use manifests with empty VpcExpose lists.

Signed-off-by: Quentin Monnet <qmo@qmon.net>

Author: qmonnet

mgmt/src/models/external/overlay/tests.rs

@@ -142,23 +142,71 @@ pub mod test {
     fn test_peering_iter() {
         let mut peering_table = VpcPeeringTable::new();
 
-        let m1 = VpcManifest::new("VPC-1");
-        let m2 = VpcManifest::new("VPC-2");
+        let mut m1 = VpcManifest::new("VPC-1");
+        m1.add_expose(
+            VpcExpose::empty()
+                .ip(Prefix::from(("1.1.1.0", 24)))
+                .as_range(Prefix::from(("1.1.2.0", 24))),
+        )
+        .expect("Failed to add expose");
+        let mut m2 = VpcManifest::new("VPC-2");
+        m2.add_expose(
+            VpcExpose::empty()
+                .ip(Prefix::from(("1.2.1.0", 24)))
+                .as_range(Prefix::from(("1.2.2.0", 24))),
+        )
+        .expect("Failed to add expose");
         let peering = VpcPeering::new("Peering-1", m1, m2);
         peering_table.add(peering).unwrap();
 
-        let m1 = VpcManifest::new("VPC-1");
-        let m2 = VpcManifest::new("VPC-3");
+        let mut m1 = VpcManifest::new("VPC-1");
+        m1.add_expose(
+            VpcExpose::empty()
+                .ip(Prefix::from(("2.1.1.0", 24)))
+                .as_range(Prefix::from(("2.2.2.0", 24))),
+        )
+        .expect("Failed to add expose");
+        let mut m2 = VpcManifest::new("VPC-3");
+        m2.add_expose(
+            VpcExpose::empty()
+                .ip(Prefix::from(("2.2.1.0", 24)))
+                .as_range(Prefix::from(("2.2.2.0", 24))),
+        )
+        .expect("Failed to add expose");
         let peering = VpcPeering::new("Peering-2", m1, m2);
         peering_table.add(peering).unwrap();
 
-        let m1 = VpcManifest::new("VPC-2");
-        let m2 = VpcManifest::new("VPC-4");
+        let mut m1 = VpcManifest::new("VPC-2");
+        m1.add_expose(
+            VpcExpose::empty()
+                .ip(Prefix::from(("3.1.1.0", 24)))
+                .as_range(Prefix::from(("3.1.2.0", 24))),
+        )
+        .expect("Failed to add expose");
+        let mut m2 = VpcManifest::new("VPC-4");
+        m2.add_expose(
+            VpcExpose::empty()
+                .ip(Prefix::from(("3.2.1.0", 24)))
+                .as_range(Prefix::from(("3.2.2.0", 24))),
+        )
+        .expect("Failed to add expose");
         let peering = VpcPeering::new("Peering-3", m1, m2);
         peering_table.add(peering).unwrap();
 
-        let m1 = VpcManifest::new("VPC-1");
-        let m2 = VpcManifest::new("VPC-4");
+        let mut m1 = VpcManifest::new("VPC-1");
+        m1.add_expose(
+            VpcExpose::empty()
+                .ip(Prefix::from(("4.1.1.0", 24)))
+                .as_range(Prefix::from(("4.1.2.0", 24))),
+        )
+        .expect("Failed to add expose");
+        let mut m2 = VpcManifest::new("VPC-4");
+        m2.add_expose(
+            VpcExpose::empty()
+                .ip(Prefix::from(("4.2.1.0", 24)))
+                .as_range(Prefix::from(("4.2.2.0", 24))),
+        )
+        .expect("Failed to add expose");
         let peering = VpcPeering::new("Peering-4", m1, m2);
         peering_table.add(peering).unwrap();
 

mgmt/src/models/external/overlay/vpcpeering.rs

@@ -36,11 +36,11 @@ impl VpcExpose {
         self.not_as.insert(prefix);
         self
     }
-    fn fixup(mut self) -> Result<Self, ApiError> {
+    fn fixup(mut self) -> Result<Self, ConfigError> {
         // Add a root prefix to the list of private prefixes if the list is empty
         if self.ips.is_empty() {
             if self.nots.is_empty() {
-                return Err(ApiError::EmptyExpose);
+                return Err(ConfigError::EmptyExpose);
             }
             let mut added_v4 = false;
             let mut added_v6 = false;
@@ -198,7 +198,7 @@ impl VpcManifest {
             ..Default::default()
         }
     }
-    fn validate_expose_addition(&self, expose: &VpcExpose) -> ApiResult {
+    fn validate_expose_addition(&self, expose: &VpcExpose) -> ConfigResult {
         // Check that prefixes in the expose don't overlap with prefixes in other exposes
         for other_expose in &self.exposes {
             validate_overlapping(
@@ -216,11 +216,11 @@ impl VpcManifest {
         }
         Ok(())
     }
-    pub fn add_expose(&mut self, new_expose: VpcExpose) -> ApiResult {
+    pub fn add_expose(&mut self, new_expose: VpcExpose) -> ConfigResult {
         let mut expose = new_expose;
         match expose.validate() {
             Ok(_) => {}
-            Err(ApiError::EmptyExpose) => {
+            Err(ConfigError::EmptyExpose) => {
                 // Fix up empty expose and give another try at validation
                 expose = expose.clone();
                 expose = expose.fixup()?;
@@ -232,6 +232,15 @@ impl VpcManifest {
         self.exposes.push(expose);
         Ok(())
     }
+    pub fn validate(&self) -> ConfigResult {
+        if self.name.is_empty() {
+            return Err(ConfigError::MissingIdentifier("Manifest name"));
+        }
+        if self.exposes.is_empty() {
+            return Err(ConfigError::IncompleteConfig("Empty manifest".to_string()));
+        }
+        Ok(())
+    }
 }
 
 #[derive(Clone, Debug)]
@@ -252,6 +261,8 @@ impl VpcPeering {
         if self.name.is_empty() {
             return Err(ConfigError::MissingIdentifier("Peering name"));
         }
+        self.left.validate()?;
+        self.right.validate()?;
         Ok(())
     }
     /// Given a peering fetch the manifests, orderly depending on the provided vpc name
@@ -280,13 +291,13 @@ impl VpcPeeringTable {
         self.0.is_empty()
     }
 
-    fn validate_peering_addition(&self, new_peering: &VpcPeering) -> ApiResult {
+    fn validate_peering_addition(&self, new_peering: &VpcPeering) -> ConfigResult {
         // Check that exposes in the new peering do not collide with any of the exposes in the
         // existing peerings in the table, for the same VPCs
         for vpc in [&new_peering.left.name, &new_peering.right.name] {
-            let (new_local, _) = new_peering.get_peers(vpc);
+            let (new_local, _) = new_peering.get_peering_manifests(vpc);
             for peering in self.peerings_vpc(vpc) {
-                let (local, _) = peering.get_peers(vpc);
+                let (local, _) = peering.get_peering_manifests(vpc);
                 for new_expose in &new_local.exposes {
                     for expose in &local.exposes {
                         validate_overlapping(
@@ -314,7 +325,7 @@ impl VpcPeeringTable {
 
         // First look for an existing entry, to avoid inserting a duplicate peering
         if self.0.contains_key(&peering.name) {
-            return Err(ApiError::DuplicateVpcPeeringId(peering.name.clone()));
+            return Err(ConfigError::DuplicateVpcPeeringId(peering.name.clone()));
         }
 
         if let Some(peering) = self.0.insert(peering.name.to_owned(), peering) {
@@ -341,7 +352,7 @@ fn validate_overlapping(
     excludes_left: &BTreeSet<Prefix>,
     prefixes_right: &BTreeSet<Prefix>,
     excludes_right: &BTreeSet<Prefix>,
-) -> Result<(), ApiError> {
+) -> Result<(), ConfigError> {
     // Find colliding prefixes
     let mut colliding = Vec::new();
     for prefix_left in prefixes_left.iter() {
@@ -448,7 +459,7 @@ fn validate_overlapping(
             // Some addresses at the intersection of both prefixes are not covered by the union of
             // all exclusion prefixes, in other words, they are available from both prefixes. This
             // is an error.
-            return Err(ApiError::OverlappingPrefixes(prefix_left, prefix_right));
+            return Err(ConfigError::OverlappingPrefixes(prefix_left, prefix_right));
         }
     }
     Ok(())
@@ -753,7 +764,7 @@ mod tests {
         let peering2 = VpcPeering::new("test_peering2", manifest1.clone(), manifest2.clone());
         assert_eq!(
             table.add(peering2),
-            Err(ApiError::OverlappingPrefixes(
+            Err(ConfigError::OverlappingPrefixes(
                 prefix_v4("10.0.0.0/16"),
                 prefix_v4("10.0.0.0/16")
             ))