diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert_test.go index 59b487bc46c..7c7256ddcfc 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert_test.go @@ -75,11 +75,6 @@ func TestStructuralRoundtripOrError(t *testing.T) { continue } f.Fuzz(x.Field(n).Addr().Interface()) - if origSchema.Nullable { - // non-empty type for nullable. nullable:true with empty type does not roundtrip because - // go-openapi does not allow to encode that (we use type slices otherwise). - origSchema.Type = "string" - } // it roundtrips or NewStructural errors out. We should never drop anything orig, err := NewStructural(origSchema) @@ -93,9 +88,8 @@ func TestStructuralRoundtripOrError(t *testing.T) { if err != nil { t.Fatal(err) } - str := nullTypeRE.ReplaceAllString(string(bs), `"type":"$1","nullable":true`) // unfold nullable type:[,"null"] -> type:,nullable:true v1beta1Schema := &apiextensionsv1beta1.JSONSchemaProps{} - err = json.Unmarshal([]byte(str), v1beta1Schema) + err = json.Unmarshal([]byte(bs), v1beta1Schema) if err != nil { t.Fatal(err) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi.go index 59b4c999085..9fbf02c8f93 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi.go @@ -20,13 +20,7 @@ import ( "github.com/go-openapi/spec" ) -// ToGoOpenAPI converts a structural schema to go-openapi schema. It is faithful and roundtrippable -// with the exception of `nullable:true` for empty type (`type:""`). -// -// WARNING: Do not use the returned schema to perform CRD validation until this restriction is solved. -// -// Nullable:true is mapped to `type:[,"null"]` -// if the structural type is non-empty, and nullable is dropped if the structural type is empty. +// ToGoOpenAPI converts a structural schema to go-openapi schema. It is faithful and roundtrippable. func (s *Structural) ToGoOpenAPI() *spec.Schema { if s == nil { return nil @@ -57,14 +51,8 @@ func (g *Generic) toGoOpenAPI(ret *spec.Schema) { if len(g.Type) != 0 { ret.Type = spec.StringOrArray{g.Type} - if g.Nullable { - // go-openapi does not support nullable, but multiple type values. - // Only when type is already non-empty, adding null to the types is correct though. - // If you add null as only type, you enforce null, in contrast to nullable being - // ineffective if no type is provided in a schema. - ret.Type = append(ret.Type, "null") - } } + ret.Nullable = g.Nullable if g.AdditionalProperties != nil { ret.AdditionalProperties = &spec.SchemaOrBool{ Allows: g.AdditionalProperties.Bool, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi_test.go index 9309c18d77c..3b13e16e4cd 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi_test.go @@ -49,25 +49,13 @@ func TestStructuralRoundtrip(t *testing.T) { case 2: s.Object = "" case 3: - s.Object = []string{} + s.Object = []interface{}{} case 4: s.Object = map[string]interface{}{} case 5: s.Object = nil } }, - func(g *Generic, c fuzz.Continue) { - c.FuzzNoCustom(g) - - // TODO: make nullable in case of empty type survive go-openapi JSON -> API schema roundtrip - // go-openapi does not support nullable. Adding it to a type slice produces OpenAPI v3 - // incompatible JSON which we cannot unmarshal (without string-replace magic to transform - // null types back into nullable). If type is empty, nullable:true is not preserved - // at all. - if len(g.Type) == 0 { - g.Nullable = false - } - }, ) f.MaxDepth(3) f.NilChance(0.5) @@ -93,9 +81,8 @@ func TestStructuralRoundtrip(t *testing.T) { if err != nil { t.Fatal(err) } - str := nullTypeRE.ReplaceAllString(string(bs), `"type":"$1","nullable":true`) // unfold nullable type:[,"null"] -> type:,nullable:true v1beta1Schema := &apiextensionsv1beta1.JSONSchemaProps{} - err = json.Unmarshal([]byte(str), v1beta1Schema) + err = json.Unmarshal(bs, v1beta1Schema) if err != nil { t.Fatal(err) } @@ -110,7 +97,7 @@ func TestStructuralRoundtrip(t *testing.T) { } if !reflect.DeepEqual(orig, s) { - t.Fatalf("original and result differ: %v", diff.ObjectDiff(orig, s)) + t.Fatalf("original and result differ: %v", diff.ObjectGoPrintDiff(orig, s)) } } } 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 04f0b0aa303..928dc9f8ed1 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 @@ -29,7 +29,7 @@ func NewSchemaValidator(customResourceValidation *apiextensions.CustomResourceVa // Convert CRD schema to openapi schema openapiSchema := &spec.Schema{} if customResourceValidation != nil { - // WARNING: do not replace this with Structural.ToGoOpenAPI until it supports nullable. + // TODO: replace with NewStructural(...).ToGoOpenAPI if err := ConvertJSONSchemaProps(customResourceValidation.OpenAPIV3Schema, openapiSchema); err != nil { return nil, nil, err } @@ -76,9 +76,7 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou 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.Nullable = in.Nullable out.Format = in.Format out.Title = in.Title out.Maximum = in.Maximum 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 e9ca5a61d43..d0d2182eedb 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 @@ -72,7 +72,6 @@ func TestRoundTrip(t *testing.T) { if err := json.Unmarshal(openAPIJSON, &j); err != nil { t.Fatal(err) } - j = convertNullTypeToNullable(j) j = stripIntOrStringType(j) openAPIJSON, err = json.Marshal(j) if err != nil { @@ -97,49 +96,6 @@ func TestRoundTrip(t *testing.T) { } } -func convertNullTypeToNullable(x interface{}) interface{} { - switch x := x.(type) { - case map[string]interface{}: - if t, found := x["type"]; found { - switch t := t.(type) { - case []interface{}: - for i, typ := range t { - if s, ok := typ.(string); !ok || s != "null" { - continue - } - t = append(t[:i], t[i+1:]...) - switch len(t) { - case 0: - delete(x, "type") - case 1: - x["type"] = t[0] - default: - x["type"] = t - } - x["nullable"] = true - break - } - case string: - if t == "null" { - delete(x, "type") - x["nullable"] = true - } - } - } - for k := range x { - x[k] = convertNullTypeToNullable(x[k]) - } - return x - case []interface{}: - for i := range x { - x[i] = convertNullTypeToNullable(x[i]) - } - return x - default: - return x - } -} - func stripIntOrStringType(x interface{}) interface{} { switch x := x.(type) { case map[string]interface{}: