Ensure fixed order in multi-line error returned by crdupgradesafety change validator

Signed-off-by: Artur Zych <[email protected]>

Author: azych

pkg/kapp/crdupgradesafety/change_validator.go

@@ -7,7 +7,9 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"maps"
 	"reflect"
+	"slices"
 
 	"github.com/openshift/crd-schema-checker/pkg/manifestcomparators"
 	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -449,7 +451,10 @@ func (cv *ChangeValidator) Validate(old, new v1.CustomResourceDefinition) error
 			continue
 		}
 
-		for field, diff := range diffs {
+		// ensure order of the potentially multi-line final error
+		for _, field := range slices.Sorted(maps.Keys(diffs)) {
+			diff := diffs[field]
+
 			handled := false
 			for _, validation := range cv.Validations {
 				ok, err := validation(diff)

pkg/kapp/crdupgradesafety/change_validator_test.go

@@ -5,6 +5,7 @@ package crdupgradesafety_test
 
 import (
 	"errors"
+	"fmt"
 	"testing"
 
 	"carvel.dev/kapp/pkg/kapp/crdupgradesafety"
@@ -235,12 +236,15 @@ func TestFlattenSchema(t *testing.T) {
 }
 
 func TestChangeValidator(t *testing.T) {
+	validationErr1 := errors.New(`version "v1alpha1", field "^" has unknown change, refusing to determine that change is safe`)
+	validationErr2 := errors.New(`version "v1alpha1", field "^": fail`)
+
 	for _, tc := range []struct {
 		name            string
 		changeValidator *crdupgradesafety.ChangeValidator
 		old             v1.CustomResourceDefinition
 		new             v1.CustomResourceDefinition
-		shouldError     bool
+		expectedError   error
 	}{
 		{
 			name: "no changes, no error",
@@ -347,7 +351,7 @@ func TestChangeValidator(t *testing.T) {
 					},
 				},
 			},
-			shouldError: true,
+			expectedError: validationErr1,
 		},
 		{
 			name: "changes, validation failed, change fully handled, error",
@@ -384,15 +388,18 @@ func TestChangeValidator(t *testing.T) {
 					},
 				},
 			},
-			shouldError: true,
+			expectedError: validationErr2,
 		},
 		{
-			name: "changes, validation failed, change not fully handled, error",
+			name: "changes, validation failed, change not fully handled, ordered error",
 			changeValidator: &crdupgradesafety.ChangeValidator{
 				Validations: []crdupgradesafety.ChangeValidation{
 					func(_ crdupgradesafety.FieldDiff) (bool, error) {
 						return false, errors.New("fail")
 					},
+					func(_ crdupgradesafety.FieldDiff) (bool, error) {
+						return false, errors.New("error")
+					},
 				},
 			},
 			old: v1.CustomResourceDefinition{
@@ -421,12 +428,16 @@ func TestChangeValidator(t *testing.T) {
 					},
 				},
 			},
-			shouldError: true,
+			expectedError: fmt.Errorf("%w\n%s\n%w", validationErr2, `version "v1alpha1", field "^": error`, validationErr1),
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			err := tc.changeValidator.Validate(tc.old, tc.new)
-			assert.Equal(t, tc.shouldError, err != nil, "should error? - %v", tc.shouldError)
+			if tc.expectedError != nil {
+				assert.EqualError(t, err, tc.expectedError.Error())
+			} else {
+				assert.NoError(t, err)
+			}
 		})
 	}
 }