소스 검색

Split plugin loading into two different phases to support entitlements (#116998) (#117215)

This change loads all the modules and creates the module layers for plugins prior to entitlement 
checking during the 2nd phase of bootstrap initialization. This will allow us to know what modules exist 
for both validation and checking prior to actually loading any plugin classes (in a follow up change).

There are now two classes:

    PluginsLoader which does the module loading and layer creation
    PluginsService which uses a PluginsLoader to create the main plugin classes and start the plugins
Jack Conradson 11 달 전
부모
커밋
e45a7b10ad

+ 2 - 2
benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java

@@ -34,6 +34,7 @@ import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType;
 import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
+import org.elasticsearch.plugins.PluginsLoader;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.plugins.ScriptPlugin;
 import org.elasticsearch.script.DocReader;
@@ -76,8 +77,7 @@ public class ScriptScoreBenchmark {
     private final PluginsService pluginsService = new PluginsService(
         Settings.EMPTY,
         null,
-        null,
-        Path.of(System.getProperty("plugins.dir"))
+        new PluginsLoader(null, Path.of(System.getProperty("plugins.dir")))
     );
     private final ScriptModule scriptModule = new ScriptModule(Settings.EMPTY, pluginsService.filterPlugins(ScriptPlugin.class).toList());
 

+ 12 - 0
server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java

@@ -17,6 +17,7 @@ import org.elasticsearch.common.settings.SecureSettings;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.NodeValidationException;
+import org.elasticsearch.plugins.PluginsLoader;
 
 import java.io.PrintStream;
 
@@ -42,6 +43,9 @@ class Bootstrap {
     // the loaded settings for the node, not valid until after phase 2 of initialization
     private final SetOnce<Environment> nodeEnv = new SetOnce<>();
 
+    // loads information about plugins required for entitlements in phase 2, used by plugins service in phase 3
+    private final SetOnce<PluginsLoader> pluginsLoader = new SetOnce<>();
+
     Bootstrap(PrintStream out, PrintStream err, ServerArgs args) {
         this.out = out;
         this.err = err;
@@ -72,6 +76,14 @@ class Bootstrap {
         return nodeEnv.get();
     }
 
+    void setPluginsLoader(PluginsLoader pluginsLoader) {
+        this.pluginsLoader.set(pluginsLoader);
+    }
+
+    PluginsLoader pluginsLoader() {
+        return pluginsLoader.get();
+    }
+
     void exitWithNodeValidationException(NodeValidationException e) {
         Logger logger = LogManager.getLogger(Elasticsearch.class);
         logger.error("node validation exception\n{}", e.getMessage());

+ 5 - 1
server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java

@@ -41,6 +41,7 @@ import org.elasticsearch.monitor.process.ProcessProbe;
 import org.elasticsearch.nativeaccess.NativeAccess;
 import org.elasticsearch.node.Node;
 import org.elasticsearch.node.NodeValidationException;
+import org.elasticsearch.plugins.PluginsLoader;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -199,6 +200,9 @@ class Elasticsearch {
             VectorUtil.class
         );
 
+        // load the plugin Java modules and layers now for use in entitlements
+        bootstrap.setPluginsLoader(new PluginsLoader(nodeEnv.modulesFile(), nodeEnv.pluginsFile()));
+
         if (Boolean.parseBoolean(System.getProperty("es.entitlements.enabled"))) {
             logger.info("Bootstrapping Entitlements");
             EntitlementBootstrap.bootstrap();
@@ -244,7 +248,7 @@ class Elasticsearch {
     private static void initPhase3(Bootstrap bootstrap) throws IOException, NodeValidationException {
         checkLucene();
 
-        Node node = new Node(bootstrap.environment()) {
+        Node node = new Node(bootstrap.environment(), bootstrap.pluginsLoader()) {
             @Override
             protected void validateNodeBeforeAcceptingRequests(
                 final BootstrapContext context,

+ 3 - 2
server/src/main/java/org/elasticsearch/node/Node.java

@@ -73,6 +73,7 @@ import org.elasticsearch.plugins.ClusterCoordinationPlugin;
 import org.elasticsearch.plugins.ClusterPlugin;
 import org.elasticsearch.plugins.MetadataUpgrader;
 import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.plugins.PluginsLoader;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.readiness.ReadinessService;
 import org.elasticsearch.repositories.RepositoriesService;
@@ -196,8 +197,8 @@ public class Node implements Closeable {
      *
      * @param environment         the initial environment for this node, which will be added to by plugins
      */
-    public Node(Environment environment) {
-        this(NodeConstruction.prepareConstruction(environment, new NodeServiceProvider(), true));
+    public Node(Environment environment, PluginsLoader pluginsLoader) {
+        this(NodeConstruction.prepareConstruction(environment, pluginsLoader, new NodeServiceProvider(), true));
     }
 
     /**

+ 5 - 3
server/src/main/java/org/elasticsearch/node/NodeConstruction.java

@@ -165,6 +165,7 @@ import org.elasticsearch.plugins.MetadataUpgrader;
 import org.elasticsearch.plugins.NetworkPlugin;
 import org.elasticsearch.plugins.PersistentTaskPlugin;
 import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.plugins.PluginsLoader;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.plugins.RecoveryPlannerPlugin;
 import org.elasticsearch.plugins.ReloadablePlugin;
@@ -260,6 +261,7 @@ class NodeConstruction {
      */
     static NodeConstruction prepareConstruction(
         Environment initialEnvironment,
+        PluginsLoader pluginsLoader,
         NodeServiceProvider serviceProvider,
         boolean forbidPrivateIndexSettings
     ) {
@@ -267,7 +269,7 @@ class NodeConstruction {
         try {
             NodeConstruction constructor = new NodeConstruction(closeables);
 
-            Settings settings = constructor.createEnvironment(initialEnvironment, serviceProvider);
+            Settings settings = constructor.createEnvironment(initialEnvironment, serviceProvider, pluginsLoader);
             constructor.loadLoggingDataProviders();
             TelemetryProvider telemetryProvider = constructor.createTelemetryProvider(settings);
             ThreadPool threadPool = constructor.createThreadPool(settings, telemetryProvider.getMeterRegistry());
@@ -400,7 +402,7 @@ class NodeConstruction {
         return Optional.of(plugin);
     }
 
-    private Settings createEnvironment(Environment initialEnvironment, NodeServiceProvider serviceProvider) {
+    private Settings createEnvironment(Environment initialEnvironment, NodeServiceProvider serviceProvider, PluginsLoader pluginsLoader) {
         // Pass the node settings to the DeprecationLogger class so that it can have the deprecation.skip_deprecated_settings setting:
         Settings envSettings = initialEnvironment.settings();
         DeprecationLogger.initialize(envSettings);
@@ -473,7 +475,7 @@ class NodeConstruction {
             (e, apmConfig) -> logger.error("failed to delete temporary APM config file [{}], reason: [{}]", apmConfig, e.getMessage())
         );
 
-        pluginsService = serviceProvider.newPluginService(initialEnvironment, envSettings);
+        pluginsService = serviceProvider.newPluginService(initialEnvironment, pluginsLoader);
         modules.bindToInstance(PluginsService.class, pluginsService);
         Settings settings = Node.mergePluginSettings(pluginsService.pluginMap(), envSettings);
 

+ 3 - 2
server/src/main/java/org/elasticsearch/node/NodeServiceProvider.java

@@ -27,6 +27,7 @@ import org.elasticsearch.indices.ExecutorSelector;
 import org.elasticsearch.indices.IndicesService;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.indices.recovery.RecoverySettings;
+import org.elasticsearch.plugins.PluginsLoader;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.readiness.ReadinessService;
 import org.elasticsearch.script.ScriptContext;
@@ -51,9 +52,9 @@ import java.util.function.LongSupplier;
  */
 class NodeServiceProvider {
 
-    PluginsService newPluginService(Environment environment, Settings settings) {
+    PluginsService newPluginService(Environment initialEnvironment, PluginsLoader pluginsLoader) {
         // this creates a PluginsService with an empty list of classpath plugins
-        return new PluginsService(settings, environment.configFile(), environment.modulesFile(), environment.pluginsFile());
+        return new PluginsService(initialEnvironment.settings(), initialEnvironment.configFile(), pluginsLoader);
     }
 
     ScriptService newScriptService(

+ 461 - 0
server/src/main/java/org/elasticsearch/plugins/PluginsLoader.java

@@ -0,0 +1,461 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.plugins;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.elasticsearch.core.PathUtils;
+import org.elasticsearch.core.SuppressForbidden;
+import org.elasticsearch.jdk.JarHell;
+import org.elasticsearch.jdk.ModuleQualifiedExportsService;
+
+import java.io.IOException;
+import java.lang.ModuleLayer.Controller;
+import java.lang.module.Configuration;
+import java.lang.module.ModuleFinder;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.file.Path;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory;
+import static org.elasticsearch.jdk.ModuleQualifiedExportsService.addExportsService;
+import static org.elasticsearch.jdk.ModuleQualifiedExportsService.exposeQualifiedExportsAndOpens;
+
+/**
+ * This class is used to load modules and module layers for each plugin during
+ * node initialization prior to enablement of entitlements. This allows entitlements
+ * to have all the plugin information they need prior to starting.
+ */
+public class PluginsLoader {
+
+    /**
+     * Contains information about the {@link ClassLoader} required to load a plugin
+     */
+    public interface PluginLayer {
+        /**
+         * @return Information about the bundle of jars used in this plugin
+         */
+        PluginBundle pluginBundle();
+
+        /**
+         * @return The {@link ClassLoader} used to instantiate the main class for the plugin
+         */
+        ClassLoader pluginClassLoader();
+    }
+
+    /**
+     * Contains information about the {@link ClassLoader}s and {@link ModuleLayer} required for loading a plugin
+     * @param pluginBundle Information about the bundle of jars used in this plugin
+     * @param pluginClassLoader The {@link ClassLoader} used to instantiate the main class for the plugin
+     * @param spiClassLoader The exported {@link ClassLoader} visible to other Java modules
+     * @param spiModuleLayer The exported {@link ModuleLayer} visible to other Java modules
+     */
+    private record LoadedPluginLayer(
+        PluginBundle pluginBundle,
+        ClassLoader pluginClassLoader,
+        ClassLoader spiClassLoader,
+        ModuleLayer spiModuleLayer
+    ) implements PluginLayer {
+
+        public LoadedPluginLayer {
+            Objects.requireNonNull(pluginBundle);
+            Objects.requireNonNull(pluginClassLoader);
+            Objects.requireNonNull(spiClassLoader);
+            Objects.requireNonNull(spiModuleLayer);
+        }
+    }
+
+    /**
+     * Tuple of module layer and loader.
+     * Modular Plugins have a plugin specific loader and layer.
+     * Non-Modular plugins have a plugin specific loader and the boot layer.
+     */
+    public record LayerAndLoader(ModuleLayer layer, ClassLoader loader) {
+
+        public LayerAndLoader {
+            Objects.requireNonNull(layer);
+            Objects.requireNonNull(loader);
+        }
+
+        public static LayerAndLoader ofLoader(ClassLoader loader) {
+            return new LayerAndLoader(ModuleLayer.boot(), loader);
+        }
+    }
+
+    private static final Logger logger = LogManager.getLogger(PluginsLoader.class);
+    private static final Module serverModule = PluginsLoader.class.getModule();
+
+    private final List<PluginDescriptor> moduleDescriptors;
+    private final List<PluginDescriptor> pluginDescriptors;
+    private final Map<String, LoadedPluginLayer> loadedPluginLayers;
+
+    /**
+     * Constructs a new PluginsLoader
+     *
+     * @param modulesDirectory The directory modules exist in, or null if modules should not be loaded from the filesystem
+     * @param pluginsDirectory The directory plugins exist in, or null if plugins should not be loaded from the filesystem
+     */
+    @SuppressWarnings("this-escape")
+    public PluginsLoader(Path modulesDirectory, Path pluginsDirectory) {
+
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports = new HashMap<>(ModuleQualifiedExportsService.getBootServices());
+        addServerExportsService(qualifiedExports);
+
+        Set<PluginBundle> seenBundles = new LinkedHashSet<>();
+
+        // load (elasticsearch) module layers
+        if (modulesDirectory != null) {
+            try {
+                Set<PluginBundle> modules = PluginsUtils.getModuleBundles(modulesDirectory);
+                moduleDescriptors = modules.stream().map(PluginBundle::pluginDescriptor).toList();
+                seenBundles.addAll(modules);
+            } catch (IOException ex) {
+                throw new IllegalStateException("Unable to initialize modules", ex);
+            }
+        } else {
+            moduleDescriptors = Collections.emptyList();
+        }
+
+        // load plugin layers
+        if (pluginsDirectory != null) {
+            try {
+                // TODO: remove this leniency, but tests bogusly rely on it
+                if (isAccessibleDirectory(pluginsDirectory, logger)) {
+                    PluginsUtils.checkForFailedPluginRemovals(pluginsDirectory);
+                    Set<PluginBundle> plugins = PluginsUtils.getPluginBundles(pluginsDirectory);
+                    pluginDescriptors = plugins.stream().map(PluginBundle::pluginDescriptor).toList();
+                    seenBundles.addAll(plugins);
+                } else {
+                    pluginDescriptors = Collections.emptyList();
+                }
+            } catch (IOException ex) {
+                throw new IllegalStateException("Unable to initialize plugins", ex);
+            }
+        } else {
+            pluginDescriptors = Collections.emptyList();
+        }
+
+        this.loadedPluginLayers = Collections.unmodifiableMap(loadPluginLayers(seenBundles, qualifiedExports));
+    }
+
+    public List<PluginDescriptor> moduleDescriptors() {
+        return moduleDescriptors;
+    }
+
+    public List<PluginDescriptor> pluginDescriptors() {
+        return pluginDescriptors;
+    }
+
+    public Stream<PluginLayer> pluginLayers() {
+        return loadedPluginLayers.values().stream().map(Function.identity());
+    }
+
+    private Map<String, LoadedPluginLayer> loadPluginLayers(
+        Set<PluginBundle> bundles,
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
+    ) {
+        Map<String, LoadedPluginLayer> loaded = new LinkedHashMap<>();
+        Map<String, Set<URL>> transitiveUrls = new HashMap<>();
+        List<PluginBundle> sortedBundles = PluginsUtils.sortBundles(bundles);
+        if (sortedBundles.isEmpty() == false) {
+            Set<URL> systemLoaderURLs = JarHell.parseModulesAndClassPath();
+            for (PluginBundle bundle : sortedBundles) {
+                PluginsUtils.checkBundleJarHell(systemLoaderURLs, bundle, transitiveUrls);
+                loadPluginLayer(bundle, loaded, qualifiedExports);
+            }
+        }
+
+        return loaded;
+    }
+
+    private void loadPluginLayer(
+        PluginBundle bundle,
+        Map<String, LoadedPluginLayer> loaded,
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
+    ) {
+        String name = bundle.plugin.getName();
+        logger.debug(() -> "Loading bundle: " + name);
+
+        PluginsUtils.verifyCompatibility(bundle.plugin);
+
+        // collect the list of extended plugins
+        List<LoadedPluginLayer> extendedPlugins = new ArrayList<>();
+        for (String extendedPluginName : bundle.plugin.getExtendedPlugins()) {
+            LoadedPluginLayer extendedPlugin = loaded.get(extendedPluginName);
+            assert extendedPlugin != null;
+            assert extendedPlugin.spiClassLoader() != null : "All non-classpath plugins should be loaded with a classloader";
+            extendedPlugins.add(extendedPlugin);
+        }
+
+        final ClassLoader parentLoader = ExtendedPluginsClassLoader.create(
+            getClass().getClassLoader(),
+            extendedPlugins.stream().map(LoadedPluginLayer::spiClassLoader).toList()
+        );
+        LayerAndLoader spiLayerAndLoader = null;
+        if (bundle.hasSPI()) {
+            spiLayerAndLoader = createSPI(bundle, parentLoader, extendedPlugins, qualifiedExports);
+        }
+
+        final ClassLoader pluginParentLoader = spiLayerAndLoader == null ? parentLoader : spiLayerAndLoader.loader();
+        final LayerAndLoader pluginLayerAndLoader = createPlugin(
+            bundle,
+            pluginParentLoader,
+            extendedPlugins,
+            spiLayerAndLoader,
+            qualifiedExports
+        );
+        final ClassLoader pluginClassLoader = pluginLayerAndLoader.loader();
+
+        if (spiLayerAndLoader == null) {
+            // use full implementation for plugins extending this one
+            spiLayerAndLoader = pluginLayerAndLoader;
+        }
+
+        loaded.put(name, new LoadedPluginLayer(bundle, pluginClassLoader, spiLayerAndLoader.loader, spiLayerAndLoader.layer));
+    }
+
+    static LayerAndLoader createSPI(
+        PluginBundle bundle,
+        ClassLoader parentLoader,
+        List<LoadedPluginLayer> extendedPlugins,
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
+    ) {
+        final PluginDescriptor plugin = bundle.plugin;
+        if (plugin.getModuleName().isPresent()) {
+            logger.debug(() -> "Loading bundle: " + plugin.getName() + ", creating spi, modular");
+            return createSpiModuleLayer(
+                bundle.spiUrls,
+                parentLoader,
+                extendedPlugins.stream().map(LoadedPluginLayer::spiModuleLayer).toList(),
+                qualifiedExports
+            );
+        } else {
+            logger.debug(() -> "Loading bundle: " + plugin.getName() + ", creating spi, non-modular");
+            return LayerAndLoader.ofLoader(URLClassLoader.newInstance(bundle.spiUrls.toArray(new URL[0]), parentLoader));
+        }
+    }
+
+    static LayerAndLoader createPlugin(
+        PluginBundle bundle,
+        ClassLoader pluginParentLoader,
+        List<LoadedPluginLayer> extendedPlugins,
+        LayerAndLoader spiLayerAndLoader,
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
+    ) {
+        final PluginDescriptor plugin = bundle.plugin;
+        if (plugin.getModuleName().isPresent()) {
+            logger.debug(() -> "Loading bundle: " + plugin.getName() + ", modular");
+            var parentLayers = Stream.concat(
+                Stream.ofNullable(spiLayerAndLoader != null ? spiLayerAndLoader.layer() : null),
+                extendedPlugins.stream().map(LoadedPluginLayer::spiModuleLayer)
+            ).toList();
+            return createPluginModuleLayer(bundle, pluginParentLoader, parentLayers, qualifiedExports);
+        } 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));
+        }
+    }
+
+    static LayerAndLoader createSpiModuleLayer(
+        Set<URL> urls,
+        ClassLoader parentLoader,
+        List<ModuleLayer> parentLayers,
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
+    ) {
+        // assert bundle.plugin.getModuleName().isPresent();
+        return createModuleLayer(
+            null,  // no entry point
+            spiModuleName(urls),
+            urlsToPaths(urls),
+            parentLoader,
+            parentLayers,
+            qualifiedExports
+        );
+    }
+
+    static LayerAndLoader createPluginModuleLayer(
+        PluginBundle bundle,
+        ClassLoader parentLoader,
+        List<ModuleLayer> parentLayers,
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
+    ) {
+        assert bundle.plugin.getModuleName().isPresent();
+        return createModuleLayer(
+            bundle.plugin.getClassname(),
+            bundle.plugin.getModuleName().get(),
+            urlsToPaths(bundle.urls),
+            parentLoader,
+            parentLayers,
+            qualifiedExports
+        );
+    }
+
+    static LayerAndLoader createModuleLayer(
+        String className,
+        String moduleName,
+        Path[] paths,
+        ClassLoader parentLoader,
+        List<ModuleLayer> parentLayers,
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
+    ) {
+        logger.debug(() -> "Loading bundle: creating module layer and loader for module " + moduleName);
+        var finder = ModuleFinder.of(paths);
+
+        var configuration = Configuration.resolveAndBind(
+            ModuleFinder.of(),
+            parentConfigurationOrBoot(parentLayers),
+            finder,
+            Set.of(moduleName)
+        );
+        var controller = privilegedDefineModulesWithOneLoader(configuration, parentLayersOrBoot(parentLayers), parentLoader);
+        var pluginModule = controller.layer().findModule(moduleName).get();
+        ensureEntryPointAccessible(controller, pluginModule, className);
+        // export/open upstream modules to this plugin module
+        exposeQualifiedExportsAndOpens(pluginModule, qualifiedExports);
+        // configure qualified exports/opens to other modules/plugins
+        addPluginExportsServices(qualifiedExports, controller);
+        logger.debug(() -> "Loading bundle: created module layer and loader for module " + moduleName);
+        return new LayerAndLoader(controller.layer(), privilegedFindLoader(controller.layer(), moduleName));
+    }
+
+    /** Determines the module name of the SPI module, given its URL. */
+    static String spiModuleName(Set<URL> spiURLS) {
+        ModuleFinder finder = ModuleFinder.of(urlsToPaths(spiURLS));
+        var mrefs = finder.findAll();
+        assert mrefs.size() == 1 : "Expected a single module, got:" + mrefs;
+        return mrefs.stream().findFirst().get().descriptor().name();
+    }
+
+    // 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;
+    }
+
+    static final String toPackageName(String className) {
+        assert className.endsWith(".") == false;
+        int index = className.lastIndexOf('.');
+        if (index == -1) {
+            throw new IllegalStateException("invalid class name:" + className);
+        }
+        return className.substring(0, index);
+    }
+
+    @SuppressForbidden(reason = "I need to convert URL's to Paths")
+    static final Path[] urlsToPaths(Set<URL> urls) {
+        return urls.stream().map(PluginsLoader::uncheckedToURI).map(PathUtils::get).toArray(Path[]::new);
+    }
+
+    static final URI uncheckedToURI(URL url) {
+        try {
+            return url.toURI();
+        } catch (URISyntaxException e) {
+            throw new AssertionError(new IOException(e));
+        }
+    }
+
+    private static List<Configuration> parentConfigurationOrBoot(List<ModuleLayer> parentLayers) {
+        if (parentLayers == null || parentLayers.isEmpty()) {
+            return List.of(ModuleLayer.boot().configuration());
+        } else {
+            return parentLayers.stream().map(ModuleLayer::configuration).toList();
+        }
+    }
+
+    /** Ensures that the plugins main class (its entry point), if any, is accessible to the server. */
+    private static void ensureEntryPointAccessible(Controller controller, Module pluginModule, String className) {
+        if (className != null) {
+            controller.addOpens(pluginModule, toPackageName(className), serverModule);
+        }
+    }
+
+    @SuppressWarnings("removal")
+    static Controller privilegedDefineModulesWithOneLoader(Configuration cf, List<ModuleLayer> parentLayers, ClassLoader parentLoader) {
+        return AccessController.doPrivileged(
+            (PrivilegedAction<Controller>) () -> ModuleLayer.defineModulesWithOneLoader(cf, parentLayers, parentLoader)
+        );
+    }
+
+    @SuppressWarnings("removal")
+    static ClassLoader privilegedFindLoader(ModuleLayer layer, String name) {
+        return AccessController.doPrivileged((PrivilegedAction<ClassLoader>) () -> layer.findLoader(name));
+    }
+
+    private static List<ModuleLayer> parentLayersOrBoot(List<ModuleLayer> parentLayers) {
+        if (parentLayers == null || parentLayers.isEmpty()) {
+            return List.of(ModuleLayer.boot());
+        } else {
+            return parentLayers;
+        }
+    }
+
+    protected void addServerExportsService(Map<String, List<ModuleQualifiedExportsService>> qualifiedExports) {
+        var exportsService = new ModuleQualifiedExportsService(serverModule) {
+            @Override
+            protected void addExports(String pkg, Module target) {
+                serverModule.addExports(pkg, target);
+            }
+
+            @Override
+            protected void addOpens(String pkg, Module target) {
+                serverModule.addOpens(pkg, target);
+            }
+        };
+        addExportsService(qualifiedExports, exportsService, serverModule.getName());
+    }
+
+    private static void addPluginExportsServices(Map<String, List<ModuleQualifiedExportsService>> qualifiedExports, Controller controller) {
+        for (Module module : controller.layer().modules()) {
+            var exportsService = new ModuleQualifiedExportsService(module) {
+                @Override
+                protected void addExports(String pkg, Module target) {
+                    controller.addExports(module, pkg, target);
+                }
+
+                @Override
+                protected void addOpens(String pkg, Module target) {
+                    controller.addOpens(module, pkg, target);
+                }
+            };
+            addExportsService(qualifiedExports, exportsService, module.getName());
+        }
+    }
+}

+ 39 - 386
server/src/main/java/org/elasticsearch/plugins/PluginsService.java

@@ -23,34 +23,22 @@ import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.set.Sets;
-import org.elasticsearch.core.PathUtils;
-import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.core.Tuple;
-import org.elasticsearch.jdk.JarHell;
-import org.elasticsearch.jdk.ModuleQualifiedExportsService;
 import org.elasticsearch.node.ReportingService;
+import org.elasticsearch.plugins.PluginsLoader.PluginLayer;
 import org.elasticsearch.plugins.scanners.StablePluginsRegistry;
 import org.elasticsearch.plugins.spi.SPIClassIterator;
 
 import java.io.IOException;
-import java.lang.ModuleLayer.Controller;
-import java.lang.module.Configuration;
-import java.lang.module.ModuleFinder;
 import java.lang.reflect.Constructor;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.net.URL;
-import java.net.URLClassLoader;
 import java.nio.file.Path;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -63,10 +51,6 @@ import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory;
-import static org.elasticsearch.jdk.ModuleQualifiedExportsService.addExportsService;
-import static org.elasticsearch.jdk.ModuleQualifiedExportsService.exposeQualifiedExportsAndOpens;
-
 public class PluginsService implements ReportingService<PluginsAndModules> {
 
     public StablePluginsRegistry getStablePluginRegistry() {
@@ -77,33 +61,18 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
      * A loaded plugin is one for which Elasticsearch has successfully constructed an instance of the plugin's class
      * @param descriptor Metadata about the plugin, usually loaded from plugin properties
      * @param instance The constructed instance of the plugin's main class
-     * @param loader   The classloader for the plugin
-     * @param layer   The module layer for the plugin
      */
-    record LoadedPlugin(PluginDescriptor descriptor, Plugin instance, ClassLoader loader, ModuleLayer layer) {
+    record LoadedPlugin(PluginDescriptor descriptor, Plugin instance) {
 
         LoadedPlugin {
             Objects.requireNonNull(descriptor);
             Objects.requireNonNull(instance);
-            Objects.requireNonNull(loader);
-            Objects.requireNonNull(layer);
-        }
-
-        /**
-         * Creates a loaded <i>classpath plugin</i>. A <i>classpath plugin</i> is a plugin loaded
-         * by the system classloader and defined to the unnamed module of the boot layer.
-         */
-        LoadedPlugin(PluginDescriptor descriptor, Plugin instance) {
-            this(descriptor, instance, PluginsService.class.getClassLoader(), ModuleLayer.boot());
         }
     }
 
     private static final Logger logger = LogManager.getLogger(PluginsService.class);
     private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(PluginsService.class);
 
-    private final Settings settings;
-    private final Path configPath;
-
     /**
      * We keep around a list of plugins and modules. The order of
      * this list is that which the plugins and modules were loaded in.
@@ -117,69 +86,32 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
     /**
      * Constructs a new PluginService
      *
-     * @param settings         The settings of the system
-     * @param modulesDirectory The directory modules exist in, or null if modules should not be loaded from the filesystem
-     * @param pluginsDirectory The directory plugins exist in, or null if plugins should not be loaded from the filesystem
+     * @param settings The settings for this node
+     * @param configPath The configuration path for this node
+     * @param pluginsLoader the information required to complete loading of plugins
      */
-    @SuppressWarnings("this-escape")
-    public PluginsService(Settings settings, Path configPath, Path modulesDirectory, Path pluginsDirectory) {
-        this.settings = settings;
-        this.configPath = configPath;
-
-        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports = new HashMap<>(ModuleQualifiedExportsService.getBootServices());
-        addServerExportsService(qualifiedExports);
-
-        Set<PluginBundle> seenBundles = new LinkedHashSet<>();
-
-        // load modules
-        List<PluginDescriptor> modulesList = new ArrayList<>();
-        Set<String> moduleNameList = new HashSet<>();
-        if (modulesDirectory != null) {
-            try {
-                Set<PluginBundle> modules = PluginsUtils.getModuleBundles(modulesDirectory);
-                modules.stream().map(PluginBundle::pluginDescriptor).forEach(m -> {
-                    modulesList.add(m);
-                    moduleNameList.add(m.getName());
-                });
-                seenBundles.addAll(modules);
-            } catch (IOException ex) {
-                throw new IllegalStateException("Unable to initialize modules", ex);
-            }
-        }
+    public PluginsService(Settings settings, Path configPath, PluginsLoader pluginsLoader) {
+        Map<String, LoadedPlugin> loadedPlugins = loadPluginBundles(settings, configPath, pluginsLoader);
 
-        // load plugins
-        List<PluginDescriptor> pluginsList = new ArrayList<>();
-        if (pluginsDirectory != null) {
-            try {
-                // TODO: remove this leniency, but tests bogusly rely on it
-                if (isAccessibleDirectory(pluginsDirectory, logger)) {
-                    PluginsUtils.checkForFailedPluginRemovals(pluginsDirectory);
-                    Set<PluginBundle> plugins = PluginsUtils.getPluginBundles(pluginsDirectory);
-                    plugins.stream().map(PluginBundle::pluginDescriptor).forEach(pluginsList::add);
-                    seenBundles.addAll(plugins);
-                }
-            } catch (IOException ex) {
-                throw new IllegalStateException("Unable to initialize plugins", ex);
-            }
-        }
-
-        LinkedHashMap<String, LoadedPlugin> loadedPlugins = loadBundles(seenBundles, qualifiedExports);
+        var modulesDescriptors = pluginsLoader.moduleDescriptors();
+        var pluginDescriptors = pluginsLoader.pluginDescriptors();
 
         var inspector = PluginIntrospector.getInstance();
-        this.info = new PluginsAndModules(getRuntimeInfos(inspector, pluginsList, loadedPlugins), modulesList);
+        this.info = new PluginsAndModules(getRuntimeInfos(inspector, pluginDescriptors, loadedPlugins), modulesDescriptors);
         this.plugins = List.copyOf(loadedPlugins.values());
 
-        checkDeprecations(inspector, pluginsList, loadedPlugins);
+        checkDeprecations(inspector, pluginDescriptors, loadedPlugins);
 
         checkMandatoryPlugins(
-            pluginsList.stream().map(PluginDescriptor::getName).collect(Collectors.toSet()),
+            pluginDescriptors.stream().map(PluginDescriptor::getName).collect(Collectors.toSet()),
             new HashSet<>(MANDATORY_SETTING.get(settings))
         );
 
         // we don't log jars in lib/ we really shouldn't log modules,
         // but for now: just be transparent so we can debug any potential issues
+        Set<String> moduleNames = new HashSet<>(modulesDescriptors.stream().map(PluginDescriptor::getName).toList());
         for (String name : loadedPlugins.keySet()) {
-            if (moduleNameList.contains(name)) {
+            if (moduleNames.contains(name)) {
                 logger.info("loaded module [{}]", name);
             } else {
                 logger.info("loaded plugin [{}]", name);
@@ -282,23 +214,11 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         return this.plugins;
     }
 
-    private LinkedHashMap<String, LoadedPlugin> loadBundles(
-        Set<PluginBundle> bundles,
-        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
-    ) {
-        LinkedHashMap<String, LoadedPlugin> loaded = new LinkedHashMap<>();
-        Map<String, Set<URL>> transitiveUrls = new HashMap<>();
-        List<PluginBundle> sortedBundles = PluginsUtils.sortBundles(bundles);
-        if (sortedBundles.isEmpty() == false) {
-            Set<URL> systemLoaderURLs = JarHell.parseModulesAndClassPath();
-            for (PluginBundle bundle : sortedBundles) {
-                PluginsUtils.checkBundleJarHell(systemLoaderURLs, bundle, transitiveUrls);
-                loadBundle(bundle, loaded, qualifiedExports);
-            }
-        }
-
-        loadExtensions(loaded.values());
-        return loaded;
+    private Map<String, LoadedPlugin> loadPluginBundles(Settings settings, Path configPath, PluginsLoader pluginsLoader) {
+        Map<String, LoadedPlugin> loadedPlugins = new LinkedHashMap<>();
+        pluginsLoader.pluginLayers().forEach(pl -> loadBundle(pl, loadedPlugins, settings, configPath));
+        loadExtensions(loadedPlugins.values());
+        return loadedPlugins;
     }
 
     // package-private for test visibility
@@ -443,68 +363,43 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         return "constructor for extension [" + extensionClass.getName() + "] of type [" + extensionPointType.getName() + "]";
     }
 
-    private void loadBundle(
-        PluginBundle bundle,
-        Map<String, LoadedPlugin> loaded,
-        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
-    ) {
-        String name = bundle.plugin.getName();
-        logger.debug(() -> "Loading bundle: " + name);
-
-        PluginsUtils.verifyCompatibility(bundle.plugin);
+    private void loadBundle(PluginLayer pluginLayer, Map<String, LoadedPlugin> loadedPlugins, Settings settings, Path configPath) {
+        String name = pluginLayer.pluginBundle().plugin.getName();
+        logger.debug(() -> "Loading plugin bundle: " + name);
 
-        // collect the list of extended plugins
+        // validate the list of extended plugins
         List<LoadedPlugin> extendedPlugins = new ArrayList<>();
-        for (String extendedPluginName : bundle.plugin.getExtendedPlugins()) {
-            LoadedPlugin extendedPlugin = loaded.get(extendedPluginName);
+        for (String extendedPluginName : pluginLayer.pluginBundle().plugin.getExtendedPlugins()) {
+            LoadedPlugin extendedPlugin = loadedPlugins.get(extendedPluginName);
             assert extendedPlugin != null;
             if (ExtensiblePlugin.class.isInstance(extendedPlugin.instance()) == false) {
                 throw new IllegalStateException("Plugin [" + name + "] cannot extend non-extensible plugin [" + extendedPluginName + "]");
             }
-            assert extendedPlugin.loader() != null : "All non-classpath plugins should be loaded with a classloader";
             extendedPlugins.add(extendedPlugin);
             logger.debug(
-                () -> "Loading bundle: " + name + ", ext plugins: " + extendedPlugins.stream().map(lp -> lp.descriptor().getName()).toList()
+                () -> "Loading plugin bundle: "
+                    + name
+                    + ", ext plugins: "
+                    + extendedPlugins.stream().map(lp -> lp.descriptor().getName()).toList()
             );
         }
 
-        final ClassLoader parentLoader = ExtendedPluginsClassLoader.create(
-            getClass().getClassLoader(),
-            extendedPlugins.stream().map(LoadedPlugin::loader).toList()
-        );
-        LayerAndLoader spiLayerAndLoader = null;
-        if (bundle.hasSPI()) {
-            spiLayerAndLoader = createSPI(bundle, parentLoader, extendedPlugins, qualifiedExports);
-        }
-
-        final ClassLoader pluginParentLoader = spiLayerAndLoader == null ? parentLoader : spiLayerAndLoader.loader();
-        final LayerAndLoader pluginLayerAndLoader = createPlugin(
-            bundle,
-            pluginParentLoader,
-            extendedPlugins,
-            spiLayerAndLoader,
-            qualifiedExports
-        );
-        final ClassLoader pluginClassLoader = pluginLayerAndLoader.loader();
-
-        if (spiLayerAndLoader == null) {
-            // use full implementation for plugins extending this one
-            spiLayerAndLoader = pluginLayerAndLoader;
-        }
+        PluginBundle pluginBundle = pluginLayer.pluginBundle();
+        ClassLoader pluginClassLoader = pluginLayer.pluginClassLoader();
 
         // reload SPI with any new services from the plugin
-        reloadLuceneSPI(pluginClassLoader);
+        reloadLuceneSPI(pluginLayer.pluginClassLoader());
 
         ClassLoader cl = Thread.currentThread().getContextClassLoader();
         try {
             // Set context class loader to plugin's class loader so that plugins
             // that have dependencies with their own SPI endpoints have a chance to load
             // and initialize them appropriately.
-            privilegedSetContextClassLoader(pluginClassLoader);
+            privilegedSetContextClassLoader(pluginLayer.pluginClassLoader());
 
             Plugin plugin;
-            if (bundle.pluginDescriptor().isStable()) {
-                stablePluginsRegistry.scanBundleForStablePlugins(bundle, pluginClassLoader);
+            if (pluginBundle.pluginDescriptor().isStable()) {
+                stablePluginsRegistry.scanBundleForStablePlugins(pluginBundle, pluginClassLoader);
                 /*
                 Contrary to old plugins we don't need an instance of the plugin here.
                 Stable plugin register components (like CharFilterFactory) in stable plugin registry, which is then used in AnalysisModule
@@ -514,16 +409,16 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
                 We need to pass a name though so that we can show that a plugin was loaded (via cluster state api)
                 This might need to be revisited once support for settings is added
                  */
-                plugin = new StablePluginPlaceHolder(bundle.plugin.getName());
+                plugin = new StablePluginPlaceHolder(pluginBundle.plugin.getName());
             } else {
 
-                Class<? extends Plugin> pluginClass = loadPluginClass(bundle.plugin.getClassname(), pluginClassLoader);
+                Class<? extends Plugin> pluginClass = loadPluginClass(pluginBundle.plugin.getClassname(), pluginClassLoader);
                 if (pluginClassLoader != pluginClass.getClassLoader()) {
                     throw new IllegalStateException(
                         "Plugin ["
                             + name
                             + "] must reference a class loader local Plugin class ["
-                            + bundle.plugin.getClassname()
+                            + pluginBundle.plugin.getClassname()
                             + "] (class loader ["
                             + pluginClass.getClassLoader()
                             + "])"
@@ -531,75 +426,12 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
                 }
                 plugin = loadPlugin(pluginClass, settings, configPath);
             }
-            loaded.put(name, new LoadedPlugin(bundle.plugin, plugin, spiLayerAndLoader.loader(), spiLayerAndLoader.layer()));
+            loadedPlugins.put(name, new LoadedPlugin(pluginBundle.plugin, plugin));
         } finally {
             privilegedSetContextClassLoader(cl);
         }
     }
 
-    static LayerAndLoader createSPI(
-        PluginBundle bundle,
-        ClassLoader parentLoader,
-        List<LoadedPlugin> extendedPlugins,
-        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
-    ) {
-        final PluginDescriptor plugin = bundle.plugin;
-        if (plugin.getModuleName().isPresent()) {
-            logger.debug(() -> "Loading bundle: " + plugin.getName() + ", creating spi, modular");
-            return createSpiModuleLayer(
-                bundle.spiUrls,
-                parentLoader,
-                extendedPlugins.stream().map(LoadedPlugin::layer).toList(),
-                qualifiedExports
-            );
-        } else {
-            logger.debug(() -> "Loading bundle: " + plugin.getName() + ", creating spi, non-modular");
-            return LayerAndLoader.ofLoader(URLClassLoader.newInstance(bundle.spiUrls.toArray(new URL[0]), parentLoader));
-        }
-    }
-
-    static LayerAndLoader createPlugin(
-        PluginBundle bundle,
-        ClassLoader pluginParentLoader,
-        List<LoadedPlugin> extendedPlugins,
-        LayerAndLoader spiLayerAndLoader,
-        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
-    ) {
-        final PluginDescriptor plugin = bundle.plugin;
-        if (plugin.getModuleName().isPresent()) {
-            logger.debug(() -> "Loading bundle: " + plugin.getName() + ", modular");
-            var parentLayers = Stream.concat(
-                Stream.ofNullable(spiLayerAndLoader != null ? spiLayerAndLoader.layer() : null),
-                extendedPlugins.stream().map(LoadedPlugin::layer)
-            ).toList();
-            return createPluginModuleLayer(bundle, pluginParentLoader, parentLayers, qualifiedExports);
-        } 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<PluginDescriptor> pluginDescriptors,
@@ -706,173 +538,6 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         return plugins().stream().filter(x -> type.isAssignableFrom(x.instance().getClass())).map(p -> ((T) p.instance()));
     }
 
-    static LayerAndLoader createPluginModuleLayer(
-        PluginBundle bundle,
-        ClassLoader parentLoader,
-        List<ModuleLayer> parentLayers,
-        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
-    ) {
-        assert bundle.plugin.getModuleName().isPresent();
-        return createModuleLayer(
-            bundle.plugin.getClassname(),
-            bundle.plugin.getModuleName().get(),
-            urlsToPaths(bundle.urls),
-            parentLoader,
-            parentLayers,
-            qualifiedExports
-        );
-    }
-
-    static final LayerAndLoader createSpiModuleLayer(
-        Set<URL> urls,
-        ClassLoader parentLoader,
-        List<ModuleLayer> parentLayers,
-        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
-    ) {
-        // assert bundle.plugin.getModuleName().isPresent();
-        return createModuleLayer(
-            null,  // no entry point
-            spiModuleName(urls),
-            urlsToPaths(urls),
-            parentLoader,
-            parentLayers,
-            qualifiedExports
-        );
-    }
-
-    private static final Module serverModule = PluginsService.class.getModule();
-
-    static LayerAndLoader createModuleLayer(
-        String className,
-        String moduleName,
-        Path[] paths,
-        ClassLoader parentLoader,
-        List<ModuleLayer> parentLayers,
-        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
-    ) {
-        logger.debug(() -> "Loading bundle: creating module layer and loader for module " + moduleName);
-        var finder = ModuleFinder.of(paths);
-
-        var configuration = Configuration.resolveAndBind(
-            ModuleFinder.of(),
-            parentConfigurationOrBoot(parentLayers),
-            finder,
-            Set.of(moduleName)
-        );
-        var controller = privilegedDefineModulesWithOneLoader(configuration, parentLayersOrBoot(parentLayers), parentLoader);
-        var pluginModule = controller.layer().findModule(moduleName).get();
-        ensureEntryPointAccessible(controller, pluginModule, className);
-        // export/open upstream modules to this plugin module
-        exposeQualifiedExportsAndOpens(pluginModule, qualifiedExports);
-        // configure qualified exports/opens to other modules/plugins
-        addPluginExportsServices(qualifiedExports, controller);
-        logger.debug(() -> "Loading bundle: created module layer and loader for module " + moduleName);
-        return new LayerAndLoader(controller.layer(), privilegedFindLoader(controller.layer(), moduleName));
-    }
-
-    private static List<ModuleLayer> parentLayersOrBoot(List<ModuleLayer> parentLayers) {
-        if (parentLayers == null || parentLayers.isEmpty()) {
-            return List.of(ModuleLayer.boot());
-        } else {
-            return parentLayers;
-        }
-    }
-
-    private static List<Configuration> parentConfigurationOrBoot(List<ModuleLayer> parentLayers) {
-        if (parentLayers == null || parentLayers.isEmpty()) {
-            return List.of(ModuleLayer.boot().configuration());
-        } else {
-            return parentLayers.stream().map(ModuleLayer::configuration).toList();
-        }
-    }
-
-    /** Ensures that the plugins main class (its entry point), if any, is accessible to the server. */
-    private static void ensureEntryPointAccessible(Controller controller, Module pluginModule, String className) {
-        if (className != null) {
-            controller.addOpens(pluginModule, toPackageName(className), serverModule);
-        }
-    }
-
-    protected void addServerExportsService(Map<String, List<ModuleQualifiedExportsService>> qualifiedExports) {
-        final Module serverModule = PluginsService.class.getModule();
-        var exportsService = new ModuleQualifiedExportsService(serverModule) {
-            @Override
-            protected void addExports(String pkg, Module target) {
-                serverModule.addExports(pkg, target);
-            }
-
-            @Override
-            protected void addOpens(String pkg, Module target) {
-                serverModule.addOpens(pkg, target);
-            }
-        };
-        addExportsService(qualifiedExports, exportsService, serverModule.getName());
-    }
-
-    private static void addPluginExportsServices(Map<String, List<ModuleQualifiedExportsService>> qualifiedExports, Controller controller) {
-        for (Module module : controller.layer().modules()) {
-            var exportsService = new ModuleQualifiedExportsService(module) {
-                @Override
-                protected void addExports(String pkg, Module target) {
-                    controller.addExports(module, pkg, target);
-                }
-
-                @Override
-                protected void addOpens(String pkg, Module target) {
-                    controller.addOpens(module, pkg, target);
-                }
-            };
-            addExportsService(qualifiedExports, exportsService, module.getName());
-        }
-    }
-
-    /** Determines the module name of the SPI module, given its URL. */
-    static String spiModuleName(Set<URL> spiURLS) {
-        ModuleFinder finder = ModuleFinder.of(urlsToPaths(spiURLS));
-        var mrefs = finder.findAll();
-        assert mrefs.size() == 1 : "Expected a single module, got:" + mrefs;
-        return mrefs.stream().findFirst().get().descriptor().name();
-    }
-
-    /**
-     * Tuple of module layer and loader.
-     * Modular Plugins have a plugin specific loader and layer.
-     * Non-Modular plugins have a plugin specific loader and the boot layer.
-     */
-    record LayerAndLoader(ModuleLayer layer, ClassLoader loader) {
-
-        LayerAndLoader {
-            Objects.requireNonNull(layer);
-            Objects.requireNonNull(loader);
-        }
-
-        static LayerAndLoader ofLoader(ClassLoader loader) {
-            return new LayerAndLoader(ModuleLayer.boot(), loader);
-        }
-    }
-
-    @SuppressForbidden(reason = "I need to convert URL's to Paths")
-    static final Path[] urlsToPaths(Set<URL> urls) {
-        return urls.stream().map(PluginsService::uncheckedToURI).map(PathUtils::get).toArray(Path[]::new);
-    }
-
-    static final URI uncheckedToURI(URL url) {
-        try {
-            return url.toURI();
-        } catch (URISyntaxException e) {
-            throw new AssertionError(new IOException(e));
-        }
-    }
-
-    static final String toPackageName(String className) {
-        assert className.endsWith(".") == false;
-        int index = className.lastIndexOf('.');
-        if (index == -1) {
-            throw new IllegalStateException("invalid class name:" + className);
-        }
-        return className.substring(0, index);
-    }
-
     @SuppressWarnings("removal")
     private static void privilegedSetContextClassLoader(ClassLoader loader) {
         AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
@@ -880,16 +545,4 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
             return null;
         });
     }
-
-    @SuppressWarnings("removal")
-    static Controller privilegedDefineModulesWithOneLoader(Configuration cf, List<ModuleLayer> parentLayers, ClassLoader parentLoader) {
-        return AccessController.doPrivileged(
-            (PrivilegedAction<Controller>) () -> ModuleLayer.defineModulesWithOneLoader(cf, parentLayers, parentLoader)
-        );
-    }
-
-    @SuppressWarnings("removal")
-    static ClassLoader privilegedFindLoader(ModuleLayer layer, String name) {
-        return AccessController.doPrivileged((PrivilegedAction<ClassLoader>) () -> layer.findLoader(name));
-    }
 }

+ 31 - 0
server/src/test/java/org/elasticsearch/plugins/PluginsLoaderTests.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.plugins;
+
+import org.elasticsearch.test.ESTestCase;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class PluginsLoaderTests extends ESTestCase {
+
+    public void testToModuleName() {
+        assertThat(PluginsLoader.toModuleName("module.name"), equalTo("module.name"));
+        assertThat(PluginsLoader.toModuleName("module-name"), equalTo("module.name"));
+        assertThat(PluginsLoader.toModuleName("module-name1"), equalTo("module.name1"));
+        assertThat(PluginsLoader.toModuleName("1module-name"), equalTo("module.name"));
+        assertThat(PluginsLoader.toModuleName("module-name!"), equalTo("module.name"));
+        assertThat(PluginsLoader.toModuleName("module!@#name!"), equalTo("module.name"));
+        assertThat(PluginsLoader.toModuleName("!module-name!"), equalTo("module.name"));
+        assertThat(PluginsLoader.toModuleName("module_name"), equalTo("module_name"));
+        assertThat(PluginsLoader.toModuleName("-module-name-"), equalTo("module.name"));
+        assertThat(PluginsLoader.toModuleName("_module_name"), equalTo("_module_name"));
+        assertThat(PluginsLoader.toModuleName("_"), equalTo("_"));
+    }
+}

+ 5 - 18
server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java

@@ -66,12 +66,12 @@ public class PluginsServiceTests extends ESTestCase {
     public static class FilterablePlugin extends Plugin implements ScriptPlugin {}
 
     static PluginsService newPluginsService(Settings settings) {
-        return new PluginsService(settings, null, null, TestEnvironment.newEnvironment(settings).pluginsFile()) {
+        return new PluginsService(settings, null, new PluginsLoader(null, TestEnvironment.newEnvironment(settings).pluginsFile()) {
             @Override
             protected void addServerExportsService(Map<String, List<ModuleQualifiedExportsService>> qualifiedExports) {
                 // tests don't run modular
             }
-        };
+        });
     }
 
     static PluginsService newMockPluginsService(List<Class<? extends Plugin>> classpathPlugins) {
@@ -875,20 +875,6 @@ public class PluginsServiceTests extends ESTestCase {
         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);
@@ -896,16 +882,17 @@ public class PluginsServiceTests extends ESTestCase {
     }
 
     // Closes the URLClassLoaders and UberModuleClassloaders of plugins loaded by the given plugin service.
+    // We can use the direct ClassLoader from the plugin because tests do not use any parent SPI ClassLoaders.
     static void closePluginLoaders(PluginsService pluginService) {
         for (var lp : pluginService.plugins()) {
-            if (lp.loader() instanceof URLClassLoader urlClassLoader) {
+            if (lp.instance().getClass().getClassLoader() instanceof URLClassLoader urlClassLoader) {
                 try {
                     PrivilegedOperations.closeURLClassLoader(urlClassLoader);
                 } catch (IOException unexpected) {
                     throw new UncheckedIOException(unexpected);
                 }
             }
-            if (lp.loader() instanceof UberModuleClassLoader loader) {
+            if (lp.instance().getClass().getClassLoader() instanceof UberModuleClassLoader loader) {
                 try {
                     PrivilegedOperations.closeURLClassLoader(loader.getInternalLoader());
                 } catch (Exception e) {

+ 5 - 3
test/framework/src/main/java/org/elasticsearch/node/MockNode.java

@@ -31,6 +31,7 @@ import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.indices.recovery.RecoverySettings;
 import org.elasticsearch.plugins.MockPluginsService;
 import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.plugins.PluginsLoader;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.readiness.MockReadinessService;
 import org.elasticsearch.readiness.ReadinessService;
@@ -279,10 +280,11 @@ public class MockNode extends Node {
         final Collection<Class<? extends Plugin>> classpathPlugins,
         final boolean forbidPrivateIndexSettings
     ) {
-        super(NodeConstruction.prepareConstruction(environment, new MockServiceProvider() {
+        super(NodeConstruction.prepareConstruction(environment, null, new MockServiceProvider() {
+
             @Override
-            PluginsService newPluginService(Environment environment, Settings settings) {
-                return new MockPluginsService(settings, environment, classpathPlugins);
+            PluginsService newPluginService(Environment environment, PluginsLoader pluginsLoader) {
+                return new MockPluginsService(environment.settings(), environment, classpathPlugins);
             }
         }, forbidPrivateIndexSettings));
 

+ 8 - 10
test/framework/src/main/java/org/elasticsearch/plugins/MockPluginsService.java

@@ -20,7 +20,6 @@ import org.elasticsearch.jdk.ModuleQualifiedExportsService;
 import org.elasticsearch.plugins.spi.SPIClassIterator;
 
 import java.lang.reflect.Constructor;
-import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -44,14 +43,18 @@ public class MockPluginsService extends PluginsService {
      * @param classpathPlugins Plugins that exist in the classpath which should be loaded
      */
     public MockPluginsService(Settings settings, Environment environment, Collection<Class<? extends Plugin>> classpathPlugins) {
-        super(settings, environment.configFile(), environment.modulesFile(), environment.pluginsFile());
+        super(settings, environment.configFile(), new PluginsLoader(environment.modulesFile(), environment.pluginsFile()) {
 
-        final Path configPath = environment.configFile();
+            @Override
+            protected void addServerExportsService(Map<String, List<ModuleQualifiedExportsService>> qualifiedExports) {
+                // tests don't run modular
+            }
+        });
 
         List<LoadedPlugin> pluginsLoaded = new ArrayList<>();
 
         for (Class<? extends Plugin> pluginClass : classpathPlugins) {
-            Plugin plugin = loadPlugin(pluginClass, settings, configPath);
+            Plugin plugin = loadPlugin(pluginClass, settings, environment.configFile());
             PluginDescriptor pluginInfo = new PluginDescriptor(
                 pluginClass.getName(),
                 "classpath plugin",
@@ -69,7 +72,7 @@ public class MockPluginsService extends PluginsService {
             if (logger.isTraceEnabled()) {
                 logger.trace("plugin loaded from classpath [{}]", pluginInfo);
             }
-            pluginsLoaded.add(new LoadedPlugin(pluginInfo, plugin, pluginClass.getClassLoader(), ModuleLayer.boot()));
+            pluginsLoaded.add(new LoadedPlugin(pluginInfo, plugin));
         }
         loadExtensions(pluginsLoaded);
         this.classpathPlugins = List.copyOf(pluginsLoaded);
@@ -169,9 +172,4 @@ public class MockPluginsService extends PluginsService {
         }
         return extensions;
     }
-
-    @Override
-    protected void addServerExportsService(Map<String, List<ModuleQualifiedExportsService>> qualifiedExports) {
-        // tests don't run modular
-    }
 }

+ 12 - 10
x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/bench/WatcherScheduleEngineBenchmark.java

@@ -16,11 +16,13 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.core.TimeValue;
+import org.elasticsearch.env.Environment;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.monitor.jvm.JvmInfo;
 import org.elasticsearch.node.InternalSettingsPreparer;
 import org.elasticsearch.node.MockNode;
 import org.elasticsearch.node.Node;
+import org.elasticsearch.plugins.PluginsLoader;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.aggregations.bucket.histogram.Histogram;
@@ -96,18 +98,18 @@ public class WatcherScheduleEngineBenchmark {
         );
         System.out.println("and heap_max=" + JvmInfo.jvmInfo().getMem().getHeapMax());
 
+        Environment internalNodeEnv = InternalSettingsPreparer.prepareEnvironment(
+            Settings.builder().put(SETTINGS).put("node.data", false).build(),
+            emptyMap(),
+            null,
+            () -> {
+                throw new IllegalArgumentException("settings must have [node.name]");
+            }
+        );
+
         // First clean everything and index the watcher (but not via put alert api!)
         try (
-            Node node = new Node(
-                InternalSettingsPreparer.prepareEnvironment(
-                    Settings.builder().put(SETTINGS).put("node.data", false).build(),
-                    emptyMap(),
-                    null,
-                    () -> {
-                        throw new IllegalArgumentException("settings must have [node.name]");
-                    }
-                )
-            ).start()
+            Node node = new Node(internalNodeEnv, new PluginsLoader(internalNodeEnv.modulesFile(), internalNodeEnv.pluginsFile())).start()
         ) {
             final Client client = node.client();
             ClusterHealthResponse response = client.admin().cluster().prepareHealth(TimeValue.THIRTY_SECONDS).setWaitForNodes("2").get();