Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KitodoServiceLoader dynamic class loading is not a plugin system #4548

Open
thomaslow opened this issue Jul 15, 2021 · 4 comments
Open

KitodoServiceLoader dynamic class loading is not a plugin system #4548

thomaslow opened this issue Jul 15, 2021 · 4 comments

Comments

@thomaslow
Copy link
Collaborator

thomaslow commented Jul 15, 2021

Kitodo.Production uses a "hack" for dynamic class loading, which was never supported by Java, even before v9, see KitodoServiceLoader.java. Since Java 9+, this "hack" does no longer work:

The application class loader is no longer an instance of java.net.URLClassLoader (an implementation detail that was never specified in previous releases). Code that assumes that ClassLoader::getSytemClassLoader returns a URLClassLoader object will need to be updated. Note that Java SE and the JDK do not provide an API for applications or libraries to dynamically augment the class path at run-time.
(Source: Java 9 Release Notes)

In my opinion, dynamic loading of classes from some directory poses a (significant) security risk. Therefore, I'd suggest to either:

  • Bundle all modules / plugins in each released war file using regular maven dependencies (easy to maintain and safe because no dynamic loading at all)
  • Use an actual plugin system, e.g. OSGi and Apache Felix, and ideally require a valid digital signature for each plugin

The fix that is provided in pull request #4547 uses a separate class loader to dynamically load classes from jar files. As a consequence, some modules and their classes will not always be visible to other classes loaded using other class loaders. An introduction to this strategy can be found in the Tomcat documentation, which also uses a separate class loader to load webapps. This new class loading strategy generally works with the currently implemented modules. However, in special scenarios class loading will fail, e.g., when there are module inter-dependencies as is the case with kitodo-data-editor and kitodo-data-format. Debugging of these problems will be very nasty if more and more modules are implemented and modules develop hierarchical dependency relations on their own.

Related Issues: #3154

Cheers,
Thomas

@thomaslow thomaslow changed the title KitodoServiceLoader Dynamic Class Loading is not a Plugin System KitodoServiceLoader dynamic class loading is not a plugin system Jul 15, 2021
@henning-gerhardt
Copy link
Collaborator

henning-gerhardt commented Jul 16, 2021

In my opinion moving the modules inside the package bundle is a bad solution as it should be possible to develop and deploy modules from outside into the application f.e. replacing the file management module by a module which is storing all the data into a media repository over HTTP api calls or providing new features as long they are development against the existing API module.

Development of a own plugin system should be avoided if there is a usable plugin system. Signing plugins should is nice to have or only required if there is a way to sign own developed modules to use them in the application.

@thomaslow
Copy link
Collaborator Author

I absolutely agree that there should be a way to introduce new features or exchange certain feature implementations. I also agree that the service pattern (define API, develop various modules for that API) is a reasonable way to do that.

However, I fear that the current strategy of "putting jar files in a directory" is not going to work out in the long run. For example, how do you plan to deal with dependencies and incompatible plugins, say module X needs a library in version v1.5 and module Y needs the same library in version v2.2, which are not compatible?

In my opinion, that is why there is a parent pom in Kitodo.Production that defines common package versions that are shared among all modules. In my mind, maven is the perfect tool to make sure that the final war bundle of several modules and their dependencies are compatible.

Maybe it would be a sensible approach to try to make it easier to create individual custom war bundles of Kitodo.Production instead?

For example, there could be a maven project template that uses the overlays feature of the maven-war-plugin, imports kitodo-core as a war dependency, and contains instructions how to build a custom war file that includes new plugins or changes to the UI, like a custom logo, a new button, an additional input field that is also stored in the database, or something similar?

@henning-gerhardt
Copy link
Collaborator

For example, how do you plan to deal with dependencies and incompatible plugins, say module X needs a library in version v1.5 and module Y needs the same library in version v2.2, which are not compatible?

I have no solution for this. This should be handled by the module / plugin / extension manager or there should be some restrictions.

Maybe it would be a sensible approach to try to make it easier to create individual custom war bundles of Kitodo.Production instead?

Not in my opinion. I don't think that every customer should build its own version of Kitodo.Production.

In my "perfect" world the version of Kitodo.Production and its default modules / plugins / extensions is created by the release management and shared to every one. If you as a customer get an other module / plugin / extension from for example a third party development company because they implemented a feature for you or implemented a default module in an other way then you only replace or add this module / plugin / extension. The application recognize this new / changed module / plugin / extension on next start and ask the administrator of the application what to do with this module / plugin / extension: accept or reject it. Only the core and maybe api of the application could not be replaced by this method.

But this is only my opinion and there are many more developers with their own opinions. Maybe they start sharing their opinion at a point in near future.

@matthias-ronge
Copy link
Collaborator

Hi Thomas! Welcome to Kitodo.Production! 👋

I think that the last sentence of your initial quote points out the problem:

Java SE and the JDK do not provide an API for applications or libraries to dynamically augment the class path at run-time.

Basically, this means that we should rethink the whole idea of the module loader.

The modular loader was introduced as a consequence of the following observations:

  1. You cannot extend the functionality of the Java application without changing the program. Changing it requires an interruption in operation, which causes the Tomcat to loose all user sessions. Changes, for which the user did not click “save”, are lost.
  2. The (legacy) program code was a big ball of mud. The same type of functionality, which differed only in details, was implemented several times.
  3. It should be possible to integrate commercial extensions from third-party providers.

These goals should be maintained or reached by whatever replacement we decide on in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants