fix(icms-translate): run LLM sidecars as a shared UID so pylon can read the worker-token#409
Open
Max-NV wants to merge 1 commit into
Open
fix(icms-translate): run LLM sidecars as a shared UID so pylon can read the worker-token#409Max-NV wants to merge 1 commit into
Max-NV wants to merge 1 commit into
Conversation
…ad the worker-token The llm-credential-manager writes the worker auth token to the shared "llm" emptyDir with mode 0600, but pylon (llm-router-client) runs as the distroless non-root user nvs (1000) while the credential-manager runs as root, so pylon cannot read it. Registration fails at token read before pylon dials the router, leaving the model unregistered and gateway invokes returning no_eligible_candidates. Render both sidecars with runAsUser/runAsGroup 1000 so they share the token owner while keeping 0600; the customer workload container runs as its own user and still cannot read it. Regenerate the container/llm and helmchart/llm goldens and sync the NVCA vendored copy. NO-REF
📝 WalkthroughWalkthroughBoth LLM sidecars now run with a shared non-root UID/GID of 1000. A test verifies the router and credential-manager containers use matching user and group security contexts. ChangesLLM sidecar security
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Max-NV
requested review from
estroz and
kristinapathak
and removed request for
balajinvda
July 24, 2026 03:06
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/libraries/go/lib/pkg/icms-translate/translate/function/llm.go`:
- Around line 50-57: Add a permission mechanism for the shared LLM emptyDir
mounted at /var/run/llm, such as configuring the pod security context with the
sidecars’ shared group via fsGroup or adding an initContainer to chown the
directory before startup. Update the relevant pod construction alongside
newLLMSidecarSecurityContext and add a test verifying worker-token is writable
by the non-root sidecars.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4718fbcb-08cb-4c72-a788-b52a8c568a86
⛔ Files ignored due to path filters (3)
src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/icms-translate/translate/function/llm.gois excluded by!**/vendor/**src/libraries/go/lib/testdata/icms-translate/function/container/llm/exp.yamlis excluded by!**/testdata/**src/libraries/go/lib/testdata/icms-translate/function/helmchart/llm/exp.yamlis excluded by!**/testdata/**
📒 Files selected for processing (2)
src/libraries/go/lib/pkg/icms-translate/translate/function/llm.gosrc/libraries/go/lib/pkg/icms-translate/translate/function/llm_test.go
Comment on lines
+50
to
+57
| // newLLMSidecarSecurityContext runs both LLM sidecars as the same UID so they | ||
| // can exchange the worker-token on the shared emptyDir. | ||
| func newLLMSidecarSecurityContext() *corev1.SecurityContext { | ||
| uid := llmSidecarRunAsUser | ||
| return &corev1.SecurityContext{ | ||
| RunAsUser: &uid, | ||
| RunAsGroup: &uid, | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
rg -n -C 5 \
'fsGroup|FSGroup|initContainers|chown|newLLMClientVolumes|/var/run/llm' \
srcRepository: NVIDIA/nvcf
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -eu
printf '\n== llm.go ==\n'
cat -n src/libraries/go/lib/pkg/icms-translate/translate/function/llm.go | sed -n '1,220p'
printf '\n== nearby security-context usage ==\n'
rg -n -C 3 'newLLMSidecarSecurityContext|newLLMClientVolumes|fsGroup|initContainers|worker-token|/var/run/llm' src/libraries/go/lib/pkg/icms-translate/translate/function
printf '\n== AGENTS files in scope ==\n'
find src/libraries/go/lib -path '*/AGENTS.md' -o -name 'AGENTS.md'Repository: NVIDIA/nvcf
Length of output: 25131
🏁 Script executed:
#!/bin/bash
set -eu
printf '\n== llm_test excerpt ==\n'
sed -n '430,470p' src/libraries/go/lib/pkg/icms-translate/translate/function/llm_test.go
printf '\n== pod security context search ==\n'
rg -n -C 4 'PodSecurityContext|FSGroup|fsGroup|RunAsUser|RunAsGroup|SupplementalGroups|initContainers|chown' \
src/libraries/go/lib/pkg/icms-translate/translate/function
printf '\n== volume definition ==\n'
sed -n '260,290p' src/libraries/go/lib/pkg/icms-translate/translate/function/llm.goRepository: NVIDIA/nvcf
Length of output: 7047
🏁 Script executed:
#!/bin/bash
set -eu
printf '\n== translate_container.go security-related excerpt ==\n'
sed -n '1,240p' src/libraries/go/lib/pkg/icms-translate/translate/function/translate_container.go
printf '\n== translate_helm.go security-related excerpt ==\n'
sed -n '1,220p' src/libraries/go/lib/pkg/icms-translate/translate/function/translate_helm.go
printf '\n== all pod security context references in subtree ==\n'
rg -n -C 2 'PodSecurityContext|FSGroup|fsGroup|SupplementalGroups|RunAsUser|RunAsGroup|fsGroupChangePolicy|initContainers' \
src/libraries/go/lib/pkg/icms-translate/translate/functionRepository: NVIDIA/nvcf
Length of output: 21286
🏁 Script executed:
#!/bin/bash
set -eu
printf '\n== SecurityContext / PodSecurityContext in pod builders ==\n'
rg -n -C 3 'SecurityContext:|PodSecurityContext:|fsGroup|FSGroup|SupplementalGroups|initContainers' \
src/libraries/go/lib/pkg/icms-translate/translate/function/translate_container.go \
src/libraries/go/lib/pkg/icms-translate/translate/function/translate_helm.go
printf '\n== llm tests mentioning pod or security context ==\n'
rg -n -C 2 'PodSecurityContext|SecurityContext|fsGroup|fsGroupChangePolicy|SupplementalGroups|initContainers' \
src/libraries/go/lib/pkg/icms-translate/translate/function/llm_test.go \
src/libraries/go/lib/pkg/icms-translate/translate/function/translate_tolerations_test.goRepository: NVIDIA/nvcf
Length of output: 5926
Add a volume-permission step for the shared LLM emptyDir
newLLMSidecarSecurityContext() makes both sidecars non-root, but the pod still mounts a plain emptyDir with no fsGroup or initContainer chown. That can leave /var/run/llm unwritable for worker-token; add a permission mechanism and a test for it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/libraries/go/lib/pkg/icms-translate/translate/function/llm.go` around
lines 50 - 57, Add a permission mechanism for the shared LLM emptyDir mounted at
/var/run/llm, such as configuring the pod security context with the sidecars’
shared group via fsGroup or adding an initContainer to chown the directory
before startup. Update the relevant pod construction alongside
newLLMSidecarSecurityContext and add a test verifying worker-token is writable
by the non-root sidecars.
apartha-nv
requested changes
Jul 24, 2026
| // pylon's image user (nvs). Both LLM sidecars run as this UID so pylon can | ||
| // read the 0600 worker-token the credential-manager writes to the shared | ||
| // emptyDir; the customer container runs as its own user and cannot. | ||
| llmSidecarRunAsUser int64 = 1000 |
Contributor
There was a problem hiding this comment.
can we make this configurable ? with a default
this could conflict on k8s platforms like openshift as the userid range is stipulated via PodSecurity admission.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Managed LLM workers reach the request router but never register, so gateway invokes return
no_eligible_candidates. The llm-credential-manager writes the worker auth token to the sharedllmemptyDir as mode 0600, but pylon (llm-router-client) runs as the distroless non-root usernvs(1000) while the credential-manager runs as root, so pylon cannot read it and fails before dialing the router.What changed
Render both LLM sidecars with
runAsUser/runAsGroup1000 so they share the token owner. The token stays 0600; the customer workload container runs as its own user and still cannot read it. Regenerated thecontainer/llmandhelmchart/llmgoldens and synced the NVCA vendored copy.Customer Release Notes
LLM workers now register with the request router in managed deployments; invocations no longer fail with no_eligible_candidates from the worker-token permission mismatch.
Testing
go test ./pkg/icms-translate/translate/function/..., including the newTestLLMSidecarsShareRunAsUser. Golden diff is only the securityContext block on both sidecars.References
Closes #410