Skip to content
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

Jump to bottom

Enhanced control over plugins registration #6700

Merged
merged 49 commits into from
Apr 19, 2024
Merged

Enhanced control over plugins registration #6700

Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
343d0ff
Add option to only load known plugins
Gabriel-Trintinalia Feb 29, 2024
4b354f2
changes to plugin logic
Gabriel-Trintinalia Mar 1, 2024
f380874
changes to plugin logic
Gabriel-Trintinalia Mar 4, 2024
0f174d0
Merge branch 'hyperledger:main' into plugin-explicit-load
Gabriel-Trintinalia Mar 6, 2024
c24a656
Merge branch 'plugin-explicit-load' of https://github.com/Gabriel-Tri…
Gabriel-Trintinalia Mar 6, 2024
5681275
Refactoring and unit tests
Gabriel-Trintinalia Mar 6, 2024
d4adb71
Refactoring and unit tests
Gabriel-Trintinalia Mar 7, 2024
a439c66
Refactoring and unit tests
Gabriel-Trintinalia Mar 7, 2024
001c6e4
Refactoring and unit tests
Gabriel-Trintinalia Mar 7, 2024
586ae9f
Simplify
Gabriel-Trintinalia Mar 7, 2024
58317de
Simplify
Gabriel-Trintinalia Mar 7, 2024
bab565d
Add tests
Gabriel-Trintinalia Mar 7, 2024
e03a8de
Add tests
Gabriel-Trintinalia Mar 7, 2024
e44856f
improve code
Gabriel-Trintinalia Mar 7, 2024
5f86fab
improve code
Gabriel-Trintinalia Mar 7, 2024
b443f93
improve code
Gabriel-Trintinalia Mar 7, 2024
b1349c4
improve code
Gabriel-Trintinalia Mar 7, 2024
41f815f
Implement CLIOptions interface
Gabriel-Trintinalia Mar 7, 2024
3659b19
Create another constructor and simplify code
Gabriel-Trintinalia Mar 8, 2024
1a189f3
Minor renaming and javadoc
Gabriel-Trintinalia Mar 8, 2024
e019a26
remove heading from plugin group
Gabriel-Trintinalia Mar 8, 2024
79dc158
Fix headers and hash
Gabriel-Trintinalia Mar 8, 2024
f9be655
Fix headers, javadoc, and build
Gabriel-Trintinalia Mar 8, 2024
f30463f
Fix javadoc and imports
Gabriel-Trintinalia Mar 8, 2024
d48a12c
Merge branch 'main' into plugin-explicit-load
Gabriel-Trintinalia Mar 8, 2024
833f920
Remove strict option as it is redundant
Gabriel-Trintinalia Mar 8, 2024
a6c2153
Merge branch 'main' into plugin-explicit-load
Gabriel-Trintinalia Mar 11, 2024
61fc125
Merge main into branch
Gabriel-Trintinalia Mar 12, 2024
053987c
fix spotless
Gabriel-Trintinalia Mar 12, 2024
fa717b6
fix javadoc
Gabriel-Trintinalia Mar 12, 2024
1932097
update CHANGELOG.md
Gabriel-Trintinalia Mar 12, 2024
867efb7
Merge main into branch
Gabriel-Trintinalia Mar 12, 2024
17d0c36
Fix missing final
Gabriel-Trintinalia Mar 12, 2024
fae5a74
Accept PR recommendations
Gabriel-Trintinalia Mar 12, 2024
988995d
Merge branch 'main' into plugin-explicit-load
Gabriel-Trintinalia Mar 12, 2024
3034105
Merge branch 'main' into plugin-explicit-load
Gabriel-Trintinalia Mar 12, 2024
960b8bc
Merge branch 'main' into plugin-explicit-load
Gabriel-Trintinalia Mar 13, 2024
13cbbda
Merge main into branch
Gabriel-Trintinalia Mar 14, 2024
a46f460
Update CHANGELOG.md
Gabriel-Trintinalia Mar 15, 2024
8655283
Remove old text
Gabriel-Trintinalia Mar 15, 2024
fa67700
Merge branch 'main' into plugin-explicit-load
Gabriel-Trintinalia Mar 15, 2024
f1b788b
Merge branch 'main' into plugin-explicit-load
Gabriel-Trintinalia Mar 15, 2024
ccbaff3
Implement strategy chain
Gabriel-Trintinalia Apr 16, 2024
64b010f
Merge main
Gabriel-Trintinalia Apr 16, 2024
a1b9beb
Merge main
Gabriel-Trintinalia Apr 16, 2024
8ce8bf0
Fix execute last
Gabriel-Trintinalia Apr 16, 2024
c4a3077
Merge branch 'plugin-explicit-load' of https://github.com/Gabriel-Tri…
Gabriel-Trintinalia Apr 16, 2024
2dfedaa
Merge branch 'main' into plugin-explicit-load
Gabriel-Trintinalia Apr 19, 2024
06ea531
Change changelog entry as per PR feedback
Gabriel-Trintinalia Apr 19, 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
- Expose bad block events via the BesuEvents plugin API [#6848](https://github.com/hyperledger/besu/pull/6848)
- Add RPC errors metric [#6919](https://github.com/hyperledger/besu/pull/6919/)
- Add `rlp decode` subcommand to decode IBFT/QBFT extraData to validator list [#6895](https://github.com/hyperledger/besu/pull/6895)
- Enhanced control over plugins registration [#6700](https://github.com/hyperledger/besu/pull/6700)
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't say what kind of control is being added. Maybe something like Allow users to specify which plugins are registered



### Bug fixes
- Fix txpool dump/restore race condition [#6665](https://github.com/hyperledger/besu/pull/6665)
Expand Down
4 changes: 3 additions & 1 deletion ...sl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.hyperledger.besu.ethereum.api.ApiConfiguration;
import org.hyperledger.besu.ethereum.api.graphql.GraphQLConfiguration;
import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters;
import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration;
import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration;
import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
Expand Down Expand Up @@ -141,7 +142,8 @@ private BesuPluginContextImpl buildPluginContext(
besuPluginContext.addService(PermissioningService.class, permissioningService);
besuPluginContext.addService(PrivacyPluginService.class, new PrivacyPluginServiceImpl());

besuPluginContext.registerPlugins(pluginsPath);
besuPluginContext.registerPlugins(new PluginConfiguration(pluginsPath));

commandLine.parseArgs(node.getConfiguration().getExtraCLIOptions().toArray(new String[0]));

// register built-in plugins
Expand Down
142 changes: 110 additions & 32 deletions ...s/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,31 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.junit.jupiter.api.Assertions.assertThrows;

import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration;
import org.hyperledger.besu.ethereum.core.plugins.PluginInfo;
import org.hyperledger.besu.plugin.BesuPlugin;
import org.hyperledger.besu.tests.acceptance.plugins.TestBesuEventsPlugin;
import org.hyperledger.besu.tests.acceptance.plugins.TestPicoCLIPlugin;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.Optional;

import org.assertj.core.api.Assertions;
import org.assertj.core.api.ThrowableAssert;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public class BesuPluginContextImplTest {
private static final Path DEFAULT_PLUGIN_DIRECTORY = Paths.get(".");
private BesuPluginContextImpl contextImpl;

@BeforeAll
public static void createFakePluginDir() throws IOException {
Expand All @@ -49,16 +56,20 @@ public void clearTestPluginState() {
System.clearProperty("testPicoCLIPlugin.testOption");
}

@BeforeEach
void setup() {
contextImpl = new BesuPluginContextImpl();
}

@Test
public void verifyEverythingGoesSmoothly() {
final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl();
assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));
assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty();

assertThat(contextImpl.getPlugins()).isEmpty();
contextImpl.registerPlugins(new File(".").toPath());
assertThat(contextImpl.getPlugins()).isNotEmpty();

final Optional<TestPicoCLIPlugin> testPluginOptional = findTestPlugin(contextImpl.getPlugins());
Assertions.assertThat(testPluginOptional).isPresent();
final Optional<TestPicoCLIPlugin> testPluginOptional =
findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class);
assertThat(testPluginOptional).isPresent();
final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get();
assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered");

Expand All @@ -72,56 +83,59 @@ public void verifyEverythingGoesSmoothly() {

@Test
public void registrationErrorsHandledSmoothly() {
final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl();
System.setProperty("testPicoCLIPlugin.testOption", "FAILREGISTER");

assertThat(contextImpl.getPlugins()).isEmpty();
contextImpl.registerPlugins(new File(".").toPath());
assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);
assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));
assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);

contextImpl.beforeExternalServices();
assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);
assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);

contextImpl.startPlugins();
assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);
assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);

