Document SPIFFE association declarations by jhrozek · Pull Request #6501 · stacklok/toolhive

jhrozek · 2026-09-03T08:47:11Z

Summary

A configuration-only feature (SPIFFE trust domains + static workload-to-client associations, no live SVID verification yet) could easily be mistaken for a completed authentication feature if it isn't clearly scoped in the architecture docs.

Add docs/arch/18-spiffe-association-declarations.md, documenting the trust/static-client flow, validation boundaries, startup collision handling, restart behavior, and an explicit list of what this feature does NOT do (bundle fetching, live SVID verification, token issuance through SPIFFE).
Cross-reference it from 09-operator-architecture.md, 11-auth-server-storage.md, and the arch docs index.
Add missing item-length CRD validation markers (MinLength/MaxLength) on SPIFFEClientConfig.Audiences/.Scopes, matching the markers already present on sibling fields on the same type — caught while writing the doc's schema description and cross-checking it against the actual CRD markers.

Refs #6200, #6205

Type of change

Documentation

Test plan

Unit tests (task test)
Linting (task lint-fix)

task operator-manifests/task crdref-gen re-run to confirm the CRD marker addition produces the expected schema/docs diff and nothing else drifts.

API Compatibility

This PR does not break the v1beta1 API — the marker addition only narrows validation on a field with no existing users (this feature isn't released yet).

Does this introduce a user-facing change?

Documentation only, plus slightly stricter (but backward-compatible) validation on two SPIFFE CRD fields.

Special notes for reviewers

This adapts a doc originally written on an earlier, abandoned branch to the current schema — every factual claim (field paths, bundleSource/resources behavior, collision-handling semantics) was independently re-verified against current source rather than carried over from the old draft; several were corrected in the process (stale field path, an outdated "fails closed on any collision" claim that's now more nuanced after #6474's hardening work, and the resources field not yet flowing into the runtime OAuth client). Stacked on #6500.

codecov · 2026-09-03T08:55:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.40%. Comparing base (e44e5c8) to head (4093b33).

Additional details and impacted files

@@                       Coverage Diff                       @@
##           spiffe-integration-split3-6    #6501      +/-   ##
===============================================================
- Coverage                        78.40%   78.40%   -0.01%     
===============================================================
  Files                              776      776              
  Lines                            76253    76253              
===============================================================
- Hits                             59788    59783       -5     
- Misses                           16460    16465       +5     
  Partials                             5        5

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JAORMX

The new architecture documentation accurately explains the intended model, but currently presents it as operational when it is not reachable.

The example and related text say that SPIFFE declarations create static OAuth clients, install the storage overlay, and reserve client IDs at startup (docs/arch/18-spiffe-association-declarations.md:3,34-78; docs/arch/11-auth-server-storage.md:435-445; docs/arch/09-operator-architecture.md:241). However, the current runtime hard-rejects every non-empty spiffe_trust_domains configuration before that setup can run (pkg/authserver/config.go:317-345). Applying the documented CRD example therefore prevents the embedded auth server from starting; the described static-client, collision, restart, and reservation behavior cannot yet occur.

Please describe these as declarations that are currently rejected pending live SVID verification, or defer the operational-runtime claims until that gate is lifted.

JAORMX

Changes requested:

docs/arch/18-spiffe-association-declarations.md:3,66-78, docs/arch/11-auth-server-storage.md:437-445, and docs/arch/09-operator-architecture.md:241: the documentation describes static-client registration as operational, but RunConfig.Validate rejects every non-empty spiffe_trust_domains configuration before auth-server initialization. State that declarations are currently rejected pending SVID verification, or defer the operational claims.

JAORMX

Changes requested:

docs/arch/18-spiffe-association-declarations.md:3,23,34,62,66-78,82-94, docs/arch/11-auth-server-storage.md:437-445, docs/arch/09-operator-architecture.md:241, and docs/operator/crd-api.md:4251-4260,4272-4275 describe static-client registration, durable reservations, decorators, restart behavior, and policy enforcement as operational. RunConfig.Validate rejects every non-empty SPIFFE trust-domain configuration before auth-server initialization (pkg/authserver/config.go:317-345), so those effects cannot occur. Describe the fields as declarations that are currently rejected pending live SVID verification, and defer the operational claims until that gate is lifted.

All current CI checks are green; no local tests were run.

JAORMX

Re-review at 65adeceb410572eb7b09d608d7274d57b15edb11: this documentation layer is unchanged apart from the rebuilt bases, so the substantive documentation blockers remain.

RunConfig.Validate rejects every non-empty SPIFFE declaration before auth-server initialization, but docs/arch/18-spiffe-association-declarations.md:3,34-78, docs/arch/11-auth-server-storage.md:435-445, and the operator/index cross-references describe registration, overlays, durable reservations, restart behavior, and policy enforcement as operational. State prominently that current operator reconciliation terminates with invalid configuration pending SVID verification; do not present the YAML as supported runtime behavior.
The example uses embeddedAuthServer.allowedAudiences (docs/arch/18-spiffe-association-declarations.md:41), which is not a CRD field; audiences are derived from the workload OIDC configuration.
Line 23 says resources do not flow into request-time enforcement, but the updated #6473 base now preserves and enforces them separately from audiences.
The restart/removal and collision explanations are still inaccurate: removed placeholders are not cleaned up, the fingerprint does not contain SPIFFE principal/domain/method identity, and CIMD does perform a durable UpsertDCRIssuedClient rather than only caching without registration.

All checks are green.

Signed-off-by: Jakub Hrozek <jakub@stacklok.com>

The SPIFFE client-auth epic needs a place to configure SPIFFE association policy without inventing a parallel trust/grant path next to the existing delegate-client and trusted-issuer configuration. As more inbound grant families (RFC 8693 token exchange, RFC 7523 JWT-bearer, SPIFFE) accumulate, they need one canonical surface to configure and reason about instead of three independent ones, without breaking deployments that already rely on the legacy fields. Add pkg/authserver/inbound_grants.go with NormalizeInboundGrants, which reconciles a new canonical RunConfig.InboundGrants surface (per-family token_exchange/jwt_bearer sub-configs whose issuer_policies reference a trusted_issuers entry by name) against the legacy top-level delegate_clients and the RFC 8693/7523 fields embedded directly on trusted_issuers. Legacy and canonical configuration for the same grant family are mutually exclusive and rejected at validation time; the two families are otherwise independent, and omitting inbound_grants entirely preserves released behavior. Thread the normalized result through RunConfig.Validate, the embedded-auth-server runner, and buildProvider/discovery, adding a DisableTokenExchange capability so RFC 8693 registration and discovery advertisement can be turned off together and can't drift out of sync. Add TrustedIssuer.Name so canonical issuer_policies can reference an issuer without duplicating its fields. SPIFFE client authentication (InboundGrants.SPIFFEClientAuth, defined in the previous commit) is deliberately kept a sibling of TokenExchange and JWTBearer here, not nested under either: SPIFFE authenticates a client, it does not by itself grant it anything, so making it subordinate to RFC 8693 enablement would mean disabling token exchange silently drops every SPIFFE association, and every SPIFFE-authenticated client would be implicitly token-exchange-capable. It is validated and wired directly from RunConfig.InboundGrants in RunConfig.Validate/embeddedauthserver.go, independent of this file's legacy/canonical projection, so authentication method and grant-family enablement stay separately configurable. Update docs/arch/17-token-exchange-delegation.md for the new inbound_grants shape and the now-conditional token-exchange discovery advertisement, and add a runner-level test proving the canonical delegate-client, SPIFFE-client, and jwt_bearer paths reach a running server (the existing tests only covered normalization in isolation). SPIFFE client-auth associations always require the token-exchange grant (the only grant type they may declare), independent of the legacy/canonical token-exchange projection above: NormalizeInboundGrants now sets Capabilities.TokenExchange true whenever InboundGrants.SPIFFEClientAuth is non-empty, so a SPIFFE-only configuration cannot leave it false and silently disable the RFC 8693 grant handler server-wide -- which would reject every SPIFFE client's own token requests before authentication is even checked. Guarded by a regression test in this package (not just the runner-level test above) since the equivalent fix was previously lost during a rebase when its only coverage lived one package away. DCR (RFC 7591 /oauth/register) now rejects a registration whose effective grant types include token-exchange when it is disabled server-wide, instead of accepting the client and only failing later, confusingly, at /oauth/token. The check runs on the post-defaulting grant types validateGrantTypes already computes (a private_key_jwt client with an empty grant_types is implicitly token-exchange-only), so it catches both the explicit and implicit cases the same way scope validation already gates DCR on ScopesSupported. Corrected two stale doc references caught in review: the SPIFFE client-policy field path (inbound_grants.spiffe_client_auth, not nested under token_exchange) and the JWT-bearer legacy/canonical conflict wording (family-wide across all issuers, not per-issuer). Refs #6200 Signed-off-by: Jakub Hrozek <jakub@stacklok.com>

The operator needs the shared inbound-grant model before it can add SPIFFE associations without inventing a separate Kubernetes API. Add v1beta1 grant-family types, CEL constraints, runtime conversion, generated schemas, and compatibility coverage for existing grant fields. Refs #6200

Operators need a visible migration signal before legacy grant fields can be removed safely. Record deprecated field paths during normalization and surface them as status conditions without changing the effective authorization policy. Refs #6200

The SPIFFE client-auth epic needs a place to configure SPIFFE association policy without inventing a parallel trust/grant path next to the existing delegate-client and trusted-issuer configuration. As more inbound grant families (RFC 8693 token exchange, RFC 7523 JWT-bearer, SPIFFE) accumulate, they need one canonical surface to configure and reason about instead of three independent ones, without breaking deployments that already rely on the legacy fields. Add pkg/authserver/inbound_grants.go with NormalizeInboundGrants, which reconciles a new canonical RunConfig.InboundGrants surface (per-family token_exchange/jwt_bearer sub-configs whose issuer_policies reference a trusted_issuers entry by name) against the legacy top-level delegate_clients and the RFC 8693/7523 fields embedded directly on trusted_issuers. Legacy and canonical configuration for the same grant family are mutually exclusive and rejected at validation time; the two families are otherwise independent, and omitting inbound_grants entirely preserves released behavior. Thread the normalized result through RunConfig.Validate, the embedded-auth-server runner, and buildProvider/discovery, adding a DisableTokenExchange capability so RFC 8693 registration and discovery advertisement can be turned off together and can't drift out of sync. Add TrustedIssuer.Name so canonical issuer_policies can reference an issuer without duplicating its fields. SPIFFE client authentication (InboundGrants.SPIFFEClientAuth, defined in the previous commit) is deliberately kept a sibling of TokenExchange and JWTBearer here, not nested under either: SPIFFE authenticates a client, it does not by itself grant it anything, so making it subordinate to RFC 8693 enablement would mean disabling token exchange silently drops every SPIFFE association, and every SPIFFE-authenticated client would be implicitly token-exchange-capable. It is validated and wired directly from RunConfig.InboundGrants in RunConfig.Validate/embeddedauthserver.go, independent of this file's legacy/canonical projection, so authentication method and grant-family enablement stay separately configurable. Update docs/arch/17-token-exchange-delegation.md for the new inbound_grants shape and the now-conditional token-exchange discovery advertisement, and add a runner-level test proving the canonical delegate-client, SPIFFE-client, and jwt_bearer paths reach a running server (the existing tests only covered normalization in isolation). SPIFFE client-auth associations always require the token-exchange grant (the only grant type they may declare), independent of the legacy/canonical token-exchange projection above: NormalizeInboundGrants now sets Capabilities.TokenExchange true whenever InboundGrants.SPIFFEClientAuth is non-empty, so a SPIFFE-only configuration cannot leave it false and silently disable the RFC 8693 grant handler server-wide -- which would reject every SPIFFE client's own token requests before authentication is even checked. Guarded by a regression test in this package (not just the runner-level test above) since the equivalent fix was previously lost during a rebase when its only coverage lived one package away. DCR (RFC 7591 /oauth/register) now rejects a registration whose effective grant types include token-exchange when it is disabled server-wide, instead of accepting the client and only failing later, confusingly, at /oauth/token. The check runs on the post-defaulting grant types validateGrantTypes already computes (a private_key_jwt client with an empty grant_types is implicitly token-exchange-only), so it catches both the explicit and implicit cases the same way scope validation already gates DCR on ScopesSupported. Corrected two stale doc references caught in review: the SPIFFE client-policy field path (inbound_grants.spiffe_client_auth, not nested under token_exchange) and the JWT-bearer legacy/canonical conflict wording (family-wide across all issuers, not per-issuer). Refs #6200 Signed-off-by: Jakub Hrozek <jakub@stacklok.com>

The SPIFFE client-authentication runtime model (trust domains + static workload-to-client associations) already exists in pkg/authserver, but was only reachable by hand-authoring an auth-server RunConfig file -- there was no way to declare it through VirtualMCPServer or MCPExternalAuthConfig. Add SPIFFETrustDomains and InboundGrants.SPIFFEClientAuth to the shared EmbeddedAuthServerConfig CRD type, with CEL admission validation for everything derivable from the object's own spec (paired configuration, no duplicate names/trust domains/client IDs/principal patterns, full cross-referencing between domains and clients, method-subset enforcement, principal/trust-domain consistency, and client-ID hygiene). Wire the new fields through to authserver.RunConfig via the same converter pattern used for DelegateClients/TrustedIssuers, and extend the existing reconcile-time revalidation (validateDelegateClientsAndTrustedIssuers) to cover SPIFFE trust domains too. Cross-field checks that need reconcile-time-derived values (AllowedAudiences, ScopesSupported -- neither is CRD-exposed) are left to that reconcile-time path rather than a premature admission-time Go check: an earlier attempt at that check passed nil for both, which the runtime validator treats as "validate against nothing" rather than "skip", silently rejecting any resources or custom scopes entry. A syntactically valid SPIFFE config is admitted by CEL but still fails reconciliation with a terminal error until RunConfig's "not yet enforced" placeholder gate is lifted by a future PR -- no live SVID verification exists yet, so this is expected, not a regression. Signed-off-by: Jakub Hrozek <jakub@stacklok.com>

The SPIFFE client-auth epic needs a place to configure SPIFFE association policy without inventing a parallel trust/grant path next to the existing delegate-client and trusted-issuer configuration. As more inbound grant families (RFC 8693 token exchange, RFC 7523 JWT-bearer, SPIFFE) accumulate, they need one canonical surface to configure and reason about instead of three independent ones, without breaking deployments that already rely on the legacy fields. Add pkg/authserver/inbound_grants.go with NormalizeInboundGrants, which reconciles a new canonical RunConfig.InboundGrants surface (per-family token_exchange/jwt_bearer sub-configs whose issuer_policies reference a trusted_issuers entry by name) against the legacy top-level delegate_clients and the RFC 8693/7523 fields embedded directly on trusted_issuers. Legacy and canonical configuration for the same grant family are mutually exclusive and rejected at validation time; the two families are otherwise independent, and omitting inbound_grants entirely preserves released behavior. Thread the normalized result through RunConfig.Validate, the embedded-auth-server runner, and buildProvider/discovery, adding a DisableTokenExchange capability so RFC 8693 registration and discovery advertisement can be turned off together and can't drift out of sync. Add TrustedIssuer.Name so canonical issuer_policies can reference an issuer without duplicating its fields. SPIFFE client authentication (InboundGrants.SPIFFEClientAuth, defined in the previous commit) is deliberately kept a sibling of TokenExchange and JWTBearer here, not nested under either: SPIFFE authenticates a client, it does not by itself grant it anything, so making it subordinate to RFC 8693 enablement would mean disabling token exchange silently drops every SPIFFE association, and every SPIFFE-authenticated client would be implicitly token-exchange-capable. It is validated and wired directly from RunConfig.InboundGrants in RunConfig.Validate/embeddedauthserver.go, independent of this file's legacy/canonical projection, so authentication method and grant-family enablement stay separately configurable. Update docs/arch/17-token-exchange-delegation.md for the new inbound_grants shape and the now-conditional token-exchange discovery advertisement, and add a runner-level test proving the canonical delegate-client, SPIFFE-client, and jwt_bearer paths reach a running server (the existing tests only covered normalization in isolation). SPIFFE client-auth associations always require the token-exchange grant (the only grant type they may declare), independent of the legacy/canonical token-exchange projection above: NormalizeInboundGrants now sets Capabilities.TokenExchange true whenever InboundGrants.SPIFFEClientAuth is non-empty, so a SPIFFE-only configuration cannot leave it false and silently disable the RFC 8693 grant handler server-wide -- which would reject every SPIFFE client's own token requests before authentication is even checked. Guarded by a regression test in this package (not just the runner-level test above) since the equivalent fix was previously lost during a rebase when its only coverage lived one package away. DCR (RFC 7591 /oauth/register) now rejects a registration whose effective grant types include token-exchange when it is disabled server-wide, instead of accepting the client and only failing later, confusingly, at /oauth/token. The check runs on the post-defaulting grant types validateGrantTypes already computes (a private_key_jwt client with an empty grant_types is implicitly token-exchange-only), so it catches both the explicit and implicit cases the same way scope validation already gates DCR on ScopesSupported. Corrected two stale doc references caught in review: the SPIFFE client-policy field path (inbound_grants.spiffe_client_auth, not nested under token_exchange) and the JWT-bearer legacy/canonical conflict wording (family-wide across all issuers, not per-issuer). Refs #6200 Signed-off-by: Jakub Hrozek <jakub@stacklok.com>

A configuration-only foundation could otherwise be mistaken for completed SVID authentication and bundle management. Document the trust and static-client flow, validation boundaries, restart behavior, and the explicit absence of live credential verification. Also add missing item-length CRD validation markers on SPIFFEClientConfig's Audiences/Scopes fields, matching the sibling fields on the same type. Refs #6200 Refs #6205 Signed-off-by: Jakub Hrozek <jakub@stacklok.com>

JAORMX

Re-review at 4093b33 (reviewed only e44e5c8..4093b33):

The prior documentation blockers remain:

docs/arch/18-spiffe-association-declarations.md:3,34,62,66-78,82-94; docs/arch/09-operator-architecture.md:241; docs/arch/11-auth-server-storage.md:437-445; docs/arch/README.md:144-147: non-empty SPIFFE declarations are rejected before runner creation pending SVID verification, but these documents describe them as operational. State the terminal current behavior prominently and do not present the YAML as deployable.
docs/arch/18-spiffe-association-declarations.md:41 uses nonexistent embeddedAuthServer.allowedAudiences; these audiences are derived from resolved incoming OIDC configuration.
docs/arch/18-spiffe-association-declarations.md:23 incorrectly says resources do not reach request-time enforcement; docs/arch/11-auth-server-storage.md:441 incorrectly says CIMD does not write durable client state.
docs/arch/18-spiffe-association-declarations.md:73-78 overstates restart/removal behavior: removed placeholders persist, and changed identity/policy conflicts rather than taking effect on restart; fingerprints also include identity and resources.

Exact-head CI is green.

jhrozek requested review from ChrisJBurns, JAORMX, amirejaz, blkt, jerm-dro, rdimitrov, reyortiz3 and tgrunnagle as code owners September 3, 2026 08:47

github-actions Bot added the size/S Small PR: 100-299 lines changed label Sep 3, 2026

jhrozek force-pushed the spiffe-integration-split3-6 branch from b758757 to f57d37e Compare September 3, 2026 10:06

jhrozek force-pushed the spiffe-integration-split3-7 branch from 5525559 to a0921d9 Compare September 3, 2026 10:08

github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Sep 3, 2026

jhrozek force-pushed the spiffe-integration-split3-6 branch from f57d37e to c5e6f6e Compare September 3, 2026 13:05

jhrozek requested a review from aponcedeleonch as a code owner September 3, 2026 13:05

jhrozek force-pushed the spiffe-integration-split3-7 branch from a0921d9 to 68125e7 Compare September 3, 2026 13:07

github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Sep 3, 2026

JAORMX requested changes Sep 3, 2026

View reviewed changes

jhrozek force-pushed the spiffe-integration-split3-6 branch from c5e6f6e to ff2fc59 Compare September 3, 2026 14:15

jhrozek force-pushed the spiffe-integration-split3-7 branch from 68125e7 to 34db653 Compare September 3, 2026 14:16

github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Sep 3, 2026

JAORMX requested changes Sep 3, 2026

View reviewed changes

jhrozek force-pushed the spiffe-integration-split3-6 branch from ff2fc59 to 3b10dc8 Compare September 3, 2026 16:24

jhrozek force-pushed the spiffe-integration-split3-7 branch from 34db653 to c31b0a1 Compare September 3, 2026 16:29

github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Sep 3, 2026

github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Sep 4, 2026

JAORMX requested changes Sep 4, 2026

View reviewed changes

jhrozek added 9 commits September 5, 2026 15:40

Fingerprint SPIFFE association identity

d2d311d

Signed-off-by: Jakub Hrozek <jakub@stacklok.com>

Report deprecated inbound grant fields

d634568

Operators need a visible migration signal before legacy grant fields can be removed safely. Record deprecated field paths during normalization and surface them as status conditions without changing the effective authorization policy. Refs #6200

jhrozek force-pushed the spiffe-integration-split3-6 branch from 66edc12 to e44e5c8 Compare September 5, 2026 13:48

jhrozek added 10 commits September 5, 2026 15:48

jhrozek force-pushed the spiffe-integration-split3-7 branch from 65adece to 4093b33 Compare September 5, 2026 13:50

github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Sep 5, 2026

JAORMX requested changes Sep 5, 2026

View reviewed changes

jhrozek force-pushed the spiffe-integration-split3-6 branch 3 times, most recently from 4d3eb12 to 0cd94a9 Compare September 6, 2026 21:03

Navigation Menu

Uh oh!

Document SPIFFE association declarations - #6501

Document SPIFFE association declarations#6501
jhrozek wants to merge 32 commits into
spiffe-integration-split3-6from
spiffe-integration-split3-7

jhrozek commented Sep 3, 2026

Uh oh!

codecov Bot commented Sep 3, 2026 •

edited

Loading

Uh oh!

JAORMX left a comment

Uh oh!

JAORMX left a comment

Uh oh!

JAORMX left a comment

Uh oh!

JAORMX left a comment

Uh oh!

JAORMX left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jhrozek commented Sep 3, 2026

Summary

Type of change

Test plan

API Compatibility

Does this introduce a user-facing change?

Special notes for reviewers

Uh oh!

codecov Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JAORMX left a comment

Choose a reason for hiding this comment

Uh oh!

JAORMX left a comment

Choose a reason for hiding this comment

Uh oh!

JAORMX left a comment

Choose a reason for hiding this comment

Uh oh!

JAORMX left a comment

Choose a reason for hiding this comment

Uh oh!

JAORMX left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 3, 2026 •

edited

Loading