feat(auth): implement workload identity token exchange - #631
Conversation
|
🌿 Preview your docs: https://nvidia-preview-auth-idp-3-rsadler.docs.buildwithfern.com/nemo-platform |
5175bba to
3795433
Compare
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds RFC 8693 workload identity token exchange end-to-end: a new Auth service token-exchange/JWKS endpoint, SDK/CLI providers for subject-token exchange, Docker/Kubernetes job-backend subject-token issuance and mounting, an HTTPS-enabled Authentik Compose/Kubernetes reference demo, and supporting schema, routing, and test updates. ChangesWorkload Identity Token Exchange
Sequence Diagram(s)sequenceDiagram
participant Job as Managed Job (Docker/K8s)
participant SDK as NeMo Platform SDK
participant Gateway as Envoy Gateway
participant Auth as Auth Service (/apis/auth/token)
participant API as NeMo Platform API
Job->>SDK: read subject token file
SDK->>Gateway: POST /apis/auth/token (subject_token, grant=token-exchange)
Gateway->>Auth: forward request
Auth->>Auth: validate subject token (JWKS / TokenReview)
Auth-->>Gateway: signed access_token
Gateway-->>SDK: access_token
SDK->>Gateway: request with Bearer access_token
Gateway->>API: forward authenticated request
API-->>SDK: response
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (12)
contrib/auth/authentik/kubernetes/postgres.yaml (1)
27-42: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueAdd securityContext to demo Postgres deployment.
Checkov flags missing
allowPrivilegeEscalation: falseand non-root enforcement. Since this is a reference deployment others may copy, worth hardening even though demo-only.🔒 Suggested hardening
containers: - name: postgres image: docker.io/library/postgres:16-alpine + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true envFrom:🤖 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 `@contrib/auth/authentik/kubernetes/postgres.yaml` around lines 27 - 42, The postgres container in the demo deployment is missing basic hardening. Update the postgres container spec to include a securityContext that disables privilege escalation and enforces non-root execution, and keep the change scoped to the postgres workload so the reference deployment is safer to copy. Use the existing postgres container definition in the Kubernetes manifest to add the required security settings.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/workload-token-exchange.yaml (1)
45-73: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd container securityContext for broker.
Same as the gateway deployment: no
securityContext, root/privilege-escalation allowed (Checkov CKV_K8S_20/CKV_K8S_23).🔒 Suggested hardening
containers: - name: broker image: my-registry/nmp-api:local workingDir: /broker command: ["python", "-m", "uvicorn", "workload_exchange_broker:app", "--host", "0.0.0.0", "--port", "8080"] + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true env:🤖 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 `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml` around lines 45 - 73, The broker container spec is missing a securityContext, so it still runs with root/privilege-escalation defaults. Update the container definition in the workload-token-exchange manifest by adding the same hardening used by the gateway deployment, and make sure the broker container explicitly disables privilege escalation and runs with a non-root security context.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/envoy-gateway.yaml (1)
16-26: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd container securityContext.
No
securityContextset; container can run as root with privilege escalation allowed (Checkov CKV_K8S_20/CKV_K8S_23).🔒 Suggested hardening
containers: - name: envoy image: envoyproxy/envoy:v1.33-latest args: ["envoy", "-c", "/etc/envoy/envoy.yaml"] + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true ports:🤖 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 `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml` around lines 16 - 26, The envoy container spec is missing a securityContext, leaving it able to run as root and allow privilege escalation. Update the container definition in the envoy gateway manifest by adding a container-level securityContext for the envoy container with non-root and no privilege escalation settings, and ensure the hardening is applied directly on the envoy container block.Source: Linters/SAST tools
packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py (1)
51-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUpdate docstring: direct mode no longer holds when workload token file env var is set.
Docstring says passing only
base_urlskips config bootstrap and auth header injection. Withhas_workload_identity_token_filenow included inshould_bootstrap, that's no longer true wheneverNMP_WORKLOAD_IDENTITY_TOKEN_FILEis set in the environment — bootstrap (and OIDC discovery) will run even for an explicitbase_url. Update the docs so callers relying on "direct mode" understand the exception.Also applies to: 78-81, 104-111, 199-201, 239-242, 265-272
🤖 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 `@packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py` around lines 51 - 53, The docstrings for enhanced client direct mode are now outdated because `has_workload_identity_token_file` changes `should_bootstrap` behavior. Update the affected docstrings in `EnhancedClient`-related methods so they state that passing only `base_url` skips config bootstrap and auth injection only when `NMP_WORKLOAD_IDENTITY_TOKEN_FILE` is not set; otherwise bootstrap/OIDC discovery still runs. Keep the wording consistent across the referenced docstrings and mention the `should_bootstrap`/`has_workload_identity_token_file` exception explicitly.contrib/auth/authentik/kubernetes/redis.yaml (1)
16-28: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd securityContext to redis container.
No
securityContextset; container can run as root with privilege escalation allowed, per Checkov CKV_K8S_20/CKV_K8S_23.🔒 Proposed fix
containers: - name: redis image: docker.io/library/redis:7-alpine + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + runAsUser: 999 args: ["--save", "60", "1", "--loglevel", "warning"]🤖 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 `@contrib/auth/authentik/kubernetes/redis.yaml` around lines 16 - 28, The redis container in the Kubernetes manifest is missing a securityContext, so it can run as root and allow privilege escalation. Add a container-level securityContext under the redis container definition and configure it to run as a non-root user with privilege escalation disabled, using the redis container block and its pod spec fields to place the change correctly. Ensure the settings satisfy the Checkov CKV_K8S_20 and CKV_K8S_23 requirements without changing the existing container behavior otherwise.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/authentik.yaml (1)
28-31: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueAdd a hardening
securityContextto both deployments.Demos get copied. Add
runAsNonRoot: trueandallowPrivilegeEscalation: falseso the reference doesn't ship a root/privesc-capable pod spec.Proposed securityContext
- name: authentik image: ghcr.io/goauthentik/server:2024.12 args: ["server"] + securityContext: + runAsNonRoot: true + allowPrivilegeEscalation: falseAlso applies to: 82-85
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml` around lines 28 - 31, Add a hardening securityContext to both Authentik deployment pod specs so they do not ship with a root- or privilege-escalation-capable container. Update the pod/container configuration around the authentik container definition to include runAsNonRoot: true and allowPrivilegeEscalation: false, and apply the same change to the second deployment mentioned in the review. Keep the change scoped to the deployment manifests so the existing image and args stay unchanged.Source: Linters/SAST tools
packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (1)
98-99: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider excluding
tokensfrom repr, consistent with_lock.
tokens(containing the live access token) lacksrepr=Falsewhile_lockalready has it — suggests an oversight rather than a deliberate choice. Same class of exposure risk as flagged inworkload_tokens.py.🔒 Proposed fix
- tokens: TokenSet = field(default_factory=lambda: TokenSet(access_token="")) + tokens: TokenSet = field(default_factory=lambda: TokenSet(access_token=""), repr=False)🤖 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 `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` around lines 98 - 99, The `tokens` field in the workload exchange dataclass is still included in the generated repr, unlike `_lock`, which looks like an oversight and can expose live access token data. Update the dataclass definition in `workload_exchange.py` so `tokens` is declared with `repr=False`, matching the existing `_lock` handling and keeping the repr safe.services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py (1)
43-82: 🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoffROPC (password) grant used to obtain the subject token.
RFC 9700 explicitly states "Resource Owner Password Credentials Grant The resource owner password credentials grant [RFC6749] MUST NOT be used." Even labeled "demo," this issuer directly handles raw username/password and normalizes an insecure pattern. If this can be wired into any production/non-demo path, replace with client_credentials or a proper federated flow.
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py` around lines 43 - 82, The OAuthPasswordGrantSubjectTokenIssuer.issue flow is using the insecure password grant and directly handling raw username/password credentials, which should not be used in production paths. Replace this issuer’s ROPC-based request construction and token exchange with a safer flow such as client_credentials or a federated token exchange, and update any callers of OAuthPasswordGrantSubjectTokenIssuer and SubjectToken issuance so they no longer depend on username/password inputs.services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py (1)
127-147: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent fallback hides workload-identity misconfig.
A transient/config error makes
is_workload_identity_token_exchange_enabled()returnFalse, so jobs launch without a subject token and fail auth downstream with no clear signal (debug log only). Log atwarningso operators can diagnose.Proposed change
except Exception: - logger.debug("Could not resolve auth config for workload identity token exchange", exc_info=True) + logger.warning("Could not resolve auth config for workload identity token exchange", exc_info=True) return Falseexcept Exception: - logger.debug("Could not resolve auth config for workload identity audience", exc_info=True) + logger.warning("Could not resolve auth config for workload identity audience", exc_info=True) return "nemo-platform"🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py` around lines 127 - 147, The silent fallback in is_workload_identity_token_exchange_enabled masks workload-identity misconfiguration by returning False on any config resolution error; update this helper to log at warning level instead of debug when get_auth_config() fails, keeping the False fallback but surfacing the problem clearly. Make the same logging-level adjustment in get_workload_identity_token_audience if it follows the same pattern, so both auth-config lookup paths in base.py are visible to operators when they fall back to defaults.services/core/jobs/tests/controllers/test_docker_backend.py (1)
707-747: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRefresher cleanup skipped if assertions fail.
refresher.stop()only runs if every assertion above it passes. Wrap infinallyso background refresher threads aren't leaked on assertion failure.♻️ Proposed fix
- job_create_call = next( - call - for call in docker_client_mock.containers.create.call_args_list - if call.kwargs.get("name") == "job-test-job-id-test-step" - ) - kwargs = job_create_call.kwargs - env = kwargs["environment"] - assert env[WORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR] == WORKLOAD_IDENTITY_TOKEN_FILE_PATH - - mounts = kwargs["mounts"] - workload_identity_mount = next(m for m in mounts if m["Target"] == WORKLOAD_IDENTITY_VOLUME_PATH) - assert workload_identity_mount["Type"] == "volume" - assert workload_identity_mount["Source"].startswith( - f"task-workload-identity-{test_job_step.workspace}-{test_job_step.job}-" - ) - assert workload_identity_mount["ReadOnly"] is True - assert any( - call.kwargs.get("name", "").startswith("workload-token-write-") - for call in docker_client_mock.containers.create.call_args_list - ) - for refresher in list(docker_job._workload_identity_refreshers.values()): - refresher.stop() + try: + job_create_call = next( + call + for call in docker_client_mock.containers.create.call_args_list + if call.kwargs.get("name") == "job-test-job-id-test-step" + ) + kwargs = job_create_call.kwargs + env = kwargs["environment"] + assert env[WORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR] == WORKLOAD_IDENTITY_TOKEN_FILE_PATH + + mounts = kwargs["mounts"] + workload_identity_mount = next(m for m in mounts if m["Target"] == WORKLOAD_IDENTITY_VOLUME_PATH) + assert workload_identity_mount["Type"] == "volume" + assert workload_identity_mount["Source"].startswith( + f"task-workload-identity-{test_job_step.workspace}-{test_job_step.job}-" + ) + assert workload_identity_mount["ReadOnly"] is True + assert any( + call.kwargs.get("name", "").startswith("workload-token-write-") + for call in docker_client_mock.containers.create.call_args_list + ) + finally: + for refresher in list(docker_job._workload_identity_refreshers.values()): + refresher.stop()🤖 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 `@services/core/jobs/tests/controllers/test_docker_backend.py` around lines 707 - 747, The test in test_docker_job_injects_workload_identity_volume_when_token_exchange_enabled leaks background refresher threads if any assertion fails before the cleanup loop. Update the test so the docker_job._workload_identity_refreshers cleanup runs from a finally block around the assertions and docker_client_mock checks, ensuring each refresher.stop() is always called even when an assertion aborts the test.contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py (2)
86-134: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSynchronous blocking network call inside an async handler.
_decode_kubernetes_subject_token(called from the asynctoken_exchangeroute) performs a blockingurllib.request.urlopenwith a 10s timeout directly on the event loop, stalling all other concurrent requests to the broker for up to 10s.Run it off the event loop, e.g.
await asyncio.to_thread(_decode_kubernetes_subject_token, subject_token, audience), or switch to an async HTTP client.Also applies to: 164-186
🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` around lines 86 - 134, The blocking TokenReview call in _decode_kubernetes_subject_token is being executed from the async token_exchange flow, which can stall the event loop. Move the synchronous urllib.request.urlopen work off the loop by calling _decode_kubernetes_subject_token via asyncio.to_thread from token_exchange, or refactor the TokenReview request to use an async HTTP client. Make sure the async route no longer waits on a direct blocking network call while preserving the existing token validation and error handling in _decode_kubernetes_subject_token.
69-83: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSubject token
audis never validated.
options={"verify_aud": False}and noaudience=passed means the exchanged subject token's audience claim isn't checked, relying only on the issuer allowlist. For a reference implementation others may copy, verifyaudtoo.🔒 Proposed fix
claims = jwt.decode( subject_token, signing_key, algorithms=["RS256"], - options={"verify_aud": False}, + audience=AUDIENCE, leeway=30, )🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` around lines 69 - 83, The subject token audience is not being checked in _decode_authentik_subject_token, so update the jwt.decode call to validate the token’s aud claim instead of disabling audience verification. Add an explicit expected audience value (or allowlist) to the decode path, keep the issuer check in place, and make sure the validation happens in the same auth flow used by _authentik_jwks_client so copied reference implementations enforce both iss and aud.
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 1122-1124: The existing-container branch in DockerBackend should
not start the workload identity refresher early, because the same
SubjectTokenRefreshLoop is started again later and may be stopped immediately or
left running on early exits. Remove the
self._start_workload_identity_refresher(container.name,
workload_identity_refresher) call from the container-is-not-none path, and keep
the refresher startup centralized in the later docker backend flow so resumed
jobs still get token refresh correctly.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Around line 132-144: The _run() loop in workload_tokens.py retries
refresh_once() after a fixed _min_sleep_seconds on every exception, which can
hammer the token endpoint during persistent failures. Update _run() to use an
exponential or capped backoff after repeated refresh failures, resetting the
backoff after a successful refresh, and keep the existing logger.exception call
so the retry behavior in WorkloadToken refresh remains clear.
- Around line 42-53: The OAuthPasswordGrantSubjectTokenIssuer dataclass
currently exposes sensitive credentials in its default repr, unlike the _lock
handling elsewhere in the diff. Update the class definition so password and
client_secret are excluded from repr (similar to how other sensitive/internal
fields are handled), and keep the rest of the dataclass behavior unchanged. This
should be done in the OAuthPasswordGrantSubjectTokenIssuer symbol so accidental
logging or traceback inspection does not reveal secrets.
---
Nitpick comments:
In `@contrib/auth/authentik/kubernetes/authentik.yaml`:
- Around line 28-31: Add a hardening securityContext to both Authentik
deployment pod specs so they do not ship with a root- or
privilege-escalation-capable container. Update the pod/container configuration
around the authentik container definition to include runAsNonRoot: true and
allowPrivilegeEscalation: false, and apply the same change to the second
deployment mentioned in the review. Keep the change scoped to the deployment
manifests so the existing image and args stay unchanged.
In `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml`:
- Around line 16-26: The envoy container spec is missing a securityContext,
leaving it able to run as root and allow privilege escalation. Update the
container definition in the envoy gateway manifest by adding a container-level
securityContext for the envoy container with non-root and no privilege
escalation settings, and ensure the hardening is applied directly on the envoy
container block.
In `@contrib/auth/authentik/kubernetes/postgres.yaml`:
- Around line 27-42: The postgres container in the demo deployment is missing
basic hardening. Update the postgres container spec to include a securityContext
that disables privilege escalation and enforces non-root execution, and keep the
change scoped to the postgres workload so the reference deployment is safer to
copy. Use the existing postgres container definition in the Kubernetes manifest
to add the required security settings.
In `@contrib/auth/authentik/kubernetes/redis.yaml`:
- Around line 16-28: The redis container in the Kubernetes manifest is missing a
securityContext, so it can run as root and allow privilege escalation. Add a
container-level securityContext under the redis container definition and
configure it to run as a non-root user with privilege escalation disabled, using
the redis container block and its pod spec fields to place the change correctly.
Ensure the settings satisfy the Checkov CKV_K8S_20 and CKV_K8S_23 requirements
without changing the existing container behavior otherwise.
In `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml`:
- Around line 45-73: The broker container spec is missing a securityContext, so
it still runs with root/privilege-escalation defaults. Update the container
definition in the workload-token-exchange manifest by adding the same hardening
used by the gateway deployment, and make sure the broker container explicitly
disables privilege escalation and runs with a non-root security context.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 86-134: The blocking TokenReview call in
_decode_kubernetes_subject_token is being executed from the async token_exchange
flow, which can stall the event loop. Move the synchronous
urllib.request.urlopen work off the loop by calling
_decode_kubernetes_subject_token via asyncio.to_thread from token_exchange, or
refactor the TokenReview request to use an async HTTP client. Make sure the
async route no longer waits on a direct blocking network call while preserving
the existing token validation and error handling in
_decode_kubernetes_subject_token.
- Around line 69-83: The subject token audience is not being checked in
_decode_authentik_subject_token, so update the jwt.decode call to validate the
token’s aud claim instead of disabling audience verification. Add an explicit
expected audience value (or allowlist) to the decode path, keep the issuer check
in place, and make sure the validation happens in the same auth flow used by
_authentik_jwks_client so copied reference implementations enforce both iss and
aud.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 98-99: The `tokens` field in the workload exchange dataclass is
still included in the generated repr, unlike `_lock`, which looks like an
oversight and can expose live access token data. Update the dataclass definition
in `workload_exchange.py` so `tokens` is declared with `repr=False`, matching
the existing `_lock` handling and keeping the repr safe.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py`:
- Around line 51-53: The docstrings for enhanced client direct mode are now
outdated because `has_workload_identity_token_file` changes `should_bootstrap`
behavior. Update the affected docstrings in `EnhancedClient`-related methods so
they state that passing only `base_url` skips config bootstrap and auth
injection only when `NMP_WORKLOAD_IDENTITY_TOKEN_FILE` is not set; otherwise
bootstrap/OIDC discovery still runs. Keep the wording consistent across the
referenced docstrings and mention the
`should_bootstrap`/`has_workload_identity_token_file` exception explicitly.
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py`:
- Around line 127-147: The silent fallback in
is_workload_identity_token_exchange_enabled masks workload-identity
misconfiguration by returning False on any config resolution error; update this
helper to log at warning level instead of debug when get_auth_config() fails,
keeping the False fallback but surfacing the problem clearly. Make the same
logging-level adjustment in get_workload_identity_token_audience if it follows
the same pattern, so both auth-config lookup paths in base.py are visible to
operators when they fall back to defaults.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Around line 43-82: The OAuthPasswordGrantSubjectTokenIssuer.issue flow is
using the insecure password grant and directly handling raw username/password
credentials, which should not be used in production paths. Replace this issuer’s
ROPC-based request construction and token exchange with a safer flow such as
client_credentials or a federated token exchange, and update any callers of
OAuthPasswordGrantSubjectTokenIssuer and SubjectToken issuance so they no longer
depend on username/password inputs.
In `@services/core/jobs/tests/controllers/test_docker_backend.py`:
- Around line 707-747: The test in
test_docker_job_injects_workload_identity_volume_when_token_exchange_enabled
leaks background refresher threads if any assertion fails before the cleanup
loop. Update the test so the docker_job._workload_identity_refreshers cleanup
runs from a finally block around the assertions and docker_client_mock checks,
ensuring each refresher.stop() is always called even when an assertion aborts
the test.
🪄 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: edca8b4b-086b-4ba6-b917-e737ef102df0
⛔ Files ignored due to path filters (9)
sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**
📒 Files selected for processing (62)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
contrib/auth/authentik/kubernetes/README.md (1)
82-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing "Next Steps" section.
File ends after the preflight section with no cross-links to related docs (e.g., Compose demo README, SDK workload identity docs).
As per coding guidelines: "Include 'Next Steps' section at the end with cross-links to related documentation content."
🤖 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 `@contrib/auth/authentik/kubernetes/README.md` around lines 82 - 101, The README currently ends after the Token Exchange Preflight section and is missing the required Next Steps cross-links. Add a “Next Steps” section at the end of the Authentik Kubernetes README and include links to related documentation such as the Compose demo README and the SDK workload identity docs, using the existing README structure and headings to place it after the preflight content.Source: Coding guidelines
packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
from __future__ import annotationsmakes all hints string-based.All imports here are already unconditional (no
TYPE_CHECKINGgating), so this import isn't strictly needed and turns concrete annotations into deferred strings, contrary to the guideline preference for concrete type hints.As per coding guidelines: "In Python code, prefer concrete type hints over string-based type hints, and do not import those types only under
TYPE_CHECKING; import them normally when possible."🤖 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 `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` at line 6, Remove the unnecessary future annotations import from workload_exchange.py so type hints remain concrete instead of string-based. Keep the existing imports used by the module as normal runtime imports, and ensure any annotations in the relevant functions or classes continue to reference the imported types directly rather than relying on deferred evaluation.Source: Coding guidelines
contrib/auth/authentik/kubernetes/postgres.yaml (1)
37-42: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPostgres data on
emptyDir— lost on any pod restart/reschedule.Authentik's entire state (users, blueprints, tokens) is wiped whenever this pod is rescheduled. Consider a
PersistentVolumeClaimso the demo survives restarts.Suggested fix
volumeMounts: - name: data mountPath: /var/lib/postgresql/data volumes: - - name: data - emptyDir: {} + - name: data + persistentVolumeClaim: + claimName: authentik-postgres-data🤖 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 `@contrib/auth/authentik/kubernetes/postgres.yaml` around lines 37 - 42, The Postgres storage in the Authentik Kubernetes manifest is using emptyDir in the postgres.yaml workload, which makes all database state disappear on pod restart or reschedule. Update the volume definition for the Postgres container to use a PersistentVolumeClaim instead of emptyDir, and ensure the existing volumeMounts for /var/lib/postgresql/data continue to point to that persistent volume. Keep the change scoped to the Postgres deployment/stateful resource so the database survives pod rescheduling.contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py (1)
46-46: 🩺 Stability & Availability | 🔵 TrivialSigning key regenerated on every restart; not scoped for >1 replica.
_private_keyis generated fresh at process start with no persistence. Any restart invalidates outstanding access tokens, and scaling beyondreplicas: 1would produce inconsistent JWKS across pods. Fine for the current single-replica demo; worth a comment/README note if this is ever scaled.🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` at line 46, The signing key is currently generated at process startup in workload_exchange_broker.py via _private_key, which means restarts invalidate issued tokens and multiple replicas will expose different JWKS. Either persist and share the key across restarts/pods or, if this is intentionally demo-only, add a clear comment/README note near _private_key and the JWT/JWKS setup in this module stating it is only safe for a single-replica deployment and not for scaled use.
🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 88-136: Both _decode_kubernetes_subject_token and
_decode_authentik_subject_token are doing blocking network I/O inside the async
token_exchange request path, which can stall the event loop. Move the
TokenReview and JWKS retrieval work off the event loop by making these fetches
asynchronous or running the blocking calls in a worker thread, and update
token_exchange to await the async helpers instead of calling them directly.
- Around line 37-44: The issuer allowlist currently fails open when
ALLOWED_SUBJECT_ISSUERS is empty, so the issuer validation in the workload
exchange path must not be skipped in that case. Update the issuer check logic
around ALLOWED_SUBJECT_ISSUERS so an empty set is treated as “reject all” (or
otherwise explicitly handled as invalid), and make the same fix anywhere the
issuer comparison is repeated in the workload exchange broker flow. Use the
ALLOWED_SUBJECT_ISSUERS symbol as the central source of truth and ensure the
condition in the issuer validation path always enforces the allowlist.
- Around line 190-199: The workload exchange flow in workload_exchange_broker.py
can raise an uncaught KeyError when building exchanged_claims because
subject_claims["sub"] is accessed after token decoding without validation.
Update the logic around the token exchange path to verify that subject_claims
contains a non-empty sub before constructing exchanged_claims, and if it is
missing, return the same sanitized invalid_grant error path used for other token
validation failures. Keep the fix localized near the existing subject_claims,
exchanged_claims, and invalid_grant handling so the exception never escapes the
try/except flow.
In `@openapi/openapi.yaml`:
- Around line 12575-12585: The workload identity token expiration fields
currently allow invalid values below Kubernetes’ minimum, so update both schema
entries for workload_identity_token_expiration_seconds in the OpenAPI definition
to enforce a minimum of 600. Make this change wherever the duplicated field
definition appears so API validation rejects unsupported
projected-service-account token expirations before job submission.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Around line 116-126: The start/stop lifecycle in the workload token refresher
is broken because stop() leaves _thread set, so start()’s guard in the same
class silently prevents restarting after a stop. Update the thread management in
start() and stop() for the workload token refresh loop so a stopped thread
clears _thread (and any needed stop state) after join, and make the start()
check use thread liveness rather than only whether _thread is non-None.
In
`@services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py`:
- Around line 33-40: The test is mocking out NeMoPlatform while also setting
NMP_WORKLOAD_IDENTITY_TOKEN_FILE, so the workload-token discovery and exchange
path is never actually exercised. Update test_workload_workspace_get_task to
either use a real temporary token file and let
nmp.hello_world.tasks.workload_workspace_get.run.NeMoPlatform run normally, or
remove the env var from this test and add a separate integration test for the
workload-token flow. Keep the assertions on task_run and
sdk.workspaces.requested, but make sure the test validates the real
discovery/provider behavior instead of a mocked constructor.
---
Nitpick comments:
In `@contrib/auth/authentik/kubernetes/postgres.yaml`:
- Around line 37-42: The Postgres storage in the Authentik Kubernetes manifest
is using emptyDir in the postgres.yaml workload, which makes all database state
disappear on pod restart or reschedule. Update the volume definition for the
Postgres container to use a PersistentVolumeClaim instead of emptyDir, and
ensure the existing volumeMounts for /var/lib/postgresql/data continue to point
to that persistent volume. Keep the change scoped to the Postgres
deployment/stateful resource so the database survives pod rescheduling.
In `@contrib/auth/authentik/kubernetes/README.md`:
- Around line 82-101: The README currently ends after the Token Exchange
Preflight section and is missing the required Next Steps cross-links. Add a
“Next Steps” section at the end of the Authentik Kubernetes README and include
links to related documentation such as the Compose demo README and the SDK
workload identity docs, using the existing README structure and headings to
place it after the preflight content.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Line 46: The signing key is currently generated at process startup in
workload_exchange_broker.py via _private_key, which means restarts invalidate
issued tokens and multiple replicas will expose different JWKS. Either persist
and share the key across restarts/pods or, if this is intentionally demo-only,
add a clear comment/README note near _private_key and the JWT/JWKS setup in this
module stating it is only safe for a single-replica deployment and not for
scaled use.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Line 6: Remove the unnecessary future annotations import from
workload_exchange.py so type hints remain concrete instead of string-based. Keep
the existing imports used by the module as normal runtime imports, and ensure
any annotations in the relevant functions or classes continue to reference the
imported types directly rather than relying on deferred evaluation.
🪄 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: 7f7bb167-2386-41ce-8798-345e31ddb507
⛔ Files ignored due to path filters (13)
sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (66)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
✅ Files skipped from review due to trivial changes (6)
- tests/auth_idp/test_docs_links.py
- contrib/auth/authentik/.gitignore
- docs/auth/deployment/configuration.mdx
- contrib/auth/authentik/kubernetes/namespace.yaml
- docs/set-up/config-reference.mdx
- docs/auth/authentication/idp-integration.mdx
🚧 Files skipped from review as they are similar to previous changes (36)
- tests/auth_idp/test_fixture_helpers.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- tests/auth_idp/test_authentik_blueprint.py
- contrib/auth/manifest.schema.yaml
- packages/nmp_common/src/nmp/common/config/base.py
- contrib/auth/authentik/kustomization.yaml
- services/core/auth/tests/test_discovery.py
- contrib/auth/authentik/gateway/envoy.yaml
- tests/auth_idp/test_provider_manifest.py
- contrib/auth/authentik/manifest.yaml
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_ext/tests/config/test_config.py
- docs/auth/deployment/credential-propagation.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- contrib/auth/authentik/docker-compose.yml
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- contrib/auth/authentik/README.md
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- services/core/jobs/tests/controllers/test_workload_tokens.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- packages/nemo_platform_plugin/tests/test_client_auth.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
- contrib/auth/authentik/run.sh
- services/core/jobs/jobs-launcher/cmd/run.go
3795433 to
1abaee3
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py (2)
955-971: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClean up task volumes when initial token issuance fails.
If refresher construction or
refresh_once()raises, scheduling exits after creating all task volumes. No container exists for cleanup discovery, so each failed attempt leaks volumes.Proposed cleanup
- workload_identity_refresher = None - if workload_identity_volume_name is not None: - workload_identity_refresher = self._build_workload_identity_refresher(workload_identity_volume_name) - workload_identity_refresher.refresh_once() + try: + workload_identity_refresher = None + if workload_identity_volume_name is not None: + workload_identity_refresher = self._build_workload_identity_refresher(workload_identity_volume_name) + workload_identity_refresher.refresh_once() + except Exception: + self.cleanup_task_storage_volumes(step.workspace, step.job, task_id) + raise🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 955 - 971, Ensure task volumes are cleaned up when workload identity refresher construction or its initial refresh fails. In the code following ensure_job_storage, wrap _build_workload_identity_refresher and refresh_once in exception handling that removes the newly created task volumes through the existing cleanup mechanism before re-raising the scheduling error; preserve the normal refresher assignment and successful path.
947-971: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReuse the existing container’s workload-identity volume on resume.
Every scheduling attempt generates a new task ID and refresher before checking for an existing container. The resumed container remains mounted to its original volume, while refreshes target an unattached new volume. Resolve the existing container and its task label before provisioning token storage.
Also applies to: 1161-1161
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 947 - 971, Resolve any existing container and read its original task label before generating or provisioning workload-identity storage. Update the scheduling/resume logic around ensure_job_storage and _build_workload_identity_refresher to reuse the existing container’s workload-identity volume and task identity; only create a new task ID, volume, and refresher when no container exists.
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml`:
- Around line 13-117: Add container-level securityContext hardening to both the
authentik-server and authentik-worker containers: set allowPrivilegeEscalation
to false and runAsNonRoot to true, preserving the existing container
configuration.
In `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml`:
- Around line 16-30: Add a container-level securityContext to the envoy
container in the Deployment, setting allowPrivilegeEscalation to false and
runAsNonRoot to true, matching the hardening used in authentik.yaml.
In `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml`:
- Around line 43-49: Add a securityContext to the broker container in the
workload-token-exchange pod: run as the image’s non-root user, set
allowPrivilegeEscalation to false, drop all Linux capabilities, and configure
seccompProfile.type as RuntimeDefault. Place these settings alongside the
container fields identified by name broker.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 183-195: Validate the requested audience against an explicit
allowlist before calling _decode_subject_token or signing the exchanged token.
In the audience handling within the token exchange flow, reject unsupported
values with the existing _oauth_error response (for example, invalid_target or
invalid_request) and only copy an allowlisted audience into
exchanged_claims["aud"].
- Around line 75-84: Validate the subject token’s audience in the JWT decode
path instead of disabling audience verification with options={"verify_aud":
False}. In the Authentik exchange flow around jwt.decode and the issuer
allowlist check, require the configured expected audience (or explicitly compare
the decoded aud claim against it) and reject mismatches before exchanging the
token.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 46-70: Validate token_endpoint in token_exchange_grant before
calling httpx.post(): require HTTPS, allowing HTTP only for loopback hosts, and
reject all other non-HTTPS endpoints before transmitting subject_token. Reuse
the same endpoint-validation policy and symbols as the plugin implementation
where available.
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 321-337: The token-writing command in
_write_workload_identity_subject_token currently applies mode 0400, leaving the
root-owned token unreadable to non-root workloads. Update the command to chown
the token to the configured workload UID/GID, or apply an appropriate readable
mode for this dedicated volume, while preserving secure permissions.
- Line 258: The controller currently initializes _workload_identity_refreshers
empty, so active containers lose token refreshes after restart. During
controller startup, restore a SubjectTokenRefreshLoop for every existing ACTIVE
container and register it in _workload_identity_refreshers, reusing the same
scheduling/setup logic used for newly scheduled containers.
- Around line 404-427: Update _is_container_owned_by_this_controller, kill/stop
handling, and cleanup filtering to support legacy containers missing
JOB_CONTROLLER_INSTANCE_ID_LABEL. Preserve the strict owner-label checks for
newly created containers, but add a safe adoption path for matching legacy
containers identified by the existing controller/backend/profile criteria,
allowing them to be stopped, killed, and cleaned up during upgrades.
---
Outside diff comments:
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 955-971: Ensure task volumes are cleaned up when workload identity
refresher construction or its initial refresh fails. In the code following
ensure_job_storage, wrap _build_workload_identity_refresher and refresh_once in
exception handling that removes the newly created task volumes through the
existing cleanup mechanism before re-raising the scheduling error; preserve the
normal refresher assignment and successful path.
- Around line 947-971: Resolve any existing container and read its original task
label before generating or provisioning workload-identity storage. Update the
scheduling/resume logic around ensure_job_storage and
_build_workload_identity_refresher to reuse the existing container’s
workload-identity volume and task identity; only create a new task ID, volume,
and refresher when no container exists.
🪄 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: 91cfadde-884e-4c7b-982b-a56e5aa08a18
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (68)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (6)
- contrib/auth/authentik/kustomization.yaml
- docs/auth/authentication/idp-integration.mdx
- contrib/auth/authentik/kubernetes/namespace.yaml
- contrib/auth/authentik/.gitignore
- docs/set-up/config-reference.mdx
- docs/auth/deployment/configuration.mdx
🚧 Files skipped from review as they are similar to previous changes (40)
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/test_fixture_helpers.py
- contrib/auth/manifest.schema.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- contrib/auth/authentik/gateway/envoy.yaml
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- packages/nmp_common/src/nmp/common/config/base.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- tests/auth_idp/test_provider_manifest.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- contrib/auth/authentik/docker-compose.yml
- services/core/auth/tests/test_discovery.py
- tests/auth_idp/test_authentik_blueprint.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- contrib/auth/authentik/blueprints/nemo.yaml
- services/core/jobs/jobs-launcher/cmd/run.go
- packages/nemo_platform_plugin/tests/test_client_auth.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- contrib/auth/authentik/manifest.yaml
- services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py
- docs/auth/deployment/credential-propagation.mdx
- services/core/jobs/tests/controllers/test_workload_tokens.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- openapi/ga/individual/platform.openapi.yaml
- packages/nemo_platform_ext/tests/config/test_config.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- contrib/auth/authentik/README.md
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- openapi/openapi.yaml
- contrib/auth/authentik/run.sh
- openapi/ga/openapi.yaml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py (2)
955-971: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClean up task volumes when initial token issuance fails.
If refresher construction or
refresh_once()raises, scheduling exits after creating all task volumes. No container exists for cleanup discovery, so each failed attempt leaks volumes.Proposed cleanup
- workload_identity_refresher = None - if workload_identity_volume_name is not None: - workload_identity_refresher = self._build_workload_identity_refresher(workload_identity_volume_name) - workload_identity_refresher.refresh_once() + try: + workload_identity_refresher = None + if workload_identity_volume_name is not None: + workload_identity_refresher = self._build_workload_identity_refresher(workload_identity_volume_name) + workload_identity_refresher.refresh_once() + except Exception: + self.cleanup_task_storage_volumes(step.workspace, step.job, task_id) + raise🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 955 - 971, Ensure task volumes are cleaned up when workload identity refresher construction or its initial refresh fails. In the code following ensure_job_storage, wrap _build_workload_identity_refresher and refresh_once in exception handling that removes the newly created task volumes through the existing cleanup mechanism before re-raising the scheduling error; preserve the normal refresher assignment and successful path.
947-971: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReuse the existing container’s workload-identity volume on resume.
Every scheduling attempt generates a new task ID and refresher before checking for an existing container. The resumed container remains mounted to its original volume, while refreshes target an unattached new volume. Resolve the existing container and its task label before provisioning token storage.
Also applies to: 1161-1161
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 947 - 971, Resolve any existing container and read its original task label before generating or provisioning workload-identity storage. Update the scheduling/resume logic around ensure_job_storage and _build_workload_identity_refresher to reuse the existing container’s workload-identity volume and task identity; only create a new task ID, volume, and refresher when no container exists.
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml`:
- Around line 13-117: Add container-level securityContext hardening to both the
authentik-server and authentik-worker containers: set allowPrivilegeEscalation
to false and runAsNonRoot to true, preserving the existing container
configuration.
In `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml`:
- Around line 16-30: Add a container-level securityContext to the envoy
container in the Deployment, setting allowPrivilegeEscalation to false and
runAsNonRoot to true, matching the hardening used in authentik.yaml.
In `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml`:
- Around line 43-49: Add a securityContext to the broker container in the
workload-token-exchange pod: run as the image’s non-root user, set
allowPrivilegeEscalation to false, drop all Linux capabilities, and configure
seccompProfile.type as RuntimeDefault. Place these settings alongside the
container fields identified by name broker.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 183-195: Validate the requested audience against an explicit
allowlist before calling _decode_subject_token or signing the exchanged token.
In the audience handling within the token exchange flow, reject unsupported
values with the existing _oauth_error response (for example, invalid_target or
invalid_request) and only copy an allowlisted audience into
exchanged_claims["aud"].
- Around line 75-84: Validate the subject token’s audience in the JWT decode
path instead of disabling audience verification with options={"verify_aud":
False}. In the Authentik exchange flow around jwt.decode and the issuer
allowlist check, require the configured expected audience (or explicitly compare
the decoded aud claim against it) and reject mismatches before exchanging the
token.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 46-70: Validate token_endpoint in token_exchange_grant before
calling httpx.post(): require HTTPS, allowing HTTP only for loopback hosts, and
reject all other non-HTTPS endpoints before transmitting subject_token. Reuse
the same endpoint-validation policy and symbols as the plugin implementation
where available.
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 321-337: The token-writing command in
_write_workload_identity_subject_token currently applies mode 0400, leaving the
root-owned token unreadable to non-root workloads. Update the command to chown
the token to the configured workload UID/GID, or apply an appropriate readable
mode for this dedicated volume, while preserving secure permissions.
- Line 258: The controller currently initializes _workload_identity_refreshers
empty, so active containers lose token refreshes after restart. During
controller startup, restore a SubjectTokenRefreshLoop for every existing ACTIVE
container and register it in _workload_identity_refreshers, reusing the same
scheduling/setup logic used for newly scheduled containers.
- Around line 404-427: Update _is_container_owned_by_this_controller, kill/stop
handling, and cleanup filtering to support legacy containers missing
JOB_CONTROLLER_INSTANCE_ID_LABEL. Preserve the strict owner-label checks for
newly created containers, but add a safe adoption path for matching legacy
containers identified by the existing controller/backend/profile criteria,
allowing them to be stopped, killed, and cleaned up during upgrades.
---
Outside diff comments:
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 955-971: Ensure task volumes are cleaned up when workload identity
refresher construction or its initial refresh fails. In the code following
ensure_job_storage, wrap _build_workload_identity_refresher and refresh_once in
exception handling that removes the newly created task volumes through the
existing cleanup mechanism before re-raising the scheduling error; preserve the
normal refresher assignment and successful path.
- Around line 947-971: Resolve any existing container and read its original task
label before generating or provisioning workload-identity storage. Update the
scheduling/resume logic around ensure_job_storage and
_build_workload_identity_refresher to reuse the existing container’s
workload-identity volume and task identity; only create a new task ID, volume,
and refresher when no container exists.
🪄 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: 91cfadde-884e-4c7b-982b-a56e5aa08a18
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (68)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (6)
- contrib/auth/authentik/kustomization.yaml
- docs/auth/authentication/idp-integration.mdx
- contrib/auth/authentik/kubernetes/namespace.yaml
- contrib/auth/authentik/.gitignore
- docs/set-up/config-reference.mdx
- docs/auth/deployment/configuration.mdx
🚧 Files skipped from review as they are similar to previous changes (40)
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/test_fixture_helpers.py
- contrib/auth/manifest.schema.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- contrib/auth/authentik/gateway/envoy.yaml
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- packages/nmp_common/src/nmp/common/config/base.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- tests/auth_idp/test_provider_manifest.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- contrib/auth/authentik/docker-compose.yml
- services/core/auth/tests/test_discovery.py
- tests/auth_idp/test_authentik_blueprint.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- contrib/auth/authentik/blueprints/nemo.yaml
- services/core/jobs/jobs-launcher/cmd/run.go
- packages/nemo_platform_plugin/tests/test_client_auth.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- contrib/auth/authentik/manifest.yaml
- services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py
- docs/auth/deployment/credential-propagation.mdx
- services/core/jobs/tests/controllers/test_workload_tokens.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- openapi/ga/individual/platform.openapi.yaml
- packages/nemo_platform_ext/tests/config/test_config.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- contrib/auth/authentik/README.md
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- openapi/openapi.yaml
- contrib/auth/authentik/run.sh
- openapi/ga/openapi.yaml
🛑 Comments failed to post (9)
contrib/auth/authentik/kubernetes/authentik.yaml (1)
13-117: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add container
securityContexthardening.Neither
authentik-servernorauthentik-workersetsallowPrivilegeEscalation: falseorrunAsNonRoot: true. Since this is a reference others copy into real deployments, bake in the hardening now.🔒 Proposed fix
- name: authentik image: ghcr.io/goauthentik/server:2024.12 args: ["server"] + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + capabilities: + drop: ["ALL"] env:🧰 Tools
🪛 Checkov (3.3.2)
[medium] 13-66: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 13-66: Minimize the admission of root containers
(CKV_K8S_23)
[medium] 67-117: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 67-117: Minimize the admission of root containers
(CKV_K8S_23)
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml` around lines 13 - 117, Add container-level securityContext hardening to both the authentik-server and authentik-worker containers: set allowPrivilegeEscalation to false and runAsNonRoot to true, preserving the existing container configuration.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/envoy-gateway.yaml (1)
16-30: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add container
securityContexthardening.Same gap as
authentik.yaml: noallowPrivilegeEscalation: false/runAsNonRoot: trueon the envoy container.🔒 Proposed fix
- name: envoy image: envoyproxy/envoy:v1.33-latest args: ["envoy", "-c", "/etc/envoy/envoy.yaml"] + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + capabilities: + drop: ["ALL"] ports:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.containers: - name: envoy image: envoyproxy/envoy:v1.33-latest args: ["envoy", "-c", "/etc/envoy/envoy.yaml"] securityContext: allowPrivilegeEscalation: false runAsNonRoot: true capabilities: drop: ["ALL"] ports: - name: http containerPort: 8080 volumeMounts: - name: config mountPath: /etc/envoy readOnly: true volumes: - name: config configMap: name: authentik-envoy-config🤖 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 `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml` around lines 16 - 30, Add a container-level securityContext to the envoy container in the Deployment, setting allowPrivilegeEscalation to false and runAsNonRoot to true, matching the hardening used in authentik.yaml.Source: Linters/SAST tools
contrib/auth/authentik/kubernetes/workload-token-exchange.yaml (1)
43-49: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== file listing ==" git ls-files contrib/auth/authentik/kubernetes echo echo "== outline ==" ast-grep outline contrib/auth/authentik/kubernetes/workload-token-exchange.yaml --view expanded || true echo echo "== relevant file with line numbers ==" cat -n contrib/auth/authentik/kubernetes/workload-token-exchange.yaml | sed -n '1,220p' echo echo "== search for securityContext in same area ==" rg -n "securityContext|runAsNonRoot|allowPrivilegeEscalation|capabilities|seccompProfile" contrib/auth/authentik/kubernetes -S || trueRepository: NVIDIA-NeMo/nemo-platform
Length of output: 3998
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== search image/broker references ==" rg -n "nmp-api:local|workload_exchange_broker|workload-token-exchange-broker|securityContext" -S . echo echo "== relevant README ==" cat -n contrib/auth/authentik/kubernetes/README.md | sed -n '1,220p' echo echo "== other k8s manifests with container securityContext ==" rg -n "securityContext:" contrib/auth/authentik/kubernetes -n -SRepository: NVIDIA-NeMo/nemo-platform
Length of output: 17423
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== auth run/docker files ==" git ls-files contrib/auth/authentik | sed -n '1,200p' echo echo "== docker-compose broker service ==" cat -n contrib/auth/authentik/docker-compose.yml | sed -n '1,180p' echo echo "== image build context references ==" rg -n "FROM |USER |nmp-api|workload-exchange-broker|workload-token-exchange" contrib/auth/authentik -SRepository: NVIDIA-NeMo/nemo-platform
Length of output: 13037
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== nmp-api image build files ==" rg -n "nmp-api|FROM .*python|USER " -S . --glob 'Dockerfile*' --glob '*.dockerfile' --glob 'compose*.yml' --glob '*.yaml' --glob '*.yml' | sed -n '1,240p' echo echo "== likely Dockerfile candidates ==" fd -a -t f 'Dockerfile*|*.dockerfile' . || trueRepository: NVIDIA-NeMo/nemo-platform
Length of output: 12145
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== docker/Dockerfile.nmp-api (surrounding lines) ==" cat -n docker/Dockerfile.nmp-api | sed -n '1,180p' echo echo "== workload exchange broker ==" cat -n contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py | sed -n '1,240p'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 14318
Add a broker securityContext. Run as the image’s non-root user and disable privilege escalation; drop all capabilities and set
seccompProfile: RuntimeDefault. This pod handles subject tokens and has TokenReview access.🤖 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 `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml` around lines 43 - 49, Add a securityContext to the broker container in the workload-token-exchange pod: run as the image’s non-root user, set allowPrivilegeEscalation to false, drop all Linux capabilities, and configure seccompProfile.type as RuntimeDefault. Place these settings alongside the container fields identified by name broker.Source: Linters/SAST tools
contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py (2)
75-84: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the file and inspect the relevant section with line numbers. sed -n '1,220p' contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py | cat -n # Find any subject-token audience handling or related validation in this broker. rg -n "aud|audience|issuer|subject_token|verify_aud|InvalidAudienceError" contrib/auth/authentik/workload-exchange-broker -S # Inspect nearby auth/config code for any configured expected audience. rg -n "ALLOWED_SUBJECT_ISSUERS|allowed.*aud|subject.*aud|expected.*aud|audience" contrib/auth/authentik -SRepository: NVIDIA-NeMo/nemo-platform
Length of output: 15885
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the Authentik-related manifest and docs for any subject-token audience requirement. sed -n '1,220p' contrib/auth/authentik/manifest.yaml | cat -n printf '\n---\n' sed -n '1,220p' contrib/auth/authentik/kubernetes/README.md | cat -n printf '\n---\n' sed -n '1,220p' contrib/auth/authentik/kubernetes/workload-token-exchange.yaml | cat -n printf '\n---\n' sed -n '1,200p' contrib/auth/authentik/gateway/envoy.yaml | cat -nRepository: NVIDIA-NeMo/nemo-platform
Length of output: 16730
🏁 Script executed:
#!/bin/bash set -euo pipefail # Search for any explicit subject-token audience requirement or exception. rg -n "subject token|subject_token|audience.*subject|validate.*aud|verify_aud|token exchange" contrib/auth/authentik -S # Read the main Authentik README if present. fd -a -g 'README.md' contrib/auth/authentikRepository: NVIDIA-NeMo/nemo-platform
Length of output: 5738
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,140p' contrib/auth/authentik/README.md | cat -n printf '\n---\n' sed -n '140,260p' contrib/auth/authentik/README.md | cat -n printf '\n---\n' sed -n '1,120p' contrib/auth/authentik/config/platform-compose-authentik.yaml | cat -nRepository: NVIDIA-NeMo/nemo-platform
Length of output: 12569
Validate the Authentik subject-token audience
contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py:75-84verify_aud=Falseaccepts any JWT from the allowed issuer. Add a subject-tokenaudcheck on the Authentik path so tokens minted for other clients/resources can’t be exchanged.🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` around lines 75 - 84, Validate the subject token’s audience in the JWT decode path instead of disabling audience verification with options={"verify_aud": False}. In the Authentik exchange flow around jwt.decode and the issuer allowlist check, require the configured expected audience (or explicitly compare the decoded aud claim against it) and reject mismatches before exchanging the token.
183-195: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject unsupported target audiences.
The caller controls
audience, which is copied into a broker-signed token without an allowlist. Any valid subject token can therefore mint credentials for arbitrary relying services that trust this issuer.Proposed fix
try: audience = str(form.get("audience") or AUDIENCE) + if audience != AUDIENCE: + return _oauth_error(400, "invalid_target", "Requested audience is not allowed") subject_claims = _decode_subject_token(str(subject_token), audience)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.try: audience = str(form.get("audience") or AUDIENCE) if audience != AUDIENCE: return _oauth_error(400, "invalid_target", "Requested audience is not allowed") subject_claims = _decode_subject_token(str(subject_token), audience) except Exception: logger.exception("Subject token validation failed") return _oauth_error(400, "invalid_grant", "Could not validate subject token") now = int(time.time()) scope = str(form.get("scope") or SCOPE) exchanged_claims: dict[str, Any] = { "iss": ISSUER, "sub": subject_claims["sub"], "aud": audience,🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py` around lines 183 - 195, Validate the requested audience against an explicit allowlist before calling _decode_subject_token or signing the exchanged token. In the audience handling within the token exchange flow, reject unsupported values with the existing _oauth_error response (for example, invalid_target or invalid_request) and only copy an allowlisted audience into exchanged_claims["aud"].packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py (1)
46-70: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate the token endpoint before sending the subject credential.
Unlike the plugin implementation, this path accepts non-loopback HTTP URLs and uploads the workload subject token in cleartext. Apply the same HTTPS-with-loopback-exception policy before
httpx.post().🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 69-69: Request-controlled URL passed to httpx; validate against an allowlist to prevent SSRF.
Context: httpx.post(token_endpoint, data=data, timeout=timeout)
Note: [CWE-918] Server-Side Request Forgery (SSRF).(avoid-ssrf)
🤖 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 `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py` around lines 46 - 70, Validate token_endpoint in token_exchange_grant before calling httpx.post(): require HTTPS, allowing HTTP only for loopback hosts, and reject all other non-HTTPS endpoints before transmitting subject_token. Reuse the same endpoint-validation policy and symbols as the plugin implementation where available.Source: Linters/SAST tools
services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py (3)
258-258: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Restore refreshers for active containers after controller restart.
The mapping starts empty and only scheduling populates it. Existing
ACTIVEcontainers therefore stop receiving token updates after a controller restart and eventually lose authentication.🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` at line 258, The controller currently initializes _workload_identity_refreshers empty, so active containers lose token refreshes after restart. During controller startup, restore a SubjectTokenRefreshLoop for every existing ACTIVE container and register it in _workload_identity_refreshers, reusing the same scheduling/setup logic used for newly scheduled containers.
321-337: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the token readable by non-root workloads.
put_archive()creates a root-owned file, then the writer applies mode0400. Job containers running as a non-root UID cannot read the configured token path. Chown it to the workload UID/GID or use an appropriate readable mode for this dedicated volume.🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 321 - 337, The token-writing command in _write_workload_identity_subject_token currently applies mode 0400, leaving the root-owned token unreadable to non-root workloads. Update the command to chown the token to the configured workload UID/GID, or apply an appropriate readable mode for this dedicated volume, while preserving secure permissions.
404-427: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Provide an adoption path for containers without the new owner label.
Containers created before this change lack
JOB_CONTROLLER_INSTANCE_ID_LABEL. They are still found by name, but kill, stop, and cleanup now reject or exclude them, stranding jobs during upgrades.Also applies to: 1467-1479, 1545-1556, 1773-1776
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 404 - 427, Update _is_container_owned_by_this_controller, kill/stop handling, and cleanup filtering to support legacy containers missing JOB_CONTROLLER_INSTANCE_ID_LABEL. Preserve the strict owner-label checks for newly created containers, but add a safe adoption path for matching legacy containers identified by the existing controller/backend/profile criteria, allowing them to be stopped, killed, and cleaned up during upgrades.
1abaee3 to
33db07e
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
contrib/auth/authentik/kubernetes/README.md (1)
1-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSeparate the how-to from the explanation.
This page mixes deployment steps with token-exchange architecture, has no Diataxis classification, lacks tabbed Python SDK/CLI task examples, and has no
Next Steps. Keep this as a HOW-TO; move lines 50-66 to an explanation page and cross-link 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 `@contrib/auth/authentik/kubernetes/README.md` around lines 1 - 100, Keep this README focused as a Diataxis HOW-TO: add an explicit HOW-TO classification, retain only actionable deployment and verification steps, and move the token-exchange architecture explanation currently in “Start NeMo With The Kubernetes Override” to a separate explanation page. Add a cross-link to that page, provide tabbed Python SDK and CLI examples for submitting the workload task, and append a “Next Steps” section with relevant follow-up links.Source: Coding guidelines
🤖 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 `@contrib/auth/authentik/kubernetes/authentik.yaml`:
- Around line 27-116: Harden both Authentik pod templates in the authentik
server and authentik-worker Deployments: set automountServiceAccountToken to
false, add pod/container security contexts enforcing non-root execution,
RuntimeDefault seccomp, allowPrivilegeEscalation false, and drop all
capabilities. Before enabling readOnlyRootFilesystem, verify Authentik can write
to required paths and add explicit writable mounts such as emptyDir volumes
where necessary; apply the same validated configuration to both containers.
In `@contrib/auth/authentik/kubernetes/envoy-gateway.yaml`:
- Around line 15-30: Update the Envoy container in the gateway Pod spec with a
restrictive securityContext: run as non-root, set allowPrivilegeEscalation to
false, drop all Linux capabilities, use RuntimeDefault seccomp, and enable a
read-only root filesystem where supported. Set automountServiceAccountToken to
false at the Pod spec level to disable the unused token mount.
- Around line 17-26: Replace the mutable envoyproxy/envoy:v1.33-latest image
reference in the Envoy container with a specific immutable image digest, and
update the matching Envoy image reference in docker-compose.yml to use the same
pinned digest.
In `@contrib/auth/authentik/kubernetes/postgres.yaml`:
- Around line 40-42: Replace the ephemeral emptyDir volume in the Authentik
PostgreSQL pod with a PersistentVolumeClaim-backed volume, referencing a defined
claim for durable database storage; alternatively, document and automate the
intentional reset behavior if persistence is not required.
- Around line 27-42: Harden the PostgreSQL Pod defined by the container named
“postgres” by adding a non-root security context, disabling privilege
escalation, dropping all Linux capabilities, and setting seccomp to
RuntimeDefault; also disable the unused ServiceAccount token mount at the Pod
spec level with automountServiceAccountToken: false.
In `@contrib/auth/authentik/kubernetes/redis.yaml`:
- Around line 15-28: Update the Redis container in the pod spec to run as
non-root with privilege escalation disabled, drop all Linux capabilities, and
use the RuntimeDefault seccomp profile; also disable automatic ServiceAccount
token mounting at the pod level. Apply these settings around the existing redis
container and pod spec without changing its data volume configuration.
In `@contrib/auth/authentik/kubernetes/workload-token-exchange.yaml`:
- Around line 43-77: Add a securityContext to the broker container in the
workload-token-exchange manifest: run as a non-root user, disable privilege
escalation, drop all Linux capabilities, and set seccompProfile.type to
RuntimeDefault. Enable readOnlyRootFilesystem and add an emptyDir volume plus
matching mount only for the broker’s required writable paths, preserving the
existing ServiceAccount token access and application behavior.
In `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 219-234: Restrict the audience used by the token exchange flow
instead of accepting arbitrary form input. Update the audience handling in the
subject-token validation block and the exchanged_claims construction to accept
only AUDIENCE or values from an explicit configured allowlist; reject disallowed
audiences with the existing invalid_grant response before signing the token.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 72-83: Update the response handling in _exchange() to validate
both error and successful JSON payloads: require JSON objects and, for 200
responses, a non-empty access_token; otherwise raise WorkloadTokenExchangeError
instead of allowing AttributeError or KeyError. Return a concrete dict[str,
object] and adjust parsing/type annotations accordingly.
In
`@services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py`:
- Around line 64-81: Update the test to avoid replacing sdk.workspaces.retrieve,
since that bypasses just-in-time authentication. Stub the underlying HTTP
transport used by task_run so the real SDK request executes, return a successful
response, and assert the transport receives the Authorization header Bearer
exchanged-access-token; update exchange_requests expectations to reflect the
actual exchange.
---
Nitpick comments:
In `@contrib/auth/authentik/kubernetes/README.md`:
- Around line 1-100: Keep this README focused as a Diataxis HOW-TO: add an
explicit HOW-TO classification, retain only actionable deployment and
verification steps, and move the token-exchange architecture explanation
currently in “Start NeMo With The Kubernetes Override” to a separate explanation
page. Add a cross-link to that page, provide tabbed Python SDK and CLI examples
for submitting the workload task, and append a “Next Steps” section with
relevant follow-up links.
🪄 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: 9139e7ab-6700-42b6-a4dc-47676a1bc083
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (69)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_workload_exchange_broker.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
✅ Files skipped from review due to trivial changes (8)
- contrib/auth/authentik/kubernetes/namespace.yaml
- docs/auth/authentication/idp-integration.mdx
- contrib/auth/authentik/kustomization.yaml
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- docs/auth/deployment/configuration.mdx
- contrib/auth/authentik/.gitignore
- tests/auth_idp/test_fixture_helpers.py
- tests/auth_idp/test_provider_manifest.py
🚧 Files skipped from review as they are similar to previous changes (39)
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- contrib/auth/authentik/manifest.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- tests/auth_idp/test_docs_links.py
- services/core/auth/tests/test_discovery.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- docs/auth/deployment/credential-propagation.mdx
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- tests/auth_idp/test_authentik_blueprint.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- packages/nemo_platform_plugin/tests/test_client_auth.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- e2e/services_pool.py
- contrib/auth/authentik/gateway/envoy.yaml
- packages/nmp_common/src/nmp/common/config/base.py
- contrib/auth/manifest.schema.yaml
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- contrib/auth/authentik/docker-compose.yml
- services/core/jobs/jobs-launcher/cmd/run.go
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/tests/config/test_config.py
- contrib/auth/authentik/README.md
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- docs/set-up/config-reference.mdx
- contrib/auth/authentik/run.sh
- openapi/ga/individual/platform.openapi.yaml
- e2e/conftest.py
- openapi/ga/openapi.yaml
- openapi/openapi.yaml
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
33db07e to
8c72a38
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@contrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.py`:
- Around line 102-108: Require the subject token to contain an expiration claim
by adding PyJWT’s required-claims option to the jwt.decode call in the token
validation flow. Update the decode configuration associated with subject_token
and ensure "exp" is included among the required claims while preserving the
existing algorithm, audience, and leeway settings.
In `@tests/auth_idp/test_authentik_kubernetes_demo.py`:
- Around line 85-98: Update
test_kubernetes_platform_override_enables_workload_identity_exchange and its
referenced platform configuration to require a secure workload-token endpoint:
configure HTTPS/mTLS as appropriate and change the endpoint assertion from
http:// to https:// while preserving the existing workload identity checks.
🪄 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: f938695c-a892-4133-98ff-23f5cd1f13e7
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (70)
contrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/blueprints/nemo.yaml.tplcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tplcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kubernetes/workload-token-exchange.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/workload-exchange-broker/workload_exchange_broker.pycontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/tests/test_discovery.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_workload_exchange_broker.pytests/auth_idp_k8s/test_authentik_kubernetes_live.py
💤 Files with no reviewable changes (3)
- tests/auth_idp/test_authentik_real_oidc.py
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (5)
- contrib/auth/authentik/kubernetes/namespace.yaml
- contrib/auth/authentik/.gitignore
- docs/auth/authentication/idp-integration.mdx
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- docs/auth/deployment/configuration.mdx
🚧 Files skipped from review as they are similar to previous changes (42)
- contrib/auth/authentik/kustomization.yaml
- contrib/auth/manifest.schema.yaml
- tests/auth_idp/test_docs_links.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- tests/auth_idp/test_fixture_helpers.py
- tests/auth_idp/test_provider_manifest.py
- docs/auth/deployment/credential-propagation.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.py
- packages/nmp_common/src/nmp/common/config/base.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- e2e/services_pool.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- contrib/auth/authentik/gateway/envoy.yaml
- contrib/auth/authentik/docker-compose.yml
- contrib/auth/authentik/blueprints/nemo.yaml
- services/core/jobs/jobs-launcher/cmd/run.go
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- packages/nemo_platform_plugin/tests/test_client_auth.py
- services/core/auth/tests/test_discovery.py
- services/core/jobs/tests/controllers/test_workload_tokens.py
- contrib/auth/authentik/run.sh
- contrib/auth/authentik/manifest.yaml
- e2e/conftest.py
- openapi/ga/individual/platform.openapi.yaml
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/tests/config/test_config.py
- contrib/auth/authentik/README.md
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml.tpl
- openapi/ga/openapi.yaml
- openapi/openapi.yaml
- contrib/auth/authentik/blueprints/nemo.yaml.tpl
- docs/set-up/config-reference.mdx
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
7761bb1 to
a2c4c1d
Compare
mckornfield
left a comment
There was a problem hiding this comment.
fun pr lol
personally my beef is just the authentik bit using kustomize, I'd rather it just be in helm if it's all the same, since we already have a helm chart in the repo (understanding it's a demo object, but unless you have a strong reason it should be kustomize, idk why we wouldn't just use helm)
f2fb467 to
ba745d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
contrib/auth/authentik/kubernetes/redis.yaml (1)
27-37: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
readOnlyRootFilesystem: true.
/datais already a dedicated volume; root fs doesn't need to stay writable.🔒 Proposed fix
securityContext: runAsNonRoot: true runAsUser: 999 allowPrivilegeEscalation: false + readOnlyRootFilesystem: true capabilities: drop: ["ALL"] seccompProfile: type: RuntimeDefault🤖 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 `@contrib/auth/authentik/kubernetes/redis.yaml` around lines 27 - 37, Add readOnlyRootFilesystem: true to the Redis pod’s securityContext alongside runAsNonRoot and allowPrivilegeEscalation settings, while keeping the existing /data volumeMount writable for Redis data.contrib/auth/authentik/kubernetes/README.md (1)
1-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a "Next Steps" section.
Doc ends abruptly after the preflight script section with no cross-links to related docs (Compose demo, auth deployment/configuration docs).
As per coding guidelines: "Include 'Next Steps' section at the end with cross-links to related documentation content."
🤖 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 `@contrib/auth/authentik/kubernetes/README.md` around lines 1 - 92, Add a “Next Steps” section at the end of the README after the k8s-test instructions, linking to the related Authentik Compose demo and the authentication deployment/configuration documentation using the repository’s correct relative Markdown paths.Source: Coding guidelines
contrib/auth/authentik/run.sh (1)
247-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded kustomization path duplicates
k8s_render'soutput_dir.
run_k8s_testshardcodescontrib/auth/authentik/.generated/kubernetesasNMP_AUTHENTIK_K8S_KUSTOMIZATION, whilek8s_rendercomputes the same path from${COMPOSE_DIR}/.generated/kubernetes. Currently safe since--compose-diris rejected fork8s-test, but the literal duplicates a value derivable fromCOMPOSE_DIR/REPO_ROOT, risking drift if either changes.♻️ Suggested fix
run_in_repo \ env "IMAGE_REGISTRY=${IMAGE_REGISTRY}" "BAKE_TAG=${BAKE_TAG}" \ - "NMP_AUTHENTIK_K8S_E2E=1" "NMP_AUTHENTIK_K8S_KUSTOMIZATION=contrib/auth/authentik/.generated/kubernetes" \ + "NMP_AUTHENTIK_K8S_E2E=1" "NMP_AUTHENTIK_K8S_KUSTOMIZATION=${COMPOSE_DIR#${REPO_ROOT}/}/.generated/kubernetes" \ uv run --frozen pytest tests/auth_idp_k8s -v --run-e2eAlso applies to: 328-330
🤖 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 `@contrib/auth/authentik/run.sh` at line 247, Update run_k8s_tests and the related kustomization setup to derive NMP_AUTHENTIK_K8S_KUSTOMIZATION from the same ${COMPOSE_DIR}/.generated/kubernetes expression used by k8s_render, instead of hardcoding the repository path. Reuse the existing output_dir or a shared variable so both code paths remain consistent if COMPOSE_DIR changes.packages/nmp_common/src/nmp/common/config/base.py (1)
198-212: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winEnforce the documented dependency between
workload_subject_jwks_uriandworkload_subject_issuers.The description states issuers are "Required when workload_subject_jwks_uri is set," but nothing enforces this. If an operator sets the JWKS URI without issuers,
_allowed_subject_issuersinworkload_token_exchange.pyreturns an empty set and every JWT-based exchange silently fails withinvalid_grant—a confusing, hard-to-diagnose misconfiguration.♻️ Suggested validator
+ `@model_validator`(mode="after") + def _validate_workload_subject_config(self) -> "OIDCConfig": + if self.workload_subject_jwks_uri and not self.workload_subject_issuers: + raise ValueError( + "workload_subject_issuers is required when workload_subject_jwks_uri is set" + ) + return self🤖 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 `@packages/nmp_common/src/nmp/common/config/base.py` around lines 198 - 212, Enforce the documented dependency between BaseConfig fields workload_subject_jwks_uri and workload_subject_issuers: add a model-level validator that rejects configurations where workload_subject_jwks_uri is set while workload_subject_issuers is empty, with a clear validation error. Preserve valid configurations where the URI is unset or issuers are provided, and ensure the validator matches the project’s existing Pydantic validation style.services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py (1)
98-127: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUncached JWKS fetch on every token exchange request.
_fetch_subject_jwkshitsworkload_subject_jwks_uriover HTTP on every call, and_select_subject_signing_keyreimplements PyJWT's ownPyJWKClient.get_signing_keys()filtering from scratch. Considerjwt.PyJWKClient(jwks_uri)(built-in caching, key refresh-on-miss, and this exact "sig"/kid filtering) instead of hand-rolling fetch + filter logic per request.♻️ Simplification using PyJWKClient
-async def _fetch_subject_jwks(config: AuthConfig) -> dict[str, Any]: - ... - -def _select_subject_signing_key(subject_token: str, jwks: dict[str, Any]) -> Any: - ... +_jwks_clients: dict[str, jwt.PyJWKClient] = {} + +def _subject_jwks_client(config: AuthConfig) -> jwt.PyJWKClient: + jwks_uri = config.oidc.workload_subject_jwks_uri + if not jwks_uri: + raise jwt.InvalidTokenError("JWT subject token validation is disabled") + if jwks_uri not in _jwks_clients: + _jwks_clients[jwks_uri] = jwt.PyJWKClient(jwks_uri) + return _jwks_clients[jwks_uri]🤖 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 `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py` around lines 98 - 127, Replace the per-request `_fetch_subject_jwks` and manual filtering in `_select_subject_signing_key` with a cached `jwt.PyJWKClient` for `config.oidc.workload_subject_jwks_uri`; retain the disabled-validation error, use `get_signing_key_from_jwt(subject_token)` for key selection, and translate PyJWT/network failures into the existing `jwt.InvalidTokenError` handling contract.packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py (1)
165-197: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winWorkload provider isn't cached like the OIDC path.
_create_workload_exchange_providerbuilds a newWorkloadTokenExchangeProvider(and triggers discovery) on every_resolve_bootstrapcall, unlike the OIDC branch which reuses providers via_get_or_create_provider. Multiple clients in the same process will each perform redundant discovery/exchange calls.Also applies to: 452-456
🤖 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 `@packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py` around lines 165 - 197, Cache the workload exchange provider in the same way as the OIDC provider: update _resolve_bootstrap and the provider cache flow around _get_or_create_provider so repeated resolutions reuse an existing WorkloadTokenExchangeProvider instead of calling _create_workload_exchange_provider and discovery each time. Include cache identity inputs such as the base URL and subject-token file, and preserve recreation when those configuration values change.openapi/ga/openapi.yaml (1)
12677-12688: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIdentical
workload_identity_token_*fields duplicated across Kubernetes and Volcano profiles.Same two fields, same descriptions/defaults, copy-pasted between
KubernetesJobExecutionProfileConfigandVolcanoJobExecutionProfileConfig. Consider a shared base schema in the source Pydantic models to avoid drift.Also applies to: 18863-18874
🤖 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 `@openapi/ga/openapi.yaml` around lines 12677 - 12688, The workload identity token fields are duplicated in the Kubernetes and Volcano execution profile schemas. Extract workload_identity_token_expiration_seconds and workload_identity_token_audience into a shared base Pydantic schema, then have KubernetesJobExecutionProfileConfig and VolcanoJobExecutionProfileConfig inherit or reference it so the generated OpenAPI definitions remain consistent.
🤖 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 @.github/workflows/ci.yaml:
- Around line 1080-1092: Gate the “Pull auth-idp Kubernetes test image” step on
needs.build-cpu-smoke-images.outputs.publish_images == 'true', matching the
existing GHCR login condition, so docker pull runs only when the image was
published; alternatively ensure the image is published before this step.
In `@contrib/auth/authentik/kubernetes/nemo.yaml`:
- Around line 43-82: Add pod- and container-level securityContext settings to
the nemo Deployment, following the hardened patterns in envoy-gateway.yaml and
postgres.yaml: run as a non-root user, disallow privilege escalation, drop all
capabilities, and enable readOnlyRootFilesystem. Add any writable volume mounts
required for runtime data.
In `@openapi/ga/openapi.yaml`:
- Around line 10025-10028: Mark DockerWorkloadIdentityConfig.client_secret as
write-only at the source Pydantic model rather than editing the generated
OpenAPI file, using SecretStr or Field metadata that emits format: password and
writeOnly: true. Regenerate the OpenAPI schema and verify the property under
DockerJobExecutionProfileConfig is no longer exposed in GET responses.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.py`:
- Around line 46-68: token_exchange_grant currently posts tokens without
validating transport security. Before calling httpx.post in
token_exchange_grant, apply the existing OIDC endpoint validation used by the
client implementation and reject any token_endpoint that is not HTTPS, including
HTTP URLs; preserve secure endpoint handling and raise the project’s established
validation error.
In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.py`:
- Around line 277-315: Validate the successful response payload in
token_exchange_grant and _exchange before accessing access_token. Reuse or
mirror the sibling helpers _response_json_object and _access_token_from_response
to require a JSON object containing a valid access_token, and raise
WorkloadTokenExchangeError for malformed or missing data instead of allowing
KeyError or AttributeError.
In `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py`:
- Around line 35-36: The module-level `_private_key` must not be regenerated on
each process start; load the workload signing key from a shared secret or
durable key store while keeping `_public_key` and the `kid` consistent across
replicas and restarts. Also update the subject JWKS retrieval used by the
workload token exchange flow to cache the fetched keys with an appropriate
refresh or expiry policy, avoiding an IdP request on every exchange.
In `@services/core/jobs/tests/controllers/test_docker_backend.py`:
- Around line 732-772: Make cleanup in
test_docker_job_injects_workload_identity_volume_when_token_exchange_enabled
exception-safe by wrapping the assertions and refresher lookup in a try/finally
block, with each workload identity refresher’s stop() called in the finally
block; alternatively, use a fixture finalizer to guarantee cleanup even when an
assertion fails.
- Around line 1698-1708: Update the stale comment above the volume assertions to
state that successful containers use three volumes per task instead of two;
leave the existing assertions and count unchanged.
- Around line 2567-2571: Update the stale comment above the volume assertions in
the multi-step job cleanup test to state that three volumes per task are
verified, matching the three volume names asserted and the expected call count.
---
Nitpick comments:
In `@contrib/auth/authentik/kubernetes/README.md`:
- Around line 1-92: Add a “Next Steps” section at the end of the README after
the k8s-test instructions, linking to the related Authentik Compose demo and the
authentication deployment/configuration documentation using the repository’s
correct relative Markdown paths.
In `@contrib/auth/authentik/kubernetes/redis.yaml`:
- Around line 27-37: Add readOnlyRootFilesystem: true to the Redis pod’s
securityContext alongside runAsNonRoot and allowPrivilegeEscalation settings,
while keeping the existing /data volumeMount writable for Redis data.
In `@contrib/auth/authentik/run.sh`:
- Line 247: Update run_k8s_tests and the related kustomization setup to derive
NMP_AUTHENTIK_K8S_KUSTOMIZATION from the same
${COMPOSE_DIR}/.generated/kubernetes expression used by k8s_render, instead of
hardcoding the repository path. Reuse the existing output_dir or a shared
variable so both code paths remain consistent if COMPOSE_DIR changes.
In `@openapi/ga/openapi.yaml`:
- Around line 12677-12688: The workload identity token fields are duplicated in
the Kubernetes and Volcano execution profile schemas. Extract
workload_identity_token_expiration_seconds and workload_identity_token_audience
into a shared base Pydantic schema, then have
KubernetesJobExecutionProfileConfig and VolcanoJobExecutionProfileConfig inherit
or reference it so the generated OpenAPI definitions remain consistent.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py`:
- Around line 165-197: Cache the workload exchange provider in the same way as
the OIDC provider: update _resolve_bootstrap and the provider cache flow around
_get_or_create_provider so repeated resolutions reuse an existing
WorkloadTokenExchangeProvider instead of calling
_create_workload_exchange_provider and discovery each time. Include cache
identity inputs such as the base URL and subject-token file, and preserve
recreation when those configuration values change.
In `@packages/nmp_common/src/nmp/common/config/base.py`:
- Around line 198-212: Enforce the documented dependency between BaseConfig
fields workload_subject_jwks_uri and workload_subject_issuers: add a model-level
validator that rejects configurations where workload_subject_jwks_uri is set
while workload_subject_issuers is empty, with a clear validation error. Preserve
valid configurations where the URI is unset or issuers are provided, and ensure
the validator matches the project’s existing Pydantic validation style.
In `@services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py`:
- Around line 98-127: Replace the per-request `_fetch_subject_jwks` and manual
filtering in `_select_subject_signing_key` with a cached `jwt.PyJWKClient` for
`config.oidc.workload_subject_jwks_uri`; retain the disabled-validation error,
use `get_signing_key_from_jwt(subject_token)` for key selection, and translate
PyJWT/network failures into the existing `jwt.InvalidTokenError` handling
contract.
🪄 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: 87388497-9cf5-4b55-8704-c08cf4a63898
⛔ Files ignored due to path filters (24)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/atif_trajectory_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_atif.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (78)
.github/workflows/ci.yamlMakefilecontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/kubernetes/authentik.yamlcontrib/auth/authentik/kubernetes/envoy-gateway.yamlcontrib/auth/authentik/kubernetes/namespace.yamlcontrib/auth/authentik/kubernetes/nemo.yamlcontrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yamlcontrib/auth/authentik/kubernetes/postgres.yamlcontrib/auth/authentik/kubernetes/redis.yamlcontrib/auth/authentik/kustomization.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/conftest.pye2e/services_pool.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/base.pyservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/providers.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
💤 Files with no reviewable changes (4)
- Makefile
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
✅ Files skipped from review due to trivial changes (4)
- contrib/auth/authentik/kubernetes/namespace.yaml
- contrib/auth/authentik/.gitignore
- docs/auth/authentication/idp-integration.mdx
- docs/auth/deployment/configuration.mdx
🚧 Files skipped from review as they are similar to previous changes (23)
- tests/auth_idp/test_docs_links.py
- packages/nemo_platform_ext/tests/auth/test_token_provider.py
- contrib/auth/manifest.schema.yaml
- e2e/services_pool.py
- services/core/jobs/tests/test_config.py
- contrib/auth/authentik/kubernetes/platform-authentik-kubernetes.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- contrib/auth/authentik/kubernetes/authentik.yaml
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- services/core/jobs/jobs-launcher/cmd/run.go
- services/core/jobs/tests/controllers/test_kubernetes_backend.py
- packages/nemo_platform_ext/tests/cli/commands/test_auth.py
- docs/auth/deployment/credential-propagation.mdx
- services/core/jobs/tests/controllers/test_workload_tokens.py
- contrib/auth/authentik/README.md
- e2e/conftest.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/tests/config/test_config.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
|
If there's something wrong with the way the helm chart installs the api container, please change the helm chart. I don't see why we are going to all this trouble to install authentik in what seems like an unusual way in addition to an apparently unusual install of the api for a test like this. There must be more context I don't know here? |
ba745d8 to
a7f6c62
Compare
9015941 to
aaa8f61
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py (1)
981-1005: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUse the existing container’s token volume when resuming.
A fresh task volume/refresher is created before existing-container detection. On resume, that refresher writes to an unattached volume, so the container’s original token expires. Detect the container first and derive its mounted volume from its task label or mounts.
Also applies to: 1057-1062, 1153-1156, 1194-1196, 1383-1383
🤖 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 `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py` around lines 981 - 1005, Update the job startup flow around workload_identity_volume_name, ensure_job_storage, and _build_workload_identity_refresher to detect an existing container before creating a fresh token volume or refresher. When resuming, derive and reuse the container’s mounted workload-identity volume from its task label or mounts; only create a new volume and refresher for new containers. Apply the same ordering and reuse behavior to the corresponding resume paths referenced by the review.
♻️ Duplicate comments (2)
contrib/auth/authentik/helm/templates/workload-token-signing-key-secret.yaml (1)
1-13: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winStill missing
requiredguard on the signing key.
.Values.workloadTokenSigningKey.privateKeyPemdefaults to""; a plainhelm upgrade --installwithoutrun.shrenders an empty secret and silently breaks workload-token signing.🛡️ Proposed fix
stringData: {{ include "nemo-platform-authentik.workloadTokenSigningKey.key" . }}: |- -{{ include "nemo-platform-authentik.workloadTokenSigningKey.privateKeyPem" . | nindent 4 }} +{{ required "workloadTokenSigningKey.privateKeyPem is required when workloadTokenSigningKey.create is true" (include "nemo-platform-authentik.workloadTokenSigningKey.privateKeyPem" .) | nindent 4 }}🤖 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 `@contrib/auth/authentik/helm/templates/workload-token-signing-key-secret.yaml` around lines 1 - 13, In the workloadTokenSigningKey Secret template, add a required-value guard for .Values.workloadTokenSigningKey.privateKeyPem before rendering the privateKeyPem content. Ensure Helm fails during rendering when the value is empty instead of creating an empty signing-key secret, while preserving the existing secret name, key, and PEM formatting.contrib/auth/authentik/docker-compose.yml (1)
154-155: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftScope the gateway TLS volume per Compose project.
The fixed Docker name and backend hardcode force parallel runs to share certificate material.
contrib/auth/authentik/docker-compose.yml#L154-L155: use a project-specific or configurable volume name.contrib/auth/authentik/config/platform-compose-authentik.yaml#L56-L59: consume the same resolved name inadditional_volume_mounts.🤖 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 `@contrib/auth/authentik/docker-compose.yml` around lines 154 - 155, Replace the fixed gateway-tls volume name in contrib/auth/authentik/docker-compose.yml:154-155 with a project-specific or configurable name, then update contrib/auth/authentik/config/platform-compose-authentik.yaml:56-59 to mount that same resolved volume name through additional_volume_mounts. Ensure parallel Compose projects do not share certificate material.
🧹 Nitpick comments (2)
packages/nemo_platform_ext/tests/cli/commands/test_auth.py (2)
739-761: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert the refresh-token value is also redacted.
A regression printing
foo-refreshstill passes. Add:assert "foo-token" not in result.output + assert "foo-refresh" not in result.output🤖 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 `@packages/nemo_platform_ext/tests/cli/commands/test_auth.py` around lines 739 - 761, Extend test_auth_status_when_cluster_unreachable_shows_local_state to assert that the refresh-token value, such as “foo-refresh”, is absent from result.output, alongside the existing foo-token redaction assertion.
318-329: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse a readable file and isolate the current environment variable.
The missing file and simultaneous legacy variable let swallowed read failures satisfy the assertion without proving workload-identity files are excluded from source labeling.
Proposed test correction
- token_file = tmp_path / "missing-workload-token.jwt" + token_file = tmp_path / "workload-token.jwt" + token_file.write_text("subject-token", encoding="utf-8") monkeypatch.delenv("NMP_ACCESS_TOKEN", raising=False) monkeypatch.delenv("NEMO_WORKLOAD_TOKEN", raising=False) - monkeypatch.setenv("NEMO_WORKLOAD_TOKEN_FILE", str(token_file)) + monkeypatch.delenv("NEMO_WORKLOAD_TOKEN_FILE", raising=False) monkeypatch.setenv(WORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR, str(token_file))The PR objective requires legacy environment variables to remain no-ops.
🤖 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 `@packages/nemo_platform_ext/tests/cli/commands/test_auth.py` around lines 318 - 329, Update test_runtime_token_source_label_ignores_workload_identity_token_file to create a readable token file and isolate the current workload-identity environment variable from the legacy NEMO_WORKLOAD_TOKEN_FILE variable. Keep access-token variables unset, set only the current variable to the readable file, and assert _runtime_token_source_label() returns None, ensuring the result proves legacy variables remain no-ops rather than relying on a swallowed file-read failure.
🤖 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 `@services/core/jobs/jobs-launcher/cmd/workload_auth_test.go`:
- Around line 188-203: Protect tokenExchangeCount in the httptest server handler
and the test assertions with a mutex. Add a mutex alongside the counter, lock
around the increment in the /apis/auth/token handler and around both reads in
the affected assertion paths, using defer or tightly scoped unlocks to ensure
balanced synchronization.
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Line 290: Update the jobs-controller initialization or synchronization flow to
reconstruct self._workload_identity_refreshers from already managed Docker
containers, using their workload-identity labels and mounted token-file
metadata. Ensure existing running containers regain SubjectTokenRefreshLoop
instances after a controller restart, while preserving the current
scheduling-time registration behavior for new jobs.
- Around line 1002-1005: Update the job setup flow around
_build_workload_identity_refresher and refresh_once so failures during issuer
construction or initial token refresh clean up the volumes already allocated by
ensure_job_storage(). Preserve the existing successful path, and ensure cleanup
occurs before propagating the failure so retries do not retain UUID-scoped
volumes.
- Around line 359-374: Update _write_workload_identity_subject_token so the
finalized token is readable by non-root workload containers: replace the
owner-only 0400 permission with an isolated-volume-readable mode, or chown the
token to the configured workload UID/GID before returning. Preserve the existing
temporary-file move and token write flow.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py`:
- Around line 130-139: Update the expires_in validation in the workload-token
handling code to require a positive finite numeric value, rejecting NaN and
positive or negative infinity while preserving boolean and non-numeric
rejection. Extend the expires_in validation cases in
services/core/jobs/tests/controllers/test_workload_tokens.py lines 182-210 to
cover NaN and infinity rejection.
---
Outside diff comments:
In `@services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py`:
- Around line 981-1005: Update the job startup flow around
workload_identity_volume_name, ensure_job_storage, and
_build_workload_identity_refresher to detect an existing container before
creating a fresh token volume or refresher. When resuming, derive and reuse the
container’s mounted workload-identity volume from its task label or mounts; only
create a new volume and refresher for new containers. Apply the same ordering
and reuse behavior to the corresponding resume paths referenced by the review.
---
Duplicate comments:
In `@contrib/auth/authentik/docker-compose.yml`:
- Around line 154-155: Replace the fixed gateway-tls volume name in
contrib/auth/authentik/docker-compose.yml:154-155 with a project-specific or
configurable name, then update
contrib/auth/authentik/config/platform-compose-authentik.yaml:56-59 to mount
that same resolved volume name through additional_volume_mounts. Ensure parallel
Compose projects do not share certificate material.
In
`@contrib/auth/authentik/helm/templates/workload-token-signing-key-secret.yaml`:
- Around line 1-13: In the workloadTokenSigningKey Secret template, add a
required-value guard for .Values.workloadTokenSigningKey.privateKeyPem before
rendering the privateKeyPem content. Ensure Helm fails during rendering when the
value is empty instead of creating an empty signing-key secret, while preserving
the existing secret name, key, and PEM formatting.
---
Nitpick comments:
In `@packages/nemo_platform_ext/tests/cli/commands/test_auth.py`:
- Around line 739-761: Extend
test_auth_status_when_cluster_unreachable_shows_local_state to assert that the
refresh-token value, such as “foo-refresh”, is absent from result.output,
alongside the existing foo-token redaction assertion.
- Around line 318-329: Update
test_runtime_token_source_label_ignores_workload_identity_token_file to create a
readable token file and isolate the current workload-identity environment
variable from the legacy NEMO_WORKLOAD_TOKEN_FILE variable. Keep access-token
variables unset, set only the current variable to the readable file, and assert
_runtime_token_source_label() returns None, ensuring the result proves legacy
variables remain no-ops rather than relying on a swallowed file-read failure.
🪄 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: 8d183c01-90de-48d6-b0e3-247da4f41680
⛔ Files ignored due to path filters (37)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/device_flow.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/tls.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_config_map_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_key_to_path.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_secret_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/json_web_key.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/json_web_key_set_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/workload_token_exchange_error_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/workload_token_exchange_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_device_flow.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_utils.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (140)
.github/actions/setup-kind-cluster/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/.helmignorecontrib/auth/authentik/helm/Chart.yamlcontrib/auth/authentik/helm/files/blueprints/nemo.yamlcontrib/auth/authentik/helm/templates/_envoy-config.tplcontrib/auth/authentik/helm/templates/_helpers.tplcontrib/auth/authentik/helm/templates/blueprint-apply-job.yamlcontrib/auth/authentik/helm/templates/blueprint-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-nemo-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-service.yamlcontrib/auth/authentik/helm/templates/shared-postgres-serviceaccount.yamlcontrib/auth/authentik/helm/templates/shared-postgres-statefulset.yamlcontrib/auth/authentik/helm/templates/tokenreview-rbac.yamlcontrib/auth/authentik/helm/templates/workload-token-signing-key-secret.yamlcontrib/auth/authentik/helm/templates/workload-token-tls.yamlcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/authz_oidc/conftest.pye2e/backends/docker_compose.pye2e/conftest.pye2e/services_pool.pyk8s/helm/README.mdk8s/helm/ci/21-api-extra-volumes.yamlk8s/helm/ci/22-envoy-config-override.yamlk8s/helm/templates/api/api-deployment.yamlk8s/helm/templates/platform-seed-job.yamlk8s/helm/templates/proxy/_helpers.tplk8s/helm/templates/proxy/envoy-configmap.yamlk8s/helm/templates/proxy/envoy-deployment.yamlk8s/helm/values.yamlopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/device_flow.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/tls.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_device_flow.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_utils.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/adapter.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/tls.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/config.pypackages/nemo_platform_plugin/tests/client/test_adapter.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nemo_platform_plugin/tests/test_config.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/__init__.pypackages/nmp_common/src/nmp/common/config/base.pypackages/nmp_common/src/nmp/common/entities/client.pypackages/nmp_common/src/nmp/common/sdk_factory.pypackages/nmp_common/src/nmp/common/service/base.pypackages/nmp_common/tests/nmp_common/test_common_config.pypackages/nmp_common/tests/sdk_factory/test_sdk.pypackages/nmp_platform_runner/src/nmp/platform_runner/config.pypackages/nmp_platform_runner/src/nmp/platform_runner/health.pypackages/nmp_platform_runner/src/nmp/platform_runner/loader.pypackages/nmp_platform_runner/src/nmp/platform_runner/registry.pypackages/nmp_platform_runner/src/nmp/platform_runner/run.pypackages/nmp_platform_runner/src/nmp/platform_runner/server.pypackages/nmp_platform_runner/tests/test_config.pypackages/nmp_platform_runner/tests/test_run.pypackages/nmp_platform_runner/tests/test_server.pypytest.iniservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/otel.goservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/jobs-launcher/cmd/workload_auth.goservices/core/jobs/jobs-launcher/cmd/workload_auth_test.goservices/core/jobs/jobs-launcher/go.modservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/core/models/tests/unit/controllers/test_deployment_reconciler.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/test_authentik_blueprint.pytests/auth_idp/test_authentik_cli_login.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp_k8s/test_authentik_kubernetes_live.pytests/test_e2e_docker_compose_backend.pytests/test_e2e_services_pool.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/vendor/vendor_package.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.pytools/nemo-platform-sdk-tools/tests/sdk/vendor/test_vendor_package.py
💤 Files with no reviewable changes (4)
- tests/auth_idp/test_authentik_real_oidc.py
- Makefile
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
🚧 Files skipped from review as they are similar to previous changes (71)
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.py
- contrib/auth/authentik/.gitignore
- pytest.ini
- k8s/helm/ci/21-api-extra-volumes.yaml
- packages/nmp_common/src/nmp/common/config/init.py
- tests/auth_idp/test_docs_links.py
- k8s/helm/templates/proxy/envoy-deployment.yaml
- k8s/helm/templates/platform-seed-job.yaml
- conftest.py
- contrib/auth/authentik/helm/templates/blueprint-configmap.yaml
- k8s/helm/templates/api/api-deployment.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/client/tls.py
- contrib/auth/authentik/helm/templates/tokenreview-rbac.yaml
- contrib/auth/manifest.schema.yaml
- k8s/helm/templates/proxy/envoy-configmap.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/tls.py
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- contrib/auth/authentik/helm/Chart.yaml
- k8s/helm/templates/proxy/_helpers.tpl
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- tests/auth_idp/runtime.py
- k8s/helm/ci/22-envoy-config-override.yaml
- docs/auth/authentication/idp-integration.mdx
- packages/nmp_common/tests/nmp_common/test_common_config.py
- .github/actions/setup-kind-cluster/action.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/device_flow.py
- packages/nmp_platform_runner/src/nmp/platform_runner/run.py
- services/core/jobs/jobs-launcher/cmd/workload_auth.go
- docs/auth/deployment/configuration.mdx
- contrib/auth/authentik/helm/values.yaml
- contrib/auth/authentik/helm/files/blueprints/nemo.yaml
- k8s/helm/values.yaml
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- packages/nmp_platform_runner/src/nmp/platform_runner/loader.py
- contrib/auth/authentik/helm/templates/_envoy-config.tpl
- services/core/auth/tests/test_discovery.py
- tools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
- contrib/auth/authentik/blueprints/nemo.yaml
- contrib/auth/authentik/manifest.yaml
- k8s/helm/README.md
- packages/nmp_platform_runner/tests/test_server.py
- tests/test_e2e_services_pool.py
- packages/nmp_platform_runner/src/nmp/platform_runner/config.py
- packages/nmp_platform_runner/src/nmp/platform_runner/health.py
- tests/auth_idp/test_provider_manifest.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- docs/auth/deployment/credential-propagation.mdx
- packages/nmp_platform_runner/tests/test_run.py
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/config.py
- packages/nmp_common/src/nmp/common/config/base.py
- contrib/auth/authentik/gateway/envoy.yaml
- services/core/auth/src/nmp/core/auth/service.py
- e2e/services_pool.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- contrib/auth/authentik/helm/templates/_helpers.tpl
- packages/nmp_common/src/nmp/common/sdk_factory.py
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/vendor/vendor_package.py
- docs/set-up/config-reference.mdx
- services/core/jobs/tests/test_config.py
- .github/workflows/ci.yaml
- services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py
- contrib/auth/authentik/README.md
- e2e/conftest.py
- services/core/jobs/jobs-launcher/cmd/run.go
- services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
- packages/nmp_platform_runner/tests/test_config.py
- tests/auth_idp/test_provider_layout.py
- tests/auth_idp/providers.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/core/jobs/tests/controllers/test_docker_backend.py (1)
751-760: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep scheduling mocks active until worker completion.
Both tests exit patched contexts before
shutdown(wait=True), allowing asynchronous workers to use real dependencies.
services/core/jobs/tests/controllers/test_docker_backend.py#L751-L760: move thread-pool shutdown inside the platform-configuration patch.services/core/jobs/tests/controllers/test_docker_backend.py#L803-L810: move thread-pool shutdown inside the auth and issuer patches.🤖 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 `@services/core/jobs/tests/controllers/test_docker_backend.py` around lines 751 - 760, The asynchronous worker must finish while its dependency patches remain active. In services/core/jobs/tests/controllers/test_docker_backend.py#L751-L760, move _container_run_threadpool.shutdown(wait=True) inside the get_platform_config patch; in services/core/jobs/tests/controllers/test_docker_backend.py#L803-L810, move the shutdown inside the auth and issuer patch contexts. Ensure both tests retain all scheduling mocks until worker completion.packages/nmp_platform_runner/src/nmp/platform_runner/config.py (1)
236-245: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle base URLs without a scheme.
urlparse("nemo-gateway:8080")yields no hostname, so this silently ignores the configured host despite Lines 215-216 promising an HTTP default.Proposed fix
- parsed = urlparse(base_url) + candidate = base_url + if "://" not in candidate and not candidate.startswith("//"): + candidate = f"http://{candidate}" + parsed = urlparse(candidate)Add a regression test using
platform.base_url: nemo-gateway:8080.🤖 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 `@packages/nmp_platform_runner/src/nmp/platform_runner/config.py` around lines 236 - 245, Update the base URL parsing logic around urlparse so scheme-less values such as “nemo-gateway:8080” are interpreted with the default HTTP scheme and their hostname is returned instead of being rejected. Preserve the existing malformed-URL fallback behavior, and add a regression test covering platform.base_url set to nemo-gateway:8080.
🤖 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/runtime_compose.py`:
- Around line 41-45: The exchange_workload_token method currently echoes the
subject token instead of exercising the configured workload_token_endpoint.
Update it to POST the RFC 8693 token-exchange grant with the subject token, then
construct and return the TokenSet from the endpoint’s exchanged access token and
claims, matching the Kubernetes runtime behavior.
In `@tests/auth_idp/test_provider_contract_gateway.py`:
- Line 43: Update the authenticated-token test assertion to accept only the
expected status codes, such as 200 and 403, instead of merely rejecting 401.
Replace the broad comparison in the existing test with an explicit
allowed-status check so 404 and 500 responses fail.
---
Outside diff comments:
In `@packages/nmp_platform_runner/src/nmp/platform_runner/config.py`:
- Around line 236-245: Update the base URL parsing logic around urlparse so
scheme-less values such as “nemo-gateway:8080” are interpreted with the default
HTTP scheme and their hostname is returned instead of being rejected. Preserve
the existing malformed-URL fallback behavior, and add a regression test covering
platform.base_url set to nemo-gateway:8080.
In `@services/core/jobs/tests/controllers/test_docker_backend.py`:
- Around line 751-760: The asynchronous worker must finish while its dependency
patches remain active. In
services/core/jobs/tests/controllers/test_docker_backend.py#L751-L760, move
_container_run_threadpool.shutdown(wait=True) inside the get_platform_config
patch; in services/core/jobs/tests/controllers/test_docker_backend.py#L803-L810,
move the shutdown inside the auth and issuer patch contexts. Ensure both tests
retain all scheduling mocks until worker completion.
🪄 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: bf763bd1-50ba-45ab-b391-8776c103f73e
📒 Files selected for processing (36)
.github/workflows/ci.yamlconftest.pycontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamle2e/services_pool.pyk8s/helm/templates/core/controller-deployment.yamlpackages/nmp_platform_runner/src/nmp/platform_runner/config.pypackages/nmp_platform_runner/tests/test_config.pypytest.iniservices/core/jobs/jobs-launcher/cmd/workload_auth_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pytests/auth_idp/authentik_live.pytests/auth_idp/common.pytests/auth_idp/conftest.pytests/auth_idp/k8s/test_authentik_kubernetes_live.pytests/auth_idp/providers.pytests/auth_idp/runtime_compose.pytests/auth_idp/runtime_contract.pytests/auth_idp/runtime_factory.pytests/auth_idp/runtime_kubernetes.pytests/auth_idp/test_authentik_cli_login.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_kubernetes_demo.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_contract_discovery.pytests/auth_idp/test_provider_contract_gateway.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_runtime_selection.py
🚧 Files skipped from review as they are similar to previous changes (10)
- services/core/jobs/jobs-launcher/cmd/workload_auth_test.go
- services/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.py
- tests/auth_idp/test_authentik_cli_login.py
- contrib/auth/authentik/helm/values.yaml
- tests/auth_idp/test_provider_manifest.py
- tests/auth_idp/test_authentik_gateway_live.py
- tests/auth_idp/authentik_live.py
- services/core/jobs/tests/controllers/test_workload_tokens.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
- tests/auth_idp/test_provider_layout.py
88fb2a8 to
285c096
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/nemo_platform_ext/tests/cli/commands/test_auth.py (1)
318-329: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a readable token file and remove the legacy variable.
A missing file only proves unreadable files return
None; it does not prove workload-identity tokens are ignored. Write a valid token to the file and set onlyWORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR.Proposed test correction
token_file = tmp_path / "missing-workload-token.jwt" +token_file.write_text( + generate_unsigned_jwt( + principal_id="workload", + email="workload@example.com", + expires_in_seconds=900, + ) +) monkeypatch.delenv("NMP_ACCESS_TOKEN", raising=False) monkeypatch.delenv("NEMO_WORKLOAD_TOKEN", raising=False) -monkeypatch.setenv("NEMO_WORKLOAD_TOKEN_FILE", str(token_file)) +monkeypatch.delenv("NEMO_WORKLOAD_TOKEN_FILE", raising=False) monkeypatch.setenv(WORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR, str(token_file))🤖 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 `@packages/nemo_platform_ext/tests/cli/commands/test_auth.py` around lines 318 - 329, The test test_runtime_token_source_label_ignores_workload_identity_token_file should create a readable token file containing a valid token, remove the legacy NEMO_WORKLOAD_TOKEN_FILE environment variable setup, and set only WORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR before asserting _runtime_token_source_label() returns None..github/workflows/ci.yaml (1)
1322-1323: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable checkout credential persistence.
make test-e2eexecutes PR-controlled code while checkout leaves the job token in Git configuration.Proposed fix
- name: Checkout code uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: falseObtain Write permission before editing this
.github/file, as required by the repository guideline.🤖 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 @.github/workflows/ci.yaml around lines 1322 - 1323, Update the actions/checkout step in the CI workflow to disable credential persistence by setting persist-credentials to false, while preserving the pinned checkout action and existing step behavior. Obtain Write permission before modifying the .github workflow file.Sources: Coding guidelines, Linters/SAST tools
tests/auth_idp/compose/test_authentik_cli_login.py (1)
81-92: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not mask workspace-creation failures during cleanup.
If creation fails, the unconditional DELETE can raise a 404 and replace the useful original exception. Clean up only after successful creation.
Proposed fix
+ workspace_created = False try: create_response = httpx.post( ... ) create_response.raise_for_status() + workspace_created = True assert create_response.json()["created_by"] == "nemo-user" finally: - _delete_workspace_for_cleanup(authentik_stack.gateway_base_url, workspace_name, headers) + if workspace_created: + _delete_workspace_for_cleanup(authentik_stack.gateway_base_url, workspace_name, headers)🤖 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/compose/test_authentik_cli_login.py` around lines 81 - 92, Update the workspace setup flow around the create_response call so _delete_workspace_for_cleanup runs only after workspace creation and validation succeed. Avoid an unconditional finally cleanup, preserving the original creation or assertion exception when setup fails.
🤖 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 @.github/workflows/ci.yaml:
- Around line 987-998: The python-auth-idp-e2e-test job currently may start
without access to the required Docker image when publish_images is false. Update
its workflow gating or setup so the E2E matrix runs only after a transferable
published image is available, or explicitly transfers the image artifact or
rebuilds it on the runner; preserve the existing prerequisite checks.
In `@contrib/auth/authentik/kubernetes/README.md`:
- Around line 3-16: Move the existing “## Prerequisites” section directly below
the README title, before the deployment description and other content. Add a “##
Next Steps” section at the end with links to the related Compose,
authentication, and production deployment documentation.
- Around line 31-42: Update the documented Helm install flow to generate a
workload-token signing key before invoking helm, then pass it through the
install command as `--set-file workloadTokenSigningKey.privateKeyPem=...`. Apply
the same provisioning behavior to the `k8s-test` flow referenced around the
later documented command, ensuring direct installs provide the key required for
workload-token signing.
In `@contrib/auth/authentik/run.sh`:
- Line 120: Update the --skip-image-load help text in run.sh to document that it
can be used with either a reused cluster or an explicit pullable --image on a
fresh cluster, while preserving the existing reuse behavior description.
In `@e2e/services_pool.py`:
- Around line 169-201: Update the service reacquisition flow, where
`_active_service_key_by_module` is assigned during acquisition, to re-add the
owner to `_remaining_modules_by_key[state.key]` whenever services are
successfully reacquired. Ensure the owner is registered before
`release_for_registered_owner` can remove it again, while preserving existing
handling for new keys and externally configured `NMP_BASE_URL` services.
In
`@packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/execution_profiles.py`:
- Line 71: Add JOB_LOGS_ENDPOINT_ENVVAR and WORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR
to the managed/reserved environment-variable collection alongside NMP_AUTH_URL,
ensuring profile and step environments cannot override the platform-injected
logging endpoint or workload identity token path.
In `@packages/nmp_common/tests/sdk_factory/test_sdk.py`:
- Around line 68-79: Update
test_get_platform_sdk_preserves_api_base_url_for_controller_only_pods to
exercise a controller API request and assert its prepared URL uses the
nemo-platform-api host rather than 127.0.0.1. Preserve the existing base_url
assertion and controller-only environment setup.
In `@services/core/auth/tests/test_workload_token_exchange.py`:
- Around line 120-128: Update the /token route schema and its OpenAPI assertions
around the workload token exchange operation to include the reachable 401
invalid_client response alongside 200 and 400. Define and verify the 401
description and error schema using the existing error-response conventions,
while preserving the current success and 400 assertions.
In `@tests/auth_idp/authentik_live.py`:
- Line 21: Update AUTHENTIK_GATEWAY_TLS_CA_BUNDLE and the related references in
the test harness to derive the certificate path from AUTHENTIK_GATEWAY_TLS_DIR
instead of hardcoding the default directory, preserving the tls.crt filename so
directory overrides are honored consistently.
In `@tests/auth_idp/contracts/test_gateway.py`:
- Around line 74-85: Update test_provider_gateway_accepts_human_token to request
a known-permitted resource through the gateway, then require a successful 2xx
response instead of accepting 403. Preserve the human-token authorization setup
and existing runtime TLS configuration.
In `@tests/auth_idp/runtime_kubernetes.py`:
- Around line 667-674: Update the cluster cleanup flow around the diagnostics
collection and teardown logic so failures from _collect_kubernetes_diagnostics
are suppressed, allowing the original startup error to propagate. Wrap
diagnostic collection in a best-effort try/finally structure and ensure
_delete_cluster runs in the finally path whenever the cluster is not being
reused or kept, then clear self.cluster afterward.
In `@tests/auth_idp/static/test_authentik_kubernetes_demo.py`:
- Around line 68-80: Update the external command helpers, including
_run_authentik_script and the Helm invocation paths, to pass explicit timeout
values to subprocess.run. Define and use separate constants for the script and
Helm timeouts, preserving the existing command arguments, environment handling,
and success assertions.
---
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 1322-1323: Update the actions/checkout step in the CI workflow to
disable credential persistence by setting persist-credentials to false, while
preserving the pinned checkout action and existing step behavior. Obtain Write
permission before modifying the .github workflow file.
In `@packages/nemo_platform_ext/tests/cli/commands/test_auth.py`:
- Around line 318-329: The test
test_runtime_token_source_label_ignores_workload_identity_token_file should
create a readable token file containing a valid token, remove the legacy
NEMO_WORKLOAD_TOKEN_FILE environment variable setup, and set only
WORKLOAD_IDENTITY_TOKEN_FILE_ENVVAR before asserting
_runtime_token_source_label() returns None.
In `@tests/auth_idp/compose/test_authentik_cli_login.py`:
- Around line 81-92: Update the workspace setup flow around the create_response
call so _delete_workspace_for_cleanup runs only after workspace creation and
validation succeed. Avoid an unconditional finally cleanup, preserving the
original creation or assertion exception when setup fails.
🪄 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: 2d3635dc-26de-4e39-a491-18009d0cd264
⛔ Files ignored due to path filters (37)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/_client.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/device_flow.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/helpers.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/auth/workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/factory.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/client/tls.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/jobs/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_workload_identity_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_config_map_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_key_to_path.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_secret_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_volume.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/json_web_key.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/json_web_key_set_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/oidc_discovery_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/workload_token_exchange_error_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/shared/workload_token_exchange_response.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_device_flow.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_token_provider.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_utils.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/auth/test_workload_exchange.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_auth.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/client/test_client.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/config/test_config.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (158)
.github/actions/setup-kind-cluster/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/authentik/.gitignorecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/.helmignorecontrib/auth/authentik/helm/Chart.yamlcontrib/auth/authentik/helm/files/blueprints/nemo.yamlcontrib/auth/authentik/helm/templates/_envoy-config.tplcontrib/auth/authentik/helm/templates/_helpers.tplcontrib/auth/authentik/helm/templates/blueprint-apply-job.yamlcontrib/auth/authentik/helm/templates/blueprint-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-initdb-configmap.yamlcontrib/auth/authentik/helm/templates/shared-postgres-nemo-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-secret.yamlcontrib/auth/authentik/helm/templates/shared-postgres-service.yamlcontrib/auth/authentik/helm/templates/shared-postgres-serviceaccount.yamlcontrib/auth/authentik/helm/templates/shared-postgres-statefulset.yamlcontrib/auth/authentik/helm/templates/tokenreview-rbac.yamlcontrib/auth/authentik/helm/templates/workload-token-signing-key-secret.yamlcontrib/auth/authentik/helm/templates/workload-token-tls.yamlcontrib/auth/authentik/helm/values.yamlcontrib/auth/authentik/kubernetes/README.mdcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/deployment/configuration.mdxdocs/auth/deployment/credential-propagation.mdxdocs/set-up/config-reference.mdxe2e/authz_oidc/conftest.pye2e/backends/docker_compose.pye2e/conftest.pye2e/services_pool.pyk8s/helm/README.mdk8s/helm/ci/21-api-extra-volumes.yamlk8s/helm/ci/22-envoy-config-override.yamlk8s/helm/templates/api/api-deployment.yamlk8s/helm/templates/core/controller-deployment.yamlk8s/helm/templates/platform-seed-job.yamlk8s/helm/templates/proxy/_helpers.tplk8s/helm/templates/proxy/envoy-configmap.yamlk8s/helm/templates/proxy/envoy-deployment.yamlk8s/helm/values.yamlopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/auth/device_flow.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/helpers.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.pypackages/nemo_platform_ext/src/nemo_platform_ext/auth/workload_exchange.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/factory.pypackages/nemo_platform_ext/src/nemo_platform_ext/client/tls.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/auth/test_device_flow.pypackages/nemo_platform_ext/tests/auth/test_token_provider.pypackages/nemo_platform_ext/tests/auth/test_utils.pypackages/nemo_platform_ext/tests/auth/test_workload_exchange.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/client/test_client.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/adapter.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/client/tls.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/config.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/execution_profiles.pypackages/nemo_platform_plugin/tests/client/test_adapter.pypackages/nemo_platform_plugin/tests/test_client_auth.pypackages/nemo_platform_plugin/tests/test_config.pypackages/nmp_common/src/nmp/common/auth/middleware.pypackages/nmp_common/src/nmp/common/config/__init__.pypackages/nmp_common/src/nmp/common/config/base.pypackages/nmp_common/src/nmp/common/entities/client.pypackages/nmp_common/src/nmp/common/sdk_factory.pypackages/nmp_common/src/nmp/common/service/base.pypackages/nmp_common/tests/nmp_common/test_common_config.pypackages/nmp_common/tests/sdk_factory/test_sdk.pypackages/nmp_platform_runner/src/nmp/platform_runner/config.pypackages/nmp_platform_runner/src/nmp/platform_runner/health.pypackages/nmp_platform_runner/src/nmp/platform_runner/loader.pypackages/nmp_platform_runner/src/nmp/platform_runner/registry.pypackages/nmp_platform_runner/src/nmp/platform_runner/run.pypackages/nmp_platform_runner/src/nmp/platform_runner/server.pypackages/nmp_platform_runner/tests/test_config.pypackages/nmp_platform_runner/tests/test_run.pypackages/nmp_platform_runner/tests/test_server.pypytest.iniservices/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.pyservices/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/src/nmp/core/auth/service.pyservices/core/auth/tests/test_discovery.pyservices/core/auth/tests/test_workload_token_exchange.pyservices/core/jobs/jobs-launcher/cmd/otel.goservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/jobs-launcher/cmd/workload_auth.goservices/core/jobs/jobs-launcher/cmd/workload_auth_test.goservices/core/jobs/jobs-launcher/go.modservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/registry.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/subprocess.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/workload_tokens.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/core/jobs/tests/controllers/test_kubernetes_backend.pyservices/core/jobs/tests/controllers/test_workload_tokens.pyservices/core/jobs/tests/test_config.pyservices/core/models/tests/unit/controllers/test_deployment_reconciler.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.pyservices/hello-world/tests/integration/tasks/test_workload_workspace_get_task.pytests/auth_idp/authentik_live.pytests/auth_idp/common.pytests/auth_idp/compose/test_authentik_cli_login.pytests/auth_idp/conftest.pytests/auth_idp/contracts/test_discovery.pytests/auth_idp/contracts/test_gateway.pytests/auth_idp/contracts/test_jobs.pytests/auth_idp/contracts/test_tokens.pytests/auth_idp/contracts/test_workspace.pytests/auth_idp/k8s/test_authentik_kubernetes_live.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/runtime_compose.pytests/auth_idp/runtime_contract.pytests/auth_idp/runtime_factory.pytests/auth_idp/runtime_kubernetes.pytests/auth_idp/static/test_authentik_blueprint.pytests/auth_idp/static/test_authentik_kubernetes_demo.pytests/auth_idp/static/test_docs_links.pytests/auth_idp/static/test_fixture_helpers.pytests/auth_idp/static/test_provider_layout.pytests/auth_idp/static/test_provider_manifest.pytests/auth_idp/static/test_runtime_compose.pytests/auth_idp/static/test_runtime_selection.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_fixture_helpers.pytests/auth_idp/test_provider_layout.pytests/test_e2e_docker_compose_backend.pytests/test_e2e_services_pool.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.pytools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/vendor/vendor_package.pytools/nemo-platform-sdk-tools/tests/sdk/openapi_test.pytools/nemo-platform-sdk-tools/tests/sdk/vendor/test_vendor_package.py
💤 Files with no reviewable changes (3)
- Makefile
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
🚧 Files skipped from review as they are similar to previous changes (79)
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/constants.py
- k8s/helm/ci/21-api-extra-volumes.yaml
- k8s/helm/templates/proxy/envoy-configmap.yaml
- pytest.ini
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/tls.py
- contrib/auth/authentik/.gitignore
- k8s/helm/templates/api/api-deployment.yaml
- packages/nmp_common/src/nmp/common/config/init.py
- e2e/authz_oidc/conftest.py
- contrib/auth/authentik/helm/templates/tokenreview-rbac.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/client/tls.py
- packages/nmp_common/src/nmp/common/service/base.py
- k8s/helm/templates/proxy/envoy-deployment.yaml
- tests/auth_idp/runtime_factory.py
- packages/nmp_common/src/nmp/common/entities/client.py
- contrib/auth/authentik/helm/templates/blueprint-configmap.yaml
- docs/auth/deployment/configuration.mdx
- tools/nemo-platform-sdk-tools/tests/sdk/openapi_test.py
- k8s/helm/templates/proxy/_helpers.tpl
- docs/auth/authentication/idp-integration.mdx
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- services/core/auth/src/nmp/core/auth/service.py
- k8s/helm/README.md
- k8s/helm/values.yaml
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- tests/auth_idp/runtime.py
- tests/auth_idp/runtime_contract.py
- k8s/helm/templates/core/controller-deployment.yaml
- services/core/auth/tests/test_discovery.py
- .github/actions/setup-kind-cluster/action.yaml
- contrib/auth/authentik/manifest.yaml
- k8s/helm/ci/22-envoy-config-override.yaml
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/oidc_factory.py
- packages/nmp_common/tests/nmp_common/test_common_config.py
- packages/nmp_platform_runner/src/nmp/platform_runner/run.py
- conftest.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/enhanced.py
- contrib/auth/authentik/gateway/envoy.yaml
- services/core/jobs/jobs-launcher/go.mod
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/core/openapi.py
- contrib/auth/authentik/helm/templates/_envoy-config.tpl
- contrib/auth/authentik/docker-compose.yml
- docs/set-up/config-reference.mdx
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/token_provider.py
- packages/nmp_common/src/nmp/common/config/base.py
- services/core/jobs/jobs-launcher/cmd/workload_auth_test.go
- packages/nmp_platform_runner/src/nmp/platform_runner/registry.py
- e2e/conftest.py
- services/core/jobs/jobs-launcher/cmd/workload_auth.go
- packages/nmp_platform_runner/src/nmp/platform_runner/health.py
- services/core/jobs/jobs-launcher/cmd/otel.go
- contrib/auth/authentik/helm/files/blueprints/nemo.yaml
- tests/test_e2e_services_pool.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- packages/nemo_platform_ext/src/nemo_platform_ext/auth/device_flow.py
- contrib/auth/authentik/helm/values.yaml
- docs/auth/deployment/credential-propagation.mdx
- services/core/jobs/jobs-launcher/cmd/run.go
- services/core/auth/src/nmp/core/auth/api/v2/discovery/endpoints.py
- packages/nmp_platform_runner/tests/test_server.py
- contrib/auth/authentik/helm/Chart.yaml
- tools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/sdk/vendor/vendor_package.py
- packages/nmp_common/src/nmp/common/sdk_factory.py
- packages/nemo_platform_ext/tests/config/test_config.py
- packages/nmp_platform_runner/src/nmp/platform_runner/server.py
- tools/nemo-platform-sdk-tools/tests/sdk/vendor/test_vendor_package.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/base.py
- services/core/auth/src/nmp/core/auth/api/v2/workload_token_exchange.py
- contrib/auth/authentik/helm/templates/_helpers.tpl
- packages/nemo_platform_plugin/src/nemo_platform_plugin/config.py
- packages/nemo_platform_plugin/src/nemo_platform_plugin/client/client.py
- openapi/ga/individual/platform.openapi.yaml
- openapi/openapi.yaml
- openapi/ga/openapi.yaml
- packages/nmp_platform_runner/src/nmp/platform_runner/config.py
- packages/nemo_platform_ext/src/nemo_platform_ext/client/factory.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
- tests/auth_idp/providers.py
6763f1b to
ec5c8b3
Compare
8a72d58 to
683fd06
Compare
Replace legacy static bearer-token env vars (NEMO_WORKLOAD_TOKEN / NEMO_WORKLOAD_TOKEN_FILE) with managed workload identity token exchange driven by NMP_WORKLOAD_IDENTITY_TOKEN_FILE. Managed job backends own the subject-token file: Docker writes and refreshes an Authentik subject token in a workload identity volume, while Kubernetes projects a service account token into the workload pod. The SDK reads that subject-token file just in time and exchanges it for a NeMo Platform access token through OAuth 2.0 Token Exchange (RFC 8693). NMP_ACCESS_TOKEN remains the explicit bearer-token override. The legacy NEMO_WORKLOAD_TOKEN and NEMO_WORKLOAD_TOKEN_FILE env vars are ignored by client config loading and are reserved/rejected in managed job environments. Add NeMo auth service token/JWKS endpoints, workload exchange discovery metadata, Authentik Compose and Helm reference flows, generated OpenAPI/SDK updates, and docs/tests for the managed backend ownership model. Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
683fd06 to
35f7d6e
Compare
Post-Merge Note
The Authentik Compose merge-group CI failure from https://github.com/NVIDIA-NeMo/nemo-platform/actions/runs/29758464168/job/88408033334 is addressed separately in #788. The deterministic
authentik-blueprint-initcompose change is not part of this merged PR.Summary
Replace legacy static workload bearer-token envs (
NEMO_WORKLOAD_TOKEN/NEMO_WORKLOAD_TOKEN_FILE) with managed workload identity token exchange. Managed job backends now injectNMP_WORKLOAD_IDENTITY_TOKEN_FILE, pointing at a backend-owned subject-token file, and the SDK exchanges that subject token for a NeMo Platform access token using OAuth 2.0 Token Exchange (RFC 8693).NMP_ACCESS_TOKENremains the explicit user override. The legacyNEMO_WORKLOAD_TOKEN/NEMO_WORKLOAD_TOKEN_FILEenv vars are no longer client bootstrap sources; managed job specs also reserve/reject those names andNMP_WORKLOAD_IDENTITY_TOKEN_FILEso users cannot override platform-owned workload auth material.Changes
Workload Token Exchange
/apis/auth/tokenfor RFC 8693 token exchange and/apis/auth/jwksfor the NeMo-signed workload access-token JWKSworkload_token_exchange_enabled,workload_client_id,workload_token_endpoint,workload_audience,workload_scope)SDK / Client
WorkloadTokenExchangeProvider, which readsNMP_WORKLOAD_IDENTITY_TOKEN_FILE, sends an RFC 8693 exchange request, and caches the returned access token until near expirynemo_platform_extandnemo_platform_pluginclient bootstrap pathsNMP_ACCESS_TOKENas the highest-precedence explicit bearer-token overrideNEMO_WORKLOAD_TOKENandNEMO_WORKLOAD_TOKEN_FILE; those legacy env vars are now ignored by client config loadingJob Backends
NMP_WORKLOAD_IDENTITY_TOKEN_FILENMP_WORKLOAD_IDENTITY_TOKEN_FILENMP_BASE_URL,NMP_AUTH_URL,NMP_JOBS_URL,NMP_FILES_URL,NMP_MODELS_URL,NMP_SECRETS_URL)NEMO_WORKLOAD_TOKENOTEL-header injection pathAuthentik Reference Demo
contrib/auth/authentik/helm/files/blueprints/nemo.yamlsvc-nemo), and E2E setup (nemo-setup)contrib/auth/authentik/compose/to route through Envoy, share generated local TLS/signing material, and use the NeMo auth service for workload token exchangecontrib/auth/authentik/helm/that depends on the official Authentik chart and the repok8s/helmNeMo Platform chartrun.shwithcompose,k8s,prepare-local,render-blueprint,run-local, anddownflows plus reuse/diagnostics optionsPlatform Config, OpenAPI, And Docs
OIDCConfigNMP_WORKLOAD_IDENTITY_TOKEN_FILEand the managed backend ownership modelHello-World Workload Task
workload_workspace_getto constructNeMoPlatform()directly; SDK bootstrap now handles managed workload token exchange automaticallyTesting
tests/auth_idp/contractsfor CLI refresh, discovery, gateway routing, tokens, workspace access, and workload jobscontrib/auth/authentik/run.sh composeandcontrib/auth/authentik/run.sh k8s