feat: Plugin relationship declarations + token-exchange/token-broker claim migration - #400
Conversation
Extends PluginCapabilities with four chain-scoped relationship fields validated at plugins.Build time (startup + hot-reload): Requires []string - ALL must be present + earlier (hard) RequiresAny []string - AT LEAST ONE must be present + run after it After []string - soft ordering; silent if absent Claims []string - <=1 per claim per chain (mutex) Closes the "accidental double-enable" class of plugin conflict where two plugins silently clobber each other. The common case today: token-exchange and token-broker both replace the outbound Authorization header if configured together. With this change the chain fails startup with a clear "plugins %q and %q both claim %q" message. Covers more than conflict detection. Requires models hard plugin deps; RequiresAny models "need at least one parser" for protocol- agnostic guardrails that read through pctx.ContentSources(); After is a soft ordering hint that's silent if the named plugin is absent. Ships authlib/contracts/claims.go with ClaimAuthorizationHeader as the initial canonical claim constant. Plugin authors reference the constant instead of a string literal so typos become compile errors and the canonical set is greppable. Third-party plugins may declare arbitrary strings; the framework enforces uniqueness of whatever it sees, not "must be from the list." Validator collects all errors per chain into one report rather than short-circuiting on the first - friendlier for operator iteration on a freshly-edited YAML. No behavior change for existing plugins - all four fields default to empty slices; the validator is a no-op when nothing is declared. Resolves rossoctl#398. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
…-broker Closes the concrete rossoctl#398 case for the relationship framework added in 2d8ba1c. Both plugins replace the outbound Authorization header when they match a route, and prior to this change configuring both in the same chain was a silent last-writer-wins hazard. Each plugin now declares: Claims: []string{contracts.ClaimAuthorizationHeader} in its PluginCapabilities. The Build-time validator rejects any chain that includes both with: plugins "token-exchange" and "token-broker" both claim "authorization_header"; configure only one per chain Adds an integration test in plugins_test.go wiring the real plugins through Build (rather than the synthetic relPlugin used in registry_test.go) to catch a future regression where one of them drops the Claims declaration or the constant drifts. Adds the tokenbroker side-effect import alongside the existing tokenexchange one so the integration test can resolve both names through the registry. No behavior change for chains that declare only one of the two. Resolves rossoctl#398. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
The referenced file authbridge-combined-sidecar.md was merged into kagenti's docs/authbridge/deployment-guide.md in kagenti@537378f. Updates two link references in demos/github-issue/demo-ui.md to point to the current location. Verified 200 with curl. Fixes rossoctl#396. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Three doc-sync fixes discovered while auditing PR rossoctl#400 for consistency with the code it ships: * framework-architecture.md §12 Versioning: add a changelog entry for the plugin relationship declarations. Every prior framework change has an entry here; the new Requires / RequiresAny / After / Claims fields + ClaimAuthorizationHeader constant were missing. * framework-architecture.md §13 Cross-references: the plugin.go entry listed ReadsBody / WritesBody / BodyAccess as the PluginCapabilities fields worth calling out, but stopped there. Extended to mention the four new chain-scoped relationship fields so a reader navigating from the architecture doc into the code knows they exist. * authlib/README.md package table: contracts/ was missing entirely (pre-existing gap, amplified by adding claims.go alongside content.go). Added an entry describing what plugins reach into the package for: role constants, the ContentSource contract, and claim constants. No code changes; text only. 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-architected plugin relationship framework with four orthogonal relationship types (Requires, RequiresAny, After, Claims) validated at startup. The validator correctly implements all semantics, collects all errors rather than short-circuiting, and handles edge cases cleanly (empty chains, same-plugin idempotency, earliest-occurrence position tracking). The token-exchange/token-broker migration is the concrete motivating case and is tested with both synthetic helpers and an integration test wiring real plugins through Build. Documentation in plugin-reference.md accurately mirrors the implementation.
Areas reviewed: Go (framework + plugins + tests), Docs (architecture, plugin-reference, README, demo link fixes)
Commits: 4 commits, all signed-off ✓
CI status: all checks passing (15/15 pass, 1 skipped: spellcheck)
Assisted-By: Claude Code
PR rossoctl#400 migrated both plugins to declare `Claims: []string{contracts.ClaimAuthorizationHeader}`, which made plugins.Build reject any chain that contained both. On review, David pointed out this diagnosis was wrong: token-exchange and token-broker have legitimate coexistence use cases (the former handles Keycloak- audience hosts, the latter handles federated-broker hosts, same deployment may need both on different routes). The correct fix for their interaction is per-route arbitration — "which handler runs for this specific host" — not per-chain mutual exclusion. Route-scoped arbitration is a separate design (still open on rossoctl#398). This commit is the narrow revert that restores coexistence. Reverts: - tokenexchange/plugin.go: Capabilities() returns empty PluginCapabilities; contracts import dropped. - tokenbroker/plugin.go: same. - plugins_test.go: removes TestBuild_TokenExchangeAndTokenBroker_ConflictingClaims (the integration test that locked in the mutex) and the tokenbroker side-effect import that only existed for that test. Retained on purpose (no behavior change, no consumer today): - PluginCapabilities.Claims field + validateRelationships Claims loop + related tests in registry_test.go — the framework-level mutex mechanism is general-purpose and may have valid uses for plugins that genuinely conflict chain-wide rather than per- route. No in-tree plugin currently declares Claims. - authlib/contracts/claims.go + ClaimAuthorizationHeader constant — kept so that a future plugin declaring this specific claim doesn't have to re-invent the vocabulary. Documented as unreferenced-by-in-tree-plugins to avoid signal confusion. Operationally: after this lands, configuring both token-exchange and token-broker in the same outbound chain becomes the operator's responsibility to partition cleanly across routes — same as the pre-rossoctl#400 status quo. The plugins' own OnRequest behavior is unchanged by this revert (it was also unchanged by rossoctl#400 — the mutex was build-time only). Relates to rossoctl#398. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Summary
Adds plugin relationship declarations to
PluginCapabilities, validatedat
plugins.Buildtime (startup + hot-reload), and migratestoken-exchangeandtoken-brokerto declare the newauthorization_headerclaim — closing the concrete silent-clobbercase from #398.
Framework (commit 1): extends
PluginCapabilitieswith fourchain-scoped relationship fields:
Requires []stringRequiresAny []stringAfter []stringClaims []stringShips
authlib/contracts/claims.gowithClaimAuthorizationHeaderasthe initial canonical claim constant — plugin authors reference the
constant so typos become compile errors and the canonical set is
greppable. Third-party plugins may declare arbitrary strings; the
framework enforces uniqueness of what it sees, not "must be from the
list."
The validator collects all errors per chain into one report rather
than short-circuiting on the first, so operators iterating on a
freshly-edited YAML see every problem at once.
No behavior change for existing plugins — all four fields default to
empty slices; the validator is a no-op when nothing is declared.
Migration (commit 2): both
token-exchangeandtoken-brokerreplace the outbound
Authorizationheader when they match a route.Configuring both in the same chain was previously a silent
last-writer-wins hazard. Now each declares
Claims: []string{contracts.ClaimAuthorizationHeader}and the chainfails startup with:
An integration test in
plugins_test.gowires the real pluginsthrough
Build(rather than the syntheticrelPluginhelper used inregistry_test.go) to catch a future regression where one of themdrops the
Claimsdeclaration.Resolves #398.
Test plan
go test -race ./...inauthbridge/authlib— passesgo test -race ./...inauthbridge/cmd/authbridge— passesgofmt -lon modified files — cleanAssisted-By: Claude (Anthropic AI) noreply@anthropic.com