connect both extensions and plugins; tests failing because client pas…

…sing change and need to update tests for passing Signed-off-by: Stephen Crawford <steecraw@amazon.com>
opensearch-project · Jul 31, 2023 · cc18cfe · cc18cfe
1 parent 56787c8
commit cc18cfe
Show file tree

Hide file tree

Showing 16 changed files with 67 additions and 31 deletions.
diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java
@@ -510,7 +510,7 @@ public class ActionModule extends AbstractModule {
     private final RequestValidators<PutMappingRequest> mappingRequestValidators;
     private final RequestValidators<IndicesAliasesRequest> indicesAliasesRequestRequestValidators;
     private final ThreadPool threadPool;
-    private final ExtensionsManager extensionsManager;
+    private final IdentityService identityService;
 
     public ActionModule(
         Settings settings,
@@ -534,7 +534,7 @@ public ActionModule(
         this.settingsFilter = settingsFilter;
         this.actionPlugins = actionPlugins;
         this.threadPool = threadPool;
-        this.extensionsManager = extensionsManager;
+        this.identityService = identityService;
         actions = setupActions(actionPlugins);
         actionFilters = setupActionFilters(actionPlugins);
         dynamicActionRegistry = new DynamicActionRegistry();
@@ -952,7 +952,7 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
 
         // Extensions API
         if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) {
-            registerHandler.accept(new RestInitializeExtensionAction(extensionsManager));
+            registerHandler.accept(new RestInitializeExtensionAction(identityService));
         }
 
         for (ActionPlugin plugin : actionPlugins) {

diff --git a/server/src/main/java/org/opensearch/cluster/ApplicationManager.java b/server/src/main/java/org/opensearch/cluster/ApplicationManager.java
@@ -14,6 +14,7 @@
 import org.opensearch.Application;
 import org.opensearch.common.collect.Tuple;
 import org.opensearch.extensions.ExtensionsManager;
+import org.opensearch.extensions.ExtensionsSettings;
 import org.opensearch.identity.ServiceAccountManager;
 import org.opensearch.plugins.Plugin;
 import org.opensearch.plugins.PluginInfo;
@@ -81,4 +82,13 @@ public void registerPluginsAndModules() {
             serviceAccountManager.getServiceAccount(pluginInfo);
         }
     }
+
+    /**
+     * Registers an Extension with the ApplicationManager
+     * @param extension The extension to be registered
+     */
+    public void registerExtension(ExtensionsSettings.Extension extension) {
+		registeredApplications.add(extension);
+        serviceAccountManager.getServiceAccount(extension);
+	}
 }
diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java
@@ -26,6 +26,7 @@
 import org.opensearch.action.ActionModule.DynamicActionRegistry;
 import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
 import org.opensearch.client.node.NodeClient;
+import org.opensearch.cluster.ApplicationManager;
 import org.opensearch.cluster.ClusterSettingsResponse;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.settings.Setting;

diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsSettings.java b/server/src/main/java/org/opensearch/extensions/ExtensionsSettings.java
@@ -8,9 +8,12 @@
 
 package org.opensearch.extensions;
 
+import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import org.opensearch.Application;
+import org.opensearch.identity.NamedPrincipal;
 
 /**
  * List of extension configurations from extension.yml
@@ -34,7 +37,7 @@ public ExtensionsSettings() {
      *
      * @opensearch.internal
     */
-    public static class Extension {
+    public static class Extension implements Application {
 
         private String name;
         private String uniqueId;
@@ -45,6 +48,7 @@ public static class Extension {
         private String minimumCompatibleVersion;
         private List<ExtensionDependency> dependencies = Collections.emptyList();
         private ExtensionScopedSettings additionalSettings;
+        private Principal extensionPrincipal;
 
         public Extension(
             String name,
@@ -66,6 +70,7 @@ public Extension(
             this.minimumCompatibleVersion = minimumCompatibleVersion;
             this.dependencies = dependencies;
             this.additionalSettings = additionalSettings;
+            extensionPrincipal = new NamedPrincipal(this.uniqueId);
         }
 
         public Extension() {
@@ -161,6 +166,10 @@ public String toString() {
                 + "]";
         }
 
+        @Override
+        public Principal getPrincipal() {
+            return extensionPrincipal;
+        }
     }
 
     public List<Extension> getExtensions() {

diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java
@@ -21,6 +21,7 @@
 import org.opensearch.extensions.ExtensionScopedSettings;
 import org.opensearch.extensions.ExtensionsManager;
 import org.opensearch.extensions.ExtensionsSettings.Extension;
+import org.opensearch.identity.IdentityService;
 import org.opensearch.rest.BaseRestHandler;
 import org.opensearch.rest.BytesRestResponse;
 import org.opensearch.rest.NamedRoute;
@@ -47,6 +48,7 @@
 public class RestInitializeExtensionAction extends BaseRestHandler {
 
     private final ExtensionsManager extensionsManager;
+    private final IdentityService identityService;
 
     @Override
     public String getName() {
@@ -58,8 +60,9 @@ public List<Route> routes() {
         return List.of(new NamedRoute.Builder().method(POST).path("/_extensions/initialize").uniqueName("extensions:initialize").build());
     }
 
-    public RestInitializeExtensionAction(ExtensionsManager extensionsManager) {
-        this.extensionsManager = extensionsManager;
+    public RestInitializeExtensionAction(IdentityService identityService) {
+        this.identityService = identityService;
+        this.extensionsManager = identityService.getApplicationManager().getExtensionManager();
     }
 
     @Override
@@ -160,6 +163,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
         );
         try {
             extensionsManager.loadExtension(extension);
+            identityService.getApplicationManager().registerExtension(extension);
             extensionsManager.initialize();
         } catch (CompletionException e) {
             Throwable cause = e.getCause();

diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java
@@ -34,6 +34,7 @@
 
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.common.SetOnce;
+import org.opensearch.common.lease.Releasables;
 import org.opensearch.common.settings.SettingsException;
 import org.opensearch.common.unit.ByteSizeUnit;
 import org.opensearch.common.unit.ByteSizeValue;

diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java
@@ -40,6 +40,13 @@
 import org.opensearch.core.common.Strings;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.core.xcontent.XContentBuilder;
+import org.opensearch.Application;
+import org.opensearch.Version;
+import org.opensearch.bootstrap.JarHell;
+import org.opensearch.core.common.Strings;
+import org.opensearch.core.xcontent.ToXContentObject;
+import org.opensearch.core.xcontent.XContentBuilder;
+import org.opensearch.identity.NamedPrincipal;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -54,16 +61,6 @@
 import java.util.Properties;
 import java.util.function.Function;
 import java.util.stream.Collectors;
-import org.opensearch.Application;
-import org.opensearch.Version;
-import org.opensearch.bootstrap.JarHell;
-import org.opensearch.common.io.stream.StreamInput;
-import org.opensearch.common.io.stream.StreamOutput;
-import org.opensearch.common.io.stream.Writeable;
-import org.opensearch.core.common.Strings;
-import org.opensearch.core.xcontent.ToXContentObject;
-import org.opensearch.core.xcontent.XContentBuilder;
-import org.opensearch.identity.NamedPrincipal;
 
 /**
  * An in-memory representation of the plugin descriptor.

diff --git a/server/src/test/java/org/opensearch/action/ActionModuleTests.java b/server/src/test/java/org/opensearch/action/ActionModuleTests.java
@@ -141,7 +141,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() throws IOException {
             null,
             usageService,
             null,
-            new IdentityService(Settings.EMPTY, new ArrayList<>()),
+            new IdentityService(Settings.EMPTY, new ArrayList<>(), null),
             new ExtensionsManager(Set.of())
         );
         actionModule.initRestHandlers(null);

diff --git a/server/src/test/java/org/opensearch/bootstrap/IdentityPluginTests.java b/server/src/test/java/org/opensearch/bootstrap/IdentityPluginTests.java
@@ -24,7 +24,7 @@ public class IdentityPluginTests extends OpenSearchTestCase {
     public void testSingleIdentityPluginSucceeds() {
         IdentityPlugin identityPlugin1 = new NoopIdentityPlugin();
         List<IdentityPlugin> pluginList1 = List.of(identityPlugin1);
-        IdentityService identityService1 = new IdentityService(Settings.EMPTY, pluginList1);
+        IdentityService identityService1 = new IdentityService(Settings.EMPTY, pluginList1, null);
         assertTrue(identityService1.getSubject().getPrincipal().getName().equalsIgnoreCase("Unauthenticated"));
         assertThat(identityService1.getTokenManager(), is(instanceOf(NoopTokenManager.class)));
     }
@@ -34,7 +34,7 @@ public void testMultipleIdentityPluginsFail() {
         IdentityPlugin identityPlugin2 = new NoopIdentityPlugin();
         IdentityPlugin identityPlugin3 = new NoopIdentityPlugin();
         List<IdentityPlugin> pluginList = List.of(identityPlugin1, identityPlugin2, identityPlugin3);
-        Exception ex = assertThrows(OpenSearchException.class, () -> new IdentityService(Settings.EMPTY, pluginList));
+        Exception ex = assertThrows(OpenSearchException.class, () -> new IdentityService(Settings.EMPTY, pluginList, null));
         assert (ex.getMessage().contains("Multiple identity plugins are not supported,"));
     }
 }
diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java
@@ -147,7 +147,7 @@ public List<Setting<?>> getExtensionSettings() {
             new NodeClient(Settings.EMPTY, threadPool),
             new NoneCircuitBreakerService(),
             new UsageService(),
-            new IdentityService(Settings.EMPTY, List.of())
+            new IdentityService(Settings.EMPTY, List.of(), null)
         );
         when(actionModule.getDynamicActionRegistry()).thenReturn(mock(DynamicActionRegistry.class));
         when(actionModule.getRestController()).thenReturn(restController);

diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java
@@ -18,6 +18,7 @@
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
@@ -26,6 +27,7 @@
 import org.junit.Before;
 import org.mockito.Mockito;
 import org.opensearch.Version;
+import org.opensearch.cluster.ApplicationManager;
 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.common.network.NetworkService;
 import org.opensearch.common.settings.Setting;
@@ -34,6 +36,7 @@
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.extensions.ExtensionsManager;
 import org.opensearch.extensions.ExtensionsSettings;
+import org.opensearch.identity.IdentityService;
 import org.opensearch.indices.breaker.NoneCircuitBreakerService;
 import org.opensearch.rest.RestRequest;
 import org.opensearch.core.common.bytes.BytesArray;
@@ -94,8 +97,8 @@ public void tearDown() throws Exception {
     }
 
     public void testRestInitializeExtensionActionResponse() throws Exception {
-        ExtensionsManager extensionsManager = mock(ExtensionsManager.class);
-        RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager);
+        IdentityService identityService = mock(IdentityService.class);
+        RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(identityService);
         final String content = "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\","
             + "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\""
             + Version.CURRENT.toString()
@@ -115,8 +118,8 @@ public void testRestInitializeExtensionActionResponse() throws Exception {
     }
 
     public void testRestInitializeExtensionActionFailure() throws Exception {
-        ExtensionsManager extensionsManager = new ExtensionsManager(Set.of());
-        RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager);
+        IdentityService identityService = mock(IdentityService.class);
+        RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(identityService);
 
         final String content = "{\"name\":\"ad-extension\",\"uniqueId\":\"\",\"hostAddress\":\"127.0.0.1\","
             + "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\""
@@ -148,14 +151,19 @@ public void testRestInitializeExtensionActionResponseWithAdditionalSettings() th
             Function.identity(),
             Setting.Property.ExtensionScope
         );
+
+        IdentityService identityService = spy(new IdentityService(Settings.EMPTY, List.of(), null));
+        ApplicationManager applicationManager = mock(ApplicationManager.class);
         ExtensionsManager extensionsManager = new ExtensionsManager(Set.of(boolSetting, stringSetting, intSetting, listSetting));
         ExtensionsManager spy = spy(extensionsManager);
 
         // optionally, you can stub out some methods:
+        doReturn(applicationManager).when(identityService.getApplicationManager());
+        when(applicationManager.getExtensionManager()).thenReturn(spy);
         when(spy.getAdditionalSettings()).thenCallRealMethod();
         Mockito.doCallRealMethod().when(spy).loadExtension(any(ExtensionsSettings.Extension.class));
         Mockito.doNothing().when(spy).initialize();
-        RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(spy);
+        RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(identityService);
         final String content = "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\","
             + "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\""
             + Version.CURRENT.toString()
@@ -195,14 +203,20 @@ public void testRestInitializeExtensionActionResponseWithAdditionalSettingsUsing
             Function.identity(),
             Setting.Property.ExtensionScope
         );
+        IdentityService identityService = spy(new IdentityService(Settings.EMPTY, List.of(), null));
+        ApplicationManager applicationManager = mock(ApplicationManager.class);
         ExtensionsManager extensionsManager = new ExtensionsManager(Set.of(boolSetting, stringSetting, intSetting, listSetting));
         ExtensionsManager spy = spy(extensionsManager);
 
+        // optionally, you can stub out some methods:
+        doReturn(applicationManager).when(identityService.getApplicationManager());
+        when(applicationManager.getExtensionManager()).thenReturn(spy);
+
         // optionally, you can stub out some methods:
         when(spy.getAdditionalSettings()).thenCallRealMethod();
         Mockito.doCallRealMethod().when(spy).loadExtension(any(ExtensionsSettings.Extension.class));
         Mockito.doNothing().when(spy).initialize();
-        RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(spy);
+        RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(identityService);
         final String content = "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\","
             + "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\""
             + Version.CURRENT.toString()

diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java
@@ -118,7 +118,7 @@ public void setup() throws Exception {
             null,
             usageService,
             null,
-            new IdentityService(Settings.EMPTY, new ArrayList<>()),
+            new IdentityService(Settings.EMPTY, new ArrayList<>(), null),
             new ExtensionsManager(Set.of())
         );
         dynamicActionRegistry = actionModule.getDynamicActionRegistry();

diff --git a/server/src/test/java/org/opensearch/rest/RestControllerTests.java b/server/src/test/java/org/opensearch/rest/RestControllerTests.java
@@ -112,7 +112,7 @@ public void setup() {
         // we can do this here only because we know that we don't adjust breaker settings dynamically in the test
         inFlightRequestsBreaker = circuitBreakerService.getBreaker(CircuitBreaker.IN_FLIGHT_REQUESTS);
 
-        identityService = new IdentityService(Settings.EMPTY, List.of());
+        identityService = new IdentityService(Settings.EMPTY, List.of(), null);
 
         HttpServerTransport httpServerTransport = new TestHttpServerTransport();
         client = new NoOpNodeClient(this.getTestName());

diff --git a/server/src/test/java/org/opensearch/rest/RestHttpResponseHeadersTests.java b/server/src/test/java/org/opensearch/rest/RestHttpResponseHeadersTests.java
@@ -106,7 +106,7 @@ public void testUnsupportedMethodResponseHttpHeader() throws Exception {
 
         final Settings settings = Settings.EMPTY;
         UsageService usageService = new UsageService();
-        final IdentityService identityService = new IdentityService(settings, List.of());
+        final IdentityService identityService = new IdentityService(settings, List.of(), null);
         RestController restController = new RestController(
             Collections.emptySet(),
             null,

diff --git a/.../src/test/java/org/opensearch/rest/action/admin/indices/RestValidateQueryActionTests.java b/.../src/test/java/org/opensearch/rest/action/admin/indices/RestValidateQueryActionTests.java
@@ -75,7 +75,7 @@ public class RestValidateQueryActionTests extends AbstractSearchTestCase {
     private static NodeClient client = new NodeClient(Settings.EMPTY, threadPool);
 
     private static UsageService usageService = new UsageService();
-    private static IdentityService identityService = new IdentityService(Settings.EMPTY, List.of());
+    private static IdentityService identityService = new IdentityService(Settings.EMPTY, List.of(), null);
     private static RestController controller = new RestController(
         emptySet(),
         null,

diff --git a/test/framework/src/main/java/org/opensearch/test/rest/RestActionTestCase.java b/test/framework/src/main/java/org/opensearch/test/rest/RestActionTestCase.java
@@ -67,7 +67,7 @@ public abstract class RestActionTestCase extends OpenSearchTestCase {
     @Before
     public void setUpController() {
         verifyingClient = new VerifyingClient(this.getTestName());
-        final IdentityService identityService = new IdentityService(Settings.EMPTY, List.of());
+        final IdentityService identityService = new IdentityService(Settings.EMPTY, List.of(), verifyingClient);
         controller = new RestController(
             Collections.emptySet(),
             null,