crossplane-contrib / crossplane-contrib/crossplane-diff
refactor(validate): split rendering out of schema_validator.go into renderer/validation subpackage
- Dominant language
- Go
- Stars
- 42
- Forks
- 15
- Avg merge
- 10h 26m
- Merged PRs (30d)
- 33
Description
## Context
Follow-up cleanup after #347 (the upstream validate-refactor) merges.
Today, `cmd/diff/diffprocessor/schema_validator.go` mixes validation logic (scope checks, CRD ensuring, defaulting, calling `pkg/validate.SchemaValidate`) with ~80 lines of pure formatting helpers — `formatValidationErrors`, `formatMissingSchemaBlock`, `formatInvalidBlock`, `formatErrorLine`, `renderBadValue`. The structured-output converter `validationFailuresFromResult` lives in a sibling file (`errors.go`).
This is the same business-logic-vs-presentation split we already do for diffs (`renderer/diff_renderer.go`, `renderer/structured_renderer.go`). Validation should follow the same pattern.
## Proposed shape
```
cmd/diff/renderer/validation/
text.go # Format(*pkgvalidate.ValidationResult) string
structured.go # ToFailures(*pkgvalidate.ValidationResult) []dt.ResourceValidationFailure
validation_test.go
```
- `schema_validator.go` calls `validation.Format` to build `SchemaValidationError.Message`.
- `errors.go` calls `validation.ToFailures` from `NewOutputError`.
- Wire types (`ResourceValidationFailure`, `FieldValidationError`) stay in `renderer/types/types.go` — they're already there.
## Constraint
The eager-vs-lazy timing of `SchemaValidationError.Message` stays unchanged. Go's `error` interface needs `.Error() string` synchronously, so the text rendering can't become as lazy as the diff renderer's. The **location** of the formatters moves; the **timing** does not. This is the one place the analog with the diff renderer diverges.
## Scope
- Pure code-move plus import updates
- No behavior change
- Estimated ~+200 lines of churn (new package, moved functions, moved tests, updated imports)
- All mechanical; reviewer cost should be low
## Why now (after #347)
#347 is already substantial — upstream API consumption, structured output exposure, integration test rework, dead-state sweep. Doing the split inside that PR would dilute its narrative. After #347 merges this becomes a focused refactor.
Contributor guide
Assessment
This issue has not been assessed yet.