From bd17ed261015c5110a426eb3df486f87cb72d7cc Mon Sep 17 00:00:00 2001 From: everettraven Date: Fri, 1 Nov 2024 17:23:07 -0400 Subject: [PATCH] better output formatting Signed-off-by: everettraven --- cli/root.go | 4 +- pkg/validations/property/default.go | 11 +- pkg/validations/property/enum.go | 8 +- pkg/validations/property/max.go | 30 ++-- pkg/validations/property/min.go | 30 ++-- pkg/validations/property/property.go | 3 +- pkg/validations/property/required.go | 8 +- pkg/validations/property/type.go | 9 +- pkg/validations/results/result.go | 39 ++++++ .../validators/crd/existingfieldremoval.go | 16 ++- pkg/validations/validators/crd/scope.go | 8 +- .../validators/crd/storedversionremoval.go | 8 +- pkg/validations/validators/crd/validator.go | 20 ++- .../validators/version/validator.go | 129 +++++++++++++----- 14 files changed, 250 insertions(+), 73 deletions(-) create mode 100644 pkg/validations/results/result.go diff --git a/cli/root.go b/cli/root.go index 1703535..f999d90 100644 --- a/cli/root.go +++ b/cli/root.go @@ -1,6 +1,7 @@ package cli import ( + "errors" "io" "log" "net/url" @@ -90,7 +91,8 @@ Example use cases: err = validator.Validate(oldCrd, newCrd) if err != nil { - log.Fatalf("comparing old and new CustomResourceDefinitions: %v", err) + baseErr := errors.New("comparing old and new CustomResourceDefinitions") + log.Fatal(errors.Join(baseErr, err)) } }, } diff --git a/pkg/validations/property/default.go b/pkg/validations/property/default.go index 51e0d16..918df66 100644 --- a/pkg/validations/property/default.go +++ b/pkg/validations/property/default.go @@ -3,15 +3,17 @@ package property import ( "bytes" "fmt" + + "github.com/everettraven/crd-diff/pkg/validations/results" ) type Default struct{} func (d *Default) Name() string { - return "Default" + return "Default" } -func (d *Default) Validate(diff Diff) (bool, error) { +func (d *Default) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -31,5 +33,8 @@ func (d *Default) Validate(diff Diff) (bool, error) { err = fmt.Errorf("default value changed from %q to %q", string(diff.Old().Default.Raw), string(diff.New().Default.Raw)) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } diff --git a/pkg/validations/property/enum.go b/pkg/validations/property/enum.go index 14acb77..e31e550 100644 --- a/pkg/validations/property/enum.go +++ b/pkg/validations/property/enum.go @@ -3,6 +3,7 @@ package property import ( "fmt" + "github.com/everettraven/crd-diff/pkg/validations/results" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/util/sets" ) @@ -13,7 +14,7 @@ func (e *Enum) Name() string { return "Enum" } -func (e *Enum) Validate(diff Diff) (bool, error) { +func (e *Enum) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -41,5 +42,8 @@ func (e *Enum) Validate(diff Diff) (bool, error) { err = fmt.Errorf("enums %v removed from the set of previously allowed values", diffEnums.UnsortedList()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } diff --git a/pkg/validations/property/max.go b/pkg/validations/property/max.go index 0052f60..97ffe2d 100644 --- a/pkg/validations/property/max.go +++ b/pkg/validations/property/max.go @@ -3,6 +3,8 @@ package property import ( "cmp" "fmt" + + "github.com/everettraven/crd-diff/pkg/validations/results" ) func maxVerification[T cmp.Ordered](older *T, newer *T) error { @@ -22,7 +24,7 @@ func (m *Maximum) Name() string { return "Maximum" } -func (m *Maximum) Validate(diff Diff) (bool, error) { +func (m *Maximum) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -36,7 +38,10 @@ func (m *Maximum) Validate(diff Diff) (bool, error) { err = fmt.Errorf("maximum: %s", err.Error()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } type MaxItems struct{} @@ -45,7 +50,7 @@ func (m *MaxItems) Name() string { return "MaxItems" } -func (m *MaxItems) Validate(diff Diff) (bool, error) { +func (m *MaxItems) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -59,7 +64,10 @@ func (m *MaxItems) Validate(diff Diff) (bool, error) { err = fmt.Errorf("maxItems: %s", err.Error()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } type MaxLength struct{} @@ -68,7 +76,7 @@ func (m *MaxLength) Name() string { return "MaxLength" } -func (m *MaxLength) Validate(diff Diff) (bool, error) { +func (m *MaxLength) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -82,7 +90,10 @@ func (m *MaxLength) Validate(diff Diff) (bool, error) { err = fmt.Errorf("maxLength: %s", err.Error()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } type MaxProperties struct{} @@ -91,7 +102,7 @@ func (m *MaxProperties) Name() string { return "MaxProperties" } -func (m *MaxProperties) Validate(diff Diff) (bool, error) { +func (m *MaxProperties) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -105,5 +116,8 @@ func (m *MaxProperties) Validate(diff Diff) (bool, error) { err = fmt.Errorf("maxProperties: %s", err.Error()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } diff --git a/pkg/validations/property/min.go b/pkg/validations/property/min.go index 9ad8e16..e2cce60 100644 --- a/pkg/validations/property/min.go +++ b/pkg/validations/property/min.go @@ -3,6 +3,8 @@ package property import ( "cmp" "fmt" + + "github.com/everettraven/crd-diff/pkg/validations/results" ) func minVerification[T cmp.Ordered](older *T, newer *T) error { @@ -22,7 +24,7 @@ func (m *Minimum) Name() string { return "Minimum" } -func (m *Minimum) Validate(diff Diff) (bool, error) { +func (m *Minimum) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -36,7 +38,10 @@ func (m *Minimum) Validate(diff Diff) (bool, error) { err = fmt.Errorf("minimum: %s", err.Error()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } type MinItems struct{} @@ -45,7 +50,7 @@ func (m *MinItems) Name() string { return "MinItems" } -func (m *MinItems) Validate(diff Diff) (bool, error) { +func (m *MinItems) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -59,7 +64,10 @@ func (m *MinItems) Validate(diff Diff) (bool, error) { err = fmt.Errorf("minItems: %s", err.Error()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } type MinLength struct{} @@ -68,7 +76,7 @@ func (m *MinLength) Name() string { return "MinLength" } -func (m *MinLength) Validate(diff Diff) (bool, error) { +func (m *MinLength) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -82,7 +90,10 @@ func (m *MinLength) Validate(diff Diff) (bool, error) { err = fmt.Errorf("minLength: %s", err.Error()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } type MinProperties struct{} @@ -91,7 +102,7 @@ func (m *MinProperties) Name() string { return "MinProperties" } -func (m *MinProperties) Validate(diff Diff) (bool, error) { +func (m *MinProperties) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -105,5 +116,8 @@ func (m *MinProperties) Validate(diff Diff) (bool, error) { err = fmt.Errorf("minProperties: %s", err.Error()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } diff --git a/pkg/validations/property/property.go b/pkg/validations/property/property.go index db8ce15..78fa64b 100644 --- a/pkg/validations/property/property.go +++ b/pkg/validations/property/property.go @@ -1,6 +1,7 @@ package property import ( + "github.com/everettraven/crd-diff/pkg/validations/results" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -30,6 +31,6 @@ func (pd *diff) New() *apiextensionsv1.JSONSchemaProps { } type Validation interface { - Validate(Diff) (bool, error) + Validate(Diff) (bool, *results.Result) Name() string } diff --git a/pkg/validations/property/required.go b/pkg/validations/property/required.go index bf73413..3e3fbfc 100644 --- a/pkg/validations/property/required.go +++ b/pkg/validations/property/required.go @@ -3,6 +3,7 @@ package property import ( "fmt" + "github.com/everettraven/crd-diff/pkg/validations/results" "k8s.io/apimachinery/pkg/util/sets" ) @@ -12,7 +13,7 @@ func (r *Required) Name() string { return "Required" } -func (r *Required) Validate(diff Diff) (bool, error) { +func (r *Required) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -30,5 +31,8 @@ func (r *Required) Validate(diff Diff) (bool, error) { err = fmt.Errorf("new required fields %v added", diffRequired.UnsortedList()) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } diff --git a/pkg/validations/property/type.go b/pkg/validations/property/type.go index 2a7e9f9..5102416 100644 --- a/pkg/validations/property/type.go +++ b/pkg/validations/property/type.go @@ -2,6 +2,8 @@ package property import ( "fmt" + + "github.com/everettraven/crd-diff/pkg/validations/results" ) type Type struct{} @@ -10,7 +12,7 @@ func (t *Type) Name() string { return "Type" } -func (t *Type) Validate(diff Diff) (bool, error) { +func (t *Type) Validate(diff Diff) (bool, *results.Result) { reset := func(diff Diff) Diff { oldProperty := diff.Old() newProperty := diff.New() @@ -24,5 +26,8 @@ func (t *Type) Validate(diff Diff) (bool, error) { err = fmt.Errorf("type changed from %q to %q", diff.Old().Type, diff.New().Type) } - return IsHandled(diff, reset), err + return IsHandled(diff, reset), &results.Result{ + Error: err, + Subresults: []*results.Result{}, + } } diff --git a/pkg/validations/results/result.go b/pkg/validations/results/result.go new file mode 100644 index 0000000..03713bb --- /dev/null +++ b/pkg/validations/results/result.go @@ -0,0 +1,39 @@ +package results + +import ( + "errors" + "fmt" + "strings" +) + +type Result struct { + Error error + Subresults []*Result +} + +func ErrorFromResult(res *Result, depth int) error { + if res == nil { + return nil + } + + if res.Error == nil && len(res.Subresults) == 0 { + return nil + } + + var out strings.Builder + nestedErrors := []error{} + for _, subresult := range res.Subresults { + nestedErrors = append(nestedErrors, ErrorFromResult(subresult, depth+1)) + } + + if res.Error != nil { + out.WriteString(fmt.Sprintf("%s%s\n", strings.Repeat(" ", depth*2), res.Error.Error())) + } + + for _, nestedErr := range nestedErrors { + if nestedErr != nil { + out.WriteString(nestedErr.Error()) + } + } + return errors.New(out.String()) +} diff --git a/pkg/validations/validators/crd/existingfieldremoval.go b/pkg/validations/validators/crd/existingfieldremoval.go index 61d8e05..d764a8b 100644 --- a/pkg/validations/validators/crd/existingfieldremoval.go +++ b/pkg/validations/validators/crd/existingfieldremoval.go @@ -4,6 +4,7 @@ import ( "errors" "strings" + "github.com/everettraven/crd-diff/pkg/validations/results" "github.com/openshift/crd-schema-checker/pkg/manifestcomparators" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -14,16 +15,21 @@ func (efr *ExistingFieldRemoval) Name() string { return "ExistingFieldRemoval" } -func (efr *ExistingFieldRemoval) Validate(old, new *apiextensionsv1.CustomResourceDefinition) error { +func (efr *ExistingFieldRemoval) Validate(old, new *apiextensionsv1.CustomResourceDefinition) *results.Result { + result := &results.Result{ + Subresults: []*results.Result{}, + } reg := manifestcomparators.NewRegistry() err := reg.AddComparator(manifestcomparators.NoFieldRemoval()) if err != nil { - return err + result.Error = err + return result } results, errs := reg.Compare(old, new) if len(errs) > 0 { - return errors.Join(errs...) + result.Error = errors.Join(errs...) + return result } errSet := []error{} @@ -34,8 +40,8 @@ func (efr *ExistingFieldRemoval) Validate(old, new *apiextensionsv1.CustomResour } } if len(errSet) > 0 { - return errors.Join(errSet...) + result.Error = errors.Join(errSet...) } - return nil + return result } diff --git a/pkg/validations/validators/crd/scope.go b/pkg/validations/validators/crd/scope.go index 2fcd161..bf10e59 100644 --- a/pkg/validations/validators/crd/scope.go +++ b/pkg/validations/validators/crd/scope.go @@ -3,6 +3,7 @@ package crd import ( "fmt" + "github.com/everettraven/crd-diff/pkg/validations/results" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -12,9 +13,12 @@ func (s *Scope) Name() string { return "Scope" } -func (s *Scope) Validate(old, new *apiextensionsv1.CustomResourceDefinition) error { +func (s *Scope) Validate(old, new *apiextensionsv1.CustomResourceDefinition) *results.Result { if old.Spec.Scope != new.Spec.Scope { - return fmt.Errorf("scope changed from %q to %q", old.Spec.Scope, new.Spec.Scope) + return &results.Result{ + Error: fmt.Errorf("scope changed from %q to %q", old.Spec.Scope, new.Spec.Scope), + Subresults: []*results.Result{}, + } } return nil } diff --git a/pkg/validations/validators/crd/storedversionremoval.go b/pkg/validations/validators/crd/storedversionremoval.go index cfb516b..d307b8b 100644 --- a/pkg/validations/validators/crd/storedversionremoval.go +++ b/pkg/validations/validators/crd/storedversionremoval.go @@ -3,6 +3,7 @@ package crd import ( "fmt" + "github.com/everettraven/crd-diff/pkg/validations/results" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/util/sets" ) @@ -13,7 +14,7 @@ func (svr *StoredVersionRemoval) Name() string { return "StoredVersionRemoval" } -func (svr *StoredVersionRemoval) Validate(old, new *apiextensionsv1.CustomResourceDefinition) error { +func (svr *StoredVersionRemoval) Validate(old, new *apiextensionsv1.CustomResourceDefinition) *results.Result { newVersions := sets.New[string]() for _, version := range new.Spec.Versions { if !newVersions.Has(version.Name) { @@ -29,7 +30,10 @@ func (svr *StoredVersionRemoval) Validate(old, new *apiextensionsv1.CustomResour } if len(removedVersions) > 0 { - return fmt.Errorf("stored versions %v removed", removedVersions) + return &results.Result{ + Error: fmt.Errorf("stored versions %v removed", removedVersions), + Subresults: []*results.Result{}, + } } return nil diff --git a/pkg/validations/validators/crd/validator.go b/pkg/validations/validators/crd/validator.go index f6ddcf1..058a8f6 100644 --- a/pkg/validations/validators/crd/validator.go +++ b/pkg/validations/validators/crd/validator.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" + "github.com/everettraven/crd-diff/pkg/validations/results" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -12,7 +13,7 @@ import ( type Validation interface { // Validate performs the validation, returning an error if the // new revision is incompatible with the old revision of the CustomResourceDefinition - Validate(old, new *apiextensionsv1.CustomResourceDefinition) error + Validate(old, new *apiextensionsv1.CustomResourceDefinition) *results.Result // Name is a human-readable name for this validation Name() string @@ -44,11 +45,20 @@ func NewValidator(opts ...ValidatorOption) *Validator { // Validate runs the validations configured in the Validator func (v *Validator) Validate(old, new *apiextensionsv1.CustomResourceDefinition) error { - errs := []error{} + result := &results.Result{ + Subresults: []*results.Result{}, + } for _, validation := range v.validations { - if err := validation.Validate(old, new); err != nil { - errs = append(errs, fmt.Errorf("%q validation failed: %w", validation.Name(), err)) + if res := validation.Validate(old, new); res != nil { + subResult := &results.Result{ + Subresults: []*results.Result{res}, + } + if res.Error != nil { + subResult.Error = fmt.Errorf("%q validation failed", validation.Name()) + result.Error = errors.New("potentially breaking changes found") + } + result.Subresults = append(result.Subresults, subResult) } } - return errors.Join(errs...) + return results.ErrorFromResult(result, 0) } diff --git a/pkg/validations/validators/version/validator.go b/pkg/validations/validators/version/validator.go index 9b42836..cb57989 100644 --- a/pkg/validations/validators/version/validator.go +++ b/pkg/validations/validators/version/validator.go @@ -6,6 +6,7 @@ import ( "slices" "github.com/everettraven/crd-diff/pkg/validations/property" + "github.com/everettraven/crd-diff/pkg/validations/results" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" versionhelper "k8s.io/apimachinery/pkg/version" ) @@ -74,26 +75,36 @@ func (v *Validator) Name() string { return "Version" } -func (v *Validator) Validate(old, new *apiextensionsv1.CustomResourceDefinition) error { - errs := []error{} +func (v *Validator) Validate(old, new *apiextensionsv1.CustomResourceDefinition) *results.Result { + result := &results.Result{ + Subresults: []*results.Result{}, + } if !v.sameVersionConfig.Skip { - err := v.ValidateSameVersions(old, new) - if err != nil { - errs = append(errs, fmt.Errorf("validating same versions: %w", err)) + res := v.ValidateSameVersions(old, new) + if res != nil { + result.Subresults = append(result.Subresults, res) + if res.Error != nil { + result.Error = errors.New("checking versions for compatibility") + } } } if !v.servedVersionConfig.Skip { - err := v.ValidateServedVersions(new) - if err != nil { - errs = append(errs, fmt.Errorf("validating new served versions: %w", err)) + res := v.ValidateServedVersions(new) + if res != nil { + result.Subresults = append(result.Subresults, res) + if res.Error != nil { + result.Error = errors.New("checking versions for compatibility") + } } } - return errors.Join(errs...) + return result } -func (v *Validator) ValidateSameVersions(old, new *apiextensionsv1.CustomResourceDefinition) error { - errs := []error{} +func (v *Validator) ValidateSameVersions(old, new *apiextensionsv1.CustomResourceDefinition) *results.Result { + result := &results.Result{ + Subresults: []*results.Result{}, + } for _, oldVersion := range old.Spec.Versions { newVersion := GetCRDVersionByName(new, oldVersion.Name) // in this case, there is nothing to compare. Generally, the removal @@ -106,14 +117,24 @@ func (v *Validator) ValidateSameVersions(old, new *apiextensionsv1.CustomResourc continue } - errs = append(errs, CompareVersions(oldVersion, *newVersion, v.sameVersionConfig.UnhandledFailureMode, v.sameVersionConfig.Validations)) + res := CompareVersions(oldVersion, *newVersion, v.sameVersionConfig.UnhandledFailureMode, v.sameVersionConfig.Validations) + if res != nil { + subResult := &results.Result{ + Subresults: []*results.Result{res}, + } + if res.Error != nil { + result.Error = errors.New("comparing same versions") + subResult.Error = fmt.Errorf("comparing version %q to version %q", oldVersion.Name, newVersion.Name) + } + result.Subresults = append(result.Subresults, subResult) + } } - return errors.Join(errs...) + return result } -func (v *Validator) ValidateServedVersions(crd *apiextensionsv1.CustomResourceDefinition) error { +func (v *Validator) ValidateServedVersions(crd *apiextensionsv1.CustomResourceDefinition) *results.Result { // If conversion webhook is specified, pass check - if crd.Spec.Conversion != nil && crd.Spec.Conversion.Strategy == apiextensionsv1.WebhookConverter { + if !v.servedVersionConfig.IgnoreConversion && crd.Spec.Conversion != nil && crd.Spec.Conversion.Strategy == apiextensionsv1.WebhookConverter { return nil } @@ -128,38 +149,82 @@ func (v *Validator) ValidateServedVersions(crd *apiextensionsv1.CustomResourceDe return versionhelper.CompareKubeAwareVersionStrings(a.Name, b.Name) }) - errs := []error{} + result := &results.Result{ + Subresults: []*results.Result{}, + } for i, oldVersion := range servedVersions[:len(servedVersions)-1] { for _, newVersion := range servedVersions[i+1:] { - errs = append(errs, CompareVersions(oldVersion, newVersion, v.servedVersionConfig.UnhandledFailureMode, v.servedVersionConfig.Validations)) + res := CompareVersions(oldVersion, newVersion, v.servedVersionConfig.UnhandledFailureMode, v.servedVersionConfig.Validations) + if res != nil { + subResult := &results.Result{ + Subresults: []*results.Result{res}, + } + if res.Error != nil { + result.Error = errors.New("comparing served versions") + subResult.Error = fmt.Errorf("comparing version %q to version %q", oldVersion.Name, newVersion.Name) + } + result.Subresults = append(result.Subresults, subResult) + } } } - return errors.Join(errs...) + return result } -func CompareVersions(old, new apiextensionsv1.CustomResourceDefinitionVersion, failureMode FailureMode, validations []property.Validation) error { +func CompareVersions(old, new apiextensionsv1.CustomResourceDefinitionVersion, failureMode FailureMode, validations []property.Validation) *results.Result { oldFlattened := FlattenCRDVersion(old) newFlattened := FlattenCRDVersion(new) diffs := FlattenedCRDVersionDiff(oldFlattened, newFlattened) - errs := []error{} + result := &results.Result{ + Subresults: []*results.Result{}, + } for property, diff := range diffs { - handled := false - for _, validation := range validations { - ok, err := validation.Validate(diff) - if err != nil { - errs = append(errs, fmt.Errorf("old version %q compared to new version %q, property %q failed validation %q: %w", old.Name, new.Name, property, validation.Name(), err)) + res := ComparePropertyDiff(diff, failureMode, validations) + if res != nil { + subResult := &results.Result{ + Subresults: []*results.Result{res}, } - // if the validation handled this difference continue to the next difference - if ok { - handled = true - break + if res.Error != nil { + result.Error = errors.New("property validation failures") + propError := fmt.Errorf("property %q", property) + subResult.Error = propError } + result.Subresults = append(result.Subresults, subResult) } + } + return result +} - if failureMode == FailureModeClosed && !handled { - errs = append(errs, fmt.Errorf("old version %q compared to new version %q, property %q has unknown change, refusing to determine that change is safe", old.Name, new.Name, property)) +func ComparePropertyDiff(diff property.Diff, failureMode FailureMode, validations []property.Validation) *results.Result { + result := &results.Result{ + Subresults: []*results.Result{}, + } + handled := false + for _, validation := range validations { + ok, res := validation.Validate(diff) + if res != nil { + subResult := &results.Result{ + Subresults: []*results.Result{res}, + } + if res.Error != nil { + result.Error = errors.New("failed validations") + subResult.Error = fmt.Errorf("%q validation failed", validation.Name()) + } + result.Subresults = append(result.Subresults, subResult) + } + // if the validation handled this difference continue to the next difference + if ok { + handled = true + break } } - return errors.Join(errs...) + + if failureMode == FailureModeClosed && !handled { + result.Error = errors.New("validation failed") + result.Subresults = append(result.Subresults, &results.Result{ + Error: errors.New("unknown change(s), refusing to determine that change is safe"), + Subresults: []*results.Result{}, + }) + } + return result }