Skip to content
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ed6d1d8
Non-modular plugins with synthetic module
ldematte Nov 26, 2024
e5e5ce5
Modularize non-modular plugins: pass multiple layers to UberModuleCla…
ldematte Nov 27, 2024
4c3f232
Fix sql x-pack plugin to include all references as extendedPlugins
ldematte Nov 27, 2024
bdcdb30
Modular spatial plugin
ldematte Nov 27, 2024
d188bec
Handling modules from multiple parent layers (including extensions)
ldematte Nov 27, 2024
374731a
Fix jvm-crash plugin: bypass server ProtectionDomain (UberModuleClass…
ldematte Nov 27, 2024
2eeb099
Fix test + license header
ldematte Nov 27, 2024
a9a768c
Blocklist for apache enum package name
ldematte Nov 27, 2024
241a5b5
Modular gcs plugin (very MAYBE)
ldematte Nov 27, 2024
c3c0127
Revert "Modular gcs plugin (very MAYBE)"
ldematte Nov 27, 2024
2ad48c9
Recursive parent layers
ldematte Nov 27, 2024
daf710c
Merge remote-tracking branch 'upstream/main' into all-plugins-module-…
ldematte Nov 27, 2024
7933ada
Merge remote-tracking branch 'upstream/main' into all-plugins-module-…
ldematte Nov 28, 2024
8707701
Merge remote-tracking branch 'upstream/main' into all-plugins-module-…
ldematte Dec 2, 2024
56c050e
GCS uses via services files
ldematte Dec 2, 2024
298405e
identity-provider uses via services files
ldematte Dec 2, 2024
a5594ea
More org.apache.commons.lang.enum filtering
ldematte Dec 2, 2024
a4d39de
Fix ingest-attachment (TikaImpl ClassLoader match)
ldematte Dec 2, 2024
0f56ab9
spotless
ldematte Dec 2, 2024
990b04d
Merge remote-tracking branch 'upstream/main' into all-plugins-module-…
ldematte Dec 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory;
Expand Down Expand Up @@ -100,8 +101,8 @@ public record LayerAndLoader(ModuleLayer layer, ClassLoader loader) {
Objects.requireNonNull(loader);
}

public static LayerAndLoader ofLoader(ClassLoader loader) {
return new LayerAndLoader(ModuleLayer.boot(), loader);
public static LayerAndLoader ofLoader(UberModuleClassLoader loader) {
return new LayerAndLoader(loader.getLayer(), loader);
}
}

Expand Down Expand Up @@ -254,7 +255,7 @@ static LayerAndLoader createSPI(
);
} else {
logger.debug(() -> "Loading bundle: " + plugin.getName() + ", creating spi, non-modular");
return LayerAndLoader.ofLoader(URLClassLoader.newInstance(bundle.spiUrls.toArray(new URL[0]), parentLoader));
return new LayerAndLoader(ModuleLayer.boot(), URLClassLoader.newInstance(bundle.spiUrls.toArray(new URL[0]), parentLoader));
}
}

Expand All @@ -267,26 +268,39 @@ static LayerAndLoader createPlugin(
) {
final PluginDescriptor plugin = bundle.plugin;
if (plugin.getModuleName().isPresent()) {
logger.debug(() -> "Loading bundle: " + plugin.getName() + ", modular");
logger.info(() -> "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");
logger.info(() -> "Loading bundle: " + plugin.getName() + ", non-modular as synthetic module");
return LayerAndLoader.ofLoader(
UberModuleClassLoader.getInstance(
pluginParentLoader,
ModuleLayer.boot(),
List.of(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));
var syntheticName = "synthetic." + toModuleName2(plugin.getName());
logger.info(() -> "Loading bundle: " + plugin.getName() + ", non-modular, as synthetic module " + syntheticName);
var parentLayers = Stream.concat(
Stream.ofNullable(spiLayerAndLoader != null ? spiLayerAndLoader.layer() : null),
Stream.concat(extendedPlugins.stream().map(LoadedPluginLayer::spiModuleLayer), Stream.of(ModuleLayer.boot()))
).toList();
var parentModuleNames = parentLayers.stream()
.flatMap(l -> l.modules().stream())
.map(Module::getName)
.distinct()
.collect(Collectors.joining(";"));
logger.info("Parent modules for {}: {}", syntheticName, parentModuleNames);
return LayerAndLoader.ofLoader(
UberModuleClassLoader.getInstance(pluginParentLoader, parentLayers, syntheticName, bundle.allUrls, Set.of())
);
}
}

Expand Down Expand Up @@ -324,7 +338,7 @@ static LayerAndLoader createPluginModuleLayer(
);
}

