Skip to content

feat(plugins): Per-plugin config + /readyz aggregation + /stats merge#378

Merged
huang195 merged 9 commits into
rossoctl:mainfrom
huang195:feat/plugin-config-scaffolding
May 7, 2026
Merged

feat(plugins): Per-plugin config + /readyz aggregation + /stats merge#378
huang195 merged 9 commits into
rossoctl:mainfrom
huang195:feat/plugin-config-scaffolding

Conversation

@huang195

@huang195 huang195 commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

Full per-plugin configuration cutover for AuthBridge. Plugins now own
their configuration end-to-end: they decode their subtree of the
runtime YAML, apply defaults, validate, and build their own internal
auth.Auth from it. The pre-migration top-level inbound: /
outbound: / identity: / bypass: / routes: blocks are gone.
Everything a plugin needs lives under
pipeline.*.plugins[].config.

Depends on #377 (structured rejections + plugin lifecycle hooks),
which has been merged into main. This branch is now rebased
directly on main.

Interfaces and YAML shape

  • pipeline.Configurable — optional; plugins that need configuration
    implement it. Configure(raw json.RawMessage) error decodes with
    DisallowUnknownFields, applies its own defaults, validates, then
    builds internal state.
  • pipeline.Initializer / pipeline.Shutdowner — already existed
    from feat(pipeline): Structured rejections + lifecycle hooks + plugin spec docs #377. Now used by jwt-validation and token-exchange to run
    background credential-file pollers.
  • pipeline.Readier — optional; plugins that have deferred state
    report ready/not-ready. Pipeline.Ready() / NotReadyPlugin() aggregate.
  • plugins.StatsSource — optional; plugins that maintain counters
    expose them for /stats aggregation.
  • config.PluginEntry{name, id, config: RawMessage}. YAML
    accepts both bare strings (- a2a-parser) and the full form
    (- {name: jwt-validation, config: {...}}). Explicit
    config: null is normalized to nil.
  • plugins.Build — type-asserts Configurable, calls Configure with
    the entry's raw bytes, errors if a non-Configurable plugin got a
    config block.
  • authlib/plugins/CONVENTIONS.md — the four-step pattern
    (decode → applyDefaults → validate → construct) for plugin
    authors, plus a note on the plugin-config / top-level strictness
    asymmetry.

Plugin migrations

jwt-validation.Configure decodes {issuer, jwks_url, audience, audience_file, audience_mode, bypass_paths}, derives
jwks_url from issuer when omitted, builds its internal
auth.Auth. Init spawns a background poll for audience_file when
the file isn't readable at Configure time; the poll runs on a
process-lifetime context and is cancelled in Shutdown. Ready()
reports when the audience has loaded.

token-exchange.Configure decodes {token_url, keycloak_url, keycloak_realm, default_policy, no_token_policy, identity: {...}, routes: {...}, audience_from_host}, derives token_url from
keycloak_url + keycloak_realm, builds its internal auth.Auth
(exchanger / cache / router / client-auth). Init handles async
credential-file loading via auth.UpdateIdentity. cfg stays
immutable after Configure returns; the background poller uses
goroutine-local variables. Ready() tracks credential arrival via an
atomic.Bool.

Kagenti-convention defaults

Most deployments don't need to spell these out:

