Align CRD generator behavior with Jackson

[bequinn-hubspot]

Is your enhancement related to a problem? Please describe

The general problem that I'm having is that the CRD generator handles types and field visibility differently than jackson does. I first had an issue with an infinite recursion in #5584, and I'm now having a circular reference issue in a guava class. Both of these issues are handled correctly by the Jackson ObjectMapper.

Describe the solution you'd like

I think the proper solution to this problem is to leverage Jackson itself to do a lot of the heavy lifting of mapping the schema.

An example of this approach is in this other json schema library https://github.com/almson/mbknor-jackson-jsonSchema-java. It leverages the ObjectMapper#acceptJsonFormatVisitor() to ensure that it maps the types that same way that the ObjectMapper does. Funnily enough, that method specifically mentions that it can be used for json schema generation.

With this approach we can be pretty confident that the schema will stay more precisely in line with the jackson representation of the object, while respecting any modules and annotations that are applied to the ObjectMapper being used. Jackson also handles a lot of weird edge cases of traversing POJOs for the purpose of serialization / mapping.

Describe alternatives you've considered

I think right now the recommended solution is to use @SchemaSwap's to handle the badly behaving fields.

Additional context

Something to mention in addition to this is that a general issue with the CRD generator is that it is designed to serialized private fields, instead of getters. The ObjectMapper that is provided by kubernetes-client, on the other hand, uses the standard Jackson behavior of looking at public methods only, so I suspect there would be many places where these schemas don't align with the serialized objects.

[shawkins]

I think the proper solution to this problem is to leverage Jackson itself to do a lot of the heavy lifting of mapping the schema.

This has been discussed in the past (see #5324 (comment) and related comments), and unfortunately does not seem like it will happen any time soon.

An example of this approach is in this other json schema library

We didn't evaluate that jar, I'm not sure if it has all of the shortcomings mentioned on the other issue - however we'll want to stick to things that are properly supported by the core jackson project where possible.

[bequinn-hubspot]

Yea, I'm fairly confident the library I provided is a fork of the one you looked at, so it would be the same. I understand this would probably represent a large change, so probably not something to expect in the near future.

Out of curiosity is there any reason why the APT plugin can't just run after the compile step instead of during it? It should be feasible to operate on the compiled code if the maven plugin runs after the compile, correct? Is there any reason in particular that the generator needs to run as an annotation processor, instead of after the compile?

[shawkins]

Out of curiosity is there any reason why the APT plugin can't just run after the compile step instead of during it? It should be feasible to operate on the compiled code if the maven plugin runs after the compile, correct? Is there any reason in particular that the generator needs to run as an annotation processor, instead of after the compile?

Beyond the rework of the existing logic, I believe there were some reasons @metacosm @andreaTP had mentioned to prefer annotation processing - maybe they could chime in here as well.

[andreaTP]

Sorry for the delay, thanks a lot for bearing with us @bequinn-hubspot !

Beyond the rework of the existing logic, I believe there were some reasons @metacosm @andreaTP had mentioned to prefer annotation processing - maybe they could chime in here as well.

I'm not an expert here, but IIRC the current implementation design is driven to be compatible with the Quarkus Extension, and I remember that there are limitations, but I haven't analyzed the problem myself.

I think the proper solution to this problem is to leverage Jackson itself to do a lot of the heavy lifting of mapping the schema.

Probably this cannot be enforced in a Quarkus Extension, also, we have had major troubles in the past attempting to respect the Jackson/Lombok/etc. behavior, hence we decided to opt out and roll a complete set of different Annotations to avoid giving the impression that we are honoring those.

Regarding this issue:

Fully recursive data types cannot be represented in a CRD as the format lacks the expressiveness for doing so, for this specific matter, running before or after Jackson is not relevant.
Instead, they can be freely encoded in Java (I assume that the project you linked is doing so - haven't verified).
The current recommendation is to use SchemaSwap which has a depth field where you can specify "where to cut" the CRD representation.

For the future, if anyone has an interest in the subject it would be great to try out a different design for this codebase and in the extension. Time has passed and limitations from the frameworks might have been smoothed.

[bequinn-hubspot]

Sorry for the delay

No problem at all, thanks for the thoughtful response! That all makes sense to me. Perhaps because it's creating native images the build process is significantly different than normal. I will take a look at the quarkus extension and see if I can glean any insight from that.

For now I was able to resolve my issue by using the @SchemaSwap annotation, which I just verified does resolve my issue.

[andreaTP]

I will take a look at the quarkus extension and see if I can glean any insight from that.

Awesome! Looking forward to the feedback!

For now I was able to resolve my issue by using the @SchemaSwap annotation, which I just verified does resolve my issue.

Nice to hear! Do you think that we can close this issue as you have a viable solution to your issue?