Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Nov 20, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ebcbf83
add class plugin loader
jdconrad Nov 13, 2024
632a832
plumb plugins loader into plugins service
jdconrad Nov 13, 2024
a197032
clean up extraneous methods
jdconrad Nov 13, 2024
ee4ce9c
removed settings and config path from plugins loader
jdconrad Nov 18, 2024
8480e75
fix mocks and remove extraneous loader info from loaded plugins
jdconrad Nov 18, 2024
bbc61d1
fix tests
jdconrad Nov 18, 2024
0e7e157
Merge branch 'main' into pluginloader
jdconrad Nov 19, 2024
1ca520c
Merge branch 'main' into pluginloader
jdconrad Nov 20, 2024
ac55388
changes based on pr comments
jdconrad Nov 20, 2024
427da67
response to pr comments
jdconrad Nov 20, 2024
5e7712f
Merge branch 'main' into pluginloader
jdconrad Nov 20, 2024
0f724a2
change to list to foreach
jdconrad Nov 20, 2024
c1f5993
Merge branch 'main' into pluginloader
jdconrad Nov 20, 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
4 changes: 2 additions & 2 deletions benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
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;
Expand Down Expand Up @@ -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")))
Copy link
Member

Choose a reason for hiding this comment

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

nit: Could this pass an empty directory for module dir? Then we could enforce non-null paths. We needed to support null before because the mock plugins service needed to call its super, but I think we could get around that with the mock loader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's do this as a follow up PR as part of adding MockPluginsLoader potentially.

);
private final ScriptModule scriptModule = new ScriptModule(Settings.EMPTY, pluginsService.filterPlugins(ScriptPlugin.class).toList());

Expand Down
12 changes: 12 additions & 0 deletions server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.env.Environment;
import org.elasticsearch.node.NodeValidationException;
import org.elasticsearch.plugins.PluginsLoader;

import java.io.PrintStream;

Expand All @@ -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;
Expand Down Expand Up @@ -72,6 +76,14 @@ Environment environment() {
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());
Expand Down
6 changes: 5 additions & 1 deletion server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
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;
Expand Down Expand Up @@ -199,6 +200,9 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException {
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();
Expand Down Expand Up @@ -244,7 +248,7 @@ private static void ensureInitialized(Class<?>... classes) {
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,
Expand Down
5 changes: 3 additions & 2 deletions server/src/main/java/org/elasticsearch/node/Node.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
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;
Expand Down Expand Up @@ -180,8 +181,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));
}

/**
Expand Down
8 changes: 5 additions & 3 deletions server/src/main/java/org/elasticsearch/node/NodeConstruction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@
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;
Expand Down Expand Up @@ -260,14 +261,15 @@ class NodeConstruction {
*/
static NodeConstruction prepareConstruction(
Environment initialEnvironment,
PluginsLoader pluginsLoader,
NodeServiceProvider serviceProvider,
boolean forbidPrivateIndexSettings
) {
List<Closeable> closeables = new ArrayList<>();
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());
Expand Down Expand Up @@ -400,7 +402,7 @@ private static <T> Optional<T> getSinglePlugin(Stream<T> plugins, Class<T> plugi
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);
Expand Down Expand Up @@ -473,7 +475,7 @@ private Settings createEnvironment(Environment initialEnvironment, NodeServicePr
(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);

Expand Down
5 changes: 3 additions & 2 deletions server/src/main/java/org/elasticsearch/node/NodeServiceProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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;
Expand All @@ -51,9 +52,9 @@
*/
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(
Expand Down
Loading