test(config,mgmt,k8s-intf): generate valid configurations and property-test the validator

config/Cargo.toml

@@ -5,6 +5,10 @@ license.workspace = true
 publish.workspace = true
 version.workspace = true
 
+[features]
+# Generators for the configuration types, for property tests in this crate and downstream.
+bolero = ["dep:bolero", "lpm/bolero"]
+
 [dependencies]
 # internal
 common = { workspace = true }
@@ -16,6 +20,7 @@ net = { workspace = true }
 
 # external
 arc-swap = { workspace = true }
+bolero = { workspace = true, optional = true, default-features = false, features = ["alloc"] }
 chrono = { workspace = true, features = ["alloc", "std"] }
 derive_builder = { workspace = true, features = [] }
 ipnet = { workspace = true }

config/src/converters/k8s/config/expose.rs

@@ -491,7 +491,8 @@ mod test {
                 "10.0.4.0/24".parse::<Prefix>().unwrap(),
             ),
         ]);
-        let expose_gen = k8s_intf::bolero::expose::LegalValueExposeGenerator::new(&subnets);
+        // One expose at a time here, so any slot will do.
+        let expose_gen = k8s_intf::bolero::expose::AnyExposeGenerator::new(0, &subnets);
         bolero::check!()
             .with_generator(expose_gen)
             .for_each(|k8s_expose| {
@@ -549,6 +550,7 @@ mod test {
                     })
                     .unwrap_or(vec![]);
                 k8s_nots.sort();
+                k8s_nots.dedup();
                 let k8s_subnets = k8s_expose
                     .ips
                     .as_ref()
@@ -565,7 +567,13 @@ mod test {
                     })
                     .unwrap_or(vec![]);
                 k8s_ips.extend(k8s_subnets);
+                // Sorted *and* deduplicated, because the conversion collects into a set-like
+                // structure: a prefix written twice in one expose means the same as writing it once,
+                // and comes out once. This only came up when the generators started producing
+                // repeats -- the previous ones drew each prefix from a uniqueness-preserving
+                // generator, so the question never arose.
                 k8s_ips.sort();
+                k8s_ips.dedup();
 
                 let k8s_as = k8s_expose.r#as.as_ref().map(|r#as| {
                     let mut ret = r#as
@@ -574,6 +582,7 @@ mod test {
                         .map(|r#as| r#as.cidr.as_ref().unwrap().clone())
                         .collect::<Vec<_>>();
                     ret.sort();
+                    ret.dedup();
                     ret
                 });
 
@@ -584,6 +593,7 @@ mod test {
                         .map(|r#as| r#as.not.as_ref().unwrap().clone())
                         .collect::<Vec<_>>();
                     ret.sort();
+                    ret.dedup();
                     ret
                 });
 

config/src/converters/k8s/config/peering.rs

@@ -91,14 +91,20 @@ mod test {
     use k8s_intf::bolero::peering::{
         LegalValuePeeringsGenerator, LegalValuePeeringsPeeringGenerator,
     };
+    use k8s_intf::bolero::{AddressFamily, NatFlavour};
     use lpm::prefix::Prefix;
 
     use crate::converters::k8s::config::{SubnetMap, VpcSubnetMap};
 
     #[test]
     fn test_vpc_manifest_conversion() {
         let subnets = SubnetMap::new(); // Let this be empty since we are test subnet conversion elsewhere
-        let generator = LegalValuePeeringsPeeringGenerator::new(&subnets);
+        // any flavour, one family, a couple of exposes: this is testing the conversion, not the
+        // rules the generators satisfy
+        let flavours = NatFlavour::all();
+        let generator =
+            // One manifest at a time here, so vpc zero: there is nothing to keep it disjoint from.
+            LegalValuePeeringsPeeringGenerator::new(&subnets, &flavours, AddressFamily::V4, 3, 0);
         bolero::check!()
             .with_generator(generator)
             .for_each(|peering| {
@@ -169,7 +175,11 @@ mod test {
                 ]),
             ),
         ]);
-        let generator = LegalValuePeeringsGenerator::new(&subnets).unwrap();
+        let flavours = NatFlavour::all();
+        let families = AddressFamily::all();
+        let groups = vec!["gwgroup-0".to_string()];
+        let generator =
+            LegalValuePeeringsGenerator::new(&subnets, &flavours, &families, 3, &groups).unwrap();
         bolero::check!()
             .with_generator(generator)
             .for_each(|peering| {

config/src/errors.rs

@@ -75,8 +75,33 @@ pub enum ConfigError {
     #[error("Invalid ACL configuration: {0}")]
     InvalidAcl(String),
     // NAT-specific
-    #[error("Mismatched prefixes sizes for static NAT: {0:?} and {1:?}")]
+    /// The two sides of a static NAT expose cover different numbers of address-port pairs.
+    //
+    // The sizes are rendered with `Debug` because `PrefixWithPortsSize` is a 145-bit bnum type
+    // with no `Display`, and Debug pads it out to a run of digits that reads as gibberish. Hence
+    // leading with what to change and leaving the numbers to the end.
+    #[error(
+        "Mismatched sizes for static NAT: the exposed prefixes and the range they translate to \
+         must cover the same number of address-port pairs (they cover {0:?} and {1:?})"
+    )]
     MismatchedPrefixSizes(PrefixWithPortsSize, PrefixWithPortsSize),
+    /// The two sides of a port-forwarding expose have prefixes of different lengths.
+    ///
+    /// Distinct from [`ConfigError::MismatchedPrefixSizes`], which compares addresses times ports.
+    /// That product can match while the lengths do not -- a `/32` carrying 100 ports and a `/30`
+    /// carrying 25 both come to 100 -- so reporting it as a size mismatch would name two numbers
+    /// that are equal. A port-forwarding rule maps addresses one for one, so it is the lengths
+    /// that have to agree.
+    #[error(
+        "Mismatched prefix lengths for port forwarding: /{private} exposed and /{public} \
+         translated to; a rule maps addresses one for one, so the two must be the same length"
+    )]
+    MismatchedPrefixLengths { private: u8, public: u8 },
+    #[error(
+        "Mismatched port range sizes for port forwarding: {private} ports exposed and {public} \
+         translated to; a rule maps ports one for one, so the two must be the same size"
+    )]
+    MismatchedPortRangeSizes { private: usize, public: usize },
     #[error("Peering {0} has manifests using incompatible NAT modes")]
     IncompatibleNatModes(String),
     #[error("Vpc {0} has a peering with no exposes")]

config/src/external/overlay/validation_tests.rs

@@ -636,6 +636,11 @@ mod test {
     }
 
     // Port forwarding: mismatched sizes rejected
+    //
+    // Reported as the prefix lengths differing rather than as `MismatchedPrefixSizes`. Port
+    // forwarding compares the two lengths and the two port counts directly now, instead of the
+    // product of the two, because a product accepts pairings a rule cannot express -- see
+    // `contract::tests::compensating_sizes_do_not_make_a_valid_expose`.
     #[test]
     fn test_port_forwarding_mismatched_sizes_rejected() {
         let expose = VpcExpose::empty()
@@ -646,7 +651,13 @@ mod test {
             .unwrap();
         let result = expose.validate();
         assert!(
-            matches!(result, Err(ConfigError::MismatchedPrefixSizes(_, _))),
+            matches!(
+                result,
+                Err(ConfigError::MismatchedPrefixLengths {
+                    private: 24,
+                    public: 25
+                })
+            ),
             "{result:?}",
         );
     }