crossplane-contrib / crossplane-contrib/crossplane-diff

refactor(validate): split rendering out of schema_validator.go into renderer/validation subpackage

Open
#349 1 comment 0 reactions 0 assignees View on GitHub
stale
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.