Skip to content

Document SPIFFE association declarations - #6501

Open
jhrozek wants to merge 1 commit into
spiffe-integration-split3-6from
spiffe-integration-split3-7
Open

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

Conversation

@jhrozek

@jhrozek jhrozek commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

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.

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

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (spiffe-integration-split3-6@ff2fc59). Learn more about missing BASE report.

Additional details and impacted files
@@                      Coverage Diff                       @@
##             spiffe-integration-split3-6    #6501   +/-   ##
==============================================================
  Coverage                               ?   78.32%           
==============================================================
  Files                                  ?      775           
  Lines                                  ?    75794           
  Branches                               ?        0           
==============================================================
  Hits                                   ?    59363           
  Misses                                 ?    16426           
  Partials                               ?        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.

@jhrozek
jhrozek force-pushed the spiffe-integration-split3-6 branch from b758757 to f57d37e Compare September 3, 2026 10:06
@jhrozek
jhrozek force-pushed the spiffe-integration-split3-7 branch from 5525559 to a0921d9 Compare September 3, 2026 10:08
@github-actions 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
jhrozek force-pushed the spiffe-integration-split3-6 branch from f57d37e to c5e6f6e Compare September 3, 2026 13:05
@jhrozek
jhrozek force-pushed the spiffe-integration-split3-7 branch from a0921d9 to 68125e7 Compare September 3, 2026 13:07
@github-actions 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 JAORMX 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.

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 JAORMX 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.

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.

@jhrozek
jhrozek force-pushed the spiffe-integration-split3-6 branch from c5e6f6e to ff2fc59 Compare September 3, 2026 14:15
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>
@jhrozek
jhrozek force-pushed the spiffe-integration-split3-7 branch from 68125e7 to 34db653 Compare September 3, 2026 14:16
@github-actions 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 JAORMX 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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants