Skip to content

Commit 79f50b3

Browse files
joelanfordjoelanford
committed
add broken test
1 parent c676250 commit 79f50b3

File tree

2 files changed

+135
-26
lines changed

2 files changed

+135
-26
lines changed

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

Lines changed: 59 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,22 @@ func getManifestString(t *testing.T, crdFile string) string {
6666
return string(buff)
6767
}
6868

69+
func wantErrorMsgs(wantMsgs []string) require.ErrorAssertionFunc {
70+
return func(t require.TestingT, haveErr error, _ ...interface{}) {
71+
for _, wantMsg := range wantMsgs {
72+
require.ErrorContains(t, haveErr, wantMsg)
73+
}
74+
}
75+
}
76+
6977
// TestInstall exists only for completeness as Install() is currently a no-op. It can be used as
7078
// a template for real tests in the future if the func is implemented.
7179
func TestInstall(t *testing.T) {
7280
tests := []struct {
7381
name string
7482
oldCrdPath string
7583
release *release.Release
76-
wantErrMsgs []string
84+
requireErr require.ErrorAssertionFunc
7785
wantCrdGetErr error
7886
}{
7987
{
@@ -91,7 +99,7 @@ func TestInstall(t *testing.T) {
9199
Name: "test-release",
92100
Manifest: "abcd",
93101
},
94-
wantErrMsgs: []string{"json: cannot unmarshal string into Go value of type unstructured.detector"},
102+
requireErr: wantErrorMsgs([]string{"json: cannot unmarshal string into Go value of type unstructured.detector"}),
95103
},
96104
{
97105
name: "release with no CRD objects",
@@ -107,7 +115,7 @@ func TestInstall(t *testing.T) {
107115
Manifest: getManifestString(t, "crd-valid-upgrade.json"),
108116
},
109117
wantCrdGetErr: fmt.Errorf("error!"),
110-
wantErrMsgs: []string{"error!"},
118+
requireErr: wantErrorMsgs( []string{"error!"}),
111119
},
112120
{
113121
name: "fail to get old crd, not found error",
@@ -123,7 +131,7 @@ func TestInstall(t *testing.T) {
123131
Name: "test-release",
124132
Manifest: getManifestString(t, "crd-invalid"),
125133
},
126-
wantErrMsgs: []string{"json: cannot unmarshal"},
134+
requireErr: wantErrorMsgs([]string{"json: cannot unmarshal"}),
127135
},
128136
{
129137
name: "valid upgrade",
@@ -142,7 +150,7 @@ func TestInstall(t *testing.T) {
142150
Name: "test-release",
143151
Manifest: getManifestString(t, "crd-invalid-upgrade.json"),
144152
},
145-
wantErrMsgs: []string{
153+
requireErr: wantErrorMsgs([]string{
146154
`scope:`,
147155
`storedVersionRemoval:`,
148156
`enum:`,
@@ -156,7 +164,7 @@ func TestInstall(t *testing.T) {
156164
`minLength:`,
157165
`minProperties:`,
158166
`default:`,
159-
},
167+
}),
160168
},
161169
{
162170
name: "new crd validation failure for existing field removal",
@@ -167,9 +175,9 @@ func TestInstall(t *testing.T) {
167175
Name: "test-release",
168176
Manifest: getManifestString(t, "crd-field-removed.json"),
169177
},
170-
wantErrMsgs: []string{
178+
requireErr: wantErrorMsgs([]string{
171179
`existingFieldRemoval:`,
172-
},
180+
}),
173181
},
174182
{
175183
name: "new crd validation should not fail on description changes",
@@ -187,10 +195,8 @@ func TestInstall(t *testing.T) {
187195
t.Run(tc.name, func(t *testing.T) {
188196
preflight := newMockPreflight(getCrdFromManifestFile(t, tc.oldCrdPath), tc.wantCrdGetErr)
189197
err := preflight.Install(context.Background(), tc.release)
190-
if len(tc.wantErrMsgs) != 0 {
191-
for _, expectedErrMsg := range tc.wantErrMsgs {
192-
require.ErrorContains(t, err, expectedErrMsg)
193-
}
198+
if tc.requireErr != nil {
199+
tc.requireErr(t, err)
194200
} else {
195201
require.NoError(t, err)
196202
}
@@ -203,7 +209,7 @@ func TestUpgrade(t *testing.T) {
203209
name string
204210
oldCrdPath string
205211
release *release.Release
206-
wantErrMsgs []string
212+
requireErr require.ErrorAssertionFunc
207213
wantCrdGetErr error
208214
}{
209215
{
@@ -221,7 +227,7 @@ func TestUpgrade(t *testing.T) {
221227
Name: "test-release",
222228
Manifest: "abcd",
223229
},
224-
wantErrMsgs: []string{"json: cannot unmarshal string into Go value of type unstructured.detector"},
230+
requireErr: wantErrorMsgs([]string{"json: cannot unmarshal string into Go value of type unstructured.detector"}),
225231
},
226232
{
227233
name: "release with no CRD objects",
@@ -237,7 +243,7 @@ func TestUpgrade(t *testing.T) {
237243
Manifest: getManifestString(t, "crd-valid-upgrade.json"),
238244
},
239245
wantCrdGetErr: fmt.Errorf("error!"),
240-
wantErrMsgs: []string{"error!"},
246+
requireErr: wantErrorMsgs( []string{"error!"}),
241247
},
242248
{
243249
name: "fail to get old crd, not found error",
@@ -253,7 +259,7 @@ func TestUpgrade(t *testing.T) {
253259
Name: "test-release",
254260
Manifest: getManifestString(t, "crd-invalid"),
255261
},
256-
wantErrMsgs: []string{"json: cannot unmarshal"},
262+
requireErr: wantErrorMsgs([]string{"json: cannot unmarshal"}),
257263
},
258264
{
259265
name: "valid upgrade",
@@ -272,7 +278,7 @@ func TestUpgrade(t *testing.T) {
272278
Name: "test-release",
273279
Manifest: getManifestString(t, "crd-invalid-upgrade.json"),
274280
},
275-
wantErrMsgs: []string{
281+
requireErr: wantErrorMsgs([]string{
276282
`scope:`,
277283
`storedVersionRemoval:`,
278284
`enum:`,
@@ -286,7 +292,7 @@ func TestUpgrade(t *testing.T) {
286292
`minLength:`,
287293
`minProperties:`,
288294
`default:`,
289-
},
295+
}),
290296
},
291297
{
292298
name: "new crd validation failure for existing field removal",
@@ -297,9 +303,9 @@ func TestUpgrade(t *testing.T) {
297303
Name: "test-release",
298304
Manifest: getManifestString(t, "crd-field-removed.json"),
299305
},
300-
wantErrMsgs: []string{
306+
requireErr: wantErrorMsgs([]string{
301307
`existingFieldRemoval:`,
302-
},
308+
}),
303309
},
304310
{
305311
name: "webhook conversion strategy exists",
@@ -316,9 +322,9 @@ func TestUpgrade(t *testing.T) {
316322
Name: "test-release",
317323
Manifest: getManifestString(t, "crd-conversion-no-webhook.json"),
318324
},
319-
wantErrMsgs: []string{
325+
requireErr: wantErrorMsgs([]string{
320326
`validating upgrade for CRD "crontabs.stable.example.com": v1 <-> v2: ^.spec.foobarbaz: enum: allowed enum values removed`,
321-
},
327+
}),
322328
},
323329
{
324330
name: "new crd validation should not fail on description changes",
@@ -330,16 +336,43 @@ func TestUpgrade(t *testing.T) {
330336
Manifest: getManifestString(t, "crd-description-changed.json"),
331337
},
332338
},
339+
{
340+
name: "success when old crd and new crd contain the exact same validation issues",
341+
oldCrdPath: "crd-conversion-no-webhook.json",
342+
release: &release.Release{
343+
Name: "test-release",
344+
Manifest: getManifestString(t, "crd-conversion-no-webhook.json"),
345+
},
346+
},
347+
{
348+
name: "failure when old crd and new crd contain the exact same validation issues, but new crd introduces another validation issue",
349+
oldCrdPath: "crd-conversion-no-webhook.json",
350+
release: &release.Release{
351+
Name: "test-release",
352+
Manifest: getManifestString(t, "crd-conversion-no-webhook-extra-issue.json"),
353+
},
354+
requireErr: func(t require.TestingT, err error, _ ...interface{}) {
355+
require.ErrorContains(t, err,
356+
`validating upgrade for CRD "crontabs.stable.example.com":`,
357+
)
358+
// The newly introduced issue is reported
359+
require.Contains(t, err.Error(),
360+
`v1 <-> v2: ^.spec.extraField: type: type changed : "boolean" -> "string"`,
361+
)
362+
// The existing issue is not reported
363+
require.NotContains(t, err.Error(),
364+
`v1 <-> v2: ^.spec.foobarbaz: enum: allowed enum values removed`,
365+
)
366+
},
367+
},
333368
}
334369

335370
for _, tc := range tests {
336371
t.Run(tc.name, func(t *testing.T) {
337372
preflight := newMockPreflight(getCrdFromManifestFile(t, tc.oldCrdPath), tc.wantCrdGetErr)
338373
err := preflight.Upgrade(context.Background(), tc.release)
339-
if len(tc.wantErrMsgs) != 0 {
340-
for _, expectedErrMsg := range tc.wantErrMsgs {
341-
require.ErrorContains(t, err, expectedErrMsg)
342-
}
374+
if tc.requireErr != nil {
375+
tc.requireErr(t, err)
343376
} else {
344377
require.NoError(t, err)
345378
}

‎internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-conversion-no-webhook-extra-issue.json

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
{
2+
"apiVersion": "apiextensions.k8s.io/v1",
3+
"kind": "CustomResourceDefinition",
4+
"metadata": {
5+
"name": "crontabs.stable.example.com"
6+
},
7+
"spec": {
8+
"group": "stable.example.com",
9+
"versions": [
10+
{
11+
"name": "v2",
12+
"served": true,
13+
"storage": false,
14+
"schema": {
15+
"openAPIV3Schema": {
16+
"type": "object",
17+
"properties": {
18+
"spec": {
19+
"type": "object",
20+
"properties": {
21+
"foobarbaz": {
22+
"type":"string",
23+
"enum":[
24+
"bark",
25+
"woof"
26+
]
27+
},
28+
"extraField": {
29+
"type":"string"
30+
}
31+
}
32+
}
33+
}
34+
}
35+
}
36+
},
37+
{
38+
"name": "v1",
39+
"served": true,
40+
"storage": false,
41+
"schema": {
42+
"openAPIV3Schema": {
43+
"type": "object",
44+
"properties": {
45+
"spec": {
46+
"type": "object",
47+
"properties": {
48+
"foobarbaz": {
49+
"type":"string",
50+
"enum":[
51+
"foo",
52+
"bar",
53+
"baz"
54+
]
55+
},
56+
"extraField": {
57+
"type":"boolean"
58+
}
59+
}
60+
}
61+
}
62+
}
63+
}
64+
}
65+
],
66+
"scope": "Cluster",
67+
"names": {
68+
"plural": "crontabs",
69+
"singular": "crontab",
70+
"kind": "CronTab",
71+
"shortNames": [
72+
"ct"
73+
]
74+
}
75+
}
76+
}

0 commit comments

Comments
 (0)