JarHell: Check the parent classloader rather than the system classloader

[pickypg]

Currently, JarHell checks the system classloader to find issues with jar collisions.

In order to support OSGi environments for testing embedded nodes, it's effectively a requirement that the parent classloader be checked instead.

Pros:

• The ability to embed ES into an OSGi environment, generally for testing capabilities in integration tests.

Cons:

• We currently do not support OSGi environments, which is why we do not test against them. Doing this type of check explicitly
• Embedding nodes is not a recommended or supported in any environment.
• This introduces the potential for unexpected issues "just" for the sake of supporting OSGi.
• Supporting Java 9's jigsaw modularity may require different semantics.

[rmuir]

There isn't a way to check an arbitrary classloader. If its a URLClassLoader, then you can, but this is not the case anymore since jigsaw. That's why java.class.path is used: it works everywhere when elasticsearch is run the way we run it (which is not OSGI).

Adding a special hook for running in an OSGI container, e.g. an instanceof URLClassLoader might seem like a good idea, but then we lose important checks for everyone on java 8 that happen in the parsing itself (these detect e.g. stale shell scripts that add empty classpath elements, which convert to surprising locations).

trying to be even smarter and conditionally do something "only when its a URLClassLoader and we are in a container" is fairly sneaky, but might work. But we have to add this "container detection" logic (which likely needs special privileges for CL.getSystemClassLoader call) during bootstrap and expose it in BootStrapInfo... and somehow know that all this is working properly when we don't test running inside any containers. And there is really no guarantee containers are using URLClassLoaders.

[s1monw]

there is also a setting in 2.2 that allows you to disable jarhell checks for testing tests.jarhell.check=false this might be the better solution for testing?

[pickypg]

@s1monw It ends up being bypassed when testing with plugins because the plugin manager bypasses the check.

[rjernst]

@pickypg Not if you construct a Node the new way. In master that would be using the Node constructor directly which takes plugin classes, or on 2.x using MockNode (as our tests do).

[s1monw]

@pickypg is this solving you issue, can we close?

[jasontedor]

We do not support embedded Elasticsearch nodes.