Repository navigation
host: delete the sandbox accessors that only tests use (SL6.6c) - #1538
Merged
Pedro Henrique Penna (ppenna) merged 1 commit intoNov 12, 2026
Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
Pedro Henrique Penna (ppenna) merged 1 commit into
Conversation
Contributor
This pull request changes existing testsIt modifies or removes 25 existing test asset(s), listed below. Review each with care: a change to a test can hide a regression that the test caught. See doc/contribute.md#changes-to-existing-tests. Test code
Relocated unchanged: 0; added: 2. Compared the merge base |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues block the sandbox helper cleanup.
No open issues 🎉
What changed in this PR
This PR removes sandbox helpers used only by tests, preparing host::sandbox to become private to the CLI in SL6.7b as part of the NVX library-slimming workstream. Retained runtime behavior is unchanged.
Changes:
- Deletes mount setting parsers, unused accessors, path-conversion wrappers, and
SandboxOptions::new. - Adapts tests to retained methods, whole-value comparisons, and test-local defaults; retires assertions for deleted parsers.
- Updates documentation and removes 21 entries from each platform’s public API snapshot.
| File | Description |
|---|---|
nvx-rs/src/host/sandbox/tests/specification.rs |
Calls retained path-conversion methods directly. |
nvx-rs/src/host/sandbox/tests.rs |
Removes the deleted kind parser’s rejection assertion. |
nvx-rs/src/host/sandbox/options/tests.rs |
Adds test-local defaults and compares complete mounts. |
nvx-rs/src/host/sandbox/options.rs |
Deletes SandboxOptions::new. |
nvx-rs/src/host/sandbox/mount/tests.rs |
Reworks assertions and calls without deleted helpers. |
nvx-rs/src/host/sandbox/mount/request.rs |
Removes unused accessors and wrappers; preserves implementation docs. |
nvx-rs/src/host/sandbox/mount/mod.rs |
Deletes name parsers, their shared helper, and unused imports. |
nvx-rs/src/host/sandbox/mount/aggregate.rs |
Replaces a deleted-method documentation link with explicit error conditions. |
nvx-rs/public-api/windows.txt |
Removes deleted methods from the Windows API snapshot. |
nvx-rs/public-api/linux.txt |
Removes deleted methods from the Linux API snapshot. |
Review effort: Balanced
Pedro Henrique Penna (ppenna)
force-pushed
the
agents/slim-sl6-4-staging-private
branch
from
November 12, 2026 14:10
508170f to
3276e77
Compare
Base automatically changed from
agents/slim-sl6-4-staging-private
to
feature/nvx-refactor
November 12, 2026 14:33
Pedro Henrique Penna (ppenna)
force-pushed
the
agents/slim-sl6-6c-sandbox-test-only
branch
from
November 12, 2026 14:35
eed421f to
e757a18
Compare
This was referenced Nov 12, 2026
SL6.7b (#1124) makes host::sandbox private to the CLI. Twelve of its items have no caller in any NVX program, only in the public API and in the module's own tests, so they would be dead once private. This change deletes them first, as SL6.6c (#1122) records: - MountAccess::from_name, MountOwner::from_name and MountKind::from_name, with their helper setting_from_name, which read a share's settings by name, as a recorded share gave them. The CLI maps the choices of --mount-owner itself and records a sandbox's export plan, not its shares' settings. - SandboxMount's accessors guest_target, access, policy, owner and kind, and absolute and absolute_policy_paths, which called absolute_with and absolute_policy_paths_with with the current directory. plan_mounts and the checks of host paths call those variants, whose docs now say what the wrappers' said, and read the fields; host_path stays for sandbox_mounts. plan_mounts' docs no longer name absolute, and say how making a host path absolute fails. - SandboxOptions::new, the options of an operation with the defaults of nvx-rs sandbox, which the CLI builds field by field instead. The tests keep what they check of the code that stays: - The share tests compare whole shares with the shares that SandboxMount::new makes, or read the fields that the mount module shows them, and make shares absolute with absolute_with and absolute_policy_paths_with and the current directory, as the deleted wrappers did. - The option tests build their options with a helper of their own, defaults, which holds what SandboxOptions::new did, and compare the shares of sandbox_mounts with whole shares, policies included. - The checks of from_name's refusals go with it: they were the only callers of what they checked. The public API snapshots lose 21 lines on each platform, the eleven public methods at both of their paths and SandboxOptions::new, and gain none. Part of #1122 (SL6.6c). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pedro Henrique Penna (ppenna)
force-pushed
the
agents/slim-sl6-6c-sandbox-test-only
branch
from
November 12, 2026 20:09
e757a18 to
8abb8f7
Compare
Pedro Henrique Penna (ppenna)
marked this pull request as ready for review
November 12, 2026 20:10
Pedro Henrique Penna (ppenna)
merged commit Nov 12, 2026
27c6134
into
feature/nvx-refactor
85 checks passed
Pedro Henrique Penna (ppenna)
deleted the
agents/slim-sl6-6c-sandbox-test-only
branch
November 12, 2026 20:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Leaving code only. This PR changes only code that this effort removes from NVX: twelve items of
host::sandboxthat no NVX program calls,MountAccess::from_name,MountOwner::from_nameandMountKind::from_namewith their helpersetting_from_name,SandboxMount::{guest_target, access, policy, owner, kind, absolute, absolute_policy_paths}, andSandboxOptions::new, with the edits that remove references to them. Nothing that stays changes, and no later PR uses what it changes, so the size limits for kept code don't apply.Part of #1122 (SL6.6c, "the sandbox accessors that only tests use are gone", recorded on #1122). Parent #1087, umbrella #1076.
It needs SL6.4 (#1533), which has merged; it starts from
55f44aa4607, the tip after SL4.4d (#1545). SL6.7b (#1124) stacks on this one and makeshost::sandboxandhost::network::policyprivate to the CLI, which would leave these twelve items dead.What and why
SL6.7b makes
host::sandboxcrate-private. Twelve of its items have no caller in any NVX program, only in the public API and in the module's own tests, so the warning-denied builds of every feature set would find them dead once private, on Windows, Linux and the macOS check. As SL6.6b did for SL6.4, this PR deletes them first, so that SL6.7b stays visibility-only. One commit,host: delete the sandbox accessors that only tests use(eed421f9):MountAccess::from_name,MountOwner::from_name,MountKind::from_nameandsetting_from_name, which read a share's settings by name, "as a recorded share gives it". The CLI maps the choices of--mount-owneritself, and records a sandbox's export plan, not its shares' settings. The imports that only they used,super::refusedandcrate::error::Resultinmount/mod.rs, go with them.SandboxMount::{guest_target, access, policy, owner, kind}, which nothing outside the tests read:plan_mounts,validate_mountsand the checks of host paths read the fields within the mount module, andhost_path, whichoptions::sandbox_mountscalls, stays.SandboxMount::{absolute, absolute_policy_paths}called the crate-privateabsolute_withandabsolute_policy_paths_withwithpaths::current_dir;plan_mountsand the checks of host paths call those variants, whose docs now say what the wrappers' said.SandboxOptions::new, the options of an operation with the defaults ofnvx-rs sandbox. The CLI builds itsSandboxOptionsfield by field from what it parsed.plan_mounts' docs no longer link to the deletedSandboxMount::absolute, and say instead how making a host path absolute fails,Iofor an unreadable current directory or a drive-relative path that Windows cannot qualify.The public API snapshots lose 21 lines on each platform: the eleven public methods at both of their paths,
nvx::host::sandbox::andnvx::host::sandbox::mount::, andnvx::host::sandbox::options::SandboxOptions::new. Nothing is added.README.md,doc/and the case matrix name none of the items.Size
Measured against
55f44aa4607, the tip after SL4.4d (#1545), by #1076's rules: leaving code only, no limit. The measuring script counts 0 added or changed code lines: production code only loses the items and their two imports (105 non-blank lines). Counted files: 3,src/host/sandbox/mount/{mod, request}.rsandsrc/host/sandbox/options.rs;src/host/sandbox/mount/aggregate.rschanges only its docs. 10 files change: +168 and −230 lines, tests and snapshots included.Old tests of the area
from_name's refusals,"unsupported sandbox mount owner \"root\"; choose vmm or caller"and"unsupported sandbox mount mode \"rx\"; choose ro or rw"inhost::sandbox::mount::tests::mount_owner_defaults_to_vmm_and_forwards_caller, and"unsupported sandbox mount kind \"socket\"; choose directory or file"inhost::sandbox::tests::file_shares_take_no_policy_paths_or_reserved_targets. The rest of both tests stays.host::sandbox, the CLI's tests, and the integration and doc tests.Test changes
python scripts/check_test_changes.py --base 55f44aa4607lists 25 changes to existing tests and 2 added helpers, hencetests-modified:nvx-rs/src/host/sandbox/mount/tests.rs(6), with a new helpershare, the directory share thatparsereturns for a value with its target, host path, access and policy:mount_parser_defaults_to_read_only_and_accepts_rwcompares the parsed shares with whole shares, so it still checks the target, the read-only and read-write access and the policy, and now also the owner and kind thatparsetook, instead of reading the accessors.mount_parser_accepts_host_paths_with_commasandmount_owner_defaults_to_vmm_and_forwards_callerread theaccessandownerfields, which the mount module's tests see; the second also loses the twofrom_nameassertions (Retired, above).mount_access_policy_is_validated_and_forwardedcompares the share thatabsolute_with(&paths::current_dir)returns with the whole share, its absolute host path and its whole policy, instead of reading its allowed and writable paths; its refusals don't change.mount_absolute_path_keeps_symbolic_link_componentsandmount_absolute_path_keeps_a_link_on_the_hostcallabsolute_with(&paths::current_dir), whatabsolutecalled.nvx-rs/src/host/sandbox/options/tests.rs(15), with a new helperdefaults, which holds whatSandboxOptions::newdid:launchingand the 9 tests that built options withSandboxOptions::newbuild them withdefaults:sandbox_rejects_systemd_entrypoint,sandbox_managed_operations_reject_dry_run_before_state_access,sandbox_exec_forwards_explicit_empty_environment_and_cwd,sandbox_exec_forwards_environment_inheritance,sandbox_run_and_exec_alone_take_the_execution_options,sandbox_environment_is_validated_before_state_access,managed_non_launch_operations_reject_network_policy_options,sandbox_launches_take_the_scratch_image_that_their_upper_needsandsandbox_command_rejects_misplaced_mount_options;policy_paths_need_a_preceding_mount_only_with_several_shares,policy_paths_attach_to_their_shares,sandbox_command_attributes_policy_paths_to_the_preceding_mountandsandbox_command_attributes_mount_deny_to_the_preceding_mountcompare the shares ofsandbox_mountswith whole shares, their policies included, instead of readingpolicy(),guest_target(),owner()andkind();sandbox_mounts_take_the_kind_of_each_host_pathtells the shared file by its host path instead of its target, and compares both shares, their kinds and thecallerowner included, with whole shares.nvx-rs/src/host/sandbox/tests.rs(1):file_shares_take_no_policy_paths_or_reserved_targets(Retired, above).nvx-rs/src/host/sandbox/tests/specification.rs(3):windows_drive_relative_and_unc_paths,windows_drive_relative_policy_paths_become_absoluteand the proptesthost_paths_become_absolutecallabsolute_withandabsolute_policy_paths_withwithpaths::current_dir, what the deleted wrappers called.doc/acceptance-case-matrix.mdnames none of these tests, so it doesn't change.Validation
local_gates.py --files(fromdev) selectstest-changes,markdown-links,test-layout, thecheck-nvx-rssteps andnvx-cli; I ran them from this branch's action definitions, with Rust 1.95.0, on Windows and onnvx-mshv-test(Azure Linux 3, kernel 6.18, linux-mshv, virtual machine), in a temporary worktree. Nothing here changes what a VM shows: no NVX program called these items. Before this merge turn, the branch was rebased onto5715bb6d709, after SL7.8c, SL6.14, SL4.4a to SL4.4c, SL6.11b, SL6.11d and SL6.13b, with no conflicts, and the checks below ran on that rebase,a63fbd07b56. At this turn it was rebased onto55f44aa4607, after SL4.4d (#1545), with no conflicts:public_api.py --updateleaves the snapshots as they are, andgit range-diffshows=against the reviewede757a1872ed, so this head,8abb8f7f031, carries the same change. Ona63fbd07b56:cargo fmt --all --check-D warnings:--all-targets --all-features,--lib,--lib --no-default-features,serde,cliandtestingalone--target x86_64-unknown-linux-gnu, and so did--all-targetswith default features--target x86_64-unknown-linux-musl --no-default-features --features guest -D warningscheck-guest-binaries)cargo check --all-targets --all-features --target aarch64-apple-darwinhost/snapshot/tests.rs, is the base'scargo test --locked --all-featurescli114 passed, 2 ignored) and the doc testscli124 passed) and the doc testscargo test --locked,cargo build --locked --lib --no-default-features,cargo test --locked --lib --no-default-featuresRUSTDOCFLAGS=-D warnings cargo doc --locked --no-deps --all-featurescargo +1.89 check --locked --all-targets --all-featurespython scripts/public_api.pycheck-guest-binaries,build-guest-binaries,test-guest-binariescheck_test_layout.py,check_markdown_links.py,check_matrix.py <worktree> 99d2022085fA code-review subagent reviewed the whole diff and found no issue. The
validate-nvxunittest list is unaffected: this PR changes no script.Left to CI: the microVM scenarios, as for any change, and
openvmm-ci, for the unchanged pin.