JSONSchemaProps does not support properties of type boolean with default value

[bbeaudreault]

Here is a valid CRD spec:

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: widgets.test.com
spec:
  group: test.com
  names:
    kind: Widget
    listKind: WidgetList
    plural: widgets
    singular: widget
  scope: Namespaced
  version: v1
  versions:
    - name: v1
      served: true
      storage: true
  preserveUnknownFields: false
  subresources:
    status: {}
  validation:
    openAPIV3Schema:
      type: object
      properties:
        spec:
          type: object
          properties:
            edges:
              type: integer
            hollow:
              type: boolean
              default: false
            dimensions:
              type: object
              properties:
                x:
                  type: integer
                y:
                  type: integer
                zoom:
                  type: boolean
                  default: false
            edgeTemplates:
              type: array
              items:
                type: object
                properties:
                  spiky:
                    type: boolean
                    default: true

This is accepted by kubernetes server version 1.15.10. However, this is unparseable via fabric8 CustomResourceDefinition.class:

String yaml = "insert above";
Serialization.unmarshal(yaml, CustomResourceDefinition.class);

This fails with the following MismatchedInputException:

Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `io.fabric8.kubernetes.api.model.apiextensions.JSON` (although at least one Creator exists): no boolean/Boolean-argument constructor/factory method to deserialize from boolean value (false)
 at [Source: (BufferedInputStream); line: 32, column: 24] (through reference chain: io.fabric8.kubernetes.api.model.apiextensions.CustomResourceDefinition["spec"]->io.fabric8.kubernetes.api.model.apiextensions.CustomResourceDefinitionSpec["validation"]->io.fabric8.kubernetes.api.model.apiextensions.CustomResourceValidation["openAPIV3Schema"]->io.fabric8.kubernetes.api.model.apiextensions.JSONSchemaProps["properties"]->java.util.LinkedHashMap["spec"]->io.fabric8.kubernetes.api.model.apiextensions.JSONSchemaProps["properties"]->java.util.LinkedHashMap["hollow"]->io.fabric8.kubernetes.api.model.apiextensions.JSONSchemaProps["default"])
	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1343)
	at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1032)
	at com.fasterxml.jackson.databind.deser.ValueInstantiator.createFromBoolean(ValueInstantiator.java:280)
	at com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.createFromBoolean(StdValueInstantiator.java:393)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromBoolean(BeanDeserializerBase.java:1426)

If you remove the default: false from hollow and zoom, it works. It seems like the default: true in spiky does work. Note: the true/false values don't matter. It appears that the parsing works within nested arrays of objects, but not nested objects. Not sure why that is.

[bbeaudreault]

I believe the only reason it works for the array type is that the deserialization is putting the type and properties fields into the JSONSchemaPropsOrArray#additionalProperties, which is just a Map<String, Object> so it's more flexible than JSONSchemaProps. I feel like that's broken in a different way.

[bbeaudreault]

Actually, I think this problem extends to more than just boolean. I tried adding a default to an integer field, and saw a similar issue. Even for a string field, it works, but if I re-serialize it as YAML it gets improperly formatted. I.E.

properties:
  foo:
    type: string
    default: bar

turns into

properties:
  foo:
    type: string
    default:
      Raw: "bar"

[rohanKanojia]

Umm, strange. I remember we fixed something like this in #1565 .

[bbeaudreault]

I think it's due to the underlying go structs in https://github.com/kubernetes/kubernetes/blob/8436dde049fb055e2c58c85f3f1dec5c06c36399/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go#L29. I'm guessing this issue will cause improper serialization for all of the fields marked JSON, which I'm noticing enum falls into as well.

[rohanKanojia]

let me take a close look into this whenever I get time

[bbeaudreault]

I've been looking further into this by reading the original PR into kubernetes for custom resources support. I think at the very least what we have missing here is a custom deserializer for JSON type. Conveniently, they summarized the JSON handling here: kubernetes/kubernetes#47263 (comment)

Note specifically the custom marshal/unmarshal code for this type here: https://github.com/kubernetes/kubernetes/pull/47263/files#diff-3fe7a62396a7708e034adc5c0bbff3fdR121-R134.

Currently we are relying upon default jackson de/serialization, which is not handling this well.

I actually wonder if we should simply replace JSON with JsonNode. Alternatively, the "proper" way to do this from a jackson perspective would probably be to use JsonSubTypes keyed off the type field. So each data type would get its own implementation of JSONSchemaProps which has a properly typed default value. That seems like more work and pushing us further from the code generation model, so perhaps playing with JsonNode would be preferable. I'm not set up to test that, however.

Another alternative would be to annotate the JSON class with @JsonDeserialize and @JsonSerialize, which do effectively what the marshal/unmarshal code I linked to does.

[bbeaudreault]

@rohanKanojia are you amenable to the above JsonNode solution? I was able to get a test case up and it solved for both default and enum. See https://github.com/HubSpot/kubernetes-client/compare/master...HubSpot:fix_JSON?expand=1

If you are open to this solution I can try to spend a little more time creating an actual junit test for it.

[rohanKanojia]

Great, Thanks. Feel free to send a PR. I haven't started looking into this.

[bbeaudreault]

I've submitted #2151 for this