static LayerAndLoader createModuleLayer(
private static LayerAndLoader createModuleLayer(
String className,
String moduleName,
Path[] paths,
Expand Down Expand Up @@ -370,6 +384,16 @@ static String toModuleName(String name) {
return result;
}

static String toModuleName2(String name) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very hacky, but I did not want to break the existing name schema for stable plugins.
And there is a ml-package-loader loader unfortunately -- ml.package.loader is not a OK module name, as package is a reserved keyword.

String result = name.replace('-', '_')
.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('.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.security.PrivilegedAction;
import java.security.SecureClassLoader;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -58,43 +59,66 @@ public class UberModuleClassLoader extends SecureClassLoader implements AutoClos
private final ModuleLayer.Controller moduleController;
private final Set<String> packageNames;

private static Map<String, Set<String>> getModuleToServiceMap(ModuleLayer moduleLayer) {
Set<String> unqualifiedExports = moduleLayer.modules()
private static Set<String> unqualifiedExportsForLayer(ModuleLayer layer) {
return layer.modules()
.stream()
.flatMap(module -> module.getDescriptor().exports().stream())
.filter(Predicate.not(ModuleDescriptor.Exports::isQualified))
.map(ModuleDescriptor.Exports::source)
.collect(Collectors.toSet());
return moduleLayer.modules()
}

private static Set<ModuleDescriptor> readableModulesForLayer(ModuleLayer layer) {
return layer.modules()
.stream()
.map(Module::getDescriptor)
.filter(ModuleSupport::hasAtLeastOneUnqualifiedExport)
.filter(md -> ModuleSupport.hasAtLeastOneUnqualifiedExport(md) || md.isOpen() || md.isAutomatic())
.collect(Collectors.toSet());
}

private static Map<String, Set<String>> getModuleToServiceMap(List<ModuleLayer> moduleLayers) {
Set<String> allUnqualifiedExports = new HashSet<>();
Set<ModuleDescriptor> allReadableModules = new HashSet<>();
for (var layer : moduleLayers) {
allUnqualifiedExports.addAll(unqualifiedExportsForLayer(layer));
allReadableModules.addAll(readableModulesForLayer(layer));
for (var parentLayer : layer.parents()) {
// ModuleLayer.boot() contains all ES libs too, so we want it to be an explicit parent
if (parentLayer != ModuleLayer.boot()) {
allUnqualifiedExports.addAll(unqualifiedExportsForLayer(parentLayer));
allReadableModules.addAll(readableModulesForLayer(parentLayer));
// TODO: recurse?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to make this fully recursive... maybe?
(right now it does not seem we ever have extensions of extensions, but...)

}
}
}

return allReadableModules.stream()
.collect(
Collectors.toMap(
ModuleDescriptor::name,
md -> md.provides()
.stream()
.map(ModuleDescriptor.Provides::service)
.filter(name -> unqualifiedExports.contains(packageName(name)))
.filter(name -> allUnqualifiedExports.contains(packageName(name)))
.collect(Collectors.toSet())
)
);
}

static UberModuleClassLoader getInstance(ClassLoader parent, String moduleName, Set<URL> jarUrls) {
return getInstance(parent, ModuleLayer.boot(), moduleName, jarUrls, Set.of());
return getInstance(parent, List.of(ModuleLayer.boot()), moduleName, jarUrls, Set.of());
}

@SuppressWarnings("removal")
static UberModuleClassLoader getInstance(
ClassLoader parent,
ModuleLayer parentLayer,
List<ModuleLayer> parentLayers,
String moduleName,
Set<URL> jarUrls,
Set<String> moduleDenyList
) {
Path[] jarPaths = jarUrls.stream().map(UberModuleClassLoader::urlToPathUnchecked).toArray(Path[]::new);
var parentLayerModuleToServiceMap = getModuleToServiceMap(parentLayer);
var parentLayerModuleToServiceMap = getModuleToServiceMap(parentLayers);
Set<String> requires = parentLayerModuleToServiceMap.keySet()
.stream()
.filter(Predicate.not(moduleDenyList::contains))
Expand All @@ -110,10 +134,15 @@ static UberModuleClassLoader getInstance(
jarPaths,
requires,
uses,
s -> isPackageInLayers(s, parentLayer)
s -> isPackageInLayers(s, parentLayers)
);
// TODO: check that denied modules are not brought as transitive dependencies (or switch to allow-list?)
Configuration cf = parentLayer.configuration().resolve(finder, ModuleFinder.of(), Set.of(moduleName));
Configuration cf = Configuration.resolve(
finder,
parentLayers.stream().map(ModuleLayer::configuration).toList(),
ModuleFinder.of(),
Set.of(moduleName)
);

Set<String> packageNames = finder.find(moduleName).map(ModuleReference::descriptor).map(ModuleDescriptor::packages).orElseThrow();

Expand All @@ -122,20 +151,25 @@ static UberModuleClassLoader getInstance(
moduleName,
jarUrls.toArray(new URL[0]),
cf,
parentLayer,
parentLayers,
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))) {
private static boolean isPackageInLayers(String packageName, List<ModuleLayer> moduleLayers) {
if (moduleLayers.stream().flatMap(x -> x.modules().stream()).map(Module::getPackages).anyMatch(p -> p.contains(packageName))) {
return true;
}
if (moduleLayer.parents().equals(List.of(ModuleLayer.empty()))) {
return false;
for (var moduleLayer : moduleLayers) {
if (moduleLayer.parents().equals(List.of(ModuleLayer.empty()))) {
continue;
}
if (moduleLayer.parents().stream().anyMatch(ml -> isPackageInLayers(packageName, List.of(ml)))) {
return true;
}
}
return moduleLayer.parents().stream().anyMatch(ml -> isPackageInLayers(packageName, ml));
return false;
}

/**
Expand All @@ -146,7 +180,7 @@ private UberModuleClassLoader(
String moduleName,
URL[] jarURLs,
Configuration cf,
ModuleLayer mparent,
List<ModuleLayer> mparent,
Set<String> packageNames
) {
super(parent);
Expand All @@ -157,7 +191,7 @@ private UberModuleClassLoader(
// Defining a module layer tells the Java virtual machine about the
// classes that may be loaded from the module, and is what makes the
// Class::getModule call return the name of our ubermodule.
this.moduleController = ModuleLayer.defineModules(cf, List.of(mparent), s -> this);
this.moduleController = ModuleLayer.defineModules(cf, mparent, s -> this);
this.module = this.moduleController.layer().findModule(moduleName).orElseThrow();

this.packageNames = packageNames;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ public class RestJvmCrashAction implements RestHandler {
static {
try {
AccessController.doPrivileged((PrivilegedExceptionAction<?>) () -> {
Class<?> unsafe = Class.forName("sun.misc.Unsafe");
// Bypass UberModuleClassLoader, as server does not have our very dangerous permissions
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shows an interesting side effect of using UberModuleClassLoader: for class loading, the server module/ProtectionDomain gets in the way. So if it does not have permissions (like accessClassInPackage.sun.misc), no matter if we grant them to the plugin, it won't work. But I don't think this is a problem.

Class<?> unsafe = ClassLoader.getSystemClassLoader().loadClass("sun.misc.Unsafe");

FREE_MEMORY = unsafe.getMethod("freeMemory", long.class);
Field f = unsafe.getDeclaredField("theUnsafe");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ grant {
permission java.lang.RuntimePermission "accessDeclaredMembers";
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";
permission java.lang.RuntimePermission "getClassLoader";
};
34 changes: 34 additions & 0 deletions x-pack/plugin/spatial/src/main/java/module-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
module org.elasticsearch.spatial {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go in a separate PR
Also, it probably isn't needed anymore: I did this before I added support for parent layers/dependencies. Now with that, we are able to have a synthetic module referencing another synthetic module (works for esql/esql-core for example)

requires org.apache.lucene.spatial3d;
requires org.elasticsearch.h3;
requires org.elasticsearch.legacy.geo;
requires org.elasticsearch.painless.spi;
requires org.elasticsearch.xcore;
requires org.elasticsearch.server;
requires org.elasticsearch.xcontent;
requires org.elasticsearch.base;
requires org.elasticsearch.geo;
requires org.apache.lucene.core;

exports org.elasticsearch.xpack.spatial.action to org.elasticsearch.server;
exports org.elasticsearch.xpack.spatial.common; // to vector-tile
exports org.elasticsearch.xpack.spatial; // to vector-tile
exports org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid; // to vector-tile

exports org.elasticsearch.xpack.spatial.index.query;
exports org.elasticsearch.xpack.spatial.index.fielddata;
exports org.elasticsearch.xpack.spatial.index.fielddata.plain;
exports org.elasticsearch.xpack.spatial.index.mapper;

exports org.elasticsearch.xpack.spatial.ingest;
exports org.elasticsearch.xpack.spatial.script.field;

exports org.elasticsearch.xpack.spatial.search.aggregations;
exports org.elasticsearch.xpack.spatial.search.aggregations.metrics;
exports org.elasticsearch.xpack.spatial.search.aggregations.support;
exports org.elasticsearch.xpack.spatial.search.runtime;

opens org.elasticsearch.xpack.spatial to org.elasticsearch.painless.spi; // whitelist resource access

provides org.elasticsearch.painless.spi.PainlessExtension with org.elasticsearch.xpack.spatial.SpatialPainlessExtension;
}
2 changes: 1 addition & 1 deletion x-pack/plugin/sql/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ esplugin {
name = 'x-pack-sql'
description 'The Elasticsearch plugin that powers SQL for Elasticsearch'
classname = 'org.elasticsearch.xpack.sql.plugin.SqlPlugin'
extendedPlugins = ['x-pack-ql', 'lang-painless']
extendedPlugins = ['x-pack-ql', 'lang-painless', 'x-pack-core']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should no longer be needed (I added recurse to parent layers/dependencies for 1 level), I'll revert this.

}

ext {
Expand Down