Skip to content

nvx-rs: parse legacy egress rules with the MXC form's helpers - #682

Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
nvx-rs-quality/legacy-egress-rules
Oct 21, 2026
Merged

Pedro Henrique Penna (ppenna) merged 1 commit into
devfrom
nvx-rs-quality/legacy-egress-rules

Conversation

@ppenna

Copy link
Copy Markdown
Contributor

Summary

parse_legacy_rule in nvx-rs/src/host/network/policy/mod.rs parses the legacy flat rules of an egress policy file, {"cidr": CIDR, "except": [...], "protocol": PROTOCOL, "port": FIRST, "endPort": LAST}. It repeated two steps that the helpers of the MXC form already take:

  • Like parse_peer, it looked up the required cidr field with its own copy of the {description}.cidr is required check, and then passed cidr and except to parse_peer_fields.
  • It wrote out the selector of a rule without a port, vec![(protocol.native(), None)], which lowering::port_selectors builds for an MXC port selector, and called port_protocol_selectors, the rest of port_selectors, only for a rule with a port.

Both rule forms now go through the same helpers, so each has one path from its fields to its destinations and selectors, and the lowering module has one function fewer. Earlier runs listed this as a follow-up in #635, #636, #637, and #678.

Changes

  • nvx-rs/src/host/network/policy/mod.rs:
    • parse_peer_fields(object, description) reads the required cidr and the optional except fields itself, failing with the same {description}.cidr is required; it took them as arguments. The two sites that now share that lookup are parse_peer, for each destination of an MXC rule, and parse_legacy_rule, for a legacy rule.
    • parse_legacy_rule passes its port and endPort fields to lowering::port_selectors.
  • nvx-rs/src/host/network/lowering.rs: port_protocol_selectors, whose only other caller was port_selectors, folds into it. Its doc comment now names both rule forms and keeps the old function's note that ICMP takes no port.
  • The sites that now share port_selectors: parse_mxc_port (an MXC rule's port selectors), parse_legacy_rule (a legacy rule's port fields), and parse_rule in host/network/mod.rs (the EgressRules that openvmm_arguments lowers).

2 files, 17 insertions and 35 deletions.

Behavior

Unchanged: every policy compiles to the same native rules, or fails at the same check with the same message and ErrorKind::Unsupported.

  • parse_peer still checks that a destination is an object with known fields before it requires cidr. parse_legacy_rule still checks its fields, then cidr, the network and its exceptions, endPort without port, port without protocol, and the protocol, in that order.
  • port_selectors refuses endPort without port too, but parse_legacy_rule refuses that combination first, before it reads the protocol, so a legacy rule never reaches port_selectors with it. A legacy rule without port still gets the one selector of its protocol, and a rule with port the ICMP check and port range that port_protocol_selectors applied.
  • The port fields are now converted before the call rather than in a match arm, but port_value only converts a JSON node and cannot fail, and without port there is no endPort to convert.

Tests that pin this:

  • The 45 policy tests of host::network::policy::tests, among them every_fixture_case_compiles_as_python_compiled_it, which replays the 784 outcomes of tests/fixtures/egress-policy.json that the Python compiler recorded: its test vectors, a seeded random corpus of 200 policies, and edge cases such as legacy endPort without port, legacy port without protocol, and legacy icmp with a port, an endPort, or neither.
  • The network tests of openvmm_arguments, in host::network::tests, and the golden egress compile scenarios of tests/cli.rs.
  • On every host, the four egress scenarios of test-microvm, which compile legacy and MXC rules with an nvx-rs egress compile built from this tree and run guests under them.

Follow-ups

Not in this pull request, to keep it to one change. I found no suspected bug.

  • host/sandbox/mount.rs: MountAccess, MountOwner, and MountKind repeat as_str, from_name with one message shape, unsupported sandbox mount NOUN ...; choose A or B, and Display, and parse_mount_value repeats MountAccess's match with a message of its own.
  • The golden tests of host/python/golden.rs and host/sandbox/golden.rs open and close 19 sections with Mismatches::new(SECTION), cases(&fixture, SECTION), and finish(); one helper that runs a section would name each section once and make finish impossible to forget.
  • guest/nvxs.rs: GuestConnection::attach, acknowledge, and credit repeat the same check, clear, encode, and write around one session call.
  • protocol/nvxs/guest.rs and host.rs: receive repeats the same nine lines after its state check in both roles.
  • host/python/fs.rs's Windows stat and lstat build the same Stat, and host/python/dumps.rs's write_value repeats its bracket, comma, and indentation loop for lists and dicts.
  • Waiting on files that open pull requests change: write_private in host/instance.rs and in host/store/files.rs share the temporary file's name, write, sync, rename, and cleanup, and differ in how they create the file; nvx-rs: add the VMM supervisor with a spawn allowlist, stop, and reattach #656 and nvx-rs: refuse read-only paths inside read-write directories on every host #672 change instance.rs.

