feat(pipeline): Structured rejections + lifecycle hooks + plugin spec docs#377
Conversation
Aimed at both internal AuthBridge contributors and external plugin-framework teams (e.g. ContextForge/CPEX) that want to integrate a sub-pipeline engine as a single AuthBridge bridge plugin. Covers: - Plugin interface (Name, Capabilities, OnRequest, OnResponse) - pipeline.Context full surface (fields, ownership, lifetime) - Named extension slots (MCP, A2A, Inference, Security, Delegation) + GetState[T]/SetState[T] generic helpers for plugin-private state - Action / capability wiring with startup validation - Session store + SessionEvent as the observability side-channel - Pipeline vs listener boundary (what the contract does NOT own: body buffering negotiation, JWT issuance, session writes, SSE streaming) - Worked native-plugin examples - §10 Integration spec for bridge plugins, with AuthBridge<->CPEX slot mapping, direction+phase hook-name encoding, capability declaration strategy, and 'must not' list - §11 Open questions for cross-team iteration Also extends authlib/README.md to index the pipeline, session, sessionapi, plugins, and observe packages which were previously undocumented at the module level. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Walk a new user from 'Kagenti installed, weather agent deployed' through building abctl, port-forwarding :9094, sending chat messages via the UI, and using the TUI to observe: - Session buckets with live event counts and token totals - The IDENTITY banner summarizing caller subject/client/scopes - Inbound and outbound event pairing with request<->response connectors - Phase-filtered event detail JSON (request rows show only request fields; response rows show only response fields) - Yank to /tmp for offline analysis - Pipeline pane with per-plugin event counts for the current session - Live SSE streaming, rate indicator, filter, and pause Includes a troubleshooting table for common states and links to related demos (token-exchange, tool-side AuthBridge) and the pipeline spec. demo-ui.md gets a cross-link so the new walkthrough is discoverable from the getting-started demo. Signed-off-by: Hai Huang <huang195@gmail.com>
Two interface improvements that were blocking third-party plugin authoring (items rossoctl#1 and rossoctl#3 from the authbridge plugin-pipeline must-fix list). Both lift patterns from CPEX's PluginViolation + init/shutdown model. 1) Rejection contract (breaking) Replace Action{Type: Reject, Status, Reason} with Action{Violation *}. Violation carries CPEX-aligned structured fields (Code, Reason, Description, Details, PluginName) plus optional HTTP-rendering hints (Status, Body, BodyType, Headers). Default body when Body is nil: { "error": "auth.missing-token", "message": "Bearer token required", "description": "No Authorization header present", "plugin": "jwt-validation", "details": {"realm": "kagenti"} } Helper constructors land the common cases in one line: pipeline.Deny("auth.invalid-token", "expired") pipeline.DenyStatus(451, "policy.forbidden", "legal") pipeline.DenyWithDetails("policy.rate-limited", "quota hit", details) pipeline.Challenge("kagenti", "authz required") // 401 + WWW-Authenticate pipeline.RateLimited(30*time.Second, "", "slow") // 429 + Retry-After A codeToStatus table maps well-known codes to HTTP status so plugins skip Status when the default is right. StatusFromCode("unknown") == 500. Behaviors fixed: - Plugin controls the error code. Previously each listener hardcoded it (extproc: "unauthorized" / "token_acquisition_failed" / "response_blocked"). - Plugin controls status. TestExtProc_Outbound_Deny now asserts 401 (what tokenexchange actually returns for "no token available") instead of the hardcoded 503 the listener used to force. - PluginName auto-stamped from Plugin.Name() by the pipeline, so support tickets trace to the offending plugin from the response body alone. - Listeners (extproc/extauthz/forwardproxy/reverseproxy) preserve custom headers (WWW-Authenticate, Retry-After, etc.) through their wire formats. Before: headers were dropped. Migration: the in-tree plugins (jwtvalidation, tokenexchange) now use DenyStatus. Parsers (a2a/mcp/inference) never reject — unchanged. No external plugins known at this time. 2) Lifecycle hooks (additive, opt-in) New optional interfaces plugins may implement: type Initializer interface { Init(ctx context.Context) error } type Shutdowner interface { Shutdown(ctx context.Context) error } Pipeline.Start iterates plugins in declaration order, calls Init on each that implements Initializer, fails fast on first error with the offending plugin's name wrapped. Pipeline.Stop iterates in reverse order, calls Shutdown on each Shutdowner, logs and continues past individual errors so every plugin gets a chance to flush. Host wiring in cmd/authbridge/main.go: Start after pipeline construction (60s bounded ctx) and before listeners accept traffic; Stop after listeners drain on SIGTERM (15s window). Existing plugins don't implement either interface — they're skipped via type-assertion, zero churn. Enables non-trivial plugin patterns that previously required sync.Once-inside-OnRequest and leaky goroutine cleanup. Tests: - 13 tests cover Violation.Render (default body, custom body, nil receiver, header clone, code-to-status fallback) + helper constructors. - 5 tests cover Start/Stop (order, fail-fast on Init error, best-effort Shutdown, reverse-order Shutdown, no-op when no plugins implement). - Full regression: authlib + cmd/authbridge + cmd/abctl all green. Docs: - pipeline/README.md §5 rewritten: new Action/Violation shape, default body example, helper list, code-to-status pointer. - pipeline/README.md §6 extended: Start/Stop methods, lifecycle contracts, worked RateLimiter example using both Init+Shutdown and the RateLimited helper. - pipeline/README.md §10 (cpex-bridge integration spec) updated: CPEX Violation -> authbridge Violation mapping is now near-isomorphic. - pipeline/README.md §12 records the Action breaking change and the lifecycle addition in the pre-1.0 interface history. Signed-off-by: Hai Huang <haih@us.ibm.com> Signed-off-by: Hai Huang <huang195@gmail.com>
- pipeline/README.md: remove the CPEX integration section and CPEX references scattered through the spec so the doc focuses on AuthBridge's own plugin contract. - weather-agent/demo-with-abctl.md: replace fabricated plugin names (session-recorder, route-resolver) in the Pipeline pane mock with the real pipeline composition, fix the BODY column rendering (yes/no, not a checkmark), fix the divider row, and add a prerequisite step that enables the a2a/mcp/inference parsers -- without which the PROTO and TOKENS columns shown later in the walkthrough stay empty. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
pdettori
left a comment
There was a problem hiding this comment.
Well-structured PR that hardens the plugin-pipeline contract with a rich rejection model (Violation struct, helpers, code-to-status table) and opt-in lifecycle interfaces. The code is clean, backward-compatible for existing plugins, and comprehensive in test coverage (18 new tests). All four listeners consistently use Render() for reject handling, eliminating the prior inconsistency where each listener hardcoded different error codes.
Areas reviewed: Go (pipeline, plugins, listeners, main.go), Docs
Commits: 4 commits, all signed-off (DCO: pass)
CI status: All 17 checks passing
| // Callers should invoke Start after Pipeline construction (pipeline.New) | ||
| // and before the listener accepts traffic. Safe to call at most once per | ||
| // Pipeline — plugins may assume Init runs exactly once per process. | ||
| func (p *Pipeline) Start(ctx context.Context) error { |
There was a problem hiding this comment.
suggestion: Start fails fast on Init error but does not call Shutdown on plugins whose Init already succeeded. If plugin A allocates resources in Init and plugin B's Init fails, A leaks. The contract is documented ("No Shutdown is invoked — the intent is hard-fail on startup, not unwind") so this is a known design choice, and since main.go calls log.Fatalf the process exits anyway (OS reclaims resources). Worth considering an unwind for a future iteration where Start callers might want to retry or recover rather than exit.
| } | ||
| if action.Status != http.StatusUnauthorized { | ||
| t.Errorf("status = %d, want 401", action.Status) | ||
| status, _, _ := action.Violation.Render(); if status != http.StatusUnauthorized { |
There was a problem hiding this comment.
nit: Multi-statement on one line (status, _, _ := action.Violation.Render(); if status != ...) is valid Go but unconventional — consider splitting to two lines for readability.
Summary
Hardens the plugin-pipeline contract ahead of exposing it to third-party plugin authors. Three landing points:
Action{Reject, ...}): replaces flatAction.Status/Action.Reasonwith a structured*Violation, adds default body synthesis, custom headers/body overrides, CPEX-aligned fields (Code,Reason,Description,Details,PluginName), and helper constructors (Deny,DenyStatus,DenyWithDetails,Challenge,RateLimited).Initializer/Shutdownerinterfaces withPipeline.Start(ctx)/Pipeline.Stop(ctx)driving them. Existing plugins are unaffected — the interfaces are opt-in via type-assertion.authbridge/authlib/pipeline/README.mdnow covers both new surfaces, with a workedRateLimiterexample exercising Init, Shutdown, and theRateLimitedhelper. The cpex-bridge integration spec in the same file got its CPEX↔authbridge mapping table updated — the two Violation shapes are now near-isomorphic. Also includes a new abctl walkthrough atauthbridge/demos/weather-agent/demo-with-abctl.md.Motivated by the plugin-interface gap analysis discussed in-session; fulfills must-fix #1 (richer rejection) and #3 (lifecycle hooks) from that list.
New plugin shape (what this enables)
Previously impossible without
sync.Once-inside-OnRequestand goroutine leaks at shutdown.Behaviors fixed / improved
"unauthorized","response_blocked", etc.) regardless of plugin intentViolation.Code; listener passes throughTestExtProc_Outbound_Denyasserted 503 because the listener forced it, hiding the plugin's actual 401Violation.PluginNameauto-stamped by pipeline fromPlugin.Name(); default body includes"plugin": "<name>"WWW-Authenticate,Retry-After) had nowhere to goViolation.Headersthrough their respective wire formatsInit(ctx)called before listeners accept traffic, fail-fast with named pluginShutdown(ctx)called after listeners drain, reverse order, best-effortTest plan
go test ./...inauthlib— all green (18 new tests across action + lifecycle)go test ./...incmd/authbridge— all green; one test assertion corrected to reflect the now-correct pass-through status behaviorgo test ./...incmd/abctl— unaffected, all greenpodman build -f cmd/authbridge/Dockerfile), loaded into kind, rolled the weather-agent pod, verified pod reaches Ready and chat requests flow through unchangedFiles changed
pipeline/action.goAction/Violationshape + 5 helpers +codeToStatus+Render()pipeline/action_test.gopipeline/plugin.goInitializer,Shutdowneroptional interfacespipeline/pipeline.goStart(ctx)/Stop(ctx); internal cancellation usesDeny(); auto-stamps PluginNamepipeline/pipeline_test.gopipeline/README.mdplugins/jwtvalidation.goDenyStatuswith status→code derivationplugins/tokenexchange.goupstream.token-exchange-failedandpolicy.forbiddenplugins/plugins_test.goViolation.Render()listener/extproc/server.gorejectFromActionhelper preserves headers viaHeaderMutation.SetHeaderslistener/extproc/server_test.golistener/extauthz/server.godeniedFromActionhelper for the DeniedResponse pathlistener/forwardproxy/server.gowriteRejectionhelperlistener/reverseproxy/server.goresponseRejectedErrornow carries the full Action;writeRejectionhelpercmd/authbridge/main.goStart(initCtx)after pipeline construction;Stop(shutdownCtx)in SIGTERM pathRelated
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com