From 15da6fbd6dd5e7d29ad45225d01fdb0a92053968 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Sun, 31 May 2026 15:32:23 -0400 Subject: [PATCH 1/4] feat(validate): structured results and --output json|yaml flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the validate command into a processing layer that returns a structured *ValidationResult and a rendering layer that writes it as text, JSON, or YAML. The processing API (SchemaValidate, ResultError, types) now lives at cmd/crossplane/pkg/validate/ so programmatic consumers — for example crossplane-diff — can call it directly without parsing free-form text output. The renderer at cmd/crossplane/pkg/validate/render/ is a sibling subpackage so consumers can depend on validation data without pulling in YAML/JSON encoding deps. The CLI gains a -o/--output text|json|yaml flag (default text). Default text output is byte-identical to the historical format. JSON and YAML emit the full ValidationResult including per-resource status and field-level error details (type, field path, message, bad value). The previous SchemaValidation(...) entry point is removed; callers move to SchemaValidate + RenderValidationResult + ResultError. errWriteOutput moves to cmd.go alongside its remaining caller in manager.go. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jonathan Ogilvie --- .../pkg/validate/apply_defaults_test.go | 311 +++++++++ cmd/crossplane/pkg/validate/doc.go | 24 + cmd/crossplane/pkg/validate/render/doc.go | 21 + cmd/crossplane/pkg/validate/render/render.go | 134 ++++ .../pkg/validate/render/render_test.go | 133 ++++ cmd/crossplane/pkg/validate/types.go | 80 +++ .../{ => pkg}/validate/unknown_fields.go | 9 +- .../pkg/validate/unknown_fields_test.go | 115 ++++ cmd/crossplane/{ => pkg}/validate/validate.go | 191 +++--- cmd/crossplane/pkg/validate/validate_test.go | 387 +++++++++++ cmd/crossplane/validate/cmd.go | 33 +- cmd/crossplane/validate/cmd_test.go | 203 ++++++ cmd/crossplane/validate/help/validate.md | 8 + cmd/crossplane/validate/validate_test.go | 640 ------------------ 14 files changed, 1561 insertions(+), 728 deletions(-) create mode 100644 cmd/crossplane/pkg/validate/apply_defaults_test.go create mode 100644 cmd/crossplane/pkg/validate/doc.go create mode 100644 cmd/crossplane/pkg/validate/render/doc.go create mode 100644 cmd/crossplane/pkg/validate/render/render.go create mode 100644 cmd/crossplane/pkg/validate/render/render_test.go create mode 100644 cmd/crossplane/pkg/validate/types.go rename cmd/crossplane/{ => pkg}/validate/unknown_fields.go (80%) create mode 100644 cmd/crossplane/pkg/validate/unknown_fields_test.go rename cmd/crossplane/{ => pkg}/validate/validate.go (59%) create mode 100644 cmd/crossplane/pkg/validate/validate_test.go create mode 100644 cmd/crossplane/validate/cmd_test.go diff --git a/cmd/crossplane/pkg/validate/apply_defaults_test.go b/cmd/crossplane/pkg/validate/apply_defaults_test.go new file mode 100644 index 0000000..437ebe4 --- /dev/null +++ b/cmd/crossplane/pkg/validate/apply_defaults_test.go @@ -0,0 +1,311 @@ +/* +Copyright 2024 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validate + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + runtimeschema "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/crossplane/crossplane-runtime/v2/pkg/test" +) + +func TestApplyDefaults(t *testing.T) { + type args struct { + resource *unstructured.Unstructured + gvk runtimeschema.GroupVersionKind + crds []*extv1.CustomResourceDefinition + } + + type want struct { + resource *unstructured.Unstructured + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NoCRDFound": { + reason: "Should return nil when no matching CRD is found (skip defaulting)", + args: args{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{"replicas": 3}, + }, + }, + gvk: runtimeschema.GroupVersionKind{Group: "test.org", Version: "v1alpha1", Kind: "Test"}, + crds: []*extv1.CustomResourceDefinition{}, + }, + want: want{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{"replicas": 3}, + }, + }, + }, + }, + "ApplySimpleDefault": { + reason: "Should apply default value to missing property", + args: args{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{"replicas": 3}, + }, + }, + gvk: runtimeschema.GroupVersionKind{Group: "test.org", Version: "v1alpha1", Kind: "Test"}, + crds: []*extv1.CustomResourceDefinition{{ + Spec: extv1.CustomResourceDefinitionSpec{ + Group: "test.org", + Names: extv1.CustomResourceDefinitionNames{Kind: "Test"}, + Versions: []extv1.CustomResourceDefinitionVersion{{ + Name: "v1alpha1", + Schema: &extv1.CustomResourceValidation{ + OpenAPIV3Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "spec": { + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "replicas": {Type: "integer"}, + "deletionPolicy": { + Type: "string", + Default: &extv1.JSON{Raw: []byte(`"Delete"`)}, + }, + }, + }, + }, + }, + }, + }}, + }, + }}, + }, + want: want{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{ + "replicas": 3, + "deletionPolicy": "Delete", + }, + }, + }, + }, + }, + "DoNotOverrideExisting": { + reason: "Should not override existing values with defaults", + args: args{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{ + "replicas": 3, + "deletionPolicy": "Retain", + }, + }, + }, + gvk: runtimeschema.GroupVersionKind{Group: "test.org", Version: "v1alpha1", Kind: "Test"}, + crds: []*extv1.CustomResourceDefinition{{ + Spec: extv1.CustomResourceDefinitionSpec{ + Group: "test.org", + Names: extv1.CustomResourceDefinitionNames{Kind: "Test"}, + Versions: []extv1.CustomResourceDefinitionVersion{{ + Name: "v1alpha1", + Schema: &extv1.CustomResourceValidation{ + OpenAPIV3Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "spec": { + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "replicas": {Type: "integer"}, + "deletionPolicy": { + Type: "string", + Default: &extv1.JSON{Raw: []byte(`"Delete"`)}, + }, + }, + }, + }, + }, + }, + }}, + }, + }}, + }, + want: want{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{ + "replicas": 3, + "deletionPolicy": "Retain", + }, + }, + }, + }, + }, + "NestedDefaults": { + reason: "Should apply defaults to nested objects", + args: args{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{ + "forProvider": map[string]any{"region": "us-east-1"}, + }, + }, + }, + gvk: runtimeschema.GroupVersionKind{Group: "test.org", Version: "v1alpha1", Kind: "Test"}, + crds: []*extv1.CustomResourceDefinition{{ + Spec: extv1.CustomResourceDefinitionSpec{ + Group: "test.org", + Names: extv1.CustomResourceDefinitionNames{Kind: "Test"}, + Versions: []extv1.CustomResourceDefinitionVersion{{ + Name: "v1alpha1", + Schema: &extv1.CustomResourceValidation{ + OpenAPIV3Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "spec": { + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "forProvider": { + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "region": {Type: "string"}, + "instanceType": { + Type: "string", + Default: &extv1.JSON{Raw: []byte(`"t3.micro"`)}, + }, + }, + }, + "deletionPolicy": { + Type: "string", + Default: &extv1.JSON{Raw: []byte(`"Delete"`)}, + }, + }, + }, + }, + }, + }, + }}, + }, + }}, + }, + want: want{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{ + "forProvider": map[string]any{ + "region": "us-east-1", + "instanceType": "t3.micro", + }, + "deletionPolicy": "Delete", + }, + }, + }, + }, + }, + "ComplexDefaults": { + reason: "Should apply complex default values (objects, arrays)", + args: args{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{"name": "test"}, + }, + }, + gvk: runtimeschema.GroupVersionKind{Group: "test.org", Version: "v1alpha1", Kind: "Test"}, + crds: []*extv1.CustomResourceDefinition{{ + Spec: extv1.CustomResourceDefinitionSpec{ + Group: "test.org", + Names: extv1.CustomResourceDefinitionNames{Kind: "Test"}, + Versions: []extv1.CustomResourceDefinitionVersion{{ + Name: "v1alpha1", + Schema: &extv1.CustomResourceValidation{ + OpenAPIV3Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "spec": { + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "name": {Type: "string"}, + "metadata": { + Type: "object", + Default: &extv1.JSON{Raw: []byte(`{"labels":{"app":"default-app"}}`)}, + }, + "tags": { + Type: "array", + Default: &extv1.JSON{Raw: []byte(`["default","tag"]`)}, + }, + }, + }, + }, + }, + }, + }}, + }, + }}, + }, + want: want{ + resource: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "spec": map[string]any{ + "name": "test", + "metadata": map[string]any{ + "labels": map[string]any{"app": "default-app"}, + }, + "tags": []any{"default", "tag"}, + }, + }, + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := applyDefaults(tc.args.resource, tc.args.gvk, tc.args.crds) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("%s\napplyDefaults(...): -want err, +got err:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.resource, tc.args.resource); diff != "" { + t.Errorf("%s\napplyDefaults(...): -want resource, +got resource:\n%s", tc.reason, diff) + } + }) + } +} diff --git a/cmd/crossplane/pkg/validate/doc.go b/cmd/crossplane/pkg/validate/doc.go new file mode 100644 index 0000000..5a092de --- /dev/null +++ b/cmd/crossplane/pkg/validate/doc.go @@ -0,0 +1,24 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package validate exposes the CLI's validation logic as a programmatic, +// I/O-free API. SchemaValidate returns a structured *ValidationResult that +// callers can inspect directly or hand to the sibling render package for +// human or machine-readable output. +// +// Downstream consumers (for example crossplane-diff) should pin a specific +// crossplane/cli version. Type and function signatures may evolve. +package validate diff --git a/cmd/crossplane/pkg/validate/render/doc.go b/cmd/crossplane/pkg/validate/render/doc.go new file mode 100644 index 0000000..e3aadd2 --- /dev/null +++ b/cmd/crossplane/pkg/validate/render/doc.go @@ -0,0 +1,21 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package render writes *validate.ValidationResult values in text, JSON or +// YAML form. It is intentionally separate from the parent validate package so +// that programmatic consumers can depend on validation data without pulling +// in YAML/JSON encoding dependencies. +package render diff --git a/cmd/crossplane/pkg/validate/render/render.go b/cmd/crossplane/pkg/validate/render/render.go new file mode 100644 index 0000000..4360b31 --- /dev/null +++ b/cmd/crossplane/pkg/validate/render/render.go @@ -0,0 +1,134 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package render + +import ( + "encoding/json" + "fmt" + "io" + + "sigs.k8s.io/yaml" + + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" + + pkgvalidate "github.com/crossplane/cli/v2/cmd/crossplane/pkg/validate" +) + +const ( + errCannotMarshalJSON = "cannot marshal validation result as JSON" + errCannotMarshalYAML = "cannot marshal validation result as YAML" + errUnknownFormat = "unknown output format" +) + +// OutputFormat specifies how validation results should be rendered. +type OutputFormat string + +// OutputFormat values. +const ( + // OutputFormatText renders results in human-readable text format with + // [x], [!], [✓] markers. + OutputFormatText OutputFormat = "text" + // OutputFormatJSON renders results as JSON. + OutputFormatJSON OutputFormat = "json" + // OutputFormatYAML renders results as YAML. + 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. + SkipSuccessResults bool +} + +// RenderValidationResult writes the validation result to w in the requested +// format. An unknown format returns an error without writing to w. +func RenderValidationResult(result *pkgvalidate.ValidationResult, format OutputFormat, w io.Writer, opts RenderOptions) error { + switch format { + case OutputFormatJSON: + return renderJSON(result, w) + case OutputFormatYAML: + return renderYAML(result, w) + case OutputFormatText, "": + return renderText(result, w, opts) + default: + return errors.Errorf("%s: %q", errUnknownFormat, format) + } +} + +func renderJSON(result *pkgvalidate.ValidationResult, w io.Writer) error { + out, err := json.MarshalIndent(result, "", " ") + if err != nil { + return errors.Wrap(err, errCannotMarshalJSON) + } + _, err = fmt.Fprintln(w, string(out)) + return err +} + +func renderYAML(result *pkgvalidate.ValidationResult, w io.Writer) error { + out, err := yaml.Marshal(result) + if err != nil { + return errors.Wrap(err, errCannotMarshalYAML) + } + _, err = fmt.Fprint(w, string(out)) + return err +} + +// renderText writes the result in the human-readable format that the +// validate CLI has historically produced, preserving line order and +// prefixes ([!], [x], [✓]). +func renderText(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOptions) error { + for _, r := range result.Resources { + gvk := fmt.Sprintf("%s, Kind=%s", r.APIVersion, r.Kind) + switch r.Status { + case pkgvalidate.ValidationStatusMissingSchema: + if _, err := fmt.Fprintf(w, "[!] could not find CRD/XRD for: %s\n", gvk); err != nil { + return err + } + case pkgvalidate.ValidationStatusDefaultingFailed: + for _, e := range r.Errors { + if e.Type == pkgvalidate.FieldErrorTypeDefaulting { + if _, err := fmt.Fprintf(w, "[!] failed to apply defaults for %s, %s: %s\n", gvk, r.Name, e.Message); err != nil { + return err + } + } + } + case pkgvalidate.ValidationStatusInvalid: + for _, e := range r.Errors { + prefix := "schema validation error" + if e.Type == pkgvalidate.FieldErrorTypeCEL { + prefix = "CEL validation error" + } + if _, err := fmt.Fprintf(w, "[x] %s %s, %s : %s\n", prefix, gvk, r.Name, e.Message); err != nil { + return err + } + } + case pkgvalidate.ValidationStatusValid: + if opts.SkipSuccessResults { + continue + } + if _, err := fmt.Fprintf(w, "[✓] %s, %s validated successfully\n", gvk, r.Name); err != nil { + return err + } + } + } + if _, err := fmt.Fprintf(w, "Total %d resources: %d missing schemas, %d success cases, %d failure cases\n", + result.Summary.Total, result.Summary.MissingSchemas, result.Summary.Valid, result.Summary.Invalid); err != nil { + return err + } + return nil +} diff --git a/cmd/crossplane/pkg/validate/render/render_test.go b/cmd/crossplane/pkg/validate/render/render_test.go new file mode 100644 index 0000000..db147e8 --- /dev/null +++ b/cmd/crossplane/pkg/validate/render/render_test.go @@ -0,0 +1,133 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package render + +import ( + "bytes" + "encoding/json" + "testing" + + "github.com/google/go-cmp/cmp" + "sigs.k8s.io/yaml" + + pkgvalidate "github.com/crossplane/cli/v2/cmd/crossplane/pkg/validate" +) + +// fixture returns a ValidationResult covering a valid, an invalid, and a +// missing-schema resource in that order. +func fixture() *pkgvalidate.ValidationResult { + return &pkgvalidate.ValidationResult{ + Summary: pkgvalidate.ValidationSummary{Total: 3, Valid: 1, Invalid: 1, MissingSchemas: 1}, + Resources: []pkgvalidate.ResourceValidationResult{ + { + APIVersion: "test.org/v1alpha1", Kind: "Test", Name: "ok", + Status: pkgvalidate.ValidationStatusValid, + }, + { + APIVersion: "test.org/v1alpha1", Kind: "Test", Name: "bad", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{ + { + Type: pkgvalidate.FieldErrorTypeSchema, + Field: "spec.replicas", + Message: `spec.replicas: Invalid value: "string": spec.replicas in body must be of type integer: "string"`, + Value: "string", + }, + }, + }, + { + APIVersion: "other.org/v1", Kind: "Unknown", Name: "missing", + Status: pkgvalidate.ValidationStatusMissingSchema, + }, + }, + } +} + +const expectedTextWithSuccess = `[✓] test.org/v1alpha1, Kind=Test, ok validated successfully +[x] schema validation error test.org/v1alpha1, Kind=Test, bad : spec.replicas: Invalid value: "string": spec.replicas in body must be of type integer: "string" +[!] could not find CRD/XRD for: other.org/v1, Kind=Unknown +Total 3 resources: 1 missing schemas, 1 success cases, 1 failure cases +` + +const expectedTextSkipSuccess = `[x] schema validation error test.org/v1alpha1, Kind=Test, bad : spec.replicas: Invalid value: "string": spec.replicas in body must be of type integer: "string" +[!] could not find CRD/XRD for: other.org/v1, Kind=Unknown +Total 3 resources: 1 missing schemas, 1 success cases, 1 failure cases +` + +func TestRenderValidationResult_Text(t *testing.T) { + cases := map[string]struct { + format OutputFormat + opts RenderOptions + expected string + }{ + "TextWithSuccess": {format: OutputFormatText, expected: expectedTextWithSuccess}, + "TextSkipSuccess": {format: OutputFormatText, opts: RenderOptions{SkipSuccessResults: true}, expected: expectedTextSkipSuccess}, + "TextEmptyFormat": {format: OutputFormat(""), expected: expectedTextWithSuccess}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + var buf bytes.Buffer + if err := RenderValidationResult(fixture(), tc.format, &buf, tc.opts); err != nil { + t.Fatalf("RenderValidationResult() unexpected error: %v", err) + } + if got := buf.String(); got != tc.expected { + t.Errorf("text output mismatch\n--- want ---\n%s\n--- got ---\n%s", tc.expected, got) + } + }) + } +} + +func TestRenderValidationResult_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) + } + 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 diff := cmp.Diff(*in, got); diff != "" { + t.Errorf("JSON round-trip mismatch (-want +got):\n%s", diff) + } +} + +func TestRenderValidationResult_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) + } + 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 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") + } + if buf.Len() != 0 { + t.Errorf("Unknown format wrote %d bytes; want 0 (content: %q)", buf.Len(), buf.String()) + } +} diff --git a/cmd/crossplane/pkg/validate/types.go b/cmd/crossplane/pkg/validate/types.go new file mode 100644 index 0000000..84e1064 --- /dev/null +++ b/cmd/crossplane/pkg/validate/types.go @@ -0,0 +1,80 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validate + +// ValidationResult contains the complete results of schema validation. +type ValidationResult struct { + Summary ValidationSummary `json:"summary"` + Resources []ResourceValidationResult `json:"resources"` +} + +// ValidationSummary provides aggregate counts across all validated resources. +type ValidationSummary struct { + Total int `json:"total"` + Valid int `json:"valid"` + Invalid int `json:"invalid"` + MissingSchemas int `json:"missingSchemas"` +} + +// ResourceValidationResult contains validation results for a single resource. +type ResourceValidationResult struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` + Status ValidationStatus `json:"status"` + Errors []FieldValidationError `json:"errors,omitempty"` +} + +// ValidationStatus indicates the validation result for a resource. +type ValidationStatus string + +// ValidationStatus values. +const ( + // ValidationStatusValid indicates the resource passed all validation checks. + ValidationStatusValid ValidationStatus = "valid" + // ValidationStatusInvalid indicates the resource failed one or more validation checks. + ValidationStatusInvalid ValidationStatus = "invalid" + // ValidationStatusMissingSchema indicates no schema (CRD/XRD) was found for the resource. + ValidationStatusMissingSchema ValidationStatus = "missingSchema" + // ValidationStatusDefaultingFailed indicates defaults could not be applied to the resource. + ValidationStatusDefaultingFailed ValidationStatus = "defaultingFailed" +) + +// 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"` + // 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. + Message string `json:"message"` + // Value is the invalid value, if applicable. + Value any `json:"value,omitempty"` +} + +// FieldErrorType categorizes the kind of validation error. +const ( + // FieldErrorTypeSchema indicates a schema validation error from OpenAPI validation. + FieldErrorTypeSchema = "schema" + // FieldErrorTypeCEL indicates a CEL rule validation error. + FieldErrorTypeCEL = "cel" + // FieldErrorTypeUnknownField indicates an unknown field was present in the resource. + FieldErrorTypeUnknownField = "unknownField" + // FieldErrorTypeDefaulting indicates defaults could not be applied to the resource. + FieldErrorTypeDefaulting = "defaulting" +) diff --git a/cmd/crossplane/validate/unknown_fields.go b/cmd/crossplane/pkg/validate/unknown_fields.go similarity index 80% rename from cmd/crossplane/validate/unknown_fields.go rename to cmd/crossplane/pkg/validate/unknown_fields.go index 6dfd562..c2dd0d4 100644 --- a/cmd/crossplane/validate/unknown_fields.go +++ b/cmd/crossplane/pkg/validate/unknown_fields.go @@ -25,18 +25,19 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" ) -// validateUnknownFields Validates the resource's unknown fields against the given schema and returns a list of errors. +// validateUnknownFields validates the resource's unknown fields against the +// given schema and returns a list of errors. func validateUnknownFields(mr map[string]any, sch *schema.Structural) field.ErrorList { 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, len(uf)) - for i, f := range uf { + for _, f := range uf { strPath := strings.Split(f, ".") child := strPath[len(strPath)-1] - errs[i] = field.Invalid(field.NewPath(f), child, fmt.Sprintf("unknown field: \"%s\"", child)) + errs = append(errs, field.Invalid(field.NewPath(f), child, fmt.Sprintf("unknown field: \"%s\"", child))) } return errs diff --git a/cmd/crossplane/pkg/validate/unknown_fields_test.go b/cmd/crossplane/pkg/validate/unknown_fields_test.go new file mode 100644 index 0000000..246cb19 --- /dev/null +++ b/cmd/crossplane/pkg/validate/unknown_fields_test.go @@ -0,0 +1,115 @@ +/* +Copyright 2024 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validate + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/crossplane/crossplane-runtime/v2/pkg/test" +) + +func TestValidateUnknownFields(t *testing.T) { + type args struct { + mr map[string]any + sch *schema.Structural + } + + type want struct { + errs field.ErrorList + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "UnknownFieldPresent": { + reason: "Should detect unknown fields in the resource and return an error", + args: args{ + mr: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "metadata": map[string]any{ + "name": "test-instance", + }, + "spec": map[string]any{ + "replicas": 3, + "unknownField": "should fail", + }, + }, + sch: &schema.Structural{ + Properties: map[string]schema.Structural{ + "spec": { + Properties: map[string]schema.Structural{ + "replicas": { + Generic: schema.Generic{Type: "integer"}, + }, + }, + }, + }, + }, + }, + want: want{ + errs: field.ErrorList{ + field.Invalid(field.NewPath("spec.unknownField"), "unknownField", `unknown field: "unknownField"`), + }, + }, + }, + "UnknownFieldNotPresent": { + reason: "Should not return an error when no unknown fields are present", + args: args{ + mr: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "metadata": map[string]any{ + "name": "test-instance", + }, + "spec": map[string]any{ + "replicas": 3, + }, + }, + sch: &schema.Structural{ + Properties: map[string]schema.Structural{ + "spec": { + Properties: map[string]schema.Structural{ + "replicas": { + Generic: schema.Generic{Type: "integer"}, + }, + }, + }, + }, + }, + }, + want: want{ + errs: field.ErrorList{}, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + errs := validateUnknownFields(tc.args.mr, tc.args.sch) + if diff := cmp.Diff(tc.want.errs, errs, test.EquateErrors()); diff != "" { + t.Errorf("%s\nvalidateUnknownFields(...): -want errs, +got errs:\n%s", tc.reason, diff) + } + }) + } +} diff --git a/cmd/crossplane/validate/validate.go b/cmd/crossplane/pkg/validate/validate.go similarity index 59% rename from cmd/crossplane/validate/validate.go rename to cmd/crossplane/pkg/validate/validate.go index 0a2b14a..da3a146 100644 --- a/cmd/crossplane/validate/validate.go +++ b/cmd/crossplane/pkg/validate/validate.go @@ -19,7 +19,6 @@ package validate import ( "context" "fmt" - "io" ext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -36,9 +35,90 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/xcrd" ) -const ( - errWriteOutput = "cannot write output" -) +// SchemaValidate performs schema validation and returns structured results. +// +// This is the processing-only API: no I/O is performed, allowing programmatic +// consumers to inspect the result directly or hand it to a renderer. The +// returned error is non-nil only for setup failures (for example, a CRD that +// cannot be converted or compiled); per-resource validation failures are +// reported via ResourceValidationResult.Status and .Errors, not via the error. +func SchemaValidate(ctx context.Context, resources []*unstructured.Unstructured, crds []*extv1.CustomResourceDefinition) (*ValidationResult, error) { //nolint:gocognit // validation has many branches + schemaValidators, structurals, err := newValidatorsAndStructurals(crds) + if err != nil { + return nil, errors.Wrap(err, "cannot create schema validators") + } + + result := &ValidationResult{ + Resources: make([]ResourceValidationResult, 0, len(resources)), + } + + for _, r := range resources { + gvk := r.GetObjectKind().GroupVersionKind() + rvr := ResourceValidationResult{ + APIVersion: gvk.GroupVersion().String(), + Kind: gvk.Kind, + Name: getResourceName(r), + Namespace: r.GetNamespace(), + } + + sv, ok := schemaValidators[gvk] + s := structurals[gvk] + + if !ok { + rvr.Status = ValidationStatusMissingSchema + result.Resources = append(result.Resources, rvr) + continue + } + + if err := applyDefaults(r, gvk, crds); err != nil { + rvr.Status = ValidationStatusDefaultingFailed + rvr.Errors = append(rvr.Errors, FieldValidationError{ + Type: FieldErrorTypeDefaulting, + Message: err.Error(), + }) + result.Resources = append(result.Resources, rvr) + continue + } + + for _, v := range sv { + for _, e := range validation.ValidateCustomResource(nil, r, *v) { + rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeSchema)) + } + for _, e := range validateUnknownFields(r.UnstructuredContent(), s) { + rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeUnknownField)) + } + + celValidator := cel.NewValidator(s, true, celconfig.PerCallLimit) + celErrs, _ := celValidator.Validate(ctx, nil, s, r.Object, nil, celconfig.PerCallLimit) + for _, e := range celErrs { + rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeCEL)) + } + } + + if len(rvr.Errors) > 0 { + rvr.Status = ValidationStatusInvalid + } else { + rvr.Status = ValidationStatusValid + } + result.Resources = append(result.Resources, rvr) + } + + result.Summary = computeSummary(result.Resources) + return result, nil +} + +// ResultError returns an error summarizing the outcome of validation, or nil +// if validation succeeded. This preserves the historical error semantics of +// SchemaValidation for programmatic consumers who want a boolean pass/fail. +func ResultError(result *ValidationResult, errorOnMissingSchemas bool) error { + if result.Summary.Invalid > 0 { + return errors.New("could not validate all resources") + } + if errorOnMissingSchemas && result.Summary.MissingSchemas > 0 { + return errors.New("could not validate all resources, schema(s) missing") + } + return nil +} func newValidatorsAndStructurals(crds []*extv1.CustomResourceDefinition) (map[runtimeschema.GroupVersionKind][]*validation.SchemaValidator, map[runtimeschema.GroupVersionKind]*schema.Structural, error) { validators := map[runtimeschema.GroupVersionKind][]*validation.SchemaValidator{} @@ -94,87 +174,33 @@ func newValidatorsAndStructurals(crds []*extv1.CustomResourceDefinition) (map[ru return validators, structurals, nil } -// SchemaValidation validates the resources against the given CRDs. -func SchemaValidation(ctx context.Context, resources []*unstructured.Unstructured, crds []*extv1.CustomResourceDefinition, errorOnMissingSchemas bool, skipSuccessLogs bool, w io.Writer) error { //nolint:gocognit // printing the output increases the cyclomatic complexity a little bit - schemaValidators, structurals, err := newValidatorsAndStructurals(crds) - if err != nil { - return errors.Wrap(err, "cannot create schema validators") - } - - failure, missingSchemas := 0, 0 - - for _, r := range resources { - gvk := r.GetObjectKind().GroupVersionKind() - sv, ok := schemaValidators[gvk] - s := structurals[gvk] // if we have a schema validator, we should also have a structural - - if !ok { - missingSchemas++ - - if _, err := fmt.Fprintf(w, "[!] could not find CRD/XRD for: %s\n", r.GroupVersionKind().String()); err != nil { - return errors.Wrap(err, errWriteOutput) - } - - continue - } - - if err := applyDefaults(r, gvk, crds); err != nil { - if _, err := fmt.Fprintf(w, "[!] failed to apply defaults for %s, %s: %v\n", r.GroupVersionKind().String(), getResourceName(r), err); err != nil { - return errors.Wrap(err, errWriteOutput) - } - } - - rf := 0 - - re := field.ErrorList{} - for _, v := range sv { - re = append(re, validation.ValidateCustomResource(nil, r, *v)...) - - re = append(re, validateUnknownFields(r.UnstructuredContent(), s)...) - for _, e := range re { - rf++ - - if _, err := fmt.Fprintf(w, "[x] schema validation error %s, %s : %s\n", r.GroupVersionKind().String(), getResourceName(r), e.Error()); err != nil { - return errors.Wrap(err, errWriteOutput) - } - } - - celValidator := cel.NewValidator(s, true, celconfig.PerCallLimit) - - re, _ = celValidator.Validate(ctx, nil, s, r.Object, nil, celconfig.PerCallLimit) - for _, e := range re { - rf++ - - if _, err := fmt.Fprintf(w, "[x] CEL validation error %s, %s : %s\n", r.GroupVersionKind().String(), getResourceName(r), e.Error()); err != nil { - return errors.Wrap(err, errWriteOutput) - } - } - - if rf == 0 { - if !skipSuccessLogs { - if _, err := fmt.Fprintf(w, "[✓] %s, %s validated successfully\n", r.GroupVersionKind().String(), getResourceName(r)); err != nil { - return errors.Wrap(err, errWriteOutput) - } - } - } else { - failure++ - } - } - } - - if _, err := fmt.Fprintf(w, "Total %d resources: %d missing schemas, %d success cases, %d failure cases\n", len(resources), missingSchemas, len(resources)-failure-missingSchemas, failure); err != nil { - return errors.Wrap(err, errWriteOutput) +// fieldErrorToFieldValidationError converts a k8s field.Error into our structured type. +func fieldErrorToFieldValidationError(e *field.Error, errType string) FieldValidationError { + out := FieldValidationError{ + Type: errType, + Field: e.Field, + Message: e.Error(), } - - if failure > 0 { - return errors.New("could not validate all resources") + if e.BadValue != nil { + out.Value = e.BadValue } + return out +} - if errorOnMissingSchemas && missingSchemas > 0 { - return errors.New("could not validate all resources, schema(s) missing") +// computeSummary calculates aggregate counts from per-resource results. +func computeSummary(results []ResourceValidationResult) ValidationSummary { + s := ValidationSummary{Total: len(results)} + for _, r := range results { + switch r.Status { + case ValidationStatusValid: + s.Valid++ + case ValidationStatusInvalid, ValidationStatusDefaultingFailed: + s.Invalid++ + case ValidationStatusMissingSchema: + s.MissingSchemas++ + } } - - return nil + return s } func getResourceName(r *unstructured.Unstructured) string { @@ -186,7 +212,8 @@ func getResourceName(r *unstructured.Unstructured) string { return r.GetAnnotations()[xcrd.AnnotationKeyCompositionResourceName] } -// applyDefaults applies default values from the CRD schema to the unstructured resource. +// applyDefaults applies default values from the CRD schema to the unstructured +// resource. func applyDefaults(resource *unstructured.Unstructured, gvk runtimeschema.GroupVersionKind, crds []*extv1.CustomResourceDefinition) error { var matchingCRD *extv1.CustomResourceDefinition diff --git a/cmd/crossplane/pkg/validate/validate_test.go b/cmd/crossplane/pkg/validate/validate_test.go new file mode 100644 index 0000000..26071f7 --- /dev/null +++ b/cmd/crossplane/pkg/validate/validate_test.go @@ -0,0 +1,387 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validate + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" + "github.com/crossplane/crossplane-runtime/v2/pkg/test" +) + +// testCRD is a minimal CRD whose schema requires spec.replicas as an integer. +var testCRD = &extv1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io/v1", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: extv1.CustomResourceDefinitionSpec{ + Group: "test.org", + Names: extv1.CustomResourceDefinitionNames{ + Kind: "Test", + ListKind: "TestList", + Plural: "tests", + Singular: "test", + }, + Scope: "Cluster", + Versions: []extv1.CustomResourceDefinitionVersion{{ + Name: "v1alpha1", + Served: true, + Storage: true, + Schema: &extv1.CustomResourceValidation{ + OpenAPIV3Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "spec": { + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "replicas": {Type: "integer"}, + }, + Required: []string{"replicas"}, + }, + }, + }, + }, + }}, + }, +} + +// testCRDWithCEL requires minReplicas <= replicas <= maxReplicas. +var testCRDWithCEL = &extv1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io/v1", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{Name: "test-cel"}, + Spec: extv1.CustomResourceDefinitionSpec{ + Group: "test.org", + Names: extv1.CustomResourceDefinitionNames{ + Kind: "TestCEL", ListKind: "TestCELList", Plural: "testcels", Singular: "testcel", + }, + Scope: "Cluster", + Versions: []extv1.CustomResourceDefinitionVersion{{ + Name: "v1alpha1", Served: true, Storage: true, + Schema: &extv1.CustomResourceValidation{ + OpenAPIV3Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "spec": { + Type: "object", + XValidations: extv1.ValidationRules{{ + Rule: "self.minReplicas <= self.replicas && self.replicas <= self.maxReplicas", + Message: "replicas should be in between minReplicas and maxReplicas", + }}, + Properties: map[string]extv1.JSONSchemaProps{ + "replicas": {Type: "integer"}, + "minReplicas": {Type: "integer"}, + "maxReplicas": {Type: "integer"}, + }, + Required: []string{"replicas", "minReplicas", "maxReplicas"}, + }, + }, + }, + }, + }}, + }, +} + +// testCRDNoMatchingVersion is a CRD that shares group+kind with testCRD but +// only declares v1beta1. When used BEFORE testCRD in the crds slice, +// applyDefaults matches it first and fails because v1alpha1 is missing. +var testCRDNoMatchingVersion = &extv1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io/v1", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{Name: "test-other-version"}, + Spec: extv1.CustomResourceDefinitionSpec{ + Group: "test.org", + Names: extv1.CustomResourceDefinitionNames{ + Kind: "Test", ListKind: "TestList", Plural: "tests", Singular: "test", + }, + Scope: "Cluster", + Versions: []extv1.CustomResourceDefinitionVersion{{ + Name: "v1beta1", Served: true, Storage: true, + Schema: &extv1.CustomResourceValidation{ + OpenAPIV3Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "spec": { + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "replicas": {Type: "integer"}, + }, + Required: []string{"replicas"}, + }, + }, + }, + }, + }}, + }, +} + +func TestSchemaValidate(t *testing.T) { + validResource := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "metadata": map[string]any{"name": "test"}, + "spec": map[string]any{"replicas": int64(1)}, + }} + invalidSchemaResource := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "metadata": map[string]any{"name": "bad-type"}, + "spec": map[string]any{"replicas": "not-an-int"}, + }} + invalidCELResource := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "TestCEL", + "metadata": map[string]any{"name": "bad-cel"}, + "spec": map[string]any{ + "replicas": int64(50), "minReplicas": int64(3), "maxReplicas": int64(10), + }, + }} + missingSchemaResource := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "other.org/v1", + "kind": "Unknown", + "metadata": map[string]any{"name": "no-crd"}, + }} + unknownFieldResource := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "metadata": map[string]any{"name": "extra"}, + "spec": map[string]any{ + "replicas": int64(1), "unknownField": "surprise", + }, + }} + defaultingFailureResource := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "metadata": map[string]any{"name": "def-fail"}, + "spec": map[string]any{"replicas": int64(1)}, + }} + + type args struct { + resources []*unstructured.Unstructured + crds []*extv1.CustomResourceDefinition + } + type expect struct { + status ValidationStatus + errorTypes []string // expected FieldValidationError.Type values (order-independent, subset) + } + type want struct { + summary ValidationSummary + perRes []expect + wantErr bool + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "Valid": { + reason: "A resource matching its CRD schema should be marked Valid with no errors.", + args: args{ + resources: []*unstructured.Unstructured{validResource}, + crds: []*extv1.CustomResourceDefinition{testCRD}, + }, + want: want{ + summary: ValidationSummary{Total: 1, Valid: 1}, + perRes: []expect{{status: ValidationStatusValid}}, + }, + }, + "InvalidSchema": { + reason: "A resource violating OpenAPI schema should be Invalid with a schema-type field error.", + args: args{ + resources: []*unstructured.Unstructured{invalidSchemaResource}, + crds: []*extv1.CustomResourceDefinition{testCRD}, + }, + want: want{ + summary: ValidationSummary{Total: 1, Invalid: 1}, + perRes: []expect{{status: ValidationStatusInvalid, errorTypes: []string{FieldErrorTypeSchema}}}, + }, + }, + "InvalidCEL": { + reason: "A resource violating a CEL rule should be Invalid with a cel-type field error.", + args: args{ + resources: []*unstructured.Unstructured{invalidCELResource}, + crds: []*extv1.CustomResourceDefinition{testCRDWithCEL}, + }, + want: want{ + summary: ValidationSummary{Total: 1, Invalid: 1}, + perRes: []expect{{status: ValidationStatusInvalid, errorTypes: []string{FieldErrorTypeCEL}}}, + }, + }, + "MissingSchema": { + reason: "A resource whose GVK has no matching CRD should be MissingSchema with no errors.", + args: args{ + resources: []*unstructured.Unstructured{missingSchemaResource}, + crds: []*extv1.CustomResourceDefinition{testCRD}, + }, + want: want{ + summary: ValidationSummary{Total: 1, MissingSchemas: 1}, + perRes: []expect{{status: ValidationStatusMissingSchema}}, + }, + }, + "UnknownField": { + reason: "A resource with a field not declared in the schema should surface an unknownField error.", + args: args{ + resources: []*unstructured.Unstructured{unknownFieldResource}, + crds: []*extv1.CustomResourceDefinition{testCRD}, + }, + want: want{ + summary: ValidationSummary{Total: 1, Invalid: 1}, + perRes: []expect{{status: ValidationStatusInvalid, errorTypes: []string{FieldErrorTypeUnknownField}}}, + }, + }, + "DefaultingFailure": { + reason: "When applyDefaults cannot find a matching version, status should be DefaultingFailed with a defaulting error.", + args: args{ + resources: []*unstructured.Unstructured{defaultingFailureResource}, + // testCRDNoMatchingVersion matches group+kind but only has v1beta1; + // applyDefaults picks it first and fails before the v1alpha1 schema (from testCRD) is consulted. + crds: []*extv1.CustomResourceDefinition{testCRDNoMatchingVersion, testCRD}, + }, + want: want{ + summary: ValidationSummary{Total: 1, Invalid: 1}, + perRes: []expect{{status: ValidationStatusDefaultingFailed, errorTypes: []string{FieldErrorTypeDefaulting}}}, + }, + }, + "Empty": { + reason: "Empty inputs should return a zero-total result without error.", + args: args{}, + want: want{ + summary: ValidationSummary{Total: 0}, + perRes: nil, + }, + }, + "MixedOrder": { + reason: "Resources are returned in input order with their respective statuses.", + args: args{ + resources: []*unstructured.Unstructured{validResource, invalidSchemaResource, missingSchemaResource}, + crds: []*extv1.CustomResourceDefinition{testCRD}, + }, + want: want{ + summary: ValidationSummary{Total: 3, Valid: 1, Invalid: 1, MissingSchemas: 1}, + perRes: []expect{ + {status: ValidationStatusValid}, + {status: ValidationStatusInvalid, errorTypes: []string{FieldErrorTypeSchema}}, + {status: ValidationStatusMissingSchema}, + }, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + result, err := SchemaValidate(context.Background(), tc.args.resources, tc.args.crds) + if (err != nil) != tc.want.wantErr { + t.Fatalf("%s\nSchemaValidate() err = %v, wantErr = %v", tc.reason, err, tc.want.wantErr) + } + if tc.want.wantErr { + return + } + if diff := cmp.Diff(tc.want.summary, result.Summary); diff != "" { + t.Errorf("%s\nSummary mismatch (-want +got):\n%s", tc.reason, diff) + } + if got, want := len(result.Resources), len(tc.want.perRes); got != want { + t.Fatalf("%s\nlen(Resources) = %d, want %d", tc.reason, got, want) + } + for i, r := range result.Resources { + exp := tc.want.perRes[i] + if r.Status != exp.status { + t.Errorf("%s\nResources[%d].Status = %q, want %q", tc.reason, i, r.Status, exp.status) + } + if !containsAllErrorTypes(r.Errors, exp.errorTypes) { + t.Errorf("%s\nResources[%d].Errors = %+v; want to include types %v", tc.reason, i, r.Errors, exp.errorTypes) + } + if len(exp.errorTypes) == 0 && len(r.Errors) != 0 { + t.Errorf("%s\nResources[%d].Errors = %+v; want empty", tc.reason, i, r.Errors) + } + } + }) + } +} + +func TestResultError(t *testing.T) { + cases := map[string]struct { + reason string + summary ValidationSummary + errorOnMissingSchemas bool + wantErr error + }{ + "Clean": { + reason: "No invalid or missing schemas should return nil.", + summary: ValidationSummary{Total: 1, Valid: 1}, + }, + "InvalidPresent": { + reason: "Invalid > 0 should return the invalid-resources error regardless of the flag.", + summary: ValidationSummary{Total: 1, Invalid: 1}, + wantErr: errors.New("could not validate all resources"), + }, + "MissingIgnored": { + reason: "MissingSchemas should be ignored when errorOnMissingSchemas is false.", + summary: ValidationSummary{Total: 1, MissingSchemas: 1}, + }, + "MissingWithFlag": { + reason: "MissingSchemas with errorOnMissingSchemas should return the missing-schemas error.", + summary: ValidationSummary{Total: 1, MissingSchemas: 1}, + errorOnMissingSchemas: true, + wantErr: errors.New("could not validate all resources, schema(s) missing"), + }, + "InvalidAndMissing": { + reason: "Invalid takes precedence over MissingSchemas when both are present.", + summary: ValidationSummary{Total: 2, Invalid: 1, MissingSchemas: 1}, + errorOnMissingSchemas: true, + wantErr: errors.New("could not validate all resources"), + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := ResultError(&ValidationResult{Summary: tc.summary}, tc.errorOnMissingSchemas) + if diff := cmp.Diff(tc.wantErr, got, test.EquateErrors()); diff != "" { + t.Errorf("%s\nResultError(): -want err, +got err:\n%s", tc.reason, diff) + } + }) + } +} + +// containsAllErrorTypes returns true when every wanted type appears at least +// once in the given FieldValidationError slice. +func containsAllErrorTypes(errs []FieldValidationError, wantTypes []string) bool { + if len(wantTypes) == 0 { + return true + } + seen := make(map[string]bool, len(errs)) + for _, e := range errs { + seen[e.Type] = true + } + for _, t := range wantTypes { + if !seen[t] { + return false + } + } + return true +} diff --git a/cmd/crossplane/validate/cmd.go b/cmd/crossplane/validate/cmd.go index f8cbb03..b8627a3 100644 --- a/cmd/crossplane/validate/cmd.go +++ b/cmd/crossplane/validate/cmd.go @@ -20,18 +20,23 @@ package validate import ( "context" "fmt" + "io" "os" "path/filepath" "strings" "github.com/alecthomas/kong" "github.com/spf13/afero" + extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/crossplane/crossplane-runtime/v2/pkg/logging" "github.com/crossplane/crossplane-runtime/v2/pkg/version" "github.com/crossplane/cli/v2/cmd/crossplane/common/load" + pkgvalidate "github.com/crossplane/cli/v2/cmd/crossplane/pkg/validate" + "github.com/crossplane/cli/v2/cmd/crossplane/pkg/validate/render" _ "embed" ) @@ -39,6 +44,10 @@ import ( //go:embed help/validate.md var helpDetail string +// errWriteOutput is the error message wrapped around I/O failures when the +// validate command writes to its output writer. +const errWriteOutput = "cannot write output" + // Cmd arguments and flags for render subcommand. type Cmd struct { // Arguments. @@ -50,6 +59,7 @@ type Cmd struct { CleanCache bool `help:"Clean the cache directory before downloading package schemas."` CrossplaneImage string `help:"Specify the Crossplane image for validating built-in schemas."` ErrorOnMissingSchemas bool `default:"false" help:"Return non zero exit code if missing schemas."` + Output string `default:"text" enum:"text,json,yaml" help:"Output format for validation results (text, json, or yaml)." short:"o"` SkipSuccessResults bool `help:"Skip printing success results."` UpdateCache bool `default:"false" help:"Update cached schemas by downloading the latest version that satisfies a constraint. May be useful if you are using semantic version constraints and want to get the latest version, but this slows down the cache lookup due to the required network calls."` @@ -116,10 +126,29 @@ func (c *Cmd) Run(k *kong.Context, _ logging.Logger) error { return errors.Wrapf(err, "cannot download and load cache") } - // Validate resources against schemas - if err := SchemaValidation(context.Background(), resources, m.crds, c.ErrorOnMissingSchemas, c.SkipSuccessResults, k.Stdout); err != nil { + // Validate resources against schemas and render in the requested format. + if err := c.validateAndRender(context.Background(), resources, m.crds, k.Stdout); err != nil { return errors.Wrapf(err, "cannot validate resources") } return nil } + +// validateAndRender runs schema validation on the given resources and CRDs, +// writes the result to w in the format configured on the Cmd, and returns a +// non-nil error when validation failed (or when ErrorOnMissingSchemas is set +// and any resource had no matching schema). It is the core of Cmd.Run, +// extracted so that tests can exercise the flag-driven behaviour without the +// file/cache loading machinery. +func (c *Cmd) validateAndRender(ctx context.Context, resources []*unstructured.Unstructured, crds []*extv1.CustomResourceDefinition, w io.Writer) error { + result, err := pkgvalidate.SchemaValidate(ctx, resources, crds) + if err != nil { + return err + } + + if err := render.RenderValidationResult(result, render.OutputFormat(c.Output), w, render.RenderOptions{SkipSuccessResults: c.SkipSuccessResults}); err != nil { + return errors.Wrap(err, "cannot render validation result") + } + + return pkgvalidate.ResultError(result, c.ErrorOnMissingSchemas) +} diff --git a/cmd/crossplane/validate/cmd_test.go b/cmd/crossplane/validate/cmd_test.go new file mode 100644 index 0000000..8687222 --- /dev/null +++ b/cmd/crossplane/validate/cmd_test.go @@ -0,0 +1,203 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validate + +import ( + "bytes" + "context" + "encoding/json" + "strings" + "testing" + + "github.com/alecthomas/kong" + extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "sigs.k8s.io/yaml" + + pkgvalidate "github.com/crossplane/cli/v2/cmd/crossplane/pkg/validate" +) + +// parseCmdFlags exercises Kong's parsing of the Cmd struct tags. It builds a +// Cmd with fake positional args so we can focus on flag behaviour. +func parseCmdFlags(t *testing.T, args ...string) *Cmd { + t.Helper() + c := &Cmd{} + // Positional args Extensions and Resources are required; supply dummies. + full := append([]string{"ext.yaml", "res.yaml"}, args...) + parser, err := kong.New(c) + if err != nil { + t.Fatalf("kong.New(): %v", err) + } + if _, err := parser.Parse(full); err != nil { + t.Fatalf("kong.Parse(%v): %v", full, err) + } + return c +} + +func TestCmd_OutputFlagParsing(t *testing.T) { + cases := map[string]struct { + args []string + want string + }{ + "default": {args: nil, want: "text"}, + "long_text": {args: []string{"--output=text"}, want: "text"}, + "long_json": {args: []string{"--output=json"}, want: "json"}, + "long_yaml": {args: []string{"--output=yaml"}, want: "yaml"}, + "short_flag": {args: []string{"-o", "json"}, want: "json"}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + c := parseCmdFlags(t, tc.args...) + if c.Output != tc.want { + t.Errorf("Cmd.Output = %q; want %q", c.Output, tc.want) + } + }) + } +} + +func TestCmd_OutputFlagRejectsUnknown(t *testing.T) { + c := &Cmd{} + parser, err := kong.New(c) + if err != nil { + t.Fatalf("kong.New(): %v", err) + } + if _, err := parser.Parse([]string{"ext.yaml", "res.yaml", "--output=xml"}); err == nil { + t.Errorf("kong.Parse(--output=xml) = nil; want non-nil enum-validation error") + } +} + +// invalidFixture mirrors the `Invalid` case from the backward-compat test — +// a resource whose replicas field is the wrong type. +func invalidFixture() ([]*unstructured.Unstructured, []*extv1.CustomResourceDefinition) { + return []*unstructured.Unstructured{{Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "metadata": map[string]any{"name": "test"}, + "spec": map[string]any{"replicas": "not-an-int"}, + }}}, + []*extv1.CustomResourceDefinition{testCRD} +} + +func validFixture() ([]*unstructured.Unstructured, []*extv1.CustomResourceDefinition) { + return []*unstructured.Unstructured{{Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "metadata": map[string]any{"name": "test"}, + "spec": map[string]any{"replicas": 1}, + }}}, + []*extv1.CustomResourceDefinition{testCRD} +} + +func missingFixture() ([]*unstructured.Unstructured, []*extv1.CustomResourceDefinition) { + return []*unstructured.Unstructured{{Object: map[string]any{ + "apiVersion": "test.org/v1alpha1", + "kind": "Test", + "metadata": map[string]any{"name": "test"}, + "spec": map[string]any{"replicas": 1}, + }}}, + []*extv1.CustomResourceDefinition{} +} + +func TestCmd_ValidateAndRender(t *testing.T) { + type fixture func() ([]*unstructured.Unstructured, []*extv1.CustomResourceDefinition) + type want struct { + stdoutSubstr string + wantErr bool + parseAs string // "json", "yaml", or "" for no structural parse + } + + cases := map[string]struct { + output string + errorOnMissingSchemas bool + skipSuccess bool + fix fixture + want want + }{ + "default_text_valid": { + output: "text", fix: validFixture, + want: want{stdoutSubstr: "[✓] test.org/v1alpha1, Kind=Test, test validated successfully"}, + }, + "output_text_explicit_invalid": { + output: "text", fix: invalidFixture, + want: want{stdoutSubstr: "[x] schema validation error", wantErr: true}, + }, + "output_json_valid": { + output: "json", fix: validFixture, + want: want{parseAs: "json"}, + }, + "output_yaml_valid": { + output: "yaml", fix: validFixture, + want: want{parseAs: "yaml"}, + }, + "output_json_invalid_exits_nonzero": { + output: "json", fix: invalidFixture, + want: want{parseAs: "json", wantErr: true}, + }, + "output_json_missing_with_flag": { + output: "json", errorOnMissingSchemas: true, fix: missingFixture, + want: want{parseAs: "json", wantErr: true}, + }, + "output_json_missing_without_flag": { + output: "json", errorOnMissingSchemas: false, fix: missingFixture, + want: want{parseAs: "json"}, + }, + "skip_success_with_json_still_has_full_payload": { + output: "json", skipSuccess: true, fix: validFixture, + want: want{parseAs: "json"}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + c := &Cmd{Output: tc.output, ErrorOnMissingSchemas: tc.errorOnMissingSchemas, SkipSuccessResults: tc.skipSuccess} + resources, crds := tc.fix() + var buf bytes.Buffer + err := c.validateAndRender(context.Background(), resources, crds, &buf) + if (err != nil) != tc.want.wantErr { + t.Errorf("validateAndRender() err = %v; wantErr = %v", err, tc.want.wantErr) + } + + if tc.want.stdoutSubstr != "" && !strings.Contains(buf.String(), tc.want.stdoutSubstr) { + t.Errorf("stdout missing substring %q\n--- got ---\n%s", tc.want.stdoutSubstr, buf.String()) + } + + switch tc.want.parseAs { + case "json": + var got pkgvalidate.ValidationResult + if e := json.Unmarshal(buf.Bytes(), &got); e != nil { + t.Fatalf("stdout is not valid JSON: %v\n%s", e, buf.String()) + } + if got.Summary.Total == 0 { + t.Errorf("JSON result.Summary.Total == 0; fixture had resources") + } + // For skip_success case, verify Valid results are still included. + if tc.skipSuccess && got.Summary.Valid == 0 { + t.Errorf("--skip-success-results must not strip valid entries from JSON payload; got %+v", got) + } + case "yaml": + var got pkgvalidate.ValidationResult + if e := yaml.Unmarshal(buf.Bytes(), &got); e != nil { + t.Fatalf("stdout is not valid YAML: %v\n%s", e, buf.String()) + } + if got.Summary.Total == 0 { + t.Errorf("YAML result.Summary.Total == 0; fixture had resources") + } + } + }) + } +} diff --git a/cmd/crossplane/validate/help/validate.md b/cmd/crossplane/validate/help/validate.md index 5760886..ba5f007 100644 --- a/cmd/crossplane/validate/help/validate.md +++ b/cmd/crossplane/validate/help/validate.md @@ -136,6 +136,14 @@ Skip success log lines (only print problems): crossplane resource validate extensionsDir/ resourceDir/ --skip-success-results ``` +Emit machine-readable results (JSON or YAML) for piping to `jq`, scripts, or +CI systems. The structured payload includes per-resource status and +field-level error details: + +```shell +crossplane resource validate extensionsDir/ resourceDir/ --output json | jq . +``` + Validate the output of render against extensions in a directory: ```shell diff --git a/cmd/crossplane/validate/validate_test.go b/cmd/crossplane/validate/validate_test.go index 59d55bb..64a3697 100644 --- a/cmd/crossplane/validate/validate_test.go +++ b/cmd/crossplane/validate/validate_test.go @@ -18,19 +18,14 @@ package validate import ( "bytes" - "context" "testing" "github.com/google/go-cmp/cmp" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - runtimeschema "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/ptr" - "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/crossplane/crossplane-runtime/v2/pkg/test" ) @@ -1075,638 +1070,3 @@ func TestConvertToCRDs(t *testing.T) { }) } } - -func TestValidateResources(t *testing.T) { - type args struct { - resources []*unstructured.Unstructured - crds []*extv1.CustomResourceDefinition - errorOnMissingSchemas bool - } - - type want struct { - err error - } - - cases := map[string]struct { - reason string - args args - want want - }{ - "Valid": { - reason: "Should not return an error if the resources are valid", - args: args{ - resources: []*unstructured.Unstructured{ - { - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{ - "name": "test", - }, - "spec": map[string]any{ - "replicas": 1, - }, - }, - }, - }, - crds: []*extv1.CustomResourceDefinition{ - testCRD, - }, - }, - }, - "ValidWithCEL": { - reason: "Should not return an error if the resources are valid", - args: args{ - resources: []*unstructured.Unstructured{ - { - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{ - "name": "test", - }, - "spec": map[string]any{ - "replicas": 5, - "minReplicas": 3, - "maxReplicas": 10, - }, - }, - }, - }, - crds: []*extv1.CustomResourceDefinition{ - testCRDWithCEL, - }, - }, - }, - "ValidWithMissingSchemasEnabled": { - reason: "Should not return an error if the resources are valid and schemas are not missing", - args: args{ - resources: []*unstructured.Unstructured{ - { - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{ - "name": "test", - }, - "spec": map[string]any{ - "replicas": 1, - }, - }, - }, - }, - crds: []*extv1.CustomResourceDefinition{ - testCRD, - }, - errorOnMissingSchemas: true, - }, - }, - "ErrorOnMissingSchemas": { - reason: "Should return an error if schemas are missing", - args: args{ - resources: []*unstructured.Unstructured{ - { - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{ - "name": "test", - }, - "spec": map[string]any{ - "replicas": 1, - }, - }, - }, - }, - crds: []*extv1.CustomResourceDefinition{}, - errorOnMissingSchemas: true, - }, - want: want{ - err: errors.New("could not validate all resources, schema(s) missing"), - }, - }, - "Invalid": { - reason: "Should return an error if the resources are invalid", - args: args{ - resources: []*unstructured.Unstructured{ - { - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{ - "name": "test", - }, - "spec": map[string]any{ - "replicas": "non-integer", - }, - }, - }, - }, - crds: []*extv1.CustomResourceDefinition{ - testCRD, - }, - }, - want: want{ - err: errors.New("could not validate all resources"), - }, - }, - "InvalidWithCEL": { - reason: "Should not return an error if the resources are valid", - args: args{ - resources: []*unstructured.Unstructured{ - { - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{ - "name": "test", - }, - "spec": map[string]any{ - "replicas": 50, - "minReplicas": 3, - "maxReplicas": 10, - }, - }, - }, - }, - crds: []*extv1.CustomResourceDefinition{ - testCRDWithCEL, - }, - }, - want: want{ - err: errors.New("could not validate all resources"), - }, - }, - "MissingCRD": { - reason: "Should not return an error if the CRD/XRD is missing", - args: args{ - resources: []*unstructured.Unstructured{ - { - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{ - "name": "test", - }, - "spec": map[string]any{ - "replicas": 1, - }, - }, - }, - }, - crds: []*extv1.CustomResourceDefinition{}, - }, - }, - } - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - w := &bytes.Buffer{} - - got := SchemaValidation(context.Background(), tc.args.resources, tc.args.crds, tc.args.errorOnMissingSchemas, false, w) - if diff := cmp.Diff(tc.want.err, got, test.EquateErrors()); diff != "" { - t.Errorf("%s\nvalidateResources(...): -want error, +got error:\n%s", tc.reason, diff) - } - }) - } -} - -func TestValidateUnknownFields(t *testing.T) { - type args struct { - mr map[string]any - sch *schema.Structural - } - - type want struct { - errs field.ErrorList - } - - cases := map[string]struct { - reason string - args args - want want - }{ - "UnknownFieldPresent": { - reason: "Should detect unknown fields in the resource and return an error", - args: args{ - mr: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{ - "name": "test-instance", - }, - "spec": map[string]any{ - "replicas": 3, - "unknownField": "should fail", // This field is not defined in the CRD schema - }, - }, - sch: &schema.Structural{ - Properties: map[string]schema.Structural{ - "spec": { - Properties: map[string]schema.Structural{ - "replicas": { - Generic: schema.Generic{Type: "integer"}, - }, - }, - }, - }, - }, - }, - want: want{ - errs: field.ErrorList{ - field.Invalid(field.NewPath("spec.unknownField"), "unknownField", `unknown field: "unknownField"`), - }, - }, - }, - "UnknownFieldNotPresent": { - reason: "Should not return an error when no unknown fields are present", - args: args{ - mr: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{ - "name": "test-instance", - }, - "spec": map[string]any{ - "replicas": 3, // No unknown fields - }, - }, - sch: &schema.Structural{ - Properties: map[string]schema.Structural{ - "spec": { - Properties: map[string]schema.Structural{ - "replicas": { - Generic: schema.Generic{Type: "integer"}, - }, - }, - }, - }, - }, - }, - want: want{ - errs: field.ErrorList{}, - }, - }, - } - - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - errs := validateUnknownFields(tc.args.mr, tc.args.sch) - if diff := cmp.Diff(tc.want.errs, errs, test.EquateErrors()); diff != "" { - t.Errorf("%s\nvalidateUnknownFields(...): -want errs, +got errs:\n%s", tc.reason, diff) - } - }) - } -} - -func TestApplyDefaults(t *testing.T) { - type args struct { - resource *unstructured.Unstructured - gvk runtimeschema.GroupVersionKind - crds []*extv1.CustomResourceDefinition - } - - type want struct { - resource *unstructured.Unstructured - err error - } - - cases := map[string]struct { - reason string - args args - want want - }{ - "NoCRDFound": { - reason: "Should return nil when no matching CRD is found (skip defaulting)", - args: args{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "replicas": 3, - }, - }, - }, - gvk: runtimeschema.GroupVersionKind{ - Group: "test.org", - Version: "v1alpha1", - Kind: "Test", - }, - crds: []*extv1.CustomResourceDefinition{}, - }, - want: want{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "replicas": 3, - }, - }, - }, - err: nil, - }, - }, - "ApplySimpleDefault": { - reason: "Should apply default value to missing property", - args: args{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "replicas": 3, - }, - }, - }, - gvk: runtimeschema.GroupVersionKind{ - Group: "test.org", - Version: "v1alpha1", - Kind: "Test", - }, - crds: []*extv1.CustomResourceDefinition{ - { - Spec: extv1.CustomResourceDefinitionSpec{ - Group: "test.org", - Names: extv1.CustomResourceDefinitionNames{ - Kind: "Test", - }, - Versions: []extv1.CustomResourceDefinitionVersion{ - { - Name: "v1alpha1", - Schema: &extv1.CustomResourceValidation{ - OpenAPIV3Schema: &extv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "spec": { - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "replicas": { - Type: "integer", - }, - "deletionPolicy": { - Type: "string", - Default: &extv1.JSON{Raw: []byte(`"Delete"`)}, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - want: want{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "replicas": 3, - "deletionPolicy": "Delete", - }, - }, - }, - err: nil, - }, - }, - "DoNotOverrideExisting": { - reason: "Should not override existing values with defaults", - args: args{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "replicas": 3, - "deletionPolicy": "Retain", - }, - }, - }, - gvk: runtimeschema.GroupVersionKind{ - Group: "test.org", - Version: "v1alpha1", - Kind: "Test", - }, - crds: []*extv1.CustomResourceDefinition{ - { - Spec: extv1.CustomResourceDefinitionSpec{ - Group: "test.org", - Names: extv1.CustomResourceDefinitionNames{ - Kind: "Test", - }, - Versions: []extv1.CustomResourceDefinitionVersion{ - { - Name: "v1alpha1", - Schema: &extv1.CustomResourceValidation{ - OpenAPIV3Schema: &extv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "spec": { - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "replicas": { - Type: "integer", - }, - "deletionPolicy": { - Type: "string", - Default: &extv1.JSON{Raw: []byte(`"Delete"`)}, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - want: want{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "replicas": 3, - "deletionPolicy": "Retain", - }, - }, - }, - err: nil, - }, - }, - "NestedDefaults": { - reason: "Should apply defaults to nested objects", - args: args{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "forProvider": map[string]any{ - "region": "us-east-1", - }, - }, - }, - }, - gvk: runtimeschema.GroupVersionKind{ - Group: "test.org", - Version: "v1alpha1", - Kind: "Test", - }, - crds: []*extv1.CustomResourceDefinition{ - { - Spec: extv1.CustomResourceDefinitionSpec{ - Group: "test.org", - Names: extv1.CustomResourceDefinitionNames{ - Kind: "Test", - }, - Versions: []extv1.CustomResourceDefinitionVersion{ - { - Name: "v1alpha1", - Schema: &extv1.CustomResourceValidation{ - OpenAPIV3Schema: &extv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "spec": { - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "forProvider": { - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "region": { - Type: "string", - }, - "instanceType": { - Type: "string", - Default: &extv1.JSON{Raw: []byte(`"t3.micro"`)}, - }, - }, - }, - "deletionPolicy": { - Type: "string", - Default: &extv1.JSON{Raw: []byte(`"Delete"`)}, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - want: want{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "forProvider": map[string]any{ - "region": "us-east-1", - "instanceType": "t3.micro", - }, - "deletionPolicy": "Delete", - }, - }, - }, - err: nil, - }, - }, - "ComplexDefaults": { - reason: "Should apply complex default values (objects, arrays)", - args: args{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "name": "test", - }, - }, - }, - gvk: runtimeschema.GroupVersionKind{ - Group: "test.org", - Version: "v1alpha1", - Kind: "Test", - }, - crds: []*extv1.CustomResourceDefinition{ - { - Spec: extv1.CustomResourceDefinitionSpec{ - Group: "test.org", - Names: extv1.CustomResourceDefinitionNames{ - Kind: "Test", - }, - Versions: []extv1.CustomResourceDefinitionVersion{ - { - Name: "v1alpha1", - Schema: &extv1.CustomResourceValidation{ - OpenAPIV3Schema: &extv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "spec": { - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "name": { - Type: "string", - }, - "metadata": { - Type: "object", - Default: &extv1.JSON{Raw: []byte(`{"labels":{"app":"default-app"}}`)}, - }, - "tags": { - Type: "array", - Default: &extv1.JSON{Raw: []byte(`["default","tag"]`)}, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - want: want{ - resource: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "spec": map[string]any{ - "name": "test", - "metadata": map[string]any{ - "labels": map[string]any{ - "app": "default-app", - }, - }, - "tags": []any{"default", "tag"}, - }, - }, - }, - err: nil, - }, - }, - } - - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - err := applyDefaults(tc.args.resource, tc.args.gvk, tc.args.crds) - if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { - t.Errorf("%s\napplyDefaults(...): -want err, +got err:\n%s", tc.reason, diff) - } - - if diff := cmp.Diff(tc.want.resource, tc.args.resource); diff != "" { - t.Errorf("%s\napplyDefaults(...): -want resource, +got resource:\n%s", tc.reason, diff) - } - }) - } -} From e0c9dc07295daabf66ce61ed4e349d00141efcde Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Sun, 31 May 2026 16:33:47 -0400 Subject: [PATCH 2/4] fix(validate): preserve historical defaulting-failure semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review surfaced three related issues with how the refactor handled applyDefaults errors. The new SchemaValidate `continue`d after a defaulting failure (skipping schema/CEL validation), counted DefaultingFailed as Invalid (causing a non-zero exit), and let renderText emit a `[!]` warning marker for resources that the summary line then counted as failure cases — output that read as self-contradictory and exit-code behavior that diverged from the deleted SchemaValidation entry point. Restore the historical "warning, then continue" semantics: - SchemaValidate no longer aborts the per-resource loop on a defaulting failure. It records a FieldErrorTypeDefaulting entry and falls through to schema, CEL, and unknown-field checks so every problem surfaces at once. - Per-resource Status is decided after validation by statusFromErrors: any real schema/CEL/unknown error means Invalid, a defaulting-only failure means DefaultingFailed, no errors means Valid. - computeSummary counts DefaultingFailed toward Valid, mirroring the pre-refactor accounting where defaulting errors did not increment the failure counter. ResultError no longer escalates a defaulting-only failure to a CLI error. - renderText delegates to a new writeErrorLine helper that picks the prefix per error type ([!] for defaulting, [x] for schema/CEL/unknown). A resource with both a defaulting error and a schema error now produces both lines, matching the historical output. New tests: - TestSchemaValidate/DefaultingFailureWithSchemaError — regression guard that schema validation still runs (and surfaces errors) when defaulting fails. - TestRenderValidationResult_TextDefaulting — exercises the per-error prefix selection for a fixture with one DefaultingFailed resource and one Invalid resource carrying both error types. DefaultingFailureOnly (renamed from DefaultingFailure) now asserts the resource is summarized as Valid, locking in the historical behavior. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jonathan Ogilvie --- cmd/crossplane/pkg/validate/render/render.go | 37 ++++++++----- .../pkg/validate/render/render_test.go | 53 +++++++++++++++++++ cmd/crossplane/pkg/validate/validate.go | 45 ++++++++++++---- cmd/crossplane/pkg/validate/validate_test.go | 24 +++++++-- 4 files changed, 132 insertions(+), 27 deletions(-) diff --git a/cmd/crossplane/pkg/validate/render/render.go b/cmd/crossplane/pkg/validate/render/render.go index 4360b31..94ae49e 100644 --- a/cmd/crossplane/pkg/validate/render/render.go +++ b/cmd/crossplane/pkg/validate/render/render.go @@ -99,21 +99,13 @@ func renderText(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOp if _, err := fmt.Fprintf(w, "[!] could not find CRD/XRD for: %s\n", gvk); err != nil { return err } - case pkgvalidate.ValidationStatusDefaultingFailed: + case pkgvalidate.ValidationStatusInvalid, pkgvalidate.ValidationStatusDefaultingFailed: + // A resource may carry a mix of defaulting (warning) and + // schema/CEL (failure) errors. Print each error with the + // prefix appropriate to its type, regardless of the + // resource's overall status. for _, e := range r.Errors { - if e.Type == pkgvalidate.FieldErrorTypeDefaulting { - if _, err := fmt.Fprintf(w, "[!] failed to apply defaults for %s, %s: %s\n", gvk, r.Name, e.Message); err != nil { - return err - } - } - } - case pkgvalidate.ValidationStatusInvalid: - for _, e := range r.Errors { - prefix := "schema validation error" - if e.Type == pkgvalidate.FieldErrorTypeCEL { - prefix = "CEL validation error" - } - if _, err := fmt.Fprintf(w, "[x] %s %s, %s : %s\n", prefix, gvk, r.Name, e.Message); err != nil { + if err := writeErrorLine(w, gvk, r.Name, e); err != nil { return err } } @@ -132,3 +124,20 @@ func renderText(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOp } return nil } + +// writeErrorLine emits a single per-error line in the historical text +// format. Defaulting failures use the [!] warning prefix; schema, CEL, +// and unknown-field errors use [x]. +func writeErrorLine(w io.Writer, gvk, name string, e pkgvalidate.FieldValidationError) error { + switch e.Type { + case pkgvalidate.FieldErrorTypeDefaulting: + _, err := fmt.Fprintf(w, "[!] failed to apply defaults for %s, %s: %s\n", gvk, name, e.Message) + return err + case pkgvalidate.FieldErrorTypeCEL: + _, err := fmt.Fprintf(w, "[x] CEL validation error %s, %s : %s\n", gvk, name, e.Message) + return err + default: + _, err := fmt.Fprintf(w, "[x] schema validation error %s, %s : %s\n", gvk, name, e.Message) + return err + } +} diff --git a/cmd/crossplane/pkg/validate/render/render_test.go b/cmd/crossplane/pkg/validate/render/render_test.go index db147e8..df5530b 100644 --- a/cmd/crossplane/pkg/validate/render/render_test.go +++ b/cmd/crossplane/pkg/validate/render/render_test.go @@ -91,6 +91,59 @@ func TestRenderValidationResult_Text(t *testing.T) { } } +// defaultingFixture covers the DefaultingFailed status (warning-only) and an +// Invalid resource that has both a defaulting error and a schema error, +// exercising the per-error prefix selection in renderText. +func defaultingFixture() *pkgvalidate.ValidationResult { + return &pkgvalidate.ValidationResult{ + // Summary: defaulting-only resource counts as Valid; the mixed-error + // resource counts as Invalid. + Summary: pkgvalidate.ValidationSummary{Total: 2, Valid: 1, Invalid: 1}, + Resources: []pkgvalidate.ResourceValidationResult{ + { + APIVersion: "test.org/v1alpha1", Kind: "Test", Name: "warn-only", + Status: pkgvalidate.ValidationStatusDefaultingFailed, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeDefaulting, + Message: "no schema found for version v1alpha1 in CRD test-other-version", + }}, + }, + { + APIVersion: "test.org/v1alpha1", Kind: "Test", Name: "mixed", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{ + { + Type: pkgvalidate.FieldErrorTypeDefaulting, + Message: "no schema found for version v1alpha1 in CRD test-other-version", + }, + { + Type: pkgvalidate.FieldErrorTypeSchema, + Field: "spec.replicas", + Message: `spec.replicas: Invalid value: "string": spec.replicas in body must be of type integer: "string"`, + Value: "string", + }, + }, + }, + }, + } +} + +const expectedTextDefaulting = `[!] failed to apply defaults for test.org/v1alpha1, Kind=Test, warn-only: no schema found for version v1alpha1 in CRD test-other-version +[!] failed to apply defaults for test.org/v1alpha1, Kind=Test, mixed: no schema found for version v1alpha1 in CRD test-other-version +[x] schema validation error test.org/v1alpha1, Kind=Test, mixed : spec.replicas: Invalid value: "string": spec.replicas in body must be of type integer: "string" +Total 2 resources: 0 missing schemas, 1 success cases, 1 failure cases +` + +func TestRenderValidationResult_TextDefaulting(t *testing.T) { + var buf bytes.Buffer + if err := RenderValidationResult(defaultingFixture(), OutputFormatText, &buf, RenderOptions{}); err != nil { + t.Fatalf("RenderValidationResult() unexpected error: %v", err) + } + if got := buf.String(); got != expectedTextDefaulting { + t.Errorf("text output mismatch\n--- want ---\n%s\n--- got ---\n%s", expectedTextDefaulting, got) + } +} + func TestRenderValidationResult_JSON(t *testing.T) { in := fixture() var buf bytes.Buffer diff --git a/cmd/crossplane/pkg/validate/validate.go b/cmd/crossplane/pkg/validate/validate.go index da3a146..e26dd1b 100644 --- a/cmd/crossplane/pkg/validate/validate.go +++ b/cmd/crossplane/pkg/validate/validate.go @@ -70,14 +70,16 @@ func SchemaValidate(ctx context.Context, resources []*unstructured.Unstructured, continue } + // A defaulting failure is recorded as a warning-class error and does + // not abort validation: schema and CEL checks still run on the + // (un-defaulted) resource so the user sees every problem at once, + // matching the historical behavior of the SchemaValidation entry + // point this package replaced. if err := applyDefaults(r, gvk, crds); err != nil { - rvr.Status = ValidationStatusDefaultingFailed rvr.Errors = append(rvr.Errors, FieldValidationError{ Type: FieldErrorTypeDefaulting, Message: err.Error(), }) - result.Resources = append(result.Resources, rvr) - continue } for _, v := range sv { @@ -95,11 +97,7 @@ func SchemaValidate(ctx context.Context, resources []*unstructured.Unstructured, } } - if len(rvr.Errors) > 0 { - rvr.Status = ValidationStatusInvalid - } else { - rvr.Status = ValidationStatusValid - } + rvr.Status = statusFromErrors(rvr.Errors) result.Resources = append(result.Resources, rvr) } @@ -174,6 +172,28 @@ func newValidatorsAndStructurals(crds []*extv1.CustomResourceDefinition) (map[ru return validators, structurals, nil } +// statusFromErrors picks the right ValidationStatus given the errors a +// resource accumulated during validation. A real schema/CEL/unknown-field +// error trumps a defaulting failure: the schema error is the actionable +// problem, and DefaultingFailed is reserved for the case where defaulting +// was the only thing that went wrong. +func statusFromErrors(errs []FieldValidationError) ValidationStatus { + if len(errs) == 0 { + return ValidationStatusValid + } + onlyDefaulting := true + for _, e := range errs { + if e.Type != FieldErrorTypeDefaulting { + onlyDefaulting = false + break + } + } + if onlyDefaulting { + return ValidationStatusDefaultingFailed + } + return ValidationStatusInvalid +} + // fieldErrorToFieldValidationError converts a k8s field.Error into our structured type. func fieldErrorToFieldValidationError(e *field.Error, errType string) FieldValidationError { out := FieldValidationError{ @@ -188,13 +208,18 @@ func fieldErrorToFieldValidationError(e *field.Error, errType string) FieldValid } // computeSummary calculates aggregate counts from per-resource results. +// +// DefaultingFailed counts toward Valid: a defaulting failure is a warning, +// not a failure, and ResultError must not surface it as an error. This +// mirrors the historical SchemaValidation semantics where defaulting errors +// produced a [!] line but did not increment the failure counter. func computeSummary(results []ResourceValidationResult) ValidationSummary { s := ValidationSummary{Total: len(results)} for _, r := range results { switch r.Status { - case ValidationStatusValid: + case ValidationStatusValid, ValidationStatusDefaultingFailed: s.Valid++ - case ValidationStatusInvalid, ValidationStatusDefaultingFailed: + case ValidationStatusInvalid: s.Invalid++ case ValidationStatusMissingSchema: s.MissingSchemas++ diff --git a/cmd/crossplane/pkg/validate/validate_test.go b/cmd/crossplane/pkg/validate/validate_test.go index 26071f7..47909fa 100644 --- a/cmd/crossplane/pkg/validate/validate_test.go +++ b/cmd/crossplane/pkg/validate/validate_test.go @@ -256,8 +256,8 @@ func TestSchemaValidate(t *testing.T) { perRes: []expect{{status: ValidationStatusInvalid, errorTypes: []string{FieldErrorTypeUnknownField}}}, }, }, - "DefaultingFailure": { - reason: "When applyDefaults cannot find a matching version, status should be DefaultingFailed with a defaulting error.", + "DefaultingFailureOnly": { + reason: "A defaulting-only failure is a warning: status DefaultingFailed, summary counts as Valid (matches historical SchemaValidation semantics).", args: args{ resources: []*unstructured.Unstructured{defaultingFailureResource}, // testCRDNoMatchingVersion matches group+kind but only has v1beta1; @@ -265,10 +265,28 @@ func TestSchemaValidate(t *testing.T) { crds: []*extv1.CustomResourceDefinition{testCRDNoMatchingVersion, testCRD}, }, want: want{ - summary: ValidationSummary{Total: 1, Invalid: 1}, + summary: ValidationSummary{Total: 1, Valid: 1}, perRes: []expect{{status: ValidationStatusDefaultingFailed, errorTypes: []string{FieldErrorTypeDefaulting}}}, }, }, + "DefaultingFailureWithSchemaError": { + reason: "Schema validation must still run when defaulting fails; both errors must be reported and the resource must be Invalid (not DefaultingFailed).", + args: args{ + // Same multi-CRD trick as DefaultingFailureOnly to force a defaulting failure, + // but the resource has a wrong-typed replicas value so schema validation also fails. + // Regression guard: a prior version of this code did `continue` after a defaulting + // failure, silently hiding the schema error. + resources: []*unstructured.Unstructured{invalidSchemaResource}, + crds: []*extv1.CustomResourceDefinition{testCRDNoMatchingVersion, testCRD}, + }, + want: want{ + summary: ValidationSummary{Total: 1, Invalid: 1}, + perRes: []expect{{ + status: ValidationStatusInvalid, + errorTypes: []string{FieldErrorTypeDefaulting, FieldErrorTypeSchema}, + }}, + }, + }, "Empty": { reason: "Empty inputs should return a zero-total result without error.", args: args{}, From 050dafcc1c6ad9cbd9094718d78b943d210c9afd Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Tue, 2 Jun 2026 12:16:03 -0400 Subject: [PATCH 3/4] refactor(validate): address PR #1 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks the validate package to address eight comments from the PR review pass. Renderer: polymorphic dispatch * render.Renderer is now an interface; textRenderer, jsonRenderer, and yamlRenderer each implement it as their own concrete type. Adding a new format means adding a Renderer and registering it in the renderers map — no switch on string. (review #3, #4) * OutputFormat.Render dispatches via the registry; the empty string defaults to text for safety of zero-value callers. * renderText handles all per-resource line emission inline; the per-error prefix selection is encapsulated in a private helper that does not pretend to handle the [✓] or "[!] could not find" cases. SchemaValidate: decomposed * Extracted validateResource as a per-resource helper. SchemaValidate's body is now a clean fan-out and the //nolint:gocognit suppression is gone. (review #2) Tests: tightened, idiomatic, end-to-end * TestSchemaValidate now asserts on the full []FieldValidationError per resource (Type + Field), with cmpopts.IgnoreFields hiding the Message and Value strings that drift across k8s validator versions. Dropped the bespoke containsAllErrorTypes "at least once" matcher. (review #7) * All tests that take a context now use t.Context() instead of context.Background(). (review #6) * render text tests no longer compare full output bytes against a string constant; they assert on line counts plus per-line substrings with a generated summary line. (review #5) * Dropped the validateAndRender helper on Cmd. The validation + rendering + error-shaping logic lives directly in Cmd.Run. cmd_test.go now drives the command end-to-end through kong.Parse + ctx.Run against real YAML fixtures, capturing stdout from a real kong.Context. To keep the e2e run offline, testdata/cache contains a stand-in crossplane image package.yaml and the tests pass a matching --crossplane-image. (review #1, #8) Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jonathan Ogilvie --- cmd/crossplane/pkg/validate/render/render.go | 124 +++++-- .../pkg/validate/render/render_test.go | 129 ++++--- cmd/crossplane/pkg/validate/validate.go | 98 ++--- cmd/crossplane/pkg/validate/validate_test.go | 102 ++++-- cmd/crossplane/validate/cmd.go | 26 +- cmd/crossplane/validate/cmd_test.go | 341 +++++++++++------- .../crossplane@v0.0.0-test/package.yaml | 25 ++ cmd/crossplane/validate/testdata/cmd/crd.yaml | 27 ++ .../testdata/cmd/resources_invalid.yaml | 6 + .../testdata/cmd/resources_missing.yaml | 4 + .../testdata/cmd/resources_valid.yaml | 6 + 11 files changed, 571 insertions(+), 317 deletions(-) create mode 100644 cmd/crossplane/validate/testdata/cache/xpkg.crossplane.io/crossplane/crossplane@v0.0.0-test/package.yaml create mode 100644 cmd/crossplane/validate/testdata/cmd/crd.yaml create mode 100644 cmd/crossplane/validate/testdata/cmd/resources_invalid.yaml create mode 100644 cmd/crossplane/validate/testdata/cmd/resources_missing.yaml create mode 100644 cmd/crossplane/validate/testdata/cmd/resources_valid.yaml diff --git a/cmd/crossplane/pkg/validate/render/render.go b/cmd/crossplane/pkg/validate/render/render.go index 94ae49e..615039c 100644 --- a/cmd/crossplane/pkg/validate/render/render.go +++ b/cmd/crossplane/pkg/validate/render/render.go @@ -34,7 +34,14 @@ const ( errUnknownFormat = "unknown output format" ) -// OutputFormat specifies how validation results should be rendered. +// 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. +type Renderer interface { + Render(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOptions) error +} + +// OutputFormat names a Renderer. type OutputFormat string // OutputFormat values. @@ -51,26 +58,56 @@ const ( // 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. + // 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 + } + return r.Render(result, w, opts) +} + // RenderValidationResult writes the validation result to w in the requested -// format. An unknown format returns an error without writing to w. +// 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 { - switch format { - case OutputFormatJSON: - return renderJSON(result, w) - case OutputFormatYAML: - return renderYAML(result, w) - case OutputFormatText, "": - return renderText(result, w, opts) - default: - return errors.Errorf("%s: %q", errUnknownFormat, format) - } + return format.Render(result, w, opts) } -func renderJSON(result *pkgvalidate.ValidationResult, w io.Writer) error { +// 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 { out, err := json.MarshalIndent(result, "", " ") if err != nil { return errors.Wrap(err, errCannotMarshalJSON) @@ -79,7 +116,11 @@ func renderJSON(result *pkgvalidate.ValidationResult, w io.Writer) error { return err } -func renderYAML(result *pkgvalidate.ValidationResult, w io.Writer) error { +// yamlRenderer emits sigs.k8s.io/yaml output. +type yamlRenderer struct{} + +// Render implements Renderer. +func (yamlRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, _ RenderOptions) error { out, err := yaml.Marshal(result) if err != nil { return errors.Wrap(err, errCannotMarshalYAML) @@ -88,10 +129,22 @@ func renderYAML(result *pkgvalidate.ValidationResult, w io.Writer) error { return err } -// renderText writes the result in the human-readable format that the -// validate CLI has historically produced, preserving line order and -// prefixes ([!], [x], [✓]). -func renderText(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOptions) error { +// textRenderer emits the human-readable text format that the validate CLI +// has historically produced. Each resource emits zero or more lines +// depending on its status and accumulated errors: +// +// - MissingSchema: one [!] "could not find CRD/XRD for ..." line. +// - Valid: one [✓] "validated successfully" line, suppressed when +// opts.SkipSuccessResults is set. +// - Invalid or DefaultingFailed: one line per FieldValidationError with +// the prefix chosen by the error's Type — [!] for defaulting (a +// warning), [x] for schema/CEL/unknown-field failures. +// +// A trailing summary line lists totals. +type textRenderer struct{} + +// Render implements Renderer. +func (textRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOptions) error { for _, r := range result.Resources { gvk := fmt.Sprintf("%s, Kind=%s", r.APIVersion, r.Kind) switch r.Status { @@ -99,16 +152,6 @@ func renderText(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOp if _, err := fmt.Fprintf(w, "[!] could not find CRD/XRD for: %s\n", gvk); err != nil { return err } - case pkgvalidate.ValidationStatusInvalid, pkgvalidate.ValidationStatusDefaultingFailed: - // A resource may carry a mix of defaulting (warning) and - // schema/CEL (failure) errors. Print each error with the - // prefix appropriate to its type, regardless of the - // resource's overall status. - for _, e := range r.Errors { - if err := writeErrorLine(w, gvk, r.Name, e); err != nil { - return err - } - } case pkgvalidate.ValidationStatusValid: if opts.SkipSuccessResults { continue @@ -116,19 +159,24 @@ func renderText(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOp if _, err := fmt.Fprintf(w, "[✓] %s, %s validated successfully\n", gvk, r.Name); err != nil { return err } + case pkgvalidate.ValidationStatusInvalid, pkgvalidate.ValidationStatusDefaultingFailed: + for _, e := range r.Errors { + if err := writeTextErrorLine(w, gvk, r.Name, e); err != nil { + return err + } + } } } - if _, err := fmt.Fprintf(w, "Total %d resources: %d missing schemas, %d success cases, %d failure cases\n", - result.Summary.Total, result.Summary.MissingSchemas, result.Summary.Valid, result.Summary.Invalid); err != nil { - return err - } - return nil + _, err := fmt.Fprintf(w, "Total %d resources: %d missing schemas, %d success cases, %d failure cases\n", + result.Summary.Total, result.Summary.MissingSchemas, result.Summary.Valid, result.Summary.Invalid) + return err } -// writeErrorLine emits a single per-error line in the historical text -// format. Defaulting failures use the [!] warning prefix; schema, CEL, -// and unknown-field errors use [x]. -func writeErrorLine(w io.Writer, gvk, name string, e pkgvalidate.FieldValidationError) error { +// writeTextErrorLine emits a single per-error line. Defaulting failures use +// the [!] warning prefix; schema, CEL, and unknown-field errors use [x]. +// Kept private to the textRenderer because the per-error format is a +// detail of the text output, not part of the package's public surface. +func writeTextErrorLine(w io.Writer, gvk, name string, e pkgvalidate.FieldValidationError) error { switch e.Type { case pkgvalidate.FieldErrorTypeDefaulting: _, err := fmt.Fprintf(w, "[!] failed to apply defaults for %s, %s: %s\n", gvk, name, e.Message) diff --git a/cmd/crossplane/pkg/validate/render/render_test.go b/cmd/crossplane/pkg/validate/render/render_test.go index df5530b..7f1b89c 100644 --- a/cmd/crossplane/pkg/validate/render/render_test.go +++ b/cmd/crossplane/pkg/validate/render/render_test.go @@ -19,6 +19,8 @@ package render import ( "bytes" "encoding/json" + "fmt" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -57,47 +59,11 @@ func fixture() *pkgvalidate.ValidationResult { } } -const expectedTextWithSuccess = `[✓] test.org/v1alpha1, Kind=Test, ok validated successfully -[x] schema validation error test.org/v1alpha1, Kind=Test, bad : spec.replicas: Invalid value: "string": spec.replicas in body must be of type integer: "string" -[!] could not find CRD/XRD for: other.org/v1, Kind=Unknown -Total 3 resources: 1 missing schemas, 1 success cases, 1 failure cases -` - -const expectedTextSkipSuccess = `[x] schema validation error test.org/v1alpha1, Kind=Test, bad : spec.replicas: Invalid value: "string": spec.replicas in body must be of type integer: "string" -[!] could not find CRD/XRD for: other.org/v1, Kind=Unknown -Total 3 resources: 1 missing schemas, 1 success cases, 1 failure cases -` - -func TestRenderValidationResult_Text(t *testing.T) { - cases := map[string]struct { - format OutputFormat - opts RenderOptions - expected string - }{ - "TextWithSuccess": {format: OutputFormatText, expected: expectedTextWithSuccess}, - "TextSkipSuccess": {format: OutputFormatText, opts: RenderOptions{SkipSuccessResults: true}, expected: expectedTextSkipSuccess}, - "TextEmptyFormat": {format: OutputFormat(""), expected: expectedTextWithSuccess}, - } - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - var buf bytes.Buffer - if err := RenderValidationResult(fixture(), tc.format, &buf, tc.opts); err != nil { - t.Fatalf("RenderValidationResult() unexpected error: %v", err) - } - if got := buf.String(); got != tc.expected { - t.Errorf("text output mismatch\n--- want ---\n%s\n--- got ---\n%s", tc.expected, got) - } - }) - } -} - // defaultingFixture covers the DefaultingFailed status (warning-only) and an // Invalid resource that has both a defaulting error and a schema error, // exercising the per-error prefix selection in renderText. func defaultingFixture() *pkgvalidate.ValidationResult { return &pkgvalidate.ValidationResult{ - // Summary: defaulting-only resource counts as Valid; the mixed-error - // resource counts as Invalid. Summary: pkgvalidate.ValidationSummary{Total: 2, Valid: 1, Invalid: 1}, Resources: []pkgvalidate.ResourceValidationResult{ { @@ -128,19 +94,88 @@ func defaultingFixture() *pkgvalidate.ValidationResult { } } -const expectedTextDefaulting = `[!] failed to apply defaults for test.org/v1alpha1, Kind=Test, warn-only: no schema found for version v1alpha1 in CRD test-other-version -[!] failed to apply defaults for test.org/v1alpha1, Kind=Test, mixed: no schema found for version v1alpha1 in CRD test-other-version -[x] schema validation error test.org/v1alpha1, Kind=Test, mixed : spec.replicas: Invalid value: "string": spec.replicas in body must be of type integer: "string" -Total 2 resources: 0 missing schemas, 1 success cases, 1 failure cases -` - -func TestRenderValidationResult_TextDefaulting(t *testing.T) { +// renderTextLines runs Render with the given format and options and returns +// 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 { + t.Helper() var buf bytes.Buffer - if err := RenderValidationResult(defaultingFixture(), OutputFormatText, &buf, RenderOptions{}); err != nil { - t.Fatalf("RenderValidationResult() unexpected error: %v", err) + if err := format.Render(in, &buf, opts); err != nil { + t.Fatalf("Render() unexpected error: %v", err) } - if got := buf.String(); got != expectedTextDefaulting { - t.Errorf("text output mismatch\n--- want ---\n%s\n--- got ---\n%s", expectedTextDefaulting, got) + raw := strings.TrimRight(buf.String(), "\n") + if raw == "" { + return nil + } + return strings.Split(raw, "\n") +} + +// 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) { + 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 + }{ + "WithSuccess": { + in: fixture(), + format: OutputFormatText, + 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()), + }, + }, + "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(""), + 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()), + }, + }, + "DefaultingMixed": { + in: defaultingFixture(), + format: OutputFormatText, + wantLineSubs: []string{ + "[!] failed to apply defaults for test.org/v1alpha1, Kind=Test, warn-only", + "[!] failed to apply defaults for test.org/v1alpha1, Kind=Test, mixed", + "[x] schema validation error test.org/v1alpha1, Kind=Test, mixed", + summaryLine(defaultingFixture()), + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + lines := renderTextLines(t, tc.in, tc.format, tc.opts) + if len(lines) != len(tc.wantLineSubs) { + t.Fatalf("line count = %d, want %d\n--- got ---\n%s", len(lines), len(tc.wantLineSubs), strings.Join(lines, "\n")) + } + for i, sub := range tc.wantLineSubs { + if !strings.Contains(lines[i], sub) { + t.Errorf("line %d: expected substring %q, got %q", i, sub, lines[i]) + } + } + }) } } diff --git a/cmd/crossplane/pkg/validate/validate.go b/cmd/crossplane/pkg/validate/validate.go index e26dd1b..3ae13e0 100644 --- a/cmd/crossplane/pkg/validate/validate.go +++ b/cmd/crossplane/pkg/validate/validate.go @@ -42,7 +42,7 @@ import ( // returned error is non-nil only for setup failures (for example, a CRD that // cannot be converted or compiled); per-resource validation failures are // reported via ResourceValidationResult.Status and .Errors, not via the error. -func SchemaValidate(ctx context.Context, resources []*unstructured.Unstructured, crds []*extv1.CustomResourceDefinition) (*ValidationResult, error) { //nolint:gocognit // validation has many branches +func SchemaValidate(ctx context.Context, resources []*unstructured.Unstructured, crds []*extv1.CustomResourceDefinition) (*ValidationResult, error) { schemaValidators, structurals, err := newValidatorsAndStructurals(crds) if err != nil { return nil, errors.Wrap(err, "cannot create schema validators") @@ -51,58 +51,68 @@ func SchemaValidate(ctx context.Context, resources []*unstructured.Unstructured, result := &ValidationResult{ Resources: make([]ResourceValidationResult, 0, len(resources)), } - for _, r := range resources { - gvk := r.GetObjectKind().GroupVersionKind() - rvr := ResourceValidationResult{ - APIVersion: gvk.GroupVersion().String(), - Kind: gvk.Kind, - Name: getResourceName(r), - Namespace: r.GetNamespace(), - } + result.Resources = append(result.Resources, validateResource(ctx, r, schemaValidators, structurals, crds)) + } + result.Summary = computeSummary(result.Resources) + return result, nil +} - sv, ok := schemaValidators[gvk] - s := structurals[gvk] +// validateResource runs every check (schema, CEL, unknown fields, defaulting) +// against a single resource and returns its ResourceValidationResult. It is +// the per-resource decomposition of SchemaValidate; pulling it out keeps the +// outer function a clean fan-out and lets each branch read top-to-bottom. +func validateResource( + ctx context.Context, + r *unstructured.Unstructured, + schemaValidators map[runtimeschema.GroupVersionKind][]*validation.SchemaValidator, + structurals map[runtimeschema.GroupVersionKind]*schema.Structural, + crds []*extv1.CustomResourceDefinition, +) ResourceValidationResult { + gvk := r.GetObjectKind().GroupVersionKind() + rvr := ResourceValidationResult{ + APIVersion: gvk.GroupVersion().String(), + Kind: gvk.Kind, + Name: getResourceName(r), + Namespace: r.GetNamespace(), + } - if !ok { - rvr.Status = ValidationStatusMissingSchema - result.Resources = append(result.Resources, rvr) - continue - } + sv, ok := schemaValidators[gvk] + if !ok { + rvr.Status = ValidationStatusMissingSchema + return rvr + } + s := structurals[gvk] + + // A defaulting failure is recorded as a warning-class error and does + // not abort validation: schema and CEL checks still run on the + // (un-defaulted) resource so the user sees every problem at once, + // matching the historical behavior of the SchemaValidation entry + // point this package replaced. + if err := applyDefaults(r, gvk, crds); err != nil { + rvr.Errors = append(rvr.Errors, FieldValidationError{ + Type: FieldErrorTypeDefaulting, + Message: err.Error(), + }) + } - // A defaulting failure is recorded as a warning-class error and does - // not abort validation: schema and CEL checks still run on the - // (un-defaulted) resource so the user sees every problem at once, - // matching the historical behavior of the SchemaValidation entry - // point this package replaced. - if err := applyDefaults(r, gvk, crds); err != nil { - rvr.Errors = append(rvr.Errors, FieldValidationError{ - Type: FieldErrorTypeDefaulting, - Message: err.Error(), - }) + for _, v := range sv { + for _, e := range validation.ValidateCustomResource(nil, r, *v) { + rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeSchema)) } - - for _, v := range sv { - for _, e := range validation.ValidateCustomResource(nil, r, *v) { - rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeSchema)) - } - for _, e := range validateUnknownFields(r.UnstructuredContent(), s) { - rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeUnknownField)) - } - - celValidator := cel.NewValidator(s, true, celconfig.PerCallLimit) - celErrs, _ := celValidator.Validate(ctx, nil, s, r.Object, nil, celconfig.PerCallLimit) - for _, e := range celErrs { - rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeCEL)) - } + for _, e := range validateUnknownFields(r.UnstructuredContent(), s) { + rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeUnknownField)) } - rvr.Status = statusFromErrors(rvr.Errors) - result.Resources = append(result.Resources, rvr) + celValidator := cel.NewValidator(s, true, celconfig.PerCallLimit) + celErrs, _ := celValidator.Validate(ctx, nil, s, r.Object, nil, celconfig.PerCallLimit) + for _, e := range celErrs { + rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeCEL)) + } } - result.Summary = computeSummary(result.Resources) - return result, nil + rvr.Status = statusFromErrors(rvr.Errors) + return rvr } // ResultError returns an error summarizing the outcome of validation, or nil diff --git a/cmd/crossplane/pkg/validate/validate_test.go b/cmd/crossplane/pkg/validate/validate_test.go index 47909fa..6b604ac 100644 --- a/cmd/crossplane/pkg/validate/validate_test.go +++ b/cmd/crossplane/pkg/validate/validate_test.go @@ -17,10 +17,10 @@ limitations under the License. package validate import ( - "context" "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -186,9 +186,13 @@ func TestSchemaValidate(t *testing.T) { resources []*unstructured.Unstructured crds []*extv1.CustomResourceDefinition } + // expect declares everything we assert about a single resource's result: + // its Status and the exact set of FieldValidationErrors. Message and + // Value are compared with cmpopts.IgnoreFields below, since k8s + // validation libraries phrase those strings differently across versions. type expect struct { - status ValidationStatus - errorTypes []string // expected FieldValidationError.Type values (order-independent, subset) + status ValidationStatus + errors []FieldValidationError } type want struct { summary ValidationSummary @@ -213,14 +217,19 @@ func TestSchemaValidate(t *testing.T) { }, }, "InvalidSchema": { - reason: "A resource violating OpenAPI schema should be Invalid with a schema-type field error.", + reason: "A resource violating OpenAPI schema should be Invalid with a schema-type field error on spec.replicas.", args: args{ resources: []*unstructured.Unstructured{invalidSchemaResource}, crds: []*extv1.CustomResourceDefinition{testCRD}, }, want: want{ summary: ValidationSummary{Total: 1, Invalid: 1}, - perRes: []expect{{status: ValidationStatusInvalid, errorTypes: []string{FieldErrorTypeSchema}}}, + perRes: []expect{{ + status: ValidationStatusInvalid, + errors: []FieldValidationError{ + {Type: FieldErrorTypeSchema, Field: "spec.replicas"}, + }, + }}, }, }, "InvalidCEL": { @@ -231,7 +240,13 @@ func TestSchemaValidate(t *testing.T) { }, want: want{ summary: ValidationSummary{Total: 1, Invalid: 1}, - perRes: []expect{{status: ValidationStatusInvalid, errorTypes: []string{FieldErrorTypeCEL}}}, + perRes: []expect{{ + status: ValidationStatusInvalid, + errors: []FieldValidationError{ + // Field is the CEL rule's location, "spec" in this fixture. + {Type: FieldErrorTypeCEL, Field: "spec"}, + }, + }}, }, }, "MissingSchema": { @@ -246,14 +261,19 @@ func TestSchemaValidate(t *testing.T) { }, }, "UnknownField": { - reason: "A resource with a field not declared in the schema should surface an unknownField error.", + reason: "A resource with a field not declared in the schema should surface an unknownField error on the offending field path.", args: args{ resources: []*unstructured.Unstructured{unknownFieldResource}, crds: []*extv1.CustomResourceDefinition{testCRD}, }, want: want{ summary: ValidationSummary{Total: 1, Invalid: 1}, - perRes: []expect{{status: ValidationStatusInvalid, errorTypes: []string{FieldErrorTypeUnknownField}}}, + perRes: []expect{{ + status: ValidationStatusInvalid, + errors: []FieldValidationError{ + {Type: FieldErrorTypeUnknownField, Field: "spec.unknownField"}, + }, + }}, }, }, "DefaultingFailureOnly": { @@ -266,11 +286,16 @@ func TestSchemaValidate(t *testing.T) { }, want: want{ summary: ValidationSummary{Total: 1, Valid: 1}, - perRes: []expect{{status: ValidationStatusDefaultingFailed, errorTypes: []string{FieldErrorTypeDefaulting}}}, + perRes: []expect{{ + status: ValidationStatusDefaultingFailed, + errors: []FieldValidationError{ + {Type: FieldErrorTypeDefaulting}, + }, + }}, }, }, "DefaultingFailureWithSchemaError": { - reason: "Schema validation must still run when defaulting fails; both errors must be reported and the resource must be Invalid (not DefaultingFailed).", + reason: "Schema validation must still run when defaulting fails; both errors must be reported in order, and the resource must be Invalid (not DefaultingFailed).", args: args{ // Same multi-CRD trick as DefaultingFailureOnly to force a defaulting failure, // but the resource has a wrong-typed replicas value so schema validation also fails. @@ -282,8 +307,13 @@ func TestSchemaValidate(t *testing.T) { want: want{ summary: ValidationSummary{Total: 1, Invalid: 1}, perRes: []expect{{ - status: ValidationStatusInvalid, - errorTypes: []string{FieldErrorTypeDefaulting, FieldErrorTypeSchema}, + status: ValidationStatusInvalid, + errors: []FieldValidationError{ + // Defaulting comes first because the implementation + // records it before running schema/CEL checks. + {Type: FieldErrorTypeDefaulting}, + {Type: FieldErrorTypeSchema, Field: "spec.replicas"}, + }, }}, }, }, @@ -305,16 +335,25 @@ func TestSchemaValidate(t *testing.T) { summary: ValidationSummary{Total: 3, Valid: 1, Invalid: 1, MissingSchemas: 1}, perRes: []expect{ {status: ValidationStatusValid}, - {status: ValidationStatusInvalid, errorTypes: []string{FieldErrorTypeSchema}}, + { + status: ValidationStatusInvalid, + errors: []FieldValidationError{ + {Type: FieldErrorTypeSchema, Field: "spec.replicas"}, + }, + }, {status: ValidationStatusMissingSchema}, }, }, }, } + // Message and Value text comes from k8s validator libraries and changes + // across versions; we assert on Type and Field instead. + ignoreErrTextFields := cmpopts.IgnoreFields(FieldValidationError{}, "Message", "Value") + for name, tc := range cases { t.Run(name, func(t *testing.T) { - result, err := SchemaValidate(context.Background(), tc.args.resources, tc.args.crds) + result, err := SchemaValidate(t.Context(), tc.args.resources, tc.args.crds) if (err != nil) != tc.want.wantErr { t.Fatalf("%s\nSchemaValidate() err = %v, wantErr = %v", tc.reason, err, tc.want.wantErr) } @@ -332,11 +371,20 @@ func TestSchemaValidate(t *testing.T) { if r.Status != exp.status { t.Errorf("%s\nResources[%d].Status = %q, want %q", tc.reason, i, r.Status, exp.status) } - if !containsAllErrorTypes(r.Errors, exp.errorTypes) { - t.Errorf("%s\nResources[%d].Errors = %+v; want to include types %v", tc.reason, i, r.Errors, exp.errorTypes) + // Treat a nil-valued exp.errors as "expect no errors". Use a + // concrete empty slice instead of nil for the comparison so + // cmp doesn't trip on slice-nilness when the validator + // returns `nil`. + wantErrs := exp.errors + if wantErrs == nil { + wantErrs = []FieldValidationError{} + } + gotErrs := r.Errors + if gotErrs == nil { + gotErrs = []FieldValidationError{} } - if len(exp.errorTypes) == 0 && len(r.Errors) != 0 { - t.Errorf("%s\nResources[%d].Errors = %+v; want empty", tc.reason, i, r.Errors) + if diff := cmp.Diff(wantErrs, gotErrs, ignoreErrTextFields); diff != "" { + t.Errorf("%s\nResources[%d].Errors mismatch (-want +got):\n%s", tc.reason, i, diff) } } }) @@ -385,21 +433,3 @@ func TestResultError(t *testing.T) { }) } } - -// containsAllErrorTypes returns true when every wanted type appears at least -// once in the given FieldValidationError slice. -func containsAllErrorTypes(errs []FieldValidationError, wantTypes []string) bool { - if len(wantTypes) == 0 { - return true - } - seen := make(map[string]bool, len(errs)) - for _, e := range errs { - seen[e.Type] = true - } - for _, t := range wantTypes { - if !seen[t] { - return false - } - } - return true -} diff --git a/cmd/crossplane/validate/cmd.go b/cmd/crossplane/validate/cmd.go index b8627a3..ad43579 100644 --- a/cmd/crossplane/validate/cmd.go +++ b/cmd/crossplane/validate/cmd.go @@ -20,15 +20,12 @@ package validate import ( "context" "fmt" - "io" "os" "path/filepath" "strings" "github.com/alecthomas/kong" "github.com/spf13/afero" - extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/crossplane/crossplane-runtime/v2/pkg/logging" @@ -126,27 +123,14 @@ func (c *Cmd) Run(k *kong.Context, _ logging.Logger) error { return errors.Wrapf(err, "cannot download and load cache") } - // Validate resources against schemas and render in the requested format. - if err := c.validateAndRender(context.Background(), resources, m.crds, k.Stdout); err != nil { - return errors.Wrapf(err, "cannot validate resources") - } - - return nil -} - -// validateAndRender runs schema validation on the given resources and CRDs, -// writes the result to w in the format configured on the Cmd, and returns a -// non-nil error when validation failed (or when ErrorOnMissingSchemas is set -// and any resource had no matching schema). It is the core of Cmd.Run, -// extracted so that tests can exercise the flag-driven behaviour without the -// file/cache loading machinery. -func (c *Cmd) validateAndRender(ctx context.Context, resources []*unstructured.Unstructured, crds []*extv1.CustomResourceDefinition, w io.Writer) error { - result, err := pkgvalidate.SchemaValidate(ctx, resources, crds) + // Validate resources against schemas, render in the requested format, + // and return a CLI-shaped error when validation didn't pass. + result, err := pkgvalidate.SchemaValidate(context.Background(), resources, m.crds) if err != nil { - return err + return errors.Wrapf(err, "cannot validate resources") } - if err := render.RenderValidationResult(result, render.OutputFormat(c.Output), w, render.RenderOptions{SkipSuccessResults: c.SkipSuccessResults}); err != nil { + if err := render.OutputFormat(c.Output).Render(result, k.Stdout, render.RenderOptions{SkipSuccessResults: c.SkipSuccessResults}); err != nil { return errors.Wrap(err, "cannot render validation result") } diff --git a/cmd/crossplane/validate/cmd_test.go b/cmd/crossplane/validate/cmd_test.go index 8687222..95e341f 100644 --- a/cmd/crossplane/validate/cmd_test.go +++ b/cmd/crossplane/validate/cmd_test.go @@ -18,186 +18,265 @@ package validate import ( "bytes" - "context" "encoding/json" "strings" "testing" "github.com/alecthomas/kong" - extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "sigs.k8s.io/yaml" + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" + pkgvalidate "github.com/crossplane/cli/v2/cmd/crossplane/pkg/validate" ) -// parseCmdFlags exercises Kong's parsing of the Cmd struct tags. It builds a -// Cmd with fake positional args so we can focus on flag behaviour. -func parseCmdFlags(t *testing.T, args ...string) *Cmd { +// runCmd parses the given CLI args through Kong and invokes the validate +// command's Run, returning whatever was written to stdout plus the error +// returned by Run. Tests interact with the command exactly the way a real +// CLI invocation would, so the helper does not bypass any of the +// production code paths in cmd.go. +func runCmd(t *testing.T, args ...string) (string, error) { t.Helper() - c := &Cmd{} - // Positional args Extensions and Resources are required; supply dummies. - full := append([]string{"ext.yaml", "res.yaml"}, args...) - parser, err := kong.New(c) + var c Cmd + parser, err := kong.New(&c) if err != nil { t.Fatalf("kong.New(): %v", err) } - if _, err := parser.Parse(full); err != nil { - t.Fatalf("kong.Parse(%v): %v", full, err) - } - return c -} - -func TestCmd_OutputFlagParsing(t *testing.T) { - cases := map[string]struct { - args []string - want string - }{ - "default": {args: nil, want: "text"}, - "long_text": {args: []string{"--output=text"}, want: "text"}, - "long_json": {args: []string{"--output=json"}, want: "json"}, - "long_yaml": {args: []string{"--output=yaml"}, want: "yaml"}, - "short_flag": {args: []string{"-o", "json"}, want: "json"}, - } - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - c := parseCmdFlags(t, tc.args...) - if c.Output != tc.want { - t.Errorf("Cmd.Output = %q; want %q", c.Output, tc.want) - } - }) + kongCtx, err := parser.Parse(args) + if err != nil { + return "", err } + var stdout bytes.Buffer + kongCtx.Stdout = &stdout + runErr := c.Run(kongCtx, logging.NewNopLogger()) + return stdout.String(), runErr } -func TestCmd_OutputFlagRejectsUnknown(t *testing.T) { - c := &Cmd{} - parser, err := kong.New(c) +// runCmdOK is runCmd plus a t.Fatal on parse error, used when the test +// is asserting against Run's behaviour rather than Kong's parsing. +func runCmdOK(t *testing.T, args ...string) (string, error) { + t.Helper() + var c Cmd + parser, err := kong.New(&c) if err != nil { t.Fatalf("kong.New(): %v", err) } - if _, err := parser.Parse([]string{"ext.yaml", "res.yaml", "--output=xml"}); err == nil { - t.Errorf("kong.Parse(--output=xml) = nil; want non-nil enum-validation error") + kongCtx, err := parser.Parse(args) + if err != nil { + t.Fatalf("kong.Parse(%v): %v", args, err) } + var stdout bytes.Buffer + kongCtx.Stdout = &stdout + runErr := c.Run(kongCtx, logging.NewNopLogger()) + return stdout.String(), runErr } -// invalidFixture mirrors the `Invalid` case from the backward-compat test — -// a resource whose replicas field is the wrong type. -func invalidFixture() ([]*unstructured.Unstructured, []*extv1.CustomResourceDefinition) { - return []*unstructured.Unstructured{{Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{"name": "test"}, - "spec": map[string]any{"replicas": "not-an-int"}, - }}}, - []*extv1.CustomResourceDefinition{testCRD} -} - -func validFixture() ([]*unstructured.Unstructured, []*extv1.CustomResourceDefinition) { - return []*unstructured.Unstructured{{Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{"name": "test"}, - "spec": map[string]any{"replicas": 1}, - }}}, - []*extv1.CustomResourceDefinition{testCRD} +// commonArgs are the fixture arguments shared by every e2e test: +// pre-populated cache + a stand-in crossplane image whose package.yaml +// lives under testdata/cache. Both keep the test offline. +var commonArgs = []string{ + "--cache-dir=testdata/cache", + "--crossplane-image=xpkg.crossplane.io/crossplane/crossplane:v0.0.0-test", } -func missingFixture() ([]*unstructured.Unstructured, []*extv1.CustomResourceDefinition) { - return []*unstructured.Unstructured{{Object: map[string]any{ - "apiVersion": "test.org/v1alpha1", - "kind": "Test", - "metadata": map[string]any{"name": "test"}, - "spec": map[string]any{"replicas": 1}, - }}}, - []*extv1.CustomResourceDefinition{} -} - -func TestCmd_ValidateAndRender(t *testing.T) { - type fixture func() ([]*unstructured.Unstructured, []*extv1.CustomResourceDefinition) - type want struct { - stdoutSubstr string - wantErr bool - parseAs string // "json", "yaml", or "" for no structural parse +// TestCmd_OutputEnumValidation asserts that Kong rejects unknown --output +// values before Run is called. +func TestCmd_OutputEnumValidation(t *testing.T) { + args := append([]string{ + "testdata/cmd/crd.yaml", + "testdata/cmd/resources_valid.yaml", + "--output=xml", + }, commonArgs...) + _, err := runCmd(t, args...) + if err == nil { + t.Errorf("kong.Parse(--output=xml) = nil; want enum-validation error") } +} +// TestCmd_Run drives the validate command end-to-end through Kong. Each +// case writes real fixture files and a real validation result to a real +// stdout writer; nothing is mocked. The pre-populated testdata/cache +// directory keeps the run offline. +func TestCmd_Run(t *testing.T) { cases := map[string]struct { - output string - errorOnMissingSchemas bool - skipSuccess bool - fix fixture - want want + extensions string + resources string + extraArgs []string + wantErr bool + // assertText is invoked when --output is text (the default). It + // receives the captured stdout. + assertText func(t *testing.T, stdout string) + // assertJSON is invoked when --output=json. It is given the + // already-parsed ValidationResult. + assertJSON func(t *testing.T, result *pkgvalidate.ValidationResult) + // assertYAML, same idea but for --output=yaml. + assertYAML func(t *testing.T, result *pkgvalidate.ValidationResult) }{ - "default_text_valid": { - output: "text", fix: validFixture, - want: want{stdoutSubstr: "[✓] test.org/v1alpha1, Kind=Test, test validated successfully"}, + "DefaultText_Valid": { + extensions: "testdata/cmd/crd.yaml", + resources: "testdata/cmd/resources_valid.yaml", + assertText: func(t *testing.T, out string) { + if !strings.Contains(out, "[✓] cmd.example.org/v1alpha1, Kind=Test, ok-instance") { + t.Errorf("missing success line in output:\n%s", out) + } + if !strings.Contains(out, "Total 1 resources: 0 missing schemas, 1 success cases, 0 failure cases") { + t.Errorf("missing summary line in output:\n%s", out) + } + }, }, - "output_text_explicit_invalid": { - output: "text", fix: invalidFixture, - want: want{stdoutSubstr: "[x] schema validation error", wantErr: true}, + "Text_InvalidExitsNonZero": { + extensions: "testdata/cmd/crd.yaml", + resources: "testdata/cmd/resources_invalid.yaml", + wantErr: true, + assertText: func(t *testing.T, out string) { + if !strings.Contains(out, "[x] schema validation error cmd.example.org/v1alpha1, Kind=Test, bad-instance") { + t.Errorf("missing schema-error line in output:\n%s", out) + } + }, }, - "output_json_valid": { - output: "json", fix: validFixture, - want: want{parseAs: "json"}, + "JSON_Valid": { + extensions: "testdata/cmd/crd.yaml", + resources: "testdata/cmd/resources_valid.yaml", + extraArgs: []string{"--output=json"}, + assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + if r.Summary.Total != 1 || r.Summary.Valid != 1 { + t.Errorf("Summary = %+v; want Total=1 Valid=1", r.Summary) + } + if len(r.Resources) != 1 || r.Resources[0].Status != pkgvalidate.ValidationStatusValid { + t.Errorf("Resources = %+v; want one Valid entry", r.Resources) + } + }, }, - "output_yaml_valid": { - output: "yaml", fix: validFixture, - want: want{parseAs: "yaml"}, + "JSON_InvalidExitsNonZero": { + extensions: "testdata/cmd/crd.yaml", + resources: "testdata/cmd/resources_invalid.yaml", + extraArgs: []string{"--output=json"}, + wantErr: true, + assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + if r.Summary.Invalid != 1 { + t.Errorf("Summary.Invalid = %d; want 1", r.Summary.Invalid) + } + if len(r.Resources) != 1 || r.Resources[0].Status != pkgvalidate.ValidationStatusInvalid { + t.Errorf("Resources = %+v; want one Invalid entry", r.Resources) + } + if len(r.Resources[0].Errors) == 0 || r.Resources[0].Errors[0].Type != pkgvalidate.FieldErrorTypeSchema { + t.Errorf("Resources[0].Errors = %+v; want at least one schema error", r.Resources[0].Errors) + } + }, }, - "output_json_invalid_exits_nonzero": { - output: "json", fix: invalidFixture, - want: want{parseAs: "json", wantErr: true}, + "YAML_Valid": { + extensions: "testdata/cmd/crd.yaml", + resources: "testdata/cmd/resources_valid.yaml", + extraArgs: []string{"--output=yaml"}, + assertYAML: func(t *testing.T, r *pkgvalidate.ValidationResult) { + if r.Summary.Total != 1 || r.Summary.Valid != 1 { + t.Errorf("Summary = %+v; want Total=1 Valid=1", r.Summary) + } + }, }, - "output_json_missing_with_flag": { - output: "json", errorOnMissingSchemas: true, fix: missingFixture, - want: want{parseAs: "json", wantErr: true}, + "JSON_MissingSchema_NoFlag": { + // Default behaviour: a missing schema is reported but doesn't fail the run. + extensions: "testdata/cmd/crd.yaml", + resources: "testdata/cmd/resources_missing.yaml", + extraArgs: []string{"--output=json"}, + assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + if r.Summary.MissingSchemas != 1 || r.Summary.Invalid != 0 { + t.Errorf("Summary = %+v; want MissingSchemas=1 Invalid=0", r.Summary) + } + if len(r.Resources) != 1 || r.Resources[0].Status != pkgvalidate.ValidationStatusMissingSchema { + t.Errorf("Resources = %+v; want one MissingSchema entry", r.Resources) + } + }, + }, + "JSON_MissingSchema_WithFlag": { + // --error-on-missing-schemas escalates a missing schema to a non-zero exit. + extensions: "testdata/cmd/crd.yaml", + resources: "testdata/cmd/resources_missing.yaml", + extraArgs: []string{"--output=json", "--error-on-missing-schemas"}, + wantErr: true, + assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + if r.Summary.MissingSchemas != 1 { + t.Errorf("Summary.MissingSchemas = %d; want 1", r.Summary.MissingSchemas) + } + }, }, - "output_json_missing_without_flag": { - output: "json", errorOnMissingSchemas: false, fix: missingFixture, - want: want{parseAs: "json"}, + "SkipSuccessResults_TextSuppressesCheckmark": { + extensions: "testdata/cmd/crd.yaml", + resources: "testdata/cmd/resources_valid.yaml", + extraArgs: []string{"--skip-success-results"}, + assertText: func(t *testing.T, out string) { + if strings.Contains(out, "[✓]") { + t.Errorf("--skip-success-results should suppress [✓] lines; got:\n%s", out) + } + if !strings.Contains(out, "1 success cases") { + t.Errorf("summary should still report success cases; got:\n%s", out) + } + }, }, - "skip_success_with_json_still_has_full_payload": { - output: "json", skipSuccess: true, fix: validFixture, - want: want{parseAs: "json"}, + "SkipSuccessResults_JSONStillIncludesValid": { + // In JSON mode the flag is ignored; success entries remain part + // of the structured payload so consumers can filter themselves. + extensions: "testdata/cmd/crd.yaml", + resources: "testdata/cmd/resources_valid.yaml", + extraArgs: []string{"--output=json", "--skip-success-results"}, + assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + if r.Summary.Valid != 1 { + t.Errorf("--skip-success-results must not strip valid entries from JSON; got %+v", r) + } + }, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - c := &Cmd{Output: tc.output, ErrorOnMissingSchemas: tc.errorOnMissingSchemas, SkipSuccessResults: tc.skipSuccess} - resources, crds := tc.fix() - var buf bytes.Buffer - err := c.validateAndRender(context.Background(), resources, crds, &buf) - if (err != nil) != tc.want.wantErr { - t.Errorf("validateAndRender() err = %v; wantErr = %v", err, tc.want.wantErr) + args := append([]string{tc.extensions, tc.resources}, append(commonArgs, tc.extraArgs...)...) + stdout, err := runCmdOK(t, args...) + if (err != nil) != tc.wantErr { + t.Fatalf("Run() err = %v, wantErr = %v\n--- stdout ---\n%s", err, tc.wantErr, stdout) } + // Strip any package-fetcher chatter the manager writes before + // the validation result so downstream parsers see only the + // payload. + payload := stripFetcherNoise(stdout) - if tc.want.stdoutSubstr != "" && !strings.Contains(buf.String(), tc.want.stdoutSubstr) { - t.Errorf("stdout missing substring %q\n--- got ---\n%s", tc.want.stdoutSubstr, buf.String()) + if tc.assertText != nil { + tc.assertText(t, payload) } - - switch tc.want.parseAs { - case "json": + if tc.assertJSON != nil { var got pkgvalidate.ValidationResult - if e := json.Unmarshal(buf.Bytes(), &got); e != nil { - t.Fatalf("stdout is not valid JSON: %v\n%s", e, buf.String()) - } - if got.Summary.Total == 0 { - t.Errorf("JSON result.Summary.Total == 0; fixture had resources") - } - // For skip_success case, verify Valid results are still included. - if tc.skipSuccess && got.Summary.Valid == 0 { - t.Errorf("--skip-success-results must not strip valid entries from JSON payload; got %+v", got) + if err := json.Unmarshal([]byte(payload), &got); err != nil { + t.Fatalf("stdout is not valid JSON: %v\n%s", err, payload) } - case "yaml": + tc.assertJSON(t, &got) + } + if tc.assertYAML != nil { var got pkgvalidate.ValidationResult - if e := yaml.Unmarshal(buf.Bytes(), &got); e != nil { - t.Fatalf("stdout is not valid YAML: %v\n%s", e, buf.String()) - } - if got.Summary.Total == 0 { - t.Errorf("YAML result.Summary.Total == 0; fixture had resources") + if err := yaml.Unmarshal([]byte(payload), &got); err != nil { + t.Fatalf("stdout is not valid YAML: %v\n%s", err, payload) } + tc.assertYAML(t, &got) } }) } } + +// stripFetcherNoise drops any header lines the manager writes to stdout +// (cache notices, "schemas does not exist, downloading: ...") so the +// downstream parsers see only the validation payload. It looks for the +// first line that starts a payload — JSON {, YAML resources:/summary: +// header, or a text marker like [✓]/[x]/[!]/Total — and returns from there. +func stripFetcherNoise(out string) string { + lines := strings.Split(out, "\n") + for i, l := range lines { + t := strings.TrimSpace(l) + switch { + case strings.HasPrefix(t, "{"): + return strings.Join(lines[i:], "\n") + case strings.HasPrefix(t, "resources:") || strings.HasPrefix(t, "summary:"): + return strings.Join(lines[i:], "\n") + case strings.HasPrefix(t, "[✓]") || strings.HasPrefix(t, "[x]") || strings.HasPrefix(t, "[!]") || strings.HasPrefix(t, "Total"): + return strings.Join(lines[i:], "\n") + } + } + return out +} diff --git a/cmd/crossplane/validate/testdata/cache/xpkg.crossplane.io/crossplane/crossplane@v0.0.0-test/package.yaml b/cmd/crossplane/validate/testdata/cache/xpkg.crossplane.io/crossplane/crossplane@v0.0.0-test/package.yaml new file mode 100644 index 0000000..808d30e --- /dev/null +++ b/cmd/crossplane/validate/testdata/cache/xpkg.crossplane.io/crossplane/crossplane@v0.0.0-test/package.yaml @@ -0,0 +1,25 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: testbuiltins.test.crossplane.io +spec: + group: test.crossplane.io + names: + kind: TestBuiltin + listKind: TestBuiltinList + plural: testbuiltins + singular: testbuiltin + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + foo: + type: string diff --git a/cmd/crossplane/validate/testdata/cmd/crd.yaml b/cmd/crossplane/validate/testdata/cmd/crd.yaml new file mode 100644 index 0000000..cc3bb71 --- /dev/null +++ b/cmd/crossplane/validate/testdata/cmd/crd.yaml @@ -0,0 +1,27 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: tests.cmd.example.org +spec: + group: cmd.example.org + names: + kind: Test + listKind: TestList + plural: tests + singular: test + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + required: + - replicas + properties: + replicas: + type: integer diff --git a/cmd/crossplane/validate/testdata/cmd/resources_invalid.yaml b/cmd/crossplane/validate/testdata/cmd/resources_invalid.yaml new file mode 100644 index 0000000..4951bc1 --- /dev/null +++ b/cmd/crossplane/validate/testdata/cmd/resources_invalid.yaml @@ -0,0 +1,6 @@ +apiVersion: cmd.example.org/v1alpha1 +kind: Test +metadata: + name: bad-instance +spec: + replicas: "not-an-int" diff --git a/cmd/crossplane/validate/testdata/cmd/resources_missing.yaml b/cmd/crossplane/validate/testdata/cmd/resources_missing.yaml new file mode 100644 index 0000000..7db097b --- /dev/null +++ b/cmd/crossplane/validate/testdata/cmd/resources_missing.yaml @@ -0,0 +1,4 @@ +apiVersion: nosuch.example.org/v1 +kind: Unknown +metadata: + name: missing-schema diff --git a/cmd/crossplane/validate/testdata/cmd/resources_valid.yaml b/cmd/crossplane/validate/testdata/cmd/resources_valid.yaml new file mode 100644 index 0000000..161c45f --- /dev/null +++ b/cmd/crossplane/validate/testdata/cmd/resources_valid.yaml @@ -0,0 +1,6 @@ +apiVersion: cmd.example.org/v1alpha1 +kind: Test +metadata: + name: ok-instance +spec: + replicas: 3 From 2b3f641f5f688c6db7b9e7c6409458cef36e2917 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Tue, 2 Jun 2026 13:46:23 -0400 Subject: [PATCH 4/4] refactor(validate): inject Renderer via Kong's MapperValue interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Signed-off-by: Jonathan Ogilvie --- cmd/crossplane/pkg/validate/render/render.go | 147 +++++++++--------- .../pkg/validate/render/render_test.go | 108 ++++++++----- cmd/crossplane/pkg/validate/types.go | 19 ++- cmd/crossplane/pkg/validate/unknown_fields.go | 3 +- cmd/crossplane/pkg/validate/validate.go | 75 +++++---- cmd/crossplane/validate/cmd.go | 44 +++++- ...d_test.go => validate_integration_test.go} | 101 ++++++------ cmd/crossplane/validate/validate_test.go | 128 ++++----------- 8 files changed, 323 insertions(+), 302 deletions(-) rename cmd/crossplane/validate/{cmd_test.go => validate_integration_test.go} (74%) diff --git a/cmd/crossplane/pkg/validate/render/render.go b/cmd/crossplane/pkg/validate/render/render.go index 615039c..e48955e 100644 --- a/cmd/crossplane/pkg/validate/render/render.go +++ b/cmd/crossplane/pkg/validate/render/render.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "io" + "strings" "sigs.k8s.io/yaml" @@ -31,17 +32,39 @@ 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" ) +// Per-line markers used in text output. Lifted into named constants so +// the line formats below read as " " rather than +// repeating the bracket literal at every call site. +const ( + markerSuccess = "[✓]" // resource passed validation + markerWarning = "[!]" // non-fatal: missing schema or defaulting failure + markerError = "[x]" // fatal: schema, CEL, or unknown-field failure +) + // 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 +78,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 @@ -143,49 +136,51 @@ func (yamlRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, _ // A trailing summary line lists totals. type textRenderer struct{} -// Render implements Renderer. -func (textRenderer) Render(result *pkgvalidate.ValidationResult, w io.Writer, opts RenderOptions) error { +// Render implements Renderer. The outer switch over ValidationStatus +// dispatches per-resource emission; the per-error switch over +// FieldErrorType lives in textErrorLine (a different enumeration +// covering a different concern, so it earns its own helper). +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) + var line string switch r.Status { case pkgvalidate.ValidationStatusMissingSchema: - if _, err := fmt.Fprintf(w, "[!] could not find CRD/XRD for: %s\n", gvk); err != nil { - return err - } + line = fmt.Sprintf(markerWarning+" could not find CRD/XRD for: %s\n", gvk) case pkgvalidate.ValidationStatusValid: if opts.SkipSuccessResults { continue } - if _, err := fmt.Fprintf(w, "[✓] %s, %s validated successfully\n", gvk, r.Name); err != nil { - return err - } + line = fmt.Sprintf(markerSuccess+" %s, %s validated successfully\n", gvk, r.Name) case pkgvalidate.ValidationStatusInvalid, pkgvalidate.ValidationStatusDefaultingFailed: + var sb strings.Builder for _, e := range r.Errors { - if err := writeTextErrorLine(w, gvk, r.Name, e); err != nil { - return err - } + sb.WriteString(textErrorLine(gvk, r.Name, e)) } + line = sb.String() + } + if _, err := fmt.Fprint(w, line); err != nil { + return errors.Wrap(err, errCannotWriteOutput) } } _, err := fmt.Fprintf(w, "Total %d resources: %d missing schemas, %d success cases, %d failure cases\n", result.Summary.Total, result.Summary.MissingSchemas, result.Summary.Valid, result.Summary.Invalid) - return err + return errors.Wrap(err, errCannotWriteOutput) } -// writeTextErrorLine emits a single per-error line. Defaulting failures use -// the [!] warning prefix; schema, CEL, and unknown-field errors use [x]. -// Kept private to the textRenderer because the per-error format is a -// detail of the text output, not part of the package's public surface. -func writeTextErrorLine(w io.Writer, gvk, name string, e pkgvalidate.FieldValidationError) error { +// textErrorLine returns the rendered text for a single +// FieldValidationError. Defaulting failures use the warning marker; +// schema, CEL, and unknown-field errors use the error marker. +func textErrorLine(gvk, name string, e pkgvalidate.FieldValidationError) string { switch e.Type { case pkgvalidate.FieldErrorTypeDefaulting: - _, err := fmt.Fprintf(w, "[!] failed to apply defaults for %s, %s: %s\n", gvk, name, e.Message) - return err + return fmt.Sprintf(markerWarning+" failed to apply defaults for %s, %s: %s\n", gvk, name, e.Message) case pkgvalidate.FieldErrorTypeCEL: - _, err := fmt.Fprintf(w, "[x] CEL validation error %s, %s : %s\n", gvk, name, e.Message) - return err + return fmt.Sprintf(markerError+" CEL validation error %s, %s : %s\n", gvk, name, e.Message) + case pkgvalidate.FieldErrorTypeSchema, pkgvalidate.FieldErrorTypeUnknownField: + return fmt.Sprintf(markerError+" schema validation error %s, %s : %s\n", gvk, name, e.Message) default: - _, err := fmt.Fprintf(w, "[x] schema validation error %s, %s : %s\n", gvk, name, e.Message) - return err + // Breadcrumb for an unhandled FieldErrorType added without updating this switch. + return fmt.Sprintf(markerError+" validation error %s, %s : %s\n", gvk, name, e.Message) } } diff --git a/cmd/crossplane/pkg/validate/render/render_test.go b/cmd/crossplane/pkg/validate/render/render_test.go index 7f1b89c..615fd1e 100644 --- a/cmd/crossplane/pkg/validate/render/render_test.go +++ b/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) + } + }) } } diff --git a/cmd/crossplane/pkg/validate/types.go b/cmd/crossplane/pkg/validate/types.go index 84e1064..1b3e0b8 100644 --- a/cmd/crossplane/pkg/validate/types.go +++ b/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" ) diff --git a/cmd/crossplane/pkg/validate/unknown_fields.go b/cmd/crossplane/pkg/validate/unknown_fields.go index c2dd0d4..55458e5 100644 --- a/cmd/crossplane/pkg/validate/unknown_fields.go +++ b/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] diff --git a/cmd/crossplane/pkg/validate/validate.go b/cmd/crossplane/pkg/validate/validate.go index 3ae13e0..ec1be1f 100644 --- a/cmd/crossplane/pkg/validate/validate.go +++ b/cmd/crossplane/pkg/validate/validate.go @@ -18,7 +18,6 @@ package validate import ( "context" - "fmt" ext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -42,6 +41,10 @@ import ( // returned error is non-nil only for setup failures (for example, a CRD that // cannot be converted or compiled); per-resource validation failures are // reported via ResourceValidationResult.Status and .Errors, not via the error. +// +// Caller-owned resources are not mutated. SchemaValidate operates on a deep +// copy of each input, so the structural defaulting and unknown-field pruning +// it performs internally are not visible after the call returns. func SchemaValidate(ctx context.Context, resources []*unstructured.Unstructured, crds []*extv1.CustomResourceDefinition) (*ValidationResult, error) { schemaValidators, structurals, err := newValidatorsAndStructurals(crds) if err != nil { @@ -62,19 +65,23 @@ func SchemaValidate(ctx context.Context, resources []*unstructured.Unstructured, // against a single resource and returns its ResourceValidationResult. It is // the per-resource decomposition of SchemaValidate; pulling it out keeps the // outer function a clean fan-out and lets each branch read top-to-bottom. +// +// The input resource is not mutated. validateResource takes a deep copy +// before applyDefaults / validateUnknownFields, both of which would +// otherwise modify the caller's r.Object in place. func validateResource( ctx context.Context, - r *unstructured.Unstructured, - schemaValidators map[runtimeschema.GroupVersionKind][]*validation.SchemaValidator, + in *unstructured.Unstructured, + schemaValidators map[runtimeschema.GroupVersionKind]*validation.SchemaValidator, structurals map[runtimeschema.GroupVersionKind]*schema.Structural, crds []*extv1.CustomResourceDefinition, ) ResourceValidationResult { - gvk := r.GetObjectKind().GroupVersionKind() + gvk := in.GetObjectKind().GroupVersionKind() rvr := ResourceValidationResult{ APIVersion: gvk.GroupVersion().String(), Kind: gvk.Kind, - Name: getResourceName(r), - Namespace: r.GetNamespace(), + Name: getResourceName(in), + Namespace: in.GetNamespace(), } sv, ok := schemaValidators[gvk] @@ -84,6 +91,13 @@ func validateResource( } s := structurals[gvk] + // Work on a copy: applyDefaults writes defaults into r.Object and + // validateUnknownFields prunes unknown keys from it. Mutating + // caller-owned manifests would make repeated validations + // non-deterministic and surprise downstream code that reuses the + // inputs. + r := in.DeepCopy() + // A defaulting failure is recorded as a warning-class error and does // not abort validation: schema and CEL checks still run on the // (un-defaulted) resource so the user sees every problem at once, @@ -96,19 +110,17 @@ func validateResource( }) } - for _, v := range sv { - for _, e := range validation.ValidateCustomResource(nil, r, *v) { - rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeSchema)) - } - for _, e := range validateUnknownFields(r.UnstructuredContent(), s) { - rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeUnknownField)) - } + for _, e := range validation.ValidateCustomResource(nil, r, *sv) { + rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeSchema)) + } + for _, e := range validateUnknownFields(r.UnstructuredContent(), s) { + rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeUnknownField)) + } - celValidator := cel.NewValidator(s, true, celconfig.PerCallLimit) - celErrs, _ := celValidator.Validate(ctx, nil, s, r.Object, nil, celconfig.PerCallLimit) - for _, e := range celErrs { - rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeCEL)) - } + celValidator := cel.NewValidator(s, true, celconfig.PerCallLimit) + celErrs, _ := celValidator.Validate(ctx, nil, s, r.Object, nil, celconfig.PerCallLimit) + for _, e := range celErrs { + rvr.Errors = append(rvr.Errors, fieldErrorToFieldValidationError(e, FieldErrorTypeCEL)) } rvr.Status = statusFromErrors(rvr.Errors) @@ -128,8 +140,14 @@ func ResultError(result *ValidationResult, errorOnMissingSchemas bool) error { return nil } -func newValidatorsAndStructurals(crds []*extv1.CustomResourceDefinition) (map[runtimeschema.GroupVersionKind][]*validation.SchemaValidator, map[runtimeschema.GroupVersionKind]*schema.Structural, error) { - validators := map[runtimeschema.GroupVersionKind][]*validation.SchemaValidator{} +// newValidatorsAndStructurals compiles a single SchemaValidator and Structural +// per (group, version, kind) declared by the input CRDs. Duplicate CRD entries +// for the same GVK are last-wins, mirroring the behaviour of the structurals +// map; this keeps validateResource from running schema, CEL, and +// unknown-field checks more than once and avoids duplicated errors in the +// reported result. +func newValidatorsAndStructurals(crds []*extv1.CustomResourceDefinition) (map[runtimeschema.GroupVersionKind]*validation.SchemaValidator, map[runtimeschema.GroupVersionKind]*schema.Structural, error) { + validators := map[runtimeschema.GroupVersionKind]*validation.SchemaValidator{} structurals := map[runtimeschema.GroupVersionKind]*schema.Structural{} for i := range crds { @@ -140,11 +158,6 @@ func newValidatorsAndStructurals(crds []*extv1.CustomResourceDefinition) (map[ru // Top-level and per-version schemas are mutually exclusive. for _, ver := range internal.Spec.Versions { - var ( - sv validation.SchemaValidator - err error - ) - gvk := runtimeschema.GroupVersionKind{ Group: internal.Spec.Group, Version: ver.Name, @@ -163,12 +176,12 @@ func newValidatorsAndStructurals(crds []*extv1.CustomResourceDefinition) (map[ru continue } - sv, _, err = validation.NewSchemaValidator(s) + sv, _, err := validation.NewSchemaValidator(s) if err != nil { return nil, nil, err } - validators[gvk] = append(validators[gvk], &sv) + validators[gvk] = &sv structural, err := schema.NewStructural(s) if err != nil { @@ -205,7 +218,7 @@ func statusFromErrors(errs []FieldValidationError) ValidationStatus { } // fieldErrorToFieldValidationError converts a k8s field.Error into our structured type. -func fieldErrorToFieldValidationError(e *field.Error, errType string) FieldValidationError { +func fieldErrorToFieldValidationError(e *field.Error, errType FieldErrorType) FieldValidationError { out := FieldValidationError{ Type: errType, Field: e.Field, @@ -277,19 +290,19 @@ func applyDefaults(resource *unstructured.Unstructured, gvk runtimeschema.GroupV } if schemaProps == nil { - return fmt.Errorf("no schema found for version %s in CRD %s", gvk.Version, matchingCRD.Name) + return errors.Errorf("no schema found for version %s in CRD %s", gvk.Version, matchingCRD.Name) } var apiExtSchema ext.JSONSchemaProps err := extv1.Convert_v1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(schemaProps, &apiExtSchema, nil) if err != nil { - return fmt.Errorf("failed to convert schema: %w", err) + return errors.Wrap(err, "cannot convert schema") } structural, err := schema.NewStructural(&apiExtSchema) if err != nil { - return fmt.Errorf("failed to create structural schema: %w", err) + return errors.Wrap(err, "cannot create structural schema") } obj := resource.UnstructuredContent() diff --git a/cmd/crossplane/validate/cmd.go b/cmd/crossplane/validate/cmd.go index ad43579..037c747 100644 --- a/cmd/crossplane/validate/cmd.go +++ b/cmd/crossplane/validate/cmd.go @@ -45,6 +45,32 @@ var helpDetail string // validate command writes to its output writer. const errWriteOutput = "cannot write output" +// rendererFlag adapts render.RendererFor to Kong's MapperValue interface +// so the --output flag is decoded straight into a typed render.Renderer +// at parse time. Cmd then carries the resolved renderer as a dependency +// instead of a format identifier. +// +// The wrapper lives in this package (the CLI consumer) rather than in +// render so the render package stays free of any kong dependency and +// can be imported by non-CLI consumers like crossplane-diff. +type rendererFlag struct { + render.Renderer +} + +// Decode implements kong.MapperValue. +func (f *rendererFlag) Decode(ctx *kong.DecodeContext) error { + var s string + if err := ctx.Scan.PopValueInto("output", &s); err != nil { + return err + } + r, err := render.RendererFor(render.OutputFormat(s)) + if err != nil { + return err + } + f.Renderer = r + return nil +} + // Cmd arguments and flags for render subcommand. type Cmd struct { // Arguments. @@ -52,13 +78,17 @@ type Cmd struct { Resources string `arg:"" help:"Resource sources as a comma-separated list of files, directories, or '-' for standard input."` // Flags. Keep them in alphabetical order. - CacheDir string `default:"~/.crossplane/cache" help:"Absolute path to the cache directory for downloaded schemas." predictor:"directory"` + CacheDir string `default:"~/.crossplane/cache" help:"Absolute path to the cache directory for downloaded schemas." predictor:"directory"` CleanCache bool `help:"Clean the cache directory before downloading package schemas."` CrossplaneImage string `help:"Specify the Crossplane image for validating built-in schemas."` ErrorOnMissingSchemas bool `default:"false" help:"Return non zero exit code if missing schemas."` - Output string `default:"text" enum:"text,json,yaml" help:"Output format for validation results (text, json, or yaml)." short:"o"` - SkipSuccessResults bool `help:"Skip printing success results."` - UpdateCache bool `default:"false" help:"Update cached schemas by downloading the latest version that satisfies a constraint. May be useful if you are using semantic version constraints and want to get the latest version, but this slows down the cache lookup due to the required network calls."` + // rendererFlag.Decode rejects unknown formats, which is what Kong's + // "enum" tag would normally enforce — but enum doesn't apply to + // MapperValue-backed fields. The help text is the user-facing list + // of valid values. + Output rendererFlag `default:"text" help:"Output format for validation results (text, json, or yaml)." short:"o"` + SkipSuccessResults bool `help:"Skip printing success results."` + UpdateCache bool `default:"false" help:"Update cached schemas by downloading the latest version that satisfies a constraint. May be useful if you are using semantic version constraints and want to get the latest version, but this slows down the cache lookup due to the required network calls."` fs afero.Fs } @@ -68,7 +98,9 @@ func (c *Cmd) Help() string { return helpDetail } -// AfterApply implements kong.AfterApply. +// AfterApply implements kong.AfterApply. The renderer is already resolved +// by Kong's MapperValue plumbing on Cmd.Output by the time this runs, so +// AfterApply only sets the filesystem. func (c *Cmd) AfterApply() error { c.fs = afero.NewOsFs() return nil @@ -130,7 +162,7 @@ func (c *Cmd) Run(k *kong.Context, _ logging.Logger) error { return errors.Wrapf(err, "cannot validate resources") } - if err := render.OutputFormat(c.Output).Render(result, k.Stdout, render.RenderOptions{SkipSuccessResults: c.SkipSuccessResults}); err != nil { + if err := c.Output.Render(result, k.Stdout, render.Options{SkipSuccessResults: c.SkipSuccessResults}); err != nil { return errors.Wrap(err, "cannot render validation result") } diff --git a/cmd/crossplane/validate/cmd_test.go b/cmd/crossplane/validate/validate_integration_test.go similarity index 74% rename from cmd/crossplane/validate/cmd_test.go rename to cmd/crossplane/validate/validate_integration_test.go index 95e341f..19f3359 100644 --- a/cmd/crossplane/validate/cmd_test.go +++ b/cmd/crossplane/validate/validate_integration_test.go @@ -30,12 +30,12 @@ import ( pkgvalidate "github.com/crossplane/cli/v2/cmd/crossplane/pkg/validate" ) -// runCmd parses the given CLI args through Kong and invokes the validate -// command's Run, returning whatever was written to stdout plus the error -// returned by Run. Tests interact with the command exactly the way a real -// CLI invocation would, so the helper does not bypass any of the -// production code paths in cmd.go. -func runCmd(t *testing.T, args ...string) (string, error) { +// parseCmd parses the given CLI args through Kong and returns the +// populated Cmd, the kong.Context, and any parse error. Failures of +// kong.New itself fatal the test — those indicate a static-struct bug +// rather than a runtime input issue. Use this directly when a test is +// asserting on Kong's parse behaviour (e.g. an invalid flag value). +func parseCmd(t *testing.T, args ...string) (*Cmd, *kong.Context, error) { t.Helper() var c Cmd parser, err := kong.New(&c) @@ -43,25 +43,16 @@ func runCmd(t *testing.T, args ...string) (string, error) { t.Fatalf("kong.New(): %v", err) } kongCtx, err := parser.Parse(args) - if err != nil { - return "", err - } - var stdout bytes.Buffer - kongCtx.Stdout = &stdout - runErr := c.Run(kongCtx, logging.NewNopLogger()) - return stdout.String(), runErr + return &c, kongCtx, err } -// runCmdOK is runCmd plus a t.Fatal on parse error, used when the test -// is asserting against Run's behaviour rather than Kong's parsing. -func runCmdOK(t *testing.T, args ...string) (string, error) { +// runCmd parses args, t.Fatals on a parse failure, then invokes +// Cmd.Run and returns whatever was written to stdout plus the error +// returned by Run. Tests asserting on Run's behaviour (the common case) +// use this; tests asserting on parse behaviour use parseCmd directly. +func runCmd(t *testing.T, args ...string) (string, error) { t.Helper() - var c Cmd - parser, err := kong.New(&c) - if err != nil { - t.Fatalf("kong.New(): %v", err) - } - kongCtx, err := parser.Parse(args) + c, kongCtx, err := parseCmd(t, args...) if err != nil { t.Fatalf("kong.Parse(%v): %v", args, err) } @@ -79,26 +70,28 @@ var commonArgs = []string{ "--crossplane-image=xpkg.crossplane.io/crossplane/crossplane:v0.0.0-test", } -// TestCmd_OutputEnumValidation asserts that Kong rejects unknown --output -// values before Run is called. -func TestCmd_OutputEnumValidation(t *testing.T) { +// TestParseRejectsUnknownOutputFormat asserts that an unknown --output +// value is rejected at parse time by rendererFlag.Decode, before Run is +// ever invoked. +func TestParseRejectsUnknownOutputFormat(t *testing.T) { args := append([]string{ "testdata/cmd/crd.yaml", "testdata/cmd/resources_valid.yaml", "--output=xml", }, commonArgs...) - _, err := runCmd(t, args...) + _, _, err := parseCmd(t, args...) if err == nil { - t.Errorf("kong.Parse(--output=xml) = nil; want enum-validation error") + t.Errorf("kong.Parse(--output=xml) = nil; want decoder error") } } -// TestCmd_Run drives the validate command end-to-end through Kong. Each -// case writes real fixture files and a real validation result to a real -// stdout writer; nothing is mocked. The pre-populated testdata/cache -// directory keeps the run offline. -func TestCmd_Run(t *testing.T) { +// TestRun drives the validate command end-to-end through Kong, against +// real fixture files and a pre-populated cache directory that keeps the +// run offline. Nothing is mocked; the case table covers +// text/json/yaml × valid/invalid/missing × flag interactions. +func TestRun(t *testing.T) { cases := map[string]struct { + reason string extensions string resources string extraArgs []string @@ -112,10 +105,12 @@ func TestCmd_Run(t *testing.T) { // assertYAML, same idea but for --output=yaml. assertYAML func(t *testing.T, result *pkgvalidate.ValidationResult) }{ - "DefaultText_Valid": { + "DefaultTextValid": { + reason: "Default text mode emits the [✓] success line and the totals summary.", extensions: "testdata/cmd/crd.yaml", resources: "testdata/cmd/resources_valid.yaml", assertText: func(t *testing.T, out string) { + t.Helper() if !strings.Contains(out, "[✓] cmd.example.org/v1alpha1, Kind=Test, ok-instance") { t.Errorf("missing success line in output:\n%s", out) } @@ -124,21 +119,25 @@ func TestCmd_Run(t *testing.T) { } }, }, - "Text_InvalidExitsNonZero": { + "TextInvalidExitsNonZero": { + reason: "An invalid resource produces an [x] schema-error line and the command exits non-zero.", extensions: "testdata/cmd/crd.yaml", resources: "testdata/cmd/resources_invalid.yaml", wantErr: true, assertText: func(t *testing.T, out string) { + t.Helper() if !strings.Contains(out, "[x] schema validation error cmd.example.org/v1alpha1, Kind=Test, bad-instance") { t.Errorf("missing schema-error line in output:\n%s", out) } }, }, - "JSON_Valid": { + "JSONValid": { + reason: "--output=json on a valid resource emits a structured payload with one Valid entry.", extensions: "testdata/cmd/crd.yaml", resources: "testdata/cmd/resources_valid.yaml", extraArgs: []string{"--output=json"}, assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + t.Helper() if r.Summary.Total != 1 || r.Summary.Valid != 1 { t.Errorf("Summary = %+v; want Total=1 Valid=1", r.Summary) } @@ -147,12 +146,14 @@ func TestCmd_Run(t *testing.T) { } }, }, - "JSON_InvalidExitsNonZero": { + "JSONInvalidExitsNonZero": { + reason: "--output=json on an invalid resource surfaces a schema-typed error and exits non-zero.", extensions: "testdata/cmd/crd.yaml", resources: "testdata/cmd/resources_invalid.yaml", extraArgs: []string{"--output=json"}, wantErr: true, assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + t.Helper() if r.Summary.Invalid != 1 { t.Errorf("Summary.Invalid = %d; want 1", r.Summary.Invalid) } @@ -164,22 +165,25 @@ func TestCmd_Run(t *testing.T) { } }, }, - "YAML_Valid": { + "YAMLValid": { + reason: "--output=yaml round-trips a valid resource through YAML decoding.", extensions: "testdata/cmd/crd.yaml", resources: "testdata/cmd/resources_valid.yaml", extraArgs: []string{"--output=yaml"}, assertYAML: func(t *testing.T, r *pkgvalidate.ValidationResult) { + t.Helper() if r.Summary.Total != 1 || r.Summary.Valid != 1 { t.Errorf("Summary = %+v; want Total=1 Valid=1", r.Summary) } }, }, - "JSON_MissingSchema_NoFlag": { - // Default behaviour: a missing schema is reported but doesn't fail the run. + "JSONMissingSchemaNoFlag": { + reason: "Without --error-on-missing-schemas, a missing schema is reported but does not fail the run.", extensions: "testdata/cmd/crd.yaml", resources: "testdata/cmd/resources_missing.yaml", extraArgs: []string{"--output=json"}, assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + t.Helper() if r.Summary.MissingSchemas != 1 || r.Summary.Invalid != 0 { t.Errorf("Summary = %+v; want MissingSchemas=1 Invalid=0", r.Summary) } @@ -188,23 +192,26 @@ func TestCmd_Run(t *testing.T) { } }, }, - "JSON_MissingSchema_WithFlag": { - // --error-on-missing-schemas escalates a missing schema to a non-zero exit. + "JSONMissingSchemaWithFlag": { + reason: "--error-on-missing-schemas escalates a missing schema to a non-zero exit.", extensions: "testdata/cmd/crd.yaml", resources: "testdata/cmd/resources_missing.yaml", extraArgs: []string{"--output=json", "--error-on-missing-schemas"}, wantErr: true, assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + t.Helper() if r.Summary.MissingSchemas != 1 { t.Errorf("Summary.MissingSchemas = %d; want 1", r.Summary.MissingSchemas) } }, }, - "SkipSuccessResults_TextSuppressesCheckmark": { + "SkipSuccessResultsTextSuppressesCheckmark": { + reason: "--skip-success-results suppresses [✓] lines but the summary still reports the success count.", extensions: "testdata/cmd/crd.yaml", resources: "testdata/cmd/resources_valid.yaml", extraArgs: []string{"--skip-success-results"}, assertText: func(t *testing.T, out string) { + t.Helper() if strings.Contains(out, "[✓]") { t.Errorf("--skip-success-results should suppress [✓] lines; got:\n%s", out) } @@ -213,13 +220,13 @@ func TestCmd_Run(t *testing.T) { } }, }, - "SkipSuccessResults_JSONStillIncludesValid": { - // In JSON mode the flag is ignored; success entries remain part - // of the structured payload so consumers can filter themselves. + "SkipSuccessResultsJSONStillIncludesValid": { + reason: "--skip-success-results is text-only; the JSON payload still includes Valid entries so consumers can filter themselves.", extensions: "testdata/cmd/crd.yaml", resources: "testdata/cmd/resources_valid.yaml", extraArgs: []string{"--output=json", "--skip-success-results"}, assertJSON: func(t *testing.T, r *pkgvalidate.ValidationResult) { + t.Helper() if r.Summary.Valid != 1 { t.Errorf("--skip-success-results must not strip valid entries from JSON; got %+v", r) } @@ -230,9 +237,9 @@ func TestCmd_Run(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { args := append([]string{tc.extensions, tc.resources}, append(commonArgs, tc.extraArgs...)...) - stdout, err := runCmdOK(t, args...) + stdout, err := runCmd(t, args...) if (err != nil) != tc.wantErr { - t.Fatalf("Run() err = %v, wantErr = %v\n--- stdout ---\n%s", err, tc.wantErr, stdout) + t.Fatalf("%s\nRun() err = %v, wantErr = %v\n--- stdout ---\n%s", tc.reason, err, tc.wantErr, stdout) } // Strip any package-fetcher chatter the manager writes before // the validation result so downstream parsers see only the diff --git a/cmd/crossplane/validate/validate_test.go b/cmd/crossplane/validate/validate_test.go index 64a3697..e5465a2 100644 --- a/cmd/crossplane/validate/validate_test.go +++ b/cmd/crossplane/validate/validate_test.go @@ -29,101 +29,41 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/test" ) -var ( - testCRD = &extv1.CustomResourceDefinition{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apiextensions.k8s.io/v1", - Kind: "CustomResourceDefinition", +var testCRD = &extv1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io/v1", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: extv1.CustomResourceDefinitionSpec{ + Group: "test.org", + Names: extv1.CustomResourceDefinitionNames{ + Kind: "Test", + ListKind: "TestList", + Plural: "tests", + Singular: "test", }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: extv1.CustomResourceDefinitionSpec{ - Group: "test.org", - Names: extv1.CustomResourceDefinitionNames{ - Kind: "Test", - ListKind: "TestList", - Plural: "tests", - Singular: "test", - }, - Scope: "Cluster", - Versions: []extv1.CustomResourceDefinitionVersion{ - { - Name: "v1alpha1", - Served: true, - Storage: true, - Schema: &extv1.CustomResourceValidation{ - OpenAPIV3Schema: &extv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "spec": { - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "replicas": { - Type: "integer", - }, - }, - Required: []string{ - "replicas", + Scope: "Cluster", + Versions: []extv1.CustomResourceDefinitionVersion{ + { + Name: "v1alpha1", + Served: true, + Storage: true, + Schema: &extv1.CustomResourceValidation{ + OpenAPIV3Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "spec": { + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "replicas": { + Type: "integer", }, }, - }, - }, - }, - }, - }, - }, - } - testCRDWithCEL = &extv1.CustomResourceDefinition{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apiextensions.k8s.io/v1", - Kind: "CustomResourceDefinition", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: extv1.CustomResourceDefinitionSpec{ - Group: "test.org", - Names: extv1.CustomResourceDefinitionNames{ - Kind: "Test", - ListKind: "TestList", - Plural: "tests", - Singular: "test", - }, - Scope: "Cluster", - Versions: []extv1.CustomResourceDefinitionVersion{ - { - Name: "v1alpha1", - Served: true, - Storage: true, - Schema: &extv1.CustomResourceValidation{ - OpenAPIV3Schema: &extv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "spec": { - Type: "object", - XValidations: extv1.ValidationRules{ - extv1.ValidationRule{ - Rule: "self.minReplicas <= self.replicas && self.replicas <= self.maxReplicas", - Message: "replicas should be in between minReplicas and maxReplicas", - }, - }, - Properties: map[string]extv1.JSONSchemaProps{ - "replicas": { - Type: "integer", - }, - "minReplicas": { - Type: "integer", - }, - "maxReplicas": { - Type: "integer", - }, - }, - Required: []string{ - "replicas", - "minReplicas", - "maxReplicas", - }, + Required: []string{ + "replicas", }, }, }, @@ -131,8 +71,8 @@ var ( }, }, }, - } -) + }, +} func TestConvertToCRDs(t *testing.T) { type args struct {