(fix): unhandle changes for crd upgrade safety

- Keep unhandled spec changes as errors; message: "unhandled changes found"

Assisted-by: Cursor

Author: camilamacedo86

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

@@ -99,8 +99,9 @@ func (p *Preflight) runPreflight(ctx context.Context, relObjects []client.Object
 			resultErrs := crdWideErrors(results)
 			resultErrs = append(resultErrs, sameVersionErrors(results)...)
 			resultErrs = append(resultErrs, servedVersionErrors(results)...)
-
-			validateErrors = append(validateErrors, fmt.Errorf("validating upgrade for CRD %q: %w", newCrd.Name, errors.Join(resultErrs...)))
+			if len(resultErrs) > 0 {
+				validateErrors = append(validateErrors, fmt.Errorf("validating upgrade for CRD %q: %w", newCrd.Name, errors.Join(resultErrs...)))
+			}
 		}
 	}
 
@@ -162,7 +163,11 @@ func sameVersionErrors(results *runner.Results) []error {
 		for property, comparisonResults := range propertyResults {
 			for _, result := range comparisonResults {
 				for _, err := range result.Errors {
-					errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, err))
+					msg := err
+					if result.Name == "unhandled" {
+						msg = "unhandled changes found"
+					}
+					errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, msg))
 				}
 			}
 		}
@@ -181,7 +186,11 @@ func servedVersionErrors(results *runner.Results) []error {
 		for property, comparisonResults := range propertyResults {
 			for _, result := range comparisonResults {
 				for _, err := range result.Errors {
-					errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, err))
+					msg := err
+					if result.Name == "unhandled" {
+						msg = "unhandled changes found"
+					}
+					errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, msg))
 				}
 			}
 		}

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

@@ -15,6 +15,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	crdifyconfig "sigs.k8s.io/crdify/pkg/config"
 
 	"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
@@ -386,3 +387,36 @@ func TestUpgrade(t *testing.T) {
 		})
 	}
 }
+
+func TestUpgrade_UnhandledChanges_InSpec_DefaultPolicy(t *testing.T) {
+	t.Run("unhandled spec changes cause error by default", func(t *testing.T) {
+		preflight := newMockPreflight(getCrdFromManifestFile(t, "crd-unhandled-old.json"), nil)
+		rel := &release.Release{
+			Name:     "test-release",
+			Manifest: getManifestString(t, "crd-unhandled-new.json"),
+		}
+		err := preflight.Upgrade(context.Background(), rel)
+		require.Error(t, err)
+		require.ErrorContains(t, err, "unhandled changes found")
+		require.NotContains(t, err.Error(), "v1.JSONSchemaProps", "error message should be concise without raw diff")
+	})
+}
+
+func TestUpgrade_UnhandledChanges_PolicyError(t *testing.T) {
+	t.Run("unhandled changes error when policy is Error", func(t *testing.T) {
+		oldCrd := getCrdFromManifestFile(t, "crd-unhandled-old.json")
+		preflight := crdupgradesafety.NewPreflight(&MockCRDGetter{oldCrd: oldCrd}, crdupgradesafety.WithConfig(&crdifyconfig.Config{
+			Conversion:           crdifyconfig.ConversionPolicyIgnore,
+			UnhandledEnforcement: crdifyconfig.EnforcementPolicyError,
+		}))
+
+		rel := &release.Release{
+			Name:     "test-release",
+			Manifest: getManifestString(t, "crd-unhandled-new.json"),
+		}
+
+		err := preflight.Upgrade(context.Background(), rel)
+		require.Error(t, err)
+		require.ErrorContains(t, err, "unhandled changes found")
+	})
+}

internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-unhandled-new.json

@@ -0,0 +1,40 @@
+{
+  "apiVersion": "apiextensions.k8s.io/v1",
+  "kind": "CustomResourceDefinition",
+  "metadata": {
+    "name": "widgets.example.com"
+  },
+  "spec": {
+    "group": "example.com",
+    "versions": [
+      {
+        "name": "v1alpha1",
+        "served": true,
+        "storage": true,
+        "schema": {
+          "openAPIV3Schema": {
+            "type": "object",
+            "properties": {
+              "spec": {
+                "type": "object",
+                "properties": {
+                  "field": {
+                    "type": "string",
+                    "format": "email"
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    ],
+    "scope": "Namespaced",
+    "names": {
+      "plural": "widgets",
+      "singular": "widget",
+      "kind": "Widget"
+    }
+  }
+}
+

internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-unhandled-old.json

@@ -0,0 +1,39 @@
+{
+  "apiVersion": "apiextensions.k8s.io/v1",
+  "kind": "CustomResourceDefinition",
+  "metadata": {
+    "name": "widgets.example.com"
+  },
+  "spec": {
+    "group": "example.com",
+    "versions": [
+      {
+        "name": "v1alpha1",
+        "served": true,
+        "storage": true,
+        "schema": {
+          "openAPIV3Schema": {
+            "type": "object",
+            "properties": {
+              "spec": {
+                "type": "object",
+                "properties": {
+                  "field": {
+                    "type": "string"
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    ],
+    "scope": "Namespaced",
+    "names": {
+      "plural": "widgets",
+      "singular": "widget",
+      "kind": "Widget"
+    }
+  }
+}
+