Skip to content

fix(kubernetes): preserve sandbox ownership when seeding workspace - #3208

Open
loveRhythm1990 wants to merge 1 commit into
NVIDIA:mainfrom
loveRhythm1990:lr90/k8s-workspace-seed-ownership
Open

fix(kubernetes): preserve sandbox ownership when seeding workspace#3208
loveRhythm1990 wants to merge 1 commit into
NVIDIA:mainfrom
loveRhythm1990:lr90/k8s-workspace-seed-ownership

Conversation

@loveRhythm1990

Copy link
Copy Markdown
Contributor

Summary

Workspace PVC seeding wrote every path as root, so a mode-0700 home directory
shipped by the sandbox image (~/.config, ~/.cache) was unreachable for the
workload 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 obvious
localized 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_persistence takes the resolved sandbox_uid alongside the
    existing sandbox_gid, and the init script builds the transfer archive with
    --owner/--group/--numeric-owner so extraction restores the sandbox
    identity 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 /sandbox fails on read-only submounts (0.0.82) #2294. Modes and timestamps are still not restored, so a nested
    read-only mount under the workspace is never chmod'ed during seeding.

  • --owner/--group are GNU extensions, and this init container runs the
    sandbox 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 tar once and falls back to the
    previous flags, so those images keep the behavior they have today.

  • architecture/compute-runtimes.md: documents the seeding ownership contract
    and the fallback.

  • docs/reference/sandbox-compute-drivers.mdx: corrects the resolved-identity
    list. It claimed the resolved UID/GID appear in the "PVC init container
    securityContext.runAsUser/runAsGroup/fsGroup", but that container is
    runAsUser: 0 with no runAsGroup, and fsGroup is a pod-level field. The
    entry now describes where the identity actually lands, including the BusyBox
    caveat.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable) — see below

Unit tests (cargo test -p openshell-driver-kubernetes, 230 passed):

  • workspace_init_seeds_content_owned_by_the_sandbox_identity — asserts the
    identity/permission contract: ownership is rewritten, numerically, and no
    chown walks the tree.
  • workspace_init_ownership_tracks_resolved_identity — drives the full
    sandbox_template_to_k8s path with an OpenShift-style UID (1000660000) so
    the seeded identity is not pinned to 1000.
  • workspace_init_falls_back_when_tar_lacks_ownership_extensions — asserts the
    probe and the fallback flags.
  • workspace_init_command_checks_sentinel — unchanged idempotency and
    no-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, mode 2770, setgid).

GNU tar image (Ubuntu 24.04 with a LibreOffice-created mode-0700
~/.config) — before and after:

current main            .config  root:sandbox    2700   uid 1000: read DENIED, write DENIED
this PR                 .config  sandbox:sandbox 2700   uid 1000: read OK,     write OK
$ git clone <repo>      # HOME set to the seeded workspace, running as uid 1000
# current main
warning: unable to access '.../.config/git/ignore': Permission denied
warning: unable to access '.../.config/git/attributes': Permission denied
# this PR
(no warnings)

Re-running the init script with the sentinel present exits 0 and leaves a file
written into .config after seeding untouched.

BusyBox tar (busybox:latest, v1.38.0), the regression this guards against:

without the probe       tar: unrecognized option '--owner=1000'
                        script exit=1, sentinel not written -> pod never starts
with the probe          exit=0, sentinel written, seeds exactly as main does today

E2E gap. There is no existing e2e coverage of workspace PVC seeding to
extend — e2e/rust/tests/workspace_*.rs cover the workspace/namespace API, and
the only PVC reference in e2e/with-kube-gateway.sh is cleanup. A first-boot
case as described in the issue's acceptance criteria needs a purpose-built
sandbox image shipping a mode-0700 workspace directory, published where the
kind-based lane can pull it. I did not want to bundle that into this fix without
guidance, especially as test:e2e-kubernetes is currently an optional gate.
Happy to add it here or in a follow-up — maintainer's call.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

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>
@copy-pr-bot

copy-pr-bot Bot commented Sep 6, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

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.

bug(k8s): workspace PVC seeded root-owned, leaving image 0700 home dirs unreadable by the workload under sidecar topology

1 participant