feat(pipeline): Config hot-reload via fsnotify + atomic pipeline swap#388
Merged
Conversation
Replaces the closed hardcoded map with RegisterPlugin(name, factory) + RegisteredPlugins(). Follows the stdlib pattern — plugins register themselves from an init() function, the same way database/sql drivers and log/slog handlers do. Why it matters for DX: - External plugin authors can now drop a Go module into their deployment and pull it in with a single side-effect import (`import _ "github.com/acme/custom-plugin"`) without forking kagenti-extensions. - In-tree plugins are located next to their registration, not in a central map that everyone has to edit and rebase. - The "unknown plugin" error from Build now lists every registered plugin so a typo in operator YAML produces an actionable diagnostic instead of a generic not-found. - Tests get clean isolation through UnregisterPlugin (paired with t.Cleanup) — no more package-level map mutation. Safety choices: - Double-registration panics. Silent last-write-wins would let a version conflict (two incompatible copies of the same plugin shipped in the same binary) poison the registry in ways that only surface as mysterious runtime behaviour. - Empty name / nil factory panic. Both are programmer errors; failing at registration is closer to the bug than failing later at Build. - Registry is guarded by sync.RWMutex. init() order across packages isn't serialized under every build mode, and UnregisterPlugin runs concurrently with t.Parallel tests. All five built-in plugins (jwt-validation, token-exchange, a2a-parser, mcp-parser, inference-parser) are migrated to init()-time registration. The central `registry` map literal is replaced by the mutex-guarded dynamic map; Build resolves through factoryFor under RLock. UnregisterPlugin lives in registry_testing.go (not _test.go so other packages' tests can import it) and is documented as a test affordance. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Adds a "Registering a plugin" section covering: - the in-tree pattern (init() next to the plugin, package-level auto-registration) - the out-of-tree pattern (separate Go module, single side-effect import in the authbridge binary to pull it in) - safety rules (panic on double-register, empty name, nil factory) - the Build error message listing registered plugins for typo diagnostics - test-isolation pattern with UnregisterPlugin + t.Cleanup Walks a realistic rate-limiter plugin example end-to-end so a third-party author can write their plugin by following the doc. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Adds Record, Allow, Skip, Observe, Modify, and DenyAndRecord methods
to pipeline.Context so plugins can emit Invocations without repeating
Plugin, Phase, or Path on every call. The framework stamps those
fields onto pctx around each plugin dispatch (SetCurrentPlugin /
ClearCurrentPlugin called from Pipeline.Run / RunResponse); the
helpers read them when building the Invocation.
Before:
appendInvocationInbound(pctx, pipeline.Invocation{
Plugin: "jwt-validation",
Phase: pipeline.InvocationPhaseRequest,
Action: pipeline.ActionAllow,
Reason: "authorized",
Path: pctx.Path,
TokenSubject: result.Claims.Subject,
TokenAudience: result.Claims.Audience,
TokenScopes: result.Claims.Scopes,
})
After (allow branch with token context):
pctx.Record(pipeline.Invocation{
Action: pipeline.ActionAllow,
Reason: "authorized",
TokenSubject: result.Claims.Subject,
TokenAudience: result.Claims.Audience,
TokenScopes: result.Claims.Scopes,
})
After (bypass without token context):
pctx.Skip("path_bypass")
Eliminates three classes of silent bugs plugin authors could hit:
- typo in the Plugin string literal
- wrong Phase (OnRequest code path accidentally writing
InvocationPhaseResponse, or vice versa)
- calling appendInvocationInbound from an outbound plugin (or
vice versa) — direction is now routed automatically from
pctx.Direction
Pipeline.Run and RunResponse now wrap each plugin dispatch with
SetCurrentPlugin / ClearCurrentPlugin. Tests that invoke plugins
directly (bypassing Pipeline.Run) use invokeOnRequest /
invokeOnResponse wrappers in plugins_test.go that do the same
stamping.
Migrates all 5 built-in plugins (jwt-validation, token-exchange,
a2a-parser, mcp-parser, inference-parser) to the new API. Deletes
duplicated appendInvocationInbound / appendInvocationOutbound helpers
from jwtvalidation.go and tokenexchange.go.
Net change across the 5 plugins: ~120 lines of invocation boilerplate
removed. Net add in pipeline/context.go: ~100 lines of helpers +
framework state.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Updates the "Emitting session events" section of CONVENTIONS.md so
plugin authors see the Record / Allow / Skip / Observe / Modify /
DenyAndRecord helpers as the primary API. The earlier
"pctx.Extensions.Invocations = &pipeline.Invocations{...}" pattern is
removed from the docs — plugins shouldn't be touching the Invocations
slot directly now that the helpers exist.
Adds worked examples for:
- one-liner passive actions (Allow / Skip / Observe / Modify)
- DenyAndRecord for rejections with matching-Reason Invocations
- Full Record form for invocations that carry diagnostic fields
like ExpectedIssuer / TokenScopes / RouteHost
Clarifies that the framework fills Plugin / Phase / Path; plugins may
override explicitly for test scenarios but shouldn't in production.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Adds a step-by-step tutorial for writing a new plugin, sibling to CONVENTIONS.md. Covers the full lifecycle with runnable examples: - Minimal plugin (HelloLog) - Recording invocations (one-liner wrappers + Record + DenyAndRecord) - Rejecting a request (Deny helpers) - Adding config (Configurable interface) - Body access capability - Out-of-tree plugins (side-effect import pattern) - Testing (invokeOnRequest helper + UnregisterPlugin for isolation) - Optional interfaces (Initializer / Shutdowner / Readier) - Gotchas CONVENTIONS.md remains the reference doc; GUIDE.md is the tutorial that walks someone through from scratch. Each file links the other. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Three-role model for the plugin developer docs: - GUIDE.md (tutorial) — step-by-step; runnable examples - CONVENTIONS.md (reference) — field-level contract; panic rules - pipeline/README.md — framework architecture Each doc now opens with an Audience + See also triplet so readers land on the right file fast. Scope: pipeline/README.md - Refresh Context, Extensions, SessionEvent sections against the PR rossoctl#385 / rossoctl#386 state (Invocations struct, 5-value action vocab, `phase: "denied"`, pctx.Record helpers, /event suffix, open registry. Add three-doc pairing at top. - Replace the long Writing a native plugin worked example with a pointer to GUIDE.md — that walkthrough now lives there. - Rewrite §11 Versioning changelog with the PR rossoctl#385/rossoctl#386 entries (SessionDenied, unified Invocation contract, Record helpers, open registry, Readier interface). - Reorganize §12 Cross-references by audience. plugins/CONVENTIONS.md - Emitting session events → field-level reference with a pointer callout to GUIDE.md Step 2 for the tutorial. Drops the runnable Allow/Skip/Observe/Modify examples (now only in GUIDE.md). - Registering a plugin → keep the factory-shape snippet and rules/guardrails table; drop the out-of-tree walkthrough (now only in GUIDE.md Step 6). Add pointer callout. plugins/GUIDE.md - Add Audience + See also header. authbridge/CLAUDE.md - Add back-link from the Invocation action vocabulary table to plugins/CONVENTIONS.md as the producer-side contract, so future edits to the vocab land in one place. Net diff: +213 / -219 across 4 files. No code changes. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> EOF ) Signed-off-by: Hai Huang <huang195@gmail.com>
Moves the three plugin-developer-facing docs into a single descriptively-named directory so readers find them without learning the Go package layout: authlib/plugins/GUIDE.md → docs/plugin-tutorial.md authlib/plugins/CONVENTIONS.md → docs/plugin-reference.md authlib/pipeline/README.md → docs/framework-architecture.md File renames are done via `git mv` so `git log --follow` traces history to the pre-move filenames. Leaves thin README.md stubs in authlib/pipeline/ and authlib/plugins/ pointing at the new docs location, so godoc / package-level readers still have a landmark. Scope of reference updates: - Internal cross-refs in the three moved docs rewired to the new flat `./filename.md` form. - authbridge/CLAUDE.md: 5 pointers updated. - authbridge/cmd/authbridge/README.md: config-pattern link. - authbridge/authlib/README.md: package table + usage prose. - authbridge/authproxy/authbridge-combined.yaml: header comment. - Go source comments: session.go, configurable.go, extensions.go, config.go, presets.go, resolve.go, jwtvalidation.go, tokenexchange.go. - demos/weather-agent/demo-with-abctl.md: "The plugin pipeline spec" link. Verification: - `grep -rn '(plugins/CONVENTIONS|plugins/GUIDE|authlib/pipeline/README)'` across authbridge/ returns zero hits. - `go build ./authlib/...` clean. No code changes; only comments, docs, and file moves. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Holder wraps *Pipeline in an atomic.Pointer so listeners can hold a stable slot pointer across pipeline rebuilds. A subsequent reloader PR will Store() a newly-built pipeline through the Holder while in-flight requests finish on the previous one. Delegating methods (Run, RunResponse, NeedsBody, Ready, NotReadyPlugin, Plugins) mean listeners migrate from *Pipeline to *Holder as a pure field-type swap. Tests with -race cover Load/Store round-trip, delegation parity with the underlying pipeline, and 10 concurrent runners against a swapper. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Pure type swap across the 4 listeners (extproc, extauthz, forwardproxy, reverseproxy) + sessionapi + main.go. Holder's delegating methods keep every call site compiling unchanged — only field types change. main.go now wraps the built pipelines in Holders right after Start() and hands the Holders to listeners, sessionapi, /readyz, and the stats closure. Listeners and the session API will pick up a subsequent pipeline swap automatically; /readyz reflects the live pipeline. Test helpers (inboundPipelineFromAuth, outboundPipelineFromAuth, serverFromAuth) updated to return / wrap in Holders. Callers that construct Server literals inline now wrap their pipelines at the literal site. No behavior change — without a reloader, the Holder contents never change, so this is functionally identical to holding the raw *Pipeline. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
New authlib/reloader package that watches the config file directory and atomically swaps the inbound / outbound plugin pipelines when content changes, so operators can edit authbridge-config-<agent> without kubectl rollout restart. Directory watch (not file watch) because ConfigMap mounts use symlink swap: ..data → ..<timestamp>. Direct file watch misses the retarget. Directory + basename filter handles plain writes, atomic renames, and symlink swaps uniformly. Reload pipeline: sha256 dedup → build + validate → refuse unreloadable diff → Start → Store(new) → background drain (sleep + Stop old) Unreloadable fields today: Mode, Listener.*. Any diff there is refused with an operator-readable error in /reload/status; pipelines keep running. Validation failure at any stage leaves the active pipelines untouched and records the error in Status. Bad YAML never brings the pod down. Tests (-race): - plain rewrite → swap - debounce coalesces bursts - content-hash dedup on no-op touch - mode change refused - listener address change refused - builder error propagates to Status - ConfigProvider reflects swap Adds github.com/fsnotify/fsnotify v1.8.0 to authlib/go.mod. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
NewStatServer now takes a ConfigProvider closure (invoked per /config
request) instead of capturing a *Config pointer. When a reloader
swaps the active config under the running process, the next /config
response shows the new content without restarting the stat server.
New functional option WithReloadStatus(http.Handler) exposes a
/reload/status endpoint. Pass the Handler returned by
reloader.Reloader.Handler() to wire it up; omit when hot-reload
isn't configured.
main.go adapts with a trivial `func() *config.Config { return cfg }`
provider — a later commit swaps this for reloader.ConfigProvider()
to make /config live.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Hot-reload is now on by default: main spawns reloader.Reloader which watches the config file directory and swaps pipeline Holders when the file changes. Operators edit authbridge-config-<agent>, wait ~60s for kubelet sync, and the new pipeline picks up traffic without pod restart. - Extract the load → preset → validate → build sequence into a buildPipelines closure shared between startup and reload, so both paths run identical code. Waypoint body-access check moves into the closure too — a bad edit at reload time surfaces on /reload/status instead of crashing the pod. - Pass reloader.ConfigProvider() to the stat server so GET /config returns the live config after a swap. - Mount reloader.Handler() at /reload/status via WithReloadStatus. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Add §9 "Config hot-reload" to framework-architecture.md covering the Holder + reloader architecture, operator workflow, the 9-step reload lifecycle, and the reloadable/unreloadable matrix. Bumps subsequent sections by one. Update §12 Versioning with the hot-reload changelog entry and §13 Cross-references with the new source files (holder.go, authlib/reloader/). authbridge/CLAUDE.md: new "Config Hot-Reload" section parallel to "Session Events API", for operator-workflow discoverability. Port-mapping table notes /reload/status on :9093. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Two CI breakages on PR rossoctl#388's "Go CI (authbridge cmd)" job, both caught by GOWORK=off (each module must be self-contained): - listener/extproc/server.go had pre-existing struct-field alignment drift that the local workspace build tolerated but `gofmt -l` under GOWORK=off flagged. - cmd/authbridge's go.sum was missing the fsnotify entry, because cmd/authbridge now transitively imports authlib/reloader which pulls fsnotify. `go mod tidy` adds it. Also ran gofmt over the other files I touched in this branch (holder_test.go, reloader.go, sessionapi/server_test.go) to clean up stray whitespace. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
4 tasks
cwiklik
approved these changes
May 9, 2026
cwiklik
left a comment
Collaborator
There was a problem hiding this comment.
Review Summary
Well-designed config hot-reload implementation:
pipeline.Holder—atomic.Pointer[Pipeline]wrapper with delegating methods (Run, RunResponse, NeedsBody, Ready). Concurrent access validated under-race(10 goroutine hammering + 1 swapper).reloaderpackage — fsnotify watcher targeting the directory (handles k8s ConfigMap symlink swaps), 250ms debounce to coalesce rapid events, SHA-256 content-hash dedup (ignores mtime-only touches),validateReloadablerefuses mode/listener changes, bounded Start with rollback on partial failure (if outbound.Start fails, inbound is unwound), drain-window-based old pipeline cleanup in background goroutine.- Observability —
/reload/statusendpoint (last attempt, last success, error string, counters) and live/configreflecting the active post-swap config.
Correctness notes:
- Status mutations use copy-on-write under
atomic.Pointer— safe because all mutations are serialized in the single watchLoop goroutine context.Background()for drain/unwind is deliberate and bounded (15s timeout)- fsnotify v1.8.0 is well-maintained, no supply-chain concern
Nit: PR body describes session events/invocation wire schema (PR #386's scope) rather than the hot-reload feature this PR uniquely adds. Title is accurate though.
Areas reviewed: Go source (Holder, reloader, statserver, main.go wiring, listener refactors), Go tests (concurrency, debounce, hash dedup, mode-change refusal, builder error), Documentation
Commits: 14, all signed-off (DCO pass)
CI: All 17 checks passing
Stacking: Shares first 7 commits with PR #386; if #386 merges first, this PR's diff shrinks to the hot-reload commits only
huang195
added a commit
to huang195/kagenti-extensions
that referenced
this pull request
May 10, 2026
…n tests PR rossoctl#388 (now on main) changed listener fields from *pipeline.Pipeline to *pipeline.Holder. The body-mutation integration tests in this PR were authored before that landed and passed raw *Pipeline to Server{…} / NewServer(…); post-rebase those four spots don't compile. Wrap with pipeline.NewHolder(): extproc/server_test.go:421 &Server{InboundPipeline: inbound, …} forwardproxy/server_test.go:204 &Server{OutboundPipeline: p, …} reverseproxy/server_test.go:174 NewServer(p, nil, backend.URL) Other call sites already use the outboundPipelineFromAuth / inboundPipelineFromAuth helpers, which return *Holder, so only the tests that build pipelines inline via pipeline.New needed changes. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the observability gap where auth decisions and per-plugin actions were invisible in
/v1/sessionsand abctl. Introduces a unified plugin-invocation contract that every plugin emits, a 5-value action vocabulary shared across gate and parser plugins, aSessionDeniedphase for rejected requests, and a rewritten abctl event pane that renders one row per plugin invocation with clear request↔response pairing.New wire-schema
Every event on
/v1/sessions/{id}and/v1/eventscarries aninvocationsfield — a per-plugin record of what each plugin did during the pipeline pass:{ "phase": "request" | "response" | "denied", "invocations": { "inbound": [ { "plugin": "jwt-validation", "action": "allow", "phase": "request", "reason": "authorized", "path": "/message/stream", "tokenSubject": "alice", "tokenAudience": ["..."], "tokenScopes": ["openid", "..."] } ], "outbound": [ { "plugin": "token-exchange", "action": "modify", "phase": "request", "reason": "token_replaced", "routeMatched": true, "routeHost": "weather-tool-mcp", "targetAudience": "spiffe://...", "cacheHit": false } ] } }5-value action vocabulary
Every plugin MUST emit exactly one of these per invocation:
Reason codes discriminate within an action value (
skip/path_bypassvsskip/no_matching_route) for finer-grained filtering.What is new in abctl
/allow,/deny,/skip,/modify,/observe.notifications/initializeddoesn't steal thetools/listresponse.PHASE=deniedcarrying full diagnostic context.Backward compatibility
Breaking for pre-existing consumers of the session wire format.
SessionEvent.Auth->SessionEvent.Invocations, andInboundAuth.Decision/OutboundAuth.Action(free-form strings) unify intoInvocation.Action(5-value typed enum). Session API has no persistent state, so no migration needed. No operator or chart changes.Test plan
go test ./...inauthlib,cmd/authbridge,cmd/abctl— all greengo vet ./...across all three modules — cleanauthbridge-envoyimage, deployed to Kind, drove A2A + MCP + inference traffic; verified one-row-per-plugin rendering, pair IDs, SSE response bodies, filtersAssisted-By: Claude (Anthropic AI) noreply@anthropic.com