From 3a9c190461e1cb829480d39e09db1d25ad129cb6 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Wed, 21 Oct 2026 05:52:17 -0300 Subject: [PATCH] nvx-rs: parse legacy egress rules with the MXC form's helpers parse_legacy_rule, which parses the legacy flat rules of an egress policy file, repeated two steps that the helpers of the MXC form already take. Like parse_peer, it required the cidr field with a copy of the "cidr is required" check before it passed cidr and except to parse_peer_fields. And it wrote out the selector of a rule without a port, which lowering::port_selectors builds for an MXC port selector, calling port_protocol_selectors, the rest of port_selectors, only for a rule with a port. parse_peer_fields now reads the required cidr and the optional except fields itself, so parse_peer and parse_legacy_rule each call it once. parse_legacy_rule passes its port fields to port_selectors, as parse_mxc_port and the lowering of openvmm_arguments do, and port_protocol_selectors, which had no other caller, folds into port_selectors. Each rule form now has one path from its fields to its destinations and selectors, and the lowering module has one function fewer. Behavior is unchanged: the checks run in the same order, with the same messages. A legacy rule is still checked for unknown fields, then for cidr, its network and exceptions, endPort without port, port without protocol, and its protocol. port_selectors refuses endPort without port too, but parse_legacy_rule refuses it before port_selectors can see it, so a legacy rule without a port still gets the one selector of its protocol, and a rule with a port the ICMP check and port range of port_protocol_selectors. Reading the port fields before the call only converts their nodes, which cannot fail. The policy tests cover both forms, among them the 784 cases of tests/fixtures/egress-policy.json, the Python compiler's recorded outcomes, whose legacy edge cases include endPort without port, port without protocol, and ICMP with a port, an endPort, or neither. The network tests cover openvmm_arguments, and the egress scenarios of test-microvm compile legacy and MXC rules with nvx-rs egress compile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/host/network/lowering.rs | 15 +++-------- nvx-rs/src/host/network/policy/mod.rs | 37 ++++++++++----------------- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/nvx-rs/src/host/network/lowering.rs b/nvx-rs/src/host/network/lowering.rs index 78b5dffc9..a9b8118f1 100644 --- a/nvx-rs/src/host/network/lowering.rs +++ b/nvx-rs/src/host/network/lowering.rs @@ -232,8 +232,9 @@ pub(crate) fn port_range( Ok((start, end)) } -/// Returns the selectors of an MXC port selector `description` whose protocol is valid, as the -/// rest of `_parse_mxc_port` does. +/// Returns the selectors of the port fields, `port` and the optional `endPort`, of +/// `description`, an MXC port selector or a legacy flat rule whose protocol is valid, as the rest +/// of `_parse_mxc_port` does. ICMP takes no port. pub(crate) fn port_selectors( protocol: RuleProtocol, port: Option, @@ -246,16 +247,6 @@ pub(crate) fn port_selectors( } return Ok(vec![(protocol.native(), None)]); }; - port_protocol_selectors(protocol, port, end_port, description) -} - -/// Returns the selectors of a protocol with ports, which ICMP does not have. -pub(crate) fn port_protocol_selectors( - protocol: RuleProtocol, - port: PortValue, - end_port: Option, - description: &str, -) -> Result> { if protocol == RuleProtocol::Icmp { return Err(invalid(format!( "{description}.port is not supported with icmp" diff --git a/nvx-rs/src/host/network/policy/mod.rs b/nvx-rs/src/host/network/policy/mod.rs index 1f510c361..28734bdf7 100644 --- a/nvx-rs/src/host/network/policy/mod.rs +++ b/nvx-rs/src/host/network/policy/mod.rs @@ -348,16 +348,17 @@ fn parse_rule(document: &Document, id: NodeId, description: &str) -> Result, - cidr: NodeId, - except: Option, description: &str, ) -> Result<(Family, Vec<(u128, u128)>)> { + let Some(cidr) = object.get("cidr") else { + return Err(invalid(format!("{description}.cidr is required"))); + }; let parent = parse_network(object.node(cidr), &format!("{description}.cidr"))?; - let exceptions = match except { + let exceptions = match object.get("except") { Some(except) => array(object.document, except, &format!("{description}.except"))?, None => &[], }; @@ -389,20 +390,13 @@ fn parse_peer( ) -> Result<(Family, Vec<(u128, u128)>)> { let peer = object(document, id, description)?; peer.check_fields(PEER_FIELDS, description)?; - let Some(cidr) = peer.get("cidr") else { - return Err(invalid(format!("{description}.cidr is required"))); - }; - parse_peer_fields(&peer, cidr, peer.get("except"), description) + parse_peer_fields(&peer, description) } /// Parses a legacy flat rule, as `_parse_legacy_rule` does. fn parse_legacy_rule(rule: &Object<'_>, description: &str) -> Result> { rule.check_fields(LEGACY_RULE_FIELDS, description)?; - let Some(cidr) = rule.get("cidr") else { - return Err(invalid(format!("{description}.cidr is required"))); - }; - let (family, addresses) = parse_peer_fields(rule, cidr, rule.get("except"), description)?; - let destinations = Some(vec![(family, addresses)]); + let destinations = Some(vec![parse_peer_fields(rule, description)?]); let port = rule.get("port"); let end_port = rule.get("endPort"); if end_port.is_some() && port.is_none() { @@ -415,15 +409,12 @@ fn parse_legacy_rule(rule: &Object<'_>, description: &str) -> Result> return Ok(lowering::combine(destinations, None)); }; let protocol = self::protocol(rule.document, protocol, description)?; - let selectors: Vec = match port { - None => vec![(protocol.native(), None)], - Some(port) => lowering::port_protocol_selectors( - protocol, - port_value(rule.node(port)), - end_port.map(|end_port| port_value(rule.node(end_port))), - description, - )?, - }; + let selectors = lowering::port_selectors( + protocol, + port.map(|port| port_value(rule.node(port))), + end_port.map(|end_port| port_value(rule.node(end_port))), + description, + )?; Ok(lowering::combine(destinations, Some(selectors))) }