Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 56 additions & 18 deletions cmd/diff/diff_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
9 changes: 5 additions & 4 deletions cmd/diff/diffprocessor/comp_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
}
Expand Down
18 changes: 11 additions & 7 deletions cmd/diff/diffprocessor/diff_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()

Expand All @@ -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)
}
Expand Down
122 changes: 120 additions & 2 deletions cmd/diff/diffprocessor/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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,
Expand All @@ -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
Expand Down
Loading
Loading