Skip to content

feat(reconcile): validate every document before applying anything#1776

Merged
rohilsurana merged 1 commit into
mainfrom
feat/reconcile-validate-before-apply
Jul 20, 2026
Merged

feat(reconcile): validate every document before applying anything#1776
rohilsurana merged 1 commit into
mainfrom
feat/reconcile-validate-before-apply

Conversation

@rohilsurana

Copy link
Copy Markdown
Member

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

  • Reconciler interface gains Validate(spec []byte) error. Run calls it for every parsed
    document before dispatching the first Reconcile.
  • Each kind implements Validate: strict decode plus its server-free entry checks
    (PlatformUser, Permission, Role, Preference). The checks already existed inside each diff;
    Validate runs the same ones up front. Server-dependent checks stay in the diff, where they
    belong (an unknown preference name needs the trait list, unaccounted/ambiguity need the live
    state), and still fail the plan there.
  • A document that has content but no kind is now an error, not a silent skip. A blank
    trailing --- 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 Validate method so it satisfies the interface after this lands; that goes on its own
branch.

Testing

  • New framework tests: a document that fails validation blocks every apply; a content-bearing
    document with no kind is rejected; a blank trailing document is still skipped.
  • go build, go test ./internal/reconcile/... ./cmd/..., go vet, gofmt, and
    golangci-lint all pass.

Note

This is the first step of adopting the rules v2 simplification. RFC 0001 should be updated to
describe the Validate step and the whole-file-first guarantee alongside this change.

@vercel

vercel Bot commented Jul 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
frontier Ready Ready Preview, Comment Jul 20, 2026 6:05am

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e87c4a4a-4ead-4571-99c1-3dcf7c9c8750

📥 Commits

Reviewing files that changed from the base of the PR and between cfe2cf3 and 2076c2d.

📒 Files selected for processing (6)
  • internal/reconcile/permission_reconciler.go
  • internal/reconcile/platformuser_reconciler.go
  • internal/reconcile/preference_reconciler.go
  • internal/reconcile/reconcile.go
  • internal/reconcile/reconcile_test.go
  • internal/reconcile/role_reconciler.go

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added preflight validation for permission, platform user, preference, and role specifications.
    • Validation now runs before any changes are applied, preventing partial updates when errors are found.
    • Added checks for duplicate names or slugs, invalid entries, empty preference names, and conflicting permission definitions.
  • Bug Fixes

    • Rejects documents containing content without a specified kind.
    • Continues to ignore truly blank YAML documents.

Walkthrough

Changes

The reconciliation framework now performs a validation pass before applying documents. Resource reconcilers validate decoded specs locally, while document parsing rejects content without a kind and skips blank separators.

Reconciler preflight validation

Layer / File(s) Summary
Parsing and preflight execution
internal/reconcile/reconcile.go
The reconciler contract adds Validate; blank documents remain skippable, content without kind fails, and Run validates all documents before reconciliation.
Resource-specific spec validation
internal/reconcile/*_reconciler.go
Permission, platform user, preference, and role specs receive local validation for malformed entries, duplicates, empty names, and conflicting definitions.
Validation behavior coverage
internal/reconcile/reconcile_test.go
Tests verify that validation failures prevent all reconciliation and that missing-kind and trailing-blank-document cases are handled correctly.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • raystack/frontier#1720: Both changes update the reconciliation framework and Reconciler contract.
  • raystack/frontier#1737: This change adds validation methods for permission and role reconciliation and integrates preflight validation.

Suggested reviewers: whoabhisheksah


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 29720684111

Coverage increased (+0.04%) to 46.146%

Details

  • Coverage increased (+0.04%) from the base build.
  • Patch coverage: 30 uncovered changes across 5 files (50 of 80 lines covered, 62.5%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
internal/reconcile/permission_reconciler.go 20 10 50.0%
internal/reconcile/preference_reconciler.go 15 9 60.0%
internal/reconcile/role_reconciler.go 16 10 62.5%
internal/reconcile/platformuser_reconciler.go 10 6 60.0%
internal/reconcile/reconcile.go 19 15 78.95%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 38400
Covered Lines: 17720
Line Coverage: 46.15%
Coverage Strength: 13.32 hits per line

💛 - Coveralls

@rohilsurana
rohilsurana marked this pull request as ready for review July 20, 2026 15:02
@rohilsurana
rohilsurana merged commit b755319 into main Jul 20, 2026
8 checks passed
@rohilsurana
rohilsurana deleted the feat/reconcile-validate-before-apply branch July 20, 2026 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants