CRD generation fails when compiled using java version 19 and above

[matteriben]

Describe the bug

CRD generation fails when compiled using java version 19 and above.

Fabric8 Kubernetes Client version

SNAPSHOT

Steps to reproduce

There is an example project here, CRD generation succeeds when built with java version 18 and fails when built with java version 19.

https://github.com/matteriben/crd-generator-date-time-reproducer/actions/runs/8583665846

job-logs.txt

Expected behavior

CRD generation is expected to succeed when project is built with java version 19 and above.

Runtime

other (please specify in additional context)

Kubernetes API Server version

1.25.3@latest

Environment

Linux

Fabric8 Kubernetes Client Logs

No response

Additional context

This seems to be related to a change in the implementation of java.time.ZonedDateTime from version 18 to 19 and above.

[baloo42]

With a recent version of the maven-compiler-plugin and proper configuration it works (at least on my machine):

matteriben/crd-generator-date-time-reproducer#1

[justSchilling]

Your fix does no longer work when your change to java version 8 is edited back to 19. But 18 works, just like matteriben described.

I'm getting the same problem with 21, plus some added information for the stack overflow cause:
this line repeats until failure
WARN io.fabric8.crd.generator.AbstractJsonSchema -- Property 'lastRulesCache' with 'java.util.concurrent.ConcurrentMap<java.lang.Integer,java.time.zone.ZoneOffsetTransition[]>' key type is mapped to 'string' because of CRD schemas limitations

• not sure if that's underlying error for 19 as well

[andreaTP]

Just to let you know, I'm looking into this, I'm able to replicate with Java 19+ (using 21 ATM) and is likely a bug.

[andreaTP]

As mentioned I started looking at the issue, and can be easily replicated, this is my testing branch:
https://github.com/fabric8io/kubernetes-client/compare/main...andreaTP:crd-gen-stackoverflow?expand=1

Ideally, to completely close this we should:

• add a CI pipeline to check against Java 21( current error maybe @manusa can point out to something ...)
• add the mentioned reproducer to the tests and get them to pass

The underlying issue, as far as I understand it:

• in Java 19 they added a field/accessor/something to ZonedDateTime in the Java std lib
• the new field detected by sundrio involves classes with circular dependencies
• I have been unable to use SchemaSwap but I haven't investigated enough what's the issue

I'm not 100% what would be the correct solution in this case; even if we get SchemaSwap to work as expected, the users will be unable to cross-compile with Java 17/21 as the SchemaSwap will be unmatched in Java 17.

One possible solution would be to have a short list of "default SchemaSwap" that will not throw if not matched.
But I'm looking for feedback from @shawkins on this.

[andreaTP]

Quick update, I'm slowly progressing on getting the CI to run on Java 17 and 21 in #5874 .
It's a background task for me, so, if anyone wants to, I'm happy to hand over the effort.

[shawkins]

One possible solution would be to have a short list of "default SchemaSwap" that will not throw if not matched.
But I'm looking for feedback from @shawkins on this.

Sorry for the delay in feedback. Here's my observations:

• the cycle detection is correct, but the error message is not that helpful since it just repeats the name of the type twice. It would be better to see the cycle.

• SchemaSwap / SchemaFrom will work, but it should be done at a higher level, for example:

@SchemaFrom(type = Double.class)
ZonedDateTime timestamp;

It's highly unlikely that you directly want the full serialization of ZonedDateTime as detected by the crd generator. Our builtin kubernetes serialization logic will produce a number like 1712771904.746000000 for a ZonedDateTime, so the above just maps it to Double.

Adding a default set of SchemaSwaps that includes ZonedDateTime is possible, we probably need a way to make that extensible.

Alternatively we'd have to to detect what handling default jackson / KubernetesSerialization does - and assume that it must be what is desired for the crd / runtime. The difference here vs when we've looked at doing that in the past is that this is a built-in class, so we can work with the actual java Class at this time. Of course there are Jackson annotations or ObjectMapper settings that can tweak the output for ZonedDateTime and other temporal classes in a way that this logic would not be aware of. The logic would look roughly like - try to do a Class.forName on the type, if it can be loaded, then use the jackson jsonSchema logic to get the schema and use that instead of our logic - this could easily be restricted to just java / javax classes.