diff --git a/docs/changelog/91869.yaml b/docs/changelog/91869.yaml new file mode 100644 index 0000000000000..1e38cfd9dd6af --- /dev/null +++ b/docs/changelog/91869.yaml @@ -0,0 +1,5 @@ +pr: 91869 +summary: Load stable plugins as synthetic modules +area: Infra/Plugins +type: enhancement +issues: [] diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index 424f58e2d557c..5bad422d8f06a 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -523,12 +523,33 @@ static LayerAndLoader createPlugin( extendedPlugins.stream().map(LoadedPlugin::layer) ).toList(); return createPluginModuleLayer(bundle, pluginParentLoader, parentLayers); + } else if (plugin.isStable()) { + logger.debug(() -> "Loading bundle: " + plugin.getName() + ", non-modular as synthetic module"); + return LayerAndLoader.ofLoader( + UberModuleClassLoader.getInstance( + pluginParentLoader, + ModuleLayer.boot(), + "synthetic." + toModuleName(plugin.getName()), + bundle.allUrls, + Set.of("org.elasticsearch.server") // TODO: instead of denying server, allow only jvm + stable API modules + ) + ); } else { logger.debug(() -> "Loading bundle: " + plugin.getName() + ", non-modular"); return LayerAndLoader.ofLoader(URLClassLoader.newInstance(bundle.urls.toArray(URL[]::new), pluginParentLoader)); } } + // package-visible for testing + static String toModuleName(String name) { + String result = name.replaceAll("\\W+", ".") // replace non-alphanumeric character strings with dots + .replaceAll("(^[^A-Za-z_]*)", "") // trim non-alpha or underscore characters from start + .replaceAll("\\.$", "") // trim trailing dot + .toLowerCase(Locale.getDefault()); + assert ModuleSupport.isPackageName(result); + return result; + } + private static void checkDeprecations( PluginIntrospector inspector, List pluginDescriptors, diff --git a/server/src/main/java/org/elasticsearch/plugins/UberModuleClassLoader.java b/server/src/main/java/org/elasticsearch/plugins/UberModuleClassLoader.java index 0981d30a9225c..5f4bec977bcf1 100644 --- a/server/src/main/java/org/elasticsearch/plugins/UberModuleClassLoader.java +++ b/server/src/main/java/org/elasticsearch/plugins/UberModuleClassLoader.java @@ -280,6 +280,13 @@ protected Class loadClass(String cn, boolean resolve) throws ClassNotFoundExc } } + // For testing in cases where code must be given access to an unnamed module + void addReadsSystemClassLoaderUnnamedModule() { + moduleController.layer() + .modules() + .forEach(module -> moduleController.addReads(module, ClassLoader.getSystemClassLoader().getUnnamedModule())); + } + /** * Returns the package name for the given class name */ diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index d06410ff0ac23..5a75a9c89d452 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -17,6 +17,8 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.index.IndexModule; +import org.elasticsearch.plugin.analysis.api.CharFilterFactory; +import org.elasticsearch.plugins.scanners.PluginInfo; import org.elasticsearch.plugins.spi.BarPlugin; import org.elasticsearch.plugins.spi.BarTestService; import org.elasticsearch.plugins.spi.TestService; @@ -823,6 +825,23 @@ public Reader create(Reader reader) { assertThat(pluginInfos.get(0).descriptor().getName(), equalTo("stable-plugin")); assertThat(pluginInfos.get(0).descriptor().isStable(), is(true)); + // check ubermodule classloader usage + Collection stablePluginInfos = pluginService.getStablePluginRegistry() + .getPluginInfosForExtensible("org.elasticsearch.plugin.analysis.api.CharFilterFactory"); + assertThat(stablePluginInfos, hasSize(1)); + ClassLoader stablePluginClassLoader = stablePluginInfos.stream().findFirst().orElseThrow().loader(); + assertThat(stablePluginClassLoader, instanceOf(UberModuleClassLoader.class)); + + if (CharFilterFactory.class.getModule().isNamed() == false) { + // test frameworks run with stable api classes on classpath, so we + // have no choice but to let our class read the unnamed module that + // owns the stable api classes + ((UberModuleClassLoader) stablePluginClassLoader).addReadsSystemClassLoaderUnnamedModule(); + } + + Class stableClass = stablePluginClassLoader.loadClass("p.A"); + assertThat(stableClass.getModule().getName(), equalTo("synthetic.stable.plugin")); + // TODO should we add something to pluginInfos.get(0).pluginApiInfo() ? } finally { closePluginLoaders(pluginService); @@ -838,6 +857,20 @@ public void testCanCreateAClassLoader() { assertEquals(this.getClass().getClassLoader(), loader.getParent()); } + public void testToModuleName() { + assertThat(PluginsService.toModuleName("module.name"), equalTo("module.name")); + assertThat(PluginsService.toModuleName("module-name"), equalTo("module.name")); + assertThat(PluginsService.toModuleName("module-name1"), equalTo("module.name1")); + assertThat(PluginsService.toModuleName("1module-name"), equalTo("module.name")); + assertThat(PluginsService.toModuleName("module-name!"), equalTo("module.name")); + assertThat(PluginsService.toModuleName("module!@#name!"), equalTo("module.name")); + assertThat(PluginsService.toModuleName("!module-name!"), equalTo("module.name")); + assertThat(PluginsService.toModuleName("module_name"), equalTo("module_name")); + assertThat(PluginsService.toModuleName("-module-name-"), equalTo("module.name")); + assertThat(PluginsService.toModuleName("_module_name"), equalTo("_module_name")); + assertThat(PluginsService.toModuleName("_"), equalTo("_")); + } + static final class Loader extends ClassLoader { Loader(ClassLoader parent) { super(parent);