OCPBUGS-63405: Backport fixes CRD upgrade-safety 4.19

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

@@ -5,6 +5,7 @@ import (
 	"cmp"
 	"fmt"
 	"reflect"
+	"slices"
 
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -38,9 +39,13 @@ func Enum(diff FieldDiff) (bool, error) {
 
 	switch {
 	case oldEnums.Len() == 0 && newEnums.Len() > 0:
-		err = fmt.Errorf("enum constraints %v added when there were no restrictions previously", newEnums.UnsortedList())
+		newEnumList := newEnums.UnsortedList()
+		slices.Sort(newEnumList)
+		err = fmt.Errorf("enum: constraints %v added when there were no restrictions previously", newEnumList)
 	case diffEnums.Len() > 0:
-		err = fmt.Errorf("enums %v removed from the set of previously allowed values", diffEnums.UnsortedList())
+		diffEnumList := diffEnums.UnsortedList()
+		slices.Sort(diffEnumList)
+		err = fmt.Errorf("enum: allowed enum values removed: %v", diffEnumList)
 	}
 
 	return isHandled(diff, reset), err
@@ -59,7 +64,9 @@ func Required(diff FieldDiff) (bool, error) {
 	var err error
 
 	if diffRequired.Len() > 0 {
-		err = fmt.Errorf("new required fields %v added", diffRequired.UnsortedList())
+		diffRequiredList := diffRequired.UnsortedList()
+		slices.Sort(diffRequiredList)
+		err = fmt.Errorf("required: new required fields %v added", diffRequiredList)
 	}
 
 	return isHandled(diff, reset), err
@@ -218,11 +225,11 @@ func Default(diff FieldDiff) (bool, error) {
 
 	switch {
 	case diff.Old.Default == nil && diff.New.Default != nil:
-		err = fmt.Errorf("default value %q added when there was no default previously", string(diff.New.Default.Raw))
+		err = fmt.Errorf("default: value %q added when there was no default previously", string(diff.New.Default.Raw))
 	case diff.Old.Default != nil && diff.New.Default == nil:
-		err = fmt.Errorf("default value %q removed", string(diff.Old.Default.Raw))
+		err = fmt.Errorf("default: value %q removed", string(diff.Old.Default.Raw))
 	case diff.Old.Default != nil && diff.New.Default != nil && !bytes.Equal(diff.Old.Default.Raw, diff.New.Default.Raw):
-		err = fmt.Errorf("default value changed from %q to %q", string(diff.Old.Default.Raw), string(diff.New.Default.Raw))
+		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
@@ -237,7 +244,7 @@ func Type(diff FieldDiff) (bool, error) {
 
 	var err error
 	if diff.Old.Type != diff.New.Type {
-		err = fmt.Errorf("type changed from %q to %q", diff.Old.Type, diff.New.Type)
+		err = fmt.Errorf("type: type changed : %q -> %q", diff.Old.Type, diff.New.Type)
 	}
 
 	return isHandled(diff, reset), err

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

@@ -53,7 +53,7 @@ func TestEnum(t *testing.T) {
 					},
 				},
 			},
-			err:     errors.New("enum constraints [foo] added when there were no restrictions previously"),
+			err:     errors.New("enum: constraints [foo] added when there were no restrictions previously"),
 			handled: true,
 		},
 		{
@@ -77,7 +77,7 @@ func TestEnum(t *testing.T) {
 					},
 				},
 			},
-			err:     errors.New("enums [foo] removed from the set of previously allowed values"),
+			err:     errors.New("enum: allowed enum values removed: [foo]"),
 			handled: true,
 		},
 		{
@@ -154,7 +154,7 @@ func TestRequired(t *testing.T) {
 					},
 				},
 			},
-			err:     errors.New("new required fields [foo] added"),
+			err:     errors.New("required: new required fields [foo] added"),
 			handled: true,
 		},
 		{
@@ -800,7 +800,7 @@ func TestDefault(t *testing.T) {
 					},
 				},
 			},
-			err:     errors.New("default value \"foo\" added when there was no default previously"),
+			err:     errors.New("default: value \"foo\" added when there was no default previously"),
 			handled: true,
 		},
 		{
@@ -813,7 +813,7 @@ func TestDefault(t *testing.T) {
 				},
 				New: &apiextensionsv1.JSONSchemaProps{},
 			},
-			err:     errors.New("default value \"foo\" removed"),
+			err:     errors.New("default: value \"foo\" removed"),
 			handled: true,
 		},
 		{
@@ -830,7 +830,7 @@ func TestDefault(t *testing.T) {
 					},
 				},
 			},
-			err:     errors.New("default value changed from \"foo\" to \"bar\""),
+			err:     errors.New("default: value changed from \"foo\" to \"bar\""),
 			handled: true,
 		},
 		{
@@ -880,7 +880,7 @@ func TestType(t *testing.T) {
 					Type: "integer",
 				},
 			},
