diff --git a/cmd/diff/diff_integration_test.go b/cmd/diff/diff_integration_test.go index b71223f8..96a67d85 100644 --- a/cmd/diff/diff_integration_test.go +++ b/cmd/diff/diff_integration_test.go @@ -220,8 +220,11 @@ func runIntegrationTest(t *testing.T, testType DiffTestType, tt IntegrationTestC // so we can pass paths directly without special handling testFiles = append(testFiles, tt.inputFiles...) - // Create a buffer to capture the output - var stdout bytes.Buffer + // Capture stdout and stderr into separate buffers. The diff command + // writes structured output (JSON / YAML / text diff) to stdout and + // human-readable error lines to stderr; mixing them defeats JSON + // parsing for tests that drive the structured assertion path. + var stdout, stderr bytes.Buffer // Create command line args that match your pre-populated struct args := []string{ @@ -304,7 +307,7 @@ func runIntegrationTest(t *testing.T, testType DiffTestType, tt IntegrationTestC // Create a Kong context with stdout parser, err := kong.New(cmd, - kong.Writers(&stdout, &stdout), + kong.Writers(&stdout, &stderr), kong.Bind(appCtx), kong.Bind(exitCode), kong.BindTo(logger, (*logging.Logger)(nil)), @@ -333,19 +336,35 @@ func runIntegrationTest(t *testing.T, testType DiffTestType, tt IntegrationTestC t.Fatalf("expected no error but got: %v", err) } - // Check for specific error message if expected - if err != nil { - if tt.expectedErrorContains != "" && strings.Contains(err.Error(), tt.expectedErrorContains) { - // This is an expected error with the expected message - t.Logf("Got expected error containing: %s", tt.expectedErrorContains) - } else { - t.Errorf("Expected no error or specific error message, got: %v", err) + // Check for specific error message if expected. + // + // Three modes: + // 1. expectedError + expectedErrorContains: substring-check the error. + // 2. expectedError + structured assertion: any error is fine here; + // the structured assertion below validates errors[]. + // 3. unexpected error: already caught by the t.Fatalf above. + if err != nil && tt.expectedError { + hasStructuredAssertion := tt.expectedStructuredOutput != nil || tt.expectedStructuredCompOutput != nil + + switch { + case tt.expectedErrorContains != "": + if strings.Contains(err.Error(), tt.expectedErrorContains) { + t.Logf("Got expected error containing: %s", tt.expectedErrorContains) + } else { + t.Errorf("Expected error containing %q, got: %v", tt.expectedErrorContains, err) + } + case hasStructuredAssertion: + t.Logf("Got expected error; structured assertion will validate errors[]") + default: + t.Errorf("expectedError set without expectedErrorContains or structured assertion; got: %v", err) } } - // For expected errors with specific messages, we've already checked above - if tt.expectedError && tt.expectedErrorContains != "" { - // Skip output check for expected error cases + // Skip stdout check for expected-error cases that use the legacy + // substring assertion. Cases that pin a structured assertion fall + // through so the JSON path is exercised against errors[] / changes. + if tt.expectedError && tt.expectedErrorContains != "" && + tt.expectedStructuredOutput == nil && tt.expectedStructuredCompOutput == nil { return } @@ -1141,7 +1160,7 @@ Summary: 2 modified, 2 removed`, noColor: true, }, "SchemaValidationError": { - reason: "Validates exit code 2 for schema validation errors", + reason: "Validates exit code 2 for schema validation errors and structured per-resource failure detail in JSON output", setupFiles: []string{ "testdata/diff/resources/xrd.yaml", "testdata/diff/resources/composition.yaml", @@ -1150,10 +1169,29 @@ Summary: 2 modified, 2 removed`, inputFiles: []string{ "testdata/diff/invalid-schema-xr.yaml", }, - expectedError: true, - expectedExitCode: dp.ExitCodeSchemaValidation, - expectedErrorContains: "schema validation", - noColor: true, + outputFormat: "json", + expectedError: true, + expectedExitCode: dp.ExitCodeSchemaValidation, + // Structured assertion: lock down the typed + // validationFailures payload for both the failing XR and + // its composed resource. Field paths and error types are + // ours to assert; the embedded k8s message wording is + // left unasserted (apimachinery upgrades can shift its + // phrasing without changing our output contract). + expectedStructuredOutput: tu.ExpectDiff(). + WithError("XNopResource/invalid-schema-xr"). + WithValidationFailure("ns.diff.example.org/v1alpha1", "XNopResource", "invalid-schema-xr", "default"). + WithStatus("invalid"). + WithFieldError("schema", "spec.coolField"). + AndFieldError(). + AndValidationFailure(). + WithValidationFailure("ns.nop.example.org/v1alpha1", "XDownstreamResource", "invalid-schema-xr", "default"). + WithStatus("invalid"). + WithFieldError("schema", "spec.forProvider.configData"). + AndFieldError(). + AndValidationFailure(). + AndError(), + noColor: true, }, "NewClaimShowsDiff": { reason: "Shows diff for new claim", diff --git a/cmd/diff/diffprocessor/comp_processor.go b/cmd/diff/diffprocessor/comp_processor.go index 097cdd3a..ac79454a 100644 --- a/cmd/diff/diffprocessor/comp_processor.go +++ b/cmd/diff/diffprocessor/comp_processor.go @@ -261,10 +261,11 @@ func (p *DefaultCompDiffProcessor) DiffComposition(ctx context.Context, composit for _, impact := range comp.ImpactAnalysis { if impact.Status == renderer.XRStatusError && impact.Error != nil { resourceID := fmt.Sprintf("%s/%s", impact.Kind, impact.Name) - output.Errors = append(output.Errors, dt.OutputError{ - ResourceID: resourceID, - Message: impact.Error.Error(), - }) + // NewOutputError surfaces typed validation failures + // via OutputError.ValidationFailures when impact.Error + // wraps a SchemaValidationError carrying a structured + // Result. + output.Errors = append(output.Errors, NewOutputError(resourceID, impact.Error)) } } } diff --git a/cmd/diff/diffprocessor/diff_processor.go b/cmd/diff/diffprocessor/diff_processor.go index b61824de..d432ff72 100644 --- a/cmd/diff/diffprocessor/diff_processor.go +++ b/cmd/diff/diffprocessor/diff_processor.go @@ -18,6 +18,7 @@ import ( dt "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer/types" "github.com/crossplane-contrib/crossplane-diff/cmd/diff/types" "github.com/crossplane/cli/v2/cmd/crossplane/render" + clixr "github.com/crossplane/cli/v2/pkg/xr" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -230,11 +231,11 @@ func (p *DefaultDiffProcessor) PerformDiff(ctx context.Context, resources []*un. "error", err) errs = append(errs, errors.Wrapf(err, "unable to process resource %s", resourceID)) - // Collect error for structured output - outputErrors = append(outputErrors, dt.OutputError{ - ResourceID: resourceID, - Message: err.Error(), - }) + // Collect error for structured output. NewOutputError + // surfaces typed validation failures via + // OutputError.ValidationFailures when err wraps a + // SchemaValidationError that carries a structured Result. + outputErrors = append(outputErrors, NewOutputError(resourceID, err)) } else { // Only merge diffs on success - we don't emit partial results for a single XR maps.Copy(allDiffs, diffs) @@ -1557,7 +1558,10 @@ func (p *DefaultDiffProcessor) applyXRDDefaults(ctx context.Context, xr *cmp.Uns return errors.Wrapf(err, "cannot find CRD for XRD %s (resource %s)", xrdName, resourceID) } - // Apply defaults using the render.DefaultValues function + // Apply defaults using the cli's pkg/xr defaulting helper. The + // previous home, render.DefaultValues, was renamed to + // xr.ApplyCRDDefaults and moved out of cmd/crossplane/render in + // crossplane/cli when that package was promoted to a library. apiVersion := xr.GetAPIVersion() xrContent := xr.UnstructuredContent() @@ -1566,7 +1570,7 @@ func (p *DefaultDiffProcessor) applyXRDDefaults(ctx context.Context, xr *cmp.Uns "apiVersion", apiVersion, "crdName", crdForDefaults.Name) - err = render.DefaultValues(xrContent, apiVersion, *crdForDefaults) + err = clixr.ApplyCRDDefaults(xrContent, apiVersion, *crdForDefaults) if err != nil { return errors.Wrapf(err, "cannot apply default values for XR %s", resourceID) } diff --git a/cmd/diff/diffprocessor/errors.go b/cmd/diff/diffprocessor/errors.go index 8f15c484..410afccc 100644 --- a/cmd/diff/diffprocessor/errors.go +++ b/cmd/diff/diffprocessor/errors.go @@ -19,6 +19,9 @@ package diffprocessor import ( "errors" "fmt" + + dt "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer/types" + pkgvalidate "github.com/crossplane/cli/v2/pkg/validate" ) // Exit codes for crossplane-diff CLI. @@ -37,11 +40,21 @@ const ( ) // SchemaValidationError indicates schema validation failed. -// Used to distinguish validation errors from other tool errors for exit code handling. +// Used to distinguish validation errors from other tool errors for exit +// code handling. +// +// Result, when non-nil, carries the structured per-resource validation +// outcome that produced this error. Output renderers use it to surface +// typed FieldValidationError records under OutputError.ValidationFailures +// in JSON / YAML output. It is left nil for paths that fail validation +// without a *pkgvalidate.ValidationResult in hand — for example +// scope-validation errors raised after schema validation succeeded — so +// the absence of structured detail is observable rather than fabricated. type SchemaValidationError struct { ResourceID string Message string Err error + Result *pkgvalidate.ValidationResult } // Error implements the error interface. @@ -58,7 +71,9 @@ func (e *SchemaValidationError) Unwrap() error { return e.Err } -// NewSchemaValidationError creates a new SchemaValidationError. +// NewSchemaValidationError creates a new SchemaValidationError without +// a structured Result. Use WithResult on the returned value to attach +// one when the failure originated from pkg/validate.SchemaValidate. func NewSchemaValidationError(resourceID, message string, err error) *SchemaValidationError { return &SchemaValidationError{ ResourceID: resourceID, @@ -67,6 +82,109 @@ func NewSchemaValidationError(resourceID, message string, err error) *SchemaVali } } +// WithResult attaches the structured *pkgvalidate.ValidationResult +// that produced this error so downstream renderers can emit typed +// per-resource failures alongside the human-readable Message. Returns +// the receiver for fluent chaining. +func (e *SchemaValidationError) WithResult(result *pkgvalidate.ValidationResult) *SchemaValidationError { + e.Result = result + return e +} + +// NewOutputError builds a structured-output entry for err, tagged with +// resourceID. When err contains a *SchemaValidationError that carries a +// pkgvalidate.ValidationResult, the returned OutputError also exposes a +// typed per-resource breakdown via ValidationFailures so machine +// consumers don't need to parse Message. Non-validation errors return +// an OutputError with only ResourceID and Message populated. +func NewOutputError(resourceID string, err error) dt.OutputError { + out := dt.OutputError{ + ResourceID: resourceID, + Message: err.Error(), + } + + var sve *SchemaValidationError + if errors.As(err, &sve) && sve.Result != nil { + out.ValidationFailures = validationFailuresFromResult(sve.Result) + } + + return out +} + +// validationFailuresFromResult maps a pkgvalidate.ValidationResult into +// the wire types crossplane-diff exposes through OutputError. Resources +// with status Valid are filtered out — ValidationFailures is "what went +// wrong", not "the full audit log". Resources with status +// DefaultingFailed are filtered out too: pkgvalidate.ResultError treats +// defaulting-only failures as success, so a SchemaValidationError +// reaching this code path should not advertise them as failures. +func validationFailuresFromResult(result *pkgvalidate.ValidationResult) []dt.ResourceValidationFailure { + if result == nil { + return nil + } + + var out []dt.ResourceValidationFailure + + for _, r := range result.Resources { + switch r.Status { + case pkgvalidate.ValidationStatusValid, + pkgvalidate.ValidationStatusDefaultingFailed: + continue + case pkgvalidate.ValidationStatusInvalid, + pkgvalidate.ValidationStatusMissingSchema: + // fall through + } + + out = append(out, dt.ResourceValidationFailure{ + APIVersion: r.APIVersion, + Kind: r.Kind, + Name: r.Name, + Namespace: r.Namespace, + Status: string(r.Status), + Errors: fieldValidationErrorsFromUpstream(r.Errors), + }) + } + + return out +} + +// fieldValidationErrorsFromUpstream converts the cli's per-field error +// slice into our wire shape. Defaulting entries are filtered out when +// at least one actionable (schema / CEL / unknown-field) error is +// present on the same resource, mirroring the suppression policy of +// formatValidationErrors so the typed and human-readable views agree. +func fieldValidationErrorsFromUpstream(errs []pkgvalidate.FieldValidationError) []dt.FieldValidationError { + if len(errs) == 0 { + return nil + } + + hasActionable := false + + for _, e := range errs { + if e.Type != pkgvalidate.FieldErrorTypeDefaulting { + hasActionable = true + break + } + } + + out := make([]dt.FieldValidationError, 0, len(errs)) + + for _, e := range errs { + if hasActionable && e.Type == pkgvalidate.FieldErrorTypeDefaulting { + continue + } + + out = append(out, dt.FieldValidationError{ + Type: string(e.Type), + Field: e.Field, + Message: e.Message, + Value: e.Value, + }) + } + + return out +} + // IsSchemaValidationError checks if any error in the chain is a schema validation error. func IsSchemaValidationError(err error) bool { var sve *SchemaValidationError diff --git a/cmd/diff/diffprocessor/errors_test.go b/cmd/diff/diffprocessor/errors_test.go index fc484a82..5ac964a4 100644 --- a/cmd/diff/diffprocessor/errors_test.go +++ b/cmd/diff/diffprocessor/errors_test.go @@ -21,6 +21,8 @@ import ( "fmt" "testing" + dt "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer/types" + pkgvalidate "github.com/crossplane/cli/v2/pkg/validate" gcmp "github.com/google/go-cmp/cmp" xperrors "github.com/crossplane/crossplane-runtime/v2/pkg/errors" @@ -135,6 +137,318 @@ func TestNewSchemaValidationError(t *testing.T) { if !errors.Is(err.Err, wrappedErr) { t.Errorf("Err = %v, want %v", err.Err, wrappedErr) } + + if err.Result != nil { + t.Errorf("Result = %v, want nil for the basic constructor", err.Result) + } +} + +func TestSchemaValidationError_WithResult(t *testing.T) { + result := &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "x", + Status: pkgvalidate.ValidationStatusInvalid, + }}, + } + + err := NewSchemaValidationError("", "msg", errors.New("inner")).WithResult(result) + if err.Result != result { + t.Errorf("Result = %v, want %v", err.Result, result) + } +} + +func TestNewOutputError(t *testing.T) { + tests := map[string]struct { + reason string + resourceID string + err error + want dt.OutputError + }{ + "NonValidationError_NoFailuresAttached": { + reason: "A plain error has no structured slot to populate; ValidationFailures stays nil and only Message is set.", + resourceID: "XR/my-xr", + err: errors.New("kube unreachable"), + want: dt.OutputError{ + ResourceID: "XR/my-xr", + Message: "kube unreachable", + }, + }, + "SchemaValidationError_WithoutResult_NoFailuresAttached": { + reason: "A SchemaValidationError without a Result (e.g. scope-validation path) contributes only Message; the typed slot stays nil so consumers can distinguish 'no failures' from 'no structured detail available'.", + resourceID: "XR/my-xr", + err: NewSchemaValidationError("", "scope failure", errors.New("inner")), + want: dt.OutputError{ + ResourceID: "XR/my-xr", + Message: "scope failure", + }, + }, + "SchemaValidationError_WithResult_ExposesTypedFailures": { + reason: "A SchemaValidationError carrying a Result populates ValidationFailures with the converted per-resource breakdown.", + resourceID: "XR/my-xr", + err: NewSchemaValidationError("", "msg", errors.New("inner")).WithResult( + &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeSchema, + Field: "spec.region", + Message: "spec.region: Required value", + }}, + }}, + }), + want: dt.OutputError{ + ResourceID: "XR/my-xr", + Message: "msg", + ValidationFailures: []dt.ResourceValidationFailure{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: "invalid", + Errors: []dt.FieldValidationError{{ + Type: "schema", + Field: "spec.region", + Message: "spec.region: Required value", + }}, + }}, + }, + }, + "WrappedSchemaValidationError_StillSurfacesFailures": { + reason: "errors.As walks the chain, so a SchemaValidationError behind errors.Wrap still has its Result extracted into ValidationFailures.", + resourceID: "XR/my-xr", + err: xperrors.Wrap( + NewSchemaValidationError("", "msg", errors.New("inner")).WithResult( + &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "other.org/v1", + Kind: "Thing", + Name: "thing", + Status: pkgvalidate.ValidationStatusMissingSchema, + }}, + }), + "cannot validate resources"), + want: dt.OutputError{ + ResourceID: "XR/my-xr", + Message: "cannot validate resources: msg", + ValidationFailures: []dt.ResourceValidationFailure{{ + APIVersion: "other.org/v1", + Kind: "Thing", + Name: "thing", + Status: "missingSchema", + }}, + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got := NewOutputError(tc.resourceID, tc.err) + if diff := gcmp.Diff(tc.want, got); diff != "" { + t.Errorf("\n%s\nNewOutputError() mismatch (-want +got):\n%s", tc.reason, diff) + } + }) + } +} + +func TestValidationFailuresFromResult(t *testing.T) { + tests := map[string]struct { + reason string + result *pkgvalidate.ValidationResult + want []dt.ResourceValidationFailure + }{ + "NilResultReturnsNil": { + reason: "A nil ValidationResult yields nil; the converter never invents structure.", + result: nil, + want: nil, + }, + "OnlyValidResourcesReturnsNil": { + reason: "ValidationFailures lists 'what failed', not the full audit log; valid resources are filtered out.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "ok", + Status: pkgvalidate.ValidationStatusValid, + }}, + }, + want: nil, + }, + "DefaultingFailedFiltered": { + reason: "pkgvalidate.ResultError treats DefaultingFailed as success, so a SchemaValidationError reaching us shouldn't advertise these resources as failures via ValidationFailures.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "x", + Status: pkgvalidate.ValidationStatusDefaultingFailed, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeDefaulting, + Message: "cannot apply defaults", + }}, + }}, + }, + want: nil, + }, + "InvalidWithMixedErrorsSuppressesDefaulting": { + reason: "Mirrors formatValidationErrors: when actionable errors are present, defaulting entries are dropped so the typed and rendered views agree on the failure detail.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Namespace: "production", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{ + {Type: pkgvalidate.FieldErrorTypeDefaulting, Message: "cannot apply defaults"}, + {Type: pkgvalidate.FieldErrorTypeSchema, Field: "spec.region", Message: "spec.region: Required value"}, + }, + }}, + }, + want: []dt.ResourceValidationFailure{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Namespace: "production", + Status: "invalid", + Errors: []dt.FieldValidationError{{ + Type: "schema", + Field: "spec.region", + Message: "spec.region: Required value", + }}, + }}, + }, + "InvalidWithOnlyDefaultingErrorsKeepsThem": { + reason: "Defensive: upstream's statusFromErrors won't produce Invalid+only-defaulting today, but if it ever did we surface the defaulting entries rather than emit an Invalid row with no errors.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeDefaulting, + Message: "cannot apply defaults", + }}, + }}, + }, + want: []dt.ResourceValidationFailure{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: "invalid", + Errors: []dt.FieldValidationError{{ + Type: "defaulting", + Message: "cannot apply defaults", + }}, + }}, + }, + "MissingSchemaSurfacedWithoutErrors": { + reason: "Missing-schema resources surface with their GVK / name / namespace and Status, but no errors[] (the validator never ran).", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "other.org/v1", + Kind: "SomeResource", + Name: "thing", + Status: pkgvalidate.ValidationStatusMissingSchema, + }}, + }, + want: []dt.ResourceValidationFailure{{ + APIVersion: "other.org/v1", + Kind: "SomeResource", + Name: "thing", + Status: "missingSchema", + }}, + }, + "BadValuePropagated": { + reason: "FieldValidationError.Value travels through unchanged, preserving its Go type, so JSON output renders it without forcing callers to parse it back.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeSchema, + Field: "spec.replicas", + Message: `spec.replicas: Invalid value: "five"`, + Value: "five", + }}, + }}, + }, + want: []dt.ResourceValidationFailure{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: "invalid", + Errors: []dt.FieldValidationError{{ + Type: "schema", + Field: "spec.replicas", + Message: `spec.replicas: Invalid value: "five"`, + Value: "five", + }}, + }}, + }, + "MultipleResourcesPreserveOrder": { + reason: "The output preserves input order across resources and skips valid ones, so consumers can rely on the position correspondence between input resources and emitted failures.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{ + { + APIVersion: "example.org/v1", + Kind: "XR", + Name: "first", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeSchema, + Message: "first error", + }}, + }, + { + APIVersion: "example.org/v1", + Kind: "XR", + Name: "ok-skipped", + Status: pkgvalidate.ValidationStatusValid, + }, + { + APIVersion: "other.org/v1", + Kind: "Thing", + Name: "third", + Status: pkgvalidate.ValidationStatusMissingSchema, + }, + }, + }, + want: []dt.ResourceValidationFailure{ + { + APIVersion: "example.org/v1", + Kind: "XR", + Name: "first", + Status: "invalid", + Errors: []dt.FieldValidationError{{ + Type: "schema", + Message: "first error", + }}, + }, + { + APIVersion: "other.org/v1", + Kind: "Thing", + Name: "third", + Status: "missingSchema", + }, + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got := validationFailuresFromResult(tc.result) + if diff := gcmp.Diff(tc.want, got); diff != "" { + t.Errorf("\n%s\nvalidationFailuresFromResult() mismatch (-want +got):\n%s", tc.reason, diff) + } + }) + } } func TestIsOnlySchemaValidationErrors(t *testing.T) { diff --git a/cmd/diff/diffprocessor/schema_validator.go b/cmd/diff/diffprocessor/schema_validator.go index c2e65334..4825e053 100644 --- a/cmd/diff/diffprocessor/schema_validator.go +++ b/cmd/diff/diffprocessor/schema_validator.go @@ -1,16 +1,15 @@ package diffprocessor import ( - "bytes" "context" "fmt" - "io" + "regexp" "strings" xp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/crossplane" k8 "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/kubernetes" - "github.com/crossplane/cli/v2/cmd/crossplane/common/loggerwriter" - "github.com/crossplane/cli/v2/cmd/crossplane/validate" + pkgvalidate "github.com/crossplane/cli/v2/pkg/validate" + clixr "github.com/crossplane/cli/v2/pkg/xr" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -108,32 +107,40 @@ func (v *DefaultSchemaValidator) ValidateResources(ctx context.Context, xr *un.U return errors.Wrap(err, "unable to ensure CRDs") } - // Create a buffer to capture validation output for error messages, - // and a MultiWriter to also send output to debug logs - var validationOutput bytes.Buffer - - multiWriter := io.MultiWriter(&validationOutput, loggerwriter.NewLoggerWriter(v.logger)) - - // SchemaValidation applies defaults IN-PLACE to the resources it sees, - // mutating the underlying Object map. For each managed resource we wrap - // `&un.Unstructured{Object: composed[i].UnstructuredContent()}` — the - // embedded unstructured.Unstructured returns its Object map by reference, - // so defaults applied here propagate back to the caller's - // `composed[i]` via the shared map. That preserves pre-existing - // defaulting behaviour for downstream diff calculation. - // - // The XR is passed by pointer; defaults applied to it land on the - // caller's object, which is then used downstream for diffing against - // cluster state. The real composite reconciler in the render pipeline - // already applied XRD schema defaults before we got here, so this is - // belt-and-braces. + // Apply CRD-level defaults in place before handing resources off to + // the diff calculator. The structured SchemaValidate API + // intentionally deep-copies inputs and does not surface defaults + // to its caller, so without this step composed resources would + // reach diff calculation undefaulted and produce spurious diffs + // for fields the cluster's defaulter would have populated. The + // previous SchemaValidation entry point fused this defaulting + // into validation; doing it explicitly here makes the + // dependency obvious. + if err := v.applyCRDDefaults(ctx, resources); err != nil { + return err + } + + // SchemaValidate is the structured-result API: it returns a + // *ValidationResult that callers inspect directly. v.logger.Debug("Performing schema validation", "resourceCount", len(resources)) - err = validate.SchemaValidation(ctx, resources, v.schemaClient.GetAllCRDs(), true, true, multiWriter) + result, err := pkgvalidate.SchemaValidate(ctx, resources, v.schemaClient.GetAllCRDs()) if err != nil { - // Parse and extract only the error lines from validation output - details := extractValidationErrors(validationOutput.String()) - return NewSchemaValidationError("", details, err) + // SchemaValidate's error return is reserved for setup + // failures (e.g. a CRD that can't be compiled into a + // validator). Wrap it as a SchemaValidationError so + // IsSchemaValidationError / DetermineExitCode classify it the + // same way the pre-refactor SchemaValidation entry point + // did — exit code 2 (schema validation), not 1 (tool error). + // No Result is attached because the validator never produced + // one. + return NewSchemaValidationError("", "schema validation setup failed", err) + } + + v.logResultDetails(result) + + if rerr := pkgvalidate.ResultError(result, true); rerr != nil { + return NewSchemaValidationError("", formatValidationErrors(result), rerr).WithResult(result) } // Additionally validate resource scope constraints (namespace requirements and cross-namespace refs) @@ -259,25 +266,215 @@ func (v *DefaultSchemaValidator) ValidateScopeConstraints(ctx context.Context, r return nil } -// extractValidationErrors parses validation output and returns clean error messages. -// It extracts lines starting with [x] (validation errors) and [!] (warnings/missing schemas), -// stripping the prefixes for cleaner display. -func extractValidationErrors(output string) string { - var validationErrs []string - - for line := range strings.SplitSeq(output, "\n") { - line = strings.TrimSpace(line) - // Use CutPrefix to check for prefix and strip it in one operation - if cleaned, found := strings.CutPrefix(line, "[x]"); found { - validationErrs = append(validationErrs, strings.TrimSpace(cleaned)) - } else if cleaned, found := strings.CutPrefix(line, "[!]"); found { - validationErrs = append(validationErrs, strings.TrimSpace(cleaned)) +// applyCRDDefaults applies CRD-derived defaults to each resource in +// place. Built-in types and resources whose CRD is unknown are +// skipped; in both cases the diff calculator already handles them +// without server-side defaulting. We treat a CRD lookup error here +// as a no-op rather than a failure: EnsureComposedResourceCRDs is +// the canonical gate on missing CRDs, and the subsequent SchemaValidate +// call surfaces the same condition through ValidationStatusMissingSchema. +func (v *DefaultSchemaValidator) applyCRDDefaults(ctx context.Context, resources []*un.Unstructured) error { + for _, r := range resources { + gvk := r.GroupVersionKind() + if !v.schemaClient.IsCRDRequired(ctx, gvk) { + continue + } + + crd, err := v.schemaClient.GetCRD(ctx, gvk) + if err != nil { + v.logger.Debug("skipping defaulting; CRD not found", "gvk", gvk.String(), "error", err) + continue + } + + if err := clixr.ApplyCRDDefaults(r.Object, r.GetAPIVersion(), *crd); err != nil { + return errors.Wrapf(err, "cannot apply CRD defaults for %s/%s", gvk.String(), r.GetName()) + } + } + + return nil +} + +// logResultDetails emits per-resource debug logging for a validation +// result. This replaces the incidental logging that the previous +// stdout-capturing implementation produced via a loggerwriter, and +// surfaces operational signals (defaulting failures, missing schemas) +// even when ResultError reports success overall. +func (v *DefaultSchemaValidator) logResultDetails(result *pkgvalidate.ValidationResult) { + for _, r := range result.Resources { + v.logger.Debug("validation result", + "apiVersion", r.APIVersion, + "kind", r.Kind, + "name", r.Name, + "status", string(r.Status), + "errors", len(r.Errors), + ) + + for _, e := range r.Errors { + v.logger.Debug("validation error", + "apiVersion", r.APIVersion, + "kind", r.Kind, + "name", r.Name, + "type", string(e.Type), + "field", e.Field, + "message", e.Message, + ) + } + } +} + +// formatValidationErrors produces a multi-line, per-resource breakdown +// of the failures in a *ValidationResult. Each resource that has +// something to report contributes a header line " :" +// followed by one indented line per FieldValidationError, with the +// error type appended in brackets and the structured Value rendered as +// "(got )" when it isn't already quoted into the message text. +// Missing-schema resources collapse to a single " : missing +// schema" line. +// +// Callers invoke this only after pkgvalidate.ResultError has already +// returned a non-nil error, so an empty result means "no per-resource +// detail to report" — we return "" and let the wrapping +// SchemaValidationError carry the underlying ResultError message. +func formatValidationErrors(result *pkgvalidate.ValidationResult) string { + var blocks []string + + for _, r := range result.Resources { + switch r.Status { + case pkgvalidate.ValidationStatusMissingSchema: + blocks = append(blocks, formatMissingSchemaBlock(r)) + case pkgvalidate.ValidationStatusInvalid: + blocks = append(blocks, formatInvalidBlock(r)) + case pkgvalidate.ValidationStatusValid, + pkgvalidate.ValidationStatusDefaultingFailed: + // Valid: nothing to report. DefaultingFailed-only resources + // are reported by ResultError as success, so this function + // never sees them via the failure path; the cases are here + // only to keep the switch exhaustive. + } + } + + return strings.Join(blocks, "\n") +} + +// resourceHeader formats the per-resource header used by both the +// missing-schema and invalid blocks. Cluster-scoped resources omit the +// namespace prefix; namespaced resources produce "/". A +// resource without metadata.name collapses to just the GVK regardless +// of namespace — the namespace alone isn't a useful identifier for a +// nameless resource and emitting "/" would leave a stray trailing +// slash. +func resourceHeader(r pkgvalidate.ResourceValidationResult) string { + if r.Name == "" { + return fmt.Sprintf("%s/%s", r.APIVersion, r.Kind) + } + + name := r.Name + if r.Namespace != "" { + name = r.Namespace + "/" + r.Name + } + + return fmt.Sprintf("%s/%s %s", r.APIVersion, r.Kind, name) +} + +// formatMissingSchemaBlock renders a single line for a resource whose +// CRD/XRD wasn't found. The resource was never validated, so there are +// no per-error lines to indent under it. +func formatMissingSchemaBlock(r pkgvalidate.ResourceValidationResult) string { + return resourceHeader(r) + ": missing schema" +} + +// formatInvalidBlock renders a header line plus one indented line per +// surfaced error. +// +// Defaulting entries are suppressed only when an actionable (schema / +// CEL / unknown-field) error is present on the same resource: those +// already convey the failure and the defaulting line would just be +// noise. Upstream's statusFromErrors today guarantees an Invalid +// resource has at least one non-defaulting error, but if a future +// change ever produced an Invalid resource whose Errors are all +// defaulting we'd rather emit them than silently drop everything. +func formatInvalidBlock(r pkgvalidate.ResourceValidationResult) string { + hasActionable := false + + for _, e := range r.Errors { + if e.Type != pkgvalidate.FieldErrorTypeDefaulting { + hasActionable = true + break + } + } + + lines := []string{resourceHeader(r) + ":"} + + for _, e := range r.Errors { + if hasActionable && e.Type == pkgvalidate.FieldErrorTypeDefaulting { + continue } + + lines = append(lines, " "+formatErrorLine(e)) + } + + return strings.Join(lines, "\n") +} + +// formatErrorLine renders one FieldValidationError as +// "[ (got )] []". The bad value tail is omitted +// when Value is nil or already present in the message. Duplication +// detection is type-aware to avoid false positives: +// +// - String values are checked in their %q (quoted) form, matching +// how k8s validators emit them (`Invalid value: "five"`); the +// surrounding quotes give the search natural delimiters so +// Value="k" against message "spec.kind: Required" doesn't +// suppress the tail just because "k" sits inside "kind". +// +// - Non-string values use a word-boundary regex so Value=42 against +// message "...Invalid value: 420..." doesn't suppress the tail +// just because "42" is a prefix of "420". +func formatErrorLine(e pkgvalidate.FieldValidationError) string { + msg := e.Message + if rendered := renderBadValue(e.Value); rendered != "" && !valueAlreadyInMessage(msg, e.Value, rendered) { + msg = fmt.Sprintf("%s (got %s)", msg, rendered) + } + + return fmt.Sprintf("%s [%s]", msg, e.Type) +} + +// valueAlreadyInMessage reports whether the rendered form of a bad +// value is already present in the validator's message text. The check +// shape depends on the value's Go type — see formatErrorLine's +// comment for the rationale. +func valueAlreadyInMessage(msg string, value any, rendered string) bool { + if _, ok := value.(string); ok { + // rendered already includes surrounding %q quotes; substring + // search is delimiter-safe. + return strings.Contains(msg, rendered) + } + + // For non-strings rendered is unquoted, so anchor the search at + // word boundaries to avoid 42 matching inside 420. + return regexp.MustCompile(`\b` + regexp.QuoteMeta(rendered) + `\b`).MatchString(msg) +} + +// renderBadValue formats a FieldValidationError.Value for display. +// Returns the empty string for a nil Value so callers can use it as a +// presence check. +// +// Strings get %q (quoted), matching how k8s validation messages embed +// them (`Invalid value: "five"`). Other types use %v: numbers and +// booleans read naturally unquoted, and structs render via their +// default Go representation. +// +// Duplication detection against the validator's message text is done +// separately by valueAlreadyInMessage, which knows how to anchor the +// search at the right boundaries for each value type. +func renderBadValue(value any) string { + if value == nil { + return "" } - if len(validationErrs) == 0 { - return "schema validation failed" + if s, ok := value.(string); ok { + return fmt.Sprintf("%q", s) } - return strings.Join(validationErrs, "; ") + return fmt.Sprintf("%v", value) } diff --git a/cmd/diff/diffprocessor/schema_validator_test.go b/cmd/diff/diffprocessor/schema_validator_test.go index d41cd13e..f2381e10 100644 --- a/cmd/diff/diffprocessor/schema_validator_test.go +++ b/cmd/diff/diffprocessor/schema_validator_test.go @@ -7,10 +7,10 @@ import ( xp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/crossplane" tu "github.com/crossplane-contrib/crossplane-diff/cmd/diff/testutils" + pkgvalidate "github.com/crossplane/cli/v2/pkg/validate" "github.com/google/go-cmp/cmp" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" @@ -20,9 +20,8 @@ import ( var _ SchemaValidator = (*tu.MockSchemaValidator)(nil) const ( - testExampleOrg = "example.org" - testComposedResource = "ComposedResource" - testCpdOrg = "cpd.org" + testExampleOrg = "example.org" + testCpdOrg = "cpd.org" ) func TestDefaultSchemaValidator_ValidateResources(t *testing.T) { @@ -56,7 +55,6 @@ func TestDefaultSchemaValidator_ValidateResources(t *testing.T) { setupClients func() (*tu.MockSchemaClient, *tu.MockDefinitionClient) xr *un.Unstructured composed []cpd.Unstructured - preloadedCRDs []*extv1.CustomResourceDefinition expectedErr bool expectedErrMsg string }{ @@ -73,10 +71,9 @@ func TestDefaultSchemaValidator_ValidateResources(t *testing.T) { return sch, tu.NewMockDefinitionClient().Build() }, - xr: xr, - composed: []cpd.Unstructured{*composedResource1, *composedResource2}, - preloadedCRDs: []*extv1.CustomResourceDefinition{}, // No longer needed - expectedErr: false, + xr: xr, + composed: []cpd.Unstructured{*composedResource1, *composedResource2}, + expectedErr: false, }, "SuccessfulValidationWithFetchedCRDs": { setupClients: func() (*tu.MockSchemaClient, *tu.MockDefinitionClient) { @@ -96,10 +93,9 @@ func TestDefaultSchemaValidator_ValidateResources(t *testing.T) { return sch, def }, - xr: xr, - composed: []cpd.Unstructured{*composedResource1, *composedResource2}, - preloadedCRDs: []*extv1.CustomResourceDefinition{}, - expectedErr: false, + xr: xr, + composed: []cpd.Unstructured{*composedResource1, *composedResource2}, + expectedErr: false, }, "MissingCRD": { setupClients: func() (*tu.MockSchemaClient, *tu.MockDefinitionClient) { @@ -118,22 +114,14 @@ func TestDefaultSchemaValidator_ValidateResources(t *testing.T) { return sch, def }, - xr: xr, - composed: []cpd.Unstructured{*composedResource1, *composedResource2}, - preloadedCRDs: []*extv1.CustomResourceDefinition{}, + xr: xr, + composed: []cpd.Unstructured{*composedResource1, *composedResource2}, // Now we expect an error because we've configured it to require a CRD but can't find it expectedErr: true, expectedErrMsg: "unable to find CRDs for", }, "ValidationError": { setupClients: func() (*tu.MockSchemaClient, *tu.MockDefinitionClient) { - // Convert CRDs to un for the mock - composedCRDUn := &un.Unstructured{} - _ = runtime.DefaultUnstructuredConverter.FromUnstructured( - MustToUnstructured(createCRDWithStringField(composedCRD)), - composedCRDUn, - ) - sch := tu.NewMockSchemaClient(). // Add GetCRD implementation for typed CRDs WithGetCRD(func(_ context.Context, gvk schema.GroupVersionKind) (*extv1.CustomResourceDefinition, error) { @@ -160,9 +148,8 @@ func TestDefaultSchemaValidator_ValidateResources(t *testing.T) { WithSpecField("field", int64(123)). Build(), composed: []cpd.Unstructured{*composedResource1, *composedResource2}, - preloadedCRDs: []*extv1.CustomResourceDefinition{createCRDWithStringField(xrCRD)}, expectedErr: true, - expectedErrMsg: "could not find CRD/XRD", + expectedErrMsg: "missing schema", }, } @@ -296,7 +283,6 @@ func TestDefaultSchemaValidator_LoadCRDs(t *testing.T) { tests := map[string]struct { setupClient func() xp.DefinitionClient - preloadedCRDs []*extv1.CustomResourceDefinition expectedErr bool expectedErrMsg string // for caching tests @@ -328,7 +314,6 @@ func TestDefaultSchemaValidator_LoadCRDs(t *testing.T) { Build(), } }, - preloadedCRDs: nil, // No preloaded CRDs expectedErr: false, callTwice: true, // Make two calls to LoadCRDs expectXRDCalls: 1, // GetXRDs should only be called once due to caching @@ -391,16 +376,6 @@ func createCRDWithStringField(baseCRD *extv1.CustomResourceDefinition) *extv1.Cu return crd } -// Helper function to convert to un. -func MustToUnstructured(obj any) map[string]any { - u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) - if err != nil { - panic(err) - } - - return u -} - // Helper type to track GetXRDs calls. type xrdCountingClient struct { tu.MockDefinitionClient @@ -501,7 +476,7 @@ func TestDefaultSchemaValidator_ValidateResources_AppliesDefaults(t *testing.T) } // Verify defaults were applied to the ORIGINAL resource - // The defaults are applied in-place by validate.SchemaValidation, so they persist + // The defaults are applied in-place by applyCRDDefaults before validation, so they persist deletionPolicy, found, err := un.NestedString(managedResource.Object, "spec", "deletionPolicy") if err != nil { t.Fatalf("Failed to get deletionPolicy: %v", err) @@ -534,50 +509,297 @@ func TestDefaultSchemaValidator_ValidateResources_AppliesDefaults(t *testing.T) // The compositionRevisionRef remains in the original resource, which is correct behavior. } -func TestExtractValidationErrors(t *testing.T) { +func TestFormatValidationErrors(t *testing.T) { tests := map[string]struct { - input string + reason string + result *pkgvalidate.ValidationResult expected string }{ - "SingleValidationError": { - input: "[x] schema validation error example.org/v1, my-xr : spec.region: Required value\nTotal 1 resources: 0 missing schemas, 0 success cases, 1 failure cases", - expected: "schema validation error example.org/v1, my-xr : spec.region: Required value", + "SingleSchemaError": { + reason: "A single field-level schema error renders as ' :' header plus an indented ' [schema]' line.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeSchema, + Field: "spec.region", + Message: "spec.region: Required value", + }}, + }}, + }, + expected: "example.org/v1/XR my-xr:\n spec.region: Required value [schema]", }, "SingleMissingSchema": { - input: "[!] could not find CRD/XRD for: other.org/v1, Kind=SomeResource\nTotal 1 resources: 1 missing schemas, 0 success cases, 0 failure cases", - expected: "could not find CRD/XRD for: other.org/v1, Kind=SomeResource", + reason: "A missing-schema resource collapses to a single ' : missing schema' line (no errors[] to expand).", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "other.org/v1", + Kind: "SomeResource", + Name: "thing", + Status: pkgvalidate.ValidationStatusMissingSchema, + }}, + }, + expected: "other.org/v1/SomeResource thing: missing schema", + }, + "MissingSchemaWithoutName": { + reason: "A resource without metadata.name collapses to just the GVK in the header rather than producing a trailing space.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "other.org/v1", + Kind: "SomeResource", + Status: pkgvalidate.ValidationStatusMissingSchema, + }}, + }, + expected: "other.org/v1/SomeResource: missing schema", }, - "MultipleErrors": { - input: "[x] schema validation error example.org/v1, my-xr : spec.region: Required value\n[!] could not find CRD/XRD for: other.org/v1\nTotal 2 resources: 1 missing schemas, 0 success cases, 1 failure cases", - expected: "schema validation error example.org/v1, my-xr : spec.region: Required value; could not find CRD/XRD for: other.org/v1", + "NamespacedResource": { + reason: "Namespaced resources render '/' in the header so cluster-vs-namespaced scope is visible.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "Thing", + Namespace: "production", + Name: "my-thing", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeSchema, + Message: "spec.size: Required value", + }}, + }}, + }, + expected: "example.org/v1/Thing production/my-thing:\n spec.size: Required value [schema]", }, - "MixedWithSuccessLines": { - input: "[✓] example.org/v1, good-xr validated successfully\n[x] schema validation error example.org/v1, bad-xr : spec.field: Invalid value\nTotal 2 resources: 0 missing schemas, 1 success cases, 1 failure cases", - expected: "schema validation error example.org/v1, bad-xr : spec.field: Invalid value", + "NamespaceWithoutNameCollapsesToGVK": { + reason: "When namespace is set but name is empty, the header collapses to just the GVK rather than rendering '/' with a stray trailing slash.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "Thing", + Namespace: "production", + Status: pkgvalidate.ValidationStatusMissingSchema, + }}, + }, + expected: "example.org/v1/Thing: missing schema", }, - "EmptyInput": { - input: "", - expected: "schema validation failed", + "MultipleErrorsGroupedUnderResource": { + reason: "Multiple errors on the same resource share a single header rather than producing a header per error.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{ + {Type: pkgvalidate.FieldErrorTypeSchema, Message: "spec.region: Required value"}, + {Type: pkgvalidate.FieldErrorTypeCEL, Message: "spec.replicas: must be > 0"}, + }, + }}, + }, + expected: "example.org/v1/XR my-xr:\n spec.region: Required value [schema]\n spec.replicas: must be > 0 [cel]", }, - "NoErrorsOnlySuccess": { - input: "[✓] example.org/v1, my-xr validated successfully\nTotal 1 resources: 0 missing schemas, 1 success cases, 0 failure cases", - expected: "schema validation failed", + "BadValueAppendedWhenAbsentFromMessage": { + reason: "When Value is set and the message doesn't already include it (typical for CEL errors), '(got )' is appended so users see what was rejected.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeCEL, + Message: "spec.replicas: must be > 0", + Value: -1, + }}, + }}, + }, + expected: "example.org/v1/XR my-xr:\n spec.replicas: must be > 0 (got -1) [cel]", }, - "WhitespaceHandling": { - input: " [x] error message with leading spaces \n", - expected: "error message with leading spaces", + "BadValueOmittedWhenAlreadyInMessage": { + reason: "k8s field errors typically embed the bad value as a quoted string in the message text; the '(got )' tail is omitted to avoid duplication.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeSchema, + Message: `spec.replicas: Invalid value: "five": must be integer`, + Value: "five", + }}, + }}, + }, + expected: `example.org/v1/XR my-xr:` + "\n" + ` spec.replicas: Invalid value: "five": must be integer [schema]`, + }, + "BadValueNotSuppressedByIncidentalSubstring": { + reason: "Regression: the duplication check matches the quoted form for strings (\"k\") so an unquoted incidental substring (\"k\" inside \"kind\") doesn't suppress the tail.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeCEL, + Message: "spec.kind: Required value", + Value: "k", + }}, + }}, + }, + expected: `example.org/v1/XR my-xr:` + "\n" + ` spec.kind: Required value (got "k") [cel]`, + }, + "NumericBadValueRendersUnquoted": { + reason: "Numbers / bools / structs aren't quoted in k8s messages and shouldn't be quoted in our display either.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeCEL, + Message: "spec.replicas: must be > 0", + Value: 42, + }}, + }}, + }, + expected: "example.org/v1/XR my-xr:\n spec.replicas: must be > 0 (got 42) [cel]", }, - "MultipleValidationErrors": { - input: "[x] error one\n[x] error two\n[x] error three", - expected: "error one; error two; error three", + "NumericBadValueNotSuppressedByDigitPrefix": { + reason: "Regression: a raw substring search would suppress the tail when the rendered number is a digit-prefix of another token (Value=42 vs message containing '420'). Word-boundary matching keeps the tail rendered so the user sees the actual rejected value.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeCEL, + Message: "spec.size: must be at most 420", + Value: 42, + }}, + }}, + }, + expected: "example.org/v1/XR my-xr:\n spec.size: must be at most 420 (got 42) [cel]", + }, + "NumericBadValueSuppressedAsToken": { + reason: "When the rendered number does appear as a standalone token in the message (delimited by non-word chars), the tail is correctly suppressed to avoid duplication.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeSchema, + Message: "spec.replicas: Invalid value: 42: must be at most 10", + Value: 42, + }}, + }}, + }, + expected: "example.org/v1/XR my-xr:\n spec.replicas: Invalid value: 42: must be at most 10 [schema]", + }, + "MultipleResourcesEachOnTheirOwnBlock": { + reason: "Failures from distinct resources produce distinct blocks joined by newlines, in input order.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{ + { + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeSchema, + Message: "spec.region: Required value", + }}, + }, + { + APIVersion: "other.org/v1", + Kind: "SomeResource", + Name: "thing", + Status: pkgvalidate.ValidationStatusMissingSchema, + }, + }, + }, + expected: "example.org/v1/XR my-xr:\n spec.region: Required value [schema]\nother.org/v1/SomeResource thing: missing schema", + }, + "ValidResourcesProduceNoBlock": { + reason: "Only failing resources contribute blocks; valid resources are filtered out so the output is the failure summary, not an audit log.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{ + { + APIVersion: "example.org/v1", + Kind: "XR", + Name: "good-xr", + Status: pkgvalidate.ValidationStatusValid, + }, + { + APIVersion: "example.org/v1", + Kind: "XR", + Name: "bad-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{{ + Type: pkgvalidate.FieldErrorTypeSchema, + Message: "spec.field: Invalid value", + }}, + }, + }, + }, + expected: "example.org/v1/XR bad-xr:\n spec.field: Invalid value [schema]", + }, + "NoApplicableEntriesReturnsEmpty": { + reason: "formatValidationErrors is only invoked after ResultError has flagged a failure; an all-valid input is unreachable in production. The contract is empty string (not a generic fallback) so the wrapping ResultError carries the message.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusValid, + }}, + }, + expected: "", + }, + "DefaultingErrorAccompanyingSchemaError": { + reason: "A defaulting entry that co-occurs with a schema-class error is suppressed: the schema entry already conveys the failure, and the defaulting line would just be noise.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{ + {Type: pkgvalidate.FieldErrorTypeDefaulting, Message: "cannot apply defaults"}, + {Type: pkgvalidate.FieldErrorTypeSchema, Message: "spec.field: Required value"}, + }, + }}, + }, + expected: "example.org/v1/XR my-xr:\n spec.field: Required value [schema]", + }, + "InvalidWithOnlyDefaultingErrorsStillEmitted": { + reason: "Defensive: upstream's statusFromErrors won't produce Invalid+only-defaulting today, but if it ever did we surface the defaulting message rather than emit an empty block.", + result: &pkgvalidate.ValidationResult{ + Resources: []pkgvalidate.ResourceValidationResult{{ + APIVersion: "example.org/v1", + Kind: "XR", + Name: "my-xr", + Status: pkgvalidate.ValidationStatusInvalid, + Errors: []pkgvalidate.FieldValidationError{ + {Type: pkgvalidate.FieldErrorTypeDefaulting, Message: "cannot apply defaults"}, + }, + }}, + }, + expected: "example.org/v1/XR my-xr:\n cannot apply defaults [defaulting]", }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { - result := extractValidationErrors(tt.input) - if result != tt.expected { - t.Errorf("extractValidationErrors() = %q, want %q", result, tt.expected) + got := formatValidationErrors(tt.result) + if got != tt.expected { + t.Errorf("\n%s\nformatValidationErrors() = %q, want %q", tt.reason, got, tt.expected) } }) } @@ -595,7 +817,6 @@ func TestDefaultSchemaValidator_ValidateScopeConstraints(t *testing.T) { tests := map[string]struct { setupClient func() *tu.MockSchemaClient - preloadedCRDs []*extv1.CustomResourceDefinition resource *un.Unstructured expectedNamespace string isClaimRoot bool @@ -608,7 +829,6 @@ func TestDefaultSchemaValidator_ValidateScopeConstraints(t *testing.T) { WithFoundCRD(testExampleOrg, "NamespacedResource", namespacedCRD). Build() }, - preloadedCRDs: []*extv1.CustomResourceDefinition{}, // No longer needed resource: tu.NewResource(testExampleOrg+"/v1", "NamespacedResource", "test-resource"). InNamespace("default"). Build(), @@ -622,7 +842,6 @@ func TestDefaultSchemaValidator_ValidateScopeConstraints(t *testing.T) { WithFoundCRD(testExampleOrg, "NamespacedResource", namespacedCRD). Build() }, - preloadedCRDs: []*extv1.CustomResourceDefinition{namespacedCRD}, resource: tu.NewResource(testExampleOrg+"/v1", "NamespacedResource", "test-resource"). Build(), // No namespace expectedNamespace: "default", @@ -636,7 +855,6 @@ func TestDefaultSchemaValidator_ValidateScopeConstraints(t *testing.T) { WithFoundCRD(testExampleOrg, "NamespacedResource", namespacedCRD). Build() }, - preloadedCRDs: []*extv1.CustomResourceDefinition{namespacedCRD}, resource: tu.NewResource(testExampleOrg+"/v1", "NamespacedResource", "test-resource"). InNamespace("wrong"). Build(), @@ -651,7 +869,6 @@ func TestDefaultSchemaValidator_ValidateScopeConstraints(t *testing.T) { WithFoundCRD(testExampleOrg, "ClusterResource", clusterCRD). Build() }, - preloadedCRDs: []*extv1.CustomResourceDefinition{clusterCRD}, resource: tu.NewResource(testExampleOrg+"/v1", "ClusterResource", "test-resource"). Build(), // No namespace - correct for cluster-scoped expectedNamespace: "", @@ -664,7 +881,6 @@ func TestDefaultSchemaValidator_ValidateScopeConstraints(t *testing.T) { WithFoundCRD(testExampleOrg, "ClusterResource", clusterCRD). Build() }, - preloadedCRDs: []*extv1.CustomResourceDefinition{clusterCRD}, resource: tu.NewResource(testExampleOrg+"/v1", "ClusterResource", "test-resource"). InNamespace("default"). Build(), @@ -679,7 +895,6 @@ func TestDefaultSchemaValidator_ValidateScopeConstraints(t *testing.T) { WithFoundCRD(testExampleOrg, "ClusterResource", clusterCRD). Build() }, - preloadedCRDs: []*extv1.CustomResourceDefinition{clusterCRD}, resource: tu.NewResource(testExampleOrg+"/v1", "ClusterResource", "test-resource"). Build(), expectedNamespace: "default", // XR is namespaced @@ -693,7 +908,6 @@ func TestDefaultSchemaValidator_ValidateScopeConstraints(t *testing.T) { WithFoundCRD(testExampleOrg, "ClusterResource", clusterCRD). Build() }, - preloadedCRDs: []*extv1.CustomResourceDefinition{clusterCRD}, resource: tu.NewResource(testExampleOrg+"/v1", "ClusterResource", "test-resource"). Build(), expectedNamespace: "default", // Claim is namespaced @@ -708,7 +922,6 @@ func TestDefaultSchemaValidator_ValidateScopeConstraints(t *testing.T) { }). Build() }, - preloadedCRDs: []*extv1.CustomResourceDefinition{}, resource: tu.NewResource(testExampleOrg+"/v1", "UnknownResource", "test-resource"). Build(), expectedNamespace: "default", diff --git a/cmd/diff/renderer/types/types.go b/cmd/diff/renderer/types/types.go index 3fbfd794..862665a8 100644 --- a/cmd/diff/renderer/types/types.go +++ b/cmd/diff/renderer/types/types.go @@ -212,9 +212,74 @@ func MakeDiffKeyFromResource(res *un.Unstructured) string { // OutputError represents an error in structured output. // Used consistently by both XR diff and comp diff for machine-readable error handling. // Note: Only JSON tags are used because sigs.k8s.io/yaml uses JSON tags for YAML serialization. +// +// ResourceID and ValidationFailures play complementary roles: +// +// - ResourceID identifies the input the diff command was processing +// (the XR or claim file the user fed in). It is "/" +// so machine consumers can correlate an error to a specific +// input across batched runs. +// +// - ValidationFailures, when non-empty, gives a structured +// per-resource breakdown of *what* the schema validator rejected +// within that input's render tree. It contains one entry per +// resource (the input itself plus any composed resource) that +// failed validation, with full GVK / namespace / typed errors, +// so consumers can drive UI or programmatic decisions without +// parsing the human-readable Message. +// +// The two fields therefore partially overlap when the input itself is +// among the failing resources — that's intentional. ValidationFailures +// is the complete failure list (so consumers iterating it never miss +// an XR-level rejection); ResourceID independently anchors the error +// to a user-supplied input. ValidationFailures is set only for schema +// validation errors; it is nil for tool, IO, and render errors. type OutputError struct { - ResourceID string `json:"resourceID,omitempty"` - Message string `json:"message"` + ResourceID string `json:"resourceID,omitempty"` + Message string `json:"message"` + ValidationFailures []ResourceValidationFailure `json:"validationFailures,omitempty"` +} + +// ResourceValidationFailure is the per-resource view inside an +// OutputError.ValidationFailures slice. It mirrors the shape of +// crossplane/cli's pkg/validate.ResourceValidationResult so the +// information transfers cleanly, but is defined here so crossplane-diff's +// JSON output schema is owned by us — consumers depend on this struct, +// not the upstream type. +type ResourceValidationFailure struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Name string `json:"name,omitempty"` + Namespace string `json:"namespace,omitempty"` + // Status is the validator-assigned outcome for this resource. + // Today the surfaced values are "invalid" and "missingSchema"; + // "valid" entries are filtered out by the converter so machine + // consumers iterating ValidationFailures see only the failure rows. + Status string `json:"status"` + Errors []FieldValidationError `json:"errors,omitempty"` +} + +// FieldValidationError is the wire shape for a single field-level +// validation error. Mirrors pkg/validate.FieldValidationError; defined +// here so consumers of our JSON output bind to a stable schema we +// control rather than the upstream type. +type FieldValidationError struct { + // Type categorizes the error: "schema", "cel", "unknownField", + // or "defaulting". + Type string `json:"type"` + // Field is the JSONPath of the offending field (e.g. + // "spec.forProvider.region"), set when the validator can pinpoint + // a path. Empty for errors with no field locality (e.g. + // defaulting failures). + Field string `json:"field,omitempty"` + // Message is the validator-emitted human-readable description. + // For k8s-derived schema errors this typically embeds the field + // path and bad value already. + Message string `json:"message"` + // Value, when set, is the offending value as the validator saw + // it. Type is preserved (string, number, bool, struct), so JSON + // consumers can present or compare it directly. + Value any `json:"value,omitempty"` } // FormatError returns a human-readable error string. diff --git a/cmd/diff/testutils/structured_assertions.go b/cmd/diff/testutils/structured_assertions.go index 81e65301..a4a37031 100644 --- a/cmd/diff/testutils/structured_assertions.go +++ b/cmd/diff/testutils/structured_assertions.go @@ -48,6 +48,7 @@ type DiffExpectation interface { type ExpectedDiff struct { summary *expectedSummary resources []*ResourceExpectation + errors []*ErrorExpectation } func (e *ExpectedDiff) expectation() *ExpectedDiff { return e } @@ -127,6 +128,145 @@ func (e *ExpectedDiff) WithModifiedResource(kind, name, namespace string) *Resou return r } +// ErrorExpectation describes one expected entry in the structured +// errors[] payload. Match is by ResourceID; ValidationFailures, when +// non-nil, asserts the per-resource typed validation breakdown. +type ErrorExpectation struct { + parent *ExpectedDiff + resourceID string + messageContains string + validationFailures []*ValidationFailureExpectation +} + +func (e *ErrorExpectation) expectation() *ExpectedDiff { return e.parent } + +// ValidationFailureExpectation describes one expected entry in +// errors[].validationFailures[]. Match is by APIVersion/Kind/Name; +// Errors asserts on the per-field validation entries within. +type ValidationFailureExpectation struct { + parent *ErrorExpectation + apiVersion string + kind string + name string + namespace string + status string // optional; if "", no assertion + errors []*FieldErrorExpectation // optional; nil = no assertion on per-field errors +} + +func (v *ValidationFailureExpectation) expectation() *ExpectedDiff { return v.parent.parent } + +// FieldErrorExpectation describes one expected entry in +// errors[].validationFailures[].errors[]. Match is by Type+Field +// (Field may be empty for non-localized errors); messageContains and +// value are optional refinements. +type FieldErrorExpectation struct { + parent *ValidationFailureExpectation + errType string + field string + messageContains string + hasValue bool + value any +} + +func (f *FieldErrorExpectation) expectation() *ExpectedDiff { return f.parent.parent.parent } + +// WithError attaches an expectation for an entry in the structured +// errors[] payload, matched by resourceID. Use the returned +// ErrorExpectation to add validationFailures expectations. +func (e *ExpectedDiff) WithError(resourceID string) *ErrorExpectation { + er := &ErrorExpectation{ + parent: e, + resourceID: resourceID, + } + e.errors = append(e.errors, er) + + return er +} + +// WithMessageContaining adds a substring assertion on the error's +// Message field. Optional — useful when machine consumers care about +// the human-readable message too. +func (e *ErrorExpectation) WithMessageContaining(s string) *ErrorExpectation { + e.messageContains = s + return e +} + +// WithValidationFailure adds an expectation for one entry in the +// error's validationFailures[] slice, matched by APIVersion/Kind/Name. +// namespace is optional (empty string skips the namespace assertion); +// pass the actual namespace to lock down namespaced resources. +func (e *ErrorExpectation) WithValidationFailure(apiVersion, kind, name, namespace string) *ValidationFailureExpectation { + v := &ValidationFailureExpectation{ + parent: e, + apiVersion: apiVersion, + kind: kind, + name: name, + namespace: namespace, + } + e.validationFailures = append(e.validationFailures, v) + + return v +} + +// AndError returns to the parent ExpectedDiff to chain another +// WithError call, mirroring the AndXR / AndComp helpers elsewhere in +// this file. +func (e *ErrorExpectation) AndError() *ExpectedDiff { + return e.parent +} + +// WithStatus pins the validation status on this resource entry +// ("invalid", "missingSchema"). Optional; empty status skips the +// assertion. +func (v *ValidationFailureExpectation) WithStatus(status string) *ValidationFailureExpectation { + v.status = status + return v +} + +// WithFieldError adds an expectation for one entry in the failure's +// errors[] slice, matched by errType ("schema", "cel", +// "unknownField", "defaulting") and field (empty matches +// non-localized errors like defaulting). +func (v *ValidationFailureExpectation) WithFieldError(errType, field string) *FieldErrorExpectation { + f := &FieldErrorExpectation{ + parent: v, + errType: errType, + field: field, + } + v.errors = append(v.errors, f) + + return f +} + +// AndValidationFailure returns to the parent ErrorExpectation to chain +// another WithValidationFailure. +func (v *ValidationFailureExpectation) AndValidationFailure() *ErrorExpectation { + return v.parent +} + +// WithMessageContaining adds a substring assertion on this field +// error's Message. Optional. +func (f *FieldErrorExpectation) WithMessageContaining(s string) *FieldErrorExpectation { + f.messageContains = s + return f +} + +// WithValue pins the bad value on this field error. Pass the value as +// it would arrive after JSON decoding (e.g. float64 for numbers, +// string for strings); for "any number is fine" omit this method. +func (f *FieldErrorExpectation) WithValue(v any) *FieldErrorExpectation { + f.hasValue = true + f.value = v + + return f +} + +// AndFieldError returns to the parent ValidationFailureExpectation to +// chain another WithFieldError. +func (f *FieldErrorExpectation) AndFieldError() *ValidationFailureExpectation { + return f.parent +} + // WithRemovedResource adds an expectation for a removed resource. func (e *ExpectedDiff) WithRemovedResource(kind, name, namespace string) *ResourceExpectation { r := &ResourceExpectation{ @@ -311,6 +451,146 @@ func AssertStructuredDiff(t *testing.T, jsonOutput string, e DiffExpectation) { if len(expected.resources) > 0 && len(output.Changes) != len(expected.resources) { t.Errorf("Expected %d changes, got %d", len(expected.resources), len(output.Changes)) } + + // Check each error expectation against output.Errors. + for _, want := range expected.errors { + got := findMatchingError(output.Errors, want.resourceID) + if got == nil { + actualIDs := make([]string, 0, len(output.Errors)) + for _, e := range output.Errors { + actualIDs = append(actualIDs, e.ResourceID) + } + + t.Errorf("Expected error for resourceID %q not found in output. Actual error IDs: %v", + want.resourceID, actualIDs) + + continue + } + + assertErrorMatch(t, *got, want) + } + + // If the test pinned a specific number of error expectations, verify + // no extras were emitted. Zero expectations = no opinion on errors. + if len(expected.errors) > 0 && len(output.Errors) != len(expected.errors) { + t.Errorf("Expected %d errors in structured output, got %d", len(expected.errors), len(output.Errors)) + } +} + +// assertErrorMatch validates a single OutputError against its +// expectation. Split out from AssertStructuredDiff so the per-error +// branching doesn't push that function further over the gocognit +// threshold its existing exemption already silences. +func assertErrorMatch(t *testing.T, got OutputError, want *ErrorExpectation) { + t.Helper() + + if want.messageContains != "" && !strings.Contains(got.Message, want.messageContains) { + t.Errorf("error %q: message %q does not contain %q", + want.resourceID, got.Message, want.messageContains) + } + + for _, wantVF := range want.validationFailures { + gotVF := findMatchingValidationFailure(got.ValidationFailures, wantVF) + if gotVF == nil { + actual := make([]string, 0, len(got.ValidationFailures)) + for _, vf := range got.ValidationFailures { + actual = append(actual, fmt.Sprintf("%s/%s %s/%s", vf.APIVersion, vf.Kind, vf.Namespace, vf.Name)) + } + + t.Errorf("error %q: expected validationFailure %s/%s %s/%s not found. Actual: %v", + want.resourceID, wantVF.apiVersion, wantVF.kind, wantVF.namespace, wantVF.name, actual) + + continue + } + + assertValidationFailureMatch(t, *gotVF, wantVF) + } + + if len(want.validationFailures) > 0 && len(got.ValidationFailures) != len(want.validationFailures) { + t.Errorf("error %q: expected %d validationFailures, got %d", + want.resourceID, len(want.validationFailures), len(got.ValidationFailures)) + } +} + +// assertValidationFailureMatch validates a single +// ResourceValidationFailure against its expectation. +func assertValidationFailureMatch(t *testing.T, got ResourceValidationFailure, want *ValidationFailureExpectation) { + t.Helper() + + if want.status != "" && got.Status != want.status { + t.Errorf("validationFailure %s/%s %s: status: expected %q, got %q", + want.apiVersion, want.kind, want.name, want.status, got.Status) + } + + for _, wantFE := range want.errors { + gotFE := findMatchingFieldError(got.Errors, wantFE) + if gotFE == nil { + actual := make([]string, 0, len(got.Errors)) + for _, fe := range got.Errors { + actual = append(actual, fmt.Sprintf("[%s] %s", fe.Type, fe.Field)) + } + + t.Errorf("validationFailure %s/%s %s: expected fieldError [%s] %s not found. Actual: %v", + want.apiVersion, want.kind, want.name, wantFE.errType, wantFE.field, actual) + + continue + } + + if wantFE.messageContains != "" && !strings.Contains(gotFE.Message, wantFE.messageContains) { + t.Errorf("validationFailure %s/%s %s: fieldError [%s] %s: message %q does not contain %q", + want.apiVersion, want.kind, want.name, wantFE.errType, wantFE.field, + gotFE.Message, wantFE.messageContains) + } + + if wantFE.hasValue && !reflect.DeepEqual(gotFE.Value, wantFE.value) { + t.Errorf("validationFailure %s/%s %s: fieldError [%s] %s: value: expected %v (%T), got %v (%T)", + want.apiVersion, want.kind, want.name, wantFE.errType, wantFE.field, + wantFE.value, wantFE.value, gotFE.Value, gotFE.Value) + } + } + + if len(want.errors) > 0 && len(got.Errors) != len(want.errors) { + t.Errorf("validationFailure %s/%s %s: expected %d field errors, got %d", + want.apiVersion, want.kind, want.name, len(want.errors), len(got.Errors)) + } +} + +func findMatchingError(errs []OutputError, resourceID string) *OutputError { + for i := range errs { + if errs[i].ResourceID == resourceID { + return &errs[i] + } + } + + return nil +} + +func findMatchingValidationFailure(vfs []ResourceValidationFailure, want *ValidationFailureExpectation) *ResourceValidationFailure { + for i := range vfs { + v := &vfs[i] + if v.APIVersion != want.apiVersion || v.Kind != want.kind || v.Name != want.name { + continue + } + + if want.namespace != "" && v.Namespace != want.namespace { + continue + } + + return v + } + + return nil +} + +func findMatchingFieldError(fes []FieldValidationError, want *FieldErrorExpectation) *FieldValidationError { + for i := range fes { + fe := &fes[i] + if fe.Type == want.errType && fe.Field == want.field { + return fe + } + } + + return nil } // findMatchingChange finds a change that matches the resource expectation. @@ -449,8 +729,27 @@ type StructuredCompDiffOutput struct { // OutputError mirrors dt.OutputError. type OutputError struct { - ResourceID string `json:"resourceID,omitempty"` - Message string `json:"message"` + ResourceID string `json:"resourceID,omitempty"` + Message string `json:"message"` + ValidationFailures []ResourceValidationFailure `json:"validationFailures,omitempty"` +} + +// ResourceValidationFailure mirrors dt.ResourceValidationFailure. +type ResourceValidationFailure struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Name string `json:"name,omitempty"` + Namespace string `json:"namespace,omitempty"` + Status string `json:"status"` + Errors []FieldValidationError `json:"errors,omitempty"` +} + +// FieldValidationError mirrors dt.FieldValidationError. +type FieldValidationError struct { + Type string `json:"type"` + Field string `json:"field,omitempty"` + Message string `json:"message"` + Value any `json:"value,omitempty"` } // CompositionDiffJSON mirrors compositionDiffJSON from the renderer. diff --git a/go.mod b/go.mod index e8ac0937..73b0cb72 100644 --- a/go.mod +++ b/go.mod @@ -6,9 +6,9 @@ require ( dario.cat/mergo v1.0.2 github.com/Masterminds/semver v1.5.0 github.com/alecthomas/kong v1.15.0 - github.com/crossplane/cli/v2 v2.3.2 - github.com/crossplane/crossplane-runtime/v2 v2.3.2 - github.com/crossplane/crossplane/apis/v2 v2.3.2 + github.com/crossplane/cli/v2 v2.4.0-rc.0.0.20260615182009-ba59fbfac34b + github.com/crossplane/crossplane-runtime/v2 v2.4.0-rc.0 + github.com/crossplane/crossplane/apis/v2 v2.4.0-rc.0 github.com/crossplane/crossplane/v2 v2.3.2 github.com/docker/docker v28.5.2+incompatible github.com/google/go-cmp v0.7.0 @@ -32,7 +32,6 @@ require ( require ( cel.dev/expr v0.25.1 // indirect github.com/MakeNowJust/heredoc v1.0.0 // indirect - github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/antlr4-go/antlr/v4 v4.13.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/chai2010/gettext-go v1.0.2 // indirect @@ -121,7 +120,6 @@ require ( github.com/google/uuid v1.6.0 github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/json-iterator/go v1.1.12 // indirect - github.com/klauspost/compress v1.18.6 // indirect github.com/mattn/go-colorable v0.1.14 // indirect github.com/mattn/go-isatty v0.0.22 // indirect github.com/moby/spdystream v0.5.1 // indirect diff --git a/go.sum b/go.sum index 7f86603e..77dadda5 100644 --- a/go.sum +++ b/go.sum @@ -47,12 +47,12 @@ github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSV github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= -github.com/crossplane/cli/v2 v2.3.2 h1:9zDVZN9uHanIjp5wCzAe8NshyxrAuQWW/4me4VkNtH4= -github.com/crossplane/cli/v2 v2.3.2/go.mod h1:r0lKyzouUxl89nXLdgdRWh5vEcynpP3CDP/jmmfAKnI= -github.com/crossplane/crossplane-runtime/v2 v2.3.2 h1:gjfJmr0PTf3/Ccg4iasogXKIRjYdEMILduiP/IZN260= -github.com/crossplane/crossplane-runtime/v2 v2.3.2/go.mod h1:POGt8DSTcxQJlTww+3yGeeXuEdLyjZ61vZ3ap5tTxhE= -github.com/crossplane/crossplane/apis/v2 v2.3.2 h1:Drs3xz59qT3zFfaszxQWqr51a0leAx20DBL4TqMnqi0= -github.com/crossplane/crossplane/apis/v2 v2.3.2/go.mod h1:o+D0ktZQKJCFcpfzMKA4n53aTo2sFqqDsADBNIRuIyE= +github.com/crossplane/cli/v2 v2.4.0-rc.0.0.20260615182009-ba59fbfac34b h1:Ol4CZMZvcS/m3IYRz4YceZGj6pCF1TLyN+nTpgMgDPg= +github.com/crossplane/cli/v2 v2.4.0-rc.0.0.20260615182009-ba59fbfac34b/go.mod h1:TVfHZdpkSlrfkE6V8VLlTOS/DT4Qsxc+ybMcUnZz3ko= +github.com/crossplane/crossplane-runtime/v2 v2.4.0-rc.0 h1:Zgiq+hrh9lbjWtv8ECCLd1A0I9knt3c8ZUELExw6M1w= +github.com/crossplane/crossplane-runtime/v2 v2.4.0-rc.0/go.mod h1:PAo3zIfmMzrS18HGyHJLXCeXIp0nFW2Md2Fn9gocMaU= +github.com/crossplane/crossplane/apis/v2 v2.4.0-rc.0 h1:4PBahj+tnK9RwSZm1bYGvOkHOU+1CSHjJF2PoPzBMD0= +github.com/crossplane/crossplane/apis/v2 v2.4.0-rc.0/go.mod h1:xaQozPfGYv6ut6yZP8maDQm7ZTynHAGUffecZ5hqmhg= github.com/crossplane/crossplane/v2 v2.3.2 h1:tRkV1QjXBbEkohlfBWyZ6hoe1UQJcD9bwPz43mTxUuU= github.com/crossplane/crossplane/v2 v2.3.2/go.mod h1:lx6VH4QhRhWDmLsd59QXv2dp9KrBrClX7NR28QQKvTM= github.com/crossplane/function-sdk-go v0.7.1 h1:BEM2b7nAr8rJYc9tjwnoMfVVy1veCA/juD8APYCVLi0=