NVIDIA / NVIDIA/OpenShell

feat(helm): expose sandbox_uid / sandbox_gid as first-class Helm values

Open
#2,697 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

state:accepted
Dominant language
Rust
Stars
8.7k
Forks
1.3k
Avg merge
2d 11h
Merged PRs (30d)
253

Description

Problem Statement

The Kubernetes compute driver already accepts sandbox_uid and sandbox_gid as first-class config fields (via [openshell.drivers.kubernetes] TOML, OPENSHELL_K8S_SANDBOX_UID / OPENSHELL_K8S_SANDBOX_GID env vars, or KubernetesComputeConfig::sandbox_uid / sandbox_gid in code — see crates/openshell-driver-kubernetes/src/main.rs:119-123 and src/config.rs). Setting them takes priority over any OpenShift SCC namespace-annotation resolution and is the platform-agnostic way to pin a sandbox identity.

However, the Helm chart's gateway ConfigMap template (deploy/helm/openshell/templates/gateway-config.yaml) does not render either field, even though it already renders every other optional KubernetesComputeConfig field via the {{- if .Values.server.XXX }} pattern (image_pull_policy, image_pull_secrets, workspace_default_storage_size, workspace_storage_class, default_runtime_class_name, app_armor_profile, supervisor_image_pull_policy, provider_spiffe_workload_api_socket_path).

