Add support in plugins for a separate spi classloader (#76288) (#76364)

SPI is how plugins provide extension points for other plugins to
customize their behavior. Plugins may have a separate SPI jar, so as not
to expose the internals of the plugin implementation. In essense, this
system was built as a stopgap for Java modules, where implementation can
be protected in the same jar. However, currently the plugins service
loads both spi and plugin implementations into the same classloader.
This means that although another plugin may only compile against spi,
they could still get jar hell from a duplicate dependency.

This commit adds support for plugins to contain an "spi" subdirectory
which is loaded into a separate classloader. This spi classloader is a
parent to the plugin implementation, as well as any other plugins which
have extended it. Additionally as a demonstration of how this works in
practice, lang-painless is setup as the first plugin to provide an spi
jar, and sql's dependence on painless is changed to only spi, so that
it now has an independent version of antlr.

closes #74448

Author: rjernst

modules/lang-painless/build.gradle

@@ -22,14 +22,19 @@ testClusters.all {
   systemProperty 'es.transport.cname_in_publish_address', 'true'
 }
 
+configurations {
+  spi
+  compileOnlyApi.extendsFrom(spi)
+}
+
 dependencies {
   api 'org.antlr:antlr4-runtime:4.5.3'
   api 'org.ow2.asm:asm-util:7.2'
   api 'org.ow2.asm:asm-tree:7.2'
   api 'org.ow2.asm:asm-commons:7.2'
   api 'org.ow2.asm:asm-analysis:7.2'
   api 'org.ow2.asm:asm:7.2'
-  api project('spi')
+  spi project('spi')
 }
 
 tasks.named("dependencyLicenses").configure {
@@ -52,7 +57,7 @@ tasks.named("test").configure {
  * Painless plugin */
 tasks.register("apiJavadoc", Javadoc) {
   source = sourceSets.main.allJava
-  classpath = sourceSets.main.runtimeClasspath
+  classpath = sourceSets.main.runtimeClasspath + configurations.spi.classpath
   include '**/org/elasticsearch/painless/api/'
   destinationDir = new File(docsDir, 'apiJavadoc')
 }
@@ -65,6 +70,13 @@ tasks.register("apiJavadocJar", Jar) {
 tasks.named("assemble").configure {
   dependsOn "apiJavadocJar"
 }
+
+tasks.named("bundlePlugin").configure {
+  it.into("spi") {
+    from(configurations.spi)
+  }
+}
+
 /**********************************************
  *           Context API Generation           *
  **********************************************/

modules/lang-painless/spi/build.gradle

@@ -13,6 +13,6 @@ group = 'org.elasticsearch.plugin'
 archivesBaseName = 'elasticsearch-scripting-painless-spi'
 
 dependencies {
-  api project(":server")
+  compileOnly project(":server")
   testImplementation project(":test:framework")
 }

modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessPlugin.java

@@ -200,8 +200,7 @@ public void loadExtensions(ExtensionLoader loader) {
         loader.loadExtensions(PainlessExtension.class).stream()
             .flatMap(extension -> extension.getContextWhitelists().entrySet().stream())
             .forEach(entry -> {
-                List<Whitelist> existing = whitelists.computeIfAbsent(entry.getKey(),
-                    c -> new ArrayList<>(BASE_WHITELISTS));
+                List<Whitelist> existing = whitelists.computeIfAbsent(entry.getKey(), c -> new ArrayList<>());
                 existing.addAll(entry.getValue());
             });
     }

modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.painless.api.txt

@@ -294,6 +294,19 @@ static PluginPolicyInfo readPolicyInfo(Path pluginRoot) throws IOException {
                 }
             }
         }
+        // also add spi jars
+        // TODO: move this to a shared function, or fix plugin layout to have jar files in lib directory
+        Path spiDir = pluginRoot.resolve("spi");
+        if (Files.exists(spiDir)) {
+            try (DirectoryStream<Path> jarStream = Files.newDirectoryStream(spiDir, "*.jar")) {
+                for (Path jar : jarStream) {
+                    URL url = jar.toRealPath().toUri().toURL();
+                    if (jars.add(url) == false) {
+                        throw new IllegalStateException("duplicate module/plugin: " + url);
+                    }
+                }
+            }
+        }
 
         // parse the plugin's policy file into a set of permissions
         Policy policy = readPolicy(policyFile.toUri().toURL(), getCodebaseJarMap(jars));

server/src/main/java/org/elasticsearch/bootstrap/PolicyUtil.java

@@ -275,9 +275,24 @@ public PluginsAndModules info() {
     static class Bundle {
         final PluginInfo plugin;
         final Set<URL> urls;
+        final Set<URL> spiUrls;
+        final Set<URL> allUrls;
 
         Bundle(PluginInfo plugin, Path dir) throws IOException {
             this.plugin = Objects.requireNonNull(plugin);
+
+            Path spiDir = dir.resolve("spi");
+            // plugin has defined an explicit api for extension
+            this.spiUrls = Files.exists(spiDir) ? gatherUrls(spiDir) : null;
+            this.urls = gatherUrls(dir);
+            Set<URL> allUrls = new LinkedHashSet<>(urls);
+            if (spiUrls != null) {
+                allUrls.addAll(spiUrls);
+            }
+            this.allUrls = allUrls;
+        }
+
+        static Set<URL> gatherUrls(Path dir) throws IOException {
             Set<URL> urls = new LinkedHashSet<>();
             // gather urls for jar files
             try (DirectoryStream<Path> jarStream = Files.newDirectoryStream(dir, "*.jar")) {
@@ -289,7 +304,14 @@ static class Bundle {
                     }
                 }
             }
-            this.urls = Objects.requireNonNull(urls);
+            return urls;
+        }
+
+        Set<URL> getExtensionUrls() {
+            if (spiUrls != null) {
+                return spiUrls;
+            }
+            return urls;
         }
 
         @Override
@@ -464,7 +486,7 @@ private static void addSortedBundle(Bundle bundle, Map<String, Bundle> bundles,
 
     private List<Tuple<PluginInfo,Plugin>> loadBundles(Set<Bundle> bundles) {
         List<Tuple<PluginInfo, Plugin>> plugins = new ArrayList<>();
-        Map<String, Plugin> loaded = new HashMap<>();
+        Map<String, Tuple<Plugin, ClassLoader>> loaded = new HashMap<>();
         Map<String, Set<URL>> transitiveUrls = new HashMap<>();
         List<Bundle> sortedBundles = sortBundles(bundles);
         for (Bundle bundle : sortedBundles) {
@@ -570,42 +592,51 @@ static void checkBundleJarHell(Set<URL> classpath, Bundle bundle, Map<String, Se
 
         try {
             final Logger logger = LogManager.getLogger(JarHell.class);
-            Set<URL> urls = new HashSet<>();
+            Set<URL> extendedPluginUrls = new HashSet<>();
             for (String extendedPlugin : exts) {
                 Set<URL> pluginUrls = transitiveUrls.get(extendedPlugin);
                 assert pluginUrls != null : "transitive urls should have already been set for " + extendedPlugin;
 
-                Set<URL> intersection = new HashSet<>(urls);
+                // consistency check: extended plugins should not have duplicate codebases with each other
+                Set<URL> intersection = new HashSet<>(extendedPluginUrls);
                 intersection.retainAll(pluginUrls);
                 if (intersection.isEmpty() == false) {
                     throw new IllegalStateException("jar hell! extended plugins " + exts +
                                                     " have duplicate codebases with each other: " + intersection);
                 }
 
-                intersection = new HashSet<>(bundle.urls);
+                // jar hell check: extended plugins (so far) do not have jar hell with each other
+                extendedPluginUrls.addAll(pluginUrls);
+                JarHell.checkJarHell(extendedPluginUrls, logger::debug);
+
+                // consistency check: each extended plugin should not have duplicate codebases with implementation+spi of this plugin
+                intersection = new HashSet<>(bundle.allUrls);
                 intersection.retainAll(pluginUrls);
                 if (intersection.isEmpty() == false) {
                     throw new IllegalStateException("jar hell! duplicate codebases with extended plugin [" +
-                                                    extendedPlugin + "]: " + intersection);
+                        extendedPlugin + "]: " + intersection);
                 }
 
-                urls.addAll(pluginUrls);
-                JarHell.checkJarHell(urls, logger::debug); // check jarhell as we add each extended plugin's urls
+                // jar hell check: extended plugins (so far) do not have jar hell with implementation+spi of this plugin
+                Set<URL> implementation = new HashSet<>(bundle.allUrls);
+                implementation.addAll(extendedPluginUrls);
+                JarHell.checkJarHell(implementation, logger::debug);
             }
 
-            urls.addAll(bundle.urls);
-            JarHell.checkJarHell(urls, logger::debug); // check jarhell of each extended plugin against this plugin
-            transitiveUrls.put(bundle.plugin.getName(), urls);
+            // Set transitive urls for other plugins to extend this plugin. Note that jarhell has already been checked above.
+            // This uses the extension urls (spi if set) since the implementation will not be in the transitive classpath at runtime.
+            extendedPluginUrls.addAll(bundle.getExtensionUrls());
+            transitiveUrls.put(bundle.plugin.getName(), extendedPluginUrls);
 
             // check we don't have conflicting codebases with core
             Set<URL> intersection = new HashSet<>(classpath);
-            intersection.retainAll(bundle.urls);
+            intersection.retainAll(bundle.allUrls);
             if (intersection.isEmpty() == false) {
                 throw new IllegalStateException("jar hell! duplicate codebases between plugin and core: " + intersection);
             }
             // check we don't have conflicting classes
             Set<URL> union = new HashSet<>(classpath);
-            union.addAll(bundle.urls);
+            union.addAll(bundle.allUrls);
             JarHell.checkJarHell(union, logger::debug);
         } catch (final IllegalStateException ise) {
             throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to jar hell", ise);
@@ -614,25 +645,33 @@ static void checkBundleJarHell(Set<URL> classpath, Bundle bundle, Map<String, Se
         }
     }
 
-    private Plugin loadBundle(Bundle bundle, Map<String, Plugin> loaded) {
+    private Plugin loadBundle(Bundle bundle, Map<String, Tuple<Plugin, ClassLoader>> loaded) {
         String name = bundle.plugin.getName();
 
         verifyCompatibility(bundle.plugin);
 
         // collect loaders of extended plugins
         List<ClassLoader> extendedLoaders = new ArrayList<>();
         for (String extendedPluginName : bundle.plugin.getExtendedPlugins()) {
-            Plugin extendedPlugin = loaded.get(extendedPluginName);
+            Tuple<Plugin, ClassLoader> extendedPlugin = loaded.get(extendedPluginName);
             assert extendedPlugin != null;
-            if (ExtensiblePlugin.class.isInstance(extendedPlugin) == false) {
+            if (ExtensiblePlugin.class.isInstance(extendedPlugin.v1()) == false) {
                 throw new IllegalStateException("Plugin [" + name + "] cannot extend non-extensible plugin [" + extendedPluginName + "]");
             }
-            extendedLoaders.add(extendedPlugin.getClass().getClassLoader());
+            extendedLoaders.add(extendedPlugin.v2());
         }
 
-        // create a child to load the plugin in this bundle
         ClassLoader parentLoader = PluginLoaderIndirection.createLoader(getClass().getClassLoader(), extendedLoaders);
-        ClassLoader loader = URLClassLoader.newInstance(bundle.urls.toArray(new URL[0]), parentLoader);
+        ClassLoader spiLoader = null;
+        if (bundle.spiUrls != null) {
+            spiLoader = URLClassLoader.newInstance(bundle.spiUrls.toArray(new URL[0]), parentLoader);
+        }
+
+        ClassLoader loader = URLClassLoader.newInstance(bundle.urls.toArray(new URL[0]), spiLoader == null ? parentLoader : spiLoader);
+        if (spiLoader == null) {
+            // use full implementation for plugins extending this one
+            spiLoader = loader;
+        }
 
         // reload SPI with any new services from the plugin
         reloadLuceneSPI(loader);
@@ -654,7 +693,7 @@ private Plugin loadBundle(Bundle bundle, Map<String, Plugin> loaded) {
                     + "] (class loader [" + pluginClass.getClassLoader() + "])");
             }
             Plugin plugin = loadPlugin(pluginClass, settings, configPath);
-            loaded.put(name, plugin);
+            loaded.put(name, Tuple.tuple(plugin, spiLoader));
             return plugin;
         } finally {
             AccessController.doPrivileged((PrivilegedAction<Void>) () -> {

server/src/main/java/org/elasticsearch/plugins/PluginsService.java

@@ -614,6 +614,50 @@ public void testJarHellTransitiveMap() throws Exception {
         assertThat(deps, containsInAnyOrder(pluginJar.toUri().toURL(), dep1Jar.toUri().toURL(), dep2Jar.toUri().toURL()));
     }
 
+    public void testJarHellSpiAddedToTransitiveDeps() throws Exception {
+        Path pluginDir = createTempDir();
+        Path pluginJar = pluginDir.resolve("plugin.jar");
+        makeJar(pluginJar, DummyClass2.class);
+        Path spiDir = pluginDir.resolve("spi");
+        Files.createDirectories(spiDir);
+        Path spiJar = spiDir.resolve("spi.jar");
+        makeJar(spiJar, DummyClass3.class);
+        Path depDir = createTempDir();
+        Path depJar = depDir.resolve("dep.jar");
+        makeJar(depJar, DummyClass1.class);
+        Map<String, Set<URL>> transitiveDeps = new HashMap<>();
+        transitiveDeps.put("dep", Collections.singleton(depJar.toUri().toURL()));
+        PluginInfo info1 = new PluginInfo("myplugin", "desc", "1.0", Version.CURRENT, "1.8",
+            "MyPlugin", Collections.singletonList("dep"), false, PluginType.ISOLATED, "", false);
+        PluginsService.Bundle bundle = new PluginsService.Bundle(info1, pluginDir);
+        PluginsService.checkBundleJarHell(JarHell.parseClassPath(), bundle, transitiveDeps);
+        Set<URL> transitive = transitiveDeps.get("myplugin");
+        assertThat(transitive, containsInAnyOrder(spiJar.toUri().toURL(), depJar.toUri().toURL()));
+    }
+
+    public void testJarHellSpiConflict() throws Exception {
+        Path pluginDir = createTempDir();
+        Path pluginJar = pluginDir.resolve("plugin.jar");
+        makeJar(pluginJar, DummyClass2.class);
+        Path spiDir = pluginDir.resolve("spi");
+        Files.createDirectories(spiDir);
+        Path spiJar = spiDir.resolve("spi.jar");
+        makeJar(spiJar, DummyClass1.class);
+        Path depDir = createTempDir();
+        Path depJar = depDir.resolve("dep.jar");
+        makeJar(depJar, DummyClass1.class);
+        Map<String, Set<URL>> transitiveDeps = new HashMap<>();
+        transitiveDeps.put("dep", Collections.singleton(depJar.toUri().toURL()));
+        PluginInfo info1 = new PluginInfo("myplugin", "desc", "1.0", Version.CURRENT, "1.8",
+            "MyPlugin", Collections.singletonList("dep"), false, PluginType.ISOLATED, "", false);
+        PluginsService.Bundle bundle = new PluginsService.Bundle(info1, pluginDir);
+        IllegalStateException e = expectThrows(IllegalStateException.class, () ->
+            PluginsService.checkBundleJarHell(JarHell.parseClassPath(), bundle, transitiveDeps));
+        assertEquals("failed to load plugin myplugin due to jar hell", e.getMessage());
+        assertThat(e.getCause().getMessage(), containsString("jar hell!"));
+        assertThat(e.getCause().getMessage(), containsString("DummyClass1"));
+    }
+
     public void testNonExtensibleDep() throws Exception {
         // This test opens a child classloader, reading a jar under the test temp
         // dir (a dummy plugin). Classloaders are closed by GC, so when test teardown

server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java

@@ -17,9 +17,7 @@ archivesBaseName = 'x-pack-eql'
 
 dependencies {
   compileOnly project(path: xpackModule('core'))
-  compileOnly(project(':modules:lang-painless')) {
-    exclude group: "org.ow2.asm"
-  }
+  compileOnly(project(':modules:lang-painless:spi'))
   api "org.antlr:antlr4-runtime:${antlrVersion}"
   compileOnly project(xpackModule('ql'))
 

x-pack/plugin/eql/build.gradle

@@ -0,0 +1 @@
+2609e36f18f7e8d593cc1cddfb2ac776dc96b8e0

x-pack/plugin/eql/licenses/antlr4-runtime-4.5.3.jar.sha1

@@ -0,0 +1,26 @@
+[The "BSD license"]
+Copyright (c) 2015 Terence Parr, Sam Harwell
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+ 1. Redistributions of source code must retain the above copyright
+    notice, this list of conditions and the following disclaimer.
+ 2. Redistributions in binary form must reproduce the above copyright
+    notice, this list of conditions and the following disclaimer in the
+    documentation and/or other materials provided with the distribution.
+ 3. The name of the author may not be used to endorse or promote products
+    derived from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

x-pack/plugin/eql/licenses/antlr4-runtime-LICENSE.txt

@@ -30,10 +30,7 @@ archivesBaseName = 'x-pack-sql'
 
 dependencies {
   compileOnly project(path: xpackModule('core'))
-  compileOnly(project(':modules:lang-painless')) {
-    // exclude ASM to not affect featureAware task on Java 10+
-    exclude group: "org.ow2.asm"
-  }
+  compileOnly(project(':modules:lang-painless:spi'))
   api project('sql-action')
   api project(':modules:aggs-matrix-stats')
   api "org.antlr:antlr4-runtime:${antlrVersion}"

x-pack/plugin/eql/licenses/antlr4-runtime-NOTICE.txt

@@ -0,0 +1 @@
+2609e36f18f7e8d593cc1cddfb2ac776dc96b8e0

x-pack/plugin/sql/build.gradle

@@ -0,0 +1,26 @@
+[The "BSD license"]
+Copyright (c) 2015 Terence Parr, Sam Harwell
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+ 1. Redistributions of source code must retain the above copyright
+    notice, this list of conditions and the following disclaimer.
+ 2. Redistributions in binary form must reproduce the above copyright
+    notice, this list of conditions and the following disclaimer in the
+    documentation and/or other materials provided with the distribution.
+ 3. The name of the author may not be used to endorse or promote products
+    derived from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.