Handle providers of optional services in ubermodule classloader

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

@@ -45,27 +45,41 @@ private ModuleSupport() {
         throw new AssertionError("Utility class, should not be instantiated");
     }
 
-    static ModuleFinder ofSyntheticPluginModule(String name, Path[] jarPaths, Set<String> requires, Set<String> uses) {
+    static ModuleFinder ofSyntheticPluginModule(
+        String name,
+        Path[] jarPaths,
+        Set<String> requires,
+        Set<String> uses,
+        Predicate<String> isPackageInParentLayers
+    ) {
         try {
             return new InMemoryModuleFinder(
-                new InMemoryModuleReference(createModuleDescriptor(name, jarPaths, requires, uses), URI.create("module:/" + name))
+                new InMemoryModuleReference(
+                    createModuleDescriptor(name, jarPaths, requires, uses, isPackageInParentLayers),
+                    URI.create("module:/" + name)
+                )
             );
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
     }
 
     @SuppressForbidden(reason = "need access to the jar file")
-    static ModuleDescriptor createModuleDescriptor(String name, Path[] jarPaths, Set<String> requires, Set<String> uses)
-        throws IOException {
+    static ModuleDescriptor createModuleDescriptor(
+        String name,
+        Path[] jarPaths,
+        Set<String> requires,
+        Set<String> uses,
+        Predicate<String> isPackageInParentLayers
+    ) throws IOException {
         var builder = ModuleDescriptor.newOpenModule(name); // open module, for now
         requires.stream().forEach(builder::requires);
         uses.stream().forEach(builder::uses);
 
         // scan the names of the entries in the JARs
         Set<String> pkgs = new HashSet<>();
         Map<String, List<String>> allBundledProviders = new HashMap<>();
-        Set<String> allBundledServices = new HashSet<>();
+        Set<String> servicesUsedInBundle = new HashSet<>();
         for (Path path : jarPaths) {
             assert path.getFileName().toString().endsWith(".jar") : "expected jars suffix, in path: " + path;
             try (JarFile jf = new JarFile(path.toFile(), true, ZipFile.OPEN_READ, Runtime.version())) {
@@ -74,13 +88,13 @@ static ModuleDescriptor createModuleDescriptor(String name, Path[] jarPaths, Set
                 if (moduleInfo != null) {
                     var descriptor = getDescriptorForModularJar(path);
                     pkgs.addAll(descriptor.packages());
-                    allBundledServices.addAll(descriptor.uses());
+                    servicesUsedInBundle.addAll(descriptor.uses());
                     for (ModuleDescriptor.Provides p : descriptor.provides()) {
                         String serviceName = p.service();
                         List<String> providersInModule = p.providers();
 
                         allBundledProviders.compute(serviceName, (k, v) -> createListOrAppend(v, providersInModule));
-                        allBundledServices.add(serviceName);
+                        servicesUsedInBundle.add(serviceName);
                     }
                 } else {
                     var scan = scan(jf);
@@ -92,21 +106,29 @@ static ModuleDescriptor createModuleDescriptor(String name, Path[] jarPaths, Set
                         List<String> providersInJar = getProvidersFromServiceFile(jf, serviceFileName);
 
                         allBundledProviders.compute(serviceName, (k, v) -> createListOrAppend(v, providersInJar));
-                        allBundledServices.add(serviceName);
+                        servicesUsedInBundle.add(serviceName);
                     }
                 }
             }
         }
 
         builder.packages(pkgs);
 
-        // the module needs to use all services it provides, for the case of internal use
-        allBundledServices.addAll(allBundledProviders.keySet());
-        // but we don't want to add any services we already got from the parent layer
-        allBundledServices.removeAll(uses);
+        // we don't want to add any services we already got from the parent layer
+        servicesUsedInBundle.removeAll(uses);
 
-        allBundledServices.forEach(builder::uses);
-        allBundledProviders.forEach(builder::provides);
+        // Services that aren't exported in the parent layer or defined in our
+        // bundle. This can happen for optional (compile-time) dependencies
+        Set<String> missingServices = servicesUsedInBundle.stream()
+            .filter(s -> isPackageInParentLayers.test(toPackageName(s, ".").orElseThrow()) == false)
+            .filter(s -> pkgs.contains(toPackageName(s, ".").orElseThrow()) == false)
+            .collect(Collectors.toSet());
+
+        servicesUsedInBundle.stream().filter(s -> missingServices.contains(s) == false).forEach(builder::uses);
+        allBundledProviders.entrySet()
+            .stream()
+            .filter(e -> missingServices.contains(e.getKey()) == false)
+            .forEach(e -> builder.provides(e.getKey(), e.getValue()));
         return builder.build();
     }
 
@@ -246,7 +268,7 @@ static boolean isJavaPlatformModule(ModuleDescriptor md) {
     @SuppressForbidden(reason = "need access to the jar file")
     private static List<String> getProvidersFromServiceFile(JarFile jf, String sf) throws IOException {
         try (BufferedReader bf = new BufferedReader(new InputStreamReader(jf.getInputStream(jf.getEntry(sf)), StandardCharsets.UTF_8))) {
-            return bf.lines().toList();
+            return bf.lines().filter(Predicate.not(l -> l.startsWith("#"))).filter(Predicate.not(String::isEmpty)).toList();
         }
     }
 

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

@@ -57,21 +57,16 @@ public class UberModuleClassLoader extends SecureClassLoader implements AutoClos
     private final ModuleLayer.Controller moduleController;
     private final Set<String> packageNames;
 
-    private static final Map<String, Set<String>> platformModulesToServices;
-
-    static {
-        Set<String> unqualifiedExports = ModuleLayer.boot()
-            .modules()
+    private static Map<String, Set<String>> getModuleToServiceMap(ModuleLayer moduleLayer) {
+        Set<String> unqualifiedExports = moduleLayer.modules()
             .stream()
             .flatMap(module -> module.getDescriptor().exports().stream())
             .filter(Predicate.not(ModuleDescriptor.Exports::isQualified))
             .map(ModuleDescriptor.Exports::source)
             .collect(Collectors.toSet());
-        platformModulesToServices = ModuleLayer.boot()
-            .modules()
+        return moduleLayer.modules()
             .stream()
             .map(Module::getDescriptor)
-            .filter(ModuleSupport::isJavaPlatformModule)
             .filter(ModuleSupport::hasAtLeastOneUnqualifiedExport)
             .collect(
                 Collectors.toMap(
@@ -86,27 +81,38 @@ public class UberModuleClassLoader extends SecureClassLoader implements AutoClos
     }
 
     static UberModuleClassLoader getInstance(ClassLoader parent, String moduleName, Set<URL> jarUrls) {
-        return getInstance(parent, moduleName, jarUrls, Set.of());
+        return getInstance(parent, ModuleLayer.boot(), moduleName, jarUrls, Set.of());
     }
 
     @SuppressWarnings("removal")
-    static UberModuleClassLoader getInstance(ClassLoader parent, String moduleName, Set<URL> jarUrls, Set<String> moduleDenyList) {
+    static UberModuleClassLoader getInstance(
+        ClassLoader parent,
+        ModuleLayer parentLayer,
+        String moduleName,
+        Set<URL> jarUrls,
+        Set<String> moduleDenyList
+    ) {
         Path[] jarPaths = jarUrls.stream().map(UberModuleClassLoader::urlToPathUnchecked).toArray(Path[]::new);
-
-        Set<String> requires = platformModulesToServices.keySet()
+        var parentLayerModuleToServiceMap = getModuleToServiceMap(parentLayer);
+        Set<String> requires = parentLayerModuleToServiceMap.keySet()
             .stream()
             .filter(Predicate.not(moduleDenyList::contains))
             .collect(Collectors.toSet());
-        Set<String> uses = platformModulesToServices.entrySet()
+        Set<String> uses = parentLayerModuleToServiceMap.entrySet()
             .stream()
             .filter(Predicate.not(entry -> moduleDenyList.contains(entry.getKey())))
             .flatMap(entry -> entry.getValue().stream())
             .collect(Collectors.toSet());
 
-        ModuleFinder finder = ModuleSupport.ofSyntheticPluginModule(moduleName, jarPaths, requires, uses);
-        ModuleLayer mparent = ModuleLayer.boot();
+        ModuleFinder finder = ModuleSupport.ofSyntheticPluginModule(
+            moduleName,
+            jarPaths,
+            requires,
+            uses,
+            s -> isPackageInLayers(s, parentLayer)
+        );
         // TODO: check that denied modules are not brought as transitive dependencies (or switch to allow-list?)
-        Configuration cf = mparent.configuration().resolve(finder, ModuleFinder.of(), Set.of(moduleName));
+        Configuration cf = parentLayer.configuration().resolve(finder, ModuleFinder.of(), Set.of(moduleName));
 
         Set<String> packageNames = finder.find(moduleName).map(ModuleReference::descriptor).map(ModuleDescriptor::packages).orElseThrow();
 
@@ -115,12 +121,22 @@ static UberModuleClassLoader getInstance(ClassLoader parent, String moduleName,
             moduleName,
             jarUrls.toArray(new URL[0]),
             cf,
-            mparent,
+            parentLayer,
             packageNames
         );
         return AccessController.doPrivileged(pa);
     }
 
+    private static boolean isPackageInLayers(String packageName, ModuleLayer moduleLayer) {
+        if (moduleLayer.modules().stream().map(Module::getPackages).anyMatch(p -> p.contains(packageName))) {
+            return true;
+        }
+        if (moduleLayer.parents().equals(List.of(ModuleLayer.empty()))) {
+            return false;
+        }
+        return moduleLayer.parents().stream().anyMatch(ml -> isPackageInLayers(packageName, ml));
+    }
+
     /**
      * Constructor
      */