From cd4e31052e016273d18e41a87f2d59a8d1bd74de Mon Sep 17 00:00:00 2001 From: Othello Maurer Date: Tue, 5 Sep 2023 16:16:37 +0200 Subject: [PATCH] Introduce bootstrap configuration for plugin (#16205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bootstrap configuration can be part of a plugin distribution but it will be loaded before the actual plugin is instantiated. This allows to e.g. specify a plugin-wide switch for a feature which can then be evaluated early in the plugin startup phase. If the feature is disabled, we can skip loading configuration with potentially required parameters and prevent validation errors. Co-authored-by: Laura Bergenthal-Grotlüschen --- .../org/graylog2/bootstrap/CmdLineTool.java | 49 ++++++++++--------- .../journal/AbstractJournalCommand.java | 4 +- .../plugin/PluginBootstrapConfig.java | 28 +++++++++++ .../graylog2/shared/plugins/PluginLoader.java | 43 +++++++++------- 4 files changed, 82 insertions(+), 42 deletions(-) create mode 100644 graylog2-server/src/main/java/org/graylog2/plugin/PluginBootstrapConfig.java diff --git a/graylog2-server/src/main/java/org/graylog2/bootstrap/CmdLineTool.java b/graylog2-server/src/main/java/org/graylog2/bootstrap/CmdLineTool.java index 4961b881ef..764ef4fc9d 100644 --- a/graylog2-server/src/main/java/org/graylog2/bootstrap/CmdLineTool.java +++ b/graylog2-server/src/main/java/org/graylog2/bootstrap/CmdLineTool.java @@ -133,8 +133,9 @@ public abstract class CmdLineTool implements CliCommand { protected String commandName = "command"; protected Injector injector; - protected Injector coreConfigInjector; + protected Injector bootstrapConfigInjector; protected FeatureFlags featureFlags; + protected PluginLoader pluginLoader; protected CmdLineTool(Configuration configuration) { this(null, configuration); @@ -158,7 +159,6 @@ public abstract class CmdLineTool implements CliCommand { this.chainingClassLoader = new ChainingClassLoader(this.getClass().getClassLoader()); } - /** * Validate the given configuration for this command. * @@ -268,21 +268,25 @@ public abstract class CmdLineTool implements CliCommand { MetricRegistry metricRegistry = MetricRegistryFactory.create(); featureFlags = getFeatureFlags(metricRegistry); + pluginLoader = new PluginLoader(getPluginPath(configFile).toFile(), chainingClassLoader); + + installCommandConfig(); + installPluginBootstrapConfig(pluginLoader); + if (isDumpDefaultConfig()) { dumpDefaultConfigAndExit(); } installConfigRepositories(); - installCommandConfig(); beforeStart(); beforeStart(parseAndGetTLSConfiguration(), parseAndGetPathConfiguration(configFile)); processConfiguration(jadConfig); - coreConfigInjector = setupCoreConfigInjector(); + bootstrapConfigInjector = setupBootstrapConfigInjector(); - final Set plugins = loadPlugins(getPluginPath(configFile), chainingClassLoader); + final Set plugins = loadPlugins(); installPluginConfig(plugins); processConfiguration(jadConfig); @@ -323,6 +327,10 @@ public abstract class CmdLineTool implements CliCommand { startCommand(); } + private void installPluginBootstrapConfig(PluginLoader pluginLoader) { + pluginLoader.loadPluginBootstrapConfigs().forEach(jadConfig::addConfigurationBean); + } + // Parse only the TLSConfiguration bean // to avoid triggering anything that might initialize the default SSLContext private TLSProtocolsConfiguration parseAndGetTLSConfiguration() { @@ -403,9 +411,8 @@ public abstract class CmdLineTool implements CliCommand { } private void dumpDefaultConfigAndExit() { - installCommandConfig(); - coreConfigInjector = setupCoreConfigInjector(); - installPluginConfig(loadPlugins(getPluginPath(configFile), chainingClassLoader)); + bootstrapConfigInjector = setupBootstrapConfigInjector(); + installPluginConfig(pluginLoader.loadPlugins(bootstrapConfigInjector)); dumpCurrentConfigAndExit(); } @@ -420,15 +427,13 @@ public abstract class CmdLineTool implements CliCommand { return new FeatureFlagsFactory().createImmutableFeatureFlags(customFeatureFlagFile, metricRegistry); } - protected Set loadPlugins(Path pluginPath, ChainingClassLoader chainingClassLoader) { + protected Set loadPlugins() { final Set plugins = new HashSet<>(); - final PluginLoader pluginLoader = new PluginLoader(pluginPath.toFile(), chainingClassLoader, - coreConfigInjector); - for (Plugin plugin : pluginLoader.loadPlugins()) { + for (Plugin plugin : pluginLoader.loadPlugins(bootstrapConfigInjector)) { final PluginMetaData metadata = plugin.metadata(); - final Configuration config = coreConfigInjector.getInstance(Configuration.class); + final Configuration config = bootstrapConfigInjector.getInstance(Configuration.class); // TODO do we want this here? We are also considering removing the deprecated CollectorPlugin entirely if (config.isCloud()) { if (metadata.getUniqueId().equals("org.graylog.plugins.collector.CollectorPlugin")) { @@ -521,16 +526,16 @@ public abstract class CmdLineTool implements CliCommand { } /** - * Set up a separate injector, containing only the core configuration bindings. It can be used to look up - * configuration values in modules at binding time. + * Set up a separate injector, containing only the core and bootstrap configuration bindings. + * It can be used to look up configuration values in modules at binding time. */ - protected Injector setupCoreConfigInjector() { + protected Injector setupBootstrapConfigInjector() { final NamedConfigParametersModule configModule = new NamedConfigParametersModule(jadConfig.getConfigurationBeans()); - Injector coreConfigInjector = null; + Injector bootstrapConfigInjector = null; try { - coreConfigInjector = Guice.createInjector(Stage.PRODUCTION, ImmutableList.of(configModule, + bootstrapConfigInjector = Guice.createInjector(Stage.PRODUCTION, ImmutableList.of(configModule, (Module) Binder::requireExplicitBindings, this::featureFlagsBinding)); } catch (CreationException e) { annotateInjectorCreationException(e); @@ -538,12 +543,12 @@ public abstract class CmdLineTool implements CliCommand { LOG.error("Injector creation failed!", e); } - if (coreConfigInjector == null) { - LOG.error("Injector for core configuration could not be created, exiting! (Please include the previous " + - "error messages in bug reports.)"); + if (bootstrapConfigInjector == null) { + LOG.error("Injector for bootstrap configuration could not be created, exiting! (Please include the " + + "previous error messages in bug reports.)"); System.exit(1); } - return coreConfigInjector; + return bootstrapConfigInjector; } private void featureFlagsBinding(Binder binder) { diff --git a/graylog2-server/src/main/java/org/graylog2/commands/journal/AbstractJournalCommand.java b/graylog2-server/src/main/java/org/graylog2/commands/journal/AbstractJournalCommand.java index 3cbc38b980..8d7849805d 100644 --- a/graylog2-server/src/main/java/org/graylog2/commands/journal/AbstractJournalCommand.java +++ b/graylog2-server/src/main/java/org/graylog2/commands/journal/AbstractJournalCommand.java @@ -31,9 +31,7 @@ import org.graylog2.shared.bindings.SchedulerBindings; import org.graylog2.shared.bindings.ServerStatusBindings; import org.graylog2.shared.journal.LocalKafkaJournal; import org.graylog2.shared.journal.LocalKafkaJournalModule; -import org.graylog2.shared.plugins.ChainingClassLoader; -import java.nio.file.Path; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -79,7 +77,7 @@ public abstract class AbstractJournalCommand extends CmdLineTool { } @Override - protected Set loadPlugins(Path pluginPath, ChainingClassLoader chainingClassLoader) { + protected Set loadPlugins() { // these commands do not need plugins, which could cause problems because of not loaded config beans return Collections.emptySet(); } diff --git a/graylog2-server/src/main/java/org/graylog2/plugin/PluginBootstrapConfig.java b/graylog2-server/src/main/java/org/graylog2/plugin/PluginBootstrapConfig.java new file mode 100644 index 0000000000..3dd1e81b5e --- /dev/null +++ b/graylog2-server/src/main/java/org/graylog2/plugin/PluginBootstrapConfig.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog2.plugin; + +import org.graylog2.shared.plugins.PluginLoader; + +/** + * A configuration bean to be processed by {@link com.github.joschi.jadconfig.JadConfig}. + *

+ * Configurations implementing this interface should be loaded via the {@link java.util.ServiceLoader} mechanism. See + * {@link PluginLoader#loadPluginBootstrapConfigs()}. + */ +public interface PluginBootstrapConfig { +} diff --git a/graylog2-server/src/main/java/org/graylog2/shared/plugins/PluginLoader.java b/graylog2-server/src/main/java/org/graylog2/shared/plugins/PluginLoader.java index 64fb9f211e..a9d4c6612e 100644 --- a/graylog2-server/src/main/java/org/graylog2/shared/plugins/PluginLoader.java +++ b/graylog2-server/src/main/java/org/graylog2/shared/plugins/PluginLoader.java @@ -24,6 +24,7 @@ import com.google.common.collect.Iterables; import com.google.inject.Injector; import org.apache.commons.io.FileUtils; import org.graylog2.plugin.Plugin; +import org.graylog2.plugin.PluginBootstrapConfig; import org.graylog2.plugin.PluginMetaData; import org.graylog2.plugin.PluginModule; import org.graylog2.plugin.PreflightCheckModule; @@ -37,13 +38,11 @@ import java.net.URL; import java.net.URLClassLoader; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Objects; import java.util.ServiceLoader; import java.util.Set; -import java.util.stream.Collectors; import static com.google.common.base.Preconditions.checkNotNull; import static java.util.Objects.requireNonNull; @@ -54,18 +53,22 @@ public class PluginLoader { private final File pluginDir; private final ChainingClassLoader classLoader; - private final Injector coreConfigInjector; - public PluginLoader(File pluginDir, ChainingClassLoader classLoader, Injector coreConfigInjector) { + public PluginLoader(File pluginDir, ChainingClassLoader classLoader) { this.pluginDir = requireNonNull(pluginDir); this.classLoader = requireNonNull(classLoader); - this.coreConfigInjector = coreConfigInjector; + + loadPluginJars(); } - public Set loadPlugins() { + public Set loadPluginBootstrapConfigs() { + return ImmutableSet.copyOf(ServiceLoader.load(PluginBootstrapConfig.class, classLoader)); + } + + public Set loadPlugins(Injector injector) { return ImmutableSortedSet.orderedBy(new PluginComparator()) - .addAll(Iterables.transform(loadJarPlugins(), new PluginAdapterFunction())) - .addAll(Iterables.transform(loadClassPathPlugins(), new PluginAdapterFunction())) + .addAll(Iterables.transform(loadJarPlugins(), new PluginAdapterFunction(injector))) + .addAll(Iterables.transform(loadClassPathPlugins(), new PluginAdapterFunction(injector))) .build(); } @@ -73,16 +76,20 @@ public class PluginLoader { return ServiceLoader.load(Plugin.class); } - @SuppressForbidden("Deliberate invocation of URL#getFile()") private Iterable loadJarPlugins() { + return ImmutableSet.copyOf(ServiceLoader.load(Plugin.class, classLoader)); + } + + @SuppressForbidden("Deliberate invocation of URL#getFile()") + private void loadPluginJars() { if (!pluginDir.exists()) { LOG.warn("Plugin directory {} does not exist, not loading plugins.", pluginDir.getAbsolutePath()); - return Collections.emptySet(); + return; } if (!pluginDir.isDirectory()) { LOG.warn("Path {} is not a directory, cannot load plugins.", pluginDir); - return Collections.emptySet(); + return; } LOG.debug("Scanning directory <{}> for plugins...", pluginDir.getAbsolutePath()); @@ -102,7 +109,7 @@ public class PluginLoader { } }) .filter(Objects::nonNull) - .collect(Collectors.toList()); + .toList(); final List sharedClassLoaderUrls = new ArrayList<>(); urls.forEach(url -> { @@ -125,10 +132,6 @@ public class PluginLoader { LOG.debug("Creating shared class loader for {} plugins: {}", sharedClassLoaderUrls.size(), sharedClassLoaderUrls); classLoader.addClassLoader(URLClassLoader.newInstance(sharedClassLoaderUrls.toArray(new URL[0]))); } - - final ServiceLoader pluginServiceLoader = ServiceLoader.load(Plugin.class, classLoader); - - return ImmutableSet.copyOf(pluginServiceLoader); } public static class PluginComparator implements Comparator { @@ -202,9 +205,15 @@ public class PluginLoader { } private class PluginAdapterFunction implements Function { + private final Injector injector; + + public PluginAdapterFunction(Injector injector) { + this.injector = injector; + } + @Override public Plugin apply(Plugin input) { - coreConfigInjector.injectMembers(input); + injector.injectMembers(input); return new PluginAdapter(input); } }