Repository navigation
test(oci): add a shared Docker and Podman suite for image USER and WORKDIR - #3931
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
🌿 Preview your docs: https://nvidia-preview-pr-3931.docs.buildwithfern.com/openshell |
|
/ok to test 2fed032 |
|
Label |
|
This is great! Thanks. One expectation I had (related to #3712) would be that at least some of the tests in I don't think that that file should be removed entirely because it also includes tests for other behaviour. |
33f5e2d to
9bf8773
Compare
9bf8773 to
e65a902
Compare
e65a902 to
f184d63
Compare
|
The shared Docker/Podman OCI suite would also be a useful place to pin the image identity contract discussed in #4030. Could we extend the scenarios to cover:
Each case should run on the existing Docker rootful, Podman rootful, and Podman rootless lanes. These assertions can cover the current behavior without waiting for #4030's diagnostic changes. Detailed rejection provenance belongs in resolver tests, and once-only interactive failure output needs a real PTY test alongside the #4030 fix. |
f184d63 to
7658c9f
Compare
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…idationFailed The supervisor exits with the reserved status when the sandbox rejects the image working directory. Docker maps that exit during readiness and monitoring and records it before removing the supervisor container, and Podman maps the supervisor companion's exit instead of the workload's. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Unrelated provisioning failures can no longer pass the rejection scenario. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
The supervisor no longer passes on a workload's own exit status 78, so drivers cannot misreport it as a workspace rejection after a runtime restart. The rejection is recognized once at agent start and carried as a typed error. Docker reports the same fixed condition message as Podman and logs the supervisor output instead, and its monitor removes the supervisor container when a stop overlaps diagnostics collection. The OCI image suite registers images for cleanup only after they build. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…image-suite Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Report a recorded Docker workspace rejection once, where supervisor readiness fails, and match its fixed message exactly. Read the Podman supervisor exit status once per inspection. Choose the supervisor exit status in one function and derive the workspace error type. The OCI image suite shares step helpers, checks that a custom workspace does not also create /sandbox, and its playbook waits for a connected gateway and collects failure diagnostics through the shared role. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
elezar
left a comment
There was a problem hiding this comment.
The shared OCI suite and workspace-rejection reporting look good. Branch Checks and Core E2E pass on d181609, including the Docker rootful DEB, Podman rootful/rootless RPM, and ARM64 Docker OCI lanes.
When updating #4120, we'll also need to move the OCI tests added here from tests/suites/features/oci-image to e2e/suites/features/oci-image, adopt the shared e2e/support/rust tooling, and update the archive/workflow paths. That migration and the previously discussed #4030 identity scenarios are follow-ups and do not block this PR.
…image-suite Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…hecks The oci-image feature suite now checks, on Docker and Podman, that the image user has no capabilities, has no_new_privs set, and cannot read the sandbox control credentials, and that a custom WORKDIR is not a mount. The checks that need container inspection (pinned image ID, network=none, supervisor user and capabilities) move to a small test in the Podman driver suite. Remove the legacy podman_oci_identity e2e test. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Restore the Podman e2e test removed in the previous commit and drop only the checks the shared oci-image suite now runs on Docker and Podman: workload capabilities, no_new_privs, credential readability, and the workspace and /sandbox mount checks. The pinned image ID and the workload/supervisor isolation checks stay, as does the Podman driver suite unchanged. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Carry the Docker control supervisor's failure reason in a typed error instead of matching status text, build the fixed workspace message where the wait result is classified, and drop the fake Docker server test. Inline image cleanup ordering in the suite, read the container engine once, share sandbox naming, and stop granting a literal /sandbox in the image identity policy. Add e2e:oci-image mise tasks and use them from the ARM64 Docker workflow and TESTING.md. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Inspect the containers the driver created through the gateway's engine in the shared suite: the workload runs the inspected image ID as the OCI identity without a network or supervisor credential mounts, and the supervisor is a separate non-root container. Keep only the Podman-specific supervisor identity and capability checks in podman_oci_identity. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
@elezar Agreed. Most of the remaining
|
Read the WorkspaceValidationFailed condition reason from sandbox get --output json instead of matching CLI output, and restore the Docker driver's original workspace failure helpers. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
@elezar this is ready for another review. Since your last review on
The driver code is unchanged from what you reviewed, and the rest is test cleanup. CI on |
Signed-off-by: Evan Lezar <elezar@nvidia.com>
elezar
left a comment
There was a problem hiding this comment.
The shared OCI suite and workspace-rejection reporting look good. The expected fixture image ID is now captured before sandbox creation, with image cleanup preserved on inspection failure. Branch Checks and Core E2E pass on fde1cc5, including the Docker rootful DEB, Podman rootful/rootless RPM, and ARM64 Docker OCI lanes.
The previously discussed #4030 identity scenarios remain nonblocking follow-ups. When updating #4120, we'll also need to migrate the OCI suite added here to e2e/suites/features/oci-image, adopt e2e/support/rust, and update its archive, task, and workflow references. That migration does not block this PR.
Summary
Adds
oci-image, a shared feature suite that checks how an image'sUSERandWORKDIRset the sandbox identity and workspace. It runs through the installed CLI on Docker rootful, Podman rootful, Podman rootless, and ARM64 Docker. It replaces Docker-only and Podman-only e2e checks that couldn't catch a regression on the other runtime.Closes #4346. Related to #2526.
The suite
tests/suites/features/oci-imageruns four scenarios:USERwith a customWORKDIR: identity, working directory, unchanged root-owned image files, writes from the main process andsandbox exec, and upload/download relative to the workspaceUSERwith private (0700)WORKDIRparents, under a policy with noprocesssection, so the imageUSERis the only identity sourceWORKDIR: falls back to the managed/sandboxWORKDIRpair: the control succeeds, and the unwritable one fails before the command runs. The suite reads theWorkspaceValidationFailedcondition reason fromopenshell sandbox get --output json, so another provisioning failure can't passEvery scenario also checks that the workload has no capabilities, has
NoNewPrivsset, and cannot read the sandbox credentials, and that a customWORKDIRis not a mount.The numeric-
USERscenario also inspects the driver's containers through the gateway's engine: the workload runs the image ID inspected before creation, starts directly as the OCI identity, has no network, and has no TLS or supervisor-state mounts, and the supervisor is a separate, non-root container.Wiring: tmachine testsuite, test archive, and playbook;
oci-imageentries in the branch, release-dev, and release-tag matrices, including ARM64 Docker.Other changes
WorkspaceValidationFailedwith a fixed message. A workload that exits with 78 itself is reported to the driver as 1.custom_image.rs. Trimspodman_oci_identity.rsto the Podman-specific supervisor checks: the companion runs as the resolved OCI identity (Docker uses a fixed unprivileged user) and has no effective capabilities.Follow-ups
USER rootimage fails with an unexplained "UID or GID zero" error #4030 identity scenarios (USER rootwith and without a policy override, and noUSER). The root rejection first needs Podman to reportIdentityResolutionFailed.tests/suitestoe2e/suites. If test(conformance): move scenario files into Cargo tests #4120 merges first, this PR moves the suite before merging. The remaining Podman-specificpodman_oci_identitychecks can move into a Podman driver suite as part of that work.driver_config_volume.rsstays as it is. Moving it needs driver-config mounts enabled on the tmachine gateways.Testing
mise run pre-commiton2bb7f665e; fmt and clippy on the suite and the e2e crated181609a7: Branch Checks and Core E2E pass, including all fouroci-imagelanesmise run e2e:oci-imagelocally on macOS Docker Desktop: sandbox creation fails before the scenarios run because the supervisor cannot reach the gateway; CI is the first real run of the latest changesChecklist
TESTING.md