Repository navigation
fix(authorization): approve the rendered surface per document so HelmRelease PRs stop conflicting - #4251
Conversation
…Release PRs stop conflicting The EKS authorization validator approved the whole rendered surface as one aggregate SHA-256 constant in main.go. Every PR that moved any selected document had to rewrite that one line, so any two such PRs conflicted by construction and each merge re-invalidated the other's approval (#3182). Replace the constant with an embedded ledger, approved-surface.txt: one `<apiVersion>|<kind>|<namespace>|<name> <sha256>` entry per selected document, sorted and separated by blank lines. The rendered surface must equal it as a multiset, which is exactly as strict as the aggregate (same entries, same multiplicity), but a change now touches only its own documents' lines. The blank separators keep approvals of neighbouring documents mergeable, and the merge queue validates the combined render. On a mismatch the validator prints the exact "-"/"+" lines to apply, so the approval-base mode and the HEAD^1 checkout that existed only to explain an aggregate mismatch are removed. The 3,225-line approval rationale that sat above the constant moves verbatim into approved-surface-history.md. The ledger was generated from the same render as the current approval and reproduces its aggregate (e5d7a55b…) exactly. Fixes #3182 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai review |
✅ Action performedReview finished.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used🪛 LanguageToolscripts/validate-eks-ci-role-policy/approved-surface.txt[grammar] ~327-~327: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) [grammar] ~379-~379: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) [grammar] ~389-~389: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) 📝 WalkthroughWalkthroughThe change adds a committed ledger of approved rendered Kubernetes documents, with tests for ledger parsing, comparison, and mismatch diagnostics. The CI workflow no longer fetches or checks out a base tree for authorization validation. It runs the tests and validator against the current checkout. Diagnostic tests now expect added ledger lines, and a HelmRelease comment refers to approved-surface entries. Priority: ⬆️ High Severity of issue fixed: High Merge Risk: ⚪ Minimal · up to The change moves the authorization approval check to per-document entries and validates the current checkout. No merge-blocking issue was identified in the supplied context. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The new gate still requires rendered authorization documents to match committed approvals, and no new bypass was established. Because this is a production authorization control, the initial approval migration and the policy governing who may approve ledger changes warrant design review. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Readiness at
|
Why
The production authorization check approves everything it guards with one single fingerprint. Every change that touches a guarded component had to rewrite that same line, so any two such changes collided and each merge forced the other to redo its approval. Chart and image updates queued behind one another.
What
The approval is now a list with one entry per guarded component, and the check stays exactly as strict, so two changes to different components edit different entries and merge cleanly. A failing check now names the exact entries to change, which removes the extra step that only existed to explain a failure. The existing approval history moves into its own file, unchanged.
Fixes #3182