Enhance Java generation from CRDs to ignore unknown properties

[mikebell90]

Is your enhancement related to a problem? Please describe

We are generating a huge number of Pojos from a set of CRDs. Recently the CRds were updated and added some more properties. While there's argument either way, it would nice to be ignore added properties

aused by: com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: Unrecognized field "numIncludedHosts" (class io.solo.gloo.policy.resilience.v2.TrimProxyConfigPolicyStatus), not marked as ignorable (2 known properties: "common", "numSelectedWorkloads"])
 at [Source: (BufferedInputStream); line: 1, column: 2846] (through reference chain: io.fabric8.kubernetes.api.model.DefaultKubernetesResourceList["items"]->java.util.ArrayList[0]-

As you can see they added numIncludedHosts

Describe the solution you'd like

A way to ignore these, ideally on a targeted basis

Describe alternatives you've considered

No response

Additional context

No response

[andreaTP]

Hi @mikebell90 and thanks for your interest in the java-generator!

I would like you to expand a bit on the use case to make sure I understand it correctly.

You want to deserialize newer versions of the CRs using the code generated from an older CRD.

The desired behavior would be to ignore the additional properties.

Is it correct?

[mikebell90]

Basically yes, though ideally it would be targeted to specific fields or subobjects, since ignoring things can be dangerous! OTOH it's annoying everytime a minor CRD patch comes out that we have to release a whole new controller immediately avoid deserialization errors.

1. Minimal and unwise - Globally ignore. Probably can be done with the object mapper, though last time I checked you guys were rearrranging how that was done.
2. Semi unwise - Ability to specify on a CRD
3. Better - be able to specify on a property of the CRD and its subobjects.
4. Possibly more feasible than #3 - expose some sort of programmatic hooks

[andreaTP]

As I see it on top of my head, probably, restoring the possibility to add additionalProperties (usually marked in the CRD with x-kubernetes-preserve-unknowns) to all of the pojos would be a good compromise to enable this use-case.

Just to spell it out, I think that what we are discussing here is pretty dangerous, and a new version should correspond to a rebuild, but I understand the pain if fast release cycles of the CRDs are happening and automation is not perfect.

Another specific work-around would be to instruct the fabric8 deserializer to ignore unknown fields, which is probably even worse than the former option (and I'll need @shawkins input on how to do it currently).

Thoughts?

[shawkins]

Let's make sure we have a common understanding of what is happening:

• you have CRD rev 1 and a corresponding Java model also rev 1
• the CRD is updated to rev 2 and the Java model is also updated to rev 2

Are you then are trying to deserialize something from rev 2 with the rev 1 object model?

If that's the case then yes additionalProperties is the right way to do this - that will ensure round tripping with the rev 1 objects won't drop anything. Changing the object mapper to simply ignore unknown fields will cause those fields to be dropped if you are doing any updating.

[andreaTP]

Just to state my position here:

• I do believe that the issue reported here should be fixed by releasing new versions and automating the process
• I do acknowledge that "adding a property" is a non-breaking change(in terms of versioning) and the client should not break
• I have removed this option in [chore] Remove deprecated configuration options from java-generator #5001 due to the lack of a good enough excuse to keep it
• I'm sympathetic to supporting this option
• the impact on the codebase is minimal

[manusa]

Aren't conversion hooks supposed to deal specifically with this scenario?

[andreaTP]

Aren't conversion hooks supposed to deal specifically with this scenario?

That's totally correct, and the first suggestion is to automate the version bumps.
At the same time, here, we are, possibly, making the life of the user "easier" for some scenarios, for example:

• a CRD is released adding an extra property - this change is backward compatible - no need for webhooks
• we have a client in the cluster operating on the CRs that have not been updated yet
• we write a CR containing the extra property
• the client in the cluster reads the CR, but, having an unmatched property a Deserialization error is thrown

Please note that the last step is not "expected" by the user who made a backward-compatible change and webhooks are not used in such circumstances.

[manusa]

I'm not against the change (I like it and probably suggested the inclusion of that field some time ago). There are many other situations where this can come in handy

I brought the point because I think it's important to highlight that the recommended and proper approach for this situation is to use conversion hooks.

[andreaTP]

Completely agreed 👍