-			err:     errors.New("type changed from \"string\" to \"integer\""),
+			err:     errors.New("type: type changed : \"string\" -> \"integer\""),
 			handled: true,
 		},
 		{

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

@@ -47,7 +47,6 @@ func NewPreflight(crdCli apiextensionsv1client.CustomResourceDefinitionInterface
 	}
 	p := &Preflight{
 		crdClient: crdCli,
-		// create a default validator. Can be overridden via the options
 		validator: &Validator{
 			Validations: []Validation{
 				NewValidationFunc("NoScopeChange", NoScopeChange),
@@ -95,26 +94,126 @@ func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release) erro
 		if err != nil {
 			return fmt.Errorf("converting object %q to unstructured: %w", obj.GetName(), err)
 		}
-		err = runtime.DefaultUnstructuredConverter.FromUnstructured(uMap, newCrd)
-		if err != nil {
+		if err = runtime.DefaultUnstructuredConverter.FromUnstructured(uMap, newCrd); err != nil {
 			return fmt.Errorf("converting unstructured to CRD object: %w", err)
 		}
 
 		oldCrd, err := p.crdClient.Get(ctx, newCrd.Name, metav1.GetOptions{})
 		if err != nil {
-			// if there is no existing CRD, there is nothing to break
-			// so it is immediately successful.
 			if apierrors.IsNotFound(err) {
 				continue
 			}
 			return fmt.Errorf("getting existing resource for CRD %q: %w", newCrd.Name, err)
 		}
 
-		err = p.validator.Validate(*oldCrd, *newCrd)
-		if err != nil {
+		if err = p.validator.Validate(*oldCrd, *newCrd); err != nil {
 			validateErrors = append(validateErrors, fmt.Errorf("validating upgrade for CRD %q failed: %w", newCrd.Name, err))
 		}
 	}
 
 	return errors.Join(validateErrors...)
 }
+
+const unhandledSummaryPrefix = "unhandled changes found"
+
+func conciseUnhandledMessage(raw string) string {
+	if !strings.Contains(raw, unhandledSummaryPrefix) {
+		return raw
+	}
+
+	details := extractUnhandledDetails(raw)
+	if len(details) == 0 {
+		return unhandledSummaryPrefix
+	}
+
+	return fmt.Sprintf("%s (%s)", unhandledSummaryPrefix, strings.Join(details, "; "))
+}
+
+func extractUnhandledDetails(raw string) []string {
+	type diffEntry struct {
+		before    string
+		after     string
+		beforeRaw string
+		afterRaw  string
+	}
+
+	entries := map[string]*diffEntry{}
+	order := []string{}
+
+	for _, line := range strings.Split(raw, "\n") {
+		trimmed := strings.TrimSpace(line)
+		if len(trimmed) < 2 {
+			continue
+		}
+
+		sign := trimmed[0]
+		if sign != '-' && sign != '+' {
+			continue
+		}
+
+		field, value, rawValue := parseUnhandledDiffValue(trimmed[1:])
+		if field == "" {
+			continue
+		}
+
+		entry, ok := entries[field]
+		if !ok {
+			entry = &diffEntry{}
+			entries[field] = entry
+			order = append(order, field)
+		}
+
+		if sign == '-' {
+			entry.before = value
+			entry.beforeRaw = rawValue
+		} else {
+			entry.after = value
+			entry.afterRaw = rawValue
+		}
+	}
+
+	details := []string{}
+	for _, field := range order {
+		entry := entries[field]
+		if entry.before == "" && entry.after == "" {
+			continue
+		}
+		if entry.before == entry.after && entry.beforeRaw == entry.afterRaw {
+			continue
+		}
+
+		before := entry.before
+		if before == "" {
+			before = "<empty>"
+		}
+		after := entry.after
+		if after == "" {
+			after = "<empty>"
+		}
+		if entry.before == entry.after && entry.beforeRaw != entry.afterRaw {
+			after = after + " (changed)"
+		}
+
+		details = append(details, fmt.Sprintf("%s %s -> %s", field, before, after))
+	}
+
+	return details
+}
+
+func parseUnhandledDiffValue(fragment string) (string, string, string) {
+	cleaned := strings.TrimSpace(fragment)
+	cleaned = strings.TrimPrefix(cleaned, "\t")
+	cleaned = strings.TrimSpace(cleaned)
+	cleaned = strings.TrimSuffix(cleaned, ",")
+
+	parts := strings.SplitN(cleaned, ":", 2)
+	if len(parts) != 2 {
+		return "", "", ""
+	}
+
+	field := strings.TrimSpace(parts[0])
+	value := strings.TrimSpace(parts[1])
+
+	stripped := strings.Trim(value, `"`)
+	return field, stripped, value
+}