fix(helm): use API Service address for NMP_BASE_URL#894
Open
svvarom wants to merge 1 commit into
Open
Conversation
The API pod set NMP_BASE_URL to loopback when the embedded PDP is enabled. That value is copied into agent workflow configs, which run in a separate container, where loopback resolves to the agent itself rather than the platform. Use the in-cluster API Service address instead, matching the controller and seed job. The embedded PDP keeps its own loopback via NMP_AUTH_POLICY_DECISION_POINT_BASE_URL. Adds a template test alongside the existing controller and seed job ones. Signed-off-by: Swarom Muley <smuley@nvidia.com>
Contributor
📝 WalkthroughWalkthroughChangesAPI URL configuration
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 `@tests/auth_idp/static/test_authentik_kubernetes_demo.py`:
- Around line 1181-1197: Extend
test_nemo_platform_api_uses_internal_api_service_url_for_agent_configs to render
the API deployment with authentication/PDP enabled and disabled, rather than
inspecting template text alone. Assert NMP_BASE_URL always uses the internal
service URL, while NMP_AUTH_POLICY_DECISION_POINT_BASE_URL is present only in
the enabled rendering and absent when disabled.
🪄 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: b2ec32e0-0bdf-4c1c-8556-c4da7829634e
📒 Files selected for processing (2)
k8s/helm/templates/api/api-deployment.yamltests/auth_idp/static/test_authentik_kubernetes_demo.py
Comment on lines
+1181
to
+1197
| def test_nemo_platform_api_uses_internal_api_service_url_for_agent_configs() -> None: | ||
| """NMP_BASE_URL on the API pod must be the Service address, never loopback. | ||
|
|
||
| The API copies this value into agent workflow configs, which run in a different | ||
| container -- loopback there resolves to the agent itself, not the platform. | ||
| Embedded PDP gets its loopback from NMP_AUTH_POLICY_DECISION_POINT_BASE_URL instead. | ||
| """ | ||
| template = Path("k8s/helm/templates/api/api-deployment.yaml").read_text(encoding="utf-8") | ||
|
|
||
| assert ( | ||
| 'name: NMP_BASE_URL\n value: {{ include "nemo-platform.internalBaseUrl" . | quote }}' | ||
| ) in template | ||
| assert "nemo-platform.apiLoopbackBaseUrl" not in template.split("NMP_AUTOMODEL")[0] | ||
| assert ( | ||
| "name: NMP_AUTH_POLICY_DECISION_POINT_BASE_URL\n value: " | ||
| '{{ include "nemo-platform.apiLoopbackBaseUrl" . | quote }}' | ||
| ) in template |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover both rendered authentication configurations.
This test only inspects template text; it never renders with authentication/PDP enabled and disabled. Add both render cases and assert that NMP_AUTH_POLICY_DECISION_POINT_BASE_URL is present only when enabled.
🤖 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 `@tests/auth_idp/static/test_authentik_kubernetes_demo.py` around lines 1181 -
1197, Extend
test_nemo_platform_api_uses_internal_api_service_url_for_agent_configs to render
the API deployment with authentication/PDP enabled and disabled, rather than
inspecting template text alone. Assert NMP_BASE_URL always uses the internal
service URL, while NMP_AUTH_POLICY_DECISION_POINT_BASE_URL is present only in
the enabled rendering and absent when disabled.
Contributor
|
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
1 participant
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.
The API pod set
NMP_BASE_URLto loopback when the embedded PDP is enabled. That value is copied into agent workflow configs, which run in a separate container — where loopback resolves to the agent itself, not the platform.Use the in-cluster API Service address instead, matching what the controller and seed job already use. The embedded PDP keeps its own loopback via
NMP_AUTH_POLICY_DECISION_POINT_BASE_URL.Adds a template test, alongside the existing ones for the controller and seed job.
Testing
helm templatewith auth on and off:NMP_BASE_URLis the Service address, PDP var still loopbackhelm lintcleanSummary by CodeRabbit