feat(podman): isolate workloads behind a separate supervisor - #3230
Conversation
|
🌿 Preview your docs: https://nvidia-preview-pr-3230.docs.buildwithfern.com/openshell |
163a191 to
355a410
Compare
|
Label |
355a410 to
4ddf65a
Compare
4ddf65a to
6179a23
Compare
342de37 to
c009765
Compare
c009765 to
a820116
Compare
c85c320 to
8e4ea44
Compare
9df0674 to
d340437
Compare
d340437 to
bedbfba
Compare
bedbfba to
93c493a
Compare
93c493a to
2e4b841
Compare
2e4b841 to
a83bc00
Compare
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
The Podman layer fits RFC 0012 and its six-PR isolation stack, but the current incremental patch has five blocking regressions affecting startup, workload configuration, runtime containment, and published setup guidance.
Action required: @drew, please address the five findings below and push a new head for a focused follow-up review.
Blocking findings:
GATOR-a83bc000-01: the workload lacks its required writable/run.GATOR-a83bc000-02: template and request environment is dropped.GATOR-a83bc000-03: the configured Podman PID limit is serialized in an ignored shape.GATOR-a83bc000-04: accepted AppArmor configuration is silently discarded.GATOR-a83bc000-05: Fern examples regress to rejected schema-v1 configuration.
Carried findings:
- None
Non-blocking suggestions:
- None
Gator metadata
- Validation: Maintainer-authored Podman implementation of the actively reviewed RFC 0012 architecture, scoped as layer 5 of 6 and reviewed incrementally against PR #3144.
- Docs: Updated, but blocking because the gateway reference replaces valid schema-v2 guidance with rejected schema-v1 examples.
- Checks: Not eligible for pipeline handoff while blocking review findings remain.
- E2E:
test:e2eis applied; its bot rerun instruction targets an older head, so current-head dispatch is deferred until review blockers are resolved. - Head SHA:
a83bc00027b57eaf852148331f3c92935a4836a5 - Base SHA:
52de5e21e18d606e8a01bdd82ca709859e0b71b4 - Merge base SHA:
52de5e21e18d606e8a01bdd82ca709859e0b71b4 - Patch ID:
becfa00352b52bbb97697f965978bec996cc831d - Gator payload:
8 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
| trusted_mount(&mount.destination) | ||
| && mount.destination != openshell_core::container_paths::NETNS_MOUNT_ROOT | ||
| }); | ||
| for destination in ["/run", "/var/log", "/tmp"] { |
There was a problem hiding this comment.
gator-agent
Critical — GATOR-a83bc000-01 · Give the workload its writable runtime directory
Summary: A normal Podman sandbox running as a non-root identity reaches proxy-mode startup and must create its boundary CA under /run/openshell-proxy-ca. This loop adds the 64 MiB, identity-owned /run tmpfs only to the supervisor companion. The workload keeps the image's root-owned /run, so boundary startup fails with permission denied and the admitted agent never starts.
Fix: Add a separate, size-bounded /run tmpfs to the workload, owned by its resolved UID/GID, while retaining the supervisor's own mount. Cover successful proxy-mode startup as that non-root identity.
Verify: Create a default-image Podman sandbox with normal network policy as UID 1000. It must install boundary CA material and start; both container specs must show their distinct writable /run mounts.
Agent context
- Ownership: This PR splits the former root supervisor into a non-root workload boundary and places the new runtime mount on only one member.
- Location:
crates/openshell-driver-podman/src/container.rs:1505
| let workload_id = self.client.create_typed_container(&specs.workload).await?; | ||
| created_workload = Some(workload_id.clone()); | ||
| self.client.verify_isolation_fence(&workload_id).await?; | ||
| let child_env = image_env |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-a83bc000-02 · Preserve the admitted workload environment
Summary: A user creating a supported Podman sandbox can set environment variables in the template or request and override image defaults. The split topology clears the workload container environment, making BoundaryConfig.child_env the only delivery path, but this code rebuilds it from image Config.Env alone. Template variables and request overrides silently disappear on initial launch and restart.
Fix: Merge image defaults, template environment, and request environment in that precedence order; remove protected OPENSHELL_* control keys consistently with the other isolated drivers; and cover initial launch plus restart preservation.
Verify: Use image A=image, template B=template, and request A=request; decode sandbox/bootstrap.json. child_env must contain A=request and B=template, with protected control keys absent, before and after restart.
Agent context
- Path: CreateSandbox → cleared container environment → boundary bootstrap child environment.
- Ownership: This PR introduces this bootstrap construction for the split Podman topology.
- Location:
crates/openshell-driver-podman/src/driver.rs:1047
| #[derive(Serialize)] | ||
| struct PidsLimits { | ||
| limit: i64, | ||
| #[serde(rename = "PidsLimit", skip_serializing_if = "Option::is_none")] |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-a83bc000-03 · Restore Podman's enforceable PID-limit shape
Summary: An operator can configure sandbox_pids_limit to cap process consumption. This change replaces libpod's OCI resource_limits.pids.limit representation with Docker-style PidsLimit, which Podman ignores. A sandbox can therefore exceed the selected ceiling and increase host denial-of-service exposure.
Fix: Restore nested resource_limits.pids.limit serialization for both generated container specs and retain a runtime-inspection regression proving a non-default limit is enforced.
Verify: Configure sandbox_pids_limit=31, create a Podman sandbox, and inspect the workload with podman inspect --format '{{.HostConfig.PidsLimit}}'. The current head reports the runtime default; the fixed path must report 31, and the create request must contain resource_limits.pids.limit=31.
Agent context
- Ownership: This PR directly replaces the working stacked-base OCI representation and changes its unit expectation to the ignored field.
- Location:
crates/openshell-driver-podman/src/container.rs:344
| cap_drop: Vec<String>, | ||
| cap_add: Vec<String>, | ||
| no_new_privileges: bool, | ||
| #[serde(skip_serializing_if = "String::is_empty")] |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-a83bc000-04 · Do not silently discard AppArmor policy
Summary: An operator can still select a non-default Podman app_armor_profile, including a local confinement profile, and driver startup accepts it. The paired container specs no longer serialize that setting for either workload or supervisor, so the runtime applies its default while the operator reasonably believes the chosen protection is active.
Fix: Define the pair semantics and apply the configured profile to the intended container or containers. If non-default profiles are incompatible with this topology, reject them during configuration validation with migration-quality guidance. Add a regression for the chosen contract.
Verify: Configure app_armor_profile=Localhost/openshell-sandbox, build the pair, and inspect both serialized specs or containers. The intended member(s) must carry that profile, or configuration must fail explicitly before resource creation.
Agent context
- Ownership: This PR removes the serialized AppArmor field while retaining the public option and its documentation.
- Location:
crates/openshell-driver-podman/src/container.rs:222
|
|
||
| [openshell] | ||
| version = 2 | ||
| version = 1 |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-a83bc000-05 · Keep Fern examples on the accepted gateway schema
Summary: An operator copying the updated gateway reference or its driver examples gets configuration that current preflight rejects. This patch replaces the stacked base's schema-v2 guidance with version = 1 and compute_drivers lists, removes its migration path, and says Podman's nonzero health interval accepts zero. These are direct, user-facing startup failures.
Fix: Restore the stacked base's schema-v2 reference and migration section, then layer only the new Podman split-topology fields onto it. Keep examples on version = 2, scalar compute_driver, correct driver-scoped values, and the actual nonzero/default health behavior.
Verify: Copy the full and Podman examples into temporary gateway configs and run preflight; both must pass. Setting the documented health value must deserialize as described rather than reject zero.
Agent context
- Ownership: The incremental diff directly replaces correct base documentation with stale schema-v1 content.
- Location:
docs/reference/gateway-config.mdx:71
johntmyers
added
the
gator:in-review
drew
changed the title
a83bc00 to
4fec39f
Compare
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
The current head rebases the Podman layer onto the updated RFC 0012 stack and adds the sandbox-runtime naming change, but it does not resolve the five prior blocking obligations. The focused follow-up found no additional blockers.
Action required: @drew, please address the five carried findings and push a new head for another focused follow-up.
Blocking findings:
- No new blocking findings.
Carried findings:
GATOR-a83bc000-01: add a separate identity-owned, size-bounded writable/runtmpfs to the workload.GATOR-a83bc000-02: preserve image, template, and request environment with correct precedence, protected-key filtering, and restart behavior.GATOR-a83bc000-03: restore Podman's enforceable nestedresource_limits.pids.limitserialization for both containers.GATOR-a83bc000-04: apply the accepted AppArmor profile to the intended container(s), or reject unsupported profiles before resource creation.GATOR-a83bc000-05: restore valid schema-v2 Fern examples and the actual nonzero Podman health-check contract.
Gator metadata
- Validation: Maintainer-authored Podman implementation of the actively reviewed RFC 0012 architecture, scoped as layer 5 of 6 and reviewed incrementally against PR #3144.
- Docs: Still blocking because copyable gateway examples publish rejected schema-v1 configuration.
- Checks: Current-head Branch Checks and E2E completed with failures; pipeline triage is deferred while code-review blockers remain.
- E2E:
test:e2eis applied and a current-head E2E run completed, so no additional dispatch action is needed in this review cycle. - Head SHA:
4fec39f7bd951d03ab6227f6cdd99eaf6f67c81d - Base SHA:
f55f3859e035d93fe0447c4ff12cc21b56820528 - Merge base SHA:
f55f3859e035d93fe0447c4ff12cc21b56820528 - Patch ID:
4b2e7da6a4563536e1baf2a49b8dfa96877f9cf4 - Gator payload:
8 - Review mode:
follow_up - Previous reviewed SHA:
a83bc00027b57eaf852148331f3c92935a4836a5 - Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
4fec39f to
1c55395
Compare
2 participants
Summary
Add the Podman implementation of RFC 0012. The driver launches separate workload and supervisor containers, connects them through a private Unix socket, and denies direct workload egress.
The protocol uses a fresh pinned TLS server identity and launch-scoped sandbox JWT over the socket. Restart preserves the admitted workload configuration while replacing the session and protected authentication state.
Related Issue
Part of #1737.
Changes
/runand reject stale or dangling runtime state.Testing
mise run pre-commitChecklist
Stack