From 44154b5e4e624e88add11880415977ba17ed3680 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 14 Oct 2024 23:50:23 -0400 Subject: [PATCH 1/8] Make extended plugins optional Signed-off-by: Craig Perkins --- .../gradle/plugin/PluginPropertiesExtension.java | 13 ++++++++++++- .../java/org/opensearch/plugins/PluginsService.java | 8 ++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java b/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java index d6117923973fa..189b0e773d0a2 100644 --- a/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java +++ b/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java @@ -53,9 +53,12 @@ public class PluginPropertiesExtension { private String customFolderName = ""; - /** Other plugins this plugin extends through SPI */ + /** Other plugins this plugin extends through SPI, these plugins are required to install this plugin */ private List extendedPlugins = new ArrayList<>(); + /** Other plugins this plugin extends through SPI, these plugins are not required for install */ + private List optionalExtendedPlugins = new ArrayList<>(); + private boolean hasNativeController; /** True if the plugin requires the opensearch keystore to exist, false otherwise. */ @@ -122,6 +125,10 @@ public List getExtendedPlugins() { return this.extendedPlugins; } + public List getOptionalExtendedPlugins() { + return this.optionalExtendedPlugins; + } + public boolean isHasNativeController() { return hasNativeController; } @@ -164,6 +171,10 @@ public void setExtendedPlugins(List extendedPlugins) { this.extendedPlugins = extendedPlugins; } + public void setOptionalExtendedPlugins(List optionalExtendedPlugins) { + this.optionalExtendedPlugins = optionalExtendedPlugins; + } + public boolean isHasClientJar() { return hasClientJar; } diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index f08c9c738f1b4..760c5352a1820 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -524,7 +524,8 @@ private static void addSortedBundle( for (String dependency : bundle.plugin.getExtendedPlugins()) { Bundle depBundle = bundles.get(dependency); if (depBundle == null) { - throw new IllegalArgumentException("Missing plugin [" + dependency + "], dependency of [" + name + "]"); + continue; + // throw new IllegalArgumentException("Missing plugin [" + dependency + "], dependency of [" + name + "]"); } addSortedBundle(depBundle, bundles, sortedBundles, dependencyStack); assert sortedBundles.contains(depBundle); @@ -653,7 +654,9 @@ static void checkBundleJarHell(Set classpath, Bundle bundle, Map urls = new HashSet<>(); for (String extendedPlugin : exts) { Set pluginUrls = transitiveUrls.get(extendedPlugin); - assert pluginUrls != null : "transitive urls should have already been set for " + extendedPlugin; + if (pluginUrls == null) { + continue; + } Set intersection = new HashSet<>(urls); intersection.retainAll(pluginUrls); @@ -690,6 +693,7 @@ static void checkBundleJarHell(Set classpath, Bundle bundle, Map Date: Mon, 14 Oct 2024 23:57:21 -0400 Subject: [PATCH 2/8] Make extended plugins optional Signed-off-by: Craig Perkins --- .../gradle/plugin/PluginPropertiesExtension.java | 13 +------------ .../java/org/opensearch/plugins/PluginsService.java | 1 - 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java b/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java index 189b0e773d0a2..d6117923973fa 100644 --- a/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java +++ b/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java @@ -53,12 +53,9 @@ public class PluginPropertiesExtension { private String customFolderName = ""; - /** Other plugins this plugin extends through SPI, these plugins are required to install this plugin */ + /** Other plugins this plugin extends through SPI */ private List extendedPlugins = new ArrayList<>(); - /** Other plugins this plugin extends through SPI, these plugins are not required for install */ - private List optionalExtendedPlugins = new ArrayList<>(); - private boolean hasNativeController; /** True if the plugin requires the opensearch keystore to exist, false otherwise. */ @@ -125,10 +122,6 @@ public List getExtendedPlugins() { return this.extendedPlugins; } - public List getOptionalExtendedPlugins() { - return this.optionalExtendedPlugins; - } - public boolean isHasNativeController() { return hasNativeController; } @@ -171,10 +164,6 @@ public void setExtendedPlugins(List extendedPlugins) { this.extendedPlugins = extendedPlugins; } - public void setOptionalExtendedPlugins(List optionalExtendedPlugins) { - this.optionalExtendedPlugins = optionalExtendedPlugins; - } - public boolean isHasClientJar() { return hasClientJar; } diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index 760c5352a1820..f96bae2f054a7 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -693,7 +693,6 @@ static void checkBundleJarHell(Set classpath, Bundle bundle, Map Date: Fri, 13 Dec 2024 19:39:06 -0500 Subject: [PATCH 3/8] Load extensions for classpath plugins Signed-off-by: Craig Perkins --- .../src/main/java/org/opensearch/plugins/PluginsService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index f96bae2f054a7..a6298b8b4da6f 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -150,7 +150,7 @@ public PluginsService( "1.8", pluginClass.getName(), null, - Collections.emptyList(), + classpathPlugins.stream().map(Class::getName).filter(cp -> !pluginClass.getName().equals(cp)).collect(Collectors.toList()), false ); if (logger.isTraceEnabled()) { @@ -159,6 +159,7 @@ public PluginsService( pluginsLoaded.add(new Tuple<>(pluginInfo, plugin)); pluginsList.add(pluginInfo); pluginsNames.add(pluginInfo.getName()); + loadExtensions(pluginsLoaded); } Set seenBundles = new LinkedHashSet<>(); From 7b4d7d7ad27d1321b4aa49c6d03e14e1365cbc63 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 13 Dec 2024 20:29:36 -0500 Subject: [PATCH 4/8] Ensure only single instance for each classpath extension Signed-off-by: Craig Perkins --- .../java/org/opensearch/plugins/PluginsService.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index a6298b8b4da6f..c3db32874d260 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -159,8 +159,8 @@ public PluginsService( pluginsLoaded.add(new Tuple<>(pluginInfo, plugin)); pluginsList.add(pluginInfo); pluginsNames.add(pluginInfo.getName()); - loadExtensions(pluginsLoaded); } + loadExtensions(pluginsLoaded); Set seenBundles = new LinkedHashSet<>(); List modulesList = new ArrayList<>(); @@ -571,9 +571,17 @@ private static void loadExtensionsForPlugin(ExtensiblePlugin extensiblePlugin, L ExtensiblePlugin.ExtensionLoader extensionLoader = new ExtensiblePlugin.ExtensionLoader() { @Override public List loadExtensions(Class extensionPointType) { + Set> seenClasses = new LinkedHashSet<>(); List result = new ArrayList<>(); + for (Plugin extendingPlugin : extendingPlugins) { - result.addAll(createExtensions(extensionPointType, extendingPlugin)); + List extensions = createExtensions(extensionPointType, extendingPlugin); + for (T extension : extensions) { + // Only add if we haven't seen this class before, needed for classpath extensions for testing + if (seenClasses.add(extension.getClass())) { + result.add(extension); + } + } } return Collections.unmodifiableList(result); } From 16147068fbd6760b697b3c4c587519efc0925c16 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sun, 22 Dec 2024 12:26:48 -0500 Subject: [PATCH 5/8] Add test for classpath plugin extended plugin loading Signed-off-by: Craig Perkins --- .../opensearch/plugins/ClasspathPluginIT.java | 52 +++++++++++++++++++ ....plugins.ClasspathPluginIT$SampleExtension | 8 +++ 2 files changed, 60 insertions(+) create mode 100644 server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java create mode 100644 server/src/internalClusterTest/resources/META-INF/services/org.opensearch.plugins.ClasspathPluginIT$SampleExtension diff --git a/server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java b/server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java new file mode 100644 index 0000000000000..fffef3f55c5b3 --- /dev/null +++ b/server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java @@ -0,0 +1,52 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugins; + +import org.opensearch.test.OpenSearchIntegTestCase; + +import java.io.IOException; +import java.util.Collection; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.hamcrest.Matchers.equalTo; + +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0) +public class ClasspathPluginIT extends OpenSearchIntegTestCase { + + public interface SampleExtension {} + + public static class SampleExtensiblePlugin extends Plugin implements ExtensiblePlugin { + public SampleExtensiblePlugin() {} + + @Override + public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { + int nLoaded = 0; + for (SampleExtension e : loader.loadExtensions(SampleExtension.class)) { + nLoaded++; + } + + assertThat(nLoaded, equalTo(1)); + } + } + + public static class SampleExtendingPlugin extends Plugin implements SampleExtension { + public SampleExtendingPlugin() {} + }; + + @Override + protected Collection> nodePlugins() { + return Stream.concat(super.nodePlugins().stream(), Stream.of(SampleExtensiblePlugin.class, SampleExtendingPlugin.class)) + .collect(Collectors.toList()); + } + + public void testPluginExtensionWithClasspathPlugins() throws IOException { + internalCluster().startNode(); + } +} diff --git a/server/src/internalClusterTest/resources/META-INF/services/org.opensearch.plugins.ClasspathPluginIT$SampleExtension b/server/src/internalClusterTest/resources/META-INF/services/org.opensearch.plugins.ClasspathPluginIT$SampleExtension new file mode 100644 index 0000000000000..f369b9d0f63b7 --- /dev/null +++ b/server/src/internalClusterTest/resources/META-INF/services/org.opensearch.plugins.ClasspathPluginIT$SampleExtension @@ -0,0 +1,8 @@ +# +# SPDX-License-Identifier: Apache-2.0 +# +# The OpenSearch Contributors require contributions made to +# this file be licensed under the Apache-2.0 license or a +# compatible open source license. + +org.opensearch.plugins.ClasspathPluginIT$SampleExtendingPlugin From 4176c4678d4170424a1db1225fee9de5a06351ba Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 23 Dec 2024 16:41:06 -0500 Subject: [PATCH 6/8] Enable testing for ExtensiblePlugins using classpath plugins Signed-off-by: Craig Perkins --- .../main/java/org/opensearch/plugins/PluginsService.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index c3db32874d260..9d3efbbfae5e9 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -525,8 +525,7 @@ private static void addSortedBundle( for (String dependency : bundle.plugin.getExtendedPlugins()) { Bundle depBundle = bundles.get(dependency); if (depBundle == null) { - continue; - // throw new IllegalArgumentException("Missing plugin [" + dependency + "], dependency of [" + name + "]"); + throw new IllegalArgumentException("Missing plugin [" + dependency + "], dependency of [" + name + "]"); } addSortedBundle(depBundle, bundles, sortedBundles, dependencyStack); assert sortedBundles.contains(depBundle); @@ -663,9 +662,7 @@ static void checkBundleJarHell(Set classpath, Bundle bundle, Map urls = new HashSet<>(); for (String extendedPlugin : exts) { Set pluginUrls = transitiveUrls.get(extendedPlugin); - if (pluginUrls == null) { - continue; - } + assert pluginUrls != null : "transitive urls should have already been set for " + extendedPlugin; Set intersection = new HashSet<>(urls); intersection.retainAll(pluginUrls); From f840850e5209f824c79047a0ce03e2294c829f5f Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 31 Dec 2024 10:06:00 -0500 Subject: [PATCH 7/8] Add to CHANGELOG Signed-off-by: Craig Perkins --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9e57a09704b0..16cc0e1943d42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add search replica stats to segment replication stats API ([#16678](https://github.com/opensearch-project/OpenSearch/pull/16678)) - Introduce a setting to disable download of full cluster state from remote on term mismatch([#16798](https://github.com/opensearch-project/OpenSearch/pull/16798/)) - Added ability to retrieve value from DocValues in a flat_object filed([#16802](https://github.com/opensearch-project/OpenSearch/pull/16802)) +- Enable testing for ExtensiblePlugins using classpath plugins ([#16908](https://github.com/opensearch-project/OpenSearch/pull/16908)) ### Dependencies - Bump `com.google.cloud:google-cloud-core-http` from 2.23.0 to 2.47.0 ([#16504](https://github.com/opensearch-project/OpenSearch/pull/16504)) From 7ddd9a376ab1706d6bc641acd46140ada9fae691 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 7 Jan 2025 20:44:36 -0500 Subject: [PATCH 8/8] Define PluginInfos from test files for classpath plugins Signed-off-by: Craig Perkins --- .../opensearch/plugins/ClasspathPluginIT.java | 6 ++ .../main/java/org/opensearch/node/Node.java | 7 +- .../opensearch/plugins/PluginsService.java | 43 ++++-------- .../plugins/PluginsServiceTests.java | 25 +++++-- .../java/org/opensearch/node/MockNode.java | 68 +++++++++++++++++-- .../test/AbstractBuilderTestCase.java | 19 +++++- .../opensearch/test/ExternalTestCluster.java | 4 +- .../opensearch/test/InternalTestCluster.java | 13 +++- .../test/NodeConfigurationSource.java | 6 ++ .../test/OpenSearchIntegTestCase.java | 13 ++++ 10 files changed, 154 insertions(+), 50 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java b/server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java index fffef3f55c5b3..e5f435887ccc7 100644 --- a/server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java @@ -12,6 +12,7 @@ import java.io.IOException; import java.util.Collection; +import java.util.Map; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -46,6 +47,11 @@ protected Collection> nodePlugins() { .collect(Collectors.toList()); } + @Override + protected Map, Class> extendedPlugins() { + return Map.of(SampleExtendingPlugin.class, SampleExtensiblePlugin.class); + } + public void testPluginExtensionWithClasspathPlugins() throws IOException { internalCluster().startNode(); } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 704a23890b07a..9edd9a432d0e1 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -212,6 +212,7 @@ import org.opensearch.plugins.NetworkPlugin; import org.opensearch.plugins.PersistentTaskPlugin; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.PluginInfo; import org.opensearch.plugins.PluginsService; import org.opensearch.plugins.RepositoryPlugin; import org.opensearch.plugins.ScriptPlugin; @@ -460,11 +461,7 @@ public Node(Environment environment) { * @param forbidPrivateIndexSettings whether or not private index settings are forbidden when creating an index; this is used in the * test framework for tests that rely on being able to set private settings */ - protected Node( - final Environment initialEnvironment, - Collection> classpathPlugins, - boolean forbidPrivateIndexSettings - ) { + protected Node(final Environment initialEnvironment, Collection classpathPlugins, boolean forbidPrivateIndexSettings) { final List resourcesToClose = new ArrayList<>(); // register everything we need to release in the case of an error boolean success = false; try { diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index 5de02a7f17f9f..e227be0fbc987 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -125,12 +125,13 @@ public List getPluginSettingsFilter() { * @param pluginsDirectory The directory plugins exist in, or null if plugins should not be loaded from the filesystem * @param classpathPlugins Plugins that exist in the classpath which should be loaded */ + @SuppressWarnings("unchecked") public PluginsService( Settings settings, Path configPath, Path modulesDirectory, Path pluginsDirectory, - Collection> classpathPlugins + Collection classpathPlugins ) { this.settings = settings; this.configPath = configPath; @@ -140,25 +141,19 @@ public PluginsService( // we need to build a List of plugins for checking mandatory plugins final List pluginsNames = new ArrayList<>(); // first we load plugins that are on the classpath. this is for tests - for (Class pluginClass : classpathPlugins) { - Plugin plugin = loadPlugin(pluginClass, settings, configPath); - PluginInfo pluginInfo = new PluginInfo( - pluginClass.getName(), - "classpath plugin", - "NA", - Version.CURRENT, - "1.8", - pluginClass.getName(), - null, - classpathPlugins.stream().map(Class::getName).filter(cp -> !pluginClass.getName().equals(cp)).collect(Collectors.toList()), - false - ); - if (logger.isTraceEnabled()) { - logger.trace("plugin loaded from classpath [{}]", pluginInfo); + for (PluginInfo pluginInfo : classpathPlugins) { + try { + Class pluginClazz = (Class) Class.forName(pluginInfo.getClassname()); + Plugin plugin = loadPlugin(pluginClazz, settings, configPath); + if (logger.isTraceEnabled()) { + logger.trace("plugin loaded from classpath [{}]", pluginInfo); + } + pluginsLoaded.add(new Tuple<>(pluginInfo, plugin)); + pluginsList.add(pluginInfo); + pluginsNames.add(pluginInfo.getName()); + } catch (ClassNotFoundException e) { + logger.error("Failed to load classpath plugin: " + pluginInfo.getClassname()); } - pluginsLoaded.add(new Tuple<>(pluginInfo, plugin)); - pluginsList.add(pluginInfo); - pluginsNames.add(pluginInfo.getName()); } loadExtensions(pluginsLoaded); @@ -576,17 +571,9 @@ private static void loadExtensionsForPlugin(ExtensiblePlugin extensiblePlugin, L ExtensiblePlugin.ExtensionLoader extensionLoader = new ExtensiblePlugin.ExtensionLoader() { @Override public List loadExtensions(Class extensionPointType) { - Set> seenClasses = new LinkedHashSet<>(); List result = new ArrayList<>(); - for (Plugin extendingPlugin : extendingPlugins) { - List extensions = createExtensions(extensionPointType, extendingPlugin); - for (T extension : extensions) { - // Only add if we haven't seen this class before, needed for classpath extensions for testing - if (seenClasses.add(extension.getClass())) { - result.add(extension); - } - } + result.addAll(createExtensions(extensionPointType, extendingPlugin)); } return Collections.unmodifiableList(result); } diff --git a/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java b/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java index f5702fa1a7ade..a78b9a3a1e943 100644 --- a/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java +++ b/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java @@ -57,6 +57,7 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -100,13 +101,23 @@ public Settings additionalSettings() { public static class FilterablePlugin extends Plugin implements ScriptPlugin {} static PluginsService newPluginsService(Settings settings, Class... classpathPlugins) { - return new PluginsService( - settings, - null, - null, - TestEnvironment.newEnvironment(settings).pluginsDir(), - Arrays.asList(classpathPlugins) - ); + Collection pluginInfos = new ArrayList<>(); + for (Class plugin : classpathPlugins) { + pluginInfos.add( + new PluginInfo( + plugin.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + plugin.getName(), + null, + Collections.emptyList(), + false + ) + ); + } + return new PluginsService(settings, null, null, TestEnvironment.newEnvironment(settings).pluginsDir(), pluginInfos); } public void testAdditionalSettings() { diff --git a/test/framework/src/main/java/org/opensearch/node/MockNode.java b/test/framework/src/main/java/org/opensearch/node/MockNode.java index 97c06962ca2e7..df416e9bbc21c 100644 --- a/test/framework/src/main/java/org/opensearch/node/MockNode.java +++ b/test/framework/src/main/java/org/opensearch/node/MockNode.java @@ -32,6 +32,7 @@ package org.opensearch.node; +import org.opensearch.Version; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.ClusterInfoService; import org.opensearch.cluster.MockInternalClusterInfoService; @@ -51,6 +52,7 @@ import org.opensearch.http.HttpServerTransport; import org.opensearch.indices.IndicesService; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.PluginInfo; import org.opensearch.script.MockScriptService; import org.opensearch.script.ScriptContext; import org.opensearch.script.ScriptEngine; @@ -72,10 +74,12 @@ import java.nio.file.Path; import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.Executor; import java.util.function.Function; +import java.util.stream.Collectors; /** * A node for testing which allows: @@ -88,10 +92,6 @@ public class MockNode extends Node { private final Collection> classpathPlugins; - public MockNode(final Settings settings, final Collection> classpathPlugins) { - this(settings, classpathPlugins, true); - } - public MockNode( final Settings settings, final Collection> classpathPlugins, @@ -100,6 +100,21 @@ public MockNode( this(settings, classpathPlugins, null, forbidPrivateIndexSettings); } + public MockNode( + final Settings settings, + final Collection> classpathPlugins, + final Path configPath, + final boolean forbidPrivateIndexSettings, + final Map, Class> extendedPlugins + ) { + this( + InternalSettingsPreparer.prepareEnvironment(settings, Collections.emptyMap(), configPath, () -> "mock_ node"), + classpathPlugins, + forbidPrivateIndexSettings, + extendedPlugins + ); + } + public MockNode( final Settings settings, final Collection> classpathPlugins, @@ -109,19 +124,58 @@ public MockNode( this( InternalSettingsPreparer.prepareEnvironment(settings, Collections.emptyMap(), configPath, () -> "mock_ node"), classpathPlugins, - forbidPrivateIndexSettings + forbidPrivateIndexSettings, + Collections.emptyMap() ); } private MockNode( final Environment environment, final Collection> classpathPlugins, - final boolean forbidPrivateIndexSettings + final boolean forbidPrivateIndexSettings, + final Map, Class> extendedPlugins ) { - super(environment, classpathPlugins, forbidPrivateIndexSettings); + super( + environment, + classpathPlugins.stream() + .map( + p -> new PluginInfo( + p.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + p.getName(), + null, + (extendedPlugins != null && extendedPlugins.containsKey(p)) + ? List.of(extendedPlugins.get(p).getName()) + : Collections.emptyList(), + false + ) + ) + .collect(Collectors.toList()), + forbidPrivateIndexSettings + ); this.classpathPlugins = classpathPlugins; } + public MockNode(final Settings settings, final Collection> classpathPlugins) { + this(settings, classpathPlugins, true); + } + + public MockNode( + final Settings settings, + final Collection> classpathPlugins, + final Map, Class> extendedPlugins + ) { + this( + InternalSettingsPreparer.prepareEnvironment(settings, Collections.emptyMap(), null, () -> "mock_ node"), + classpathPlugins, + true, + extendedPlugins + ); + } + /** * The classpath plugins this node was constructed with. */ diff --git a/test/framework/src/main/java/org/opensearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/opensearch/test/AbstractBuilderTestCase.java index 7a8cf4963c4f1..87d5c2def4b96 100644 --- a/test/framework/src/main/java/org/opensearch/test/AbstractBuilderTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/AbstractBuilderTestCase.java @@ -81,6 +81,7 @@ import org.opensearch.node.InternalSettingsPreparer; import org.opensearch.plugins.MapperPlugin; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.PluginInfo; import org.opensearch.plugins.PluginsService; import org.opensearch.plugins.ScriptPlugin; import org.opensearch.plugins.SearchPlugin; @@ -389,7 +390,23 @@ private static class ServiceHolder implements Closeable { throw new AssertionError("node.name must be set"); }); PluginsService pluginsService; - pluginsService = new PluginsService(nodeSettings, null, env.modulesDir(), env.pluginsDir(), plugins); + Collection pluginInfos = new ArrayList<>(); + for (Class plugin : plugins) { + pluginInfos.add( + new PluginInfo( + plugin.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + plugin.getName(), + null, + Collections.emptyList(), + false + ) + ); + } + pluginsService = new PluginsService(nodeSettings, null, env.modulesDir(), env.pluginsDir(), pluginInfos); client = (Client) Proxy.newProxyInstance(Client.class.getClassLoader(), new Class[] { Client.class }, clientInvocationHandler); ScriptModule scriptModule = createScriptModule(pluginsService.filterPlugins(ScriptPlugin.class)); diff --git a/test/framework/src/main/java/org/opensearch/test/ExternalTestCluster.java b/test/framework/src/main/java/org/opensearch/test/ExternalTestCluster.java index 3eb1680069b06..d023aed1ddbe9 100644 --- a/test/framework/src/main/java/org/opensearch/test/ExternalTestCluster.java +++ b/test/framework/src/main/java/org/opensearch/test/ExternalTestCluster.java @@ -59,6 +59,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; @@ -98,6 +99,7 @@ public ExternalTestCluster( Function clientWrapper, String clusterName, Collection> pluginClasses, + Map, Class> extendedPlugins, TransportAddress... transportAddresses ) { super(0); @@ -129,7 +131,7 @@ public ExternalTestCluster( pluginClasses = new ArrayList<>(pluginClasses); pluginClasses.add(MockHttpTransport.TestPlugin.class); Settings clientSettings = clientSettingsBuilder.build(); - MockNode node = new MockNode(clientSettings, pluginClasses); + MockNode node = new MockNode(clientSettings, pluginClasses, extendedPlugins); Client client = clientWrapper.apply(node.client()); try { node.start(); diff --git a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java index 7b2c653e9bdb2..4454d672793bf 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java @@ -518,6 +518,10 @@ public Collection> getPlugins() { return plugins; } + public Map, Class> getExtendedPlugins() { + return nodeConfigurationSource.extendedPlugins(); + } + private static Settings getRandomNodeSettings(long seed) { Random random = new Random(seed); Builder builder = Settings.builder(); @@ -803,6 +807,7 @@ private synchronized NodeAndClient buildNode(int nodeId, Settings settings, bool assert Thread.holdsLock(this); ensureOpen(); Collection> plugins = getPlugins(); + Map, Class> extendedPlugins = getExtendedPlugins(); String name = settings.get("node.name"); final NodeAndClient nodeAndClient = nodes.get(name); @@ -817,7 +822,13 @@ private synchronized NodeAndClient buildNode(int nodeId, Settings settings, bool // we clone this here since in the case of a node restart we might need it again secureSettings = ((MockSecureSettings) secureSettings).clone(); } - MockNode node = new MockNode(settings, plugins, nodeConfigurationSource.nodeConfigPath(nodeId), forbidPrivateIndexSettings); + MockNode node = new MockNode( + settings, + plugins, + nodeConfigurationSource.nodeConfigPath(nodeId), + forbidPrivateIndexSettings, + extendedPlugins + ); node.injector().getInstance(TransportService.class).addLifecycleListener(new LifecycleListener() { @Override public void afterStart() { diff --git a/test/framework/src/main/java/org/opensearch/test/NodeConfigurationSource.java b/test/framework/src/main/java/org/opensearch/test/NodeConfigurationSource.java index ff9a723176f17..b6f24fa36daea 100644 --- a/test/framework/src/main/java/org/opensearch/test/NodeConfigurationSource.java +++ b/test/framework/src/main/java/org/opensearch/test/NodeConfigurationSource.java @@ -37,6 +37,7 @@ import java.nio.file.Path; import java.util.Collection; import java.util.Collections; +import java.util.Map; public abstract class NodeConfigurationSource { @@ -63,4 +64,9 @@ public Path nodeConfigPath(int nodeOrdinal) { public Collection> nodePlugins() { return Collections.emptyList(); } + + /** Returns a map corresponding to plugin dependencies to other classpath plugins */ + public Map, Class> extendedPlugins() { + return Collections.emptyMap(); + } } diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index 1c26ea4ca2c91..197521392eb3e 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -1993,6 +1993,13 @@ protected Collection> nodePlugins() { return Collections.emptyList(); } + /** + * Returns a collection of plugins that should be loaded on each node. + */ + protected Map, Class> extendedPlugins() { + return Collections.emptyMap(); + } + private ExternalTestCluster buildExternalCluster(String clusterAddresses, String clusterName) throws IOException { String[] stringAddresses = clusterAddresses.split(","); TransportAddress[] transportAddresses = new TransportAddress[stringAddresses.length]; @@ -2008,6 +2015,7 @@ private ExternalTestCluster buildExternalCluster(String clusterAddresses, String getClientWrapper(), clusterName, nodePlugins(), + extendedPlugins(), transportAddresses ); } @@ -2114,6 +2122,11 @@ public Path nodeConfigPath(int nodeOrdinal) { public Collection> nodePlugins() { return OpenSearchIntegTestCase.this.nodePlugins(); } + + @Override + public Map, Class> extendedPlugins() { + return OpenSearchIntegTestCase.this.extendedPlugins(); + } }; }