From d7041d4221dc1bc4fb9e7e529df4468931d64a11 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Fri, 16 Aug 2024 15:09:31 +1000 Subject: [PATCH] CLI option for disabling auto-registration of external plugins (#7470) Signed-off-by: Gabriel-Trintinalia --- .../dsl/node/ThreadBesuNodeRunner.java | 3 +- .../services/BesuPluginContextImplTest.java | 46 +++++-- .../org/hyperledger/besu/cli/BesuCommand.java | 9 +- .../besu/cli/DefaultCommandValues.java | 3 + .../stable/PluginsConfigurationOptions.java | 38 +++++- .../besu/services/BesuPluginContextImpl.java | 30 +++-- .../besu/cli/PluginsOptionsTest.java | 118 ++++++++++++++++++ .../src/test/resources/everything_config.toml | 4 + .../core/plugins/PluginConfiguration.java | 94 +++++++------- 9 files changed, 269 insertions(+), 76 deletions(-) create mode 100644 besu/src/test/java/org/hyperledger/besu/cli/PluginsOptionsTest.java diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java index 7e8d435c72..001406cb78 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java @@ -432,7 +432,8 @@ public class ThreadBesuNodeRunner implements BesuNodeRunner { besuPluginContext.addService(PermissioningService.class, permissioningService); besuPluginContext.addService(PrivacyPluginService.class, new PrivacyPluginServiceImpl()); - besuPluginContext.registerPlugins(new PluginConfiguration(pluginsPath)); + besuPluginContext.registerPlugins( + new PluginConfiguration.Builder().pluginsDir(pluginsPath).build()); // register built-in plugins new RocksDBPlugin().register(besuPluginContext); diff --git a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java index c266eaf66a..7e27704177 100644 --- a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java +++ b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java @@ -64,7 +64,8 @@ public class BesuPluginContextImplTest { @Test public void verifyEverythingGoesSmoothly() { assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); + contextImpl.registerPlugins( + PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty(); final Optional testPluginOptional = @@ -86,7 +87,8 @@ public class BesuPluginContextImplTest { System.setProperty("testPicoCLIPlugin.testOption", "FAILREGISTER"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); + contextImpl.registerPlugins( + PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.beforeExternalServices(); @@ -104,7 +106,8 @@ public class BesuPluginContextImplTest { System.setProperty("testPicoCLIPlugin.testOption", "FAILSTART"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); + contextImpl.registerPlugins( + PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -129,7 +132,8 @@ public class BesuPluginContextImplTest { System.setProperty("testPicoCLIPlugin.testOption", "FAILSTOP"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); + contextImpl.registerPlugins( + PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -151,7 +155,9 @@ public class BesuPluginContextImplTest { @Test public void lifecycleExceptions() throws Throwable { final ThrowableAssert.ThrowingCallable registerPlugins = - () -> contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); + () -> + contextImpl.registerPlugins( + PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build()); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::startPlugins); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::stopPlugins); @@ -173,7 +179,8 @@ public class BesuPluginContextImplTest { @Test public void shouldRegisterAllPluginsWhenNoPluginsOption() { - final PluginConfiguration config = createConfigurationForAllPlugins(); + final PluginConfiguration config = + PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build(); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(config); @@ -227,12 +234,33 @@ public class BesuPluginContextImplTest { assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); } - private PluginConfiguration createConfigurationForAllPlugins() { - return new PluginConfiguration(null, DEFAULT_PLUGIN_DIRECTORY); + @Test + void shouldNotRegisterAnyPluginsIfExternalPluginsDisabled() { + PluginConfiguration config = + PluginConfiguration.builder() + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .externalPluginsEnabled(false) + .build(); + contextImpl.registerPlugins(config); + assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isTrue(); + } + + @Test + void shouldRegisterPluginsIfExternalPluginsEnabled() { + PluginConfiguration config = + PluginConfiguration.builder() + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .externalPluginsEnabled(true) + .build(); + contextImpl.registerPlugins(config); + assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isFalse(); } private PluginConfiguration createConfigurationForSpecificPlugin(final String pluginName) { - return new PluginConfiguration(List.of(new PluginInfo(pluginName)), DEFAULT_PLUGIN_DIRECTORY); + return PluginConfiguration.builder() + .requestedPlugins(List.of(new PluginInfo(pluginName))) + .pluginsDir(DEFAULT_PLUGIN_DIRECTORY) + .build(); } private Optional findTestPlugin( diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index a0e99db69f..ba61a869ff 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -1122,7 +1122,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { runner.startExternalServices(); startPlugins(runner); - validatePluginOptions(); + validatePrivacyPluginOptions(); setReleaseMetrics(); preSynchronization(); @@ -1347,7 +1347,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { besuPluginContext.startPlugins(); } - private void validatePluginOptions() { + private void validatePrivacyPluginOptions() { // plugins do not 'wire up' until start has been called // consequently you can only do some configuration checks // after start has been called on plugins @@ -1491,6 +1491,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable { validateGraphQlOptions(); validateApiOptions(); validateConsensusSyncCompatibilityOptions(); + validatePluginOptions(); p2pTLSConfigOptions.checkP2PTLSOptionsDependencies(logger, commandLine); } @@ -1511,6 +1512,10 @@ public class BesuCommand implements DefaultCommandValues, Runnable { } } + private void validatePluginOptions() { + pluginsConfigurationOptions.validate(commandLine); + } + private void validateApiOptions() { apiConfigurationOptions.validate(commandLine, logger); } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java b/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java index 84055db663..ba05f45524 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java @@ -128,6 +128,9 @@ public interface DefaultCommandValues { /** The constant DEFAULT_PLUGINS_OPTION_NAME. */ String DEFAULT_PLUGINS_OPTION_NAME = "--plugins"; + /** The constant DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME. */ + String DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME = "--Xplugins-external-enabled"; + /** * Gets default besu data path. * diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index 88f88c06d4..47df831c57 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -14,6 +14,7 @@ */ package org.hyperledger.besu.cli.options.stable; +import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME; import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_OPTION_NAME; import org.hyperledger.besu.cli.converter.PluginInfoConverter; @@ -28,6 +29,15 @@ import picocli.CommandLine; /** The Plugins Options options. */ public class PluginsConfigurationOptions implements CLIOptions { + + @CommandLine.Option( + names = {DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME}, + description = "Enables external plugins (default: ${DEFAULT-VALUE})", + hidden = true, + defaultValue = "true", + arity = "1") + private Boolean externalPluginsEnabled = true; + @CommandLine.Option( names = {DEFAULT_PLUGINS_OPTION_NAME}, description = "Comma-separated list of plugin names", @@ -42,7 +52,24 @@ public class PluginsConfigurationOptions implements CLIOptions registeringPlugins = - matchAndValidateRequestedPlugins(requestedPlugins, detectedPlugins); - - registerPlugins(registeringPlugins); + if (config.isExternalPluginsEnabled()) { + detectedPlugins = detectPlugins(config); + + if (config.getRequestedPlugins().isEmpty()) { + // If no plugins were specified, register all detected plugins + registerPlugins(detectedPlugins); + } else { + // Register only the plugins that were explicitly requested and validated + requestedPlugins = config.getRequestedPlugins(); + // Match and validate the requested plugins against the detected plugins + List registeringPlugins = + matchAndValidateRequestedPlugins(requestedPlugins, detectedPlugins); + + registerPlugins(registeringPlugins); + } } else { - // If no plugins were specified, register all detected plugins - registerPlugins(detectedPlugins); + LOG.debug("External plugins are disabled. Skipping plugins registration."); } + state = Lifecycle.REGISTERED; } private List matchAndValidateRequestedPlugins( @@ -182,7 +187,6 @@ public class BesuPluginContextImpl implements BesuContext, PluginVersionsProvide registeredPlugins.add(plugin); } } - state = Lifecycle.REGISTERED; } private boolean registerPlugin(final BesuPlugin plugin) { diff --git a/besu/src/test/java/org/hyperledger/besu/cli/PluginsOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/PluginsOptionsTest.java new file mode 100644 index 0000000000..1adfb585e9 --- /dev/null +++ b/besu/src/test/java/org/hyperledger/besu/cli/PluginsOptionsTest.java @@ -0,0 +1,118 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.cli; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; + +import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; + +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; + +public class PluginsOptionsTest extends CommandTestAbstract { + + @Captor protected ArgumentCaptor pluginConfigurationArgumentCaptor; + + @Test + public void shouldParsePluginOptionForSinglePlugin() { + parseCommand("--plugins", "pluginA"); + verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture()); + assertThat(pluginConfigurationArgumentCaptor.getValue().getRequestedPlugins()) + .isEqualTo(List.of("pluginA")); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void shouldParsePluginOptionForMultiplePlugins() { + parseCommand("--plugins", "pluginA,pluginB"); + verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture()); + assertThat(pluginConfigurationArgumentCaptor.getValue().getRequestedPlugins()) + .isEqualTo(List.of("pluginA", "pluginB")); + + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void shouldNotUsePluginOptionWhenNoPluginsSpecified() { + parseCommand(); + verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture()); + assertThat(pluginConfigurationArgumentCaptor.getValue().getRequestedPlugins()) + .isEqualTo(List.of()); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void shouldNotParseAnyPluginsWhenPluginOptionIsEmpty() { + parseCommand("--plugins", ""); + verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture()); + assertThat(pluginConfigurationArgumentCaptor.getValue().getRequestedPlugins()) + .isEqualTo(List.of()); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void shouldParsePluginsExternalEnabledOptionWhenFalse() { + parseCommand("--Xplugins-external-enabled=false"); + verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture()); + + assertThat(pluginConfigurationArgumentCaptor.getValue().isExternalPluginsEnabled()) + .isEqualTo(false); + + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void shouldParsePluginsExternalEnabledOptionWhenTrue() { + parseCommand("--Xplugins-external-enabled=true"); + verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture()); + + assertThat(pluginConfigurationArgumentCaptor.getValue().isExternalPluginsEnabled()) + .isEqualTo(true); + + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void shouldEnablePluginsExternalByDefault() { + parseCommand(); + verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture()); + assertThat(pluginConfigurationArgumentCaptor.getValue().isExternalPluginsEnabled()) + .isEqualTo(true); + + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + } + + @Test + public void shouldFailWhenPluginsIsDisabledAndPluginsExplicitlyRequested() { + parseCommand("--Xplugins-external-enabled=false", "--plugins", "pluginA"); + verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture()); + + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)) + .contains("--plugins option can only be used when --Xplugins-external-enabled is true"); + } +} diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml index 27ea5c1645..b489df9fa2 100644 --- a/besu/src/test/resources/everything_config.toml +++ b/besu/src/test/resources/everything_config.toml @@ -242,3 +242,7 @@ Xp2p-tls-clienthello-sni=false #contracts Xevm-jumpdest-cache-weight-kb=32000 + +# plugins +Xplugins-external-enabled=true +plugins=["none"] \ No newline at end of file diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java index dbc742fb2f..ebc99f647a 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java @@ -14,58 +14,25 @@ */ package org.hyperledger.besu.ethereum.core.plugins; -import static java.util.Objects.requireNonNull; - import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collections; import java.util.List; -/** - * Configuration for managing plugins, including their information, detection type, and directory. - */ public class PluginConfiguration { private final List requestedPlugins; private final Path pluginsDir; + private final boolean externalPluginsEnabled; - /** - * Constructs a new PluginConfiguration with the specified plugin information and requestedPlugins - * directory. - * - * @param requestedPlugins List of {@link PluginInfo} objects representing the requestedPlugins. - * @param pluginsDir The directory where requestedPlugins are located. - */ - public PluginConfiguration(final List requestedPlugins, final Path pluginsDir) { + public PluginConfiguration( + final List requestedPlugins, + final Path pluginsDir, + final boolean externalPluginsEnabled) { this.requestedPlugins = requestedPlugins; this.pluginsDir = pluginsDir; + this.externalPluginsEnabled = externalPluginsEnabled; } - /** - * Constructs a PluginConfiguration with specified plugins using the default directory. - * - * @param requestedPlugins List of plugins for consideration or registration. discoverable plugins - * are. - */ - public PluginConfiguration(final List requestedPlugins) { - this.requestedPlugins = requestedPlugins; - this.pluginsDir = PluginConfiguration.defaultPluginsDir(); - } - - /** - * Constructs a PluginConfiguration with the specified plugins directory - * - * @param pluginsDir The directory where plugins are located. Cannot be null. - */ - public PluginConfiguration(final Path pluginsDir) { - this.requestedPlugins = null; - this.pluginsDir = requireNonNull(pluginsDir); - } - - /** - * Returns the names of requested plugins, or an empty list if none. - * - * @return List of requested plugin names, never {@code null}. - */ public List getRequestedPlugins() { return requestedPlugins == null ? Collections.emptyList() @@ -76,17 +43,46 @@ public class PluginConfiguration { return pluginsDir; } - /** - * Returns the default plugins directory based on system properties. - * - * @return The default {@link Path} to the plugin's directory. - */ + public boolean isExternalPluginsEnabled() { + return externalPluginsEnabled; + } + public static Path defaultPluginsDir() { - final String pluginsDirProperty = System.getProperty("besu.plugins.dir"); - if (pluginsDirProperty == null) { - return Paths.get(System.getProperty("besu.home", "."), "plugins"); - } else { - return Paths.get(pluginsDirProperty); + String pluginsDirProperty = System.getProperty("besu.plugins.dir"); + return pluginsDirProperty == null + ? Paths.get(System.getProperty("besu.home", "."), "plugins") + : Paths.get(pluginsDirProperty); + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private List requestedPlugins; + private Path pluginsDir; + private boolean externalPluginsEnabled = true; + + public Builder requestedPlugins(final List requestedPlugins) { + this.requestedPlugins = requestedPlugins; + return this; + } + + public Builder pluginsDir(final Path pluginsDir) { + this.pluginsDir = pluginsDir; + return this; + } + + public Builder externalPluginsEnabled(final boolean externalPluginsEnabled) { + this.externalPluginsEnabled = externalPluginsEnabled; + return this; + } + + public PluginConfiguration build() { + if (pluginsDir == null) { + pluginsDir = PluginConfiguration.defaultPluginsDir(); + } + return new PluginConfiguration(requestedPlugins, pluginsDir, externalPluginsEnabled); } } }