Validation

  • BASE: 916a61f0297900db13d7d9d04d170f01ef9ed71f, origin/dev when the run started and still when this pull request was opened. No open pull request to dev changes the files of this one.

  • TREE: 2f7b0791b5763dabf3db541939d349d8fcc19ff2, the tree of this commit. Every host applied the patch to BASE and checked that git write-tree equals TREE.

  • Release on every host: v0.1.0-dev.dad3e41d7f05, the nearest first-parent ancestor of BASE that had packages, two commits earlier. Its OpenVMM, d0bdbed8f6e3b86271277dc22658109e2e633b2d, is the pinned revision, as each host checked in build/openvmm.provenance.json.

  • Each host replaced the release's nvx-rs with one built from TREE by the Cargo command that nvx.py uses, recorded its provenance with record-nvx-rs-provenance, and checked that the record names TREE's sources (source_sha256 7a8a4abf7d77cb206a00a390261b27dff8df3315e116e04fdab790c06e2d494d) and the executable that it built. Each host built the managed_lifecycle and openvmm_e2e test binaries before its hypervisor tests, whose logs show no compilation.

  • Guest artifacts were rebuilt from TREE on prometheus30, whose user namespaces let Alpine's apk run its scripts (they are denied on prometheus32):

    • build-guest-binaries --output, from guest sources with digest b2e9b0bb21ce7435221e02af32eed4ba188e633bb1e6a7db15bf7f6808d34495;
    • test-guest-binaries --binaries, where 34 tests passed;
    • the Alpine build-initramfs with NVX_GUEST_BINARIES.

    initramfs.cpio.gz (SHA-256 2d12a797f23e00d7363ba9290f7efdb86b0b401227a380f6bc13c8e58f690f4b), its packages manifest, and its provenance went through this machine to the other two hosts, where the SHA-256 digests matched.

  • Local checks with Rust 1.95.0 on Windows: rustfmt; Clippy with -D warnings, with all targets and features and on the library without default features; and the tests with all features, the 45 policy tests among them, and with the default ones. This machine also ran the steps that no host could run, because the hosts lack the toolchain or target: the Windows MSRV (1.89) checks of nvx, aci_edge_sandboxes, and the runtime adapter's protocol-only and host-only configurations, on Windows; and, on WSL Ubuntu, the macOS (aarch64-apple-darwin) checks of nvx and aci_edge_sandboxes and the MSRV check of the runtime adapter's guest-only configuration, which ran as the action defines it, starting with its rustup target add, which gave the local 1.89 the musl target. I removed that target again afterwards.

  • The change adds, moves, and deletes no file and changes no Markdown, so the CLI tests of validate-nvx and the link check had nothing to cover.

  • A harness note: the first run of prometheus32's gates was started in the background with & from a non-interactive shell, which left SIGINT, SIGQUIT, and SIGHUP ignored, so the five Ctrl-C tests of tests/cli.rs failed: the nvx-rs that they interrupt inherited the ignored SIGINT. I reran that job, and prometheus30's guest build, under setsid -f with the default signal dispositions. Every step passed, the five Ctrl-C tests among them, and the rebuilt initramfs was byte-identical. The table reports the reruns.

Host profile Backend Host type check-nvx-rs and check-aci-edge-sandboxes without a hypervisor Guest artifacts test-aci-edge-sandboxes test-microvm, default suite
prometheus32 kvm baremetal Pass: every Linux step, including the MSRV (1.89) checks, 752 nvx tests with all features (the 45 policy tests and the 13 of tests/cli.rs among them) and 362 with the default ones, artifact bundling (15), check-guest-binaries (397 tests), the runtime adapter's protocol-only, host-only, and musl guest-only configurations (13, 14, and 21 tests), 158 and 85 aci_edge_sandboxes tests over the two feature sets, and agent bundling (4). The macOS checks and the guest-only MSRV check ran locally Copied in, digests match Pass (18/18) Pass (41 scenario runs)
prometheus30 mshv baremetal None; prometheus32 and prometheus28 ran the gates Built and tested here (34 tests) Pass (18/18) Pass (41 scenario runs)
prometheus28 whp baremetal Pass: every Windows step, including 429 nvx tests with all features (the 45 policy tests among them) and 358 with the default ones, artifact bundling (15), the runtime adapter's protocol-only and host-only configurations (13 and 14 tests), 158 and 85 aci_edge_sandboxes tests over the two feature sets, and agent bundling (4). The MSRV checks ran locally Copied in, digests match Pass (18/18) Pass (42 scenario runs)

Left to CI: the test-microvm scenarios that run only when named (smp-lapic, managed-exec-config, and sandbox-lifecycle), which need artifacts that packages omit; the Ubuntu and Azure Linux guests; test-openvmm-unit and test-openvmm, since OpenVMM is unchanged; the Docker guest build and verify-guest-determinism; the benchmarks and performance gates; and check-quality and validate-nvx, which no Python, shell, or Markdown change touches.

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>
Copilot AI balanced review requested due to automatic review settings October 21, 2026 08:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The narrow refactor preserves behavior, has existing regression coverage, and has no unresolved findings.

No open issues 🎉

What changed in this PR

Consolidates legacy and MXC egress-policy parsing around shared helpers while preserving validation order, error messages, and native rule output.

Changes:

  • Centralizes required cidr lookup and exception parsing.
  • Routes legacy port fields through port_selectors, removing the redundant helper.
File Description
nvx-rs/​src/​host/​network/​policy/​mod.rs Reuses shared destination and port-selector parsing.
nvx-rs/​src/​host/​network/​lowering.rs Folds port handling into port_selectors and updates its documentation.

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants