Allow qualified exports to extended modules (#95243)

In #94884 the ability to add qualified exports and opens from jars
upstream of server was added. Some Elasticsearch components need to
qualify their exports to another component. This commit tweaks the
loading of the exports services so that each loaded plugin/component
has their qualified exports handled automatically.

libs/core/src/main/java/org/elasticsearch/jdk/ModuleQualifiedExportsService.java

@@ -32,15 +32,19 @@ import java.util.stream.Stream;
  */
 public abstract class ModuleQualifiedExportsService {
 
-    protected final Module selfModule = getClass().getModule();
-
+    protected final Module module;
     private final Map<String, List<String>> qualifiedExports;
     private final Map<String, List<String>> qualifiedOpens;
     private final Set<String> targets;
 
     protected ModuleQualifiedExportsService() {
-        this.qualifiedExports = invert(selfModule.getDescriptor().exports(), Exports::isQualified, Exports::source, Exports::targets);
-        this.qualifiedOpens = invert(selfModule.getDescriptor().opens(), Opens::isQualified, Opens::source, Opens::targets);
+        this(null);
+    }
+
+    protected ModuleQualifiedExportsService(Module module) {
+        this.module = module == null ? getClass().getModule() : module;
+        this.qualifiedExports = invert(this.module.getDescriptor().exports(), Exports::isQualified, Exports::source, Exports::targets);
+        this.qualifiedOpens = invert(this.module.getDescriptor().opens(), Opens::isQualified, Opens::source, Opens::targets);
         this.targets = Stream.concat(qualifiedExports.keySet().stream(), qualifiedOpens.keySet().stream())
             .collect(Collectors.toUnmodifiableSet());
     }
@@ -62,6 +66,7 @@ public abstract class ModuleQualifiedExportsService {
                 targetsToSources.computeIfAbsent(target, k -> new ArrayList<>()).add(source);
             }
         }
+        targetsToSources.replaceAll((k, v) -> List.copyOf(v));
         return Map.copyOf(targetsToSources);
     }
 
@@ -80,7 +85,7 @@ public abstract class ModuleQualifiedExportsService {
         String targetName = target.getName();
         if (targets.contains(targetName) == false) {
             throw new IllegalArgumentException(
-                "Module " + selfModule.getName() + " does not contain qualified exports or opens for module " + targetName
+                "Module " + module.getName() + " does not contain qualified exports or opens for module " + targetName
             );
         }
         List<String> exports = qualifiedExports.getOrDefault(targetName, List.of());
@@ -96,7 +101,7 @@ public abstract class ModuleQualifiedExportsService {
     /**
      * Add a qualified export. This should always be implemented as follows:
      * <code>
-     *     selfModule.addExports(pkg, target);
+     *     module.addExports(pkg, target);
      * </code>
      */
     protected abstract void addExports(String pkg, Module target);
@@ -104,7 +109,7 @@ public abstract class ModuleQualifiedExportsService {
     /**
      * Add a qualified open. This should always be implemented as follows:
      * <code>
-     *     selfModule.addOpens(pkg, target);
+     *     module.addOpens(pkg, target);
      * </code>
      */
     protected abstract void addOpens(String pkg, Module target);

libs/preallocate/src/main/java/org/elasticsearch/preallocate/PreallocateModuleExportsService.java

@@ -10,13 +10,14 @@ package org.elasticsearch.preallocate;
 import org.elasticsearch.jdk.ModuleQualifiedExportsService;
 
 public class PreallocateModuleExportsService extends ModuleQualifiedExportsService {
+
     @Override
     protected void addExports(String pkg, Module target) {
-        selfModule.addExports(pkg, target);
+        module.addExports(pkg, target);
     }
 
     @Override
     protected void addOpens(String pkg, Module target) {
-        selfModule.addOpens(pkg, target);
+        module.addOpens(pkg, target);
     }
 }

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

@@ -36,7 +36,6 @@ import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.lang.ModuleLayer.Controller;
 import java.lang.module.Configuration;
-import java.lang.module.ModuleDescriptor;
 import java.lang.module.ModuleFinder;
 import java.lang.reflect.Constructor;
 import java.net.URI;
@@ -100,19 +99,6 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
     private static final Logger logger = LogManager.getLogger(PluginsService.class);
     private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(PluginsService.class);
 
-    // TODO: don't load this statically, so it can be tested
-    private static final Map<String, List<ModuleQualifiedExportsService>> qualifiedExports;
-    static {
-        Map<String, List<ModuleQualifiedExportsService>> targetToServices = new HashMap<>();
-        var loader = ServiceLoader.load(ModuleQualifiedExportsService.class);
-        for (var exportService : loader) {
-            for (String targetName : exportService.getTargets()) {
-                targetToServices.computeIfAbsent(targetName, k -> new ArrayList<>()).add(exportService);
-            }
-        }
-        qualifiedExports = Map.copyOf(targetToServices);
-    }
-
     private final Settings settings;
     private final Path configPath;
 
@@ -142,6 +128,10 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         this.settings = settings;
         this.configPath = configPath;
 
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports = new HashMap<>();
+        loadExportsServices(qualifiedExports, PluginsService.class.getClassLoader());
+        addServerExportsService(qualifiedExports);
+
         Set<PluginBundle> seenBundles = new LinkedHashSet<>();
 
         // load modules
@@ -176,7 +166,7 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
             }
         }
 
-        LinkedHashMap<String, LoadedPlugin> loadedPlugins = loadBundles(seenBundles);
+        LinkedHashMap<String, LoadedPlugin> loadedPlugins = loadBundles(seenBundles, qualifiedExports);
 
         var inspector = PluginIntrospector.getInstance();
         this.info = new PluginsAndModules(getRuntimeInfos(inspector, pluginsList, loadedPlugins), modulesList);
@@ -294,14 +284,17 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         return this.plugins;
     }
 
-    private LinkedHashMap<String, LoadedPlugin> loadBundles(Set<PluginBundle> bundles) {
+    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);
         Set<URL> systemLoaderURLs = JarHell.parseModulesAndClassPath();
         for (PluginBundle bundle : sortedBundles) {
             PluginsUtils.checkBundleJarHell(systemLoaderURLs, bundle, transitiveUrls);
-            loadBundle(bundle, loaded);
+            loadBundle(bundle, loaded, qualifiedExports);
         }
 
         loadExtensions(loaded.values());
@@ -429,7 +422,11 @@ 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) {
+    private void loadBundle(
+        PluginBundle bundle,
+        Map<String, LoadedPlugin> loaded,
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
+    ) {
         String name = bundle.plugin.getName();
         logger.debug(() -> "Loading bundle: " + name);
 
@@ -456,11 +453,17 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         );
         LayerAndLoader spiLayerAndLoader = null;
         if (bundle.hasSPI()) {
-            spiLayerAndLoader = createSPI(bundle, parentLoader, extendedPlugins);
+            spiLayerAndLoader = createSPI(bundle, parentLoader, extendedPlugins, qualifiedExports);
         }
 
         final ClassLoader pluginParentLoader = spiLayerAndLoader == null ? parentLoader : spiLayerAndLoader.loader();
-        final LayerAndLoader pluginLayerAndLoader = createPlugin(bundle, pluginParentLoader, extendedPlugins, spiLayerAndLoader);
+        final LayerAndLoader pluginLayerAndLoader = createPlugin(
+            bundle,
+            pluginParentLoader,
+            extendedPlugins,
+            spiLayerAndLoader,
+            qualifiedExports
+        );
         final ClassLoader pluginClassLoader = pluginLayerAndLoader.loader();
 
         if (spiLayerAndLoader == null) {
@@ -477,6 +480,7 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
             // that have dependencies with their own SPI endpoints have a chance to load
             // and initialize them appropriately.
             privilegedSetContextClassLoader(pluginClassLoader);
+
             Plugin plugin;
             if (bundle.pluginDescriptor().isStable()) {
                 stablePluginsRegistry.scanBundleForStablePlugins(bundle, pluginClassLoader);
@@ -512,11 +516,21 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         }
     }
 
-    static LayerAndLoader createSPI(PluginBundle bundle, ClassLoader parentLoader, List<LoadedPlugin> extendedPlugins) {
+    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());
+            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));
@@ -527,7 +541,8 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         PluginBundle bundle,
         ClassLoader pluginParentLoader,
         List<LoadedPlugin> extendedPlugins,
-        LayerAndLoader spiLayerAndLoader
+        LayerAndLoader spiLayerAndLoader,
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports
     ) {
         final PluginDescriptor plugin = bundle.plugin;
         if (plugin.getModuleName().isPresent()) {
@@ -536,7 +551,7 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
                 Stream.ofNullable(spiLayerAndLoader != null ? spiLayerAndLoader.layer() : null),
                 extendedPlugins.stream().map(LoadedPlugin::layer)
             ).toList();
-            return createPluginModuleLayer(bundle, pluginParentLoader, parentLayers);
+            return createPluginModuleLayer(bundle, pluginParentLoader, parentLayers, qualifiedExports);
         } else if (plugin.isStable()) {
             logger.debug(() -> "Loading bundle: " + plugin.getName() + ", non-modular as synthetic module");
             return LayerAndLoader.ofLoader(
@@ -680,25 +695,37 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         return settings -> new PluginsService(settings, environment.configFile(), environment.modulesFile(), environment.pluginsFile());
     }
 
-    static final LayerAndLoader createPluginModuleLayer(PluginBundle bundle, ClassLoader parentLoader, List<ModuleLayer> parentLayers) {
+    static final 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
+            parentLayers,
+            qualifiedExports
         );
     }
 
-    static final LayerAndLoader createSpiModuleLayer(Set<URL> urls, ClassLoader parentLoader, List<ModuleLayer> parentLayers) {
+    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
+            parentLayers,
+            qualifiedExports
         );
     }
 
@@ -709,7 +736,8 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         String moduleName,
         Path[] paths,
         ClassLoader parentLoader,
-        List<ModuleLayer> parentLayers
+        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);
@@ -723,7 +751,10 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
         var controller = privilegedDefineModulesWithOneLoader(configuration, parentLayersOrBoot(parentLayers), parentLoader);
         var pluginModule = controller.layer().findModule(moduleName).get();
         ensureEntryPointAccessible(controller, pluginModule, className);
-        addQualifiedExportsAndOpens(pluginModule);
+        // 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));
     }
@@ -752,35 +783,63 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
     }
 
     /**
-     * Adds qualified exports and opens declared in the server module descriptor to the target module.
+     * Adds qualified exports and opens declared in other upstream modules to the target module.
      * This is required since qualified statements targeting yet-to-be-created modules, i.e. plugins,
      * are silently dropped when the boot layer is created.
      */
-    private static void addQualifiedExportsAndOpens(Module target) {
-        // TODO: implement server module exports with ModuleQualifiedExportsService
-        serverModule.getDescriptor()
-            .exports()
-            .stream()
-            .filter(ModuleDescriptor.Exports::isQualified)
-            .filter(exports -> exports.targets().contains(target.getName()))
-            .forEach(exports -> serverModule.addExports(exports.source(), target));
-        serverModule.getDescriptor()
-            .opens()
-            .stream()
-            .filter(ModuleDescriptor.Opens::isQualified)
-            .filter(opens -> opens.targets().contains(target.getName()))
-            .forEach(opens -> serverModule.addOpens(opens.source(), target));
-
+    private static void exposeQualifiedExportsAndOpens(Module target, Map<String, List<ModuleQualifiedExportsService>> qualifiedExports) {
         qualifiedExports.getOrDefault(target.getName(), List.of()).forEach(exportService -> exportService.addExportsAndOpens(target));
     }
 
-    /**
-     * Adds qualified opens declared in the server module descriptor to the target module.
-     * This is required since qualified opens targeting yet-to-be-created modules, i.e. plugins,
-     * are silently dropped when the boot layer is created.
-     */
-    private static void addQualifiedOpens(Module target) {
+    private static void loadExportsServices(Map<String, List<ModuleQualifiedExportsService>> qualifiedExports, ClassLoader classLoader) {
+        var loader = ServiceLoader.load(ModuleQualifiedExportsService.class, classLoader);
+        for (var exportsService : loader) {
+            addExportsService(qualifiedExports, exportsService, exportsService.getClass().getModule().getName());
+        }
+    }
 
+    private static void addExportsService(
+        Map<String, List<ModuleQualifiedExportsService>> qualifiedExports,
+        ModuleQualifiedExportsService exportsService,
+        String moduleName
+    ) {
+        for (String targetName : exportsService.getTargets()) {
+            logger.debug("Registered qualified export from module " + moduleName + " to " + targetName);
+            qualifiedExports.computeIfAbsent(targetName, k -> new ArrayList<>()).add(exportsService);
+        }
+    }
+
+    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. */

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

@@ -17,6 +17,7 @@ import org.elasticsearch.core.Strings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.index.IndexModule;
+import org.elasticsearch.jdk.ModuleQualifiedExportsService;
 import org.elasticsearch.plugin.analysis.CharFilterFactory;
 import org.elasticsearch.plugins.scanners.PluginInfo;
 import org.elasticsearch.plugins.spi.BarPlugin;
@@ -63,7 +64,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, 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) {

test/framework/src/main/java/org/elasticsearch/plugins/MockPluginsService.java

@@ -14,6 +14,7 @@ import org.elasticsearch.Version;
 import org.elasticsearch.action.admin.cluster.node.info.PluginsAndModules;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.jdk.ModuleQualifiedExportsService;
 import org.elasticsearch.plugins.spi.SPIClassIterator;
 
 import java.lang.reflect.Constructor;
@@ -23,6 +24,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 public class MockPluginsService extends PluginsService {
@@ -129,4 +131,9 @@ public class MockPluginsService extends PluginsService {
         }
         return extensions;
     }
+
+    @Override
+    protected void addServerExportsService(Map<String, List<ModuleQualifiedExportsService>> qualifiedExports) {
+        // tests don't run modular
+    }
 }

x-pack/plugin/core/src/main/java/module-info.java

@@ -213,6 +213,4 @@ module org.elasticsearch.xcore {
     exports org.elasticsearch.xpack.core.watcher.trigger;
     exports org.elasticsearch.xpack.core.watcher.watch;
     exports org.elasticsearch.xpack.core.watcher;
-
-    opens org.elasticsearch.license.internal; // spi
 }