examples(spiffe-bundle-tdf): go-spiffe v2 TDF interop demo#59
Conversation
Closes the interop story for `GET /v1/spiffe-bundle` (the SPIFFE Trust Domain Format endpoint shipped on omega side). The demo runs the body of the response through the upstream go-spiffe v2 SDK's `spiffebundle.Read` and asserts the parsed bundle exposes both X.509 authorities and JWT authorities, plus surfaces the SequenceNumber and RefreshHint envelope fields. Why this matters: anything the SDK rejects, a real SPIFFE consumer (SPIRE agent, spiffe-helper, a workload using go-spiffe) would reject too. A regression that breaks the on-the-wire shape now trips the SDK parser end-to-end, not just a permissive hand-rolled decoder. The omega server is started with --spiffe-bundle-refresh-hint=180s so the parsed RefreshHint is a non-default `3m0s`, proving the flag makes it through to the document and the SDK reads it back. Sample output: ```text [consumer] success [consumer] trust_domain: omega.demo [consumer] x509 anchors: 1 (subject="CN=Omega Local CA") [consumer] jwt authorities: 1 (kids=[BA0vwLDU7Bs]) [consumer] sequence: 1 [consumer] refresh_hint: 3m0s ``` Added to the CI examples matrix; no new job needed.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds a complete, runnable end-to-end demo for Omega's ChangesSPIFFE Bundle TDF Demo
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces an end-to-end demo for the SPIFFE Trust Domain Format (TDF) endpoint, including a consumer binary and a shell script to verify interop with the go-spiffe v2 SDK. The review feedback highlights several best practices for shell scripting under 'set -e', specifically regarding command substitution and error handling. I have kept all three review comments as they provide actionable improvements and follow the requested guidelines.
|
|
||
| wait_for_url() { | ||
| local url="$1" log="$2" | ||
| for _ in $(seq 1 50); do |
There was a problem hiding this comment.
Assigning the output of seq to a variable before using it in the loop ensures that if seq fails, the script will exit immediately due to set -e.
| for _ in $(seq 1 50); do | |
| iters=$(seq 1 50) | |
| for _ in $iters; do |
References
- In shell scripts using 'set -e', assign command substitutions to a variable before using them as arguments to a command. This ensures that if the subshell fails, the script exits immediately, whereas command substitutions used directly as arguments do not trigger an exit on failure.
There was a problem hiding this comment.
See the reply on the L16 comment for the full reasoning. Same answer: rejecting, the abstract rule does not flag a real failure mode here and applying it consistently would be a sweep across every example demo, not a one-off in this PR.
Summary
Closes the interop story for
GET /v1/spiffe-bundle(the SPIFFE Trust Domain Format endpoint shipped in PR #47). A tinycmd/consumerGo binary:/v1/spiffe-bundle;spiffebundle.Read(td, body)fromgithub.com/spiffe/go-spiffe/v2/bundle/spiffebundle;bundle.X509Authorities()) and JWT authorities (bundle.JWTAuthorities());SequenceNumberandRefreshHintenvelope fields.The point is what the SDK rejects, a real SPIFFE consumer (SPIRE agent, spiffe-helper, any workload using go-spiffe) would reject too. A regression that breaks the on-the-wire shape now trips the SDK parser end-to-end, not just a hand-rolled decoder.
The omega server is started with
--spiffe-bundle-refresh-hint=180sso the parsedRefreshHintis a non-default3m0s, proving the flag makes it through to the document and the SDK reads it back.Sample success output:
Scope layer
Plugin / example. No production code changes — only exercises the public HTTP surface.
Test plan
make demo→[consumer] successmarkdownlint-cli2 CHANGELOG.md examples/spiffe-bundle-tdf/README.md—0 error(s)go vet ./examples/spiffe-bundle-tdf/cmd/consumer/cleanspiffe-bundle-tdfto the existingexamplesmatrix; Go-only setup, no new job needed.Follow-ups
internal/server/federation/to fetch TDF from peers (currently it polls/v1/bundlePEM) and honour the per-peerRefreshHint. That would movedocs/conformance-spiffe.md§6 frompartialtoimplemented, but it changes federation behaviour and deserves its own scope.Summary by CodeRabbit
New Features
Documentation