diff --git a/.bingo/Variables.mk b/.bingo/Variables.mk index 5ee29939cc..4944836589 100644 --- a/.bingo/Variables.mk +++ b/.bingo/Variables.mk @@ -35,11 +35,11 @@ $(CONTROLLER_GEN): $(BINGO_DIR)/controller-gen.mod @echo "(re)installing $(GOBIN)/controller-gen-v0.20.1" @cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=controller-gen.mod -o=$(GOBIN)/controller-gen-v0.20.1 "sigs.k8s.io/controller-tools/cmd/controller-gen" -CRD_DIFF := $(GOBIN)/crd-diff-v0.5.0 +CRD_DIFF := $(GOBIN)/crd-diff-v0.5.1-0.20260309184313-54162f2e3097 $(CRD_DIFF): $(BINGO_DIR)/crd-diff.mod @# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies. - @echo "(re)installing $(GOBIN)/crd-diff-v0.5.0" - @cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=crd-diff.mod -o=$(GOBIN)/crd-diff-v0.5.0 "sigs.k8s.io/crdify" + @echo "(re)installing $(GOBIN)/crd-diff-v0.5.1-0.20260309184313-54162f2e3097" + @cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=crd-diff.mod -o=$(GOBIN)/crd-diff-v0.5.1-0.20260309184313-54162f2e3097 "sigs.k8s.io/crdify" CRD_REF_DOCS := $(GOBIN)/crd-ref-docs-v0.3.0 $(CRD_REF_DOCS): $(BINGO_DIR)/crd-ref-docs.mod diff --git a/.bingo/crd-diff.mod b/.bingo/crd-diff.mod index 65f8c6a0f9..ada6dbc70b 100644 --- a/.bingo/crd-diff.mod +++ b/.bingo/crd-diff.mod @@ -2,4 +2,4 @@ module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT go 1.24.6 -require sigs.k8s.io/crdify v0.5.0 +require sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097 diff --git a/.bingo/crd-diff.sum b/.bingo/crd-diff.sum index acf7a5ab91..a2a9858c62 100644 --- a/.bingo/crd-diff.sum +++ b/.bingo/crd-diff.sum @@ -251,6 +251,8 @@ sigs.k8s.io/controller-runtime v0.16.2 h1:mwXAVuEk3EQf478PQwQ48zGOXvW27UJc8NHktQ sigs.k8s.io/controller-runtime v0.16.2/go.mod h1:vpMu3LpI5sYWtujJOa2uPK61nB5rbwlN7BAB8aSLvGU= sigs.k8s.io/crdify v0.5.0 h1:mrMH9CgXQPTZUpTU6Klqfnlys8bggv/7uvLT2lXSP7A= sigs.k8s.io/crdify v0.5.0/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU= +sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097 h1:gwDRFCc64lhEpxY944IJFW+CrmMFXWH+JjpE0JHp42Y= +sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0= sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4= diff --git a/.bingo/variables.env b/.bingo/variables.env index ec6e75c3fe..783b778af8 100644 --- a/.bingo/variables.env +++ b/.bingo/variables.env @@ -14,7 +14,7 @@ CONFTEST="${GOBIN}/conftest-v0.62.0" CONTROLLER_GEN="${GOBIN}/controller-gen-v0.20.1" -CRD_DIFF="${GOBIN}/crd-diff-v0.5.0" +CRD_DIFF="${GOBIN}/crd-diff-v0.5.1-0.20260309184313-54162f2e3097" CRD_REF_DOCS="${GOBIN}/crd-ref-docs-v0.3.0" diff --git a/go.mod b/go.mod index ca715c9fe9..9c987cf5e6 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,7 @@ require ( pkg.package-operator.run/boxcutter v0.12.0 sigs.k8s.io/controller-runtime v0.23.3 sigs.k8s.io/controller-tools v0.20.1 - sigs.k8s.io/crdify v0.5.0 + sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097 sigs.k8s.io/structured-merge-diff/v6 v6.3.2 sigs.k8s.io/yaml v1.6.0 ) diff --git a/go.sum b/go.sum index b60ab84ffa..e0a502ca68 100644 --- a/go.sum +++ b/go.sum @@ -804,8 +804,8 @@ sigs.k8s.io/controller-runtime v0.23.3 h1:VjB/vhoPoA9l1kEKZHBMnQF33tdCLQKJtydy4i sigs.k8s.io/controller-runtime v0.23.3/go.mod h1:B6COOxKptp+YaUT5q4l6LqUJTRpizbgf9KSRNdQGns0= sigs.k8s.io/controller-tools v0.20.1 h1:gkfMt9YodI0K85oT8rVi80NTXO/kDmabKR5Ajn5GYxs= sigs.k8s.io/controller-tools v0.20.1/go.mod h1:b4qPmjGU3iZwqn34alUU5tILhNa9+VXK+J3QV0fT/uU= -sigs.k8s.io/crdify v0.5.0 h1:mrMH9CgXQPTZUpTU6Klqfnlys8bggv/7uvLT2lXSP7A= -sigs.k8s.io/crdify v0.5.0/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU= +sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097 h1:gwDRFCc64lhEpxY944IJFW+CrmMFXWH+JjpE0JHp42Y= +sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU= sigs.k8s.io/gateway-api v1.5.0 h1:duoo14Ky/fJXpjpmyMISE2RTBGnfCg8zICfTYLTnBJA= sigs.k8s.io/gateway-api v1.5.0/go.mod h1:GvCETiaMAlLym5CovLxGjS0NysqFk3+Yuq3/rh6QL2o= sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 h1:IpInykpT6ceI+QxKBbEflcR5EXP7sU1kvOlxwZh5txg= diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go index e7830ce620..3bcd0280ae 100644 --- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go @@ -160,15 +160,15 @@ func sameVersionErrors(results *runner.Results) []error { } errs := []error{} - for version, propertyResults := range results.SameVersionValidation { - for property, comparisonResults := range propertyResults { - for _, result := range comparisonResults { + for _, versionResult := range results.SameVersionValidation { + for _, propertyResult := range versionResult.PropertyComparisons { + for _, result := range propertyResult.ComparisonResults { for _, err := range result.Errors { msg := err if result.Name == "unhandled" { msg = conciseUnhandledMessage(err) } - errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, msg)) + errs = append(errs, fmt.Errorf("%s: %s: %s: %s", versionResult.Version, propertyResult.Property, result.Name, msg)) } } } @@ -183,15 +183,15 @@ func servedVersionErrors(results *runner.Results) []error { } errs := []error{} - for version, propertyResults := range results.ServedVersionValidation { - for property, comparisonResults := range propertyResults { - for _, result := range comparisonResults { + for _, versionResult := range results.ServedVersionValidation { + for _, propertyResult := range versionResult.PropertyComparisons { + for _, result := range propertyResult.ComparisonResults { for _, err := range result.Errors { msg := err if result.Name == "unhandled" { msg = conciseUnhandledMessage(err) } - errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, msg)) + errs = append(errs, fmt.Errorf("%s: %s: %s: %s", versionResult.Version, propertyResult.Property, result.Name, msg)) } } } diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go index 9fb275880d..02f1bd7bd0 100644 --- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go @@ -191,6 +191,14 @@ func TestInstall(t *testing.T) { Manifest: getManifestString(t, "crd-description-changed.json"), }, }, + { + name: "optional field addition should not fail", + oldCrdPath: "crd-optional-field-old.json", + release: &release.Release{ + Name: "test-release", + Manifest: getManifestString(t, "crd-optional-field-new.json"), + }, + }, } for _, tc := range tests { @@ -370,6 +378,35 @@ func TestUpgrade(t *testing.T) { ) }, }, + { + name: "optional field addition should not fail", + oldCrdPath: "crd-optional-field-old.json", + release: &release.Release{ + Name: "test-release", + Manifest: getManifestString(t, "crd-optional-field-new.json"), + }, + }, + { + name: "complex breaking changes should fail", + oldCrdPath: "crd-complex-breaking-changes-old.json", + release: &release.Release{ + Name: "test-release", + Manifest: getManifestString(t, "crd-complex-breaking-changes-new.json"), + }, + // This test verifies detection of multiple breaking changes in a single CRD upgrade: + // 1. Type changed from "object" to "" - Properly detected by type validator + // 2. Nullable changed from false to true - Properly detected by nullable validator + // 3. OneOf constraint added - Reported as "unhandled" (needs crdify support) + // See: https://github.com/kubernetes-sigs/crdify/issues/25 + // The upgrade is correctly blocked, but OneOf changes need better categorization. + requireErr: wantErrorMsgs([]string{ + `validating upgrade for CRD "services.networking.example.com"`, + `type: type changed`, + `nullable: nullable added`, + `unhandled: unhandled changes found`, + `OneOf`, + }), + }, } for _, tc := range tests { diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-complex-breaking-changes-new.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-complex-breaking-changes-new.json new file mode 100644 index 0000000000..024027fce8 --- /dev/null +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-complex-breaking-changes-new.json @@ -0,0 +1,106 @@ +{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "services.networking.example.com" + }, + "spec": { + "group": "networking.example.com", + "versions": [ + { + "name": "v1beta1", + "served": true, + "storage": true, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "spec": { + "type": "object", + "properties": { + "ingress": { + "type": "object", + "properties": { + "gateway": { + "type": "object", + "properties": { + "servers": { + "type": "array", + "items": { + "type": "object", + "properties": { + "hosts": { + "description": "One or more hosts exposed by this gateway", + "type": "array", + "items": { + "type": "string" + } + }, + "port": { + "type": "object", + "properties": { + "name": { + "description": "Label assigned to the port", + "type": "string" + }, + "number": { + "description": "Port number", + "type": "integer" + } + } + }, + "tls": { + "nullable": true, + "oneOf": [ + { + "required": ["mode", "credentialName"] + }, + { + "required": ["httpsRedirect"] + } + ], + "properties": { + "credentialName": { + "description": "TLS certificate name", + "type": "string" + }, + "httpsRedirect": { + "description": "If set to true, the load balancer will send a 301 redirect to HTTPS", + "type": "boolean" + }, + "mode": { + "description": "TLS mode", + "type": "string" + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + ], + "scope": "Namespaced", + "names": { + "plural": "services", + "singular": "service", + "kind": "Service", + "shortNames": [ + "svc" + ] + } + }, + "status": { + "storedVersions": [ + "v1beta1" + ] + } +} diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-complex-breaking-changes-old.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-complex-breaking-changes-old.json new file mode 100644 index 0000000000..ba7239f945 --- /dev/null +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-complex-breaking-changes-old.json @@ -0,0 +1,95 @@ +{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "services.networking.example.com" + }, + "spec": { + "group": "networking.example.com", + "versions": [ + { + "name": "v1beta1", + "served": true, + "storage": true, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "spec": { + "type": "object", + "properties": { + "ingress": { + "type": "object", + "properties": { + "gateway": { + "type": "object", + "properties": { + "servers": { + "type": "array", + "items": { + "type": "object", + "properties": { + "hosts": { + "description": "One or more hosts exposed by this gateway", + "type": "array", + "items": { + "type": "string" + } + }, + "port": { + "type": "object", + "properties": { + "name": { + "description": "Label assigned to the port", + "type": "string" + }, + "number": { + "description": "Port number", + "type": "integer" + } + } + }, + "tls": { + "type": "object", + "nullable": false, + "properties": { + "credentialName": { + "description": "TLS certificate name", + "type": "string" + }, + "mode": { + "description": "TLS mode", + "type": "string" + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + } + ], + "scope": "Namespaced", + "names": { + "plural": "services", + "singular": "service", + "kind": "Service", + "shortNames": [ + "svc" + ] + } + }, + "status": { + "storedVersions": [ + "v1beta1" + ] + } +} diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-optional-field-new.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-optional-field-new.json new file mode 100644 index 0000000000..0e5bc42f3d --- /dev/null +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-optional-field-new.json @@ -0,0 +1,160 @@ +{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "myapps.apps.example.com" + }, + "spec": { + "group": "apps.example.com", + "versions": [ + { + "name": "v1alpha1", + "served": true, + "storage": true, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "spec": { + "type": "object", + "properties": { + "application": { + "type": "object", + "properties": { + "extraFiles": { + "type": "object", + "properties": { + "secrets": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "Key in the object", + "type": "string" + }, + "mountPath": { + "description": "Path to mount the Object. If not specified default-path/Name will be used", + "type": "string" + }, + "name": { + "description": "Name of the object\nWe support only ConfigMaps and Secrets.", + "type": "string" + } + } + } + }, + "configMaps": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "Key in the object", + "type": "string" + }, + "mountPath": { + "description": "Path to mount the Object. If not specified default-path/Name will be used", + "type": "string" + }, + "name": { + "description": "Name of the object\nWe support only ConfigMaps and Secrets.", + "type": "string" + } + } + } + } + } + } + } + } + } + } + } + } + } + }, + { + "name": "v1alpha3", + "served": true, + "storage": false, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "spec": { + "type": "object", + "properties": { + "application": { + "type": "object", + "properties": { + "extraFiles": { + "type": "object", + "properties": { + "secrets": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "Key in the object", + "type": "string" + }, + "mountPath": { + "description": "Path to mount the Object. If not specified default-path/Name will be used", + "type": "string" + }, + "name": { + "description": "Name of the object\nWe support only ConfigMaps and Secrets.", + "type": "string" + } + } + } + }, + "configMaps": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "Key in the object", + "type": "string" + }, + "mountPath": { + "description": "Path to mount the Object. If not specified default-path/Name will be used", + "type": "string" + }, + "name": { + "description": "Name of the object\nWe support only ConfigMaps and Secrets.", + "type": "string" + } + } + } + } + } + } + } + } + } + } + } + } + } + } + ], + "scope": "Namespaced", + "names": { + "plural": "myapps", + "singular": "myapp", + "kind": "MyApp", + "shortNames": [ + "bs" + ] + } + }, + "status": { + "storedVersions": [ + "v1alpha1" + ] + } +} diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-optional-field-old.json b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-optional-field-old.json new file mode 100644 index 0000000000..fc2d4582e3 --- /dev/null +++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-optional-field-old.json @@ -0,0 +1,144 @@ +{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": { + "name": "myapps.apps.example.com" + }, + "spec": { + "group": "apps.example.com", + "versions": [ + { + "name": "v1alpha1", + "served": true, + "storage": true, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "spec": { + "type": "object", + "properties": { + "application": { + "type": "object", + "properties": { + "extraFiles": { + "type": "object", + "properties": { + "secrets": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "Key in the object", + "type": "string" + }, + "name": { + "description": "Name of the object\nWe support only ConfigMaps and Secrets.", + "type": "string" + } + } + } + }, + "configMaps": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "Key in the object", + "type": "string" + }, + "name": { + "description": "Name of the object\nWe support only ConfigMaps and Secrets.", + "type": "string" + } + } + } + } + } + } + } + } + } + } + } + } + } + }, + { + "name": "v1alpha3", + "served": true, + "storage": false, + "schema": { + "openAPIV3Schema": { + "type": "object", + "properties": { + "spec": { + "type": "object", + "properties": { + "application": { + "type": "object", + "properties": { + "extraFiles": { + "type": "object", + "properties": { + "secrets": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "Key in the object", + "type": "string" + }, + "name": { + "description": "Name of the object\nWe support only ConfigMaps and Secrets.", + "type": "string" + } + } + } + }, + "configMaps": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "Key in the object", + "type": "string" + }, + "name": { + "description": "Name of the object\nWe support only ConfigMaps and Secrets.", + "type": "string" + } + } + } + } + } + } + } + } + } + } + } + } + } + } + ], + "scope": "Namespaced", + "names": { + "plural": "myapps", + "singular": "myapp", + "kind": "MyApp", + "shortNames": [ + "bs" + ] + } + }, + "status": { + "storedVersions": [ + "v1alpha1" + ] + } +}