chore: remove redundant orderKappsValidateErr

Author: azych

internal/operator-controller/rukpak/preflights/crdupgradesafety/checks_test.go

@@ -907,84 +907,6 @@ func TestType(t *testing.T) {
 	}
 }
 
-func TestOrderKappsValidateErr(t *testing.T) {
-	testErr1 := errors.New("fallback1")
-	testErr2 := errors.New("fallback2")
-
-	generateErrors := func(n int, base string) []error {
-		var result []error
-		for i := n; i >= 0; i-- {
-			result = append(result, fmt.Errorf("%s%d", base, i))
-		}
-		return result
-	}
-
-	joinedAndNested := func(format string, errs ...error) error {
-		return fmt.Errorf(format, errors.Join(errs...))
-	}
-
-	testCases := []struct {
-		name          string
-		inpuError     error
-		expectedError error
-	}{
-		{
-			name:          "fallback: initial error was not error.Join'ed",
-			inpuError:     testErr1,
-			expectedError: testErr1,
-		},
-		{
-			name:          "fallback: nested error was not wrapped",
-			inpuError:     errors.Join(testErr1),
-			expectedError: testErr1,
-		},
-		{
-			name:          "fallback: multiple nested errors, one was not wrapped",
-			inpuError:     errors.Join(testErr2, fmt.Errorf("%w", testErr1)),
-			expectedError: errors.Join(testErr2, fmt.Errorf("%w", testErr1)),
-		},
-		{
-			name:          "fallback: nested error did not contain \":\"",
-			inpuError:     errors.Join(fmt.Errorf("%w", testErr1)),
-			expectedError: testErr1,
-		},
-		{
-			name:          "fallback: multiple nested errors, one did not contain \":\"",
-			inpuError:     errors.Join(joinedAndNested("fail: %w", testErr2), joinedAndNested("%w", testErr1)),
-			expectedError: errors.Join(fmt.Errorf("fail: %w", testErr2), testErr1),
-		},
-		{
-			name:          "fallback: nested error was not error.Join'ed",
-			inpuError:     errors.Join(fmt.Errorf("fail: %w", testErr1)),
-			expectedError: fmt.Errorf("fail: %w", testErr1),
-		},
-		{
-			name:          "fallback: multiple nested errors, one was not error.Join'ed",
-			inpuError:     errors.Join(joinedAndNested("fail: %w", testErr2), fmt.Errorf("fail: %w", testErr1)),
-			expectedError: fmt.Errorf("fail: %w\nfail: %w", testErr2, testErr1),
-		},
-		{
-			name:          "ensures order for a single group of multiple deeply nested errors",
-			inpuError:     errors.Join(joinedAndNested("fail: %w", testErr2, testErr1)),
-			expectedError: fmt.Errorf("fail: %w\n%w", testErr1, testErr2),
-		},
-		{
-			name: "ensures order for multiple groups of deeply nested errors",
-			inpuError: errors.Join(
-				joinedAndNested("fail: %w", testErr2, testErr1),
-				joinedAndNested("validation: %w", generateErrors(5, "err")...),
-			),
-			expectedError: fmt.Errorf("fail: %w\n%w\nvalidation: err0\nerr1\nerr2\nerr3\nerr4\nerr5", testErr1, testErr2),
-		},
-	}
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			err := orderKappsValidateErr(tc.inpuError)
-			require.EqualError(t, err, tc.expectedError.Error())
-		})
-	}
-}
-
 func TestServedVersionValidator(t *testing.T) {
 	validationErr1 := errors.New(`version "v1alpha1", field "^" has unknown change, refusing to determine that change is safe`)
 	validationErr2 := errors.New(`version upgrade "v1alpha1" to "v1alpha2", field "^": fail`)

internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go

@@ -1,11 +1,9 @@
 package crdupgradesafety
 
 import (
-	"cmp"
 	"context"
 	"errors"
 	"fmt"
-	"slices"
 	"strings"
 
 	"helm.sh/helm/v3/pkg/release"
@@ -119,64 +117,3 @@ func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release) erro
 
 	return errors.Join(validateErrors...)
 }
-
-// orderKappsValidateErr is meant as a temporary solution to the problem
-// of randomly ordered multi-line validation error returned by kapp's validator.Validate()
-//
-// The problem is that kapp's field validations are performed in map iteration order, which is not fixed.
-// Errors from those validations are then error.Join'ed, fmt.Errorf'ed and error.Join'ed again,
-// which means original messages are available at 3rd level of nesting, and this is where we need to
-// sort them to ensure we do not enter into constant reconciliation loop because of random order of
-// failure message we ultimately set in ClusterExtension's status conditions.
-//
-// This helper attempts to do that and falls back to original unchanged error message
-// in case of any unforeseen issues which likely mean that the internals of validator.Validate
-// have changed.
-//
-// For full context see:
-// github.com/operator-framework/operator-controller/issues/1456 (original issue and comments)
-// github.com/carvel-dev/kapp/pull/1047 (PR to ensure order in upstream)
-//
-// TODO: remove this once ordering has been handled by the upstream.
-func orderKappsValidateErr(err error) error {
-	joinedValidationErrs, ok := err.(interface{ Unwrap() []error })
-	if !ok {
-		return err
-	}
-
-	// nolint: prealloc
-	var errs []error
-	for _, validationErr := range joinedValidationErrs.Unwrap() {
-		unwrappedValidationErr := errors.Unwrap(validationErr)
-		// validator.Validate did not error.Join'ed validation errors
-		// kapp's internals changed - fallback to original error
-		if unwrappedValidationErr == nil {
-			return err
-		}
-
-		prefix, _, ok := strings.Cut(validationErr.Error(), ":")
-		// kapp's internal error format changed - fallback to original error
-		if !ok {
-			return err
-		}
-
-		// attempt to unwrap and sort field errors
-		joinedFieldErrs, ok := unwrappedValidationErr.(interface{ Unwrap() []error })
-		// ChangeValidator did not error.Join'ed field validation errors
-		// kapp's internals changed - fallback to original error
-		if !ok {
-			return err
-		}
-
-		// ensure order of the field validation errors
-		unwrappedFieldErrs := joinedFieldErrs.Unwrap()
-		slices.SortFunc(unwrappedFieldErrs, func(a, b error) int {
-			return cmp.Compare(a.Error(), b.Error())
-		})
-
-		// re-join the sorted field errors keeping the original error prefix from kapp
-		errs = append(errs, fmt.Errorf("%s: %w", prefix, errors.Join(unwrappedFieldErrs...)))
-	}
-
-	return errors.Join(errs...)
-}