fix(agents): make deployed agent inference URL container-reachable - #899
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAgent deployments now resolve container-reachable gateway URLs, rewrite embedded NAT model URLs, expose Kubernetes internal URLs, and fail fast for unreachable targets. Documentation and tests cover Docker, Kubernetes, overrides, validation, and environment precedence. ChangesContainer-Reachable Agent Deployments
Sequence Diagram(s)sequenceDiagram
participant DeploymentsRunnerBackend
participant resolve_agent_gateway_url
participant rewrite_config_base_urls
participant build_deployment_config
DeploymentsRunnerBackend->>resolve_agent_gateway_url: resolve container-reachable gateway
resolve_agent_gateway_url-->>DeploymentsRunnerBackend: return URL or fail
DeploymentsRunnerBackend->>rewrite_config_base_urls: rewrite inference-gateway NAT URLs
DeploymentsRunnerBackend->>build_deployment_config: build configuration from rewritten NAT data
build_deployment_config-->>DeploymentsRunnerBackend: deployment configuration
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py`:
- Around line 103-140: The docker branch of container_gateway_url must rewrite
bracketed IPv6 loopback URLs such as http://[::1]:8080. Replace the raw
host-string substitution with parsed URL reconstruction that preserves the
scheme, port, path, query, and fragment while setting the hostname to
host.docker.internal; add coverage for the ::1 case.
🪄 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: a26e4eb5-8eb8-4478-8bc8-afaa3f627a59
📒 Files selected for processing (8)
docs/agents/deploy-agents.mdxk8s/helm/templates/api/api-deployment.yamlk8s/helm/templates/core/controller-deployment.yamlplugins/nemo-agents/src/nemo_agents_plugin/config.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_runner_deployments.pyplugins/nemo-agents/tests/unit/test_utils.py
benmccown
force-pushed
the
agent-deployment-url-fix
branch
from
July 24, 2026 22:33
ec0b8ef to
407d787
Compare
Contributor
|
Deployed agents baked the API pod's own base URL into their NAT config llms.*.base_url via inject_gateway_url() at API-create time. Under embedded-PDP auth the API pod's NMP_BASE_URL is a loopback address (http://localhost:8080), and in k8s an agent pod resolves loopback to itself, so agent model calls never reach the platform. If the platform base URL instead pointed at an auth front-door proxy, agents looped forever on 503s and could DoS it (and everything behind it). The only mode-aware rewrite (container_gateway_url) fed a dead NMP_GATEWAY_BASE_URL env var that nothing reads, so neither the docker loopback rewrite nor any k8s fix ever reached the config the agent runs. This fix hands an agent only a base URL we know is container-reachable, never the raw platform base URL. - resolve_agent_gateway_url() returns a known-good target per mode: k8s uses the in-cluster API Service DNS; docker rewrites loopback (including IPv6 [::1]) to host.docker.internal and passes other hosts through. It raises for unsupported modes or when k8s has no internal URL, rather than deploying an agent that cannot reach the platform. - rewrite_config_base_urls() rebases each Inference Gateway llms.*.base_url onto that reachable address, preserving the path. - get_internal_base_url() reads NEMO_INTERNAL_BASE_URL then NMP_INTERNAL_BASE_URL; also agents.deployments.k8s_internal_base_url. - Removes the dead NMP_GATEWAY_BASE_URL env var. - Helm sets NMP_INTERNAL_BASE_URL to the internal API Service DNS on the api and controller pods. - Updates deploy-agents docs; adds unit tests for docker/k8s resolution, the fail-fast path, and config rebasing. Signed-off-by: Ben McCown <bmccown@nvidia.com>
benmccown
force-pushed
the
agent-deployment-url-fix
branch
from
July 24, 2026 22:57
407d787 to
663e76d
Compare
mckornfield
approved these changes
Jul 24, 2026
tylersbray
reviewed
Jul 24, 2026
tylersbray
approved these changes
Jul 24, 2026
Contributor
|
Reviewed and satisfied with the fix — approving. Root cause and the "only ever hand a container-reachable URL" invariant look right, fail-fast + no orphaned config is clean, and the Not blockers for the release, just some adjacent edge cases worth tracking as follow-ups:
|
benmccown
added a commit
that referenced
this pull request
Jul 27, 2026
…h fallback Addresses PR feedback. - Docker backend now injects the auth-proxy sidecar too (build_docker_plan), reusing build_auth_proxy_container. The sidecar shares the primary container's network namespace, so the agent reaches it on localhost:8090 — the same loopback address used in k8s. Previously the sidecar was injected only in the k8s compiler, so auth-on docker agent deployments got no identity and would 401. - The docker sidecar's upstream (NMP_BASE_URL) is rewritten to a docker-reachable host via determine_loopback_override() (e.g. host.docker.internal on macOS) when the platform base URL is a loopback, mirroring the agent-side rewrite from #899. K8s uses the Service DNS verbatim. - Documented platform_auth_enabled()'s fail-to-False behavior: the realistic failure is ImportError (package used outside the platform image); other failures are effectively unreachable in the controller (missing config -> defaults; malformed config would have crashed startup; the read is cached). - Tests: docker auth-proxy injection, auth-off no-op, and loopback upstream rewrite. Signed-off-by: Ben McCown <bmccown@nvidia.com>
AnuradhaKaruppiah
pushed a commit
to AnuradhaKaruppiah/nemo-platform
that referenced
this pull request
Aug 27, 2026
…car (NVIDIA-NeMo#914) * feat(agents): authenticate deployed agents via service-principal sidecar When platform auth is enabled, a deployed agent's Inference Gateway calls were rejected with 401 because the agent (a NAT runtime whose OpenAI HTTP client we do not control) carries no platform credential — its LLM api_key is the placeholder "not-used". This injects a loopback auth-proxy sidecar (the nmp-api image running `nemo services run --sidecars auth-proxy`) into k8s/docker agent deployments when auth is enabled. The agent targets the sidecar on localhost; the sidecar forwards to the platform stamping X-NMP-Principal-Id: service:agents, which the OPA policy authorizes via the ServiceSystem role. This is the same static service identity the platform's own SDK clients use (get_platform_sdk as_service=...); the proxy exists only for workloads that cannot set the header themselves. The LoRA adapters sidecar already self-injects service:models via the SDK and is unaffected. - New workload_proxy sidecar: loopback FastAPI forwarder that strips inbound auth/principal headers and stamps the service principal, streaming responses. Registered as "auth-proxy" in AVAILABLE_SIDECARS. - Agents backend injects the sidecar (as a native init-container sidecar with restartPolicy=Always) and points the agent's llms.*.base_url at it when auth is enabled; unchanged behavior when auth is off. - Fix k8s compiler exec-probe: V1Probe needs the `_exec` kwarg (`exec` is a Python keyword); the sidecar uses an exec readiness probe because it binds loopback only and a pod-IP httpGet probe would be refused. - Unit tests for the sidecar forwarder, agents injection/base_url wiring, and the exec-probe compiler path. Verified end-to-end on kind with auth enabled: agent deployment reaches 2/2 Running (sidecar ready via exec probe) and the agent's IGW call authenticates (reaches model resolution instead of 401). Signed-off-by: Ben McCown <bmccown@nvidia.com> * refactor(deployments): own auth-proxy sidecar compilation via config flags Addresses PR feedback: the auth-proxy sidecar belongs to nemo-deployments (which compiles containers into deployments), not nemo-agents. Also removes a direct nmp_common reference from the agents plugin. - DeploymentConfig gains `auth_proxy_sidecar: bool` and `auth_proxy_sidecar_identity: str | None`. The deployments plugin compiles and injects the sidecar (nemo_deployments_plugin.auth_proxy) from these flags, interpolating the identity into the service-principal header. Injection is a no-op when platform auth is disabled. - nemo-agents now just sets the two flags (identity="agents") and points the agent's llms.*.base_url at the proxy port when auth is enabled; it no longer builds the sidecar container or reads deployments image/port config. - Plugins no longer import nmp_common: added nemo_platform_plugin.auth .platform_auth_enabled() wrapper; both agents and deployments consult it. - Sidecar image/port config moved from AgentsConfig to DeploymentsConfig. - Tests: deployments-side build_auth_proxy_container (requested/auth-off no-op/ identity default) and agents-side flag-setting + base_url wiring. Signed-off-by: Ben McCown <bmccown@nvidia.com> * feat(deployments): inject auth-proxy sidecar for docker; document auth fallback Addresses PR feedback. - Docker backend now injects the auth-proxy sidecar too (build_docker_plan), reusing build_auth_proxy_container. The sidecar shares the primary container's network namespace, so the agent reaches it on localhost:8090 — the same loopback address used in k8s. Previously the sidecar was injected only in the k8s compiler, so auth-on docker agent deployments got no identity and would 401. - The docker sidecar's upstream (NMP_BASE_URL) is rewritten to a docker-reachable host via determine_loopback_override() (e.g. host.docker.internal on macOS) when the platform base URL is a loopback, mirroring the agent-side rewrite from NVIDIA-NeMo#899. K8s uses the Service DNS verbatim. - Documented platform_auth_enabled()'s fail-to-False behavior: the realistic failure is ImportError (package used outside the platform image); other failures are effectively unreachable in the controller (missing config -> defaults; malformed config would have crashed startup; the read is cached). - Tests: docker auth-proxy injection, auth-off no-op, and loopback upstream rewrite. Signed-off-by: Ben McCown <bmccown@nvidia.com> * refactor(deployments): require auth-proxy identity; trim sidecar header stripping Addresses PR feedback. - Remove the default service-principal identity. A DeploymentConfig with auth_proxy_sidecar=True now requires auth_proxy_sidecar_identity: a model_validator rejects the invalid combination, surfaced as a 4xx at the create endpoint. The sidecar's run() likewise requires NMP_AUTH_PROXY_PRINCIPAL rather than defaulting. No silent "agents" fallback anywhere. - Trim the auth-proxy's header sanitization to only what would be actively wrong. Request: host + content-length (httpx recomputes), authorization + x-nmp-principal-id (we stamp the identity; must not be spoofed/conflict). Response: content-length + transfer-encoding (responses are streamed). Dropped the hop-by-hop policing that a trusted loopback sidecar doesn't need. - Tests: replace the identity-default test with a validation-rejection test. Signed-off-by: Ben McCown <bmccown@nvidia.com> * refactor(auth-proxy): close upstream client on shutdown; drop redundant cleanup Addresses review feedback on the auth-proxy sidecar. - Add a FastAPI lifespan that closes the shared httpx AsyncClient on shutdown so its connection pool is released gracefully instead of relying on process exit. - Remove the redundant BackgroundTask(response.aclose): the finally in the streaming generator already runs on normal completion, exception, and client disconnect, so the response is always closed there. httpx's aclose() is idempotent, so the duplicate was harmless but implied cleanup was missing. - Test that the lifespan actually closes the upstream client. Not changed: the request body is still buffered before forwarding. Requests through this proxy are a workload's outbound platform API calls (bounded in size), and streaming them would force chunked transfer-encoding on every request since we strip content-length. Revisit if large payloads are routed through the proxy. Signed-off-by: Ben McCown <bmccown@nvidia.com> --------- Signed-off-by: Ben McCown <bmccown@nvidia.com>
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
3 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.
TL;DR
Deployed agents were calling the wrong URL for inference. Fixed by only ever handing an agent a URL we know is reachable from inside its container.
The bug
inject_gateway_url()bakes the API pod's own base URL into each agent'sllms.*.base_urlat create time. That URL is wrong from inside an agent container:NMP_BASE_URLishttp://localhost:8080(correct for its own PDP self-call). Baked into an agent,localhostis the agent itself → calls never reach the platform.The one mode-aware rewrite that existed only wrote
NMP_GATEWAY_BASE_URL— a dead env var nothing reads — so it never reached the config the agent runs.The fix
Enforce one invariant: an agent is only ever handed a known-good, container-reachable URL — never the raw platform base URL.
resolve_agent_gateway_url()emits only known-good values per mode:NMP_INTERNAL_BASE_URL, set by Helm), or an explicit override. No internal URL → fail fast instead of propagating something unverified.host.docker.internal; other hosts pass through.llms.*.base_urlonto that URL (preserving the IGW path).NEMO_INTERNAL_BASE_URL/NMP_INTERNAL_BASE_URL(andagents.deployments.k8s_internal_base_url); Helm sets it on the api + controller pods.NMP_GATEWAY_BASE_URL.Verify
helm template --set platformConfig.auth.enabled=trueon the API pod:ruff + ty +
make docs-checkclean; unit tests for docker/k8s resolution, the fail-fast path, and config rebasing all pass.Notes
gateway_url_override(documented).nemo_fabric, CLI table assertions) reproduce onmain.Summary by CodeRabbit
New Features
Documentation