refactor(validate): inject Renderer via Kong's MapperValue interface

Reviewer pointed out that even with the polymorphic registry, the
OutputFormat string was still flowing through the command — we were
dispatching at every Render call site instead of resolving once at the
boundary. Subsequent passes worked through the implications: the
renderer types are stateless empty structs, so the map cache was
needless ceremony; the dispatch can collapse to a single fallible
boundary that takes a string-shaped format identifier and returns a
typed Renderer; and the natural place for that boundary is Kong itself,
which has first-class support for typed flag decoding.

Final shape:

* render: a single typed boundary.
  * OutputFormat is a defined string type with three named constants
    (OutputFormatText, OutputFormatJSON, OutputFormatYAML) so call
    sites use symbolic names rather than embedded "json"/"yaml"
    string literals.
  * RendererFor(OutputFormat) (Renderer, error) is the only public
    factory. Empty maps to text for ergonomics with zero-valued
    config; any other unrecognised value returns an error.
  * No format identifier or wrapper type is otherwise exposed;
    downstream code receives only the Renderer interface and works
    against the typed dependency.

* validate.Cmd: gained a private rendererFlag wrapper that implements
  Kong's MapperValue interface, decoding --output straight into a
  typed render.Renderer at parse time. Cmd.Output is now of type
  rendererFlag (which embeds render.Renderer), so Cmd.Run calls
  c.Output.Render(...) directly. AfterApply only sets the filesystem;
  the renderer is already resolved by the time it runs. The wrapper
  lives in this package, not in render, so the render package stays
  free of any kong dependency and remains importable from non-CLI
  consumers like crossplane-diff.
  * Kong's enum:"" tag doesn't apply to MapperValue-backed fields, so
    rendererFlag.Decode performs the validation itself; --help text
    enumerates the valid values for users.

* render tests: switched to OutputFormat constants throughout. The
  format-boundary test (TestRendererFor_FormatBoundary) covers the
  named constants, the empty-string-as-text ergonomics, and unknown
  rejection via OutputFormat("xml").

Smoke: `crossplane resource validate ... --output=json` emits valid
JSON; `--output=xml` is rejected by Kong with the wrapped Decode error
"--output: unknown output format: \"xml\"".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Author: jcogilvie

cmd/crossplane/pkg/validate/render/render.go

