authentik-gate-sync learns OIDC clients, declared as an OIDCClient resource next to the app, so apps like YouTrack need no blueprint entries and nothing on their Ingress.
What
CRDoidcclients.auth.renner.uno (v1alpha1, namespaced, status subresource) in infrastructure/authentik-gate-sync-crds, like kuma-sync-crds.
Spec: launchURL, redirectURIs[] {url, matchingMode strict|regex}, optional slug (default metadata.name), group, authorizationFlow.
Controller, per OIDCClient:
confidential OAuth2/OpenID provider oidc-<slug>: client ID = slug, secret generated by Authentik, grants authorization_code + refresh_token, scopes openid/profile/email, signed with authentik Self-signed Certificate;
with group: the same group expression policy + binding as gates (the group itself stays managed in Authentik);
status: Ready condition with the reason, and issuer.
Fail closed and ownership as for gates: a missing group, a foreign application/provider with the name, or two OIDCClients with one slug withhold/remove the client; owned clients whose OIDCClient is gone are swept. Missing CRD = no clients, gates unaffected.
RBAC: ClusterRole gets get/list/watch on oidcclients and patch on oidcclients/status; the Authentik role (blueprint gate-sync.yaml) gets authentik_providers_oauth2.{view,add,change,delete}_oauth2provider, view_scopemapping, authentik_crypto.view_certificatekeypair.
Verified
python3 -m unittest: 49 tests green (spec parsing/validation, status, creation, idempotent second pass, missing group, foreign app, redirect drift, ownership scan + removal next to a foreign OAuth2 app).
API fields/endpoints checked against Authentik 2026.8.4 source (providers/oauth2/api/providers.py, urls.py; client_secret defaults to generate_client_secret, readable with change permission).
kubectl kustomize for infrastructure, apps/authentik, the CRD dir; yaml-citations.sh green. Server-side CRD validation not possible with my read-only access.
Not verified until merge: the image against the live API. #1534 (YouTrack) uses this and should merge after it is live.
authentik-gate-sync learns OIDC clients, declared as an `OIDCClient` resource next to the app, so apps like YouTrack need no blueprint entries and nothing on their Ingress.
## What
- **CRD** `oidcclients.auth.renner.uno` (v1alpha1, namespaced, status subresource) in `infrastructure/authentik-gate-sync-crds`, like `kuma-sync-crds`.
Spec: `launchURL`, `redirectURIs[] {url, matchingMode strict|regex}`, optional `slug` (default `metadata.name`), `group`, `authorizationFlow`.
- **Controller**, per OIDCClient:
- confidential OAuth2/OpenID provider `oidc-<slug>`: client ID = slug, secret generated by Authentik, grants `authorization_code` + `refresh_token`, scopes openid/profile/email, signed with `authentik Self-signed Certificate`;
- application `<slug>` → issuer `https://auth.renner.uno/application/o/<slug>/`, `meta_publisher: authentik-gate-sync`;
- with `group`: the same group expression policy + binding as gates (the group itself stays managed in Authentik);
- status: `Ready` condition with the reason, and `issuer`.
- Fail closed and ownership as for gates: a missing group, a foreign application/provider with the name, or two OIDCClients with one slug withhold/remove the client; owned clients whose OIDCClient is gone are swept. Missing CRD = no clients, gates unaffected.
- RBAC: ClusterRole gets get/list/watch on `oidcclients` and patch on `oidcclients/status`; the Authentik role (blueprint `gate-sync.yaml`) gets `authentik_providers_oauth2.{view,add,change,delete}_oauth2provider`, `view_scopemapping`, `authentik_crypto.view_certificatekeypair`.
## Verified
- `python3 -m unittest`: 49 tests green (spec parsing/validation, status, creation, idempotent second pass, missing group, foreign app, redirect drift, ownership scan + removal next to a foreign OAuth2 app).
- API fields/endpoints checked against Authentik 2026.8.4 source (`providers/oauth2/api/providers.py`, `urls.py`; `client_secret` defaults to `generate_client_secret`, readable with change permission).
- `kubectl kustomize` for infrastructure, apps/authentik, the CRD dir; `yaml-citations.sh` green. Server-side CRD validation not possible with my read-only access.
Not verified until merge: the image against the live API. #1534 (YouTrack) uses this and should merge after it is live.
An Ingress annotated auth.renner.uno/oidc-client: <slug> gets a
confidential OAuth2/OpenID provider (client ID = slug, secret generated by
Authentik), an application with that slug and, with auth.renner.uno/group,
the same group expression policy as gates. auth.renner.uno/oidc-redirect-uri
pins strict redirect URIs on the Ingress host; default is any path on it.
Same fail-closed and ownership rules as gates: owned clients are found via
the OAuth2 provider list and meta_publisher and swept when their Ingress
goes. The service-account role gains the OAuth2 provider permissions plus
view on scope mappings and certificate key pairs.
So apps that log in through Authentik via OIDC (first: YouTrack, #1534)
need no blueprint entries; the groups stay managed in Authentik.
hermes-bot
changed title from feat(authentik-gate-sync): OIDC clients from auth.renner.uno/oidc-client to feat(authentik-gate-sync): OIDC clients from OIDCClient resources2026-11-01 09:31:44 +00:00
OIDC is not part of the Ingress. The client is now declared by an
OIDCClient (auth.renner.uno/v1alpha1, CRD in
infrastructure/authentik-gate-sync-crds, like kuma-sync-crds) next to the
app: launchURL, redirectURIs, optional slug, group, authorizationFlow.
The controller writes a Ready condition and the issuer to its status.
The auth.renner.uno/oidc-* Ingress annotations are gone; RBAC gains
get/list/watch on oidcclients and patch on their status.
🤖Automated review — model claude-opus-5-5, commit 3e0557cfe812d377c831ad1e1a61d4485014afa5.
Automated review by the review-bot: it approves only when it found nothing red or yellow, and a human still merges. Request a review from review-bot again for a fresh one.
Summary
This PR teaches authentik-gate-sync to reconcile a new namespaced OIDCClient CRD into a confidential Authentik OAuth2 provider, an application and an optional group policy. It also extends the ownership sweep to OAuth2 providers and adds the RBAC and Authentik permissions the controller needs. The change is well built: it fails closed, refuses foreign applications and providers, detects duplicate slugs and comes with unit tests. I found no verified defect. The main gap is that OIDC slugs and gate names share one namespace and nothing separates them. mergeable is true, and no other open or closed PR covers this work (#1534 depends on it; #1222 was an earlier per-app OIDC change and is closed).
⚪ Unverified
OAuth2 field names and the live API. I did not check the grant_types field, the propertymappings/provider/scope/ endpoint, or whether the redirect_uris returned by the API compare equal to the desired body (controller.py:303-313, 547). If any of these differ from what the controller expects, every pass would re-PATCH the provider. The PR says it checked them against the Authentik source, but not against a running instance. After the deploy, check the logs for repeated "updated OAuth2 provider" lines.
Signing-key name. The certificate is matched only by its exact name, authentik Self-signed Certificate (controller.py:57, 544). If the instance names it differently, every client stays in "transient" state. That is safe, but worth confirming before #1534 merges.
🟡 Suggestions
Gate names and OIDC slugs can collide. Both are application slugs and both go into the same gates set, but ensure_oidc only checks meta_publisher == MANAGER (controller.py:560-562). If an OIDCClient's slug equals an existing gate name (<ns>-<ingress>-<hash6>, controller.py:113-118), two things go wrong:
On success, ensure_oidc creates oidc-<slug> and then PATCHes the gate's application so its provider points to the OAuth2 provider (controller.py:585-586). This takes over the gate.
On a GateError, gates.discard(slug) (controller.py:703) drops the gate's name, so the sweep deletes the gate (controller.py:725-727).
The hash suffix makes an accidental collision unlikely, but anyone who can create an OIDCClient in any namespace can cause it on purpose. A cheap guard is to refuse an existing application whose meta_description does not start with OIDCClient (or whose provider is not in providers/oauth2/). Another option is to keep OIDC slugs in their own set and reject a slug that is already in the gates set.
Unassigned providers are adopted. If a provider named oidc-<slug> already exists without an application, it is taken over and PATCHed (controller.py:565, 570-577). For gates this is intended, after an interrupted teardown. For OIDC it could also take over a hand-made provider of the same name. Consider requiring that the existing provider was created by this controller, or document this behaviour in the module docstring.
Status is patched on every pass. Each OIDCClient's status is written every pass (every 60 s), even when nothing changed (controller.py:710-712). Comparing oidc_status(...) with cr.get("status") first would avoid constant writes to the API server and watch events for anyone watching the resource.
🟢 Nits
The removal log line still says "removed gate from Authentik" for OIDC clients too (controller.py:615).
The CRD's slug pattern ^[a-z0-9]([a-z0-9-]*[a-z0-9])?$ plus maxLength: 50 matches SLUG_RE, but a default slug taken from metadata.name is only checked by the controller. That is fine, but the CRD description could say so.
Tests
The new tests cover spec parsing and validation, status and transition-time stability, creation, a second idempotent pass, a missing group, a foreign application, redirect drift, and the ownership scan plus removal next to a foreign OAuth2 app. No test drives reconcile() with OIDC clients, so several paths are untested:
a duplicate slug;
the transient path keeping the client;
the sweep of an OIDC client whose CR is gone;
an OIDCClient slug that collides with a gate name (see the first suggestion).
Recommendation
Looks reasonable — no blocking issues; comments above are optional.
🤖 **Automated review** — model `claude-opus-5-5`, commit `3e0557cfe812d377c831ad1e1a61d4485014afa5`.
> Automated review by the `review-bot`: it approves only when it found nothing red or yellow, and a human still merges. Request a review from `review-bot` again for a fresh one.
---
## Summary
This PR teaches authentik-gate-sync to reconcile a new namespaced `OIDCClient` CRD into a confidential Authentik OAuth2 provider, an application and an optional group policy. It also extends the ownership sweep to OAuth2 providers and adds the RBAC and Authentik permissions the controller needs. The change is well built: it fails closed, refuses foreign applications and providers, detects duplicate slugs and comes with unit tests. I found no verified defect. The main gap is that OIDC slugs and gate names share one namespace and nothing separates them. `mergeable` is true, and no other open or closed PR covers this work (#1534 depends on it; #1222 was an earlier per-app OIDC change and is closed).
## ⚪ Unverified
- **OAuth2 field names and the live API.** I did not check the `grant_types` field, the `propertymappings/provider/scope/` endpoint, or whether the `redirect_uris` returned by the API compare equal to the desired body (`controller.py:303-313`, `547`). If any of these differ from what the controller expects, every pass would re-PATCH the provider. The PR says it checked them against the Authentik source, but not against a running instance. After the deploy, check the logs for repeated "updated OAuth2 provider" lines.
- **Signing-key name.** The certificate is matched only by its exact name, `authentik Self-signed Certificate` (`controller.py:57`, `544`). If the instance names it differently, every client stays in "transient" state. That is safe, but worth confirming before #1534 merges.
## 🟡 Suggestions
- **Gate names and OIDC slugs can collide.** Both are application slugs and both go into the same `gates` set, but `ensure_oidc` only checks `meta_publisher == MANAGER` (`controller.py:560-562`). If an OIDCClient's `slug` equals an existing gate name (`<ns>-<ingress>-<hash6>`, `controller.py:113-118`), two things go wrong:
- On success, `ensure_oidc` creates `oidc-<slug>` and then PATCHes the gate's application so its `provider` points to the OAuth2 provider (`controller.py:585-586`). This takes over the gate.
- On a `GateError`, `gates.discard(slug)` (`controller.py:703`) drops the gate's name, so the sweep deletes the gate (`controller.py:725-727`).
The hash suffix makes an accidental collision unlikely, but anyone who can create an OIDCClient in any namespace can cause it on purpose. A cheap guard is to refuse an existing application whose `meta_description` does not start with `OIDCClient ` (or whose provider is not in `providers/oauth2/`). Another option is to keep OIDC slugs in their own set and reject a slug that is already in the gates set.
- **Unassigned providers are adopted.** If a provider named `oidc-<slug>` already exists without an application, it is taken over and PATCHed (`controller.py:565`, `570-577`). For gates this is intended, after an interrupted teardown. For OIDC it could also take over a hand-made provider of the same name. Consider requiring that the existing provider was created by this controller, or document this behaviour in the module docstring.
- **Status is patched on every pass.** Each OIDCClient's status is written every pass (every 60 s), even when nothing changed (`controller.py:710-712`). Comparing `oidc_status(...)` with `cr.get("status")` first would avoid constant writes to the API server and watch events for anyone watching the resource.
## 🟢 Nits
- The removal log line still says "removed gate from Authentik" for OIDC clients too (`controller.py:615`).
- The CRD's slug pattern `^[a-z0-9]([a-z0-9-]*[a-z0-9])?$` plus `maxLength: 50` matches `SLUG_RE`, but a default slug taken from `metadata.name` is only checked by the controller. That is fine, but the CRD description could say so.
## Tests
The new tests cover spec parsing and validation, status and transition-time stability, creation, a second idempotent pass, a missing group, a foreign application, redirect drift, and the ownership scan plus removal next to a foreign OAuth2 app. No test drives `reconcile()` with OIDC clients, so several paths are untested:
- a duplicate slug;
- the transient path keeping the client;
- the sweep of an OIDC client whose CR is gone;
- an OIDCClient slug that collides with a gate name (see the first suggestion).
## Recommendation
Looks reasonable — no blocking issues; comments above are optional.
<!-- pr-review:sha=3e0557cfe812d377c831ad1e1a61d4485014afa5 -->
Live API comparison: checked. RedirectURISerializer (2026.8.4) also returns redirect_uri_type (default authorization), so the provider would have been re-PATCHed on every pass. The desired body now includes it. grant_types is a serializer field, and propertymappings/provider/scope is registered in providers/oauth2/urls.py.
Signing key:authentik Self-signed Certificate is the key the four existing OAuth2 providers in the blueprint use, so it exists live.
Gate/OIDC collision: fixed in three places. A slug equal to a gate name is refused. An existing application without this controller's oidc-<slug> provider is refused, so no re-pointing. The sweep keeps proxy and OAuth2 applications in separate sets.
Unassigned provider: adoption stays as it is, because teardown deletes the application first. It is now documented in the module docstring.
Status: it is written only when it changed.
Nits: the removal log now says "OIDC client", and the CRD slug description mentions the controller-only check.
Tests:reconcile() takes injectable clients. There are now loop tests for create-then-quiet (no API calls or status writes on pass 2), duplicate slug, the transient path keeping the client, the sweep after the CR is gone, and a slug equal to a gate name leaving the gate untouched. 55 tests pass.
Addressed in the latest commit:
- **Live API comparison:** checked. `RedirectURISerializer` (2026.8.4) also returns `redirect_uri_type` (default `authorization`), so the provider would have been re-PATCHed on every pass. The desired body now includes it. `grant_types` is a serializer field, and `propertymappings/provider/scope` is registered in `providers/oauth2/urls.py`.
- **Signing key:** `authentik Self-signed Certificate` is the key the four existing OAuth2 providers in the blueprint use, so it exists live.
- **Gate/OIDC collision:** fixed in three places. A slug equal to a gate name is refused. An existing application without this controller's `oidc-<slug>` provider is refused, so no re-pointing. The sweep keeps proxy and OAuth2 applications in separate sets.
- **Unassigned provider:** adoption stays as it is, because teardown deletes the application first. It is now documented in the module docstring.
- **Status:** it is written only when it changed.
- **Nits:** the removal log now says "OIDC client", and the CRD slug description mentions the controller-only check.
- **Tests:** `reconcile()` takes injectable clients. There are now loop tests for create-then-quiet (no API calls or status writes on pass 2), duplicate slug, the transient path keeping the client, the sweep after the CR is gone, and a slug equal to a gate name leaving the gate untouched. 55 tests pass.
hermes-bot
requested review from review-bot 2026-11-01 09:49:34 +00:00
- Desired redirect URIs carry redirect_uri_type: authorization, which the
API returns on every entry (RedirectURISerializer default in 2026.8.4);
without it every pass would re-PATCH the provider.
- OIDC slugs and gate names are kept apart: a slug that is a gate name is
refused, an existing application without this controller's OAuth2
provider is refused, and the sweep keeps proxy and OAuth2 applications
by their own sets, so an OIDCClient can neither take over nor delete a
gate.
- Status is written only when it changed.
- Adoption of an unassigned oidc-<slug> provider is documented; removal
logs say OIDC client.
- reconcile() takes k8s/ak for tests; new loop tests: create then quiet,
duplicate slug, transient error keeps the client, sweep after the
resource is gone, slug equal to a gate name.
🤖Automated review — model claude-opus-5-5, commit f76defb1e1688a0c757cad51ab7a4374aa1dc7fe.
Automated review by the review-bot: it approves only when it found nothing red or yellow, and a human still merges. Request a review from review-bot again for a fresh one.
Summary
Since my review at 3e0557cf, one commit (f76defb1) has been pushed. It answers every earlier point:
Gate/OIDC slug collision (🟡): fixed. reconcile() now refuses a slug that equals a gate name (controller.py:704-705). ensure_oidc refuses an existing application unless it already points at this controller's oidc-<slug> provider (controller.py:570-572). The sweep checks OAuth2 applications against keep_clients and proxy applications against gates, so a failing OIDCClient can no longer remove a gate's name (controller.py:741-742).
Adoption of an unassigned oidc-<slug> provider (🟡): the author kept this on purpose, because teardown deletes the application first, and documented it in the module docstring (lines 27-29). I consider it closed.
Status written on every pass (🟡): fixed. Status is now patched only when oidc_status(...) differs from the stored status (controller.py:726-727). Unchanged conditions keep their lastTransitionTime, so the comparison holds.
Nits: both fixed. The removal log line now says "OIDC client", and the CRD slug description says the default slug is checked only by the controller.
Re-PATCH on every pass (earlier ⚪): handled by adding redirect_uri_type: "authorization" to the desired redirect entries (controller.py:268-269).
The new code has no blocking problems. mergeable is true.
⚪ Unverified
redirect_uri_type in API responses. The fix relies on the Authentik serializer returning redirect_uri_type on every redirect entry. That is the author's reading of the 2026.8.4 source, and I did not check it against the source or a running instance. If the field were missing from responses, the only effect would be a PATCH on every pass. After the deploy, check that "updated OAuth2 provider" is not logged on every pass.
🟢 Nits
The second half of the elif in ensure_oidc can no longer be reached. The new check just above it already raises whenever the application exists and does not point at the provider:
(app is not None and app.get("provider") != provider["pk"])
The clause could be dropped.
An empty line was doubled before test_refuses_application_without_its_provider in test_controller.py.
Tests
The new ReconcileLoop tests drive reconcile() end to end through the injected FakeK8s and FakeAuthentik. They cover:
create, then a quiet second pass with no API calls and no status writes;
a duplicate slug withholding both clients;
a transient error keeping the client;
the sweep after the OIDCClient is gone;
a slug equal to a gate name leaving the gate untouched.
test_refuses_application_without_its_provider covers the no-re-pointing guard. Together these cover every test gap I listed in the earlier review.
Recommendation
Looks reasonable — no blocking issues; comments above are optional. The earlier review had no 🔴 points, every 🟡 point has been fixed or answered with a reason, and the new commit adds no 🔴.
🤖 **Automated review** — model `claude-opus-5-5`, commit `f76defb1e1688a0c757cad51ab7a4374aa1dc7fe`.
> Automated review by the `review-bot`: it approves only when it found nothing red or yellow, and a human still merges. Request a review from `review-bot` again for a fresh one.
---
## Summary
Since my review at `3e0557cf`, one commit (`f76defb1`) has been pushed. It answers every earlier point:
- **Gate/OIDC slug collision (🟡):** fixed. `reconcile()` now refuses a slug that equals a gate name (`controller.py:704-705`). `ensure_oidc` refuses an existing application unless it already points at this controller's `oidc-<slug>` provider (`controller.py:570-572`). The sweep checks OAuth2 applications against `keep_clients` and proxy applications against `gates`, so a failing OIDCClient can no longer remove a gate's name (`controller.py:741-742`).
- **Adoption of an unassigned `oidc-<slug>` provider (🟡):** the author kept this on purpose, because teardown deletes the application first, and documented it in the module docstring (lines 27-29). I consider it closed.
- **Status written on every pass (🟡):** fixed. Status is now patched only when `oidc_status(...)` differs from the stored status (`controller.py:726-727`). Unchanged conditions keep their `lastTransitionTime`, so the comparison holds.
- **Nits:** both fixed. The removal log line now says "OIDC client", and the CRD slug description says the default slug is checked only by the controller.
- **Re-PATCH on every pass (earlier ⚪):** handled by adding `redirect_uri_type: "authorization"` to the desired redirect entries (`controller.py:268-269`).
The new code has no blocking problems. `mergeable` is true.
## ⚪ Unverified
- **`redirect_uri_type` in API responses.** The fix relies on the Authentik serializer returning `redirect_uri_type` on every redirect entry. That is the author's reading of the 2026.8.4 source, and I did not check it against the source or a running instance. If the field were missing from responses, the only effect would be a PATCH on every pass. After the deploy, check that "updated OAuth2 provider" is not logged on every pass.
## 🟢 Nits
- The second half of the `elif` in `ensure_oidc` can no longer be reached. The new check just above it already raises whenever the application exists and does not point at the provider:
```
(app is not None and app.get("provider") != provider["pk"])
```
The clause could be dropped.
- An empty line was doubled before `test_refuses_application_without_its_provider` in `test_controller.py`.
## Tests
The new `ReconcileLoop` tests drive `reconcile()` end to end through the injected `FakeK8s` and `FakeAuthentik`. They cover:
- create, then a quiet second pass with no API calls and no status writes;
- a duplicate slug withholding both clients;
- a transient error keeping the client;
- the sweep after the OIDCClient is gone;
- a slug equal to a gate name leaving the gate untouched.
`test_refuses_application_without_its_provider` covers the no-re-pointing guard. Together these cover every test gap I listed in the earlier review.
## Recommendation
Looks reasonable — no blocking issues; comments above are optional. The earlier review had no 🔴 points, every 🟡 point has been fixed or answered with a reason, and the new commit adds no 🔴.
<!-- pr-review:sha=f76defb1e1688a0c757cad51ab7a4374aa1dc7fe -->
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
authentik-gate-sync learns OIDC clients, declared as an
OIDCClientresource next to the app, so apps like YouTrack need no blueprint entries and nothing on their Ingress.What
oidcclients.auth.renner.uno(v1alpha1, namespaced, status subresource) ininfrastructure/authentik-gate-sync-crds, likekuma-sync-crds.Spec:
launchURL,redirectURIs[] {url, matchingMode strict|regex}, optionalslug(defaultmetadata.name),group,authorizationFlow.oidc-<slug>: client ID = slug, secret generated by Authentik, grantsauthorization_code+refresh_token, scopes openid/profile/email, signed withauthentik Self-signed Certificate;<slug>→ issuerhttps://auth.renner.uno/application/o/<slug>/,meta_publisher: authentik-gate-sync;group: the same group expression policy + binding as gates (the group itself stays managed in Authentik);Readycondition with the reason, andissuer.oidcclientsand patch onoidcclients/status; the Authentik role (blueprintgate-sync.yaml) getsauthentik_providers_oauth2.{view,add,change,delete}_oauth2provider,view_scopemapping,authentik_crypto.view_certificatekeypair.Verified
python3 -m unittest: 49 tests green (spec parsing/validation, status, creation, idempotent second pass, missing group, foreign app, redirect drift, ownership scan + removal next to a foreign OAuth2 app).providers/oauth2/api/providers.py,urls.py;client_secretdefaults togenerate_client_secret, readable with change permission).kubectl kustomizefor infrastructure, apps/authentik, the CRD dir;yaml-citations.shgreen. Server-side CRD validation not possible with my read-only access.Not verified until merge: the image against the live API. #1534 (YouTrack) uses this and should merge after it is live.
feat(authentik-gate-sync): OIDC clients from auth.renner.uno/oidc-clientto feat(authentik-gate-sync): OIDC clients from OIDCClient resources🤖 Automated review — model
claude-opus-5-5, commit3e0557cfe812d377c831ad1e1a61d4485014afa5.Summary
This PR teaches authentik-gate-sync to reconcile a new namespaced
OIDCClientCRD into a confidential Authentik OAuth2 provider, an application and an optional group policy. It also extends the ownership sweep to OAuth2 providers and adds the RBAC and Authentik permissions the controller needs. The change is well built: it fails closed, refuses foreign applications and providers, detects duplicate slugs and comes with unit tests. I found no verified defect. The main gap is that OIDC slugs and gate names share one namespace and nothing separates them.mergeableis true, and no other open or closed PR covers this work (#1534 depends on it; #1222 was an earlier per-app OIDC change and is closed).⚪ Unverified
grant_typesfield, thepropertymappings/provider/scope/endpoint, or whether theredirect_urisreturned by the API compare equal to the desired body (controller.py:303-313,547). If any of these differ from what the controller expects, every pass would re-PATCH the provider. The PR says it checked them against the Authentik source, but not against a running instance. After the deploy, check the logs for repeated "updated OAuth2 provider" lines.authentik Self-signed Certificate(controller.py:57,544). If the instance names it differently, every client stays in "transient" state. That is safe, but worth confirming before #1534 merges.🟡 Suggestions
Gate names and OIDC slugs can collide. Both are application slugs and both go into the same
gatesset, butensure_oidconly checksmeta_publisher == MANAGER(controller.py:560-562). If an OIDCClient'sslugequals an existing gate name (<ns>-<ingress>-<hash6>,controller.py:113-118), two things go wrong:ensure_oidccreatesoidc-<slug>and then PATCHes the gate's application so itsproviderpoints to the OAuth2 provider (controller.py:585-586). This takes over the gate.GateError,gates.discard(slug)(controller.py:703) drops the gate's name, so the sweep deletes the gate (controller.py:725-727).The hash suffix makes an accidental collision unlikely, but anyone who can create an OIDCClient in any namespace can cause it on purpose. A cheap guard is to refuse an existing application whose
meta_descriptiondoes not start withOIDCClient(or whose provider is not inproviders/oauth2/). Another option is to keep OIDC slugs in their own set and reject a slug that is already in the gates set.Unassigned providers are adopted. If a provider named
oidc-<slug>already exists without an application, it is taken over and PATCHed (controller.py:565,570-577). For gates this is intended, after an interrupted teardown. For OIDC it could also take over a hand-made provider of the same name. Consider requiring that the existing provider was created by this controller, or document this behaviour in the module docstring.Status is patched on every pass. Each OIDCClient's status is written every pass (every 60 s), even when nothing changed (
controller.py:710-712). Comparingoidc_status(...)withcr.get("status")first would avoid constant writes to the API server and watch events for anyone watching the resource.🟢 Nits
controller.py:615).^[a-z0-9]([a-z0-9-]*[a-z0-9])?$plusmaxLength: 50matchesSLUG_RE, but a default slug taken frommetadata.nameis only checked by the controller. That is fine, but the CRD description could say so.Tests
The new tests cover spec parsing and validation, status and transition-time stability, creation, a second idempotent pass, a missing group, a foreign application, redirect drift, and the ownership scan plus removal next to a foreign OAuth2 app. No test drives
reconcile()with OIDC clients, so several paths are untested:Recommendation
Looks reasonable — no blocking issues; comments above are optional.
Addressed in the latest commit:
RedirectURISerializer(2026.8.4) also returnsredirect_uri_type(defaultauthorization), so the provider would have been re-PATCHed on every pass. The desired body now includes it.grant_typesis a serializer field, andpropertymappings/provider/scopeis registered inproviders/oauth2/urls.py.authentik Self-signed Certificateis the key the four existing OAuth2 providers in the blueprint use, so it exists live.oidc-<slug>provider is refused, so no re-pointing. The sweep keeps proxy and OAuth2 applications in separate sets.reconcile()takes injectable clients. There are now loop tests for create-then-quiet (no API calls or status writes on pass 2), duplicate slug, the transient path keeping the client, the sweep after the CR is gone, and a slug equal to a gate name leaving the gate untouched. 55 tests pass.New commits pushed, approval review dismissed automatically according to repository settings
🤖 Automated review — model
claude-opus-5-5, commitf76defb1e1688a0c757cad51ab7a4374aa1dc7fe.Summary
Since my review at
3e0557cf, one commit (f76defb1) has been pushed. It answers every earlier point:reconcile()now refuses a slug that equals a gate name (controller.py:704-705).ensure_oidcrefuses an existing application unless it already points at this controller'soidc-<slug>provider (controller.py:570-572). The sweep checks OAuth2 applications againstkeep_clientsand proxy applications againstgates, so a failing OIDCClient can no longer remove a gate's name (controller.py:741-742).oidc-<slug>provider (🟡): the author kept this on purpose, because teardown deletes the application first, and documented it in the module docstring (lines 27-29). I consider it closed.oidc_status(...)differs from the stored status (controller.py:726-727). Unchanged conditions keep theirlastTransitionTime, so the comparison holds.redirect_uri_type: "authorization"to the desired redirect entries (controller.py:268-269).The new code has no blocking problems.
mergeableis true.⚪ Unverified
redirect_uri_typein API responses. The fix relies on the Authentik serializer returningredirect_uri_typeon every redirect entry. That is the author's reading of the 2026.8.4 source, and I did not check it against the source or a running instance. If the field were missing from responses, the only effect would be a PATCH on every pass. After the deploy, check that "updated OAuth2 provider" is not logged on every pass.🟢 Nits
elifinensure_oidccan no longer be reached. The new check just above it already raises whenever the application exists and does not point at the provider:test_refuses_application_without_its_providerintest_controller.py.Tests
The new
ReconcileLooptests drivereconcile()end to end through the injectedFakeK8sandFakeAuthentik. They cover:test_refuses_application_without_its_providercovers the no-re-pointing guard. Together these cover every test gap I listed in the earlier review.Recommendation
Looks reasonable — no blocking issues; comments above are optional. The earlier review had no 🔴 points, every 🟡 point has been fixed or answered with a reason, and the new commit adds no 🔴.