apiextensions: generalize nullable to arbitrary types

This commit is contained in:
Dr. Stefan Schimanski
2019-03-01 17:29:15 +01:00
parent 975d537ff8
commit 23b7d8b7b9
5 changed files with 39 additions and 25 deletions

View File

@@ -20,7 +20,7 @@ import (
"reflect" "reflect"
"strings" "strings"
"github.com/google/gofuzz" fuzz "github.com/google/gofuzz"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -113,6 +113,9 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} {
validRef := "validRef" validRef := "validRef"
obj.Ref = &validRef obj.Ref = &validRef
} }
if len(obj.Type) == 0 {
obj.Nullable = false // because this does not roundtrip through go-openapi
}
}, },
func(obj *apiextensions.JSONSchemaPropsOrBool, c fuzz.Continue) { func(obj *apiextensions.JSONSchemaPropsOrBool, c fuzz.Continue) {
if c.RandBool() { if c.RandBool() {

View File

@@ -647,9 +647,6 @@ func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps
if schema.Type == "null" { if schema.Type == "null" {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("type"), "type cannot be set to null, use nullable as an alternative")) allErrs = append(allErrs, field.Forbidden(fldPath.Child("type"), "type cannot be set to null, use nullable as an alternative"))
} }
if schema.Nullable && schema.Type != "object" && schema.Type != "array" {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("nullable"), "nullable can only be set for object and array types"))
}
if schema.Items != nil && len(schema.Items.JSONSchemas) != 0 { if schema.Items != nil && len(schema.Items.JSONSchemas) != 0 {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("items"), "items must be a schema object and not an array")) allErrs = append(allErrs, field.Forbidden(fldPath.Child("items"), "items must be a schema object and not an array"))

View File

@@ -1259,24 +1259,10 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
}, },
}, },
}, },
wantError: true, wantError: false,
}, },
{ {
name: "nullable with wrong type", name: "nullable with types",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"string": {
Type: "string",
Nullable: true,
},
},
},
},
wantError: true,
},
{
name: "nullable with right types",
input: apiextensions.CustomResourceValidation{ input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{ Properties: map[string]apiextensions.JSONSchemaProps{
@@ -1288,6 +1274,14 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
Type: "array", Type: "array",
Nullable: true, Nullable: true,
}, },
"number": {
Type: "number",
Nullable: true,
},
"string": {
Type: "string",
Nullable: true,
},
}, },
}, },
}, },

View File

@@ -70,11 +70,10 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou
out.Description = in.Description out.Description = in.Description
if in.Type != "" { if in.Type != "" {
out.Type = spec.StringOrArray([]string{in.Type}) out.Type = spec.StringOrArray([]string{in.Type})
}
if in.Nullable { if in.Nullable {
// by validation, in.Type is either "object" or "array"
out.Type = append(out.Type, "null") out.Type = append(out.Type, "null")
} }
}
out.Format = in.Format out.Format = in.Format
out.Title = in.Title out.Title = in.Title
out.Maximum = in.Maximum out.Maximum = in.Maximum

View File

@@ -46,6 +46,7 @@ func TestRoundTrip(t *testing.T) {
} }
seed := rand.Int63() seed := rand.Int63()
t.Logf("seed: %d", seed)
fuzzerFuncs := fuzzer.MergeFuzzerFuncs(apiextensionsfuzzer.Funcs) fuzzerFuncs := fuzzer.MergeFuzzerFuncs(apiextensionsfuzzer.Funcs)
f := fuzzer.FuzzerFor(fuzzerFuncs, rand.NewSource(seed), codecs) f := fuzzer.FuzzerFor(fuzzerFuncs, rand.NewSource(seed), codecs)
@@ -90,7 +91,7 @@ func TestRoundTrip(t *testing.T) {
} }
if !apiequality.Semantic.DeepEqual(internal, internalRoundTripped) { if !apiequality.Semantic.DeepEqual(internal, internalRoundTripped) {
t.Fatalf("expected\n\t%#v, got \n\t%#v", internal, internalRoundTripped) t.Fatalf("%d: expected\n\t%#v, got \n\t%#v", i, internal, internalRoundTripped)
} }
} }
} }
@@ -214,6 +215,26 @@ func TestNullable(t *testing.T) {
}, },
map[string]interface{}{}, map[string]interface{}{},
}, false}, }, false},
{"nullable and no type against non-nil", args{
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{
Properties: map[string]apiextensions.JSONSchemaProps{
"field": {
Nullable: true,
},
},
},
map[string]interface{}{"field": nil},
}, false},
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {