feat(auth): add authentik IdP reference example#524
Merged
Merged
Conversation
Contributor
|
🌿 Preview your docs: https://nvidia-preview-auth-idp.docs.buildwithfern.com/nemo-platform |
Contributor
|
ironcommit
force-pushed
the
auth-idp
branch
4 times, most recently
from
June 30, 2026 21:16
8d6fdee to
359a925
Compare
ironcommit
changed the title
ironcommit
marked this pull request as ready for review
June 30, 2026 23:07
Contributor
|
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:
📝 WalkthroughWalkthroughAdds auth-idp CI detection and Authentik reference assets, backend-aware E2E harnessing, auth_idp loaders/fixtures/tests, and a workload access-token flow across SDK config, CLI auth, job authorization, Docker finalization, and a sample task. ChangesAuth-IdP E2E Integration
Workload Access Token Support
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/services_pool.py (1)
53-72: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStop sidecar handles on teardown.
_terminate_services()never closesservices.sidecar_handles, so e2e sidecars can leak past pool shutdown. MoveRunningSidecaraboveRunningServicesif you switchsidecar_handlestolist[RunningSidecar].🤖 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 `@e2e/services_pool.py` around lines 53 - 72, _teardown in _terminate_services() does not close services.sidecar_handles, so sidecars can survive pool shutdown. Update the RunningServices teardown path to iterate over sidecar_handles and call each RunningSidecar.close before clearing the pool state. If you change the type annotation to list[RunningSidecar], place RunningSidecar before RunningServices so the reference resolves cleanly.Source: Coding guidelines
🧹 Nitpick comments (4)
services/core/auth/tests/integration/test_gateway_header_spoofing.py (1)
27-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded indices assume a single listener/filter-chain/cluster per provider.
listeners[0],filter_chains[0],filters[0],virtual_hosts[0],clusters[0]will break with anIndexError/KeyErrorif a future provider's envoy config (Keycloak, Zitadel, etc.) uses multiple listeners or a different filter chain ordering. Consider searching by name/type (e.g., find the HTTP connection manager filter, find listener namedlistener_0) instead of positional indexing.🤖 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/tests/integration/test_gateway_header_spoofing.py` around lines 27 - 43, The gateway Envoy tests rely on brittle positional access in test_all_compose_backed_gateways_strip_trusted_identity_headers and test_all_compose_backed_gateways_define_minimal_envoy_runtime, which can fail if a provider has multiple listeners, filter chains, or clusters. Update the test helpers to locate the HTTP connection manager, virtual host, listener, and cluster by name/type rather than using listeners[0], filter_chains[0], filters[0], virtual_hosts[0], and clusters[0]. Keep the same assertions, but make the lookup logic resilient to different Envoy config layouts across providers.contrib/auth/authentik/README.md (1)
7-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit this into one docs quadrant and add the required scaffolding.
## Local referenceis a HOW-TO, while## Production adaptationis EXPLANATION. Keep one here, move the other to a linked page, add prerequisites before the steps, and finish withNext Steps.
As per coding guidelines, "Each documentation page should fit ONE Diataxis quadrant; do not mix tutorials with reference tables or how-tos with architecture explanations; use cross-links instead", "Always list prerequisites at the top of documentation pages before other content", and "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/README.md` around lines 7 - 21, The README mixes a HOW-TO section with an explanation section, so split it into a single Diataxis quadrant page and move the other section to a linked page. In the Authentik docs, keep only one purpose under the existing headings, add a prerequisites section before any steps in the Local reference content, and ensure the page ends with a Next Steps section linking to the related page. Preserve the existing Authentik-specific guidance while relocating either the `## Local reference` steps or the `## Production adaptation` notes to the appropriate cross-linked document.Source: Coding guidelines
tests/auth_idp/providers.py (1)
68-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
load_provider_matrix()is still hardcoded to Authentik.That bypasses the manifest-driven contract this PR introduces: a second
compose-ciprovider can validate and load correctly, but it will never reach the live matrix. If this helper is meant to reflect manifests, filter bymodeonly.🤖 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/providers.py` around lines 68 - 72, The live matrix helper is still hardcoded to the Authentik provider, which prevents other compose-ci providers from being included. Update load_provider_matrix() in providers.py to rely on the manifest-driven provider set by filtering only on mode == "compose-ci" and remove the provider.name == "authentik" restriction. Keep the change localized to load_provider_matrix() so any future compose-ci ProviderConfig entries returned by load_provider_configs() are automatically picked up.docs/auth/authentication/idp-integration.mdx (1)
1-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis page mixes a how-to title with reference content.
Pick one Diataxis quadrant here. As written, the title reads like a task page, but the body is just a short reference list. Please add prerequisites at the top and a
Next Stepssection, or rename/reframe the page as pure reference. As per coding guidelines, "Each documentation page should fit ONE Diataxis quadrant; do not mix tutorials with reference tables or how-tos with architecture explanations" and "Always list prerequisites at the top of documentation pages before other content" and "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 `@docs/auth/authentication/idp-integration.mdx` around lines 1 - 15, This page mixes task-oriented and reference-style content, so align it to one Diataxis quadrant by either reframing the title/body as pure reference or expanding it into a true how-to. Update the page anchored by the frontmatter title and the “Provider References” section to include prerequisites at the top, then add a “Next Steps” section at the end with links to related docs. If you keep the current content, rename the page so the title matches a reference page rather than an integration task.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 @.github/actions/changes/action.yaml:
- Around line 103-106: The auth-idp path filter is too narrow and misses root
harness/CI files, so updates there will not trigger the auth-idp build/test
workflow. Expand the `auth-idp` changeset in
`.github/actions/changes/action.yaml` to include the root auth-idp harness and
pipeline files that affect `python-auth-idp-test`, specifically `conftest.py`,
`pytest.ini`, `Makefile`, and the relevant CI workflow files, alongside the
existing `tests/auth_idp/**`, `e2e/**`, and `contrib/auth/**` patterns.
In @.github/workflows/ci.yaml:
- Around line 916-956: The auth-idp job is trying to consume image tags from
GHCR even when the build job only loaded them locally on its runner; update this
job so it does not pass IMAGE_REGISTRY and BAKE_TAG from build-cpu-smoke-images
unless publish_images is true. Use the existing job symbols such as
build-cpu-smoke-images.outputs.publish_images, IMAGE_REGISTRY, BAKE_TAG, and
e2e/services_pool.py image resolution to either gate the test step on published
images or rebuild/load the images within this job for non-publishing runs.
In `@contrib/auth/manifest.schema.yaml`:
- Around line 2-53: The manifest schema is missing required contract fields used
by the auth harness, so typos can slip through validation and fail later at
runtime. Update manifest.schema.yaml to model the full contract expected by
load_provider_config() and the token helper by adding token_acquisition and the
missing machine_identity fields such as principal_id and expected_groups, and
tighten the object shapes in principal_contract, human_identity,
machine_identity, and token_acquisition with required properties and
additionalProperties: false where appropriate.
In `@e2e/services_pool.py`:
- Around line 96-98: The teardown path is missing sidecar cleanup, so Authentik
compose stacks can remain running after tests or process exit. Update
`_terminate_services()` to invoke `_stop_sidecar_handles()` for any populated
`sidecar_handles`, alongside the existing service shutdown flow, and make sure
the handles are closed in the same teardown sequence used by the service pool
cleanup logic.
- Around line 87-90: The e2e sidecar validation in the normalization path is too
narrow: `e2e_sidecars` values that are not mappings (for example a scalar) can
still reach `.items()`/`.get()` and fail later instead of being rejected up
front. Update the sidecar parsing/validation logic around `services_pool` and
the `start_authentik_sidecar` call sites so any non-dict `e2e_sidecars` shape
raises `pytest.UsageError` immediately, while still allowing only the supported
`"authentik"` entry.
In `@services/core/auth/tests/test_machine_group_authz.py`:
- Around line 41-47: The machine-group authz tests are asserting access
immediately after creating a role binding, which can race the PDP refresh loop
configured in AuthConfig. Update the affected test setup and assertions in the
machine group auth tests to wait for role propagation before checking for 200,
using the existing authz helper/test flow around the role-binding creation and
access check so the read only happens after the PDP has had time to refresh.
- Around line 21-24: The standalone prefix assertion in
test_external_machine_identity_uses_group_binding_without_service_prefix does
not exercise the authorization contract, so it should be removed or folded into
the end-to-end access test that actually validates machine identity
authorization. Update the relevant auth test in test_machine_group_authz.py to
assert prefix handling only as part of the real access flow, using the existing
test helpers and auth path rather than checking a locally generated string in
isolation.
In `@tests/auth_idp/compose.py`:
- Around line 135-152: The token polling loop in the machine-token helper
currently retries only on 5xx responses, but httpx.RequestError from
httpx.post() will fail immediately during startup. Update the token poll logic
in the function that reads provider.token_endpoint to catch request/connection
errors around the httpx.post call, keep retrying until the deadline, and only
raise after the loop if the endpoint never becomes reachable.
In `@tests/auth_idp/conftest.py`:
- Around line 80-89: The token request in the auth_idp conftest setup is
hardcoding password-flow fields, which breaks grant types like
client_credentials. Update the request-building logic around the token exchange
in the relevant setup/helper function to construct the form body from the
selected grant dict and only include fields that exist for that grant. Handle
the grant_type case explicitly so password grants include
username/password/scope, while client_credentials uses only its supported
parameters, avoiding KeyError and incorrect payloads.
In `@tests/auth_idp/runtime.py`:
- Around line 20-23: The _find_free_port helper is racy because it closes the
socket before Compose starts, so another process can take the port and the same
host port may also be reused for both mappings. Update the auth_idp runtime flow
to avoid pre-picking ports: either keep the sockets reserved until the
containers are started, or switch the harness to let Docker assign published
ports and read them back afterward. Focus on the _find_free_port function and
the startup/mapping logic that consumes its result.
In `@tests/auth_idp/test_authentik_gateway_live.py`:
- Line 9: The module-level AUTHENTIK_RUNTIME initialization is running
get_authentik_docker_test_runtime() at import/collection time, causing
unconditional port allocation and runtime rendering. Move this logic into a
session-scoped fixture in test_authentik_gateway_live.py (or a nearby shared
fixture) and have the tests access that fixture instead of a global so the
Authentik runtime is only created when the docker-backed auth_idp tests actually
execute.
- Around line 33-42: The spoofed-header test only proves unauthenticated
requests are rejected, so it does not verify that the gateway strips
X-NMP-Principal-* headers. Update
test_authentik_gateway_rejects_spoofed_principal_headers to obtain a valid
machine token using authentik_stack.wait_for_token_endpoint and
authentik_stack.token_endpoint, send it in Authorization with the spoofed
headers, and then assert the resolved identity corresponds to the authenticated
principal rather than the injected X-NMP-Principal-Id/X-NMP-Principal-Email
values.
In `@tests/auth_idp/test_authentik_real_oidc.py`:
- Around line 34-36: The auth tests for the deny path are too broad because they
accept any APIStatusError, so tighten the assertions in the machine identity
tests to verify a 403 forbidden response instead. Update the relevant
pytest.raises checks in the authentik OIDC test cases, capture the exception
object, and assert its status code or forbidden type in the test methods such as
test_authentik_machine_identity_is_denied_before_binding and the other deny-path
test around the same section.
In `@tests/auth_idp/test_docs_links.py`:
- Around line 11-18: The test in test_auth_docs_link_to_contrib_references does
not actually enforce the docs contract because manifest_path.exists() is
guaranteed by the glob. Update the check so each provider discovered from
contrib/auth/manifest.yaml is explicitly validated against
docs/auth/authentication/idp-integration.mdx using the provider_path derived
from manifest_path.parent.name, and assert that the docs content contains that
reference for every manifest.
---
Outside diff comments:
In `@e2e/services_pool.py`:
- Around line 53-72: _teardown in _terminate_services() does not close
services.sidecar_handles, so sidecars can survive pool shutdown. Update the
RunningServices teardown path to iterate over sidecar_handles and call each
RunningSidecar.close before clearing the pool state. If you change the type
annotation to list[RunningSidecar], place RunningSidecar before RunningServices
so the reference resolves cleanly.
---
Nitpick comments:
In `@contrib/auth/authentik/README.md`:
- Around line 7-21: The README mixes a HOW-TO section with an explanation
section, so split it into a single Diataxis quadrant page and move the other
section to a linked page. In the Authentik docs, keep only one purpose under the
existing headings, add a prerequisites section before any steps in the Local
reference content, and ensure the page ends with a Next Steps section linking to
the related page. Preserve the existing Authentik-specific guidance while
relocating either the `## Local reference` steps or the `## Production
adaptation` notes to the appropriate cross-linked document.
In `@docs/auth/authentication/idp-integration.mdx`:
- Around line 1-15: This page mixes task-oriented and reference-style content,
so align it to one Diataxis quadrant by either reframing the title/body as pure
reference or expanding it into a true how-to. Update the page anchored by the
frontmatter title and the “Provider References” section to include prerequisites
at the top, then add a “Next Steps” section at the end with links to related
docs. If you keep the current content, rename the page so the title matches a
reference page rather than an integration task.
In `@services/core/auth/tests/integration/test_gateway_header_spoofing.py`:
- Around line 27-43: The gateway Envoy tests rely on brittle positional access
in test_all_compose_backed_gateways_strip_trusted_identity_headers and
test_all_compose_backed_gateways_define_minimal_envoy_runtime, which can fail if
a provider has multiple listeners, filter chains, or clusters. Update the test
helpers to locate the HTTP connection manager, virtual host, listener, and
cluster by name/type rather than using listeners[0], filter_chains[0],
filters[0], virtual_hosts[0], and clusters[0]. Keep the same assertions, but
make the lookup logic resilient to different Envoy config layouts across
providers.
In `@tests/auth_idp/providers.py`:
- Around line 68-72: The live matrix helper is still hardcoded to the Authentik
provider, which prevents other compose-ci providers from being included. Update
load_provider_matrix() in providers.py to rely on the manifest-driven provider
set by filtering only on mode == "compose-ci" and remove the provider.name ==
"authentik" restriction. Keep the change localized to load_provider_matrix() so
any future compose-ci ProviderConfig entries returned by load_provider_configs()
are automatically picked up.
🪄 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: f2e8c010-73de-4b2c-a194-755e92fb7e96
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (46)
.github/actions/changes/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/README.mdcontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/seed/README.mdcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/authentication/index.mdxdocs/fern/gated-nav.ymle2e/configs/auth-idp-docker.yamle2e/conftest.pye2e/services_pool.pypackages/nmp_testing/src/nmp/testing/e2e/docker.pypytest.iniservices/core/auth/tests/integration/test_gateway_header_spoofing.pyservices/core/auth/tests/test_machine_group_authz.pytests/auth_idp/compose.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/sidecars.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_runtime_render.pytests/auth_idp/test_authentik_stack_smoke.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_harness_unit.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_provider_matrix.pytests/auth_idp/test_services_pool_authentik_sidecar.pytests/auth_idp/test_services_pool_sidecar_contract.pytests/auth_idp/test_xdist_grouping.pytests/auth_idp/xdist.pytests/test_e2e_services_pool.py
ironcommit
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/ci.yaml (1)
916-968: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
timeout-minutesto bound this job.Docker-based E2E/OIDC tests can hang; other similarly heavy jobs (
build-cpu-smoke-images: 90m,python-e2e-test: 15m) cap runtime but this job doesn't.Proposed fix
python-auth-idp-test: name: Python auth-idp tests needs: [changes, policy-wasm, build-cpu-smoke-images] + timeout-minutes: 30 if: >🤖 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 916 - 968, The python auth-idp job is missing a runtime bound, so add a timeout to the `python-auth-idp-test` workflow job to match other long-running E2E jobs. Update the job definition for `python-auth-idp-test` in the CI workflow by adding `timeout-minutes` with an appropriate cap consistent with `build-cpu-smoke-images` and `python-e2e-test`, keeping the existing `needs`, `if`, and test steps unchanged.
🤖 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/docker-compose.yml`:
- Around line 11-24: The nemo service currently has no healthcheck, which makes
gateway’s depends_on service_healthy dependency invalid and breaks startup. Add
a healthcheck to the nemo service in the docker-compose definition, using nemo’s
existing runtime/HTTP endpoint or another reliable readiness check, so gateway
can wait on a healthy nemo instance. Keep the change aligned with the existing
nemo service block and ensure gateway’s service_healthy dependency remains
valid.
- Around line 20-23: Remove the unnecessary Docker socket bind mount from the
nemo service in the docker-compose configuration. The service is already
configured for jobs.executors with backend subprocess and runtime none, so keep
only the existing config and data volumes and delete the /var/run/docker.sock
entry from the volume list.
---
Nitpick comments:
In @.github/workflows/ci.yaml:
- Around line 916-968: The python auth-idp job is missing a runtime bound, so
add a timeout to the `python-auth-idp-test` workflow job to match other
long-running E2E jobs. Update the job definition for `python-auth-idp-test` in
the CI workflow by adding `timeout-minutes` with an appropriate cap consistent
with `build-cpu-smoke-images` and `python-e2e-test`, keeping the existing
`needs`, `if`, and test steps unchanged.
🪄 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: 3e0202b2-5ded-44ed-868a-6f68b8e8490f
📒 Files selected for processing (22)
.github/actions/changes/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/README.mdcontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/seed/README.mdcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/authentication/index.mdxdocs/fern/gated-nav.ymle2e/configs/auth-idp-docker.yamle2e/conftest.pye2e/services_pool.pypackages/nmp_testing/src/nmp/testing/e2e/docker.pypytest.ini
✅ Files skipped from review due to trivial changes (6)
- contrib/auth/authentik/seed/README.md
- docs/auth/authentication/index.mdx
- docs/auth/authentication/idp-integration.mdx
- contrib/auth/README.md
- pytest.ini
- contrib/auth/authentik/README.md
🚧 Files skipped from review as they are similar to previous changes (12)
- docs/fern/gated-nav.yml
- contrib/auth/authentik/config/nemo-auth.yaml
- contrib/auth/authentik/gateway/envoy.yaml
- .github/actions/changes/action.yaml
- contrib/auth/manifest.schema.yaml
- e2e/conftest.py
- contrib/auth/authentik/manifest.yaml
- contrib/auth/authentik/blueprints/nemo.yaml
- e2e/configs/auth-idp-docker.yaml
- conftest.py
- packages/nmp_testing/src/nmp/testing/e2e/docker.py
- e2e/services_pool.py
Contributor
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 (1)
.github/workflows/ci.yaml (1)
1579-1608: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
python-auth-idp-testmissing fromci-statusneeds.New job isn't wired into the aggregate status check, so its failures won't block CI.
Fix
- opa-policy-test + - python-auth-idp-test - guardrails-benchmark - guardrails-benchmark-analyze🤖 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 1579 - 1608, The aggregate CI gate in ci-status is missing the new python-auth-idp-test job, so its result is not being enforced. Update the ci-status needs list to include python-auth-idp-test alongside the other Python test jobs, using the ci-status workflow job name to locate the section. Keep the dependency ordering consistent with the existing needs entries so failures in python-auth-idp-test block the overall status check.
♻️ Duplicate comments (3)
.github/workflows/ci.yaml (2)
917-926: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winJob still lacks
publish_imagesgate.This exact gap was flagged previously and marked "Addressed," but the current
if:doesn't checkpublish_images. Non-publishing runs will pullIMAGE_REGISTRY/BAKE_TAGvalues that were never pushed (build job onlydocker-loads locally when not publishing), so this job will fail on forked PRs / non-publish events.Fix
if: > !cancelled() && ( github.event_name == 'workflow_dispatch' || needs.changes.outputs.cpu-smoke == 'true' || needs.changes.outputs.auth-idp == 'true' ) && + needs.build-cpu-smoke-images.outputs.publish_images == 'true' && needs.build-cpu-smoke-images.result == 'success'Also applies to: 959-960
🤖 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 917 - 926, The python auth-idp test jobs still run without the publish_images guard, so they can attempt to use image tags that were only loaded locally and never pushed. Update the `if:` condition for `python-auth-idp-test` and the other affected auth-idp job(s) to also require `needs.changes.outputs.publish_images == 'true'`, alongside the existing `changes` and `build-cpu-smoke-images` checks, so these jobs only execute when the images are actually published.
932-933: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSet
persist-credentials: falseon checkout.Inconsistent with other checkout steps in this file; zizmor flags credential persistence risk.
Fix
- name: Checkout code uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: 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 @.github/workflows/ci.yaml around lines 932 - 933, The Checkout code step in the CI workflow is missing the same credential-safety setting used elsewhere. Update the actions/checkout usage in the checkout job so it explicitly sets persist-credentials to false, matching the other checkout steps and preventing credentials from being left on disk.Source: Linters/SAST tools
tests/auth_idp/test_authentik_compose_helpers.py (1)
89-171: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
wait_for_token_endpointstill hardcodes password-flow fields — test data masks it.Per
tests/auth_idp/compose.py:154-188(relevant snippet),wait_for_token_endpointbuilds the token body unconditionally withusername/password/scope, the same bug already fixed inconftest.pyvia_token_request_body. These tests pass because themachine_grantfixtures here includeusername/passwordeven forclient_credentials, so a real client_credentials grant without those keys will still raiseKeyErrorin production use.🐛 Suggested fix (in compose.py)
- response = httpx.post( - provider.token_endpoint, - data={ - "grant_type": provider.machine_grant["grant_type"], - "client_id": provider.machine_grant["client_id"], - "client_secret": provider.machine_grant["client_secret"], - "username": provider.machine_grant["username"], - "password": provider.machine_grant["password"], - "scope": provider.machine_grant["scope"], - }, + response = httpx.post( + provider.token_endpoint, + data=_token_request_body(provider.machine_grant), timeout=30.0, )🤖 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/test_authentik_compose_helpers.py` around lines 89 - 171, The `wait_for_token_endpoint` logic is still using password-flow fields directly, and these tests currently hide that by putting `username`/`password` into the `machine_grant` fixture. Update the tests in `test_authentik_compose_helpers` to model a real `client_credentials` grant without password-only keys, so they validate the shared token-body builder behavior instead of masking the bug. Use the existing `wait_for_token_endpoint` and `ProviderConfig` setup to keep the retry assertions, but make the fixture data match the actual grant type.
🧹 Nitpick comments (2)
Makefile (1)
129-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated env-var defaults across three lines.
IMAGE_REGISTRY:-local/BAKE_TAG:-authentik-localare re-declared identically on lines 133, 135, and 136. Hoist into Make variables to avoid drift if a default changes.♻️ Proposed consolidation
+IMAGE_REGISTRY ?= local +BAKE_TAG ?= authentik-local + .PHONY: run-contrib-auth-authentik run-contrib-auth-authentik: ## Build/load the local nmp-api image and run the Authentik example stack in the foreground - IMAGE_REGISTRY=$${IMAGE_REGISTRY:-local} BAKE_TAG=$${BAKE_TAG:-authentik-local} $(MAKE) docker-load TARGET=docker-cpu + IMAGE_REGISTRY=$(IMAGE_REGISTRY) BAKE_TAG=$(BAKE_TAG) $(MAKE) docker-load TARGET=docker-cpu `@set` -e; \ - trap 'cd $(AUTHENTIK_EXAMPLE_COMPOSE_DIR) && IMAGE_REGISTRY=$${IMAGE_REGISTRY:-local} BAKE_TAG=$${BAKE_TAG:-authentik-local} docker compose down -v' EXIT INT TERM; \ - cd $(AUTHENTIK_EXAMPLE_COMPOSE_DIR) && IMAGE_REGISTRY=$${IMAGE_REGISTRY:-local} BAKE_TAG=$${BAKE_TAG:-authentik-local} docker compose up + trap 'cd $(AUTHENTIK_EXAMPLE_COMPOSE_DIR) && IMAGE_REGISTRY=$(IMAGE_REGISTRY) BAKE_TAG=$(BAKE_TAG) docker compose down -v' EXIT INT TERM; \ + cd $(AUTHENTIK_EXAMPLE_COMPOSE_DIR) && IMAGE_REGISTRY=$(IMAGE_REGISTRY) BAKE_TAG=$(BAKE_TAG) docker compose up🤖 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 `@Makefile` around lines 129 - 136, The run-contrib-auth-authentik target repeats the same IMAGE_REGISTRY and BAKE_TAG default expansion in multiple shell commands, which risks drift. Hoist those defaults into Make variables near AUTHENTIK_EXAMPLE_COMPOSE_DIR and reference them from the target’s docker-load, trap cleanup, and docker compose up commands so the Authentik flow uses one shared source of truth.tests/auth_idp/compose.py (1)
27-31: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on
docker composesubprocess calls.A hung Docker daemon/compose invocation blocks indefinitely with no bound.
Example fix
def _compose_services(compose_file: ComposeFiles, project_name: str | None, *extra_args: str) -> set[str]: args = _compose_base_args(compose_file, project_name) args.extend(extra_args) - result = subprocess.run(args, check=True, capture_output=True, text=True) + result = subprocess.run(args, check=True, capture_output=True, text=True, timeout=60) return {line for line in result.stdout.splitlines() if line}Apply similarly to the other
subprocess.runcalls in this file (compose_up,compose_down,compose_published_port,collect_compose_diagnostics).Also applies to: 67-98, 101-104, 107-121
🤖 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.py` around lines 27 - 31, The docker compose subprocess calls in _compose_services, compose_up, compose_down, compose_published_port, and collect_compose_diagnostics do not set a timeout, so a hung daemon can block forever. Update each subprocess.run invocation to use a bounded timeout, keeping the existing check/capture_output/text behavior, and make sure any timeout handling propagates a useful failure in these helper functions.
🤖 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 `@Makefile`:
- Around line 125-127: The test-auth-idp Makefile target is running pytest
without the xdist scheduler, so it won’t exercise the auth-idp scheduler hook.
Update the test-auth-idp recipe to invoke pytest with xdist enabled, using the
existing auth_idp test path and the test-auth-idp target name as the locator.
Keep the change limited to that target so the scheduler is used whenever this
suite is run.
In `@tests/auth_idp/conftest.py`:
- Around line 59-68: `idp_e2e_enabled` currently hardcodes `True`, so
`require_idp_e2e` can never skip and the `--run-e2e` gate is ineffective. Update
`idp_e2e_enabled(pytestconfig)` to read the relevant pytest option from
`pytestconfig` instead of returning a constant, and keep `require_idp_e2e` using
that value to decide whether to call `pytest.skip`. Make sure the option name
matches the existing `--run-e2e` message and the fixture names remain
`idp_e2e_enabled` and `require_idp_e2e`.
---
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 1579-1608: The aggregate CI gate in ci-status is missing the new
python-auth-idp-test job, so its result is not being enforced. Update the
ci-status needs list to include python-auth-idp-test alongside the other Python
test jobs, using the ci-status workflow job name to locate the section. Keep the
dependency ordering consistent with the existing needs entries so failures in
python-auth-idp-test block the overall status check.
---
Duplicate comments:
In @.github/workflows/ci.yaml:
- Around line 917-926: The python auth-idp test jobs still run without the
publish_images guard, so they can attempt to use image tags that were only
loaded locally and never pushed. Update the `if:` condition for
`python-auth-idp-test` and the other affected auth-idp job(s) to also require
`needs.changes.outputs.publish_images == 'true'`, alongside the existing
`changes` and `build-cpu-smoke-images` checks, so these jobs only execute when
the images are actually published.
- Around line 932-933: The Checkout code step in the CI workflow is missing the
same credential-safety setting used elsewhere. Update the actions/checkout usage
in the checkout job so it explicitly sets persist-credentials to false, matching
the other checkout steps and preventing credentials from being left on disk.
In `@tests/auth_idp/test_authentik_compose_helpers.py`:
- Around line 89-171: The `wait_for_token_endpoint` logic is still using
password-flow fields directly, and these tests currently hide that by putting
`username`/`password` into the `machine_grant` fixture. Update the tests in
`test_authentik_compose_helpers` to model a real `client_credentials` grant
without password-only keys, so they validate the shared token-body builder
behavior instead of masking the bug. Use the existing `wait_for_token_endpoint`
and `ProviderConfig` setup to keep the retry assertions, but make the fixture
data match the actual grant type.
---
Nitpick comments:
In `@Makefile`:
- Around line 129-136: The run-contrib-auth-authentik target repeats the same
IMAGE_REGISTRY and BAKE_TAG default expansion in multiple shell commands, which
risks drift. Hoist those defaults into Make variables near
AUTHENTIK_EXAMPLE_COMPOSE_DIR and reference them from the target’s docker-load,
trap cleanup, and docker compose up commands so the Authentik flow uses one
shared source of truth.
In `@tests/auth_idp/compose.py`:
- Around line 27-31: The docker compose subprocess calls in _compose_services,
compose_up, compose_down, compose_published_port, and
collect_compose_diagnostics do not set a timeout, so a hung daemon can block
forever. Update each subprocess.run invocation to use a bounded timeout, keeping
the existing check/capture_output/text behavior, and make sure any timeout
handling propagates a useful failure in these helper functions.
🪄 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: 33ff7047-8a9f-4521-83c3-cf3472cfb6e3
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (46)
.github/actions/changes/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/README.mdcontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/seed/README.mdcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/authentication/index.mdxdocs/fern/gated-nav.ymle2e/conftest.pye2e/services_pool.pypackages/nmp_testing/src/nmp/testing/e2e/docker.pypytest.iniservices/core/auth/tests/integration/test_gateway_header_spoofing.pyservices/core/auth/tests/test_machine_group_authz.pytests/auth_idp/authentik_live.pytests/auth_idp/compose.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/sidecars.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_runtime_render.pytests/auth_idp/test_authentik_stack_smoke.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_provider_matrix.pytests/auth_idp/test_services_pool_authentik_sidecar.pytests/auth_idp/test_services_pool_sidecar_contract.pytests/auth_idp/test_xdist_grouping.pytests/auth_idp/xdist.pytests/test_e2e_services_pool.py
✅ Files skipped from review due to trivial changes (7)
- contrib/auth/authentik/seed/README.md
- contrib/auth/authentik/gateway/envoy.yaml
- docs/fern/gated-nav.yml
- docs/auth/authentication/index.mdx
- contrib/auth/README.md
- contrib/auth/authentik/README.md
- docs/auth/authentication/idp-integration.mdx
🚧 Files skipped from review as they are similar to previous changes (23)
- pytest.ini
- tests/auth_idp/test_provider_layout.py
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/test_xdist_grouping.py
- tests/auth_idp/xdist.py
- contrib/auth/authentik/manifest.yaml
- e2e/conftest.py
- tests/auth_idp/test_authentik_fixture_cleanup.py
- contrib/auth/manifest.schema.yaml
- contrib/auth/authentik/blueprints/nemo.yaml
- tests/auth_idp/test_provider_manifest.py
- .github/actions/changes/action.yaml
- contrib/auth/authentik/config/nemo-auth.yaml
- tests/auth_idp/sidecars.py
- tests/auth_idp/test_authentik_stack_smoke.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- tests/auth_idp/test_provider_matrix.py
- tests/auth_idp/providers.py
- packages/nmp_testing/src/nmp/testing/e2e/docker.py
- contrib/auth/authentik/docker-compose.yml
- services/core/auth/tests/integration/test_gateway_header_spoofing.py
- conftest.py
- e2e/services_pool.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/auth_idp/runtime.py`:
- Around line 21-24: The test helper _container_reachable_host maps local
loopback hosts to host.docker.internal, but the gateway container in the
Authentik compose setup does not resolve that name on Linux. Update the gateway
service in docker-compose.yml to include the host.docker.internal host-gateway
extra_hosts entry so the address returned by _container_reachable_host works
consistently in tests.
🪄 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: 5d4b6984-4817-46b1-af97-dea299c1cb39
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.github/actions/changes/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/README.mdcontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/seed/README.mdcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/authentication/index.mdxdocs/fern/gated-nav.ymle2e/conftest.pye2e/services_pool.pypackages/nmp_testing/src/nmp/testing/e2e/docker.pypytest.iniservices/core/auth/tests/integration/test_gateway_header_spoofing.pyservices/core/auth/tests/test_machine_group_authz.pytests/auth_idp/authentik_live.pytests/auth_idp/compose.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/sidecars.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_runtime_render.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_provider_matrix.pytests/auth_idp/test_services_pool_authentik_sidecar.pytests/auth_idp/test_services_pool_sidecar_contract.pytests/auth_idp/test_xdist_grouping.pytests/auth_idp/xdist.pytests/test_e2e_services_pool.py
✅ Files skipped from review due to trivial changes (6)
- contrib/auth/authentik/seed/README.md
- docs/fern/gated-nav.yml
- contrib/auth/authentik/manifest.yaml
- docs/auth/authentication/index.mdx
- contrib/auth/README.md
- docs/auth/authentication/idp-integration.mdx
🚧 Files skipped from review as they are similar to previous changes (31)
- pytest.ini
- tests/auth_idp/test_authentik_fixture_cleanup.py
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/test_authentik_provider_helpers.py
- tests/auth_idp/authentik_live.py
- contrib/auth/authentik/config/nemo-auth.yaml
- .github/actions/changes/action.yaml
- tests/auth_idp/test_services_pool_sidecar_contract.py
- services/core/auth/tests/integration/test_gateway_header_spoofing.py
- tests/auth_idp/test_services_pool_authentik_sidecar.py
- tests/auth_idp/xdist.py
- tests/auth_idp/test_xdist_grouping.py
- tests/auth_idp/test_provider_layout.py
- tests/auth_idp/test_provider_manifest.py
- tests/auth_idp/test_authentik_real_oidc.py
- contrib/auth/manifest.schema.yaml
- contrib/auth/authentik/gateway/envoy.yaml
- tests/auth_idp/test_provider_matrix.py
- contrib/auth/authentik/docker-compose.yml
- tests/auth_idp/test_authentik_compose_helpers.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- tests/auth_idp/providers.py
- conftest.py
- Makefile
- contrib/auth/authentik/blueprints/nemo.yaml
- tests/test_e2e_services_pool.py
- packages/nmp_testing/src/nmp/testing/e2e/docker.py
- contrib/auth/authentik/README.md
- e2e/conftest.py
- tests/auth_idp/sidecars.py
- e2e/services_pool.py
ironcommit
force-pushed
the
auth-idp
branch
2 times, most recently
from
July 2, 2026 21:25
a369328 to
81083d3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 932-933: The Checkout code step in this job is missing the same
credential-safety setting used elsewhere in ci.yaml, so update the
actions/checkout usage for this job to disable credential persistence. Apply the
fix on the Checkout code step by adding persist-credentials: false, matching the
pattern already used in the other checkout jobs.
- Around line 917-926: The python-auth-idp-test job in the CI workflow is
missing a success check for policy-wasm, so it can still run after that
dependency fails. Update the job’s if expression for python-auth-idp-test to
explicitly require needs.policy-wasm.result == 'success' alongside the existing
build-cpu-smoke-images check, while keeping the current cancellation and
dispatch/change conditions intact.
In `@tests/auth_idp/compose.py`:
- Around line 151-165: The healthcheck polling in wait_for_healthchecks
currently uses one shared deadline for every check, which can starve later HTTP
checks of time. Move the timeout calculation so each healthcheck in the
provider.healthchecks loop gets its own deadline, and keep the existing
per-check polling logic and TimeoutError behavior for the individual check that
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: e08cadc5-66a0-4f3c-8dc0-d00359dfeb61
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.github/actions/changes/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/README.mdcontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/seed/README.mdcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/authentication/index.mdxdocs/fern/gated-nav.ymle2e/conftest.pye2e/services_pool.pypackages/nmp_testing/src/nmp/testing/e2e/docker.pypytest.iniservices/core/auth/tests/integration/test_gateway_header_spoofing.pyservices/core/auth/tests/test_machine_group_authz.pytests/auth_idp/authentik_live.pytests/auth_idp/compose.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/sidecars.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_runtime_render.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_provider_matrix.pytests/auth_idp/test_services_pool_authentik_sidecar.pytests/auth_idp/test_services_pool_sidecar_contract.pytests/auth_idp/test_xdist_grouping.pytests/auth_idp/xdist.pytests/test_e2e_services_pool.py
✅ Files skipped from review due to trivial changes (8)
- contrib/auth/authentik/config/nemo-auth.yaml
- docs/auth/authentication/idp-integration.mdx
- docs/fern/gated-nav.yml
- contrib/auth/README.md
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- docs/auth/authentication/index.mdx
- contrib/auth/authentik/seed/README.md
- contrib/auth/authentik/README.md
🚧 Files skipped from review as they are similar to previous changes (30)
- pytest.ini
- tests/auth_idp/authentik_live.py
- tests/auth_idp/test_authentik_fixture_cleanup.py
- services/core/auth/tests/integration/test_gateway_header_spoofing.py
- tests/auth_idp/test_provider_layout.py
- tests/auth_idp/test_xdist_grouping.py
- e2e/conftest.py
- tests/auth_idp/xdist.py
- tests/auth_idp/test_authentik_provider_helpers.py
- tests/auth_idp/test_docs_links.py
- contrib/auth/authentik/manifest.yaml
- tests/auth_idp/test_services_pool_sidecar_contract.py
- contrib/auth/authentik/gateway/envoy.yaml
- tests/auth_idp/test_provider_manifest.py
- tests/test_e2e_services_pool.py
- contrib/auth/manifest.schema.yaml
- tests/auth_idp/test_authentik_compose_helpers.py
- tests/auth_idp/test_authentik_real_oidc.py
- tests/auth_idp/providers.py
- packages/nmp_testing/src/nmp/testing/e2e/docker.py
- contrib/auth/authentik/blueprints/nemo.yaml
- tests/auth_idp/test_services_pool_authentik_sidecar.py
- Makefile
- tests/auth_idp/runtime.py
- .github/actions/changes/action.yaml
- tests/auth_idp/sidecars.py
- tests/auth_idp/test_provider_matrix.py
- conftest.py
- contrib/auth/authentik/docker-compose.yml
- e2e/services_pool.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/auth_idp/sidecars.py (1)
33-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffReturn type
Any+ in-function import works around a circular import.
RunningSidecaris imported locally (line 98) instead of at module scope, presumably to avoid a cycle withe2e.services_pool(which importsstart_authentik_sidecar). The function is annotated-> Anyinstead of the concreteRunningSidecar. As per coding guidelines, "Always prefer concrete type hints over string-based ones... do not import types under TYPE_CHECKING, instead import types as regular imports when possible" — consider movingRunningSidecar/RunningServicesto a shared module both sides can import at top level, so the return type can be concrete.Also applies to: 98-108
🤖 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/sidecars.py` around lines 33 - 38, The `start_authentik_sidecar` function is hiding a circular import by using a local import and `-> Any` instead of a concrete return type. Move `RunningSidecar` (and any related `RunningServices` dependency) into a shared module that both `tests/auth_idp/sidecars.py` and `e2e.services_pool` can import at module scope, then update `start_authentik_sidecar` to return the concrete `RunningSidecar` type and remove the in-function import.Source: Coding guidelines
tests/auth_idp/test_authentik_gateway_live.py (1)
48-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUnclear failure on missing admin member.
next(...)raisesStopIteration(shown as a cryptic error, not an assertion) if no member has theAdminrole.🔧 Proposed fix
- admin_member = next(member for member in members_response.json()["data"] if "Admin" in member["roles"]) + admin_member = next( + (member for member in members_response.json()["data"] if "Admin" in member["roles"]), + None, + ) + assert admin_member is not None, f"no Admin role-binding found: {members_response.json()['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 `@tests/auth_idp/test_authentik_gateway_live.py` around lines 48 - 58, The admin member lookup in the authentik gateway live test can fail with a cryptic StopIteration instead of a clear test failure. In test_authentik_gateway_live.py, replace the direct next(...) search on members_response.json()["data"] with an explicit assertion or guarded lookup in the same test block so a missing Admin role produces a readable failure message before using admin_member. Keep the existing auth/member checks, but make the failure path explicit around the members_response parsing and admin_member selection.
🤖 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.py`:
- Around line 140-148: The runtime identifiers in get_authentik_test_runtime()
are inconsistent because runtime_root and compose_project_name each use a
separate uuid.uuid4() call. Generate the suffix once inside
get_authentik_test_runtime, reuse it for both runtime_root and
compose_project_name, and keep the existing render_authentik_runtime_bundle call
unchanged so the on-disk runtime directory and compose project name stay
correlated.
In `@tests/auth_idp/sidecars.py`:
- Around line 68-108: The startup flow in sidecars setup leaves Docker resources
running if any step after compose_up fails. Add failure cleanup around the
compose_up / finalize_authentik_runtime_bundle / wait_for_healthchecks /
wait_for_gateway_listener sequence so compose_down is always attempted on
exceptions, and keep the existing RunningSidecar.close cleanup for the success
path. Use the existing compose_up, compose_down, and
finalize_authentik_runtime_bundle flow in the auth sidecar setup to place the
cleanup in the right spot.
---
Nitpick comments:
In `@tests/auth_idp/sidecars.py`:
- Around line 33-38: The `start_authentik_sidecar` function is hiding a circular
import by using a local import and `-> Any` instead of a concrete return type.
Move `RunningSidecar` (and any related `RunningServices` dependency) into a
shared module that both `tests/auth_idp/sidecars.py` and `e2e.services_pool` can
import at module scope, then update `start_authentik_sidecar` to return the
concrete `RunningSidecar` type and remove the in-function import.
In `@tests/auth_idp/test_authentik_gateway_live.py`:
- Around line 48-58: The admin member lookup in the authentik gateway live test
can fail with a cryptic StopIteration instead of a clear test failure. In
test_authentik_gateway_live.py, replace the direct next(...) search on
members_response.json()["data"] with an explicit assertion or guarded lookup in
the same test block so a missing Admin role produces a readable failure message
before using admin_member. Keep the existing auth/member checks, but make the
failure path explicit around the members_response parsing and admin_member
selection.
🪄 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: 8ee2fd63-bfac-49b4-90ab-bf66ccc7cb8e
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.github/actions/changes/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/README.mdcontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/seed/README.mdcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/authentication/index.mdxdocs/fern/gated-nav.ymle2e/conftest.pye2e/services_pool.pypackages/nmp_testing/src/nmp/testing/e2e/docker.pypytest.iniservices/core/auth/tests/integration/test_gateway_header_spoofing.pyservices/core/auth/tests/test_machine_group_authz.pytests/auth_idp/authentik_live.pytests/auth_idp/compose.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/sidecars.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_runtime_render.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_provider_matrix.pytests/auth_idp/test_services_pool_authentik_sidecar.pytests/auth_idp/test_services_pool_sidecar_contract.pytests/auth_idp/test_xdist_grouping.pytests/auth_idp/xdist.pytests/test_e2e_services_pool.py
✅ Files skipped from review due to trivial changes (8)
- docs/fern/gated-nav.yml
- tests/auth_idp/xdist.py
- pytest.ini
- docs/auth/authentication/idp-integration.mdx
- docs/auth/authentication/index.mdx
- contrib/auth/README.md
- contrib/auth/authentik/seed/README.md
- contrib/auth/authentik/README.md
🚧 Files skipped from review as they are similar to previous changes (27)
- tests/auth_idp/authentik_live.py
- e2e/conftest.py
- tests/auth_idp/test_services_pool_sidecar_contract.py
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/test_authentik_fixture_cleanup.py
- tests/auth_idp/test_xdist_grouping.py
- services/core/auth/tests/integration/test_gateway_header_spoofing.py
- tests/auth_idp/test_provider_layout.py
- contrib/auth/authentik/config/nemo-auth.yaml
- contrib/auth/authentik/manifest.yaml
- tests/auth_idp/test_authentik_real_oidc.py
- contrib/auth/authentik/gateway/envoy.yaml
- tests/auth_idp/test_authentik_provider_helpers.py
- .github/actions/changes/action.yaml
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- tests/auth_idp/providers.py
- contrib/auth/authentik/blueprints/nemo.yaml
- contrib/auth/manifest.schema.yaml
- .github/workflows/ci.yaml
- tests/test_e2e_services_pool.py
- tests/auth_idp/test_provider_manifest.py
- Makefile
- packages/nmp_testing/src/nmp/testing/e2e/docker.py
- contrib/auth/authentik/docker-compose.yml
- conftest.py
- tests/auth_idp/test_provider_matrix.py
- e2e/services_pool.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/auth_idp/test_authentik_cli_login.py (1)
16-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
_jwt_claimshelper.Same function is (re)defined in
test_authentik_gateway_live.py. Move it into the sharedtests/auth_idp/authentik_live.pymodule, already imported by both files.♻️ Suggested consolidation
-def _jwt_claims(token: str) -> dict[str, object]: - parts = token.split(".") - if len(parts) != 3: - return {} - payload = parts[1] + "=" * (-len(parts[1]) % 4) - return json.loads(base64.urlsafe_b64decode(payload)) +from tests.auth_idp.authentik_live import AUTHENTIK_DOCKER_PYTESTMARK, jwt_claims as _jwt_claims🤖 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/test_authentik_cli_login.py` around lines 16 - 21, The _jwt_claims helper is duplicated here and in test_authentik_gateway_live.py, so move the shared parsing logic into tests/auth_idp/authentik_live.py and update both test modules to import it from there. Keep the existing _jwt_claims behavior unchanged, but remove the local redefinition in test_authentik_cli_login.py and reference the shared helper instead.
🤖 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 `@e2e/backends/docker_compose.py`:
- Around line 71-97: The start logic in DockerComposeBackend.start treats an
empty expected_services set from _services("config", "--services") as success,
which can falsely report readiness for an empty or broken compose file. Add an
explicit guard in start (before the reuse check and the startup loop) to fail
fast when expected_services is empty, using the DockerComposeBackend.start path
and the expected_services variable to locate the fix. Raise a clear error that
indicates no services were discovered so startup cannot proceed.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py`:
- Around line 141-147: The token-file fallback in config loading can crash on
unreadable or missing paths when `Path(token_path).read_text()` is called.
Update the `env_values["access_token"]` fallback logic in the config loader to
catch file I/O errors around `read_text` and handle them as a clear
configuration error or skip setting the token, so `NEMO_WORKLOAD_TOKEN_FILE`
does not raise an uncaught exception during startup.
In `@tests/auth_idp/test_authentik_runtime_render.py`:
- Around line 24-58: The test in
test_authentik_live_marker_uses_compose_harness_metadata is tautologically
asserting AUTHENTIK_E2E_LIFECYCLE against a value read from the same
AUTHENTIK_DOCKER_E2E_CONFIG, so replace that self-comparison with an explicit
expected lifecycle value. Keep the existing assertions on
AUTHENTIK_DOCKER_E2E_CONFIG.args and the harness env, but define the lifecycle
expectation independently and compare it directly in the env map so the test
validates real behavior. The relevant symbols to update are
AUTHENTIK_DOCKER_E2E_CONFIG, expected_lifecycle, and AUTHENTIK_E2E_LIFECYCLE.
---
Nitpick comments:
In `@tests/auth_idp/test_authentik_cli_login.py`:
- Around line 16-21: The _jwt_claims helper is duplicated here and in
test_authentik_gateway_live.py, so move the shared parsing logic into
tests/auth_idp/authentik_live.py and update both test modules to import it from
there. Keep the existing _jwt_claims behavior unchanged, but remove the local
redefinition in test_authentik_cli_login.py and reference the shared helper
instead.
🪄 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: d0f27626-54f0-4532-aa81-18a57f7ef4a8
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/src/nemo_platform/config/config.pyis excluded by!sdk/**
📒 Files selected for processing (43)
Makefilecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxe2e/backends/docker_compose.pye2e/conftest.pye2e/services_pool.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/config/test_config.pypytest.iniservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/tests/test_embedded_pdp.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/__main__.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/compose.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/test_authentik_cli_login.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_runtime_render.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_provider_matrix.pytests/test_e2e_docker_compose_backend.pytests/test_e2e_services_pool.py
✅ Files skipped from review due to trivial changes (5)
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/main.py
- docs/auth/authentication/idp-integration.mdx
- pytest.ini
- e2e/conftest.py
- contrib/auth/authentik/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- contrib/auth/authentik/config/nemo-auth.yaml
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- tests/auth_idp/test_provider_matrix.py
ironcommit
force-pushed
the
auth-idp
branch
2 times, most recently
from
July 6, 2026 21:11
6820409 to
7b07862
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py`:
- Around line 53-68: The `_runtime_token_source_label` helper is reimplementing
token precedence and directly reading `NEMO_WORKLOAD_TOKEN_FILE` without error
handling, which can crash `login`, `logout`, or `status`. Update this helper to
use the same source-of-truth logic as `Config._load_from_env()` in `config.py`,
or expose that precedence/label resolution from `Config` and call it here. If
file access is still needed, wrap the `Path(token_file).read_text(...)` path in
error handling and return a safe `None`/friendly auth-specific fallback instead
of letting an unhandled exception propagate.
🪄 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: e964a526-44f5-4768-9a86-53d720b660af
⛔ Files ignored due to path filters (5)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.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/**uv.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
.github/actions/changes/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/README.mdcontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/seed/README.mdcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/authentication/index.mdxdocs/auth/authorization/permissions-reference.mdxdocs/auth/authorization/roles-and-permissions.mdxdocs/fern/gated-nav.ymle2e/backends/docker_compose.pye2e/conftest.pye2e/services_pool.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nmp_testing/src/nmp/testing/e2e/docker.pypytest.iniservices/core/auth/scripts/auth-tools.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/tests/integration/test_gateway_header_spoofing.pyservices/core/auth/tests/test_embedded_pdp.pyservices/core/auth/tests/test_machine_group_authz.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/__main__.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/compose.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/test_authentik_cli_login.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_provider_matrix.pytests/auth_idp/test_xdist_grouping.pytests/auth_idp/xdist.pytests/test_e2e_docker_compose_backend.pytests/test_e2e_services_pool.py
💤 Files with no reviewable changes (32)
- tests/auth_idp/test_authentik_fixture_cleanup.py
- tests/auth_idp/test_provider_layout.py
- tests/auth_idp/runtime.py
- services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/xdist.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/main.py
- services/core/auth/tests/test_embedded_pdp.py
- tests/auth_idp/test_xdist_grouping.py
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py
- services/core/auth/tests/integration/test_gateway_header_spoofing.py
- services/core/auth/tests/test_machine_group_authz.py
- services/core/auth/scripts/auth-tools.py
- tests/auth_idp/test_authentik_cli_login.py
- tests/auth_idp/test_authentik_provider_helpers.py
- tests/auth_idp/authentik_live.py
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- tests/auth_idp/test_provider_manifest.py
- tests/auth_idp/test_authentik_startup_smoke.py
- tests/auth_idp/providers.py
- tests/test_e2e_docker_compose_backend.py
- tests/auth_idp/test_provider_matrix.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
- tests/auth_idp/test_authentik_real_oidc.py
- services/core/jobs/jobs-launcher/cmd/run.go
- tests/test_e2e_services_pool.py
- tests/auth_idp/test_authentik_compose_helpers.py
- services/core/jobs/tests/controllers/test_docker_backend.py
- tests/auth_idp/test_authentik_gateway_live.py
- tests/auth_idp/compose.py
- tests/auth_idp/conftest.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
✅ Files skipped from review due to trivial changes (7)
- pytest.ini
- docs/auth/authentication/index.mdx
- contrib/auth/authentik/seed/README.md
- e2e/conftest.py
- contrib/auth/README.md
- docs/auth/authentication/idp-integration.mdx
- contrib/auth/authentik/README.md
🚧 Files skipped from review as they are similar to previous changes (11)
- docs/fern/gated-nav.yml
- contrib/auth/authentik/config/nemo-auth.yaml
- contrib/auth/authentik/manifest.yaml
- contrib/auth/manifest.schema.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- .github/actions/changes/action.yaml
- packages/nemo_platform_ext/tests/config/test_config.py
- conftest.py
- packages/nmp_testing/src/nmp/testing/e2e/docker.py
- .github/workflows/ci.yaml
- contrib/auth/authentik/docker-compose.yml
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/auth_idp/test_authentik_compose_helpers.py (1)
189-221: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
ProviderConfigfixture across three tests.Same large fixture object repeated three times with only a couple of fields differing. Consider a shared factory/fixture.
Also applies to: 238-270, 291-319
🤖 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/test_authentik_compose_helpers.py` around lines 189 - 221, The three tests repeat the same large ProviderConfig setup, with only a few fields changing, so extract the shared configuration into a reusable factory or fixture. Update the authentik compose helper tests to construct ProviderConfig through a common helper and override only the per-test differences, keeping the test names like test_wait_for_token_endpoint_retries_until_machine_grant_succeeds focused on behavior instead of setup duplication.
🤖 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.
Nitpick comments:
In `@tests/auth_idp/test_authentik_compose_helpers.py`:
- Around line 189-221: The three tests repeat the same large ProviderConfig
setup, with only a few fields changing, so extract the shared configuration into
a reusable factory or fixture. Update the authentik compose helper tests to
construct ProviderConfig through a common helper and override only the per-test
differences, keeping the test names like
test_wait_for_token_endpoint_retries_until_machine_grant_succeeds focused on
behavior instead of setup duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7f8c4c7f-ec45-4ed5-af1f-269e684c3722
⛔ Files ignored due to path filters (4)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.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 (28)
Makefilecontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shdocs/auth/authorization/permissions-reference.mdxdocs/auth/authorization/roles-and-permissions.mdxe2e/backends/docker_compose.pye2e/services_pool.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/config/test_config.pyservices/core/auth/scripts/auth-tools.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/tests/integration/test_gateway_header_spoofing.pyservices/core/auth/tests/test_embedded_pdp.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/tests/controllers/test_docker_backend.pytests/auth_idp/authentik_live.pytests/auth_idp/compose.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/test_e2e_docker_compose_backend.py
💤 Files with no reviewable changes (1)
- tests/auth_idp/authentik_live.py
✅ Files skipped from review due to trivial changes (2)
- tests/auth_idp/test_authentik_fixture_cleanup.py
- contrib/auth/authentik/README.md
🚧 Files skipped from review as they are similar to previous changes (21)
- services/core/auth/tests/integration/test_gateway_header_spoofing.py
- services/core/auth/tests/test_embedded_pdp.py
- tests/auth_idp/test_authentik_provider_helpers.py
- Makefile
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- contrib/auth/authentik/run.sh
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- docs/auth/authorization/roles-and-permissions.mdx
- services/core/auth/scripts/auth-tools.py
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_ext/tests/config/test_config.py
- docs/auth/authorization/permissions-reference.mdx
- contrib/auth/authentik/docker-compose.yml
- contrib/auth/authentik/manifest.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- services/core/jobs/tests/controllers/test_docker_backend.py
- e2e/backends/docker_compose.py
- tests/auth_idp/test_authentik_real_oidc.py
- e2e/services_pool.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
maxdubrinsky
approved these changes
Jul 6, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
tests/auth_idp/compose.py (1)
246-285: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate token-request-body logic vs
conftest.py.The grant→form-body construction here duplicates
_token_request_bodyintests/auth_idp/conftest.py, but without itsValueErroron unsupported grant types. Consider extracting a shared helper (e.g. inproviders.py) to avoid the two implementations drifting again.🤖 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.py` around lines 246 - 285, The token request body assembly in wait_for_token_endpoint is duplicated from _token_request_body, and this copy is missing the unsupported-grant validation. Reuse the shared helper instead of rebuilding the form data inline, or extract the logic into a common helper in providers.py and call it from both wait_for_token_endpoint and the existing conftest.py path so the grant handling stays consistent.tests/auth_idp/test_authentik_cli_login.py (1)
70-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCleanup delete can mask the real test failure.
If
create_response.raise_for_status()raises, thefinallyblock'shttpx.deleteon a never-created workspace likely 404s and raises too, hiding the original failure.🧹 Proposed fix
finally: - httpx.delete( - f"{authentik_stack.gateway_base_url}/apis/entities/v2/workspaces/{workspace_name}", - headers=headers, - timeout=10.0, - ) + try: + httpx.delete( + f"{authentik_stack.gateway_base_url}/apis/entities/v2/workspaces/{workspace_name}", + headers=headers, + timeout=10.0, + ) + except httpx.HTTPError: + pass🤖 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/test_authentik_cli_login.py` around lines 70 - 84, The cleanup in the workspace creation test can mask the real failure because the unconditional httpx.delete in the finally block may raise when create_response.raise_for_status() fails. Update the test around the workspace creation flow so cleanup only runs if the workspace was actually created, using the existing create_response logic in test_authentik_cli_login to guard the delete or otherwise swallow expected not-found cleanup failures. Keep the assert on created_by and preserve cleanup for successful creation.
🤖 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 `@e2e/services_pool.py`:
- Around line 728-764: The docker-compose startup path in
_start_services_docker_compose is missing the auth readiness wait that the other
startup helpers perform. After backend.start() succeeds, use the computed
auth_enabled value from RunningServices setup and call _wait_for_auth_ready with
the backend’s service_url before returning, matching the behavior of
_start_services_docker and _start_services_subprocess. Keep the existing cleanup
behavior in the try/except and ensure the wait happens only when auth is
enabled.
In
`@services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/run.py`:
- Around line 33-44: The workload token is being loaded unconditionally in
run(), so injected sdk callers still fail before the fallback can be used. Move
_load_workload_token() and the NeMoPlatform(default_headers=...) construction
behind the sdk is None path in run(), so prebuilt sdk instances can skip
token/env requirements while preserving the bearer-token setup for the default
path.
In `@tests/auth_idp/test_authentik_gateway_live.py`:
- Around line 68-79: The test in test_authentik_gateway_forwards_workload_groups
is too weak because it only accepts either success or failure and checks the
local JWT claim instead of gateway behavior. Update it to exercise a
deterministic authorization path by provisioning a role binding for one of the
token’s groups, similar to the approach used in test_machine_group_authz, then
assert a specific 200 response from the gateway. If needed, verify the
downstream authorization decision explicitly using the gateway_base_url request
and the machine_token so the test proves groups are actually forwarded and
honored.
---
Nitpick comments:
In `@tests/auth_idp/compose.py`:
- Around line 246-285: The token request body assembly in
wait_for_token_endpoint is duplicated from _token_request_body, and this copy is
missing the unsupported-grant validation. Reuse the shared helper instead of
rebuilding the form data inline, or extract the logic into a common helper in
providers.py and call it from both wait_for_token_endpoint and the existing
conftest.py path so the grant handling stays consistent.
In `@tests/auth_idp/test_authentik_cli_login.py`:
- Around line 70-84: The cleanup in the workspace creation test can mask the
real failure because the unconditional httpx.delete in the finally block may
raise when create_response.raise_for_status() fails. Update the test around the
workspace creation flow so cleanup only runs if the workspace was actually
created, using the existing create_response logic in test_authentik_cli_login to
guard the delete or otherwise swallow expected not-found cleanup failures. Keep
the assert on created_by and preserve cleanup for successful creation.
🪄 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: 6e4fc200-6592-4caa-81ab-08fde89b7f70
⛔ Files ignored due to path filters (5)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.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/**uv.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
.github/actions/changes/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/README.mdcontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/seed/README.mdcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/authentication/index.mdxdocs/auth/authorization/permissions-reference.mdxdocs/auth/authorization/roles-and-permissions.mdxdocs/fern/gated-nav.ymle2e/backends/docker_compose.pye2e/conftest.pye2e/services_pool.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nmp_testing/src/nmp/testing/e2e/docker.pypytest.iniservices/core/auth/scripts/auth-tools.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/tests/integration/test_gateway_header_spoofing.pyservices/core/auth/tests/test_embedded_pdp.pyservices/core/auth/tests/test_machine_group_authz.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/__main__.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/compose.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/test_authentik_cli_login.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_provider_matrix.pytests/auth_idp/test_xdist_grouping.pytests/auth_idp/xdist.pytests/test_e2e_docker_compose_backend.pytests/test_e2e_services_pool.py
✅ Files skipped from review due to trivial changes (10)
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/main.py
- docs/auth/authentication/index.mdx
- docs/fern/gated-nav.yml
- pytest.ini
- contrib/auth/authentik/config/nemo-auth.yaml
- contrib/auth/authentik/seed/README.md
- e2e/conftest.py
- contrib/auth/README.md
- contrib/auth/authentik/README.md
- docs/auth/authentication/idp-integration.mdx
🚧 Files skipped from review as they are similar to previous changes (36)
- tests/auth_idp/test_xdist_grouping.py
- tests/auth_idp/runtime.py
- tests/auth_idp/test_provider_layout.py
- tests/auth_idp/test_docs_links.py
- tests/auth_idp/test_authentik_provider_helpers.py
- services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py
- Makefile
- .github/actions/changes/action.yaml
- tests/auth_idp/xdist.py
- services/core/auth/tests/integration/test_gateway_header_spoofing.py
- services/core/jobs/jobs-launcher/cmd/run.go
- services/core/auth/scripts/auth-tools.py
- contrib/auth/authentik/manifest.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/config/config.py
- tests/auth_idp/test_provider_manifest.py
- packages/nmp_testing/src/nmp/testing/e2e/docker.py
- docs/auth/authorization/roles-and-permissions.mdx
- contrib/auth/authentik/blueprints/nemo.yaml
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- contrib/auth/authentik/gateway/envoy.yaml
- tests/test_e2e_services_pool.py
- services/core/auth/tests/test_embedded_pdp.py
- docs/auth/authorization/permissions-reference.mdx
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- conftest.py
- contrib/auth/manifest.schema.yaml
- packages/nemo_platform_ext/tests/config/test_config.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- tests/auth_idp/test_authentik_real_oidc.py
- tests/auth_idp/providers.py
- tests/auth_idp/test_provider_matrix.py
- .github/workflows/ci.yaml
- contrib/auth/authentik/docker-compose.yml
- contrib/auth/authentik/run.sh
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tests/auth_idp/test_authentik_gateway_live.py (1)
68-79: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStill doesn't verify group forwarding by the gateway.
status_code in {200, 403}passes regardless of forwarding correctness, andisinstance(claims.get("groups"), str)only checks the local JWT, not gateway behavior. Provision a role binding for one of the token's groups (as intest_machine_group_authz.py) and assert a deterministic200.🤖 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/test_authentik_gateway_live.py` around lines 68 - 79, The test in test_authentik_gateway_forwards_workload_groups does not actually prove the gateway forwards groups because it accepts either 200 or 403 and only inspects the local JWT claims. Update the test to use a real role binding for one of machine_token’s groups, following the pattern from test_machine_group_authz, then assert the gateway request to AuthentikStack.gateway_base_url returns a deterministic 200 so the forwarding behavior is verified through the response.e2e/services_pool.py (1)
728-764: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDocker-compose startup still skips auth-readiness wait.
_start_services_docker_composecomputesauth_enabledbut never calls_wait_for_auth_ready, unlike_start_services_docker(Lines 707-710) and_start_services_subprocess(Lines 676-686). Tests using this harness with auth enabled can proceed before auth seeding completes.🛠️ Proposed fix
try: backend.start() except Exception: backend.stop() raise + auth_enabled = _e2e_auth_enabled(config_data) + if auth_enabled and not _wait_for_auth_ready(backend.service_url, None): + backend.stop() + pytest.fail(f"Platform auth seed did not become ready within {_AUTH_READY_TIMEOUT}s.") + return RunningServices( url=backend.service_url, log_path=None, proc=None, config_path=config_path, close=backend.stop, - auth_enabled=_e2e_auth_enabled(config_data), + auth_enabled=auth_enabled, key=_services_pool_key(config_hash), )🤖 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 `@e2e/services_pool.py` around lines 728 - 764, The Docker Compose startup path in _start_services_docker_compose computes auth_enabled but never waits for auth readiness, unlike _start_services_docker and _start_services_subprocess. After backend.start() succeeds and before returning RunningServices, call _wait_for_auth_ready using the returned service URL (and the computed auth_enabled/auth context) so tests do not proceed until auth seeding is complete. Keep the fix localized to _start_services_docker_compose and reuse the existing auth-readiness helper and RunningServices setup.
🧹 Nitpick comments (1)
tests/auth_idp/test_authentik_cli_login.py (1)
50-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the configured human-token fixture here.
Lines 51-59 duplicate the CLI grant instead of reusing
human_tokenorauthentik_stack.human_grant. That makes this live test drift from the manifest-driven provider contract.Proposed change
-def test_authentik_cli_provider_token_is_accepted_by_gateway(authentik_stack): - token_response = httpx.post( - authentik_stack.token_endpoint, - data={ - "grant_type": "password", - "client_id": "nemo-platform-cli", - "username": "nemo-user", - "password": "nemo-user-token-secret-dev", - "scope": "openid email offline_access groups", - }, - timeout=30.0, - ) - token_response.raise_for_status() - access_token = token_response.json()["access_token"] +def test_authentik_cli_provider_token_is_accepted_by_gateway(authentik_stack, human_token: str): + access_token = human_token🤖 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/test_authentik_cli_login.py` around lines 50 - 63, The authentik CLI login test is duplicating the password grant flow instead of using the configured human-token fixture, which can drift from the manifest-driven provider contract. Update test_authentik_cli_provider_token_is_accepted_by_gateway to reuse human_token or authentik_stack.human_grant for obtaining the access token, and remove the direct httpx.post token request so the test stays aligned with the shared fixture setup.
🤖 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/compose.py`:
- Around line 250-270: The polling helper in compose.py is ignoring its own
deadline because the first httpx.post still uses a fixed 30.0s timeout even when
the caller’s remaining budget is smaller. Update the token request in the
machine grant loop to compute the remaining time from deadline and clamp the
request timeout to that remaining poll budget, using the existing deadline,
timeout, and response flow in the same helper.
In `@tests/auth_idp/test_authentik_cli_login.py`:
- Around line 79-84: The workspace cleanup in the finally block is ignoring the
DELETE response, so failures can silently leave resources behind. Update the
cleanup logic in test_authentik_cli_login to check the result of the
httpx.delete call and fail the test when it returns a 4xx/5xx response, using
the existing workspace_name and authentik_stack.gateway_base_url cleanup path to
keep the teardown reliable.
---
Duplicate comments:
In `@e2e/services_pool.py`:
- Around line 728-764: The Docker Compose startup path in
_start_services_docker_compose computes auth_enabled but never waits for auth
readiness, unlike _start_services_docker and _start_services_subprocess. After
backend.start() succeeds and before returning RunningServices, call
_wait_for_auth_ready using the returned service URL (and the computed
auth_enabled/auth context) so tests do not proceed until auth seeding is
complete. Keep the fix localized to _start_services_docker_compose and reuse the
existing auth-readiness helper and RunningServices setup.
In `@tests/auth_idp/test_authentik_gateway_live.py`:
- Around line 68-79: The test in test_authentik_gateway_forwards_workload_groups
does not actually prove the gateway forwards groups because it accepts either
200 or 403 and only inspects the local JWT claims. Update the test to use a real
role binding for one of machine_token’s groups, following the pattern from
test_machine_group_authz, then assert the gateway request to
AuthentikStack.gateway_base_url returns a deterministic 200 so the forwarding
behavior is verified through the response.
---
Nitpick comments:
In `@tests/auth_idp/test_authentik_cli_login.py`:
- Around line 50-63: The authentik CLI login test is duplicating the password
grant flow instead of using the configured human-token fixture, which can drift
from the manifest-driven provider contract. Update
test_authentik_cli_provider_token_is_accepted_by_gateway to reuse human_token or
authentik_stack.human_grant for obtaining the access token, and remove the
direct httpx.post token request so the test stays aligned with the shared
fixture setup.
🪄 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: 80fb7746-13d0-4e89-93f5-4a4d60f00214
⛔ Files ignored due to path filters (5)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/auth.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/**uv.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
.github/actions/changes/action.yaml.github/workflows/ci.yamlMakefileconftest.pycontrib/auth/README.mdcontrib/auth/authentik/README.mdcontrib/auth/authentik/blueprints/nemo.yamlcontrib/auth/authentik/config/nemo-auth.yamlcontrib/auth/authentik/config/platform-compose-authentik.yamlcontrib/auth/authentik/docker-compose.ymlcontrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/manifest.yamlcontrib/auth/authentik/run.shcontrib/auth/authentik/seed/README.mdcontrib/auth/manifest.schema.yamldocs/auth/authentication/idp-integration.mdxdocs/auth/authentication/index.mdxdocs/auth/authorization/permissions-reference.mdxdocs/auth/authorization/roles-and-permissions.mdxdocs/fern/gated-nav.ymle2e/backends/docker_compose.pye2e/conftest.pye2e/services_pool.pypackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.pypackages/nemo_platform_ext/src/nemo_platform_ext/config/config.pypackages/nemo_platform_ext/tests/cli/commands/test_auth.pypackages/nemo_platform_ext/tests/config/test_config.pypackages/nmp_testing/src/nmp/testing/e2e/docker.pypytest.iniservices/core/auth/scripts/auth-tools.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/auth/tests/integration/test_gateway_header_spoofing.pyservices/core/auth/tests/test_embedded_pdp.pyservices/core/auth/tests/test_machine_group_authz.pyservices/core/jobs/jobs-launcher/cmd/run.goservices/core/jobs/jobs-launcher/cmd/run_test.goservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/tests/controllers/test_docker_backend.pyservices/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/__main__.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/compose.pytests/auth_idp/conftest.pytests/auth_idp/providers.pytests/auth_idp/runtime.pytests/auth_idp/test_authentik_cli_login.pytests/auth_idp/test_authentik_compose_helpers.pytests/auth_idp/test_authentik_fixture_cleanup.pytests/auth_idp/test_authentik_gateway_live.pytests/auth_idp/test_authentik_provider_helpers.pytests/auth_idp/test_authentik_real_oidc.pytests/auth_idp/test_authentik_startup_smoke.pytests/auth_idp/test_docs_links.pytests/auth_idp/test_provider_layout.pytests/auth_idp/test_provider_manifest.pytests/auth_idp/test_provider_matrix.pytests/auth_idp/test_xdist_grouping.pytests/auth_idp/xdist.pytests/test_e2e_docker_compose_backend.pytests/test_e2e_services_pool.py
✅ Files skipped from review due to trivial changes (7)
- services/hello-world/src/nmp/hello_world/tasks/workload_workspace_get/main.py
- docs/auth/authentication/index.mdx
- contrib/auth/authentik/seed/README.md
- contrib/auth/README.md
- e2e/conftest.py
- docs/auth/authentication/idp-integration.mdx
- contrib/auth/authentik/README.md
🚧 Files skipped from review as they are similar to previous changes (39)
- contrib/auth/authentik/config/nemo-auth.yaml
- tests/auth_idp/xdist.py
- tests/auth_idp/test_xdist_grouping.py
- services/hello-world/tests/integration/tasks/test_workload_workspace_get_task.py
- docs/fern/gated-nav.yml
- .github/actions/changes/action.yaml
- Makefile
- tests/test_e2e_services_pool.py
- contrib/auth/authentik/manifest.yaml
- tests/auth_idp/test_provider_manifest.py
- tests/auth_idp/runtime.py
- tests/auth_idp/test_authentik_provider_helpers.py
- tests/auth_idp/test_provider_layout.py
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- tests/auth_idp/test_docs_links.py
- pytest.ini
- services/core/auth/tests/test_embedded_pdp.py
- services/core/auth/tests/integration/test_gateway_header_spoofing.py
- services/core/auth/scripts/auth-tools.py
- contrib/auth/manifest.schema.yaml
- contrib/auth/authentik/gateway/envoy.yaml
- tests/auth_idp/test_provider_matrix.py
- .github/workflows/ci.yaml
- services/core/jobs/jobs-launcher/cmd/run.go
- packages/nemo_platform_ext/tests/config/test_config.py
- packages/nmp_testing/src/nmp/testing/e2e/docker.py
- packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/auth.py
- services/core/jobs/jobs-launcher/cmd/run_test.go
- docs/auth/authorization/roles-and-permissions.mdx
- tests/auth_idp/test_authentik_real_oidc.py
- docs/auth/authorization/permissions-reference.mdx
- contrib/auth/authentik/blueprints/nemo.yaml
- tests/auth_idp/providers.py
- conftest.py
- contrib/auth/authentik/run.sh
- contrib/auth/authentik/config/platform-compose-authentik.yaml
- contrib/auth/authentik/docker-compose.yml
- services/core/jobs/tests/controllers/test_docker_backend.py
- services/core/jobs/src/nmp/core/jobs/controllers/backends/docker.py
ironcommit
force-pushed
the
auth-idp
branch
4 times, most recently
from
July 8, 2026 03:41
d5aff34 to
95a5800
Compare
crookedstorm
approved these changes
Jul 8, 2026
crookedstorm
left a comment
crookedstorm
left a comment
Contributor
There was a problem hiding this comment.
Looks fine on the GHA end of things.
svvarom
approved these changes
Jul 8, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 8, 2026
ironcommit
enabled auto-merge
July 9, 2026 17:24
Signed-off-by: Ryan S <[email protected]>
This was referenced Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a manifest-driven identity provider (IdP) reference contract under
contrib/auth/and a full E2E test harness (tests/auth_idp/) that validates NeMo Platform authentication against real OIDC providers running in Docker Compose.What this includes
IdP reference contract (
contrib/auth/)manifest.schema.yaml) defining the contract every provider bundle must satisfy: OIDC discovery, gateway header stripping, human + machine identities, token acquisition, and healthcheck endpointsX-NMP-Principal-*headers), authentik blueprint provisioning (OAuth2 provider, groups, service account), and NeMo auth configAuth-idp test harness (
tests/auth_idp/)providers.py)compose.py) with diagnostics collectionsidecars.py) for attaching IdP stacks to the E2E services poolruntime.py) for merging provider auth config into platform configxdist.py) so provider-specific tests share the right Compose stackE2E infrastructure changes
e2e/services_pool.pygains a Docker backend path (_start_services_docker) and sidecar lifecycle managemente2e/configs/auth-idp-docker.yaml— Docker-backed E2E config for auth-idp live testspackages/nmp_testing/.../docker.py— exposes network name, alias, and container port for sidecar networking; adds stable container namingAuth service tests
test_machine_group_authz.py— verifies external machine identities authenticate via OIDC group bindings (notservice:*prefix)test_gateway_header_spoofing.py— asserts all provider gateways strip trusted identity headersCI
auth-idpchange filter and dedicated CI job (python-auth-idp-test) that runsmake test-auth-idpDocs
idp-integration.mdxpage linked from the authentication index and gated navTest plan
make test-auth-idppasses (unit tests for harness helpers)make test-unitpasses (machine group authz + gateway header spoofing tests)python-auth-idp-testjob triggers ontests/auth_idp/**,e2e/**, orcontrib/auth/**changesSummary by CodeRabbit
NEMO_WORKLOAD_TOKEN/NEMO_WORKLOAD_TOKEN_FILEacross CLI/SDK and workload tasks.