Null value serialization

[shawkins]

Prior to 6.6 an unintended consequence of the UnmatchedFieldTypeModule was that null values were suppressed from serialization - even if the pojo lacked a JsonInclude annotation.

This change https://github.com/fabric8io/kubernetes-client/pull/5028/files#diff-6c29389b01c29298ef063e303aac679b95bf2f65273f327f58dbbc5b627221a8R77 preserved the nullSerializer and thus the expected Jackson behavior

If a user has omitted the JsonInclude annotations, they may now run into errors with some operations in 6.6 - such as patch or put with messages like 'Invalid value: "null": spec.events[0] in body must be of type object: "null"'

I'm not entirely clear on when via the crd an explicit null is not allowed - this seems to occur even in places with generated crds that lack the NotNull annotation.

So the questions here are:

• Do we simply recommend to users to update their pojos? This is most inline with Jackson recommendations
• Recommend the user takes responsibility for objectmapper global changes - with the understanding that usage of Serialization may mean there are multiple mappers that need updated.
• Try to more broadly determine when an explicit null is allowable by kubernetes - it may be that we can further tune serialization defaults.

@manusa @metacosm @andreaTP

[vmuzikar]

This almost sounds like a regression, or a breaking change at least (that users suddenly need to explicitly omit null values). Would it be safe to configure the objectmapper to ignore the null values at a global level in some contexts, like e.g. Operator SDK?

[shawkins]

It's not a regression in the sense that it was never intentionally the behavior of the UnmatchedFieldTypeModule to omit all nulls - using fabric8 releases prior to 5.12, the introduction of UnmatchedFieldTypeModule, would have required the use of JsonInclude.

This handling is inline with the Kubernetes recommendation: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#optional-vs-required - in that they recommend the explicit use of omitempty and highlight the very specific case where one would want to have different handling for explicit nil vs when the field is omitted.

However this does seem cumbersome since we do have control over the global handling of the mapper(s). I'm fine if we flip the paradigm - default to omitting null, and document that you can add @JsonInclude(value = Include.ALWAYS) as needed to include nulls. Presumably since users between 5.12 and 6.5 didn't complain about nulls not being included, this is rarely needed. If we do move that direction, then we can treat the 6.6 and 6.7 behavior as regressive.

cc @manusa

[metacosm]

I do agree that it is a regression in observable behavior, however intentional it was in the preceding versions, the fact is that people rely on that behavior. So we do need to provide a clear path for our users to handle this change, and maybe that means reconfiguring the mapper to have the same behavior as previously until such a time (7.0?) that we can break things.