contextImpl.stopPlugins();
assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);
assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);
}

@Test
public void startErrorsHandledSmoothly() {
final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl();
System.setProperty("testPicoCLIPlugin.testOption", "FAILSTART");

assertThat(contextImpl.getPlugins()).isEmpty();
contextImpl.registerPlugins(new File(".").toPath());
assertThat(contextImpl.getPlugins()).extracting("class").contains(TestPicoCLIPlugin.class);
assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));
assertThat(contextImpl.getRegisteredPlugins())
.extracting("class")
.contains(TestPicoCLIPlugin.class);

final Optional<TestPicoCLIPlugin> testPluginOptional = findTestPlugin(contextImpl.getPlugins());
final Optional<TestPicoCLIPlugin> testPluginOptional =
findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class);
assertThat(testPluginOptional).isPresent();
final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get();
assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered");

contextImpl.beforeExternalServices();
contextImpl.startPlugins();
assertThat(testPicoCLIPlugin.getState()).isEqualTo("failstart");
assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);
assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);

contextImpl.stopPlugins();
assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);
assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);
}

@Test
public void stopErrorsHandledSmoothly() {
final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl();
System.setProperty("testPicoCLIPlugin.testOption", "FAILSTOP");

assertThat(contextImpl.getPlugins()).isEmpty();
contextImpl.registerPlugins(new File(".").toPath());
assertThat(contextImpl.getPlugins()).extracting("class").contains(TestPicoCLIPlugin.class);
assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));
assertThat(contextImpl.getRegisteredPlugins())
.extracting("class")
.contains(TestPicoCLIPlugin.class);

