diff --git a/config/src/converters/k8s/config/gateway_config.rs b/config/src/converters/k8s/config/gateway_config.rs index 044a60a5f..a6f42fe7c 100644 --- a/config/src/converters/k8s/config/gateway_config.rs +++ b/config/src/converters/k8s/config/gateway_config.rs @@ -152,7 +152,10 @@ impl TryFrom<&GatewayAgent> for ExternalConfig { #[cfg(test)] mod test { + use std::num::NonZero; + use k8s_intf::bolero::LegalValue; + use k8s_intf::bolero::mutate::Mutated; use k8s_intf::gateway_agent_crd::GatewayAgent; use crate::ExternalConfig; @@ -169,4 +172,116 @@ mod test { // Other assertions are implicit via the unwrap of the conversion }); } + + /// A legal `GatewayAgent` must not merely convert -- it must validate. + /// + /// `LegalValue` is only worth the name if it agrees with the system's own definition of a + /// valid config. Stopping at `try_from` would leave every rule in `ExternalConfig::validate` + /// (and everything it delegates to, notably the overlay and its peerings) unfuzzed, and would + /// let cross-reference bugs in the generators hide as silent under-coverage. + #[test] + fn test_gateway_config_validates() { + bolero::check!() + .with_type::>() + .for_each(|ga| { + let ga = ga.as_ref(); + let external_config = ExternalConfig::try_from(ga).unwrap(); + let genid = external_config.genid; + + let validated = external_config + .validate() + .unwrap_or_else(|e| panic!("legal config failed validation: {e}")); + + assert_eq!(validated.genid(), genid); + assert_eq!( + validated.external().gwname(), + ga.metadata.name.as_ref().unwrap() + ); + }); + } + + /// `fabricBFD` must reach every underlay BGP neighbor, or none of them. + /// + /// The flag is applied by a fan-out loop after the underlay has already been converted, so it + /// is exactly the kind of post-hoc mutation that a per-subtree converter test cannot see. + #[test] + fn test_fabric_bfd_fans_out_to_all_neighbors() { + bolero::check!() + .with_type::>() + .for_each(|ga| { + let ga = ga.as_ref(); + let expected = ga.spec.config.as_ref().and_then(|c| c.fabric_bfd); + let external_config = ExternalConfig::try_from(ga).unwrap(); + + let Some(bgp) = external_config.underlay.vrf.bgp.as_ref() else { + return; + }; + match expected { + // Only `Some(true)` turns BFD on; absent and `Some(false)` both leave the + // neighbours as the underlay converter produced them. + Some(true) => assert!( + bgp.neighbors.iter().all(|n| n.bfd), + "fabricBFD set but some neighbors have bfd off" + ), + _ => assert!( + bgp.neighbors.iter().all(|n| !n.bfd), + "fabricBFD unset but some neighbors have bfd on" + ), + } + }); + } + + /// A config with one thing deliberately wrong must be *rejected*, never panic. + /// + /// The property is deliberately coarse. Asserting a specific error variant per mutation would + /// pin down the current phrasing of every guard and break on harmless refactors; asserting only + /// "does not panic" would pass even if the config plane silently accepted a dangling reference. + /// Rejection is the property that actually matters: bad config from outside the dataplane must + /// not be applied. + /// + /// A mutation that found nothing to corrupt (no peerings to break a group reference in, say) + /// leaves the config legal, and is required to still validate. + #[test] + fn test_mutated_config_is_rejected() { + bolero::check!().with_type::().for_each(|mutated| { + let result = ExternalConfig::try_from(mutated.agent()) + .and_then(|c| c.validate().map_err(Into::into)); + + if mutated.is_noop() { + assert!( + result.is_ok(), + "{:?} changed nothing, so the config should still be valid: {:?}", + mutated.mutation(), + result.err(), + ); + } else { + assert!( + result.is_err(), + "{:?} was applied but the config was still accepted", + mutated.mutation(), + ); + } + }); + } + + /// `flowTableCapacity` must survive the `u32` -> `NonZero` narrowing intact. + #[test] + fn test_flow_table_capacity_conversion() { + bolero::check!() + .with_type::>() + .for_each(|ga| { + let ga = ga.as_ref(); + let expected = ga + .spec + .gateway + .as_ref() + .and_then(|gw| gw.flow_table_capacity); + let external_config = ExternalConfig::try_from(ga).unwrap(); + + assert_eq!( + external_config.flow_table_capacity.map(NonZero::get), + expected.map(|c| c as usize), + ); + }); + } } diff --git a/config/src/converters/k8s/config/peering.rs b/config/src/converters/k8s/config/peering.rs index 45ae4a8e2..4b8d0c6f6 100644 --- a/config/src/converters/k8s/config/peering.rs +++ b/config/src/converters/k8s/config/peering.rs @@ -169,13 +169,18 @@ mod test { ]), ), ]); - let generator = LegalValuePeeringsGenerator::new(&subnets).unwrap(); + let gwgroups = ["group-a".to_string(), "group-b".to_string()]; + let generator = LegalValuePeeringsGenerator::new(&subnets, &gwgroups).unwrap(); bolero::check!() .with_generator(generator) .for_each(|peering| { let peering_name = "test-peering"; - let peering = VpcPeering::try_from((&subnets, peering_name, peering)).unwrap(); - assert_eq!(peering.name, peering_name); + let converted = VpcPeering::try_from((&subnets, peering_name, peering)).unwrap(); + assert_eq!(converted.name, peering_name); + // The generator only ever names a group it was given, and the ACL it generates is + // relative to this peering's two VPCs, so both must survive conversion. + assert!(gwgroups.contains(&converted.gwgroup)); + assert_eq!(converted.acl.is_some(), peering.acl.is_some()); // Rest of the assertions come from the types and the unwrap in the conversion above }); } diff --git a/k8s-intf/src/bolero/acl.rs b/k8s-intf/src/bolero/acl.rs new file mode 100644 index 000000000..acc8b83cb --- /dev/null +++ b/k8s-intf/src/bolero/acl.rs @@ -0,0 +1,366 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Generators for the peering-scoped ACL section of the CRD. +//! +//! An ACL rule is only legal *relative to its peering*: `from`/`to` must name the peering's two +//! VPCs (or be blank, to be inferred), and a `vpcSubnet` reference in `match.src` resolves against +//! the from-side VPC's subnets while one in `match.dst` resolves against the to-side VPC's. None +//! of that is expressible as a `TypeGenerator` on the CRD type, so this is a `ValueGenerator` +//! carrying the peering's two VPC names and the subnet map as borrowed context. + +use std::ops::Bound; + +use bolero::{Driver, ValueGenerator}; + +use crate::bolero::support::{choose, generate_port_list, generate_prefixes}; +use crate::bolero::{SubnetMap, VpcSubnetMap}; +use crate::gateway_agent_crd::{ + GatewayAgentPeeringsAcl, GatewayAgentPeeringsAclDefault, GatewayAgentPeeringsAclRules, + GatewayAgentPeeringsAclRulesAction, GatewayAgentPeeringsAclRulesMatch, + GatewayAgentPeeringsAclRulesMatchDst, GatewayAgentPeeringsAclRulesMatchSrc, + GatewayAgentPeeringsAclRulesScope, +}; + +/// Largest number of rules a generated ACL may hold. +const MAX_RULES: usize = 8; +/// Largest number of match entries per rule side. +const MAX_MATCH_ENTRIES: u16 = 4; + +/// Every legal spelling of a rule's `from`/`to` pair, given the peering's two VPC names. +/// +/// `complete_from_to` accepts a matching pair in either direction, or one side matching with the +/// other blank (inferred). Blank is spelled two ways in the CRD -- absent and empty-string -- and +/// the converter treats them identically via `unwrap_or_default`, so both are generated. +#[derive(Debug, Clone, Copy)] +enum FromToShape { + LeftToRight, + RightToLeft, + LeftOnlyAbsent, + LeftOnlyEmpty, + RightOnlyAbsent, + OnlyToLeftAbsent, + OnlyToRightEmpty, +} + +const FROM_TO_SHAPES: &[FromToShape] = &[ + FromToShape::LeftToRight, + FromToShape::RightToLeft, + FromToShape::LeftOnlyAbsent, + FromToShape::LeftOnlyEmpty, + FromToShape::RightOnlyAbsent, + FromToShape::OnlyToLeftAbsent, + FromToShape::OnlyToRightEmpty, +]; + +impl FromToShape { + /// The `(from, to)` field values for this shape. + fn fields(self, left: &str, right: &str) -> (Option, Option) { + match self { + FromToShape::LeftToRight => (Some(left.to_string()), Some(right.to_string())), + FromToShape::RightToLeft => (Some(right.to_string()), Some(left.to_string())), + FromToShape::LeftOnlyAbsent => (Some(left.to_string()), None), + FromToShape::LeftOnlyEmpty => (Some(left.to_string()), Some(String::new())), + FromToShape::RightOnlyAbsent => (Some(right.to_string()), None), + FromToShape::OnlyToLeftAbsent => (None, Some(left.to_string())), + FromToShape::OnlyToRightEmpty => (Some(String::new()), Some(right.to_string())), + } + } + + /// The VPC names the converter will resolve this shape to, in `(from, to)` order. + /// + /// This mirrors `complete_from_to`'s inference so the generator knows which subnet map each + /// side of `match` must draw its `vpcSubnet` references from. + fn resolved<'a>(self, left: &'a str, right: &'a str) -> (&'a str, &'a str) { + match self { + FromToShape::LeftToRight + | FromToShape::LeftOnlyAbsent + | FromToShape::LeftOnlyEmpty + | FromToShape::OnlyToRightEmpty => (left, right), + FromToShape::RightToLeft + | FromToShape::RightOnlyAbsent + | FromToShape::OnlyToLeftAbsent => (right, left), + } + } +} + +/// One entry of a rule's `match.src` / `match.dst` list. +/// +/// The converter accepts `cidr` xor `vpcSubnet` (or neither, meaning "any address"), each with an +/// optional non-empty `ports` list. +struct MatchEntry { + cidr: Option, + vpc_subnet: Option, + ports: Option>, +} + +impl From for GatewayAgentPeeringsAclRulesMatchSrc { + fn from(e: MatchEntry) -> Self { + Self { + cidr: e.cidr, + ports: e.ports, + vpc_subnet: e.vpc_subnet, + } + } +} + +impl From for GatewayAgentPeeringsAclRulesMatchDst { + fn from(e: MatchEntry) -> Self { + Self { + cidr: e.cidr, + ports: e.ports, + vpc_subnet: e.vpc_subnet, + } + } +} + +/// Whether a rule's generated protocol supports port matching. +/// +/// `AclPattern::validate` rejects ports on anything but TCP and UDP, and a numeric `6`/`17` +/// normalizes to those variants, so this is decided from the *converted* protocol, not the string. +fn proto_supports_ports(proto: Option<&str>) -> bool { + matches!(proto, Some("tcp" | "udp" | "6" | "17")) +} + +/// Generate one side (`src` or `dst`) of a rule's match block. +/// +/// `subnets` must be the subnet map of the VPC that this side's `vpcSubnet` references resolve +/// against -- the from-side VPC for `src`, the to-side VPC for `dst`. `want_v4` fixes the address +/// family: `AclPattern::validate` requires `src` and `dst` to agree on it. `allow_ports` reflects +/// whether the rule's protocol supports port matching at all. +fn generate_match_side( + d: &mut D, + subnets: &SubnetMap, + want_v4: bool, + allow_ports: bool, +) -> Option> { + let count = d.gen_u16(Bound::Included(&0), Bound::Included(&MAX_MATCH_ENTRIES))?; + if count == 0 { + return Some(Vec::new()); + } + + let subnet_names: Vec<&String> = subnets.keys().collect(); + + // Draw each entry's shape and ports first, then generate exactly as many prefixes as the + // `cidr` entries need. Generating one per entry up front would spend driver draws on + // prefixes the `vpcSubnet` and "any address" entries discard, which costs nothing in output + // but blunts shrinking; asking for them in one call is what keeps them distinct, so a `cidr` + // entry never duplicates another. + let mut shapes = Vec::with_capacity(usize::from(count)); + for _ in 0..count { + // A present-but-empty ports list is rejected by the converter, so only ever emit + // `None` (match all ports) or a non-empty list. + let ports = if allow_ports && d.gen_bool(None)? { + Some(vec![generate_port_list(d)?]) + } else { + None + }; + + // Three legal address shapes: shape 0 is a CIDR, shape 1 a named VPC subnet, and shape 2 + // neither ("any address within the peering", which the converter routes through a separate + // code path). Shape 1 falls through to "any address" when the VPC has no subnets. + let shape = d.gen_usize(Bound::Included(&0), Bound::Included(&2))?; + let vpc_subnet = if shape == 1 && !subnet_names.is_empty() { + Some(choose(d, &subnet_names)?.clone()) + } else { + None + }; + shapes.push((shape == 0, vpc_subnet, ports)); + } + + let cidr_count = u16::try_from(shapes.iter().filter(|(cidr, _, _)| *cidr).count()) + .unwrap_or_else(|_| unreachable!("at most MAX_MATCH_ENTRIES entries")); + let mut prefixes = if want_v4 { + generate_prefixes(d, cidr_count, 0)? + } else { + generate_prefixes(d, 0, cidr_count)? + } + .into_iter(); + + let mut entries = Vec::with_capacity(usize::from(count)); + for (wants_cidr, vpc_subnet, ports) in shapes { + let cidr = if wants_cidr { prefixes.next() } else { None }; + + // "Any address" with no ports means "match everything"; the converter short-circuits the + // whole side on it, discarding any other entry. Skip it unless it is the only entry, so + // that generated multi-entry sides stay meaningful. + if cidr.is_none() && vpc_subnet.is_none() && ports.is_none() && count > 1 { + continue; + } + + entries.push(MatchEntry { + cidr, + vpc_subnet, + ports, + }); + } + Some(entries) +} + +/// One of the protocol spellings the converter accepts. +/// +/// `Absent` is a distinct case from any string: it means "match any protocol", and it also decides +/// whether ports are legal on the rule. +enum ProtoChoice { + Absent, + Named(String), +} + +impl From for Option { + fn from(choice: ProtoChoice) -> Self { + match choice { + ProtoChoice::Absent => None, + ProtoChoice::Named(name) => Some(name), + } + } +} + +/// Protocols the converter accepts: absent, the two names, or a numeric value. +fn generate_proto(d: &mut D) -> Option { + match d.gen_usize(Bound::Included(&0), Bound::Included(&3))? { + 0 => Some(ProtoChoice::Absent), + 1 => Some(ProtoChoice::Named("tcp".to_string())), + 2 => Some(ProtoChoice::Named("udp".to_string())), + // Any u8 is accepted; 6 and 17 normalize to the Tcp/Udp variants rather than Other. + _ => Some(ProtoChoice::Named(d.produce::()?.to_string())), + } +} + +/// Generate a legal, peering-relative ACL. +/// +/// By default the generated ACL is legal with respect to *conversion*: `Acl::try_from` accepts it +/// for the peering it was built against. It is not necessarily legal with respect to +/// `ValidatedAcl` -- see [`LegalValueAclGenerator::independent_of_exposes`]. +pub struct LegalValueAclGenerator<'a> { + vpc_subnets: &'a VpcSubnetMap, + left_name: &'a str, + right_name: &'a str, + independent_of_exposes: bool, +} + +impl<'a> LegalValueAclGenerator<'a> { + /// `left_name` and `right_name` must be the two VPC names of the peering this ACL belongs to. + #[must_use] + pub fn new(vpc_subnets: &'a VpcSubnetMap, left_name: &'a str, right_name: &'a str) -> Self { + Self { + vpc_subnets, + left_name, + right_name, + independent_of_exposes: false, + } + } + + /// Restrict generation to ACLs whose legality does not depend on the peering's exposes. + /// + /// Two validation rules couple an ACL to the exposes on either side of its peering, neither of + /// which is visible from the ACL section alone: + /// + /// - `ValidatedAclRule::validate_patterns_coverage` requires a rule's `src`/`dst` addresses to + /// intersect what the corresponding side actually exposes. An empty side is exempt: it is + /// read as "everything the manifest exposes". + /// - `ValidatedAclRule::validate_scope` rejects `scope: flow` -- which is also the default -- + /// unless every expose on one side of the peering uses masquerade or port forwarding. + /// + /// In this mode, rules carry no address matches (so coverage is trivially satisfied) and always + /// set `scope: packet`. Everything else -- `from`/`to` inference, actions, protocols, logging, + /// the default action -- is still generated. + /// + /// FIXME: a generator that also covers address matches and `scope: flow` needs the *generated + /// exposes* of both sides as further context, and would have to mirror `manifest_coverage_set` + /// to know which prefixes are in scope. Until then, address matches are exercised at the + /// conversion level only (`converters::k8s::config::peering`). + #[must_use] + pub fn independent_of_exposes(mut self) -> Self { + self.independent_of_exposes = true; + self + } +} + +impl ValueGenerator for LegalValueAclGenerator<'_> { + type Output = GatewayAgentPeeringsAcl; + + fn generate(&self, d: &mut D) -> Option { + let default = match d.gen_usize(Bound::Included(&0), Bound::Included(&2))? { + 0 => GatewayAgentPeeringsAclDefault::Deny, + 1 => GatewayAgentPeeringsAclDefault::DenyUnlessExposed, + // The CRD's `""` member: documented to mean the same as "deny-unless-exposed". + _ => GatewayAgentPeeringsAclDefault::KopiumEmpty, + }; + + // `Acl::validate` rejects an ACL with no rules at all, so a present ACL always has one. + let num_rules = d.gen_usize(Bound::Included(&1), Bound::Included(&MAX_RULES))?; + let empty_map = SubnetMap::new(); + let mut rules = Vec::with_capacity(num_rules); + + for index in 0..num_rules { + let shape = choose(d, FROM_TO_SHAPES)?; + let (from, to) = shape.fields(self.left_name, self.right_name); + let (from_vpc, to_vpc) = shape.resolved(self.left_name, self.right_name); + + let src_subnets = self.vpc_subnets.get(from_vpc).unwrap_or(&empty_map); + let dst_subnets = self.vpc_subnets.get(to_vpc).unwrap_or(&empty_map); + + let r#match = if d.gen_bool(None)? { + let proto: Option = generate_proto(d)?.into(); + let allow_ports = proto_supports_ports(proto.as_deref()); + let (src, dst) = if self.independent_of_exposes { + (Vec::new(), Vec::new()) + } else { + // One family per rule: `AclPattern::validate` rejects a rule whose `src` and + // `dst` disagree on IP version. Drawn here rather than above so that the + // no-address-match mode does not spend a draw on it. + let want_v4 = d.gen_bool(None)?; + ( + generate_match_side(d, src_subnets, want_v4, allow_ports)?, + generate_match_side(d, dst_subnets, want_v4, allow_ports)?, + ) + }; + Some(GatewayAgentPeeringsAclRulesMatch { + src: Some(src.into_iter().map(Into::into).collect::>()) + .filter(|s: &Vec| !s.is_empty()), + dst: Some(dst.into_iter().map(Into::into).collect::>()) + .filter(|s: &Vec| !s.is_empty()), + proto, + }) + } else { + None + }; + + // Rule names must be unique within an ACL when non-empty (empty ones are exempt), so + // suffix with the index rather than relying on the name space being large. + let name = d + .produce::>()? + .map(|n| format!("{n}-{index}")); + + let scope = if self.independent_of_exposes { + Some(GatewayAgentPeeringsAclRulesScope::Packet) + } else { + match d.gen_usize(Bound::Included(&0), Bound::Included(&3))? { + 0 => None, + 1 => Some(GatewayAgentPeeringsAclRulesScope::Flow), + 2 => Some(GatewayAgentPeeringsAclRulesScope::Packet), + _ => Some(GatewayAgentPeeringsAclRulesScope::KopiumEmpty), + } + }; + + rules.push(GatewayAgentPeeringsAclRules { + action: if d.gen_bool(None)? { + GatewayAgentPeeringsAclRulesAction::Allow + } else { + GatewayAgentPeeringsAclRulesAction::Deny + }, + from, + to, + log: d.produce::>()?, + r#match, + name, + scope, + }); + } + + Some(GatewayAgentPeeringsAcl { + default, + rules: Some(rules), + }) + } +} diff --git a/k8s-intf/src/bolero/crd.rs b/k8s-intf/src/bolero/crd.rs index 92fc069b5..781a0407b 100644 --- a/k8s-intf/src/bolero/crd.rs +++ b/k8s-intf/src/bolero/crd.rs @@ -41,3 +41,196 @@ impl TypeGenerator for LegalValue { })) } } + +/// Tests for the deserialization boundary itself. +/// +/// This is the real trust boundary in both intake modes: k8s-less parses YAML from a watched file +/// (`k8s_less::kubeless_watch_gateway_agent_crd` -> [`crate::utils::load_crd_from_file`]) and the +/// k8s watcher parses JSON off the API server. Both land in the same kopium-generated types, and +/// `build.rs` rewrites those types' integer widths textually (`i64` -> `u64`, `i32` -> `u32`, with +/// specific fields patched back). Nothing else checks that those rewrites round-trip. +#[cfg(test)] +mod test { + use crate::bolero::LegalValue; + use crate::gateway_agent_crd::GatewayAgentSpec; + + /// A spec must survive YAML serialization, which is what k8s-less reads. + #[test] + fn test_spec_yaml_round_trip() { + bolero::check!() + .with_type::>() + .for_each(|spec| { + let spec = spec.as_ref(); + let yaml = serde_yaml_ng::to_string(spec).expect("failed to serialize as YAML"); + let parsed: GatewayAgentSpec = serde_yaml_ng::from_str(&yaml) + .unwrap_or_else(|e| panic!("failed to parse YAML: {e}\n{yaml}")); + assert_eq!(*spec, parsed, "YAML round-trip changed the spec:\n{yaml}"); + }); + } + + /// A spec must survive JSON serialization, which is what the k8s watcher receives. + #[test] + fn test_spec_json_round_trip() { + bolero::check!() + .with_type::>() + .for_each(|spec| { + let spec = spec.as_ref(); + let json = serde_json::to_string(spec).expect("failed to serialize as JSON"); + let parsed: GatewayAgentSpec = serde_json::from_str(&json) + .unwrap_or_else(|e| panic!("failed to parse JSON: {e}\n{json}")); + assert_eq!(*spec, parsed, "JSON round-trip changed the spec:\n{json}"); + }); + } + + /// The spec generator must actually reach the shapes it claims to cover. + /// + /// A property test passes just as easily against a generator that has quietly degenerated -- + /// `GatewayAgentGroups` used to draw a member count in `0..=10` and then emit exactly one, so + /// nothing ever exercised a multi-member group. Nothing detects that from the outside, so + /// assert reachability directly: sample specs and require each interesting shape to show up. + /// + /// Ignored under emulation -- miri, and qemu-user for a cross-arch test archive -- because + /// `bolero`'s budget is wall-clock (one second unless iterations are pinned), and a second of + /// emulation buys about one sample, which cannot exhibit every shape. The iteration count is + /// pinned so that what this asserts does not depend on how fast the machine running it happens + /// to be. + #[test] + #[cfg_attr(emulated, ignore = "needs many samples; emulation buys too few")] + fn test_spec_generator_is_not_vacuous() { + // Deliberately `std::sync` rather than the `concurrency::sync` facade: under the `shuttle` + // and `loom` features that facade hands out model-checker atomics, which panic outside a + // model-checked execution, and this is an ordinary `#[test]`. + use std::sync::atomic::{AtomicU32, Ordering}; // nosemgrep: rust-no-direct-std-sync-import + + /// Samples to draw. Every shape below is reachable with probability well over 1/16, so + /// this is many orders of magnitude more than "reliable". + const ITERATIONS: usize = 512; + + /// Shapes a generated spec can exhibit; every one must show up somewhere in the sample. + /// Index in this array is the bit position used to record it. + const SHAPES: &[&str] = &[ + "multiple vpcs", + "peerings", + "acl", + "multi-rule acl", + "default expose", + "multiple exposes", + "empty group", + "multi-member group", + "fabricBFD on", + "fabricBFD off", + "fabricBFD absent", + "flow table capacity", + "profiling", + // `config::converters::k8s::config::gateway_config`'s + // `test_fabric_bfd_fans_out_to_all_neighbors` asserts over every underlay BGP + // neighbor, which the converter builds one-for-one from `gateway.neighbors`. An + // empty list satisfies that assertion vacuously, so require a non-empty one to be + // reachable here, where the reachability claims live. + "bgp neighbors", + ]; + + // An atomic bitmask rather than a collection behind a lock: `bolero` runs each case inside + // `catch_unwind`, so whatever the closure captures has to be `RefUnwindSafe`, and the + // project disallows `std::sync::Mutex`. + let seen = AtomicU32::new(0); + let record = |shape: &str| { + #[allow(clippy::expect_used)] + let bit = SHAPES + .iter() + .position(|s| *s == shape) + .expect("shape not listed in SHAPES"); + seen.fetch_or(1 << bit, Ordering::Relaxed); + }; + + bolero::check!() + .with_type::>() + .with_iterations(ITERATIONS) + .for_each(|spec| { + let spec = spec.as_ref(); + + if spec.vpcs.as_ref().is_some_and(|v| v.len() > 1) { + record("multiple vpcs"); + } + if let Some(peerings) = spec.peerings.as_ref().filter(|p| !p.is_empty()) { + record("peerings"); + for peering in peerings.values() { + if let Some(acl) = peering.acl.as_ref() { + record("acl"); + if acl.rules.as_ref().is_some_and(|r| r.len() > 1) { + record("multi-rule acl"); + } + } + for side in peering.peering.iter().flat_map(|p| p.values()) { + let Some(exposes) = side.expose.as_ref() else { + continue; + }; + if exposes.iter().any(|e| e.default == Some(true)) { + record("default expose"); + } + if exposes.len() > 1 { + record("multiple exposes"); + } + } + } + } + for group in spec.groups.iter().flat_map(|g| g.values()) { + match group.members.as_ref().map_or(0, Vec::len) { + 0 => record("empty group"), + 1 => {} + _ => record("multi-member group"), + } + } + match spec.config.as_ref().and_then(|c| c.fabric_bfd) { + Some(true) => record("fabricBFD on"), + Some(false) => record("fabricBFD off"), + None => record("fabricBFD absent"), + } + if let Some(gw) = spec.gateway.as_ref() { + if gw.flow_table_capacity.is_some() { + record("flow table capacity"); + } + if gw.profiling.is_some() { + record("profiling"); + } + if gw.neighbors.as_ref().is_some_and(|n| !n.is_empty()) { + record("bgp neighbors"); + } + } + }); + + let seen = seen.load(Ordering::Relaxed); + let missing: Vec<&str> = SHAPES + .iter() + .enumerate() + .filter(|(bit, _)| seen & (1 << bit) == 0) + .map(|(_, shape)| *shape) + .collect(); + assert!( + missing.is_empty(), + "the spec generator never produced: {missing:?}" + ); + } + + /// The two intake encodings must agree. + /// + /// A field that survives one encoding but not the other would make the k8s and k8s-less modes + /// disagree about the same configuration, which is the failure this pins down. + #[test] + fn test_yaml_and_json_agree() { + bolero::check!() + .with_type::>() + .for_each(|spec| { + let spec = spec.as_ref(); + let from_yaml: GatewayAgentSpec = serde_yaml_ng::from_str( + &serde_yaml_ng::to_string(spec).expect("failed to serialize as YAML"), + ) + .expect("failed to parse YAML"); + let from_json: GatewayAgentSpec = serde_json::from_str( + &serde_json::to_string(spec).expect("failed to serialize as JSON"), + ) + .expect("failed to parse JSON"); + assert_eq!(from_yaml, from_json, "YAML and JSON intake disagree"); + }); + } +} diff --git a/k8s-intf/src/bolero/expose.rs b/k8s-intf/src/bolero/expose.rs index 8750ed9c2..180094eb5 100644 --- a/k8s-intf/src/bolero/expose.rs +++ b/k8s-intf/src/bolero/expose.rs @@ -5,7 +5,7 @@ use std::ops::Bound; use bolero::{Driver, TypeGenerator, ValueGenerator}; -use crate::bolero::support::generate_prefixes; +use crate::bolero::support::{PrefixPool, generate_routable_prefixes}; use crate::bolero::{LegalValue, SubnetMap}; use crate::gateway_agent_crd::{ GatewayAgentPeeringsPeeringExpose, GatewayAgentPeeringsPeeringExposeAs, @@ -17,19 +17,102 @@ use crate::gateway_agent_crd::{ GatewayAgentPeeringsPeeringExposeNatStatic, }; +/// The one legal shape of a "default" expose. +/// +/// A default expose carries no prefixes at all -- the converter rejects `ips`/`as` on one, and +/// `VpcExpose::validate` rejects `nots`/`nat` as well -- so there is nothing here to randomize. +/// A manifest may hold at most one, which is why this is a plain constructor for the caller that +/// owns the expose list rather than a branch inside [`LegalValueExposeGenerator`]. +#[must_use] +pub fn default_expose() -> GatewayAgentPeeringsPeeringExpose { + GatewayAgentPeeringsPeeringExpose { + r#as: None, + ips: None, + default: Some(true), + nat: None, + } +} + /// Generate a legal value for `GatewayAgentPeeringsPeeringExpose` /// /// This is not exhaustive over all legal values due to the complexity of doing this. For example, /// the CIDR generators are not exhaustive; and we use a single port range for all CIDRs rather than /// trying different combinations. +/// +/// Never produces a "default" expose; see [`default_expose`]. +/// +/// By default the result is legal with respect to *conversion* only. For one that also survives +/// `VpcExpose::validate`, see [`LegalValueExposeGenerator::from_pool`]. +/// +/// Both modes draw addresses from the reserved-block-avoiding generators in +/// [`crate::bolero::support`], even though conversion accepts any parseable prefix and only +/// validation rejects reserved space. That makes the default mode narrower than it needs to be, +/// deliberately: nothing on the conversion path branches on prefix length or on which block a +/// prefix falls in -- `process_ip_block` parses the string and stores it -- so the wider range +/// buys no coverage, and one address-generation path is easier to reason about than two. pub struct LegalValueExposeGenerator<'a> { subnets: &'a SubnetMap, + pool: Option<&'a PrefixPool>, } impl<'a> LegalValueExposeGenerator<'a> { #[must_use] pub fn new(subnets: &'a SubnetMap) -> Self { - Self { subnets } + Self { + subnets, + pool: None, + } + } + + /// Draw addresses from `pool`, and restrict generation to exposes that pass validation. + /// + /// `VpcExpose::validate` couples an expose's fields together in ways that cannot be satisfied + /// by generating each field independently: + /// + /// - `ips`, `nots`, `as` and `notAs` must all be the same IP version; + /// - static NAT requires `ips` and `as` to cover the same *number of addresses*; + /// - port forwarding requires exactly one prefix per side, no exclusions, and a port range on + /// each side; + /// - masquerade forbids port ranges entirely; + /// - and, across the whole manifest, no two exposes may overlap. + /// + /// This mode satisfies all of them by generating the tractable subset: one address family per + /// expose, `ips` drawn from the shared pool (so overlap is impossible by construction), and no + /// exclusions, NAT, or `vpcSubnet` references. + /// + /// FIXME: extending this to NAT means generating `ips`/`as` in matched sizes per NAT mode, and + /// to `vpcSubnet` references means coordinating the VPC subnet generator with the same pool. + /// Until then, those shapes are exercised at the conversion level only + /// (`converters::k8s::config::expose`). + #[must_use] + pub fn from_pool(mut self, pool: &'a PrefixPool) -> Self { + self.pool = Some(pool); + self + } + + /// The validation-legal subset: a single family, pool-allocated `ips`, nothing else. + fn generate_from_pool( + d: &mut D, + pool: &PrefixPool, + ) -> Option { + let v4 = d.gen_bool(None)?; + let count = d.gen_u16(Bound::Included(&1), Bound::Included(&4))?; + let ips = pool + .take(count, v4) + .into_iter() + .map(|cidr| GatewayAgentPeeringsPeeringExposeIps { + cidr: Some(cidr), + not: None, + vpc_subnet: None, + }) + .collect(); + Some(GatewayAgentPeeringsPeeringExpose { + r#as: None, + ips: Some(ips), + // Explicit `false` is legal and takes the same path as absent; cover both spellings. + default: d.gen_bool(None)?.then_some(false), + nat: None, + }) } } @@ -37,6 +120,10 @@ impl ValueGenerator for LegalValueExposeGenerator<'_> { type Output = GatewayAgentPeeringsPeeringExpose; fn generate(&self, d: &mut D) -> Option { + if let Some(pool) = self.pool { + return Self::generate_from_pool(d, pool); + } + let num_ips = d.gen_u16(Bound::Included(&1), Bound::Included(&16))?; let num_nots = d.gen_u16(Bound::Included(&0), Bound::Included(&16))?; let num_subnets = std::cmp::max( @@ -57,7 +144,7 @@ impl ValueGenerator for LegalValueExposeGenerator<'_> { let num_v4_not_as = d.gen_u16(Bound::Included(&0), Bound::Included(&num_as_not))?; let num_v6_not_as = num_as_not - num_v4_not_as; - let ips = generate_prefixes(d, num_v4_ips, num_v6_ips)? + let ips = generate_routable_prefixes(d, num_v4_ips, num_v6_ips)? .into_iter() .map(|p| GatewayAgentPeeringsPeeringExposeIps { cidr: Some(p), @@ -65,7 +152,7 @@ impl ValueGenerator for LegalValueExposeGenerator<'_> { vpc_subnet: None, }) .collect::>(); - let nots = generate_prefixes(d, num_v4_nots, num_v6_nots)? + let nots = generate_routable_prefixes(d, num_v4_nots, num_v6_nots)? .into_iter() .map(|p| GatewayAgentPeeringsPeeringExposeIps { cidr: None, @@ -73,13 +160,13 @@ impl ValueGenerator for LegalValueExposeGenerator<'_> { vpc_subnet: None, }) .collect::>(); - let r#as = generate_prefixes(d, num_v4_as, num_v6_as)? + let r#as = generate_routable_prefixes(d, num_v4_as, num_v6_as)? .into_iter() .map(|p| GatewayAgentPeeringsPeeringExposeAs { cidr: Some(p), not: None, }); - let not_as = generate_prefixes(d, num_v4_not_as, num_v6_not_as)? + let not_as = generate_routable_prefixes(d, num_v4_not_as, num_v6_not_as)? .into_iter() .map(|p| GatewayAgentPeeringsPeeringExposeAs { cidr: None, @@ -112,7 +199,8 @@ impl ValueGenerator for LegalValueExposeGenerator<'_> { Some(GatewayAgentPeeringsPeeringExpose { r#as: Some(final_as).filter(|f| !f.is_empty()), ips: Some(final_ips).filter(|f| !f.is_empty()), - default: None, + // Explicit `false` is legal and takes the same path as absent; cover both spellings. + default: d.gen_bool(None)?.then_some(false), nat: if has_as { Some( d.produce::>()? diff --git a/k8s-intf/src/bolero/gateway.rs b/k8s-intf/src/bolero/gateway.rs index ceb92d1b3..39bcc6102 100644 --- a/k8s-intf/src/bolero/gateway.rs +++ b/k8s-intf/src/bolero/gateway.rs @@ -14,7 +14,7 @@ use crate::bolero::Normalize; use crate::gateway_agent_crd::{ GatewayAgentGateway, GatewayAgentGatewayGroups, GatewayAgentGatewayInterfaces, - GatewayAgentGatewayLogs, GatewayAgentGatewayNeighbors, + GatewayAgentGatewayLogs, GatewayAgentGatewayNeighbors, GatewayAgentGatewayProfiling, }; impl TypeGenerator for LegalValue { @@ -42,12 +42,18 @@ impl TypeGenerator for LegalValue { Some(LegalValue(GatewayAgentGateway { asn: Some(d.gen_u32(Bound::Included(&1), Bound::Unbounded)?), - flow_table_capacity: None, + // The converter requires this to be non-zero and to fit in a `usize`; zero is an + // illegal value, so it is left to the hostile generators to produce. + flow_table_capacity: d.produce::>()?.map(|capacity| capacity.max(1)), groups: Some(groups), logs: Some(d.produce::>()?.take()), interfaces: Some(interfaces).filter(|i| !i.is_empty()), neighbors: Some(neighbors).filter(|n| !n.is_empty()), - profiling: None, // FIXME(mvachhar) Add a proper implementation + profiling: d + .produce::>()? + .map(|enabled| GatewayAgentGatewayProfiling { + enabled: Some(enabled), + }), protocol_ip: Some(format!( "{}/{}", d.produce::()?, diff --git a/k8s-intf/src/bolero/gwgroups.rs b/k8s-intf/src/bolero/gwgroups.rs index 5cf5dc82c..d62082950 100644 --- a/k8s-intf/src/bolero/gwgroups.rs +++ b/k8s-intf/src/bolero/gwgroups.rs @@ -1,45 +1,135 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright Open Network Fabric Authors -use bolero::{Driver, TypeGenerator}; +use bolero::{Driver, TypeGenerator, ValueGenerator}; use net::ipv4::UnicastIpv4Addr; +use std::collections::BTreeSet; +use std::net::Ipv4Addr; use std::ops::Bound; use crate::bolero::LegalValue; +use crate::bolero::support::{K8sName, K8sNameGenerator}; use crate::gateway_agent_crd::{ GatewayAgentGatewayGroups, GatewayAgentGroups, GatewayAgentGroupsMembers, }; +/// Largest number of members a generated group may have. +/// +/// A group's members are ranked by position after sorting, and `ExternalConfig::validate` demands +/// a community for every rank, so `LegalValue` must supply at least this many +/// communities for the resulting config to validate. +pub const MAX_GROUP_MEMBERS: usize = 10; + impl TypeGenerator for LegalValue { fn generate(driver: &mut D) -> Option { let g = GatewayAgentGatewayGroups { - name: driver.produce::(), + name: driver + .produce::>()? + .map(crate::bolero::support::K8sName::take), priority: Some(driver.gen_u32(Bound::Included(&0), Bound::Included(&10))?), }; Some(LegalValue(g)) } } -impl TypeGenerator for GatewayAgentGroupsMembers { +/// Generate one member of a gateway group. +/// +/// `priority` is deliberately narrow (`0..=10`) so that generated groups contain ties, which is +/// what exercises the name/address tiebreak in `GwGroupMember`'s `Ord`. That restriction is why +/// this is a `LegalValue` rather than a bare `TypeGenerator` on the CRD type. +impl TypeGenerator for LegalValue { fn generate(driver: &mut D) -> Option { let gmember = GatewayAgentGroupsMembers { - name: driver.produce::()?, + name: driver.produce::()?.take(), priority: driver.gen_u32(Bound::Included(&0), Bound::Included(&10))?, vtep_ip: driver.produce::()?.to_string(), }; - Some(gmember) + Some(LegalValue(gmember)) } } -impl TypeGenerator for GatewayAgentGroups { +/// Generate a gateway group with `0..=MAX_GROUP_MEMBERS` members. +/// +/// `GwGroup::add_member` rejects duplicate member names *and* duplicate member addresses, so both +/// have to be unique within a group for the value to be legal. Collisions are repaired in place +/// (lengthen the name, walk the address) rather than by redrawing, so that every value the driver +/// produced still contributes and a collision costs a bounded amount of work. +impl TypeGenerator for LegalValue { fn generate(driver: &mut D) -> Option { - let num_members = driver.gen_usize(Bound::Included(&0), Bound::Included(&10))?; - let mut members = vec![]; - if num_members > 0 { - members.push(driver.produce::()?); + let num_members = + driver.gen_usize(Bound::Included(&0), Bound::Included(&MAX_GROUP_MEMBERS))?; + + let mut members = Vec::with_capacity(num_members); + let mut names = BTreeSet::new(); + let mut addrs = BTreeSet::new(); + + for _ in 0..num_members { + let mut member = driver + .produce::>()? + .take(); + + while !names.insert(member.name.clone()) { + member.name.push('x'); + } + + // `LegalValue` draws a unicast address, so parsing it back + // cannot fail. Walk it on collision, skipping over the non-unicast space. + #[allow(clippy::unwrap_used)] + let mut addr = member.vtep_ip.parse::().unwrap().to_bits(); + while !addrs.insert(addr) { + addr = addr.wrapping_add(1); + let candidate = Ipv4Addr::from(addr); + if candidate.is_multicast() || candidate.is_broadcast() { + addr = 0x0100_0000; + } + } + member.vtep_ip = Ipv4Addr::from(addr).to_string(); + + members.push(member); } - Some(GatewayAgentGroups { + + Some(LegalValue(GatewayAgentGroups { members: Some(members), - }) + })) + } +} + +/// Generate the `spec.groups` map along with the group names, which peerings must reference. +/// +/// Returns the map plus the sorted list of names, so the caller can hand the names to +/// [`crate::bolero::peering::LegalValuePeeringsGenerator`]: a peering naming a group that is not +/// in this table fails `ExternalConfig::validate`. +pub struct LegalValueGroupsTableGenerator { + count: usize, +} + +impl LegalValueGroupsTableGenerator { + #[must_use] + pub fn new(count: usize) -> Self { + Self { count } + } +} + +impl ValueGenerator for LegalValueGroupsTableGenerator { + type Output = ( + std::collections::BTreeMap, + Vec, + ); + + fn generate(&self, d: &mut D) -> Option { + let mut groups = std::collections::BTreeMap::new(); + let name_gen = K8sNameGenerator::new(1, 12); + let mut names = Vec::with_capacity(self.count); + for i in 0..self.count { + // Suffix with the index so group names are distinct without a retry loop; + // `GwGroupTable::add_group` rejects duplicates. + let name = format!("{}-{i}", name_gen.generate(d)?); + groups.insert( + name.clone(), + d.produce::>()?.take(), + ); + names.push(name); + } + Some((groups, names)) } } diff --git a/k8s-intf/src/bolero/mod.rs b/k8s-intf/src/bolero/mod.rs index 078ef00a1..e43739b4d 100644 --- a/k8s-intf/src/bolero/mod.rs +++ b/k8s-intf/src/bolero/mod.rs @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright Open Network Fabric Authors +pub mod acl; pub mod bgp; pub mod crd; pub mod expose; @@ -8,6 +9,7 @@ pub mod gateway; pub mod gwgroups; pub mod interface; pub mod logs; +pub mod mutate; pub mod peering; pub mod spec; pub mod support; diff --git a/k8s-intf/src/bolero/mutate.rs b/k8s-intf/src/bolero/mutate.rs new file mode 100644 index 000000000..a8188043d --- /dev/null +++ b/k8s-intf/src/bolero/mutate.rs @@ -0,0 +1,423 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Hostile generators: legal configs with one thing deliberately wrong. +//! +//! Every other generator in this module produces values the system should *accept*. That leaves +//! the rejection paths -- the ~30 `FromK8sConversionError` and `ConfigError` variants a malformed +//! CRD can provoke -- covered only by hand-written cases. Those paths matter: config arrives from +//! outside the dataplane, so a malformed `GatewayAgent` must be refused cleanly rather than +//! panicking or half-applying. +//! +//! The mutation is carried *in the generated value* rather than applied by a loop in the test body. +//! That is what makes failures useful: `bolero` can shrink to the smallest config that still +//! exhibits the problem, and the failure report names the mutation that caused it. +//! +//! For that naming to mean anything, a mutation has to trip the guard it is named after rather than +//! some unrelated one. The ACL mutations are the case worth checking, since they graft a `match` +//! onto a rule whose other fields are whatever the generator drew: a `proto` of, say, `58` makes +//! ports illegal on any rule, and an address match has to intersect what its side exposes. Both of +//! those are `ValidatedAcl` rules, and every guard these mutations aim at -- `resolve_prefix` for a +//! dangling or doubly-specified subnet, `parse_entry_ports` for an empty list, `parse_port_ranges` +//! for an inverted range -- lives in `AclPattern::try_from`, which runs during conversion and so +//! fails first. Nothing here needs to pin `proto` or the addresses to keep the labels honest. + +use std::collections::BTreeMap; +use std::ops::Bound; + +use bolero::{Driver, TypeGenerator}; + +use crate::bolero::LegalValue; +use crate::gateway_agent_crd::{GatewayAgent, GatewayAgentPeeringsAclRulesMatchSrc}; + +/// A single, named way to corrupt an otherwise legal [`GatewayAgent`]. +/// +/// Each variant targets a specific guard in the intake path. Variants whose target is absent from +/// a given config are no-ops for that config -- see [`Mutated::is_noop`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Mutation { + /// Clear `metadata.generation`, which `validate_metadata` requires. + DropGeneration, + /// Set `metadata.generation` to zero, which `validate_metadata` rejects explicitly. + ZeroGeneration, + /// Clear `metadata.name`. + DropName, + /// Blank `metadata.name`; distinct from absent, and separately rejected. + EmptyName, + /// Move the object out of the `default` namespace, the only one accepted. + WrongNamespace, + /// Clear `spec.gateway`, without which there is no device or underlay to build. + DropGateway, + /// Point a peering at a gateway group that does not exist. + DanglingGatewayGroup, + /// Name a VPC subnet that does not exist from an ACL match. + DanglingAclSubnet, + /// Give two VPCs the same VNI. + DuplicateVni, + /// Give two VPCs the same internal id. + DuplicateVpcInternalId, + /// Set a VPC's VNI to zero, outside the legal 1..2^24 range. + ZeroVni, + /// Corrupt a VPC subnet CIDR's mask beyond its address family. + OversizeSubnetMask, + /// Replace a VPC subnet CIDR with text that is not a prefix at all. + MalformedSubnetCidr, + /// Set `flowTableCapacity` to zero, which must be a non-zero value. + ZeroFlowTableCapacity, + /// Declare a peering over three VPCs; a peering is strictly between two. + ThreeSidedPeering, + /// Give an ACL rule a `from` naming neither of the peering's VPCs. + AclFromUnknownVpc, + /// Give an ACL match an inverted port range. + InvertedAclPortRange, + /// Give an ACL match an unparseable protocol. + MalformedAclProto, + /// Present an empty `ports` list, which is rejected rather than read as "all ports". + EmptyAclPortsList, + /// Set both `cidr` and `vpcSubnet` on an ACL match entry; they are mutually exclusive. + AclCidrAndSubnet, + /// Map a community to a non-numeric priority. + MalformedCommunityPriority, + /// Give a gateway group two members with the same name. + DuplicateGroupMember, + /// Give a gateway group member an unparseable VTEP address. + MalformedGroupMemberAddress, +} + +/// Every mutation, for the generator to draw from. +const MUTATIONS: &[Mutation] = &[ + Mutation::DropGeneration, + Mutation::ZeroGeneration, + Mutation::DropName, + Mutation::EmptyName, + Mutation::WrongNamespace, + Mutation::DropGateway, + Mutation::DanglingGatewayGroup, + Mutation::DanglingAclSubnet, + Mutation::DuplicateVni, + Mutation::DuplicateVpcInternalId, + Mutation::ZeroVni, + Mutation::OversizeSubnetMask, + Mutation::MalformedSubnetCidr, + Mutation::ZeroFlowTableCapacity, + Mutation::ThreeSidedPeering, + Mutation::AclFromUnknownVpc, + Mutation::InvertedAclPortRange, + Mutation::MalformedAclProto, + Mutation::EmptyAclPortsList, + Mutation::AclCidrAndSubnet, + Mutation::MalformedCommunityPriority, + Mutation::DuplicateGroupMember, + Mutation::MalformedGroupMemberAddress, +]; + +/// A [`GatewayAgent`] that was legal until one [`Mutation`] was applied. +/// +/// The system must reject this -- or, when the mutation found nothing to corrupt, accept it. Which +/// of the two is expected is reported by [`Mutated::is_noop`], so a test can assert the strong +/// property (rejected, with a specific error) without treating "nothing to mutate" as a failure. +#[derive(Debug, Clone)] +pub struct Mutated { + agent: GatewayAgent, + mutation: Mutation, + applied: bool, +} + +impl Mutated { + #[must_use] + pub fn agent(&self) -> &GatewayAgent { + &self.agent + } + + #[must_use] + pub fn mutation(&self) -> Mutation { + self.mutation + } + + /// Whether the mutation found nothing to corrupt, leaving the config legal. + /// + /// A config with no peerings has no gateway group reference to break, for instance. Rather + /// than resample -- which would bias the distribution and complicate shrinking -- the value is + /// returned as-is and flagged. + #[must_use] + pub fn is_noop(&self) -> bool { + !self.applied + } +} + +/// Mutate the first VPC subnet CIDR found, via `edit`. +/// +/// Reports "not applied" when the spec has no peerings, because nothing then reads the subnet: +/// `Vpc::try_from` ignores `subnets` outright, and the CIDRs are parsed only by the overlay +/// converter's `extract_subnets`, which runs only when both VPCs and peerings are present. A +/// malformed CIDR on an unpeered VPC is therefore accepted as-is, so a mutation there has corrupted +/// nothing the intake path looks at. +fn edit_first_subnet_cidr(agent: &mut GatewayAgent, edit: impl FnOnce(&mut String)) -> bool { + if agent.spec.peerings.as_ref().is_none_or(BTreeMap::is_empty) { + return false; + } + let Some(vpcs) = agent.spec.vpcs.as_mut() else { + return false; + }; + for vpc in vpcs.values_mut() { + let Some(subnets) = vpc.subnets.as_mut() else { + continue; + }; + for subnet in subnets.values_mut() { + if let Some(cidr) = subnet.cidr.as_mut() { + edit(cidr); + return true; + } + } + } + false +} + +/// Mutate the first ACL rule found, via `edit`. +fn edit_first_acl_rule( + agent: &mut GatewayAgent, + edit: impl FnOnce(&mut crate::gateway_agent_crd::GatewayAgentPeeringsAclRules), +) -> bool { + let Some(peerings) = agent.spec.peerings.as_mut() else { + return false; + }; + for peering in peerings.values_mut() { + let Some(acl) = peering.acl.as_mut() else { + continue; + }; + let Some(rules) = acl.rules.as_mut() else { + continue; + }; + if let Some(rule) = rules.first_mut() { + edit(rule); + return true; + } + } + false +} + +/// Give the first ACL rule found a `match.src` consisting of the single supplied entry. +fn set_first_acl_src(agent: &mut GatewayAgent, src: GatewayAgentPeeringsAclRulesMatchSrc) -> bool { + edit_first_acl_rule(agent, |rule| { + let m = rule.r#match.get_or_insert( + crate::gateway_agent_crd::GatewayAgentPeeringsAclRulesMatch { + dst: None, + proto: None, + src: None, + }, + ); + m.src = Some(vec![src]); + }) +} + +/// Apply `mutation` to `agent`, reporting whether it found anything to change. +#[allow(clippy::too_many_lines)] +fn apply(agent: &mut GatewayAgent, mutation: Mutation) -> bool { + match mutation { + Mutation::DropGeneration => agent.metadata.generation.take().is_some(), + Mutation::ZeroGeneration => { + agent.metadata.generation = Some(0); + true + } + Mutation::DropName => agent.metadata.name.take().is_some(), + Mutation::EmptyName => { + agent.metadata.name = Some(String::new()); + true + } + Mutation::WrongNamespace => { + agent.metadata.namespace = Some("kube-system".to_string()); + true + } + Mutation::DropGateway => agent.spec.gateway.take().is_some(), + Mutation::DanglingGatewayGroup => { + let Some(peerings) = agent.spec.peerings.as_mut() else { + return false; + }; + let Some(peering) = peerings.values_mut().next() else { + return false; + }; + peering.gateway_group = Some("no-such-group".to_string()); + true + } + Mutation::DanglingAclSubnet => set_first_acl_src( + agent, + GatewayAgentPeeringsAclRulesMatchSrc { + cidr: None, + ports: None, + vpc_subnet: Some("no-such-subnet".to_string()), + }, + ), + Mutation::AclCidrAndSubnet => set_first_acl_src( + agent, + GatewayAgentPeeringsAclRulesMatchSrc { + cidr: Some("10.0.0.0/24".to_string()), + ports: None, + vpc_subnet: Some("also-a-subnet".to_string()), + }, + ), + Mutation::EmptyAclPortsList => set_first_acl_src( + agent, + GatewayAgentPeeringsAclRulesMatchSrc { + cidr: Some("10.0.0.0/24".to_string()), + ports: Some(Vec::new()), + vpc_subnet: None, + }, + ), + Mutation::InvertedAclPortRange => set_first_acl_src( + agent, + GatewayAgentPeeringsAclRulesMatchSrc { + cidr: Some("10.0.0.0/24".to_string()), + ports: Some(vec!["9000-80".to_string()]), + vpc_subnet: None, + }, + ), + Mutation::MalformedAclProto => edit_first_acl_rule(agent, |rule| { + let m = rule.r#match.get_or_insert( + crate::gateway_agent_crd::GatewayAgentPeeringsAclRulesMatch { + dst: None, + proto: None, + src: None, + }, + ); + m.proto = Some("not-a-protocol".to_string()); + }), + Mutation::AclFromUnknownVpc => edit_first_acl_rule(agent, |rule| { + rule.from = Some("no-such-vpc".to_string()); + rule.to = None; + }), + Mutation::DuplicateVni => { + let Some(vpcs) = agent.spec.vpcs.as_mut() else { + return false; + }; + let Some(first_vni) = vpcs.values().next().and_then(|vpc| vpc.vni) else { + return false; + }; + // Needs a second VPC to collide with. + if vpcs.len() < 2 { + return false; + } + #[allow(clippy::unwrap_used)] // len >= 2 checked above + let second = vpcs.values_mut().nth(1).unwrap(); + if second.vni == Some(first_vni) { + return false; + } + second.vni = Some(first_vni); + true + } + Mutation::DuplicateVpcInternalId => { + let Some(vpcs) = agent.spec.vpcs.as_mut() else { + return false; + }; + if vpcs.len() < 2 { + return false; + } + let Some(first_id) = vpcs.values().next().and_then(|vpc| vpc.internal_id.clone()) + else { + return false; + }; + #[allow(clippy::unwrap_used)] // len >= 2 checked above + let second = vpcs.values_mut().nth(1).unwrap(); + if second.internal_id.as_ref() == Some(&first_id) { + return false; + } + second.internal_id = Some(first_id); + true + } + Mutation::ZeroVni => { + let Some(vpcs) = agent.spec.vpcs.as_mut() else { + return false; + }; + let Some(vpc) = vpcs.values_mut().next() else { + return false; + }; + vpc.vni = Some(0); + true + } + Mutation::OversizeSubnetMask => edit_first_subnet_cidr(agent, |cidr| { + // A v4 prefix cannot have a /33, nor a v6 one a /129. + let oversize = if cidr.contains(':') { "/129" } else { "/33" }; + if let Some((addr, _)) = cidr.split_once('/') { + *cidr = format!("{addr}{oversize}"); + } + }), + Mutation::MalformedSubnetCidr => edit_first_subnet_cidr(agent, |cidr| { + *cidr = "definitely/not/a/prefix".to_string(); + }), + Mutation::ZeroFlowTableCapacity => { + let Some(gateway) = agent.spec.gateway.as_mut() else { + return false; + }; + gateway.flow_table_capacity = Some(0); + true + } + Mutation::ThreeSidedPeering => { + let Some(peerings) = agent.spec.peerings.as_mut() else { + return false; + }; + let Some(peering) = peerings.values_mut().next() else { + return false; + }; + let Some(sides) = peering.peering.as_mut() else { + return false; + }; + let Some(extra) = sides.values().next().cloned() else { + return false; + }; + sides.insert("a-third-vpc".to_string(), extra); + true + } + Mutation::MalformedCommunityPriority => { + let communities = agent + .spec + .communities + .get_or_insert_with(std::collections::BTreeMap::new); + communities.insert("not-a-number".to_string(), "65000:999".to_string()); + true + } + Mutation::DuplicateGroupMember => { + let Some(groups) = agent.spec.groups.as_mut() else { + return false; + }; + for group in groups.values_mut() { + let Some(members) = group.members.as_mut() else { + continue; + }; + let Some(first) = members.first().cloned() else { + continue; + }; + members.push(first); + return true; + } + false + } + Mutation::MalformedGroupMemberAddress => { + let Some(groups) = agent.spec.groups.as_mut() else { + return false; + }; + for group in groups.values_mut() { + let Some(members) = group.members.as_mut() else { + continue; + }; + if let Some(member) = members.first_mut() { + member.vtep_ip = "300.1.2.3".to_string(); + return true; + } + } + false + } + } +} + +impl TypeGenerator for Mutated { + fn generate(d: &mut D) -> Option { + let mut agent = d.produce::>()?.take(); + let index = d.gen_usize(Bound::Included(&0), Bound::Excluded(&MUTATIONS.len()))?; + let mutation = MUTATIONS[index]; + let applied = apply(&mut agent, mutation); + Some(Self { + agent, + mutation, + applied, + }) + } +} diff --git a/k8s-intf/src/bolero/peering.rs b/k8s-intf/src/bolero/peering.rs index cf7ad87af..60eb20ec3 100644 --- a/k8s-intf/src/bolero/peering.rs +++ b/k8s-intf/src/bolero/peering.rs @@ -6,7 +6,9 @@ use std::ops::Bound; use bolero::{Driver, ValueGenerator}; +use crate::bolero::acl::LegalValueAclGenerator; use crate::bolero::expose::LegalValueExposeGenerator; +use crate::bolero::support::{PrefixPool, choose}; use crate::bolero::{SubnetMap, VpcSubnetMap}; use crate::gateway_agent_crd::{GatewayAgentPeerings, GatewayAgentPeeringsPeering}; @@ -14,14 +16,43 @@ use crate::gateway_agent_crd::{GatewayAgentPeerings, GatewayAgentPeeringsPeering /// /// This does not attempt to be exhaustive for vpc names, just generate relevant legal values. /// In particular, subnet names are restricted. Lengths of various lists is also limited to 16 +/// +/// By default this never generates a "default" expose, because legality of one is not decidable +/// from a single side of a peering: `VpcManifest::validate` allows at most one per manifest, and +/// `Peering::validate` forbids a default expose on *both* sides of the same peering. Use +/// [`LegalValuePeeringsPeeringGenerator::allow_default_expose`] from a caller that owns both +/// sides and can uphold that. pub struct LegalValuePeeringsPeeringGenerator<'a> { subnets: &'a SubnetMap, + allow_default: bool, + pool: Option<&'a PrefixPool>, } impl<'a> LegalValuePeeringsPeeringGenerator<'a> { #[must_use] pub fn new(subnets: &'a SubnetMap) -> Self { - Self { subnets } + Self { + subnets, + allow_default: false, + pool: None, + } + } + + /// Permit this side to carry (at most one) default expose. + /// + /// The caller must not enable this for both sides of the same peering. + #[must_use] + pub fn allow_default_expose(mut self) -> Self { + self.allow_default = true; + self + } + + /// Draw expose addresses from `pool`; see + /// [`crate::bolero::expose::LegalValueExposeGenerator::from_pool`]. + #[must_use] + pub fn from_pool(mut self, pool: &'a PrefixPool) -> Self { + self.pool = Some(pool); + self } } @@ -31,10 +62,18 @@ impl ValueGenerator for LegalValuePeeringsPeeringGenerator<'_> { fn generate(&self, d: &mut D) -> Option { let num_expose = d.gen_usize(Bound::Included(&1), Bound::Included(&16))?; let expose_gen = LegalValueExposeGenerator::new(self.subnets); - let expose = (0..num_expose) + let expose_gen = match self.pool { + Some(pool) => expose_gen.from_pool(pool), + None => expose_gen, + }; + let mut expose = (0..num_expose) .map(|_| expose_gen.generate(d)) .collect::>>()?; + if self.allow_default && d.gen_bool(None)? { + expose.push(crate::bolero::expose::default_expose()); + } + Some(GatewayAgentPeeringsPeering { expose: Some(expose).filter(|e| !e.is_empty()), }) @@ -44,9 +83,15 @@ impl ValueGenerator for LegalValuePeeringsPeeringGenerator<'_> { /// Generate legal values for `GatewayAgentPeerings` /// /// This does not attempt to be exhaustive for vpc names, just generate relevant legal values. +/// +/// `gwgroup_names` must be the names present in the spec's `groups` table: +/// `ExternalConfig::validate` rejects a peering whose `gatewayGroup` is not in that table, so a +/// peering generated against an unrelated name is convertible but not *valid*. pub struct LegalValuePeeringsGenerator<'a> { vpc_subnets: &'a VpcSubnetMap, vpc_names: Vec<&'a String>, + gwgroup_names: &'a [String], + pool: Option<&'a PrefixPool>, } impl<'a> LegalValuePeeringsGenerator<'a> { @@ -54,17 +99,37 @@ impl<'a> LegalValuePeeringsGenerator<'a> { /// /// # Errors /// - /// Returns an error if there are less than two VPCs in the subnet map. - pub fn new(vpc_subnets: &'a VpcSubnetMap) -> Result { + /// Returns an error if there are less than two VPCs in the subnet map, or if no gateway group + /// names were supplied for the peering to reference. + pub fn new(vpc_subnets: &'a VpcSubnetMap, gwgroup_names: &'a [String]) -> Result { if vpc_subnets.len() < 2 { return Err("At least two VPCs are required to generate peerings".to_string()); } + if gwgroup_names.is_empty() { + return Err("At least one gateway group is required to generate peerings".to_string()); + } let vpc_names = vpc_subnets.keys().collect(); Ok(Self { vpc_subnets, vpc_names, + gwgroup_names, + pool: None, }) } + + /// Restrict generation to peerings that survive `ExternalConfig::validate`, drawing expose + /// addresses from `pool`. + /// + /// Without this, a generated peering is legal with respect to conversion only: validation + /// couples exposes to each other and ACL rules to the exposes. See + /// [`crate::bolero::expose::LegalValueExposeGenerator::from_pool`] and + /// [`crate::bolero::acl::LegalValueAclGenerator::independent_of_exposes`] for what each + /// restriction costs in coverage. + #[must_use] + pub fn validation_legal(mut self, pool: &'a PrefixPool) -> Self { + self.pool = Some(pool); + self + } } fn pick2<'a, D: Driver, T>(d: &mut D, items: &[&'a T]) -> Option<[&'a T; 2]> { @@ -78,23 +143,105 @@ fn pick2<'a, D: Driver, T>(d: &mut D, items: &[&'a T]) -> Option<[&'a T; 2]> { Some([items[index1], items[index2]]) } -impl ValueGenerator for LegalValuePeeringsGenerator<'_> { - type Output = GatewayAgentPeerings; +impl LegalValuePeeringsGenerator<'_> { + /// Generate a peering between two named VPCs, optionally letting one side carry a default + /// expose. + /// + /// Prefer this over [`ValueGenerator::generate`] when generating several peerings for one + /// spec: two validation rules span the whole overlay and only the caller building the whole set + /// can uphold them. + /// + /// - `Vpc::check_peering_count` rejects an overlay in which the same pair of VPCs peers more + /// than once, so the caller must not repeat a pair. + /// - `VpcRouteTable::validate` rejects a VPC with more than one default destination, and a + /// default expose on one side of a peering *is* a default destination for the other side. So + /// `default_side` must be `None` unless the opposite VPC has no default from any peering. + pub fn generate_for_pair( + &self, + d: &mut D, + vpc_a: &str, + vpc_b: &str, + default_side: Option, + ) -> Option { + self.generate_inner(d, [vpc_a, vpc_b], default_side) + } - fn generate(&self, d: &mut D) -> Option { - let vpc_names = pick2(d, &self.vpc_names)?; + fn generate_inner( + &self, + d: &mut D, + vpc_names: [&str; 2], + default_side: Option, + ) -> Option { let empty_map = SubnetMap::new(); - let peerings_gens = vpc_names.map(|n| { - LegalValuePeeringsPeeringGenerator::new(self.vpc_subnets.get(n).unwrap_or(&empty_map)) - }); - let peering = (0..=1) - .map(|i| Some((vpc_names[i].clone(), peerings_gens[i].generate(d)?))) - .collect::>>()?; + + let side_gen = |name: &str| { + let side = LegalValuePeeringsPeeringGenerator::new( + self.vpc_subnets.get(name).unwrap_or(&empty_map), + ); + match self.pool { + Some(pool) => side.from_pool(pool), + None => side, + } + }; + let first = side_gen(vpc_names[0]); + let second = side_gen(vpc_names[1]); + let (first, second) = match default_side { + Some(0) => (first.allow_default_expose(), second), + Some(1) => (first, second.allow_default_expose()), + _ => (first, second), + }; + + let peering = BTreeMap::from([ + (vpc_names[0].to_string(), first.generate(d)?), + (vpc_names[1].to_string(), second.generate(d)?), + ]); + + // `VpcPeering::try_from` names the sides by iterating the `peering` map, so `left` is the + // lexicographically smaller of the two VPC names. The ACL's `from`/`to` must agree with + // that, so derive the names from the map rather than from `vpc_names`. + let mut side_names = peering.keys(); + let left_name = side_names.next()?; + let right_name = side_names.next()?; + + let acl = if d.gen_bool(None)? { + let acl_gen = LegalValueAclGenerator::new(self.vpc_subnets, left_name, right_name); + let acl_gen = if self.pool.is_some() { + acl_gen.independent_of_exposes() + } else { + acl_gen + }; + Some(acl_gen.generate(d)?) + } else { + None + }; Some(GatewayAgentPeerings { - gateway_group: Some(d.produce::()?), + gateway_group: Some(choose(d, self.gwgroup_names)?), peering: Some(peering), - acl: None, // FIXME: Add a proper implementation when used + acl, }) } } + +/// Generates a peering between a randomly chosen pair of the known VPCs. +/// +/// Safe for a spec holding a single peering; use +/// [`LegalValuePeeringsGenerator::generate_for_pair`] when generating several, so that no pair of +/// VPCs peers twice and no VPC collects two default destinations. +impl ValueGenerator for LegalValuePeeringsGenerator<'_> { + type Output = GatewayAgentPeerings; + + fn generate(&self, d: &mut D) -> Option { + let vpc_names = pick2(d, &self.vpc_names)?; + // With only one peering in play, either side may carry a default. + let default_side = match d.gen_usize(Bound::Included(&0), Bound::Included(&2))? { + side @ (0 | 1) => Some(side), + _ => None, + }; + self.generate_inner( + d, + [vpc_names[0].as_str(), vpc_names[1].as_str()], + default_side, + ) + } +} diff --git a/k8s-intf/src/bolero/spec.rs b/k8s-intf/src/bolero/spec.rs index e89abbda4..100406994 100644 --- a/k8s-intf/src/bolero/spec.rs +++ b/k8s-intf/src/bolero/spec.rs @@ -8,10 +8,12 @@ use bolero::{Driver, TypeGenerator, ValueGenerator}; use lpm::prefix::Prefix; +use crate::bolero::gwgroups::{LegalValueGroupsTableGenerator, MAX_GROUP_MEMBERS}; use crate::bolero::peering::LegalValuePeeringsGenerator; +use crate::bolero::support::{K8sName, PrefixPool}; use crate::bolero::{LegalValue, SubnetMap, VpcSubnetMap}; use crate::gateway_agent_crd::{ - GatewayAgentGateway, GatewayAgentGroups, GatewayAgentSpec, GatewayAgentVpcs, + GatewayAgentConfig, GatewayAgentGateway, GatewayAgentSpec, GatewayAgentVpcs, }; fn extract_subnets(vpcs: &BTreeMap) -> VpcSubnetMap { @@ -46,6 +48,14 @@ fn increment_string(s: &mut str) { /// Generate a random legal `GatewayAgentSpec` /// +/// "Legal" here means the spec converts *and* validates: the whole value has to hold together, +/// not just each subtree on its own. Two cross-references make that non-local: +/// +/// - a peering's `gatewayGroup` must name a group in `groups`, so the group table is generated +/// first and its names handed to the peering generator; +/// - `ExternalConfig::validate` demands a community for every member rank within a group, so the +/// community table is sized to the largest group that could be generated. +/// /// This does not cover all legal `GatewayAgentSpecs`, /// it is limited by the underlying generators and it generates /// vpcs and peerings with a fixed name pattern and not all @@ -78,30 +88,96 @@ impl TypeGenerator for LegalValue { let vpc_subnet_map = extract_subnets(&vpcs); + // Generate the group table first: peerings have to reference it by name. + let num_groups = d.gen_usize(Bound::Included(&1), Bound::Included(&6))?; + let (groups, gwgroup_names) = + LegalValueGroupsTableGenerator::new(num_groups).generate(d)?; + let mut peerings = BTreeMap::new(); if num_peerings > 0 { - let peering_gen = LegalValuePeeringsGenerator::new(&vpc_subnet_map).unwrap(); - for i in 0..num_peerings { - peerings.insert(format!("peering{i}"), peering_gen.generate(d)?); - } - } + // One pool for the whole spec, so no two exposes anywhere in it can overlap -- a + // stronger invariant than validation demands, and a cheaper one to be sure of. + let pool = PrefixPool::new(d)?; + + // A default expose reaches the route table as `0.0.0.0/0`, which overlaps every other + // v4 destination, and `VpcRouteTable::validate` requires overlapping destinations to + // share a gateway group. So a spec can have default exposes or a diversity of gateway + // groups, but not both. Generate each shape half the time rather than dropping one. + let allow_defaults = d.gen_bool(None)?; + let groups_in_play: &[String] = if allow_defaults { + let pick = + d.gen_usize(Bound::Included(&0), Bound::Excluded(&gwgroup_names.len()))?; + &gwgroup_names[pick..=pick] + } else { + &gwgroup_names + }; - let num_groups = d.gen_usize(Bound::Included(&0), Bound::Included(&6))?; - let mut groups = BTreeMap::new(); - for i in 0..=num_groups { - groups.insert(format!("gwgroup-{i}"), d.produce::()?); + #[allow(clippy::unwrap_used)] + // Cannot fail: `num_peerings > 0` implies `num_vpcs > 1`, and `num_groups >= 1`. + let peering_gen = LegalValuePeeringsGenerator::new(&vpc_subnet_map, groups_in_play) + .unwrap() + .validation_legal(&pool); + + // `Vpc::check_peering_count` rejects an overlay where the same pair of VPCs peers more + // than once, so draw peerings from the set of distinct pairs without replacement. + let names: Vec<&String> = vpc_subnet_map.keys().collect(); + let mut pairs: Vec<(usize, usize)> = (0..names.len()) + .flat_map(|a| ((a + 1)..names.len()).map(move |b| (a, b))) + .collect(); + + // A default expose on one side of a peering is a default *destination* for the other + // side, and `VpcRouteTable::validate` allows a VPC only one of those. Track which + // VPCs have been offered one and refuse to offer a second. The bookkeeping is + // conservative: a VPC is marked when a default is *permitted*, not when one is + // actually emitted, since that choice is the side generator's to make. + let mut offered_default: HashSet = HashSet::new(); + + for i in 0..num_peerings.min(pairs.len()) { + let choice = d.gen_usize(Bound::Included(&0), Bound::Excluded(&pairs.len()))?; + let (a, b) = pairs.swap_remove(choice); + + // Side 0 carrying a default targets `b`, and side 1 targets `a`. + let mut candidates = Vec::with_capacity(2); + if allow_defaults && !offered_default.contains(&b) { + candidates.push((0_usize, b)); + } + if allow_defaults && !offered_default.contains(&a) { + candidates.push((1_usize, a)); + } + let default_side = if candidates.is_empty() || d.gen_bool(None)? { + None + } else { + let pick = + d.gen_usize(Bound::Included(&0), Bound::Excluded(&candidates.len()))?; + let (side, target) = candidates[pick]; + offered_default.insert(target); + Some(side) + }; + + peerings.insert( + format!("peering{i}"), + peering_gen.generate_for_pair(d, names[a], names[b], default_side)?, + ); + } } - let num_communities = d.gen_usize(Bound::Included(&0), Bound::Included(&9))?; + // A group of N members occupies ranks `0..N`, and validation requires a community at + // every rank it finds. Sizing the table to the largest group a generated spec can hold + // keeps that satisfied without coupling to how many members were actually drawn. let mut communities = BTreeMap::new(); - for i in 0..=num_communities { - let community = format!("65000:{}", 100 + i); - communities.insert(i.to_string(), community); + for i in 0..MAX_GROUP_MEMBERS { + communities.insert(i.to_string(), format!("65000:{}", 100 + i)); } Some(LegalValue(GatewayAgentSpec { - agent_version: None, - config: None, + agent_version: d.produce::>()?.map(K8sName::take), + // `fabricBFD` fans out over every underlay BGP neighbor in the converter, so it is + // worth generating rather than pinning to `None`. + config: d + .produce::>()? + .map(|fabric_bfd| GatewayAgentConfig { + fabric_bfd: Some(fabric_bfd), + }), groups: Some(groups), communities: Some(communities), gateway: Some(d.produce::>()?.take()), diff --git a/k8s-intf/src/bolero/support.rs b/k8s-intf/src/bolero/support.rs index b1880a381..35b6ddac0 100644 --- a/k8s-intf/src/bolero/support.rs +++ b/k8s-intf/src/bolero/support.rs @@ -4,7 +4,7 @@ use std::net::{Ipv4Addr, Ipv6Addr}; use std::ops::Bound; -use bolero::{Driver, ValueGenerator}; +use bolero::{Driver, TypeGenerator, ValueGenerator}; fn v4cdir_from_bytes(addr_bytes: u32, mask: u8) -> String { let and_mask = u32::MAX.unbounded_shl(32 - u32::from(mask)); @@ -254,6 +254,138 @@ pub fn choose(d: &mut D, choices: &[T]) -> Option { Some(choices[index].clone()) } +/// Characters legal anywhere in an RFC 1123 DNS label. +const NAME_ALNUM: &[u8] = b"abcdefghijklmnopqrstuvwxyz0123456789"; +/// Characters legal in the interior of an RFC 1123 DNS label. +const NAME_INNER: &[u8] = b"abcdefghijklmnopqrstuvwxyz0123456789-"; + +/// Maximum length of an RFC 1123 DNS label. +const NAME_MAX_LEN: usize = 63; + +/// A syntactically legal k8s object name. +/// +/// k8s object names are RFC 1123 DNS labels: lowercase alphanumerics plus `-`, starting and +/// ending with an alphanumeric, at most 63 characters. `bolero`'s [`String`] generator produces +/// arbitrary Unicode, which is *not* a legal name, so any CRD field naming a k8s object must draw +/// from here instead — a `TypeGenerator` must never produce an illegal value (see +/// `development/code/property-testing.md`). +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct K8sName(String); + +impl K8sName { + #[must_use] + pub fn take(self) -> String { + self.0 + } + + #[must_use] + pub fn as_str(&self) -> &str { + self.0.as_str() + } +} + +impl AsRef for K8sName { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} + +impl std::fmt::Display for K8sName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +/// Covers every legal RFC 1123 DNS label. +impl TypeGenerator for K8sName { + fn generate(d: &mut D) -> Option { + K8sNameGenerator::new(1, NAME_MAX_LEN).generate(d) + } +} + +/// A [`K8sName`] restricted to a length range. +/// +/// Prefer this over [`K8sName`]'s [`TypeGenerator`] where 63-character names would only bloat the +/// generated config without exercising anything. +pub struct K8sNameGenerator { + min_len: usize, + max_len: usize, +} + +impl K8sNameGenerator { + /// # Panics + /// + /// Panics if the requested length range is not a sub-range of `1..=63`. + #[must_use] + pub fn new(min_len: usize, max_len: usize) -> Self { + assert!( + min_len >= 1 && min_len <= max_len && max_len <= NAME_MAX_LEN, + "illegal k8s name length range {min_len}..={max_len}" + ); + Self { min_len, max_len } + } +} + +impl ValueGenerator for K8sNameGenerator { + type Output = K8sName; + + fn generate(&self, d: &mut D) -> Option { + let len = d.gen_usize( + Bound::Included(&self.min_len), + Bound::Included(&self.max_len), + )?; + let mut name = String::with_capacity(len); + for position in 0..len { + // Only the interior may hold a `-`; the first and last character must be alphanumeric. + let alphabet = if position == 0 || position == len - 1 { + NAME_ALNUM + } else { + NAME_INNER + }; + let index = d.gen_usize(Bound::Included(&0), Bound::Excluded(&alphabet.len()))?; + name.push(alphabet[index] as char); + } + Some(K8sName(name)) + } +} + +/// Generate the textual form of a legal port range: `""` or `"-"`. +/// +/// Both forms parse to an `lpm::prefix::PortRange`, which requires only `start <= end`. +pub fn generate_port_range(d: &mut D) -> Option { + let a = d.produce::()?; + let b = d.produce::()?; + if a == b && d.gen_bool(None)? { + // Cover the single-port spelling, which takes a distinct branch in `PortRange::from_str`. + return Some(format!("{a}")); + } + Some(format!("{}-{}", a.min(b), a.max(b))) +} + +/// Generate a legal `ports` list entry: one or more comma-separated port ranges. +/// +/// The converters split these on `,` before parsing each element, and reject a present-but-empty +/// list, so this never yields an empty string. +pub fn generate_port_list(d: &mut D) -> Option { + let count = d.gen_usize(Bound::Included(&1), Bound::Included(&4))?; + let mut ranges = Vec::with_capacity(count); + for _ in 0..count { + ranges.push(generate_port_range(d)?); + } + Some(ranges.join(",")) +} + +/// [`generate_port_list`] as a standalone [`ValueGenerator`]. +pub struct PortListGenerator; + +impl ValueGenerator for PortListGenerator { + type Output = String; + + fn generate(&self, d: &mut D) -> Option { + generate_port_list(d) + } +} + pub fn generate_v4_prefixes(d: &mut D, count: u16) -> Option> { let cidr4_gen = UniqueV4CidrGenerator::new(count, d.gen_u8(Bound::Included(&0), Bound::Included(&32))?); @@ -266,6 +398,280 @@ pub fn generate_v6_prefixes(d: &mut D, count: u16) -> Option bool { + let len = u32::from(a_len.min(b_len)); + let mask = u32::MAX.unbounded_shl(32 - len); + (a_base & mask) == (b_base & mask) +} + +fn v6_overlaps(a_base: u128, a_len: u8, b_base: u128, b_len: u8) -> bool { + let len = u32::from(a_len.min(b_len)); + let mask = u128::MAX.unbounded_shl(128 - len); + (a_base & mask) == (b_base & mask) +} + +/// Whether a prefix overlaps any reserved block. Exposed for this module's own tests only; the +/// generators go through the skip-ahead helpers below. +#[cfg(test)] +pub(crate) fn test_only_v4_is_reserved(base: u32, len: u8) -> bool { + RESERVED_V4 + .iter() + .any(|&(r_base, r_len)| v4_overlaps(base, len, r_base, r_len)) +} + +#[cfg(test)] +pub(crate) fn test_only_v6_is_reserved(base: u128, len: u8) -> bool { + RESERVED_V6 + .iter() + .any(|&(r_base, r_len)| v6_overlaps(base, len, r_base, r_len)) +} + +/// The first prefix base at or after `base` that is not reserved. +/// +/// Stepping one prefix at a time would be unusable: `240.0.0.0/4` alone holds 2^28 `/32`s. So on +/// a hit, jump past the whole offending block instead, aligned back down to a prefix boundary. +fn v4_skip_reserved(mut base: u32, len: u8) -> u32 { + let align = u32::MAX.unbounded_shl(u32::from(32 - len)); + let step = 1_u32.unbounded_shl(u32::from(32 - len)); + // At most one pass per reserved block: each jump lands strictly past the block it skipped, and + // the only way to revisit lower addresses is the single wrap at the top of the space. + for _ in 0..=RESERVED_V4.len() { + let Some(&(r_base, r_len)) = RESERVED_V4 + .iter() + .find(|&&(r_base, r_len)| v4_overlaps(base, len, r_base, r_len)) + else { + return base; + }; + // Round *up* to the next prefix boundary past the reserved block. Rounding down would + // land back inside the block whenever the block is narrower than the prefix being + // generated -- a `/8` containing `169.254.0.0/16`, for instance. + let block_end = r_base | !u32::MAX.unbounded_shl(u32::from(32 - r_len)); + base = block_end.wrapping_add(step) & align; + } + // Falling out of the loop would mean returning a reserved base, which surfaces downstream as a + // validation failure a long way from its cause. Say so here instead. + unreachable!( + "v4 reserved-block skip did not converge at /{len}; the loop bound assumes every jump \ + lands strictly past the block it skipped, so a new entry in RESERVED_V4 has broken that" + ) +} + +fn v6_skip_reserved(mut base: u128, len: u8) -> u128 { + let align = u128::MAX.unbounded_shl(u32::from(128 - len)); + let step = 1_u128.unbounded_shl(u32::from(128 - len)); + for _ in 0..=RESERVED_V6.len() { + let Some(&(r_base, r_len)) = RESERVED_V6 + .iter() + .find(|&&(r_base, r_len)| v6_overlaps(base, len, r_base, r_len)) + else { + return base; + }; + let block_end = r_base | !u128::MAX.unbounded_shl(u32::from(128 - r_len)); + base = block_end.wrapping_add(step) & align; + } + // See [`v4_skip_reserved`]. + unreachable!( + "v6 reserved-block skip did not converge at /{len}; the loop bound assumes every jump \ + lands strictly past the block it skipped, so a new entry in RESERVED_V6 has broken that" + ) +} + +/// Generate `count` distinct IPv4 prefixes, none overlapping a reserved block. +/// +/// Use this wherever a prefix ends up in a `VpcExpose`; [`UniqueV4CidrGenerator`] is fine for +/// contexts with no such restriction (ACL matches, for instance). +pub struct RoutableV4CidrGenerator { + count: u16, + mask: u8, +} + +impl RoutableV4CidrGenerator { + /// # Panics + /// + /// Panics unless `mask` is in `MIN_ROUTABLE_MASK..=32`. + #[must_use] + pub fn new(count: u16, mask: u8) -> Self { + assert!( + (MIN_ROUTABLE_MASK..=32).contains(&mask), + "illegal v4 prefix length /{mask}" + ); + Self { count, mask } + } +} + +impl ValueGenerator for RoutableV4CidrGenerator { + type Output = Vec; + + fn generate(&self, d: &mut D) -> Option { + if self.count == 0 { + return Some(Vec::new()); + } + let step = 1_u32.unbounded_shl(u32::from(32 - self.mask)); + let seed = d.gen_u32(Bound::Included(&0), Bound::Included(&u32::MAX))?; + // Align the seed down to a prefix boundary so the walk stays aligned. + let mut base = seed & u32::MAX.unbounded_shl(u32::from(32 - self.mask)); + + let mut cidrs = Vec::with_capacity(usize::from(self.count)); + for _ in 0..self.count { + base = v4_skip_reserved(base, self.mask); + cidrs.push(format!("{}/{}", Ipv4Addr::from(base), self.mask)); + base = base.wrapping_add(step); + } + Some(cidrs) + } +} + +/// Generate `count` distinct IPv6 prefixes, none overlapping a reserved block. +pub struct RoutableV6CidrGenerator { + count: u16, + mask: u8, +} + +impl RoutableV6CidrGenerator { + /// # Panics + /// + /// Panics unless `mask` is in `MIN_ROUTABLE_MASK..=128`. + #[must_use] + pub fn new(count: u16, mask: u8) -> Self { + assert!( + (MIN_ROUTABLE_MASK..=128).contains(&mask), + "illegal v6 prefix length /{mask}" + ); + Self { count, mask } + } +} + +impl ValueGenerator for RoutableV6CidrGenerator { + type Output = Vec; + + fn generate(&self, d: &mut D) -> Option { + if self.count == 0 { + return Some(Vec::new()); + } + let step = 1_u128.unbounded_shl(u32::from(128 - self.mask)); + let seed = d.gen_u128(Bound::Included(&0), Bound::Included(&u128::MAX))?; + let mut base = seed & u128::MAX.unbounded_shl(u32::from(128 - self.mask)); + + let mut cidrs = Vec::with_capacity(usize::from(self.count)); + for _ in 0..self.count { + base = v6_skip_reserved(base, self.mask); + cidrs.push(format!("{}/{}", Ipv6Addr::from(base), self.mask)); + base = base.wrapping_add(step); + } + Some(cidrs) + } +} + +/// Hands out mutually non-overlapping, non-reserved prefixes. +/// +/// `VpcManifest::validate` rejects a manifest whose exposes overlap each other, so prefixes cannot +/// be drawn independently per expose -- something has to own the address space for the whole +/// config. Every prefix a pool returns has the same length as its siblings and a distinct base, +/// which makes non-overlap immediate rather than something to check. +/// +/// Uses interior mutability so it can be shared by `&` through the generator tree, since +/// [`ValueGenerator::generate`] takes `&self`. +pub struct PrefixPool { + v4_next: std::cell::Cell, + v6_next: std::cell::Cell, +} + +impl PrefixPool { + /// Block sizes handed out. Small enough that a config can hold plenty, large enough to be + /// realistic tenant blocks. + const V4_MASK: u8 = 24; + const V6_MASK: u8 = 64; + + /// Seed a pool at a driver-chosen point in the address space. + pub fn new(d: &mut D) -> Option { + let v4_seed = d.gen_u32(Bound::Included(&0), Bound::Included(&u32::MAX))?; + let v6_seed = d.gen_u128(Bound::Included(&0), Bound::Included(&u128::MAX))?; + Some(Self { + v4_next: std::cell::Cell::new( + v4_seed & u32::MAX.unbounded_shl(u32::from(32 - Self::V4_MASK)), + ), + v6_next: std::cell::Cell::new( + v6_seed & u128::MAX.unbounded_shl(u32::from(128 - Self::V6_MASK)), + ), + }) + } + + /// The next unused IPv4 prefix. + pub fn next_v4(&self) -> String { + let base = v4_skip_reserved(self.v4_next.get(), Self::V4_MASK); + let step = 1_u32.unbounded_shl(u32::from(32 - Self::V4_MASK)); + self.v4_next.set(base.wrapping_add(step)); + format!("{}/{}", Ipv4Addr::from(base), Self::V4_MASK) + } + + /// The next unused IPv6 prefix. + pub fn next_v6(&self) -> String { + let base = v6_skip_reserved(self.v6_next.get(), Self::V6_MASK); + let step = 1_u128.unbounded_shl(u32::from(128 - Self::V6_MASK)); + self.v6_next.set(base.wrapping_add(step)); + format!("{}/{}", Ipv6Addr::from(base), Self::V6_MASK) + } + + /// `count` unused prefixes, all of one family. + pub fn take(&self, count: u16, v4: bool) -> Vec { + (0..count) + .map(|_| if v4 { self.next_v4() } else { self.next_v6() }) + .collect() + } +} + +/// [`generate_prefixes`], restricted to prefixes legal inside a `VpcExpose`. +pub fn generate_routable_prefixes( + d: &mut D, + v4_count: u16, + v6_count: u16, +) -> Option> { + let mut prefixes = Vec::with_capacity(usize::from(v4_count) + usize::from(v6_count)); + if v4_count > 0 { + let mask = d.gen_u8(Bound::Included(&MIN_ROUTABLE_MASK), Bound::Included(&32))?; + prefixes.extend(RoutableV4CidrGenerator::new(v4_count, mask).generate(d)?); + } + if v6_count > 0 { + let mask = d.gen_u8(Bound::Included(&MIN_ROUTABLE_MASK), Bound::Included(&128))?; + prefixes.extend(RoutableV6CidrGenerator::new(v6_count, mask).generate(d)?); + } + Some(prefixes) +} + pub fn generate_prefixes( d: &mut D, v4_count: u16, @@ -294,6 +700,99 @@ mod test { _ => 1000, }; + /// A generated name must be a legal RFC 1123 DNS label, which is the whole reason this + /// generator exists in place of `produce::()`. + #[test] + fn test_k8s_name_generator_is_rfc1123() { + bolero::check!() + .with_type::() + .for_each(|name| { + let name = name.as_str(); + assert!(!name.is_empty(), "empty name"); + assert!(name.len() <= 63, "name too long: {name}"); + assert!( + name.bytes() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == b'-'), + "illegal character in {name}" + ); + let first = name.bytes().next().unwrap(); + let last = name.bytes().next_back().unwrap(); + assert!(first.is_ascii_alphanumeric(), "leading '-' in {name}"); + assert!(last.is_ascii_alphanumeric(), "trailing '-' in {name}"); + }); + } + + /// Both spellings a port list can take must parse, and the range spelling must be ordered. + #[test] + fn test_generated_port_lists_parse() { + bolero::check!() + .with_generator(crate::bolero::support::PortListGenerator) + .for_each(|ports| { + for range in ports.split(',') { + let range = range + .parse::() + .unwrap_or_else(|e| panic!("failed to parse port range {range}: {e}")); + assert!(range.start() <= range.end()); + } + }); + } + + /// Routable prefixes must be distinct and must avoid every reserved block. + /// + /// `config`'s validation is the real authority here; this test exists so that a regression in + /// the skip-ahead logic fails locally and quickly rather than as a mystifying failure (or, as + /// happened once, a hang) inside a downstream config property test. + #[test] + fn test_routable_v4_prefixes_avoid_reserved() { + for mask in 8..=32 { + let generator = crate::bolero::support::RoutableV4CidrGenerator::new(8, mask); + bolero::check!() + .with_generator(generator) + .with_iterations(ITERATIONS) + .for_each(|cidrs| { + let mut seen = std::collections::HashSet::new(); + assert_eq!(cidrs.len(), 8); + for cidr in cidrs { + assert!(seen.insert(cidr), "duplicate prefix {cidr}"); + let (addr, len) = cidr.split_once('/').unwrap(); + let addr = addr.parse::().unwrap(); + let len = len.parse::().unwrap(); + assert_eq!(len, mask); + assert!( + !crate::bolero::support::test_only_v4_is_reserved(addr.to_bits(), len), + "generated reserved prefix {cidr}" + ); + } + }); + } + } + + #[test] + #[cfg_attr(miri, ignore = "just too slow on miri")] + fn test_routable_v6_prefixes_avoid_reserved() { + for mask in [8, 16, 32, 48, 64, 96, 127, 128] { + let generator = crate::bolero::support::RoutableV6CidrGenerator::new(8, mask); + bolero::check!() + .with_generator(generator) + .with_iterations(ITERATIONS) + .for_each(|cidrs| { + let mut seen = std::collections::HashSet::new(); + assert_eq!(cidrs.len(), 8); + for cidr in cidrs { + assert!(seen.insert(cidr), "duplicate prefix {cidr}"); + let (addr, len) = cidr.split_once('/').unwrap(); + let addr = addr.parse::().unwrap(); + let len = len.parse::().unwrap(); + assert_eq!(len, mask); + assert!( + !crate::bolero::support::test_only_v6_is_reserved(addr.to_bits(), len), + "generated reserved prefix {cidr}" + ); + } + }); + } + } + #[test] fn test_unique_v4_cidr_generator() { for mask in 0..=32 {