Skip to content

Commit ba1e4cb

Browse files
camilamacedo86camilamacedo86
authored
🌱 OCPBUGS-60693, OCPBUGS-60958: Upgrade sigs.k8s.io/crdify v0.5.0 => v0.5.1-0.20260309184313-54162f2e3097 and add tests to ensure bug fix scenarios (#2612)
* Upgrade sigs.k8s.io/crdify v0.5.0 => v0.5.1-0.20260309184313-54162f2e3097 * Add tests to ensure scenarios with preflight * Upgrade bingo version used for crddiff. From v0.5.0 => v0.5.1-0.20260309184313-54162f2e3097
1 parent 1586800 commit ba1e4cb

12 files changed

Lines changed: 560 additions & 16 deletions

File tree

‎.bingo/Variables.mk‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ $(CONTROLLER_GEN): $(BINGO_DIR)/controller-gen.mod
3535
@echo "(re)installing $(GOBIN)/controller-gen-v0.20.1"
3636
@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"
3737

38-
CRD_DIFF := $(GOBIN)/crd-diff-v0.5.0
38+
CRD_DIFF := $(GOBIN)/crd-diff-v0.5.1-0.20260309184313-54162f2e3097
3939
$(CRD_DIFF): $(BINGO_DIR)/crd-diff.mod
4040
@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
41-
@echo "(re)installing $(GOBIN)/crd-diff-v0.5.0"
42-
@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=crd-diff.mod -o=$(GOBIN)/crd-diff-v0.5.0 "sigs.k8s.io/crdify"
41+
@echo "(re)installing $(GOBIN)/crd-diff-v0.5.1-0.20260309184313-54162f2e3097"
42+
@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"
4343

4444
CRD_REF_DOCS := $(GOBIN)/crd-ref-docs-v0.3.0
4545
$(CRD_REF_DOCS): $(BINGO_DIR)/crd-ref-docs.mod

‎.bingo/crd-diff.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT
22

33
go 1.24.6
44

5-
require sigs.k8s.io/crdify v0.5.0
5+
require sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097

‎.bingo/crd-diff.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,8 @@ sigs.k8s.io/controller-runtime v0.16.2 h1:mwXAVuEk3EQf478PQwQ48zGOXvW27UJc8NHktQ
251251
sigs.k8s.io/controller-runtime v0.16.2/go.mod h1:vpMu3LpI5sYWtujJOa2uPK61nB5rbwlN7BAB8aSLvGU=
252252
sigs.k8s.io/crdify v0.5.0 h1:mrMH9CgXQPTZUpTU6Klqfnlys8bggv/7uvLT2lXSP7A=
253253
sigs.k8s.io/crdify v0.5.0/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU=
254+
sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097 h1:gwDRFCc64lhEpxY944IJFW+CrmMFXWH+JjpE0JHp42Y=
255+
sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU=
254256
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
255257
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
256258
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=

‎.bingo/variables.env‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ CONFTEST="${GOBIN}/conftest-v0.62.0"
1414

1515
CONTROLLER_GEN="${GOBIN}/controller-gen-v0.20.1"
1616

17-
CRD_DIFF="${GOBIN}/crd-diff-v0.5.0"
17+
CRD_DIFF="${GOBIN}/crd-diff-v0.5.1-0.20260309184313-54162f2e3097"
1818

1919
CRD_REF_DOCS="${GOBIN}/crd-ref-docs-v0.3.0"
2020

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ require (
4848
pkg.package-operator.run/boxcutter v0.12.0
4949
sigs.k8s.io/controller-runtime v0.23.3
5050
sigs.k8s.io/controller-tools v0.20.1
51-
sigs.k8s.io/crdify v0.5.0
51+
sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097
5252
sigs.k8s.io/structured-merge-diff/v6 v6.3.2
5353
sigs.k8s.io/yaml v1.6.0
5454
)

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -804,8 +804,8 @@ sigs.k8s.io/controller-runtime v0.23.3 h1:VjB/vhoPoA9l1kEKZHBMnQF33tdCLQKJtydy4i
804804
sigs.k8s.io/controller-runtime v0.23.3/go.mod h1:B6COOxKptp+YaUT5q4l6LqUJTRpizbgf9KSRNdQGns0=
805805
sigs.k8s.io/controller-tools v0.20.1 h1:gkfMt9YodI0K85oT8rVi80NTXO/kDmabKR5Ajn5GYxs=
806806
sigs.k8s.io/controller-tools v0.20.1/go.mod h1:b4qPmjGU3iZwqn34alUU5tILhNa9+VXK+J3QV0fT/uU=
807-
sigs.k8s.io/crdify v0.5.0 h1:mrMH9CgXQPTZUpTU6Klqfnlys8bggv/7uvLT2lXSP7A=
808-
sigs.k8s.io/crdify v0.5.0/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU=
807+
sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097 h1:gwDRFCc64lhEpxY944IJFW+CrmMFXWH+JjpE0JHp42Y=
808+
sigs.k8s.io/crdify v0.5.1-0.20260309184313-54162f2e3097/go.mod h1:ZIFxaYNgKYmFtZCLPysncXQ8oqwnNlHQbRUfxJHZwzU=
809809
sigs.k8s.io/gateway-api v1.5.0 h1:duoo14Ky/fJXpjpmyMISE2RTBGnfCg8zICfTYLTnBJA=
810810
sigs.k8s.io/gateway-api v1.5.0/go.mod h1:GvCETiaMAlLym5CovLxGjS0NysqFk3+Yuq3/rh6QL2o=
811811
sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 h1:IpInykpT6ceI+QxKBbEflcR5EXP7sU1kvOlxwZh5txg=

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,15 +160,15 @@ func sameVersionErrors(results *runner.Results) []error {
160160
}
161161

162162
errs := []error{}
163-
for version, propertyResults := range results.SameVersionValidation {
164-
for property, comparisonResults := range propertyResults {
165-
for _, result := range comparisonResults {
163+
for _, versionResult := range results.SameVersionValidation {
164+
for _, propertyResult := range versionResult.PropertyComparisons {
165+
for _, result := range propertyResult.ComparisonResults {
166166
for _, err := range result.Errors {
167167
msg := err
168168
if result.Name == "unhandled" {
169169
msg = conciseUnhandledMessage(err)
170170
}
171-
errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, msg))
171+
errs = append(errs, fmt.Errorf("%s: %s: %s: %s", versionResult.Version, propertyResult.Property, result.Name, msg))
172172
}
173173
}
174174
}
@@ -183,15 +183,15 @@ func servedVersionErrors(results *runner.Results) []error {
183183
}
184184

185185
errs := []error{}
186-
for version, propertyResults := range results.ServedVersionValidation {
187-
for property, comparisonResults := range propertyResults {
188-
for _, result := range comparisonResults {
186+
for _, versionResult := range results.ServedVersionValidation {
187+
for _, propertyResult := range versionResult.PropertyComparisons {
188+
for _, result := range propertyResult.ComparisonResults {
189189
for _, err := range result.Errors {
190190
msg := err
191191
if result.Name == "unhandled" {
192192
msg = conciseUnhandledMessage(err)
193193
}
194-
errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, msg))
194+
errs = append(errs, fmt.Errorf("%s: %s: %s: %s", versionResult.Version, propertyResult.Property, result.Name, msg))
195195
}
196196
}
197197
}

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,14 @@ func TestInstall(t *testing.T) {
191191
Manifest: getManifestString(t, "crd-description-changed.json"),
192192
},
193193
},
194+
{
195+
name: "optional field addition should not fail",
196+
oldCrdPath: "crd-optional-field-old.json",
197+
release: &release.Release{
198+
Name: "test-release",
199+
Manifest: getManifestString(t, "crd-optional-field-new.json"),
200+
},
201+
},
194202
}
195203

196204
for _, tc := range tests {
@@ -370,6 +378,35 @@ func TestUpgrade(t *testing.T) {
370378
)
371379
},
372380
},
381+
{
382+
name: "optional field addition should not fail",
383+
oldCrdPath: "crd-optional-field-old.json",
384+
release: &release.Release{
385+
Name: "test-release",
386+
Manifest: getManifestString(t, "crd-optional-field-new.json"),
387+
},
388+
},
389+
{
390+
name: "complex breaking changes should fail",
391+
oldCrdPath: "crd-complex-breaking-changes-old.json",
392+
release: &release.Release{
393+
Name: "test-release",
394+
Manifest: getManifestString(t, "crd-complex-breaking-changes-new.json"),
395+
},
396+
// This test verifies detection of multiple breaking changes in a single CRD upgrade:
397+
// 1. Type changed from "object" to "" - Properly detected by type validator
398+
// 2. Nullable changed from false to true - Properly detected by nullable validator
399+
// 3. OneOf constraint added - Reported as "unhandled" (needs crdify support)
400+
// See: https://github.com/kubernetes-sigs/crdify/issues/25
401+
// The upgrade is correctly blocked, but OneOf changes need better categorization.
402+
requireErr: wantErrorMsgs([]string{
403+
`validating upgrade for CRD "services.networking.example.com"`,
404+
`type: type changed`,
405+
`nullable: nullable added`,
406+
`unhandled: unhandled changes found`,
407+
`OneOf`,
408+
}),
409+
},
373410
}
374411

375412
for _, tc := range tests {

‎internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-complex-breaking-changes-new.json‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
{
2+
"apiVersion": "apiextensions.k8s.io/v1",
3+
"kind": "CustomResourceDefinition",
4+
"metadata": {
5+
"name": "services.networking.example.com"
6+
},
7+
"spec": {
8+
"group": "networking.example.com",
9+
"versions": [
10+
{
11+
"name": "v1beta1",
12+
"served": true,
13+
"storage": true,
14+
"schema": {
15+
"openAPIV3Schema": {
16+
"type": "object",
17+
"properties": {
18+
"spec": {
19+
"type": "object",
20+
"properties": {
21+
"ingress": {
22+
"type": "object",
23+
"properties": {
24+
"gateway": {
25+
"type": "object",
26+
"properties": {
27+
"servers": {
28+
"type": "array",
29+
"items": {
30+
"type": "object",
31+
"properties": {
32+
"hosts": {
33+
"description": "One or more hosts exposed by this gateway",
34+
"type": "array",
35+
"items": {
36+
"type": "string"
37+
}
38+
},
39+
"port": {
40+
"type": "object",
41+
"properties": {
42+
"name": {
43+
"description": "Label assigned to the port",
44+
"type": "string"
45+
},
46+
"number": {
47+
"description": "Port number",
48+
"type": "integer"
49+
}
50+
}
51+
},
52+
"tls": {
53+
"nullable": true,
54+
"oneOf": [
55+
{
56+
"required": ["mode", "credentialName"]
57+
},
58+
{
59+
"required": ["httpsRedirect"]
60+
}
61+
],
62+
"properties": {
63+
"credentialName": {
64+
"description": "TLS certificate name",
65+
"type": "string"
66+
},
67+
"httpsRedirect": {
68+
"description": "If set to true, the load balancer will send a 301 redirect to HTTPS",
69+
"type": "boolean"
70+
},
71+
"mode": {
72+
"description": "TLS mode",
73+
"type": "string"
74+
}
75+
}
76+
}
77+
}
78+
}
79+
}
80+
}
81+
}
82+
}
83+
}
84+
}
85+
}
86+
}
87+
}
88+
}
89+
}
90+
],
91+
"scope": "Namespaced",
92+
"names": {
93+
"plural": "services",
94+
"singular": "service",
95+
"kind": "Service",
96+
"shortNames": [
97+
"svc"
98+
]
99+
}
100+
},
101+
"status": {
102+
"storedVersions": [
103+
"v1beta1"
104+
]
105+
}
106+
}

‎internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-complex-breaking-changes-old.json‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
{
2+
"apiVersion": "apiextensions.k8s.io/v1",
3+
"kind": "CustomResourceDefinition",
4+
"metadata": {
5+
"name": "services.networking.example.com"
6+
},
7+
"spec": {
8+
"group": "networking.example.com",
9+
"versions": [
10+
{
11+
"name": "v1beta1",
12+
"served": true,
13+
"storage": true,
14+
"schema": {
15+
"openAPIV3Schema": {
16+
"type": "object",
17+
"properties": {
18+
"spec": {
19+
"type": "object",
20+
"properties": {
21+
"ingress": {
22+
"type": "object",
23+
"properties": {
24+
"gateway": {
25+
"type": "object",
26+
"properties": {
27+
"servers": {
28+
"type": "array",
29+
"items": {
30+
"type": "object",
31+
"properties": {
32+
"hosts": {
33+
"description": "One or more hosts exposed by this gateway",
34+
"type": "array",
35+
"items": {
36+
"type": "string"
37+
}
38+
},
39+
"port": {
40+
"type": "object",
41+
"properties": {
42+
"name": {
43+
"description": "Label assigned to the port",
44+
"type": "string"
45+
},
46+
"number": {
47+
"description": "Port number",
48+
"type": "integer"
49+
}
50+
}
51+
},
52+
"tls": {
53+
"type": "object",
54+
"nullable": false,
55+
"properties": {
56+
"credentialName": {
57+
"description": "TLS certificate name",
58+
"type": "string"
59+
},
60+
"mode": {
61+
"description": "TLS mode",
62+
"type": "string"
63+
}
64+
}
65+
}
66+
}
67+
}
68+
}
69+
}
70+
}
71+
}
72+
}
73+
}
74+
}
75+
}
76+
}
77+
}
78+
}
79+
],
80+
"scope": "Namespaced",
81+
"names": {
82+
"plural": "services",
83+
"singular": "service",
84+
"kind": "Service",
85+
"shortNames": [
86+
"svc"
87+
]
88+
}
89+
},
90+
"status": {
91+
"storedVersions": [
92+
"v1beta1"
93+
]
94+
}
95+
}

0 commit comments

Comments
 (0)