Skip to content

Commit 5fe9101

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#78815 from sttts/sttts-crd-int-or-string-validation
apiextensions: validate x-kubernetes-int-or-string in CRs
2 parents c7972d9 + bfa4b66 commit 5fe9101

File tree

3 files changed

+174
-70
lines changed

3 files changed

+174
-70
lines changed

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} {
123123
if len(obj.Type) == 0 {
124124
obj.Nullable = false // because this does not roundtrip through go-openapi
125125
}
126+
if obj.XIntOrString {
127+
obj.Type = ""
128+
}
126129
},
127130
func(obj *apiextensions.JSONSchemaPropsOrBool, c fuzz.Continue) {
128131
if c.RandBool() {

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,13 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou
7171
out.Description = in.Description
7272
if in.Type != "" {
7373
out.Type = spec.StringOrArray([]string{in.Type})
74-
if in.Nullable {
75-
out.Type = append(out.Type, "null")
76-
}
74+
}
75+
if in.XIntOrString {
76+
out.VendorExtensible.AddExtension("x-kubernetes-int-or-string", true)
77+
out.Type = spec.StringOrArray{"integer", "string"}
78+
}
79+
if out.Type != nil && in.Nullable {
80+
out.Type = append(out.Type, "null")
7781
}
7882
out.Format = in.Format
7983
out.Title = in.Title
@@ -201,9 +205,6 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou
201205
if in.XEmbeddedResource {
202206
out.VendorExtensible.AddExtension("x-kubernetes-embedded-resource", true)
203207
}
204-
if in.XIntOrString {
205-
out.VendorExtensible.AddExtension("x-kubernetes-int-or-string", true)
206-
}
207208

208209
return nil
209210
}

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go‎

Lines changed: 164 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ func TestRoundTrip(t *testing.T) {
7373
t.Fatal(err)
7474
}
7575
j = convertNullTypeToNullable(j)
76+
j = stripIntOrStringType(j)
7677
openAPIJSON, err = json.Marshal(j)
7778
if err != nil {
7879
t.Fatal(err)
@@ -139,126 +140,225 @@ func convertNullTypeToNullable(x interface{}) interface{} {
139140
}
140141
}
141142

142-
func TestValidateCustomResource(t *testing.T) {
143-
type args struct {
144-
schema apiextensions.JSONSchemaProps
145-
object interface{}
143+
func stripIntOrStringType(x interface{}) interface{} {
144+
switch x := x.(type) {
145+
case map[string]interface{}:
146+
if t, found := x["type"]; found {
147+
switch t := t.(type) {
148+
case []interface{}:
149+
if len(t) == 2 && t[0] == "integer" && t[1] == "string" && x["x-kubernetes-int-or-string"] == true {
150+
delete(x, "type")
151+
}
152+
}
153+
}
154+
for k := range x {
155+
x[k] = stripIntOrStringType(x[k])
156+
}
157+
return x
158+
case []interface{}:
159+
for i := range x {
160+
x[i] = stripIntOrStringType(x[i])
161+
}
162+
return x
163+
default:
164+
return x
146165
}
166+
}
167+
168+
func TestValidateCustomResource(t *testing.T) {
147169
tests := []struct {
148-
name string
149-
args args
150-
wantErr bool
170+
name string
171+
schema apiextensions.JSONSchemaProps
172+
objects []interface{}
173+
failingObjects []interface{}
151174
}{
152-
// TODO: make more complete
153-
{"!nullable against non-null", args{
154-
apiextensions.JSONSchemaProps{
175+
{name: "!nullable",
176+
schema: apiextensions.JSONSchemaProps{
155177
Properties: map[string]apiextensions.JSONSchemaProps{
156178
"field": {
157179
Type: "object",
158180
Nullable: false,
159181
},
160182
},
161183
},
162-
map[string]interface{}{"field": map[string]interface{}{}},
163-
}, false},
164-
{"!nullable against null", args{
165-
apiextensions.JSONSchemaProps{
166-
Properties: map[string]apiextensions.JSONSchemaProps{
167-
"field": {
168-
Type: "object",
169-
Nullable: false,
170-
},
171-
},
184+
objects: []interface{}{
185+
map[string]interface{}{},
186+
map[string]interface{}{"field": map[string]interface{}{}},
172187
},
173-
map[string]interface{}{"field": nil},
174-
}, true},
175-
{"!nullable against undefined", args{
176-
apiextensions.JSONSchemaProps{
188+
failingObjects: []interface{}{
189+
map[string]interface{}{"field": "foo"},
190+
map[string]interface{}{"field": 42},
191+
map[string]interface{}{"field": true},
192+
map[string]interface{}{"field": 1.2},
193+
map[string]interface{}{"field": []interface{}{}},
194+
},
195+
},
196+
{name: "nullable",
197+
schema: apiextensions.JSONSchemaProps{
177198
Properties: map[string]apiextensions.JSONSchemaProps{
178199
"field": {
179200
Type: "object",
180-
Nullable: false,
201+
Nullable: true,
181202
},
182203
},
183204
},
184-
map[string]interface{}{},
185-
}, false},
186-
{"nullable against non-null", args{
187-
apiextensions.JSONSchemaProps{
205+
objects: []interface{}{
206+
map[string]interface{}{},
207+
map[string]interface{}{"field": map[string]interface{}{}},
208+
map[string]interface{}{"field": nil},
209+
},
210+
failingObjects: []interface{}{
211+
map[string]interface{}{"field": "foo"},
212+
map[string]interface{}{"field": 42},
213+
map[string]interface{}{"field": true},
214+
map[string]interface{}{"field": 1.2},
215+
map[string]interface{}{"field": []interface{}{}},
216+
},
217+
},
218+
{name: "nullable and no type",
219+
schema: apiextensions.JSONSchemaProps{
188220
Properties: map[string]apiextensions.JSONSchemaProps{
189221
"field": {
190-
Type: "object",
191222
Nullable: true,
192223
},
193224
},
194225
},
195-
map[string]interface{}{"field": map[string]interface{}{}},
196-
}, false},
197-
{"nullable against null", args{
198-
apiextensions.JSONSchemaProps{
226+
objects: []interface{}{
227+
map[string]interface{}{},
228+
map[string]interface{}{"field": map[string]interface{}{}},
229+
map[string]interface{}{"field": nil},
230+
map[string]interface{}{"field": "foo"},
231+
map[string]interface{}{"field": 42},
232+
map[string]interface{}{"field": true},
233+
map[string]interface{}{"field": 1.2},
234+
map[string]interface{}{"field": []interface{}{}},
235+
},
236+
},
237+
{name: "x-kubernetes-int-or-string",
238+
schema: apiextensions.JSONSchemaProps{
199239
Properties: map[string]apiextensions.JSONSchemaProps{
200240
"field": {
201-
Type: "object",
202-
Nullable: true,
241+
XIntOrString: true,
203242
},
204243
},
205244
},
206-
map[string]interface{}{"field": nil},
207-
}, false},
208-
{"!nullable against undefined", args{
209-
apiextensions.JSONSchemaProps{
245+
objects: []interface{}{
246+
map[string]interface{}{},
247+
map[string]interface{}{"field": 42},
248+
map[string]interface{}{"field": "foo"},
249+
},
250+
failingObjects: []interface{}{
251+
map[string]interface{}{"field": nil},
252+
map[string]interface{}{"field": true},
253+
map[string]interface{}{"field": 1.2},
254+
map[string]interface{}{"field": map[string]interface{}{}},
255+
map[string]interface{}{"field": []interface{}{}},
256+
},
257+
},
258+
{name: "nullable and x-kubernetes-int-or-string",
259+
schema: apiextensions.JSONSchemaProps{
210260
Properties: map[string]apiextensions.JSONSchemaProps{
211261
"field": {
212-
Type: "object",
213-
Nullable: true,
262+
Nullable: true,
263+
XIntOrString: true,
214264
},
215265
},
216266
},
217-
map[string]interface{}{},
218-
}, false},
219-
{"nullable and no type against non-nil", args{
220-
apiextensions.JSONSchemaProps{
267+
objects: []interface{}{
268+
map[string]interface{}{},
269+
map[string]interface{}{"field": 42},
270+
map[string]interface{}{"field": "foo"},
271+
map[string]interface{}{"field": nil},
272+
},
273+
failingObjects: []interface{}{
274+
map[string]interface{}{"field": true},
275+
map[string]interface{}{"field": 1.2},
276+
map[string]interface{}{"field": map[string]interface{}{}},
277+
map[string]interface{}{"field": []interface{}{}},
278+
},
279+
},
280+
{name: "nullable, x-kubernetes-int-or-string and user-provided anyOf",
281+
schema: apiextensions.JSONSchemaProps{
221282
Properties: map[string]apiextensions.JSONSchemaProps{
222283
"field": {
223-
Nullable: true,
284+
Nullable: true,
285+
XIntOrString: true,
286+
AnyOf: []apiextensions.JSONSchemaProps{
287+
{Type: "integer"},
288+
{Type: "string"},
289+
},
224290
},
225291
},
226292
},
227-
map[string]interface{}{"field": 42},
228-
}, false},
229-
{"nullable and no type against nil", args{
230-
apiextensions.JSONSchemaProps{
293+
objects: []interface{}{
294+
map[string]interface{}{},
295+
map[string]interface{}{"field": nil},
296+
map[string]interface{}{"field": 42},
297+
map[string]interface{}{"field": "foo"},
298+
},
299+
failingObjects: []interface{}{
300+
map[string]interface{}{"field": true},
301+
map[string]interface{}{"field": 1.2},
302+
map[string]interface{}{"field": map[string]interface{}{}},
303+
map[string]interface{}{"field": []interface{}{}},
304+
},
305+
},
306+
{name: "nullable, x-kubernetes-int-or-string and user-provider allOf",
307+
schema: apiextensions.JSONSchemaProps{
231308
Properties: map[string]apiextensions.JSONSchemaProps{
232309
"field": {
233-
Nullable: true,
310+
Nullable: true,
311+
XIntOrString: true,
312+
AllOf: []apiextensions.JSONSchemaProps{
313+
{
314+
AnyOf: []apiextensions.JSONSchemaProps{
315+
{Type: "integer"},
316+
{Type: "string"},
317+
},
318+
},
319+
},
234320
},
235321
},
236322
},
237-
map[string]interface{}{"field": nil},
238-
}, false},
239-
{"invalid regex", args{
240-
apiextensions.JSONSchemaProps{
323+
objects: []interface{}{
324+
map[string]interface{}{},
325+
map[string]interface{}{"field": nil},
326+
map[string]interface{}{"field": 42},
327+
map[string]interface{}{"field": "foo"},
328+
},
329+
failingObjects: []interface{}{
330+
map[string]interface{}{"field": true},
331+
map[string]interface{}{"field": 1.2},
332+
map[string]interface{}{"field": map[string]interface{}{}},
333+
map[string]interface{}{"field": []interface{}{}},
334+
},
335+
},
336+
{name: "invalid regex",
337+
schema: apiextensions.JSONSchemaProps{
241338
Properties: map[string]apiextensions.JSONSchemaProps{
242339
"field": {
243340
Type: "string",
244341
Pattern: "+",
245342
},
246343
},
247344
},
248-
map[string]interface{}{"field": "foo"},
249-
}, true},
345+
failingObjects: []interface{}{map[string]interface{}{"field": "foo"}},
346+
},
250347
}
251348
for _, tt := range tests {
252349
t.Run(tt.name, func(t *testing.T) {
253-
validator, _, err := NewSchemaValidator(&apiextensions.CustomResourceValidation{OpenAPIV3Schema: &tt.args.schema})
350+
validator, _, err := NewSchemaValidator(&apiextensions.CustomResourceValidation{OpenAPIV3Schema: &tt.schema})
254351
if err != nil {
255352
t.Fatal(err)
256353
}
257-
if err := ValidateCustomResource(tt.args.object, validator); (err != nil) != tt.wantErr {
258-
if err == nil {
259-
t.Error("expected error, but didn't get one")
260-
} else {
261-
t.Errorf("unexpected validation error: %v", err)
354+
for _, obj := range tt.objects {
355+
if err := ValidateCustomResource(obj, validator); err != nil {
356+
t.Errorf("unexpected validation error for %v: %v", obj, err)
357+
}
358+
}
359+
for _, obj := range tt.failingObjects {
360+
if err := ValidateCustomResource(obj, validator); err == nil {
361+
t.Errorf("missing error for %v", obj)
262362
}
263363
}
264364
})

0 commit comments

Comments
 (0)