Skip to content

Commit 23b7d8b

Browse files
committed
apiextensions: generalize nullable to arbitrary types
1 parent 975d537 commit 23b7d8b

File tree

5 files changed

+39
-25
lines changed

5 files changed

+39
-25
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"reflect"
2121
"strings"
2222

23-
"github.com/google/gofuzz"
23+
fuzz "github.com/google/gofuzz"
2424

2525
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -113,6 +113,9 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} {
113113
validRef := "validRef"
114114
obj.Ref = &validRef
115115
}
116+
if len(obj.Type) == 0 {
117+
obj.Nullable = false // because this does not roundtrip through go-openapi
118+
}
116119
},
117120
func(obj *apiextensions.JSONSchemaPropsOrBool, c fuzz.Continue) {
118121
if c.RandBool() {

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -647,9 +647,6 @@ func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps
647647
if schema.Type == "null" {
648648
allErrs = append(allErrs, field.Forbidden(fldPath.Child("type"), "type cannot be set to null, use nullable as an alternative"))
649649
}
650-
if schema.Nullable && schema.Type != "object" && schema.Type != "array" {
651-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("nullable"), "nullable can only be set for object and array types"))
652-
}
653650

654651
if schema.Items != nil && len(schema.Items.JSONSchemas) != 0 {
655652
allErrs = append(allErrs, field.Forbidden(fldPath.Child("items"), "items must be a schema object and not an array"))

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

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,24 +1259,10 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
12591259
},
12601260
},
12611261
},
1262-
wantError: true,
1263-
},
1264-
{
1265-
name: "nullable with wrong type",
1266-
input: apiextensions.CustomResourceValidation{
1267-
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
1268-
Properties: map[string]apiextensions.JSONSchemaProps{
1269-
"string": {
1270-
Type: "string",
1271-
Nullable: true,
1272-
},
1273-
},
1274-
},
1275-
},
1276-
wantError: true,
1262+
wantError: false,
12771263
},
12781264
{
1279-
name: "nullable with right types",
1265+
name: "nullable with types",
12801266
input: apiextensions.CustomResourceValidation{
12811267
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
12821268
Properties: map[string]apiextensions.JSONSchemaProps{
@@ -1288,6 +1274,14 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
12881274
Type: "array",
12891275
Nullable: true,
12901276
},
1277+
"number": {
1278+
Type: "number",
1279+
Nullable: true,
1280+
},
1281+
"string": {
1282+
Type: "string",
1283+
Nullable: true,
1284+
},
12911285
},
12921286
},
12931287
},

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,9 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou
7070
out.Description = in.Description
7171
if in.Type != "" {
7272
out.Type = spec.StringOrArray([]string{in.Type})
73-
}
74-
if in.Nullable {
75-
// by validation, in.Type is either "object" or "array"
76-
out.Type = append(out.Type, "null")
73+
if in.Nullable {
74+
out.Type = append(out.Type, "null")
75+
}
7776
}
7877
out.Format = in.Format
7978
out.Title = in.Title

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ func TestRoundTrip(t *testing.T) {
4646
}
4747

4848
seed := rand.Int63()
49+
t.Logf("seed: %d", seed)
4950
fuzzerFuncs := fuzzer.MergeFuzzerFuncs(apiextensionsfuzzer.Funcs)
5051
f := fuzzer.FuzzerFor(fuzzerFuncs, rand.NewSource(seed), codecs)
5152

@@ -90,7 +91,7 @@ func TestRoundTrip(t *testing.T) {
9091
}
9192

9293
if !apiequality.Semantic.DeepEqual(internal, internalRoundTripped) {
93-
t.Fatalf("expected\n\t%#v, got \n\t%#v", internal, internalRoundTripped)
94+
t.Fatalf("%d: expected\n\t%#v, got \n\t%#v", i, internal, internalRoundTripped)
9495
}
9596
}
9697
}
@@ -214,6 +215,26 @@ func TestNullable(t *testing.T) {
214215
},
215216
map[string]interface{}{},
216217
}, false},
218+
{"nullable and no type against non-nil", args{
219+
apiextensions.JSONSchemaProps{
220+
Properties: map[string]apiextensions.JSONSchemaProps{
221+
"field": {
222+
Nullable: true,
223+
},
224+
},
225+
},
226+
map[string]interface{}{"field": 42},
227+
}, false},
228+
{"nullable and no type against nil", args{
229+
apiextensions.JSONSchemaProps{
230+
Properties: map[string]apiextensions.JSONSchemaProps{
231+
"field": {
232+
Nullable: true,
233+
},
234+
},
235+
},
236+
map[string]interface{}{"field": nil},
237+
}, false},
217238
}
218239
for _, tt := range tests {
219240
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)