@@ -31,17 +31,30 @@ import (
 const (
 	errCannotMarshalJSON = "cannot marshal validation result as JSON"
 	errCannotMarshalYAML = "cannot marshal validation result as YAML"
+	errCannotWriteOutput = "cannot write validation result"
 	errUnknownFormat     = "unknown output format"
 )
 
 // Renderer writes a *ValidationResult to an io.Writer in some specific
-// encoding. New formats are added by implementing Renderer and registering
-// a value under an OutputFormat in renderers below.
+// encoding. Implementations are obtained via RendererFor; the package
+// exposes no other constructors so the supported set is fully closed.
 type Renderer interface {
-	Render(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOptions) error
+	Render(result *pkgvalidate.ValidationResult, w io.Writer, opts Options) error
 }
 
-// OutputFormat names a Renderer.
+// Options configures how a validation result is rendered.
+type Options struct {
+	// SkipSuccessResults suppresses per-resource success lines in text output.
+	// It has no effect on JSON or YAML output, where success entries are
+	// always part of the structured payload.
+	SkipSuccessResults bool
+}
+
+// OutputFormat names a Renderer. It is a defined string type rather than
+// a bare string so that call sites can use the symbolic constants below
+// (OutputFormatText, OutputFormatJSON, OutputFormatYAML) instead of
+// embedding raw "text"/"json"/"yaml" literals, and so the compiler
+// catches accidental cross-wiring of unrelated string flags.
 type OutputFormat string
 
 // OutputFormat values.
@@ -55,78 +68,48 @@ const (
 	OutputFormatYAML OutputFormat = "yaml"
 )
 
-// RenderOptions configures how a validation result is rendered.
-type RenderOptions struct {
-	// SkipSuccessResults suppresses per-resource success lines in text output.
-	// It has no effect on JSON or YAML output, where success entries are
-	// always part of the structured payload.
-	SkipSuccessResults bool
-}
-
-// renderers is the polymorphic registry: each known OutputFormat names a
-// Renderer value. Adding a new format means adding a type that implements
-// Renderer and one entry below — nothing else in this package needs to
-// change.
-var renderers = map[OutputFormat]Renderer{
-	OutputFormatText: textRenderer{},
-	OutputFormatJSON: jsonRenderer{},
-	OutputFormatYAML: yamlRenderer{},
-}
-
-// Renderer returns the Renderer registered for f, or an error if f is not a
-// known format. The empty string is treated as text, so callers passing the
-// zero value (e.g. struct defaults) get sensible behaviour.
-func (f OutputFormat) Renderer() (Renderer, error) {
-	if f == "" {
-		f = OutputFormatText
-	}
-	r, ok := renderers[f]
-	if !ok {
-		return nil, errors.Errorf("%s: %q", errUnknownFormat, f)
-	}
-	return r, nil
-}
-
-// Render dispatches to the Renderer registered for f.
-func (f OutputFormat) Render(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOptions) error {
-	r, err := f.Renderer()
-	if err != nil {
-		return err
+// RendererFor returns the Renderer for the given format. The empty
+// string is accepted as OutputFormatText for ergonomics with
+// zero-valued config; any other unrecognised value returns an error.
+// This is the one and only boundary between a format identifier and
+// the typed Renderer dependency that downstream code receives.
+func RendererFor(format OutputFormat) (Renderer, error) {
+	switch format {
+	case OutputFormatText, "":
+		return textRenderer{}, nil
+	case OutputFormatJSON:
+		return jsonRenderer{}, nil
+	case OutputFormatYAML:
+		return yamlRenderer{}, nil
+	default:
+		return nil, errors.Errorf("%s: %q", errUnknownFormat, format)
 	}
-	return r.Render(result, w, opts)
-}
-
-// RenderValidationResult writes the validation result to w in the requested
-// format. It is a free-function shim around OutputFormat.Render kept for
-// callers that prefer the procedural style.
-func RenderValidationResult(result *pkgvalidate.ValidationResult, format OutputFormat, w io.Writer, opts RenderOptions) error {
-	return format.Render(result, w, opts)
 }
 
 // jsonRenderer emits indented JSON with a trailing newline.
 type jsonRenderer struct{}
 
 // Render implements Renderer.
-func (jsonRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, _ RenderOptions) error {
+func (jsonRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, _ Options) error {
 	out, err := json.MarshalIndent(result, "", "  ")
 	if err != nil {
 		return errors.Wrap(err, errCannotMarshalJSON)
 	}
 	_, err = fmt.Fprintln(w, string(out))
-	return err
+	return errors.Wrap(err, errCannotWriteOutput)
 }
 
 // yamlRenderer emits sigs.k8s.io/yaml output.
 type yamlRenderer struct{}
 
 // Render implements Renderer.
-func (yamlRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, _ RenderOptions) error {
+func (yamlRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, _ Options) error {
 	out, err := yaml.Marshal(result)
 	if err != nil {
 		return errors.Wrap(err, errCannotMarshalYAML)
 	}
 	_, err = fmt.Fprint(w, string(out))
-	return err
+	return errors.Wrap(err, errCannotWriteOutput)
 }
 
 // textRenderer emits the human-readable text format that the validate CLI
@@ -144,7 +127,7 @@ func (yamlRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, _
 type textRenderer struct{}
 
 // Render implements Renderer.
-func (textRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOptions) error {
+func (textRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, opts Options) error {
 	for _, r := range result.Resources {
 		gvk := fmt.Sprintf("%s, Kind=%s", r.APIVersion, r.Kind)
 		switch r.Status {
@@ -184,8 +167,14 @@ func writeTextErrorLine(w io.Writer, gvk, name string, e pkgvalidate.FieldValida
 	case pkgvalidate.FieldErrorTypeCEL:
 		_, err := fmt.Fprintf(w, "[x] CEL validation error %s, %s : %s\n", gvk, name, e.Message)
 		return err
-	default:
+	case pkgvalidate.FieldErrorTypeSchema, pkgvalidate.FieldErrorTypeUnknownField:
 		_, err := fmt.Fprintf(w, "[x] schema validation error %s, %s : %s\n", gvk, name, e.Message)
 		return err
 	}
+	// Unreachable in practice: FieldErrorType is a closed enum populated
+	// only by SchemaValidate. Treat any future-added kind as a schema-class
+	// failure so output stays useful if a new error type is added without
+	// updating this switch.
+	_, err := fmt.Fprintf(w, "[x] schema validation error %s, %s : %s\n", gvk, name, e.Message)
+	return err
 }

cmd/crossplane/pkg/validate/render/render_test.go

@@ -94,13 +94,25 @@ func defaultingFixture() *pkgvalidate.ValidationResult {
 	}
 }
 
-// renderTextLines runs Render with the given format and options and returns
-// the non-empty lines of the resulting output. It centralises the call so
+// rendererForT resolves a Renderer through the public RendererFor API
+// and t.Fatals on error. Used by the rendering tests below; the parse
+// boundary itself has its own dedicated test.
+func rendererForT(t *testing.T, format OutputFormat) Renderer {
+	t.Helper()
+	r, err := RendererFor(format)
+	if err != nil {
+		t.Fatalf("RendererFor(%q): %v", format, err)
+	}
+	return r
+}
+
+// renderTextLines runs the named renderer against in, returning the
+// non-empty lines of the resulting output. It centralises the call so
 // individual cases can focus on assertions.
-func renderTextLines(t *testing.T, in *pkgvalidate.ValidationResult, format OutputFormat, opts RenderOptions) []string {
+func renderTextLines(t *testing.T, in *pkgvalidate.ValidationResult, format OutputFormat, opts Options) []string {
 	t.Helper()
 	var buf bytes.Buffer
-	if err := format.Render(in, &buf, opts); err != nil {
+	if err := rendererForT(t, format).Render(in, &buf, opts); err != nil {
 		t.Fatalf("Render() unexpected error: %v", err)
 	}
 	raw := strings.TrimRight(buf.String(), "\n")
@@ -110,18 +122,29 @@ func renderTextLines(t *testing.T, in *pkgvalidate.ValidationResult, format Outp
 	return strings.Split(raw, "\n")
 }
 
+// renderBytes runs the named renderer and returns its output as a byte
+// slice. Used by the structural JSON and YAML tests below.
+func renderBytes(t *testing.T, in *pkgvalidate.ValidationResult, format OutputFormat) []byte {
+	t.Helper()
+	var buf bytes.Buffer
+	if err := rendererForT(t, format).Render(in, &buf, Options{}); err != nil {
+		t.Fatalf("Render() unexpected error: %v", err)
+	}
+	return buf.Bytes()
+}
+
 // summaryLine builds the trailing summary line for the given result.
 func summaryLine(r *pkgvalidate.ValidationResult) string {
 	return fmt.Sprintf("Total %d resources: %d missing schemas, %d success cases, %d failure cases",
 		r.Summary.Total, r.Summary.MissingSchemas, r.Summary.Valid, r.Summary.Invalid)
 }
 
-func TestRenderValidationResult_Text(t *testing.T) {
+func TestRendererFor_Text(t *testing.T) {
 	cases := map[string]struct {
 		in           *pkgvalidate.ValidationResult
 		format       OutputFormat
-		opts         RenderOptions
-		wantLineSubs []string // every entry must appear as a substring of some output line, in order
+		opts         Options
+		wantLineSubs []string // every entry must appear as a substring of the output line at the same index
 	}{
 		"WithSuccess": {
 			in:     fixture(),
@@ -136,18 +159,8 @@ func TestRenderValidationResult_Text(t *testing.T) {
 		"SkipSuccess": {
 			in:     fixture(),
 			format: OutputFormatText,
-			opts:   RenderOptions{SkipSuccessResults: true},
-			wantLineSubs: []string{
-				"[x] schema validation error test.org/v1alpha1, Kind=Test, bad",
-				"[!] could not find CRD/XRD for: other.org/v1, Kind=Unknown",
-				summaryLine(fixture()),
-			},
-		},
-		"EmptyFormatActsAsText": {
-			in:     fixture(),
-			format: OutputFormat(""),
+			opts:   Options{SkipSuccessResults: true},
 			wantLineSubs: []string{
-				"[✓] test.org/v1alpha1, Kind=Test, ok",
 				"[x] schema validation error test.org/v1alpha1, Kind=Test, bad",
 				"[!] could not find CRD/XRD for: other.org/v1, Kind=Unknown",
 				summaryLine(fixture()),
@@ -179,43 +192,60 @@ func TestRenderValidationResult_Text(t *testing.T) {
 	}
 }
 
-func TestRenderValidationResult_JSON(t *testing.T) {
+func TestRendererFor_JSON(t *testing.T) {
 	in := fixture()
-	var buf bytes.Buffer
-	if err := RenderValidationResult(in, OutputFormatJSON, &buf, RenderOptions{}); err != nil {
-		t.Fatalf("RenderValidationResult(JSON) err = %v", err)
-	}
+	out := renderBytes(t, in, OutputFormatJSON)
 	var got pkgvalidate.ValidationResult
-	if err := json.Unmarshal(buf.Bytes(), &got); err != nil {
-		t.Fatalf("json.Unmarshal() err = %v; output was:\n%s", err, buf.String())
+	if err := json.Unmarshal(out, &got); err != nil {
+		t.Fatalf("json.Unmarshal() err = %v; output was:\n%s", err, string(out))
 	}
 	if diff := cmp.Diff(*in, got); diff != "" {
 		t.Errorf("JSON round-trip mismatch (-want +got):\n%s", diff)
 	}
 }
 
-func TestRenderValidationResult_YAML(t *testing.T) {
+func TestRendererFor_YAML(t *testing.T) {
 	in := fixture()
-	var buf bytes.Buffer
-	if err := RenderValidationResult(in, OutputFormatYAML, &buf, RenderOptions{}); err != nil {
-		t.Fatalf("RenderValidationResult(YAML) err = %v", err)
-	}
+	out := renderBytes(t, in, OutputFormatYAML)
 	var got pkgvalidate.ValidationResult
-	if err := yaml.Unmarshal(buf.Bytes(), &got); err != nil {
-		t.Fatalf("yaml.Unmarshal() err = %v; output was:\n%s", err, buf.String())
+	if err := yaml.Unmarshal(out, &got); err != nil {
+		t.Fatalf("yaml.Unmarshal() err = %v; output was:\n%s", err, string(out))
 	}
 	if diff := cmp.Diff(*in, got); diff != "" {
 		t.Errorf("YAML round-trip mismatch (-want +got):\n%s", diff)
 	}
 }
 
-func TestRenderValidationResult_Unknown(t *testing.T) {
-	var buf bytes.Buffer
-	err := RenderValidationResult(fixture(), OutputFormat("bogus"), &buf, RenderOptions{})
-	if err == nil {
-		t.Fatal("RenderValidationResult(bogus) = nil; want non-nil error")
+// TestRendererFor_FormatBoundary covers the only failable behaviour of
+// RendererFor: the OutputFormat-to-Renderer mapping. Empty maps to the
+// text renderer; an unrecognised value returns a non-nil error.
+func TestRendererFor_FormatBoundary(t *testing.T) {
+	cases := map[string]struct {
+		in       OutputFormat
+		wantType Renderer
+		wantErr  bool
+	}{
+		"Text":         {in: OutputFormatText, wantType: textRenderer{}},
+		"JSON":         {in: OutputFormatJSON, wantType: jsonRenderer{}},
+		"YAML":         {in: OutputFormatYAML, wantType: yamlRenderer{}},
+		"EmptyIsText":  {in: "", wantType: textRenderer{}},
+		"UnknownFails": {in: OutputFormat("xml"), wantErr: true},
 	}
-	if buf.Len() != 0 {
-		t.Errorf("Unknown format wrote %d bytes; want 0 (content: %q)", buf.Len(), buf.String())
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			got, err := RendererFor(tc.in)
+			if (err != nil) != tc.wantErr {
+				t.Fatalf("RendererFor(%q) err = %v, wantErr = %v", tc.in, err, tc.wantErr)
+			}
+			if tc.wantErr {
+				if got != nil {
+					t.Errorf("RendererFor(%q) returned non-nil Renderer %v on error", tc.in, got)
+				}
+				return
+			}
+			if fmt.Sprintf("%T", got) != fmt.Sprintf("%T", tc.wantType) {
+				t.Errorf("RendererFor(%q) = %T, want %T", tc.in, got, tc.wantType)
+			}
+		})
 	}
 }

cmd/crossplane/pkg/validate/types.go

@@ -57,8 +57,8 @@ const (
 
 // FieldValidationError represents a single field-level validation error.
 type FieldValidationError struct {
-	// Type categorizes the error (e.g. "schema", "cel", "unknownField", "defaulting").
-	Type string `json:"type"`
+	// Type categorizes the error.
+	Type FieldErrorType `json:"type"`
 	// Field is the path to the invalid field (e.g. "spec.forProvider.region").
 	Field string `json:"field,omitempty"`
 	// Message is a human-readable description of the error.
@@ -67,14 +67,19 @@ type FieldValidationError struct {
 	Value any `json:"value,omitempty"`
 }
 
-// FieldErrorType categorizes the kind of validation error.
+// FieldErrorType categorizes the kind of validation error a
+// FieldValidationError describes. The supported set is closed; producers
+// and consumers should use the named constants below.
+type FieldErrorType string
+
+// FieldErrorType values.
 const (
 	// FieldErrorTypeSchema indicates a schema validation error from OpenAPI validation.
-	FieldErrorTypeSchema = "schema"
+	FieldErrorTypeSchema FieldErrorType = "schema"
 	// FieldErrorTypeCEL indicates a CEL rule validation error.
-	FieldErrorTypeCEL = "cel"
+	FieldErrorTypeCEL FieldErrorType = "cel"
 	// FieldErrorTypeUnknownField indicates an unknown field was present in the resource.
-	FieldErrorTypeUnknownField = "unknownField"
+	FieldErrorTypeUnknownField FieldErrorType = "unknownField"
 	// FieldErrorTypeDefaulting indicates defaults could not be applied to the resource.
-	FieldErrorTypeDefaulting = "defaulting"
+	FieldErrorTypeDefaulting FieldErrorType = "defaulting"
 )

cmd/crossplane/pkg/validate/unknown_fields.go

@@ -31,9 +31,8 @@ func validateUnknownFields(mr map[string]any, sch *schema.Structural) field.Erro
 	opts := schema.UnknownFieldPathOptions{
 		TrackUnknownFieldPaths: true, // to get the list of pruned unknown fields
 	}
-	errs := field.ErrorList{}
-
 	uf := pruning.PruneWithOptions(mr, sch, true, opts)
+	errs := make(field.ErrorList, 0, len(uf))
 	for _, f := range uf {
 		strPath := strings.Split(f, ".")
 		child := strPath[len(strPath)-1]