diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go index 51ab78021ed..615e8b2eb70 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go @@ -123,6 +123,9 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} { if len(obj.Type) == 0 { obj.Nullable = false // because this does not roundtrip through go-openapi } + if obj.XIntOrString { + obj.Type = "" + } }, func(obj *apiextensions.JSONSchemaPropsOrBool, c fuzz.Continue) { if c.RandBool() { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go index 039c37b1246..5ea1f0076b8 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go @@ -71,9 +71,13 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou out.Description = in.Description if in.Type != "" { out.Type = spec.StringOrArray([]string{in.Type}) - if in.Nullable { - out.Type = append(out.Type, "null") - } + } + if in.XIntOrString { + out.VendorExtensible.AddExtension("x-kubernetes-int-or-string", true) + out.Type = spec.StringOrArray{"integer", "string"} + } + if out.Type != nil && in.Nullable { + out.Type = append(out.Type, "null") } out.Format = in.Format out.Title = in.Title @@ -201,9 +205,6 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou if in.XEmbeddedResource { out.VendorExtensible.AddExtension("x-kubernetes-embedded-resource", true) } - if in.XIntOrString { - out.VendorExtensible.AddExtension("x-kubernetes-int-or-string", true) - } return nil } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go index c0bf419d8bd..e9bf6e12630 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go @@ -73,6 +73,7 @@ func TestRoundTrip(t *testing.T) { t.Fatal(err) } j = convertNullTypeToNullable(j) + j = stripIntOrStringType(j) openAPIJSON, err = json.Marshal(j) if err != nil { t.Fatal(err) @@ -139,19 +140,40 @@ func convertNullTypeToNullable(x interface{}) interface{} { } } -func TestValidateCustomResource(t *testing.T) { - type args struct { - schema apiextensions.JSONSchemaProps - object interface{} +func stripIntOrStringType(x interface{}) interface{} { + switch x := x.(type) { + case map[string]interface{}: + if t, found := x["type"]; found { + switch t := t.(type) { + case []interface{}: + if len(t) == 2 && t[0] == "integer" && t[1] == "string" && x["x-kubernetes-int-or-string"] == true { + delete(x, "type") + } + } + } + for k := range x { + x[k] = stripIntOrStringType(x[k]) + } + return x + case []interface{}: + for i := range x { + x[i] = stripIntOrStringType(x[i]) + } + return x + default: + return x } +} + +func TestValidateCustomResource(t *testing.T) { tests := []struct { - name string - args args - wantErr bool + name string + schema apiextensions.JSONSchemaProps + objects []interface{} + failingObjects []interface{} }{ - // TODO: make more complete - {"!nullable against non-null", args{ - apiextensions.JSONSchemaProps{ + {name: "!nullable", + schema: apiextensions.JSONSchemaProps{ Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "object", @@ -159,32 +181,20 @@ func TestValidateCustomResource(t *testing.T) { }, }, }, - map[string]interface{}{"field": map[string]interface{}{}}, - }, false}, - {"!nullable against null", args{ - apiextensions.JSONSchemaProps{ - Properties: map[string]apiextensions.JSONSchemaProps{ - "field": { - Type: "object", - Nullable: false, - }, - }, + objects: []interface{}{ + map[string]interface{}{}, + map[string]interface{}{"field": map[string]interface{}{}}, }, - map[string]interface{}{"field": nil}, - }, true}, - {"!nullable against undefined", args{ - apiextensions.JSONSchemaProps{ - Properties: map[string]apiextensions.JSONSchemaProps{ - "field": { - Type: "object", - Nullable: false, - }, - }, + failingObjects: []interface{}{ + map[string]interface{}{"field": "foo"}, + map[string]interface{}{"field": 42}, + map[string]interface{}{"field": true}, + map[string]interface{}{"field": 1.2}, + map[string]interface{}{"field": []interface{}{}}, }, - map[string]interface{}{}, - }, false}, - {"nullable against non-null", args{ - apiextensions.JSONSchemaProps{ + }, + {name: "nullable", + schema: apiextensions.JSONSchemaProps{ Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "object", @@ -192,52 +202,139 @@ func TestValidateCustomResource(t *testing.T) { }, }, }, - map[string]interface{}{"field": map[string]interface{}{}}, - }, false}, - {"nullable against null", args{ - apiextensions.JSONSchemaProps{ - Properties: map[string]apiextensions.JSONSchemaProps{ - "field": { - Type: "object", - Nullable: true, - }, - }, + objects: []interface{}{ + map[string]interface{}{}, + map[string]interface{}{"field": map[string]interface{}{}}, + map[string]interface{}{"field": nil}, }, - map[string]interface{}{"field": nil}, - }, false}, - {"!nullable against undefined", args{ - apiextensions.JSONSchemaProps{ - Properties: map[string]apiextensions.JSONSchemaProps{ - "field": { - Type: "object", - Nullable: true, - }, - }, + failingObjects: []interface{}{ + map[string]interface{}{"field": "foo"}, + map[string]interface{}{"field": 42}, + map[string]interface{}{"field": true}, + map[string]interface{}{"field": 1.2}, + map[string]interface{}{"field": []interface{}{}}, }, - map[string]interface{}{}, - }, false}, - {"nullable and no type against non-nil", args{ - apiextensions.JSONSchemaProps{ + }, + {name: "nullable and no type", + schema: apiextensions.JSONSchemaProps{ Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Nullable: true, }, }, }, - map[string]interface{}{"field": 42}, - }, false}, - {"nullable and no type against nil", args{ - apiextensions.JSONSchemaProps{ + objects: []interface{}{ + map[string]interface{}{}, + map[string]interface{}{"field": map[string]interface{}{}}, + map[string]interface{}{"field": nil}, + map[string]interface{}{"field": "foo"}, + map[string]interface{}{"field": 42}, + map[string]interface{}{"field": true}, + map[string]interface{}{"field": 1.2}, + map[string]interface{}{"field": []interface{}{}}, + }, + }, + {name: "x-kubernetes-int-or-string", + schema: apiextensions.JSONSchemaProps{ Properties: map[string]apiextensions.JSONSchemaProps{ "field": { - Nullable: true, + XIntOrString: true, }, }, }, - map[string]interface{}{"field": nil}, - }, false}, - {"invalid regex", args{ - apiextensions.JSONSchemaProps{ + objects: []interface{}{ + map[string]interface{}{}, + map[string]interface{}{"field": 42}, + map[string]interface{}{"field": "foo"}, + }, + failingObjects: []interface{}{ + map[string]interface{}{"field": nil}, + map[string]interface{}{"field": true}, + map[string]interface{}{"field": 1.2}, + map[string]interface{}{"field": map[string]interface{}{}}, + map[string]interface{}{"field": []interface{}{}}, + }, + }, + {name: "nullable and x-kubernetes-int-or-string", + schema: apiextensions.JSONSchemaProps{ + Properties: map[string]apiextensions.JSONSchemaProps{ + "field": { + Nullable: true, + XIntOrString: true, + }, + }, + }, + objects: []interface{}{ + map[string]interface{}{}, + map[string]interface{}{"field": 42}, + map[string]interface{}{"field": "foo"}, + map[string]interface{}{"field": nil}, + }, + failingObjects: []interface{}{ + map[string]interface{}{"field": true}, + map[string]interface{}{"field": 1.2}, + map[string]interface{}{"field": map[string]interface{}{}}, + map[string]interface{}{"field": []interface{}{}}, + }, + }, + {name: "nullable, x-kubernetes-int-or-string and user-provided anyOf", + schema: apiextensions.JSONSchemaProps{ + Properties: map[string]apiextensions.JSONSchemaProps{ + "field": { + Nullable: true, + XIntOrString: true, + AnyOf: []apiextensions.JSONSchemaProps{ + {Type: "integer"}, + {Type: "string"}, + }, + }, + }, + }, + objects: []interface{}{ + map[string]interface{}{}, + map[string]interface{}{"field": nil}, + map[string]interface{}{"field": 42}, + map[string]interface{}{"field": "foo"}, + }, + failingObjects: []interface{}{ + map[string]interface{}{"field": true}, + map[string]interface{}{"field": 1.2}, + map[string]interface{}{"field": map[string]interface{}{}}, + map[string]interface{}{"field": []interface{}{}}, + }, + }, + {name: "nullable, x-kubernetes-int-or-string and user-provider allOf", + schema: apiextensions.JSONSchemaProps{ + Properties: map[string]apiextensions.JSONSchemaProps{ + "field": { + Nullable: true, + XIntOrString: true, + AllOf: []apiextensions.JSONSchemaProps{ + { + AnyOf: []apiextensions.JSONSchemaProps{ + {Type: "integer"}, + {Type: "string"}, + }, + }, + }, + }, + }, + }, + objects: []interface{}{ + map[string]interface{}{}, + map[string]interface{}{"field": nil}, + map[string]interface{}{"field": 42}, + map[string]interface{}{"field": "foo"}, + }, + failingObjects: []interface{}{ + map[string]interface{}{"field": true}, + map[string]interface{}{"field": 1.2}, + map[string]interface{}{"field": map[string]interface{}{}}, + map[string]interface{}{"field": []interface{}{}}, + }, + }, + {name: "invalid regex", + schema: apiextensions.JSONSchemaProps{ Properties: map[string]apiextensions.JSONSchemaProps{ "field": { Type: "string", @@ -245,20 +342,23 @@ func TestValidateCustomResource(t *testing.T) { }, }, }, - map[string]interface{}{"field": "foo"}, - }, true}, + failingObjects: []interface{}{map[string]interface{}{"field": "foo"}}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validator, _, err := NewSchemaValidator(&apiextensions.CustomResourceValidation{OpenAPIV3Schema: &tt.args.schema}) + validator, _, err := NewSchemaValidator(&apiextensions.CustomResourceValidation{OpenAPIV3Schema: &tt.schema}) if err != nil { t.Fatal(err) } - if err := ValidateCustomResource(tt.args.object, validator); (err != nil) != tt.wantErr { - if err == nil { - t.Error("expected error, but didn't get one") - } else { - t.Errorf("unexpected validation error: %v", err) + for _, obj := range tt.objects { + if err := ValidateCustomResource(obj, validator); err != nil { + t.Errorf("unexpected validation error for %v: %v", obj, err) + } + } + for _, obj := range tt.failingObjects { + if err := ValidateCustomResource(obj, validator); err == nil { + t.Errorf("missing error for %v", obj) } } })