Consequences on a Helm-installed gateway today:

  • Non-OpenShift Kubernetes users who want a specific sandbox UID have exactly one lever — the openshift.io/sa.scc.uid-range annotation on the target namespace. This works on vanilla K8s (we've been using it in a Dell PowerScale Kerberos-per-UID prototype), but it's an OpenShift-branded knob with <start>/<size> semantics that most non-OpenShift operators don't recognize, and it's set on the namespace rather than on the gateway that owns the sandbox model.
  • The platform-agnostic sandbox_uid config — designed exactly for this — is unreachable from a stock helm install. Operators either patch the rendered ConfigMap by hand (breaks Helm ownership) or fall back to the OpenShift annotation.
  • RFC 0011 Phase 3 (#2656) and the per-sandbox UID injection work (#1959) will both benefit from having sandbox_uid reachable from Helm, because multi-workspace and multi-tenant deployments need per-workspace UID selection driven by chart values rather than annotations on hand-created namespaces.

Context: I noted this three weeks ago as a comment on #1959 (July 21) and offered to send the PR. No response from maintainers, so filing as a standalone issue per the offer in that comment. Happy to keep #1959 tightly scoped to driver/policy/supervisor work.

Proposed Design

Add two optional Helm values on the server object and render them into the gateway ConfigMap when set. Matches the exact {{- if .Values.server.XXX }} pattern already used by every sibling KubernetesComputeConfig field.

deploy/helm/openshell/values.yaml (additions under server:):

server:
  # ... existing fields ...

  # Optional: sandbox process UID/GID rendered into the Kubernetes
  # compute driver config. When unset, the driver falls back to
  # openshift.io/sa.scc.uid-range namespace annotation or the
  # built-in default (1000).
  sandboxUid: ""
  sandboxGid: ""

deploy/helm/openshell/templates/gateway-config.yaml (additions inside the [openshell.drivers.kubernetes] block, following the pattern established by image_pull_policy, workspace_default_storage_size, etc.):

    {{- if .Values.server.sandboxUid }}
    sandbox_uid                  = {{ .Values.server.sandboxUid }}
    {{- end }}
    {{- if .Values.server.sandboxGid }}
    sandbox_gid                  = {{ .Values.server.sandboxGid }}
    {{- end }}

Both integer-valued fields are emitted without quotes to match the numeric-scalar semantics the driver expects (validated by KubernetesComputeConfig::validate_sandbox_identity_config() at crates/openshell-driver-kubernetes/src/config.rs).

Values contract:

  • Both fields default to empty (""). When empty, they render nothing, preserving current behavior exactly.
  • sandboxUid, when set, must be a positive integer in the range enforced by openshell_policy::MIN_SANDBOX_UID..=MAX_SANDBOX_UID. Same validation as the direct TOML path.
  • sandboxGid follows the same rules; when only sandboxUid is set, the driver's resolve_sandbox_gid() already handles the fallback correctly (uses sandboxUid as GID).
  • If a user sets both a sandboxUid chart value AND has an SCC uid-range annotation on the target namespace, the driver's existing resolution order applies: config value wins (see KubernetesComputeConfig::resolve_sandbox_uid at config.rs:406).

Documentation:

  • Add both fields to docs/reference/gateway-config.mdx under the Kubernetes driver section.
  • Cross-reference from the docs/openshift guide noting that non-OpenShift users can use server.sandboxUid instead of the SCC annotation.

Alternatives Considered

  1. Namespace annotation (current state). Works but is OpenShift-branded and requires the operator to hand-annotate every namespace the gateway serves. Awkward for non-OpenShift shops, and doesn't compose with Helm's declarative model (annotation lives on a resource the chart doesn't own). This is what motivated the issue.

  2. Manual ConfigMap patch after helm install. Operators post-process the rendered ConfigMap with kubectl patch or a helm.sh/hook. Breaks Helm ownership; the next helm upgrade clobbers the patch. Rejected as un-declarative.

  3. extraGatewayConfig free-form string in values. Some charts allow a raw TOML/YAML snippet to be appended to the config. Trade-off: totally flexible but bypasses all validation and shape-checking. Rejected as more error-prone than a typed field. Kept as a possible fallback if the maintainers prefer minimally-typed Helm values, but the pattern used by every sibling driver field in the same ConfigMap argues for the typed approach.

  4. Environment variable injection via extraEnv. Set OPENSHELL_K8S_SANDBOX_UID on the deployment container via the chart's existing env-var override support. Works today with no chart changes, but is a per-instance ordering-sensitive workaround that doesn't surface as a first-class user-facing knob. Rejected: hides intent, undiscoverable in helm show values.

The proposed design is the smallest change that:

  • Uses only existing chart patterns
  • Requires zero code changes in openshell-driver-kubernetes
  • Preserves back-compat when unset (renders nothing → same behavior as today)
  • Composes cleanly with Helm's declarative model

Agent Investigation

  • Kubernetes driver already accepts sandbox_uid / sandbox_gid — verified via crates/openshell-driver-kubernetes/src/main.rs:119-123 and src/config.rs (both fields are Option<u32> on KubernetesComputeConfig, with resolve_sandbox_uid() and resolve_sandbox_gid() handling the resolution order documented at lines 398-431).
  • Existing pattern in deploy/helm/openshell/templates/gateway-config.yaml — verified via lines 134-165, where six sibling KubernetesComputeConfig fields are rendered with the same {{- if .Values.server.XXX }} idiom.
  • values.yaml has no sandboxUid / sandboxGid entries — verified by grep -n on the current main branch (as of c825b1f8).
  • No commits since 2026-07-21 (my #1959 comment) have touched either file with sandbox_uid-adjacent content — verified via git log --since='2026-07-21' -S 'sandbox_uid'.
  • Cross-references validated:
    • crates/openshell-policy UID range constants
    • Docs page docs/reference/gateway-config.mdx
    • Related open work: #1959 (per-sandbox UID injection), #2486 (workspace→namespace), #2656 (RFC 0011 Phase 3 in test:e2e)

Checklist

  • I've reviewed existing issues and the architecture docs
  • This is a design proposal, not a "please build this" request

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with deploy/helm/openshell/values.yaml and deploy/helm/openshell/templates/gateway-config.yaml, comparing the requested values with the existing Kubernetes driver fields. Check the related Kubernetes configuration in crates/openshell-driver-kubernetes/src/config.rs, then update the referenced gateway and OpenShift documentation; done means unset values preserve current rendering and set values appear in the gateway configuration.

Written by the indexing model from the issue text.

Assessment

Tech stack
helm, kubernetes, rust
Domain
devops, documentation, infrastructure
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.