Plugin Field Default
jwt-validation jwks_url <issuer>/protocol/openid-connect/certs
jwt-validation audience_file /shared/client-id.txt
jwt-validation bypass_paths /.well-known/*, /healthz, /readyz, /livez
jwt-validation audience_mode static
token-exchange token_url derived from keycloak_url + keycloak_realm
token-exchange default_policy passthrough
token-exchange no_token_policy deny (WARN if defaulted)
token-exchange routes.file /etc/authproxy/routes.yaml
token-exchange identity.client_id_file /shared/client-id.txt (both types)
token-exchange identity.client_secret_file /shared/client-secret.txt (client-secret)
token-exchange identity.jwt_svid_path /opt/jwt_svid.token (spiffe)

A minimum-viable config fits in ~15 lines. File-path defaults only
kick in when the matching inline value is also empty, so inline
credentials suppress file-read defaults. identity.type stays
required (spiffe vs. client-secret is deployment-level).

Behavior changes worth reading

  • Schema cutover is breaking. Old-shape YAML (top-level
    inbound: / outbound: / identity: / bypass: / routes:)
    silently yields empty pipelines under yaml.v3's default
    forgiveness. config.Validate now rejects empty pipelines with
    an error naming the offending section and the likely cause
    (un-migrated schema).
  • no_token_policy default changed. The pre-PR default was
    mode-specific (envoy-sidecar: client-credentials, waypoint: allow,
    proxy-sidecar: deny); the new uniform default is deny. Operators
    who relied on either old default get a boot-time WARN naming the
    three old defaults and how to pin behavior with one line of YAML.
  • Credential polling is indefinite. The old 60s bounded poll
    became "poll forever" under per-plugin Init goroutines. A WARN
    fires every ~60s while a file is still missing so operators notice
    wrong paths / missing volume mounts.
  • /readyz now gates on plugin readiness. Previously unconditional
    OK after Start; now ANDs Ready() across plugins, returns 503 +
    the not-ready plugin name while any plugin is still waiting. Kubelet
    holds traffic off the pod until all plugins have credentials.
  • /stats now works. Previously returned empty counters; now
    aggregates per-plugin counters at each request.

Concurrency and lifetime

  • bgCancel on both plugins is held in atomic.Pointer[context.CancelFunc]
    — safe even if Shutdown is called from a different goroutine than
    Init. Shutdown is idempotent via atomic Swap(nil).
  • Pipeline.Start on Init failure unwinds successful Inits via
    reverse-order Shutdown before returning — closes the
    orphaned-goroutine window for embedded / multi-tenant callers.
  • Plugins spawn background pollers with context.Background()-derived
    contexts, not Init's 60s budget, so a slow client-registration
    during pod boot doesn't orphan the plugin after Init's deadline.
  • token-exchange.cfg is immutable after Configure returns;
    pollCredentials reads locals and feeds values through
    auth.UpdateIdentity.

Race detector clean on auth, pipeline, plugins, observe packages.

Schema changes (breaking)

  • Removed from config.Config: Inbound, Outbound, Identity,
    Bypass, Routes — their content moved into per-plugin config.
  • Removed from config/resolve.go: Resolve, deriveKeycloakURLs,
    deriveJWKSURL, resolveRouter, ResolveCredentialFiles,
    ResolveClientAuth. Kept ReadCredentialFile /
    WaitForCredentialFile as shared helpers used by both plugins.
  • Registry factory: func() pipeline.Plugin (no more *auth.Auth).
  • main.go: no global auth.Auth, no credential-load goroutine, no
    UpdateIdentity hotswap. Each plugin owns its own auth instance.

Test utilities

authbridge/authlib/plugins/plugintesting/ — stub plugin adapters
around a pre-built auth.Auth for listener tests. Lives in a
dedicated sub-package so production binaries can't import it; the
stubs mimic the real plugins' OnRequest without touching plugin
internals.

Demo manifests

  • authbridge/authproxy/authbridge-combined.yaml — minimum-viable
    form using defaults.
  • authbridge/demos/mcp-parser/README.md — minimum-viable form.
  • authbridge/cmd/authbridge/README.md — both minimum and full
    forms, side-by-side, with a defaults table.
  • authbridge/CLAUDE.md — updated "Configuration loading" bullets
    and the credential-wait gotcha.
  • authbridge/authlib/README.md — package table + Usage example
    updated to go through plugins.Build rather than config.Resolve.

Test plan

  • go build ./... and go vet ./... clean on both authlib and
    cmd/authbridge modules.
  • go test ./... passes.
  • go test -race ./auth/... ./pipeline/... ./plugins/... ./observe/... clean.
  • TestAuthbridgeCombinedYAML_Loads parses the shipped default
    config with env vars set and asserts both pipelines Build + Validate.
  • Configure validation matrix for both plugins (missing
    required, unknown fields, defaults, file paths, inline suppression).
  • pipeline.Configurable contract coverage.
  • Empty-pipeline rejection with error-message contract.
  • PluginEntry YAML: bare strings, full form, null config.
  • Pipeline.Start unwinds successful Inits on failure.
  • Plugin Ready() matrix: sync-loaded, pending, per-host, inline
    credentials, pending credentials.
  • auth.MergeStats correctness + non-destructive + nil/empty safe.
  • plugins.CollectStats skips non-StatsSource plugins, nil-safe.
  • Credential-file heartbeat branch reachable without panic.
  • Local kind-cluster smoke test — deploy the weather-agent,
    verify inbound validation + outbound passthrough work end-to-end.
    (Pre-merge gate; not yet run.)

Draft

Still draft. #377 is merged and the branch is rebased on main.
Remaining gate before ready-for-review: local kind-cluster smoke
test (listed in Test plan above).

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

@huang195
huang195 force-pushed the feat/plugin-config-scaffolding branch from 7a2b810 to 7e23f2d Compare May 7, 2026 03:14
@huang195 huang195 changed the title feat(plugins): Add Configurable interface and PluginEntry config shape feat(plugins): Per-plugin configuration cutover May 7, 2026
@huang195 huang195 changed the title feat(plugins): Per-plugin configuration cutover feat(plugins): Per-plugin config + /readyz aggregation + /stats merge May 7, 2026
huang195 added 9 commits May 7, 2026 09:27
Scaffolding for per-plugin local configuration. No plugin implements
Configurable yet — this change only puts the interface, the YAML shape,
and the Build integration in place so subsequent PRs can migrate
jwt-validation and token-exchange one at a time without schema churn.

What's in this PR:

- pipeline.Configurable: optional interface a plugin implements when it
  needs its own config. Plugins receive a json.RawMessage and decode it
  themselves (DisallowUnknownFields, defaults, validation). Parsers and
  other config-free plugins simply don't implement it.

- config.PluginEntry: the richer plugin entry shape. YAML accepts both
  the bare-name form (backward compatible) and the full form with
  name/id/config. Existing pipeline configs parse unchanged because
  UnmarshalYAML handles the ScalarNode case.

- plugins.Build: iterates []config.PluginEntry, type-asserts each
  plugin against Configurable, calls Configure with the entry's raw
  config bytes. A config: block on a plugin that doesn't accept
  configuration is a startup error — catches typos and stale entries
  at boot instead of silently ignoring them.

- CONVENTIONS.md: documents the four-step Configure pattern (decode →
  applyDefaults → validate → construct) and related guidance for new
  plugin authors.

The auth.Auth instance is still threaded through PluginFactory. Each
plugin will drop that dependency when it migrates to Configure() in
the follow-up PRs.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Completes the per-plugin configuration cutover. Plugins now decode
their own config from a json.RawMessage subtree (DisallowUnknownFields
-> applyDefaults -> validate -> construct), build their own auth.Auth
internally, and handle async credential-file loading through Init
goroutines. main.go no longer constructs a process-wide auth.Auth;
that responsibility moved into each plugin's Configure.

Schema changes (breaking — top-level legacy blocks are gone):
  inbound:   { issuer, jwks_url }                   -> pipeline.inbound.plugins[jwt-validation].config
  outbound:  { token_url, keycloak_url, ... }       -> pipeline.outbound.plugins[token-exchange].config
  identity:  { type, client_id, ... }               -> token-exchange's identity sub-block
  bypass:    { inbound_paths }                      -> jwt-validation's bypass_paths
  routes:    { file, rules }                        -> token-exchange's routes sub-block

jwt-validation's new config:
  issuer, jwks_url (optional — derived from issuer), audience,
  audience_file, audience_mode (static|per-host), bypass_paths.

token-exchange's new config:
  token_url, keycloak_url + keycloak_realm (optional — derives
  token_url), default_policy, no_token_policy, identity {type,
  client_id, client_secret, client_id_file, client_secret_file,
  jwt_svid_path}, routes {file, rules}, audience_from_host.

Async credentials: when identity.client_id_file or client_secret_file
is set and not yet readable at Configure time, the plugin's Init
starts a background poll and calls auth.UpdateIdentity when the file
appears. jwt-validation's audience_file follows the same pattern.

config package shrinks: Resolve(), deriveKeycloakURLs,
deriveJWKSURL, resolveRouter, and the IdentityConfig / InboundConfig
/ OutboundConfig / BypassConfig / RoutesConfig types are all gone —
what each one computed is now the responsibility of the plugin whose
config it belonged to. Kept ReadCredentialFile /
WaitForCredentialFile as shared helpers because both plugins
frequently read the same file (/shared/client-id.txt).

Registry factory signature changes:
  func(a *auth.Auth) pipeline.Plugin -> func() pipeline.Plugin

plugins.Build no longer accepts *auth.Auth; each plugin builds its
own. BuildForTest / NewJWTValidationForTest / NewTokenExchangeForTest
are added to testutil.go so listener tests in sibling packages can
still inject pre-built auth.Auth mocks.

main.go: remove auth.New, handler.UpdateIdentity, the background
credential-load goroutine, and the credential-ready gate on /readyz.
Plugins own all of this now. /readyz returns OK unconditionally after
pipeline Start succeeds; OnRequest returns 503 if traffic arrives at
a plugin whose credentials haven't landed yet.

StatServer keeps its shape for /config (renders the runtime YAML
verbatim) and /stats (now backed by a fresh auth.NewStats — aggregate
per-plugin stats collection is a follow-up).

Demo YAMLs updated:
  - authbridge/authproxy/authbridge-combined.yaml
  - authbridge/demos/mcp-parser/README.md (inline YAML example)

Out of scope, tracked for follow-up:
  - Aggregate plugin stats on /stats.
  - Plugin-level Ready() reporting for /readyz.
  - kagenti-operator webhook template rewrite (separate repo).
  - kagenti Helm chart image-tag bump (separate repo).

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
- cmd/authbridge/README.md: rewrite the Configuration section for the
  per-plugin YAML shape, including worked examples for envoy-sidecar,
  waypoint, and proxy-sidecar modes, plus bare-name entry, URL
  derivation, and credential-file waiting behavior.
- authbridge/CLAUDE.md: update the "Configuration loading" bullets
  and the library-package list to describe the new flow (plugins own
  their config and build their own auth.Auth; env vars from the
  operator ConfigMap are expanded into per-plugin config blocks).
- authlib/README.md: update the package table (auth composition is
  now plugin-internal) and rewrite the Usage example to go through
  plugins.Build rather than config.Resolve + auth.New.

Env var contract (KEYCLOAK_URL, TOKEN_URL, ISSUER, etc.) is
unchanged — these are still populated by the operator's
authbridge-config ConfigMap and expanded by authbridge-combined.yaml
into the appropriate plugin config subtree.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Lets operators omit boilerplate values that, in Kagenti, always land
on the same paths. A minimum-viable config now fits in ~15 lines of
YAML instead of 30+.

jwt-validation defaults:
  audience_file = /shared/client-id.txt   (when audience & audience_file both empty)
  bypass_paths  = .well-known/*, /healthz, /readyz, /livez

token-exchange defaults (per identity.type):
  routes.file                 = /etc/authproxy/routes.yaml
  identity.client_id_file     = /shared/client-id.txt
  identity.client_secret_file = /shared/client-secret.txt   (client-secret)
  identity.jwt_svid_path      = /opt/jwt_svid.token         (spiffe)

Every default kicks in only when the matching inline value is also
empty, so operators who write inline credentials or paths never get
surprise file reads. Inline Audience suppresses the audience_file
default; inline ClientID/ClientSecret suppresses their file defaults.

Identity.type is still required — the choice between SPIFFE and
client-secret is deployment-level. Identity file paths are not.

Tests:
  - TestJWTValidation_Configure_DefaultAudienceFile
  - TestJWTValidation_Configure_DefaultBypassPaths
  - TestJWTValidation_Configure_InlineAudienceSuppressesFileDefault
  - TestTokenExchange_Configure_DefaultIdentityPaths_SPIFFE
  - TestTokenExchange_Configure_DefaultIdentityPaths_ClientSecret
  - TestTokenExchange_Configure_InlineIdentitySuppressesFileDefaults
  - TestTokenExchange_Configure_DefaultRoutesFile

Removed obsolete validation cases from
TestTokenExchange_Configure_IdentityValidation that are no longer
reachable via static validation (e.g. "spiffe no path" —
JWTSVIDPath now auto-defaults to /opt/jwt_svid.token).

Updated demo YAMLs:
  authproxy/authbridge-combined.yaml  — minimum-viable form
  demos/mcp-parser/README.md          — minimum-viable form
  cmd/authbridge/README.md            — both minimum and full forms,
                                        with a defaults table

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Must-fix:
- rossoctl#1+rossoctl#7: Plugins spawned credential watchers with initCtx (60s), so a
  slow client-registration orphaned the plugin after Start's deadline
  and OnRequest returned 503 for the pod's lifetime. Watchers now run
  on process-lifetime contexts created in Init via context.Background()
  + context.WithCancel; the cancel func is stored on the plugin and
  fired in Shutdown. Both JWTValidation and TokenExchange now
  implement pipeline.Shutdowner. Removed the orphaned
  context.WithCancel at main.go:89 that looked like it was meant to
  be that context.

- rossoctl#2: testutil.go moved out of the plugins package into a dedicated
  sub-package authlib/plugins/plugintesting/ so it can't be imported
  by the production binary. Rewrote the helpers as stub plugin
  adapters that mimic jwt-validation / token-exchange OnRequest but
  don't touch plugin internals — listener tests inject an
  *auth.Auth via plugintesting.NewJWTValidation / NewTokenExchange,
  wire through plugintesting.BuildPipeline, and never see the real
  Configure path. All four listener tests updated.

- rossoctl#11: Removed identity sub-validation that applyDefaults made
  unreachable; class-of-error compensated by logging a boot-time
  WARN when Configure's best-effort ReadCredentialFile fails (see
  also rossoctl#12).

Suggestions:
- rossoctl#3: pollCredentials no longer mutates p.cfg. Reads credential
  values into locals and feeds them through auth.UpdateIdentity. cfg
  is now immutable after Configure returns. Added a
  buildClientAuthFrom helper that takes explicit args so the
  goroutine doesn't read p.cfg either.

- rossoctl#4: Rewrote the NoTokenPolicy doc comment to state the current
  behavior ("deny in all modes; operators who need allow or
  client-credentials must set it explicitly") instead of referring
  to the removed config.NoTokenPolicyForMode.

- rossoctl#5: Deleted unused DefaultInboundPlugins/DefaultOutboundPlugins
  and the whole defaults.go file. Fixed the stale PipelineConfig
  docstring in config.go that said "defaults kick in when
  pipeline section is omitted" — they don't.

- rossoctl#6: PluginEntry.UnmarshalYAML normalizes explicit `config: null`
  (and any other !!null-tagged scalar under `config:`) to a nil
  RawMessage, so Build's "plugin does not accept configuration"
  gate doesn't fire spuriously on the four bytes "null".

- rossoctl#12: Deleted unreachable validate() branches: static-mode's
  audience emptiness check, spiffe's missing-path check,
  client-secret's missing-id / missing-secret checks. applyDefaults
  fills the matching field in every case; the branches were dead
  code that obscured what the plugin actually validated.

Nits:
- rossoctl#8: Renamed TestPluginEntry_IDDefaultsToName →
  TestPluginEntry_IDOmittedStaysEmpty.

- rossoctl#9: TestBuild_ConfigForNonConfigurablePlugin now asserts the error
  text contains "does not accept configuration" — operator-facing
  contract.

- rossoctl#10: Renamed TestConfigurable_ErrorAborts →
  TestConfigurable_ErrorPropagates and noted in the comment that
  Build-level abort coverage lives in plugins_test.go.

- rossoctl#13: Documented on jwtValidationConfig.AudienceFile that empty-
  string is treated as "unset" and triggers the default. Operators
  who want no file backing must supply an explicit Audience.

- rossoctl#14: Added TestAuthbridgeCombinedYAML_Loads — parses the in-repo
  authbridge-combined.yaml with env vars set, asserts both inbound
  and outbound pipelines Build cleanly. A future rename of any
  default constant that the YAML relies on breaks this test in CI
  rather than silently shipping a broken image.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Two gaps found during comprehensive PR review.

Critical: empty pipelines silently produced an open proxy.

The old top-level schema (inbound:, outbound:, identity:, bypass:,
routes:) carried both the plugin composition (implicitly) and the
per-plugin settings. When it was removed, yaml.Unmarshal in Load()
retained its default behavior of silently dropping unknown keys. An
operator upgrading their image without migrating their config.yaml
would produce a Config with an empty Pipeline section — Validate()
passed it, plugins.Build returned an empty pipeline with zero plugins,
listeners started, and every inbound request passed through without
JWT validation, every outbound request passed through without token
exchange. No warning, no error — just a silently open proxy.

Fix: Validate() now errors if either Pipeline.Inbound.Plugins or
Pipeline.Outbound.Plugins is empty. The error names the offending
section and points at the new schema, including the likely cause
(old-shape YAML not migrated) for fast diagnosis.

Minor: Init's bgCancel overwrite on double-call.

The Pipeline.Start contract says Init runs exactly once per process,
but a defensive guard costs nothing. JWTValidation and TokenExchange
now early-return from Init when bgCancel is already set, preventing
a leaked goroutine if the contract is ever bent.

Tests:
- TestValidate_EmptyPipelineRejected — asserts the error surface
  and the error message contract (operator-facing).
- TestValidate_EmptyOutboundPipelineRejected — same for outbound.
- TestPluginEntry_NullConfigIsTreatedAsUnset — previously only
  asserted implicitly via the TestBuild_ConfigForNonConfigurablePlugin
  path; now explicit so the yaml.Tag == "!!null" handling in
  PluginEntry.UnmarshalYAML has direct coverage.
- TestValidate_ValidConfigs updated to populate a pipeline
  (previously passed because empty was accepted).
- TestAuthbridgeCombinedYAML_Loads now also calls Validate to prove
  the shipped default YAML doesn't trip the new empty-pipeline gate.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Should-fix items from the comprehensive PR evaluation.

rossoctl#3 — WARN on NoTokenPolicy implicit default:
The pre-migration default for no_token_policy was mode-dependent
(envoy-sidecar=client-credentials, waypoint=allow, proxy-sidecar=deny).
The migration hardcodes "deny" for all modes. Waypoint operators whose
YAML doesn't explicitly set no_token_policy would silently start
denying outbound requests that previously allowed.

token-exchange.Configure now tracks whether NoTokenPolicy arrived
explicitly, and emits a WARN at boot when the default was used AND
default_policy is "exchange" (the configuration shape where the
behavior change is visible). The WARN names the three valid values
so operators can pin the behavior by adding one line to their YAML.

rossoctl#4 — /stats stub visibility:
The stat server returns empty counters because plugin-level stats
aggregation is a follow-up. Without any signal, operators curling
/stats see zeros and assume the service is broken. main.go now logs
a WARN at the same point where it constructs the stub Stats object,
explaining that /stats is a stub in this release but /config remains
accurate.

rossoctl#5 — Heartbeat during credential-file wait:
WaitForCredentialFile emitted one WARN at Configure time when a file
wasn't readable, then went silent. If the file never appeared
(typo'd path, missing volume mount), operators chased 503s instead
of seeing the wait in logs.

It now emits a WARN every 60s (overridable via the package-scope
heartbeatInterval, used from the test) while still waiting, carrying
the path and total wait duration. Loud enough to notice during an
operational incident, quiet enough to not drown out real signal. The
test asserts the heartbeat branch of the select is reachable without
panics.

rossoctl#7 — Stale CLAUDE.md credential-wait description:
The authbridge/CLAUDE.md gotcha bullet said "Authbridge waits up to
60s for [credential files]" — stale since the switch to indefinite
background polling. Rewrote to match the new behavior and point at
the heartbeat WARN as the diagnostic signal.

rossoctl#8 — Reorder token-exchange Configure to avoid partial cfg mutation:
Under the old ordering, p.cfg = c ran before clientAuth / router
construction. If either failed, p.cfg was already set even though
Configure returned an error. Since Configure errors take the process
down today, this didn't manifest, but it was sloppy state management
and a foot-gun for any future call path that Configure from
non-startup contexts.

Rewrote Configure to build clientAuth, router, and authCfg against
the local `c` before any assignment to p.* fields. buildClientAuth
and buildRouter methods removed; replaced with the pure
buildClientAuthFrom and new buildRouterFrom functions that take
explicit args. pollCredentials already used buildClientAuthFrom, so
the migration is additive.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
rossoctl#1 atomic.Pointer for bgCancel:
Both JWTValidation and TokenExchange now hold bgCancel in an
atomic.Pointer[context.CancelFunc] rather than a plain field. Today's
callers (pipeline.Start / pipeline.Stop) are serialized so there's no
actual race, but a future caller invoking Shutdown from a different
goroutine would have been latent-racing the Init assignment. Shutdown
uses Swap(nil) so it's also idempotent.

-race passes clean on pipeline, plugins, config packages.

rossoctl#2 Widen NoTokenPolicy WARN:
Previously only fired when default_policy was "exchange". That missed
envoy-sidecar deployers relying on the old client-credentials default
for routed hosts. WARN now fires whenever no_token_policy was
defaulted, with the message listing all three pre-PR mode-specific
defaults so operators can see which one their deployment used to get.

rossoctl#3 audience_file WARN mentions defaulted origin:
When audience_file was defaulted to /shared/client-id.txt (Kagenti
convention) and the file isn't readable, the new WARN spells that
out — so non-Kagenti deployers notice their config didn't put the
plugin where it thinks it is. Explicit-path case keeps the shorter
original message.

rossoctl#4 Drop client_id from credential-loaded log:
OAuth spec treats client_id as non-secret, but some operators treat
it as sensitive. The signal (credentials loaded) doesn't need the
identifier.

rossoctl#5 TODO markers for /stats and Ready():
Grep-able TODO(follow-up) comments at the stub sites in main.go with
sketch notes for the future work. These sit next to the existing
behavior comments so anyone reading those surfaces finds the
follow-up plan.

rossoctl#6 Pipeline.Start failure unwind:
On Init error at plugin N, Start now calls Shutdown on plugins
[0..N-1] in reverse order before returning the error. Prevents leaking
goroutines spawned by earlier plugins' Init when a downstream peer
rejects its config at boot. Not a correctness bug for production
(log.Fatalf then exit, kernel cleans up) but matters for any embedded
or multi-tenant caller.

New test TestPipelineStart_UnwindsSuccessfulInitsOnFailure asserts
the reverse-order Shutdown on the subset that Init'd, and that plugins
after the failing one are neither Init'd nor Shutdown'd.

rossoctl#8 Document strictness asymmetry in CONVENTIONS.md:
Plugin config is strict (DisallowUnknownFields); top-level runtime
YAML is forgiving (unknown keys silently dropped). Note explains the
forward-compat rationale and points at config.Validate's
empty-pipeline guard as the mechanism that closes the obvious
stale-schema gap.

rossoctl#7 is procedural (kind smoke test) — not in code.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Two follow-ups previously marked TODO, now landed in this PR.

Ready() for /readyz
-------------------

pipeline.Readier is an optional interface. Plugins that finish
Configure but still have deferred work (credential files not yet
present on disk) return false; the host /readyz handler ANDs Ready()
across inbound and outbound pipelines and returns 503 + the
not-ready plugin's name while any plugin reports not-ready.

JWTValidation.Ready delegates to auth.Auth.Ready (which flips when
Identity.Audience is populated — either synchronously in Configure
or via Init's audience_file poller calling UpdateIdentity). Per-host
mode is always ready because the audience is derived from pctx.Host
per request.

TokenExchange.Ready reads an atomic.Bool set either at Configure
time (inline credentials or sync file read succeeded) or at
pollCredentials success. auth.Auth.Ready keys off Identity.Audience
which token-exchange doesn't use, so per-plugin tracking is cleaner
than bending auth.Auth.

Closes the 503-storm-at-pod-boot window: previously, /readyz was
unconditional OK once Start() succeeded, so the kubelet routed
traffic to the pod before credentials had landed and the first
requests 503'd. Now the kubelet holds traffic off until every
Readier-implementing plugin says go.

New tests:
- pipeline.Pipeline.Ready / NotReadyPlugin — all-ready, one-blocks,
  no-Readiers.
- JWTValidation.Ready — sync-loaded, pending, per-host.
- TokenExchange.Ready — inline creds, pending without creds.

/stats aggregation
------------------

Per-plugin counters collected at each /stats HTTP request. Previously
a fresh auth.NewStats() was wired in at process start so /stats
always returned zeros.

New pieces:
- auth.MergeStats(srcs ...*Stats) *Stats — pure function, takes
  each source's mutex independently (no deadlock), returns a fresh
  Stats. Tolerant of nil sources and empty srcs.
- plugins.StatsSource interface + plugins.CollectStats(p) —
  walks a pipeline, collects non-nil *auth.Stats from every
  StatsSource-implementing plugin.
- observe.StatsProvider = func() *auth.Stats — passed to
  NewStatServer instead of a fixed *Stats; called per /stats
  request for live aggregation.
- main.go wires a closure that merges CollectStats across inbound
  and outbound. Ship warning about /stats being a stub is removed
  (it isn't one anymore).

Kept plugin.go free of an auth import: Statser lives in the plugins
package, where auth was already a dependency. Pipeline package
retains its "no protocol deps" invariant.

New tests:
- auth.MergeStats — sums counters, preserves sources, tolerates
  nil/empty inputs.
- plugins.CollectStats — collects only StatsSource-implementing
  plugins (parsers skipped), nil-pipeline safe.

Race-detector clean on auth, pipeline, plugins, observe packages.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
@huang195
huang195 force-pushed the feat/plugin-config-scaffolding branch from cf48ff4 to 4431f1f Compare May 7, 2026 13:28
@huang195
huang195 marked this pull request as ready for review May 7, 2026 13:45

@cwiklik cwiklik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Major architectural cutover: plugins now own their configuration end-to-end via pipeline.Configurable. The old shared auth.Auth handler + top-level config sections are removed in favor of per-plugin config subtrees under pipeline.*.plugins[].config.

Well-designed safety mechanisms: empty pipeline rejection (prevents "open proxy" failure mode on upgrade), DisallowUnknownFields (catches config typos at startup), Start unwind (cleans up initialized plugins on downstream failure), and re-entrancy guards.

110 test functions covering edge cases. The Ready()/NotReadyPlugin() API gives operators clear kubectl describe diagnostics.

Areas reviewed: Go (architecture, concurrency, interfaces), Security, Docs
Commits: 9, all signed-off | CI: all passing

Two minor suggestions below — neither blocks merge.

switch key.Value {
case "name":
if err := val.Decode(&p.Name); err != nil {
return fmt.Errorf("plugin entry name: %w", err)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: normalizeYAMLMaps traverses the full config subtree on every plugin's yamlNodeToJSON call. Fine for startup-only code, but worth noting if this ever moves to a hot path.

// WARN can distinguish "operator pointed at the wrong path" from
// "defaulted to the Kagenti convention and you might not have
// noticed." applyDefaults fills AudienceFile in when both audience
// and audience_file are empty, erasing the signal.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

suggestion: Init spawns a context.Background() goroutine that polls indefinitely when the file never appears. The WARN log every ~60s is good operational visibility, but consider adding a metric counter (e.g. credential_file_poll_attempts) so operators can alert on it without grepping logs. Could be a follow-up.

@huang195
huang195 merged commit 935251b into rossoctl:main May 7, 2026
17 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Kagenti Issue Prioritization May 7, 2026
@huang195
huang195 deleted the feat/plugin-config-scaffolding branch May 7, 2026 14:53
huang195 added a commit to huang195/kagenti-extensions that referenced this pull request May 7, 2026
## Problem

Pre-PR-rossoctl#378 the binary ran `config.Resolve()` → `deriveJWKSURL()` to
derive `inbound.jwks_url` from `outbound.token_url` (which itself came
from `keycloak_url` + `keycloak_realm` — the INTERNAL service URL).
After the per-plugin cutover, each plugin decodes its config in
isolation, so jwt-validation no longer sees token-exchange's fields.
Its only fallback is to derive jwks_url from its own `issuer` — but
`issuer` is the PUBLIC hostname that appears in token `iss` claims,
which in split-horizon deployments (Kagenti's typical Kind and
OpenShift setups) isn't reachable from inside the sidecar pod.

Observed failure (CI run kagenti/kagenti#25525390807, weather-tool-
advanced envoy-proxy stderr):

    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"

Every inbound request → 401. Authbridge Weather (advanced) E2E broke.

## Fix

Add `keycloak_url` + `keycloak_realm` fields to jwt-validation's
config, symmetric with token-exchange's fields of the same name.
`applyDefaults()` now derives `jwks_url` with this priority:

  1. Explicit jwks_url wins
  2. keycloak_url + keycloak_realm (the internal URL)
  3. Issuer (single-horizon fallback — existing behavior)

Chart/operator/backend callers pass `keycloak_url` + `keycloak_realm`
that they already know; the plugin figures out the JWKS path. No one
has to hand-roll the full URL anymore.

## Scope

- `jwtvalidation.go` — two new fields, updated applyDefaults comment
- `plugins_test.go` — 4 new tests + tightened existing JWKS test to
  assert the exact derived URL (so a future refactor of the priority
  chain can't silently fall through to a 404 path). Partial-keycloak
  fall-through is covered in both directions (url-without-realm and
  realm-without-url) to catch a future AND-check refactor.
- `authbridge-combined.yaml` — pass KEYCLOAK_URL/KEYCLOAK_REALM into
  inbound jwt-validation as well as outbound token-exchange. Header
  comment explains where the env vars land from (envFrom on the
  Deployment, injected by operator/chart).

## Backward compatibility

Purely additive:
- Existing configs with only `issuer` still parse and work
  (single-horizon deployments unaffected).
- Existing configs with explicit `jwks_url` still win
  (custom JWKS proxies unaffected).
- New fields share namespace with token-exchange but decode
  independently — plugin configs remain isolated.

## Follow-ups

Once this lands and a new extensions tag ships:

- kagenti-operator: update `synthesizePipeline` to pass
  keycloak_url + keycloak_realm through to jwt-validation (it already
  has them in NamespaceConfig). Release a new operator alpha tag.
- kagenti chart: drop the temporary explicit `jwks_url` added in
  kagenti#1507 once both new pins are available.

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