final Optional<TestPicoCLIPlugin> testPluginOptional = findTestPlugin(contextImpl.getPlugins());
final Optional<TestPicoCLIPlugin> testPluginOptional =
findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class);
assertThat(testPluginOptional).isPresent();
final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get();
assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered");
Expand All @@ -136,9 +150,8 @@ public void stopErrorsHandledSmoothly() {

@Test
public void lifecycleExceptions() throws Throwable {
final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl();
final ThrowableAssert.ThrowingCallable registerPlugins =
() -> contextImpl.registerPlugins(new File(".").toPath());
() -> contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));

assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::startPlugins);
assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::stopPlugins);
Expand All @@ -158,9 +171,74 @@ public void lifecycleExceptions() throws Throwable {
assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::stopPlugins);
}

private Optional<TestPicoCLIPlugin> findTestPlugin(final List<BesuPlugin> plugins) {
@Test
public void shouldRegisterAllPluginsWhenNoPluginsOption() {
final PluginConfiguration config = createConfigurationForAllPlugins();

assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
contextImpl.registerPlugins(config);
final Optional<TestPicoCLIPlugin> testPluginOptional =
findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class);
assertThat(testPluginOptional).isPresent();
final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get();
assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered");
}

@Test
public void shouldRegisterOnlySpecifiedPluginWhenPluginsOptionIsSet() {
final PluginConfiguration config = createConfigurationForSpecificPlugin("TestPicoCLIPlugin");

assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
contextImpl.registerPlugins(config);

final Optional<TestPicoCLIPlugin> requestedPlugin =
findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class);

assertThat(requestedPlugin).isPresent();
assertThat(requestedPlugin.get().getState()).isEqualTo("registered");

final Optional<TestPicoCLIPlugin> nonRequestedPlugin =
findTestPlugin(contextImpl.getRegisteredPlugins(), TestBesuEventsPlugin.class);

assertThat(nonRequestedPlugin).isEmpty();
}

@Test
public void shouldNotRegisterUnspecifiedPluginsWhenPluginsOptionIsSet() {
final PluginConfiguration config = createConfigurationForSpecificPlugin("TestPicoCLIPlugin");
assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
contextImpl.registerPlugins(config);

final Optional<TestPicoCLIPlugin> nonRequestedPlugin =
findTestPlugin(contextImpl.getRegisteredPlugins(), TestBesuEventsPlugin.class);
assertThat(nonRequestedPlugin).isEmpty();
}

@Test
void shouldThrowExceptionIfExplicitlySpecifiedPluginNotFound() {
PluginConfiguration config = createConfigurationForSpecificPlugin("NonExistentPlugin");

String exceptionMessage =
assertThrows(NoSuchElementException.class, () -> contextImpl.registerPlugins(config))
.getMessage();
final String expectedMessage =
"The following requested plugins were not found: NonExistentPlugin";
assertThat(exceptionMessage).isEqualTo(expectedMessage);
assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
}

private PluginConfiguration createConfigurationForAllPlugins() {
return new PluginConfiguration(null, DEFAULT_PLUGIN_DIRECTORY);
}

private PluginConfiguration createConfigurationForSpecificPlugin(final String pluginName) {
return new PluginConfiguration(List.of(new PluginInfo(pluginName)), DEFAULT_PLUGIN_DIRECTORY);
}

private Optional<TestPicoCLIPlugin> findTestPlugin(
final List<BesuPlugin> plugins, final Class<?> type) {
return plugins.stream()
.filter(p -> p instanceof TestPicoCLIPlugin)
.filter(p -> type.equals(p.getClass()))
.map(p -> (TestPicoCLIPlugin) p)
.findFirst();
}
Expand Down
Loading
Loading