[#10295] Allow using the informer interface, and trace down all types

[ctron]

This change also evaluates implementations of ResourceEventHandler,
which is the corresponding interface for the informer. This allows to
use watchers or informers the same way.

Also it traverses through the type information, adding all required
types to the list of types enabling reflection. If only the direct class
is added, deserialization will fail, unless all the references classes
are manually annotated with @RegisterForReflection.

Signed-off-by: Jens Reimann [email protected]

[gsmet]

/cc @geoand @iocanel

[geoand]

I'll be looking at this tomorrow

[geoand]

Thanks a lot for this @ctron.

Can you add a description of what exactly the type expansion is doing and why it's needed? Perhaps with an example?
I would really like to know what's going on because we would really like to avoid adding too many things to reflection configuration (I'm not saying that's the case here, it's just that I don't understand exactly why all these classes need to be added for reflection).

[geoand]

I think that we'll need a better name for this, it's a little too generic IMHO

[ctron]

Sure, do you have suggestions?

[geoand]

Maybe EXCLUDED_WATCH_CLASSES?

[ctron]

Can you add a description of what exactly the type expansion is doing and why it's needed? Perhaps with an example?

I absolutely understand. Do you want this description in the GitHub comments? Or in the code itself?

[geoand]

I'd say lets start with a GH comment and we'll see if we need anything else :)

[ctron]

The problem is: the kubernetes extension finds the main class, used in the watcher (or informer) and enables this class for the reflection mechanism. So when the kubernetes client pulls in some JSON from the server, it can deserialize the JSON, with that class information. Which is done by Jackson, using reflection.

All attributes that are on this specific class are covered. However, if you inherit fields from another class, or reference some other class, then these attribute cannot be found via reflection. Annotating them with @RegisterForReflection did fix issue.

The "expand" functionality (and I am open for a different name) takes the root class, extracted from the Watcher implementation, and walks through the type hierarchy, and also walks through the types of the the fields and getters, adding all referenced types to the reflection system.

Assuming you have the following structure:

class AbstractHasMetadata implements HasMetadata {
  String kind;
  String apiVersion;
}

class MyCrd extends AbstractHasMetadata {
  MyCrdSpec spec;
}

class MyCrdSpec {
  List<MyCrdConfig> allTheConfigs;
}

class MyCrdConfig {
  String value;
}

void startWatcher() {
  client.crd(MyCrd.class).watch(new Watcher<>() {
    public void onAdd() {}
    public void onRemove() {}
    public void onUpdate() {}
});
}

The extension currently detects MyCrd and registers that. But doesn't care about the other classes in the type hierarchy/context.

So deserialization fails, when it e.g. finds the field "kind" from the abstract base class. Or when it finds the allTheConfigs field.

Now the extensions walks the type hierarchy up, adding the abstract class as well. But it also walks the fields of each type it processes, and finds the type MyCrdSpec and MyCrdConfig.

[geoand]

Now I get it, thanks for the explanation @ctron.

Last question: Instead of doing the manual walking of the hierarchy, did you try producing ReflectiveHierarchyBuildItem for the classes identified for watcher and informer?
I believe it pretty much covers what you did manually

[ctron]

Last question: Instead of doing the manual walking of the hierarchy, did you try producing ReflectiveHierarchyBuildItem for the classes identified for watcher and informer?
I believe it pretty much covers what you did manually

No I didn't, because I wasn't ware of that 😀 … I will take a look and give it a try.

[geoand]

No I didn't, because I wasn't ware of that … I will take a look and give it a try.

Yeah sorry, that's my fault that I didn't bring it to your attention :(

Let me know how it goes

[ctron]

Works as expected. I updated the PR.

[geoand]

LGTM, thanks!