Skip to content

feat(webhook): Pass keycloak_url + keycloak_realm to jwt-validation in synthesizePipeline#336

Merged
pdettori merged 2 commits into
rossoctl:mainfrom
huang195:feat/jwt-validation-keycloak-url
May 8, 2026
Merged

feat(webhook): Pass keycloak_url + keycloak_realm to jwt-validation in synthesizePipeline#336
pdettori merged 2 commits into
rossoctl:mainfrom
huang195:feat/jwt-validation-keycloak-url

Conversation

@huang195

@huang195 huang195 commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to rossoctl/cortex#383 on the operator side. When a namespace's authbridge-runtime-config ConfigMap has no pipeline: of its own, synthesizePipeline builds one from NamespaceConfig. Today it passes keycloak_url + keycloak_realm only to the outbound token-exchange plugin; jwt-validation gets just issuer.

kagenti-extensions#383 extends jwt-validation to accept those same two fields and derive jwks_url from the INTERNAL Keycloak URL (required so the sidecar can actually reach the JWKS endpoint from inside the cluster — issuer is the public hostname, typically unreachable from inside the pod). This PR updates the operator to hand those fields to jwt-validation so synthesized configs work in split-horizon deployments.

Why we hit this

Observed in rossoctl/rossoctl#1507 CI (weather-tool-advanced envoy-proxy stderr, with base chart-provided runtime-config — but synthesizePipeline is the fallback path for namespaces that bypass the chart entirely):

msg="JWT validation failed" error="fetching JWKS:
  Get \"http://keycloak.localtest.me:8080/realms/kagenti/protocol/openid-connect/certs\":
  dial tcp [::1]:8080: connect: connection refused"

The synthesized config emitted only issuer. jwt-validation fell back to issuer-derivation, got the public hostname, resolved it to 127.0.0.1 via localtest.me wildcard, and hit connection refused.

Pre-PR-#378 the binary ran config.Resolve()deriveJWKSURL() which derived inbound.jwks_url from outbound.token_url via a cross-plugin pass. Per-plugin decode gone that pass away; this PR pays that forward by handing the same hints to both plugins.

Scope

  • pod_mutator.go: add two lines to synthesizePipeline — pass KeycloakURL / KeycloakRealm into jwtCfg alongside the existing pass into tokenCfg.
  • pod_mutator_test.go: extend TestEnsurePerAgentConfigMap_EmptyBaseYAML_FallbackFromNsConfig to assert jwt-validation now receives both.

Doc comment on synthesizePipeline updated to explain the split-horizon expectation and link to extensions#383.

Compatibility

Requires the authbridge sidecar binary to accept the new jwt-validation fields (decoded with DisallowUnknownFields). The chart in rossoctl/rossoctl#1507 bumps both pins together — the operator PR and the chart PR gate each other's merge.

Test plan

  • go test ./internal/webhook/injector/... — all green (9 ensure-configmap tests, including the updated synthesize assertions)
  • E2E green via the chart PR with matched pins

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

…n synthesizePipeline

## Why

When a namespace's `authbridge-runtime-config` ConfigMap has no
`pipeline:` of its own, the webhook synthesizes one from the
namespace's `authbridge-config` env-var contract (NamespaceConfig).
The synthesis passed `keycloak_url` + `keycloak_realm` only to the
outbound token-exchange plugin. jwt-validation got just `issuer`.

kagenti-extensions#383 extends the jwt-validation plugin to accept
these two fields and derive jwks_url from the INTERNAL Keycloak
URL — the sidecar actually reaches the JWKS endpoint from inside
the cluster, and `issuer` is the PUBLIC hostname (required for
`iss`-claim matching but typically unreachable from inside the
pod). Without the keycloak_* hints, jwt-validation falls back to
issuer-derivation and every inbound request fails with
"connection refused" fetching the JWKS → 401.

## Scope

