Enable testing for ExtensiblePlugins using classpath plugins

CHANGELOG.md

@@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Introduce framework for auxiliary transports and an experimental gRPC transport plugin ([#16534](https://github.com/opensearch-project/OpenSearch/pull/16534))
 - Changes to support IP field in star tree indexing([#16641](https://github.com/opensearch-project/OpenSearch/pull/16641/))
 - Support object fields in star-tree index([#16728](https://github.com/opensearch-project/OpenSearch/pull/16728/))
+- 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))

server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java

@@ -0,0 +1,58 @@
+/*
+ * 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.Map;
+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<Class<? extends Plugin>> nodePlugins() {
+        return Stream.concat(super.nodePlugins().stream(), Stream.of(SampleExtensiblePlugin.class, SampleExtendingPlugin.class))
+            .collect(Collectors.toList());
+    }
+
+    @Override
+    protected Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins() {
+        return Map.of(SampleExtendingPlugin.class, SampleExtensiblePlugin.class);
+    }
+
+    public void testPluginExtensionWithClasspathPlugins() throws IOException {
+        internalCluster().startNode();
+    }
+}

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

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<Class<? extends Plugin>> classpathPlugins,
-        boolean forbidPrivateIndexSettings
-    ) {
+    protected Node(final Environment initialEnvironment, Collection<PluginInfo> classpathPlugins, boolean forbidPrivateIndexSettings) {
         final List<Closeable> resourcesToClose = new ArrayList<>(); // register everything we need to release in the case of an error
         boolean success = false;
         try {

server/src/main/java/org/opensearch/plugins/PluginsService.java

@@ -125,12 +125,13 @@
      * @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<Class<? extends Plugin>> classpathPlugins
+        Collection<PluginInfo> classpathPlugins
     ) {
         this.settings = settings;
         this.configPath = configPath;
@@ -140,26 +141,21 @@
         // we need to build a List of plugins for checking mandatory plugins
         final List<String> pluginsNames = new ArrayList<>();
         // first we load plugins that are on the classpath. this is for tests
-        for (Class<? extends Plugin> pluginClass : classpathPlugins) {
-            Plugin plugin = loadPlugin(pluginClass, settings, configPath);
-            PluginInfo pluginInfo = new PluginInfo(
-                pluginClass.getName(),
-                "classpath plugin",
-                "NA",
-                Version.CURRENT,
-                "1.8",
-                pluginClass.getName(),
-                null,
-                Collections.emptyList(),
-                false
-            );
-            if (logger.isTraceEnabled()) {
-                logger.trace("plugin loaded from classpath [{}]", pluginInfo);
+        for (PluginInfo pluginInfo : classpathPlugins) {
+            try {
+                Class<? extends Plugin> pluginClazz = (Class<? extends Plugin>) 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);
 
         Set<Bundle> seenBundles = new LinkedHashSet<>();
         List<PluginInfo> modulesList = new ArrayList<>();

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<? extends Plugin>... classpathPlugins) {
-        return new PluginsService(
-            settings,
-            null,
-            null,
-            TestEnvironment.newEnvironment(settings).pluginsDir(),
-            Arrays.asList(classpathPlugins)
-        );
+        Collection<PluginInfo> pluginInfos = new ArrayList<>();
+        for (Class<? extends Plugin> 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() {

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<Class<? extends Plugin>> classpathPlugins;
 
-    public MockNode(final Settings settings, final Collection<Class<? extends Plugin>> classpathPlugins) {
-        this(settings, classpathPlugins, true);
-    }
-
     public MockNode(
         final Settings settings,
         final Collection<Class<? extends Plugin>> classpathPlugins,
@@ -100,6 +100,21 @@ public MockNode(
         this(settings, classpathPlugins, null, forbidPrivateIndexSettings);
     }
 
+    public MockNode(
+        final Settings settings,
+        final Collection<Class<? extends Plugin>> classpathPlugins,
+        final Path configPath,
+        final boolean forbidPrivateIndexSettings,
+        final Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins
+    ) {
+        this(
+            InternalSettingsPreparer.prepareEnvironment(settings, Collections.emptyMap(), configPath, () -> "mock_ node"),
+            classpathPlugins,
+            forbidPrivateIndexSettings,
+            extendedPlugins
+        );
+    }
+
     public MockNode(
         final Settings settings,
         final Collection<Class<? extends Plugin>> 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<Class<? extends Plugin>> classpathPlugins,
-        final boolean forbidPrivateIndexSettings
+        final boolean forbidPrivateIndexSettings,
+        final Map<Class<? extends Plugin>, Class<? extends Plugin>> 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<Class<? extends Plugin>> classpathPlugins) {
+        this(settings, classpathPlugins, true);
+    }
+
+    public MockNode(
+        final Settings settings,
+        final Collection<Class<? extends Plugin>> classpathPlugins,
+        final Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins
+    ) {
+        this(
+            InternalSettingsPreparer.prepareEnvironment(settings, Collections.emptyMap(), null, () -> "mock_ node"),
+            classpathPlugins,
+            true,
+            extendedPlugins
+        );
+    }
+
     /**
      * The classpath plugins this node was constructed with.
      */

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<PluginInfo> pluginInfos = new ArrayList<>();
+            for (Class<? extends Plugin> 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));

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<Client, Client> clientWrapper,
         String clusterName,
         Collection<Class<? extends Plugin>> pluginClasses,
+        Map<Class<? extends Plugin>, Class<? extends Plugin>> 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();