Repository navigation
fix: resolve every downward API env fieldRef the kubelet supports - #594
Merged
dciangot merged 1 commit intoOct 28, 2026
Merged
Conversation
With SkipDownwardAPIResolution set, the virtual-kubelet library leaves downward API env vars to the provider, and resolveEnvRefs handled only metadata.name, metadata.namespace, metadata.uid, status.podIP and metadata.annotations['<KEY>']. Any other fieldRef fell into the default branch and was silently set to an empty string: spec.nodeName (the usual NODE_NAME), spec.serviceAccountName, metadata.labels['<KEY>'], status.hostIP, status.podIPs and status.hostIPs. A missing annotation also kept its valueFrom instead of resolving to an empty value. resolveDownwardAPIFieldPath, used for downwardAPI volumes, already knew most of these but not the subscripted labels/annotations, which are valid volume items too. Make resolveEnvRefs use resolveDownwardAPIFieldPath and extend it: - metadata.labels['<KEY>'] and metadata.annotations['<KEY>'], with an empty value for a missing key, as the kubelet does; - status.podIPs and status.hostIPs, which hold the single IP the virtual kubelet reports (see PodPhase). A field path that still cannot be resolved is logged and left alone instead of being emptied without a trace. Fixes interlink-hq#593 Signed-off-by: NopeCharon <[email protected]>
✅ Deploy Preview for interlink-dev canceled.
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the shared downward API field-resolution change.
No open issues 🎉
What changed in this PR
This PR addresses #593 by sharing downward API field resolution between environment variables and volumes, preventing supported env field references from silently becoming empty.
Changes:
- Routes env
fieldRefvalues through the shared resolver and adds metadata subscripts and single-IP list aliases. - Logs unsupported env field paths and leaves them unresolved.
- Adds tests for env field references and metadata subscripts in projected volumes.
| File | Description |
|---|---|
pkg/virtualkubelet/execute.go |
Extends the shared resolver and uses it for env field references. |
pkg/virtualkubelet/execute_test.go |
Adds env resolution coverage and projected-volume metadata assertions. |
Review effort: Balanced
dciangot
self-requested a review
October 28, 2026 10:17
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
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.
Summary
When
SkipDownwardAPIResolutionis on, the virtual-kubelet library leaves downward API env vars to interLink'sresolveEnvRefs. That function only handled five field paths. Every otherfieldRefwas silently set to an empty string, so offloaded containers gotNODE_NAME=''fromspec.nodeName, and the same happened forspec.serviceAccountName,metadata.labels['<KEY>'],status.hostIP,status.podIPsandstatus.hostIPs. There was no log line for any of it.The downwardAPI volume code already has a resolver (
resolveDownwardAPIFieldPath) that covers most of these. This PR makes env vars use it too, so both go through one resolver, and extends it with:metadata.labels['<KEY>']/metadata.annotations['<KEY>']. These are valid in downwardAPI volume items too, where they used to be dropped with an "unsupported FieldPath" warning. A missing key resolves to an empty value, as with the kubelet. Before, a missing annotation left the env var with itsvalueFrom.status.podIPs/status.hostIPs. The VK reports one pod/host IP (PodPhase), so these hold that IP.A field path that still can't be resolved is logged and left unresolved instead of being emptied silently. The Slurm plugin reads only
env.Value, so the job sees the same thing as before; the operator now gets a warning. Kubernetes validation already rejects such paths, so this should not happen in practice.Tests:
TestResolveEnvRefsFieldRefcovers every env field path, plus a missing annotation and an unsupported path;TestRemoteExecutionHandleProjectedSourceDownwardAPIFieldRefnow also checksmetadata.labels['tier']andmetadata.annotations['my.annotation/key']volume items.Without the change, both tests fail on the paths above.
go test -race ./pkg/...andgolangci-lint run(v2.14.0, as in CI) pass.Related issue : #593