Pure addition to `synthesizePipeline`: the same two fields that
already feed token-exchange now also feed jwt-validation. Both
plugins end up with the same "where is Keycloak internally" hint,
mirroring the pre-PR-rossoctl#378 binary behavior where jwks_url was
derived from outbound.token_url via a cross-plugin pass.

Test: extend `TestEnsurePerAgentConfigMap_EmptyBaseYAML_Fallback
FromNsConfig` to assert jwt-validation now receives keycloak_url
and keycloak_realm.

## Compatibility

Requires kagenti-extensions ≥ the tag that includes rossoctl#383 — older
authbridge binaries reject the new fields at DisallowUnknownFields
decode. The chart in rossoctl/rossoctl#1507 bumps both pins together.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
…ture

PR rossoctl#321 moved keycloak-admin-secret reads to the OperatorNamespace
(security hardening: agent namespaces should not hold realm admin
credentials). The table-driven tests and the happy-path test were
updated, but TestClientRegistration_EndToEnd_CredentialsAuthenticate
was missed — it still seeds the secret in the agent namespace and
constructs the reconciler with an empty OperatorNamespace.

Result: post-merge CI on main and every downstream PR fails the
same test with `result={false 30s <nil>}` — the reconciler can't
find the admin secret in the (empty) operator namespace so it
requeues after 30s, tripping the `res != ctrl.Result{}` assertion.

Fix: seed the secret under clientRegistrationTestOperatorNS and set
OperatorNamespace on the reconciler, matching the adjacent happy-
path test at line 355.

Not my PR's scope but it blocks CI and is a one-line structural
fix symmetric with the sibling tests — bundling rather than
waiting on a separate cleanup PR.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, focused change. synthesizePipeline now passes keycloak_url + keycloak_realm to jwt-validation with the same guards as token-exchange — both plugins get the internal Keycloak hint needed for split-horizon deployments.

CI: all green except E2E which depends on the paired chart bump in kagenti#1507 (expected).

The bundled test fix for #321's admin-secret namespace mismatch is appreciated — symmetric with the sibling tests and unblocks CI.

@pdettori

pdettori commented May 8, 2026

Copy link
Copy Markdown
Member

E2E Failure Analysis — False Negative (not caused by this PR)

The E2E failure (should route HTTP traffic through the injected envoy proxy + should auto-create AgentCard with Synced=True) is a cross-repo timing issue, not a regression from this PR's code.

Root Cause

The envoy-proxy container crashloops with:

yaml: unmarshal errors:
  line 5: cannot unmarshal !!map into string
  line 10: cannot unmarshal !!map into string

This happens because the E2E pulled authbridge-envoy:latest after the v0.5.0-rc.1 tag was published (at 23:21Z), which includes extensions #378's config refactoring. The rc.1 binary changed how plugin config fields are decoded — it now expects string-typed config, but the operator generates nested YAML maps.

Timeline

Time (UTC) Event
19:57 Last Build-Publish on extensions main → pushes :latest
22:48 Main CI E2E passes (pulls the old :latest)
23:21 v0.5.0-rc.1 Build-Publish → overwrites :latest with rc.1 code
23:47 This PR's E2E pulls :latest → gets the rc.1 image → 💥
01:27 Extensions #383 merges to main → new :latest fixes the parsing

Evidence

  • The PR only changes synthesizePipeline, but the E2E uses the legacy ValueFrom config path (confirmed by controller logs: "Using legacy ValueFrom config path"). synthesizePipeline is never called.
  • Main's E2E passed at 22:48Z with the same operator code (commit 4a7d291), before the rc.1 image was published.
  • Unit Tests + Integration Tests pass on this PR.

Fix

Re-running CI should resolve this — authbridge-envoy:latest now includes both #378 and #383 (merged at 01:27Z), which properly handles nested-map config fields.

@pdettori
pdettori merged commit 469cf71 into rossoctl:main May 8, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants