feat(reconcile): validate every document before applying anything#1776
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe reconciliation framework now performs a validation pass before applying documents. Resource reconcilers validate decoded specs locally, while document parsing rejects content without a Reconciler preflight validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 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 |
Coverage Report for CI Build 29720684111Coverage increased (+0.04%) to 46.146%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
What
Rule 3 from the proposed rules v2: nothing applies until the whole file passes. Adds a
Validate(spec)step so a bad entry in a later document stops the run before the first change,instead of being found only when that document is dispatched (after earlier documents already
applied).
Changes
Reconcilerinterface gainsValidate(spec []byte) error.Runcalls it for every parseddocument before dispatching the first
Reconcile.Validate: strict decode plus its server-free entry checks(PlatformUser, Permission, Role, Preference). The checks already existed inside each diff;
Validateruns the same ones up front. Server-dependent checks stay in the diff, where theybelong (an unknown preference name needs the trait list, unaccounted/ambiguity need the live
state), and still fail the plan there.
kindis now an error, not a silent skip. A blanktrailing
---is still skipped.The diffs are unchanged, so every existing diff-level test still passes; validation simply also
runs before the first apply now.
Scope
Covers the four kinds on main. The Webhook kind (still in review, #1772) needs
the same
Validatemethod so it satisfies the interface after this lands; that goes on its ownbranch.
Testing
document with no kind is rejected; a blank trailing document is still skipped.
go build,go test ./internal/reconcile/... ./cmd/...,go vet,gofmt, andgolangci-lintall pass.Note
This is the first step of adopting the rules v2 simplification. RFC 0001 should be updated to
describe the
Validatestep and the whole-file-first guarantee alongside this change.