using jandex for kubernetesdeserializer

[shawkins]

Description

Addresses #3923

The classes will be discovered by the jandex index rather than by package.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change
• Chore (non-breaking change which doesn't affect codebase;
  test, version modification, documentation, etc.)

Checklist

• Code contributed by me aligns with current project license: Apache 2.0
• I Added CHANGELOG entry regarding this change
• I have implemented unit tests to cover my changes
• I have added/updated the javadocs and other documentation accordingly
• No new bugs, code smells, etc. in SonarCloud report
• I tested my code in Kubernetes
• I tested my code in OpenShift

[shawkins]

My guess at ways forward here are:

• at some place in the osgi feature make the models explicit requirements
• leave in a serviceloader based mechanism, such that class discovery delegated to that service's classloader.

[iocanel]

Caused by: org.apache.felix.resolver.reason.ReasonException: Unable to resolve io.fabric8.kubernetes-model-core/6.0.0.SNAPSHOT: missing requirement [io.fabric8.kubernetes-model-core/6.0.0.SNAPSHOT] osgi.wiring.package; filter:="(&(osgi.wiring.package=org.jboss.jandex)(version>=2.4.0)(!(version>=3.0.0)))"

It seems that kubernetes-model-core requires org.jboss.jandex. If jandex is needed at runtime then you just need to added to the feature. If not the kubernetes-mode-core should not import it (maybe add it to the ignore list).

TBH, I am not a big fan of introducing jandex as a runtime dependency. So, if this is the case I think we might want to reconsider.

[shawkins]

TBH, I am not a big fan of introducing jandex as a runtime dependency. So, if this is the case I think we might want to reconsider.

This is something that we talked about in the last call. It makes sense to use jandex as the indexes are already being built. Requiring the manual creation of a class that registers all of the type classes is not desirable.

[shawkins]

It seems that kubernetes-model-core requires org.jboss.jandex

Right I had assumed that was taken care of automatically. The next issue is that we are including the jandex files in the bundles at META-INF/jandex.idx - however that is only for use within the bundle and there doesn't seem to be a simple way to export that.

[iocanel]

TBH, I am not a big fan of introducing jandex as a runtime dependency. So, if this is the case I think we might want to reconsider.

This is something that we talked about in the last call. It makes sense to use jandex as the indexes are already being built. Requiring the manual creation of a class that registers all of the type classes is not desirable.

Why not use the SPI ?

[shawkins]

Separated the resolving changes into #4039 - once that is committed we'll revisit this for the usage of jandex.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

81.0% Coverage
0.0% Duplication

[shawkins]

@iocanel @manusa here's what it looks like now:

1. the KubernetesDeserializer and the thread context classloader will be scanned for jandex.index files, any that are found will be processed - this is sufficient in any flat classloading environment to find everything. However in OSGI we'll see only the index for the kuberentes-model-core.

2. For OSGI to handle extensions we service load KubernetesMappingResourceProvider off of those same classloaders to obtain the respective classloader for the module to look for the jandex index, and to call the KubernetesMappingResourceProvider method to directly supply classes (not currently used).

3. For OSGI to find classes in built-in modules, like kubernetes-model-apps, there is a list of classes (not packages) in the KubernetesDeserializer to serve as a way to get the classloader for a given module so that we can find the jandex index. This revision is kind of an in-between solution. Obviously you could take these changes further to introduce a KubernetesMappingResourceProvider into each built-in module - it was just faster for me to get things into this state.

What else is out there:

• xxx.properties, which is created by manifest.vm. These files seem to match closely to what was in the extension KubernetesMappingResourceProviders. This has the same problem as looking for resource files in general with OSGI - it at least would need to be moved to a package, instead of the root (although I haven't seen that work - even with a valid package exported, I don't see the resources). It would also need a consistent name, instead of the module specific one it currently has. And it is missing types - it does not contain list types (which could be worked around pretty easily by convention) and just a cursory scan there's other things missing as well. For example Binding, Status, and *Options are not listed in core.properties, but all are resolvable via the jandex logic or the legacy logic that matched kind to class name - not that I can think of scenario where wouldn't have wanted to parse DeleteOptions. For those not using the model generator it is harder to use than building a jandex index.

• kube-shema.json - already included in the bundles for projects built by a generator. Looks to be smaller than the jandex index and can be parsed with just jackson. However even when I export the schema package from a module, this still doesn't seem to be visible to the KubernetesDeserializer class loaders from getResources("schema/kube-schema.json"). For those not using the model generator this is a non-starter.

• KubeSchema.java - omitted from the bundles. If it were present at a consistent location we could use reflection to infer all top level types.

[iocanel]

Is there any way to remove the jandex dependency from runtime?

[shawkins]

Is there any way to remove the jandex dependency from runtime?

@iocanel unfortunately there is not as we need to read the index to find the classes.

[shawkins]

closing in favor of #4511