GitOps hardening: console ingress mode, idempotent DB delete, JWT override warning, roles_claim validation - #218
Conversation
AuthorizeApi is attached on the parent apiV1Router, which in gorilla/mux runs its .Use() middleware BEFORE the child subrouters. The child-level AuthenticateAccountJWT is what populates the username context, so reading auth.GetUsernameFromContext at this parent-router middleware always saw an empty username and rejected every authenticated request with "401 Unauthorized: missing identity" whenever RBAC_ENFORCE=true. Derive identity via auth.GetAuthPayload(r) instead, exactly like the sibling UserProvisioningMiddleware (also attached at the parent level). On HTTP the framework's global jwtHandler has already validated the token and placed it in the request context, so GetAuthPayload works at any router depth while GetUsernameFromContext does not. HTTP-only fix: the gRPC path is already correct. hypershell registers its RBAC interceptors as post-auth (plugins/rbac/grpc_init.go) and the framework's AuthUnaryInterceptor calls SetUsernameContext before post-auth interceptors run, so GetUsernameFromContext is populated there. Adds TestAuthorizeApiAllowsBoundUserFromJWTContext as a regression guard and updates the existing conceal-mutations test to inject identity the way the real HTTP flow does (validated token in context). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…0-1, P0-2) The per-gateway console unconditionally emitted a Gateway API HTTPRoute whose parentRef.name came from GATEWAY_API_GATEWAY_NAME. On a route-mode cluster that var is unset, so the name rendered empty, the API server rejected the object, and the self-heal loop retried every tick -- the console never became servable under the managed Keycloak posture. - reconcileConsoleIngress now follows the effective ingress mode: an OpenShift edge-terminated Route in route mode, the HTTPRoute in gateway-api mode, and nothing in none mode (mirrors the gateway ingress path). - gateway-api mode fails fast when GATEWAY_API_GATEWAY_NAME is unset instead of submitting an invalid HTTPRoute (P0-2), mirroring reconcileGatewayAPIResources. - console_address readiness (syncConsoleAddress) and the self-heal path now resolve mode too: ConsoleExposureReady dispatches to the HTTPRoute or the OpenShift Route accordingly, and selfHealConsole sets HasGatewayAPI so the console reconcile picks the same mode the provisioning path would. - deleteConsole sweeps both ingress objects regardless of mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ete as idempotent (P1-1) A gateway delete-reconcile resolves the gateway's ManagedDatabase to obtain teardown config. When the ManagedDatabase was already deleted, GetManagedDatabase returns NotFound, which was appended to deleteErrs and failed the whole delete-reconcile. The watcher then retried every 30s forever, because the ManagedDatabase never comes back. Treat NotFound as already-cleaned and continue finalizing the gateway; any other resolve error still fails so a transient condition is retried. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…=true (P2-3) The development environment's OverrideConfig runs after CLI flags are parsed and force-disabled Auth.EnableJWT with no signal, so --enable-jwt=true silently turned into 401 'missing identity' on every request. Log a loud warning naming the overridden flag and pointing to API_ENV=development_oidc before applying the override. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…apping is set (P3-1) A gateway with admin_role/user_role but no roles_claim can never map a token to a role, producing a dead-on-arrival gateway (the default roles_claim=groups is absent on realms that emit roles under 'roles'/'realm_access.roles'). Reject that combination at config-validation time and validate that any roles_claim is a well-formed dot-separated JWT claim path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Amber reviewStatus: Complete VerdictREQUEST_CHANGES. The three genuinely net-new fixes (idempotent DB-NotFound delete, dev JWT override warning, roles_claim validation) are well-reasoned and well-tested, but this branch is cut from a stale base and re-implements two changes that are already on Amber Analysis This PR bundles four GitOps-hardening fixes. Two of them (console OpenShift Route ingress + readiness, and RBAC identity derivation from the JWT payload) have already landed on Blocking / structural1. [Major] Stale base — console ingress + RBAC identity are already merged on 2. [Major] 3. [Major] What's good (keep)
Cross-PR coordinationNo material cross-PR coordination issue requires maintainer action. Findings Summary (ordered by severity, highest first):
Convention Checklist:
|
jsell-rh
left a comment
There was a problem hiding this comment.
Verdict
REQUEST_CHANGES. The three genuinely net-new fixes (idempotent DB-NotFound delete, dev JWT override warning, roles_claim validation) are well-reasoned and well-tested, but this branch is cut from a stale base and re-implements two changes that are already on main with a different design, so it will not merge cleanly and needs a rebase plus one open question on the roles_claim tightening.
Amber Analysis
This PR bundles four GitOps-hardening fixes. Two of them (console OpenShift Route ingress + readiness, and RBAC identity derivation from the JWT payload) have already landed on main since this branch's base commit (b9f05c7), so a large part of the diff is duplicated and, in the console case, diverges behaviorally from the merged implementation. The remaining three fixes are solid, with one optional -> required validation change that needs a fallback decision.
Blocking / structural
1. [Major] Stale base — console ingress + RBAC identity are already merged on main, with a divergent design.
This branch's merge-base is b9f05c7. main is now at 73d12be and already includes the RBAC identity fix (#215, AuthorizeApi reading the JWT payload) and the console OpenShift Route ingress support (#216). main's console.go already defines ConsoleExposureReady (branching on ingress mode), consoleOpenShiftRouteReady, consoleHTTPRouteReady, and a reconcileConsole that branches ingress creation on gatewayIngressMode(opts) (including IngressModeNone). This PR re-introduces the same behavior under different names (ConsoleOpenShiftRouteReady, ConsoleRouteReady, consoleRouteGVR, reconcileConsoleIngress, buildConsoleRoute) and re-applies the identical authorization.go change. As-is the branch will conflict on rebase and duplicates merged work. Please rebase onto current main, drop the console-ingress and authorization.go changes that are now redundant, and keep only the net-new P1-1 / P2-3 / P3-1 fixes.
2. [Major] ConsoleExposureReady treats unknown / "none" ingress mode as ready, diverging from the merged implementation.
In this branch, ConsoleExposureReady's default case returns (true, "", nil). The merged version on main returns false for IngressModeNone ("no console ingress mode selected") and an error for an unsupported mode. Returning ready when there is no managed console ingress lets syncConsoleAddress publish console_address against a host that has no Route/HTTPRoute — exactly the dead-link failure this PR set out to eliminate. This concern disappears once you adopt main's implementation (finding 1), which is the recommended path.
3. [Major] roles_claim moves from optional to required with no fallback for pre-existing BYO configs.
ValidateOIDCConfig now rejects any config where admin_role/user_role are set but roles_claim is empty, and ValidateGatewayConfig runs on the reconcile path (reconciler.go:67). manifests.go:164-165 only emits roles_claim when non-empty, so a blank value previously delegated to the gateway's own default. Managed gateways are unaffected because the provisioning path always sets RolesClaim: "hypershell.roles" (reconciler.go:1585). But a custom/BYO OIDC config that set admin_role/user_role and left roles_claim blank — relying on the OpenShell gateway's default claim — would now fail validation on the next reconcile with no migration or fallback. Please confirm whether the gateway defaults roles_claim (e.g. to roles) when the key is omitted. If it does, default RolesClaim to that value instead of erroring; if it does not, the "dead on arrival" justification holds and this is fine as written. This is the optional -> required scrutiny case (records could predate the change), so it needs an explicit answer before merge.
What's good (keep)
- P1-1 (idempotent DB-NotFound delete) is correct.
resolveDatabaseConfigwraps the gRPC error with%w, andstatus.Code/status.FromErrorunder grpcv1.83.0unwraps viaerrors.As, sostatus.Code(dbErr) == codes.NotFoundmatches; the in-process gRPC test exercises the real path and proves it. The NotFound carve-out is correctly scoped so transient errors still fail and retry. - P2-3 (dev JWT override warning) is clean: the override behavior is unchanged, the warning is only emitted when
--enable-jwt=truewas explicitly requested, the helper is extracted for testability, and no secrets are logged. - Tests are additive and individually justified — no pre-existing assertion was flipped from accept to reject.
Cross-PR coordination
No material cross-PR coordination issue requires maintainer action.
Findings Summary (ordered by severity, highest first):
- [Major] Stale base — console ingress + RBAC identity already merged on
main; rebase and drop duplicates - Structural / Spec Consistency (console.go, authorization.go) - [Major]
ConsoleExposureReadyreturns ready for unknown/"none" mode, diverging from merged design; can publish a dead console link - Reconciliation (console.go L118-125) - [Major]
roles_claimoptional -> required with no fallback for BYO configs; confirm gateway default - Test Diff Scrutiny (validation.go L176-188) - [Minor] Confusing readiness naming:
ConsoleRouteReadychecks the HTTPRoute, not an OpenShift Route - Convention (console.go L136)
Convention Checklist:
| Convention | Result |
|---|---|
No panic() in production code |
Pass |
Errors wrapped with fmt.Errorf context |
Pass |
errors.IsNotFound/gRPC NotFound handled |
Pass |
| No secrets in logs or responses | Pass |
| Input validated (roles_claim / JWT claim path) | Pass |
| Reconcile pattern (delete idempotency) | Pass |
| Test Diff Scrutiny (optional -> required) | Needs confirmation |
Branch current with main / no duplicated merged work |
Fail |
| case IngressModeGatewayAPI: | ||
| return ConsoleRouteReady(ctx, dynamicClient, namespace) | ||
| default: | ||
| return true, "", nil |
There was a problem hiding this comment.
[Major] Divergent readiness for unknown/"none" ingress mode.
The default case returns (true, "", nil), so any mode that isn't route/gateway-api (including IngressModeNone) is reported ready. syncConsoleAddress then publishes console_address even though no console Route/HTTPRoute exists — the dead-link case this PR is trying to prevent.
The version already merged on main (#216) returns false for IngressModeNone and errors on an unsupported mode. Rebasing onto main and adopting that implementation resolves this — this whole function is now duplicated there.
| // the gateway is dead on arrival (P3-1). The claim path itself must be a valid | ||
| // JWT claim reference so it can locate the roles in the token. | ||
| if rolesMapped && oidc.RolesClaim == "" { | ||
| return fmt.Errorf("roles_claim is required when admin_role/user_role are set; " + |
There was a problem hiding this comment.
[Major] optional -> required without a fallback for pre-existing BYO configs.
roles_claim is now required whenever admin_role/user_role are set, and ValidateGatewayConfig runs on the reconcile path (reconciler.go:67). manifests.go:164-165 only emits roles_claim when non-empty, so a blank value previously delegated to the gateway's own default. Managed gateways are unaffected (reconciler.go:1585 always sets hypershell.roles), but a custom OIDC config that set the roles but left roles_claim blank — relying on the gateway default — would now fail on its next reconcile with no migration/fallback.
Please confirm whether the OpenShell gateway defaults roles_claim (e.g. to roles) when the key is omitted. If it does, default RolesClaim to that value here instead of erroring; if it does not, the "dead on arrival" justification holds and this is fine. Needs an explicit answer before merge.
…dening # Conflicts: # components/control-plane/internal/gateway/console.go # components/control-plane/internal/reconciler/health.go # components/control-plane/internal/reconciler/reconciler.go
Amber reviewStatus: Complete VerdictThree well-scoped GitOps-hardening fixes (idempotent DB-NotFound delete, dev-JWT override warning, Amber AnalysisP1-1 — idempotent ManagedDatabase NotFound on delete ( P2-3 — dev JWT override warning ( P3-1 — Test Diff Scrutiny: Cross-PR coordinationNo material cross-PR coordination issue requires maintainer action. Findings Summary (ordered by severity, highest first)
Convention Checklist
|
jsell-rh
left a comment
There was a problem hiding this comment.
Verdict
Three well-scoped GitOps-hardening fixes (idempotent DB-NotFound delete, dev-JWT override warning, roles_claim validation), each shipped with focused tests and clear WHY-comments. The code is correct and convention-compliant; I have only minor, non-blocking observations plus a documentation nudge for the optional->required roles_claim tightening.
Amber Analysis
P1-1 — idempotent ManagedDatabase NotFound on delete (reconciler.go)
Correct. On NotFound, deleteDBConfig stays zero-valued, so the later deleteDBConfig.Provider == "deployment" branch is skipped and no phantom deployment-DB delete is attempted — the right behavior when the record is already gone. status.Code(dbErr) is called on the error returned by resolveDatabaseConfig, which wraps the gRPC error with %w; this works because grpc-go's status.FromError unwraps the chain, and the new TestGatewayDeleteWithAlreadyDeletedDatabaseIsIdempotent exercises exactly that wrapped path. The companion ...ResolveErrorStillFails test keeps the carve-out scoped to NotFound. Good aggregation via stderrors.Join.
P2-3 — dev JWT override warning (e_development.go)
Good. glog is already the api-server's logger (main.go, pkg/rbac/*), so this is consistent. Extracting devJWTOverrideWarning for testability is a nice touch, and the message names the overridden flag and points to development_oidc. No secrets logged.
P3-1 — roles_claim validation (validation.go)
Solid. The regex is linear (no ReDoS), messages are actionable, and error strings carry no secrets. See the inline note on the optional->required tightening.
Test Diff Scrutiny: validation_test.go is purely additive (new TestValidateOIDCConfig, 0 deletions) — no pre-existing assertion was flipped. e_development_test.go is new. No removed guarantees.
Cross-PR coordination
No material cross-PR coordination issue requires maintainer action.
Findings Summary (ordered by severity, highest first)
- [Minor]
roles_claimmoves from optional to required whenadmin_role/user_roleare set; the managed Keycloak path always supplies it, but manually-authored OIDC configs that predate this rule will now fail validation — worth calling out in the PR/spec — Test Diff Scrutiny / Spec Completeness (validation.go L182) - [Minor] NotFound idempotency relies on
status.Codeunwrapping the%w-wrapped error fromresolveDatabaseConfig; a one-line note (or asserting on the wrapped error) would protect the behavior against future refactors — Maintainability (reconciler.go L1335)
Convention Checklist
| Convention | Result |
|---|---|
No panic() in production code |
Pass |
Errors wrapped with fmt.Errorf context |
Pass |
errors.IsNotFound/gRPC NotFound handled |
Pass |
| No secrets in logs or error messages | Pass |
| Input validated (JWT claim path) | Pass |
| Status/error paths propagate (no swallowed failures) | Pass |
| Reconcile idempotency on delete | Pass |
| Test diff adds coverage without flipping guarantees | Pass |
| Conventional commit messages | Pass |
| // without a claim to read roles from, no token can ever map to admin/user and | ||
| // the gateway is dead on arrival (P3-1). The claim path itself must be a valid | ||
| // JWT claim reference so it can locate the roles in the token. | ||
| if rolesMapped && oidc.RolesClaim == "" { |
There was a problem hiding this comment.
[Minor] optional -> required tightening. Requiring roles_claim whenever admin_role/user_role are set is the right call for catching dead-on-arrival gateways. The managed Keycloak path (reconciler.go sets RolesClaim: "hypershell.roles" alongside the roles) always supplies it, so the default deployment flow is safe. The one gap: a manually-authored OIDC config persisted before this rule that has roles set but no roles_claim will now fail ValidateGatewayConfig on the next reconcile. Such a config never actually mapped roles, so failing loudly is an improvement rather than a regression — but please call this out explicitly in the PR body/spec so operators expect the new rejection instead of being surprised by it.
| // deleteErrs instead would fail the whole delete-reconcile, which the | ||
| // watcher retries every 30s -- forever, because the ManagedDatabase | ||
| // never comes back. Any other error still fails so it is retried. | ||
| if status.Code(dbErr) == codes.NotFound { |
There was a problem hiding this comment.
[Minor] correctness note. This branch depends on status.Code(dbErr) seeing NotFound even though resolveDatabaseConfig returns fmt.Errorf("resolve ManagedDatabase %s: %w", ...). It works today because grpc-go's status.FromError unwraps the %w chain, and the new idempotency test covers the wrapped path. To keep this robust against future refactors of the error wrapping, consider a short comment here (or asserting on the wrapped-error form) so a later change that swaps %w for a non-unwrapping wrapper doesn't silently turn the delete back into an infinite retry. Also worth noting for the record: on NotFound, deleteDBConfig stays zero-valued, so the later Provider == "deployment" delete is correctly skipped — good.
…cile Addresses Amber Finding 3 on #218. The P3-1 change made roles_claim a hard requirement whenever admin_role/user_role are set. Because ValidateGatewayConfig runs on the reconcile path (ReconcileGateway), a pre-existing BYO OIDC config that set the roles but omitted roles_claim -- relying on the gateway's own default (groups) -- would start failing to reconcile with no migration. Drop the required check and keep only the format validation for a non-empty roles_claim. A blank value continues to delegate to the gateway default, so manifests.go emits nothing and behavior is unchanged for existing gateways. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the review. Status on the four findings: Finding 1 (stale base — duplicate console/RBAC + redundant Finding 2 ( Finding 3 ( Finding 4 ( |
Amber reviewStatus: Complete VerdictThree tightly-scoped, well-tested bug fixes (idempotent DB-NotFound delete, dev JWT-override warning, roles_claim format validation) that all follow HyperShell conventions and introduce no blockers. My only substantive concerns are that the PR description overstates the actual net diff and one adjacent defense-in-depth gap; the code itself is safe to merge. Amber Analysis The net diff against Findings[Minor] PR description overstates scope — P0-1/P0-2 console ingress is not in this diff (Docs consistency, High confidence) [Minor] PR description contradicts the shipped roles_claim behavior (Docs / Test Diff Scrutiny, High confidence) [Minor] Defense-in-depth: sibling OIDC fields are still interpolated into TOML unvalidated (Security, Medium confidence) What I verified
Cross-PR coordinationNo material cross-PR coordination issue requires maintainer action. Findings Summary (ordered by severity, highest first):
Convention Checklist:
|
jsell-rh
left a comment
There was a problem hiding this comment.
Verdict
Three tightly-scoped, well-tested bug fixes (idempotent DB-NotFound delete, dev JWT-override warning, roles_claim format validation) that all follow HyperShell conventions and introduce no blockers. My only substantive concerns are that the PR description overstates the actual net diff and one adjacent defense-in-depth gap; the code itself is safe to merge.
Amber Analysis
The net diff against main is 6 files: the delete-path idempotency fix, the dev JWT warning, and roles_claim format validation, each with tests. The changes are correct and idiomatic. Notably, the branch handles the classic optional→required trap well: the final commit (2fb845c) deliberately reverses P3-1 from "require roles_claim" to "default a blank roles_claim to the gateway's own default," so no pre-existing OIDC config is broken on the reconcile path. The new TestValidateOIDCConfig is additive and no pre-existing test assertion was flipped.
Findings
[Minor] PR description overstates scope — P0-1/P0-2 console ingress is not in this diff (Docs consistency, High confidence)
The body lists console-ingress mode branching (e309909) as a change of this PR, but the net diff against main contains no console/manifests changes — that work is already on main (it was merged and then merged back into this branch). Reviewers and the eventual merger should not expect console behavior changes here. Please trim the body to match the actual diff.
[Minor] PR description contradicts the shipped roles_claim behavior (Docs / Test Diff Scrutiny, High confidence)
The body says "roles_claim is now required ... whenever admin_role/user_role are set." The shipped code (validation.go:184) does the opposite and correctly so: a blank roles_claim is delegated to the gateway default and only a non-empty value is format-validated. The description should be updated to describe the backward-safe behavior that actually merged, otherwise a future reader may "fix" it back into a breaking required-field.
[Minor] Defense-in-depth: sibling OIDC fields are still interpolated into TOML unvalidated (Security, Medium confidence)
roles_claim is now format-validated (claimPathRegex) before it is written into the gateway TOML, which closes a config-injection surface for that field. The adjacent fields rendered in manifests.go — issuer, audience, admin_role, user_role, scopes_claim — are still fmt.Sprintf-ed into quoted TOML strings with no format validation, so a value containing a quote/newline could still break or inject into the rendered config. Out of this PR's strict scope, but worth a follow-up to apply the same validation posture to the siblings.
What I verified
- Delete idempotency is correctly scoped.
resolveDatabaseConfigwraps the gRPC error with%w;status.Code(dbErr)(grpc v1.83) unwraps viaerrors.As, so thecodes.NotFoundcheck matches the wrapped status. The companionTestGatewayDeleteDatabaseResolveErrorStillFailsproves a non-NotFound (Unavailable) error still fails the reconcile so transient conditions are retried. Good. - Dev JWT warning fires only when
--enable-jwt=truewas explicitly requested, names the flag, and points todevelopment_oidc; the override itself is unchanged and still covered byTestDevOverrideConfigDisablesJWT. - No secrets logged; the delete-path log uses only the resource IDs.
Cross-PR coordination
No material cross-PR coordination issue requires maintainer action.
Findings Summary (ordered by severity, highest first):
- [Minor] PR description claims console-ingress (P0-1/P0-2) changes that are not in the net diff - Docs consistency (body)
- [Minor] PR description says roles_claim is "required" but shipped code makes it optional/format-only - Docs / Test Diff Scrutiny (L184)
- [Minor] Sibling OIDC fields (issuer/audience/admin_role/user_role/scopes_claim) rendered into TOML without format validation - Security (defense-in-depth) (manifests.go)
Convention Checklist:
| Convention | Result |
|---|---|
No panic() in production code |
Pass |
Errors wrapped with fmt.Errorf context |
Pass |
gRPC/IsNotFound handled for 404 scenarios |
Pass |
| No secrets in logs or responses | Pass |
| Input validated (roles_claim claim path) | Pass |
| Reconcile idempotency (delete path) | Pass |
| Context propagation (stream ctx) | Pass |
| Test Diff Scrutiny (no flipped assertions; additive tests) | Pass |
| Conventional commit messages | Pass |
| // its reconciliation with no migration. A claim the realm never emits still | ||
| // can't be caught here without the token (P3-1); this only rejects a | ||
| // syntactically invalid claim path. | ||
| if oidc.RolesClaim != "" && !claimPathRegex.MatchString(oidc.RolesClaim) { |
There was a problem hiding this comment.
Correct call to make this optional rather than required — a blank roles_claim delegates to the gateway default, so pre-existing BYO OIDC configs keep reconciling. This closes the optional→required trap.
Two notes:
- The PR body still says
roles_claimis "now required whenever admin_role/user_role are set," which is the opposite of what shipped here. Please update the description so a future reader doesn't reintroduce the breaking required-field. - This regex now guards
roles_claimagainst TOML injection when it is interpolated inmanifests.go. The sibling OIDC fields there (issuer,audience,admin_role,user_role,scopes_claim) are still written into quoted TOML strings without format validation — consider a follow-up to apply the same posture.
| // deleteErrs instead would fail the whole delete-reconcile, which the | ||
| // watcher retries every 30s -- forever, because the ManagedDatabase | ||
| // never comes back. Any other error still fails so it is retried. | ||
| if status.Code(dbErr) == codes.NotFound { |
There was a problem hiding this comment.
Verified this works with the wrapped error: resolveDatabaseConfig returns fmt.Errorf("resolve ManagedDatabase %s: %w", ...), and status.Code (grpc v1.83) unwraps via errors.As, so the codes.NotFound branch matches. The companion test also confirms a non-NotFound (Unavailable) error still fails so the watcher retries. Nice, idempotent and correctly scoped.


Summary
GitOps-hardening fixes surfaced while rolling Vault-backed OIDC + JWT/RBAC and the per-gateway Keycloak provisioner on route-mode ROSA clusters. Each item was validated against the code before fixing; every fix ships with tests.
Changes
P0-1 / P0-2 — Console ingress ignores
GATEWAY_INGRESS_MODE(e309909)Console ingress now branches on the mode like the gateway path does:
route→ OpenShiftRoute(edge, redirect),gateway-api→HTTPRoutewith a fail-fast guard on emptyGATEWAY_API_GATEWAY_NAME,none→ skip. Previously it unconditionally emitted anHTTPRoutewithparentRef.name: "", which the API rejected forever in a 30s self-heal loop, leaving the console unservable on route-mode clusters.P1-1 —
ManagedDatabase NotFoundduring delete loops forever (a01a522)Gateway delete-reconcile now treats a
NotFoundfromresolveDatabaseConfigas already-cleaned and finalizes the gateway. Any other resolve error still fails so transient conditions are retried. Previously the deleted-database case failed the whole reconcile, retried every 30s indefinitely.P2-3 —
API_ENV=developmentsilently overrode--enable-jwt=true(31c977c)The dev environment still force-disables JWT, but now logs a loud WARN naming the overridden flag and pointing to
API_ENV=development_oidcinstead of silently turning every request into a 401.P3-1 —
roles_claimvalidation gap (5d0cd7c)roles_claimis now required (and format-validated as a dot-separated JWT claim path) wheneveradmin_role/user_roleare set, catching dead-on-arrival gateways at config-validation time instead of at first login.RBAC identity derivation (
5263139) — carried on this branch.Testing
go test ./...incomponents/control-planeand the relevantcomponents/api-serverpackages — all green.go build+go vetclean on both components.ValidateOIDCConfigtable (10 subtests).Notes / out of scope
hsctldelete verb) is already resolved —components/cliis built ashsctl(Makefile:167) and already ships a fulldeletetree includinghsctl delete gateway <id>. No change needed.🤖 Generated with Claude Code