fix(kubernetes): preserve sandbox ownership when seeding workspace - #3208
fix(kubernetes): preserve sandbox ownership when seeding workspace#3208loveRhythm1990 wants to merge 1 commit into
Conversation
The workspace-init container seeds a fresh PVC as root and extracted with --no-same-owner, so every seeded path landed owned by uid 0. A mode-0700 home directory shipped by the image (~/.config, ~/.cache) was then unreachable for the workload. fsGroup does not compensate: kubelet applies it when the volume is mounted, which is before the init container writes anything. Under the sidecar topology the supervisor's privileged workspace reconciliation does not run, so the seeded ownership is load-bearing. Rewrite ownership to the resolved sandbox identity while building the transfer archive instead. Seeding stays root, so images that ship root-owned private content still seed, and no recursive chown runs over the workspace (the pattern that broke on read-only submounts in NVIDIA#2294). Modes and timestamps are still not restored. --owner/--group are GNU extensions and this init container runs the sandbox image itself, so the script probes tar once and falls back to the previous flags on a minimal base such as Alpine, which provides BusyBox tar. Those images keep the behavior they have today rather than failing to start. Signed-off-by: loveRhythm1990 <qiuweimin@126.com>
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
This localized Kubernetes workspace-seeding fix is project-valid and the user-facing documentation is updated, but one storage-backend regression must be addressed before CI handoff.
Action required: @loveRhythm1990, make ownership restoration fall back safely when the destination filesystem rejects chown, and add a constrained behavioral regression test.
Blocking findings:
GATOR-f5508202-01: ownership restoration can prevent sandbox startup on writable filesystems that reject ownership changes.
Carried findings:
- None
Gator metadata
- Validation: Localized bug fix linked to #2761 with a concrete production path, reproduction, and no duplicate candidate.
- Docs: Fern compute-driver reference updated; navigation already covers the existing page.
- Checks: Required current-head workflows have not started; copy-pr validation is still pending.
- E2E:
test:e2eand Kubernetes-specific coverage are required after review feedback is resolved; not dispatched yet. - Head SHA:
f5508202e5de30185d8e79154dec3712947f9b87 - Base SHA:
320d4ef79dd572c642133f175f12bafc20d89fd9 - Merge base SHA:
320d4ef79dd572c642133f175f12bafc20d89fd9 - Patch ID:
f1044b20935f30a89efda6aa8eec7cba011ea349 - Gator payload:
8 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
| (cd {WORKSPACE_MOUNT_PATH} && find . -mindepth 1 -maxdepth 1 -exec tar $own -cf \"$tmp\" {{}} +) && \ | ||
| if [ -f \"$tmp\" ]; then \ | ||
| tar -C {WORKSPACE_INIT_MOUNT_PATH} --no-same-owner --no-same-permissions --touch -xf \"$tmp\" && \ | ||
| tar -C {WORKSPACE_INIT_MOUNT_PATH} $ext --no-same-permissions --touch -xf \"$tmp\" && \ |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-f5508202-01 · Ownership restore can block startup
Summary: An operator using a writable PVC backend that rejects ownership changes, such as root-squashed NFS, can create a sandbox whose GNU-tar init container now fails while restoring numeric ownership. The old path seeded degraded root-owned content; this path exits before writing the sentinel, so the pod remains stuck retrying initialization.
Fix: If numeric-owner extraction fails because the destination cannot change ownership, retry the existing archive with --no-same-owner; write the sentinel only if one extraction succeeds. Add a behavioral regression test for successful ownership restoration and the denied-chown fallback.
Verify: Run the generated script as UID 0 without CAP_CHOWN against a writable destination. Numeric ownership should fail, fallback extraction should succeed, and the sentinel should exist; if both attempts fail, it must remain absent.
Agent context
- Location:
crates/openshell-driver-kubernetes/src/driver.rs:3517 - Ownership: This PR replaces unconditional
--no-same-ownerextraction with ownership restoration after probing only tar option support.
Summary
Workspace PVC seeding wrote every path as root, so a mode-
0700home directoryshipped by the sandbox image (
~/.config,~/.cache) was unreachable for theworkload under the sidecar topology. This rewrites ownership to the resolved
sandbox identity while building the transfer archive, so seeded content is
usable on first boot.
Related Issue
Fixes #2761.
The issue is still
state:triage-needed, so this is submitted as an obviouslocalized bug fix rather than accepted work: it changes one init-container
command string in the Kubernetes driver, adds no configuration surface, and
introduces no new behavior beyond correcting the seeded ownership. The reported
behavior was independently reproduced on the issue by @jiridanek. Happy to hold
this until the issue is triaged if maintainers prefer that order.
Changes
apply_workspace_persistencetakes the resolvedsandbox_uidalongside theexisting
sandbox_gid, and the init script builds the transfer archive with--owner/--group/--numeric-ownerso extraction restores the sandboxidentity instead of root.
Rewriting at archive time — rather than seeding as the sandbox user, or
chowning the tree afterwards — keeps root's ability to read every source path,
so images that ship root-owned private content still seed. It also avoids a
recursive chown over the workspace, the pattern that broke on read-only
submounts in bug: Kubernetes sandbox crashes EROFS — recursive
chown /sandboxfails on read-only submounts (0.0.82) #2294. Modes and timestamps are still not restored, so a nestedread-only mount under the workspace is never chmod'ed during seeding.
--owner/--groupare GNU extensions, and this init container runs thesandbox image itself. A minimal base such as Alpine seeds with BusyBox
tar,which rejects them; without a guard the init container would exit non-zero and
the pod would never start. The script probes
taronce and falls back to theprevious flags, so those images keep the behavior they have today.
architecture/compute-runtimes.md: documents the seeding ownership contractand the fallback.
docs/reference/sandbox-compute-drivers.mdx: corrects the resolved-identitylist. It claimed the resolved UID/GID appear in the "PVC init container
securityContext.runAsUser/runAsGroup/fsGroup", but that container isrunAsUser: 0with norunAsGroup, andfsGroupis a pod-level field. Theentry now describes where the identity actually lands, including the BusyBox
caveat.
Testing
mise run pre-commitpassesUnit tests (
cargo test -p openshell-driver-kubernetes, 230 passed):workspace_init_seeds_content_owned_by_the_sandbox_identity— asserts theidentity/permission contract: ownership is rewritten, numerically, and no
chownwalks the tree.workspace_init_ownership_tracks_resolved_identity— drives the fullsandbox_template_to_k8spath with an OpenShift-style UID (1000660000) sothe seeded identity is not pinned to
1000.workspace_init_falls_back_when_tar_lacks_ownership_extensions— asserts theprobe and the fallback flags.
workspace_init_command_checks_sentinel— unchanged idempotency andno-metadata-restore coverage.
Behavioral verification: I extracted the script this code actually generates and
ran it as root in two containers, against a PVC root prepared the way kubelet
leaves it (
root:1000, mode2770, setgid).GNU tar image (Ubuntu 24.04 with a LibreOffice-created mode-
0700~/.config) — before and after:Re-running the init script with the sentinel present exits 0 and leaves a file
written into
.configafter seeding untouched.BusyBox
tar(busybox:latest, v1.38.0), the regression this guards against:E2E gap. There is no existing e2e coverage of workspace PVC seeding to
extend —
e2e/rust/tests/workspace_*.rscover the workspace/namespace API, andthe only PVC reference in
e2e/with-kube-gateway.shis cleanup. A first-bootcase as described in the issue's acceptance criteria needs a purpose-built
sandbox image shipping a mode-
0700workspace directory, published where thekind-based lane can pull it. I did not want to bundle that into this fix without
guidance, especially as
test:e2e-kubernetesis currently an optional gate.Happy to add it here or in a follow-up — maintainer's call.
Checklist