From 62389485d9a2c484f2580d10b501eec8927042f6 Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Wed, 7 Jun 2023 16:26:11 -0700 Subject: [PATCH 01/12] Implemented REST API for initializing extension Signed-off-by: Owais Kazi --- .../org/opensearch/action/ActionModule.java | 12 +- .../extensions/ExtensionDependency.java | 22 +++ .../extensions/ExtensionsManager.java | 172 ++++-------------- .../extensions/NoopExtensionsManager.java | 5 - .../main/java/org/opensearch/node/Node.java | 4 +- .../RestInitializeExtensionAction.java | 123 +++++++++++++ 6 files changed, 190 insertions(+), 148 deletions(-) create mode 100644 server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 168fbae84fdf4..1581991a64f9d 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -294,8 +294,10 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsFilter; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionProxyAction; import org.opensearch.extensions.action.ExtensionProxyTransportAction; +import org.opensearch.rest.extensions.RestInitializeExtensionAction; import org.opensearch.index.seqno.RetentionLeaseActions; import org.opensearch.identity.IdentityService; import org.opensearch.indices.SystemIndices; @@ -508,6 +510,7 @@ public class ActionModule extends AbstractModule { private final RequestValidators mappingRequestValidators; private final RequestValidators indicesAliasesRequestRequestValidators; private final ThreadPool threadPool; + private final ExtensionsManager extensionsManager; public ActionModule( Settings settings, @@ -521,7 +524,8 @@ public ActionModule( CircuitBreakerService circuitBreakerService, UsageService usageService, SystemIndices systemIndices, - IdentityService identityService + IdentityService identityService, + ExtensionsManager extensionsManager ) { this.settings = settings; this.indexNameExpressionResolver = indexNameExpressionResolver; @@ -530,6 +534,7 @@ public ActionModule( this.settingsFilter = settingsFilter; this.actionPlugins = actionPlugins; this.threadPool = threadPool; + this.extensionsManager = extensionsManager; actions = setupActions(actionPlugins); actionFilters = setupActionFilters(actionPlugins); dynamicActionRegistry = new DynamicActionRegistry(); @@ -947,6 +952,11 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestDeleteSearchPipelineAction()); } + // Extensions API + if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) { + registerHandler.accept(new RestInitializeExtensionAction(extensionsManager)); + } + for (ActionPlugin plugin : actionPlugins) { for (RestHandler handler : plugin.getRestHandlers( settings, diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java b/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java index d2106cf8d399c..6205529f8affa 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java @@ -16,6 +16,9 @@ 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.xcontent.XContentParser; + +import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; /** * This class handles the dependent extensions information @@ -54,6 +57,25 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVersion(version); } + public static ExtensionDependency parse(XContentParser parser) throws IOException { + String uniqueId = null; + Version version = null; + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String fieldName = parser.currentName(); + parser.nextToken(); + + if ("uniqueId".equals(fieldName)) { + uniqueId = parser.text(); + } else if ("version".equals(fieldName)) { + version = Version.fromString(parser.text()); + } + + } + return new ExtensionDependency(uniqueId, version); + + } + /** * The uniqueId of the dependency extension * diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index 9d74e8f22d2b1..07b108e65dce9 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -9,12 +9,10 @@ package org.opensearch.extensions; import java.io.IOException; -import java.io.InputStream; import java.net.InetAddress; import java.nio.file.Files; +import java.net.UnknownHostException; import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -26,7 +24,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; -import java.util.Arrays; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -41,6 +38,7 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Setting; import org.opensearch.core.util.FileSystemUtils; +import org.opensearch.common.util.concurrent.AbstractRunnable; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsModule; @@ -65,7 +63,6 @@ import org.opensearch.transport.TransportResponse; import org.opensearch.transport.TransportResponseHandler; import org.opensearch.transport.TransportService; -import org.yaml.snakeyaml.Yaml; import org.opensearch.env.EnvironmentSettingsResponse; /** @@ -137,12 +134,6 @@ public ExtensionsManager(Path extensionsPath, Set> additionalSettings } this.client = null; this.extensionTransportActionsHandler = null; - - /* - * Now Discover extensions - */ - discover(); - } /** @@ -306,40 +297,13 @@ private void registerRequestHandler(DynamicActionRegistry dynamicActionRegistry) ); } - /* - * Load and populate all extensions - */ - protected void discover() throws IOException { - logger.info("Loading extensions : {}", extensionsPath); - if (!FileSystemUtils.isAccessibleDirectory(extensionsPath, logger)) { - return; - } - - List extensions = new ArrayList(); - if (Files.exists(extensionsPath.resolve("extensions.yml"))) { - try { - extensions = readFromExtensionsYml(extensionsPath.resolve("extensions.yml")).getExtensions(); - } catch (IOException e) { - throw new IOException("Could not read from extensions.yml", e); - } - for (Extension extension : extensions) { - loadExtension(extension); - } - if (!extensionIdMap.isEmpty()) { - logger.info("Loaded all extensions"); - } - } else { - logger.warn("Extensions.yml file is not present. No extensions will be loaded."); - } - } - /** * Loads a single extension * @param extension The extension to be loaded */ - private void loadExtension(Extension extension) throws IOException { + public void loadExtension(Extension extension) throws IOException { if (extensionIdMap.containsKey(extension.getUniqueId())) { - logger.info("Duplicate uniqueId " + extension.getUniqueId() + ". Did not load extension: " + extension); + throw new IOException("Duplicate uniqueId " + extension.getUniqueId() + ". Did not load extension: " + extension); } else { try { DiscoveryExtensionNode discoveryExtensionNode = new DiscoveryExtensionNode( @@ -351,12 +315,11 @@ private void loadExtension(Extension extension) throws IOException { Version.fromString(extension.getMinimumCompatibleVersion()), extension.getDependencies() ); - extensionIdMap.put(extension.getUniqueId(), discoveryExtensionNode); extensionSettingsMap.put(extension.getUniqueId(), extension); logger.info("Loaded extension with uniqueId " + extension.getUniqueId() + ": " + extension); } catch (OpenSearchException e) { - logger.error("Could not load extension with uniqueId " + extension.getUniqueId() + " due to " + e); + throw new OpenSearchException("Could not load extension with uniqueId " + extension.getUniqueId() + " due to " + e); } catch (IllegalArgumentException e) { throw e; } @@ -407,27 +370,34 @@ public String executor() { return ThreadPool.Names.GENERIC; } }; - try { - logger.info("Sending extension request type: " + REQUEST_EXTENSION_ACTION_NAME); - transportService.connectToExtensionNode(extension); - transportService.sendRequest( - extension, - REQUEST_EXTENSION_ACTION_NAME, - new InitializeExtensionRequest(transportService.getLocalNode(), extension), - initializeExtensionResponseHandler - ); - inProgressFuture.orTimeout(EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS).join(); - } catch (CompletionException | ConnectTransportException e) { - if (e.getCause() instanceof TimeoutException || e instanceof ConnectTransportException) { - logger.info("No response from extension to request.", e); - } else if (e.getCause() instanceof RuntimeException) { - throw (RuntimeException) e.getCause(); - } else if (e.getCause() instanceof Error) { - throw (Error) e.getCause(); - } else { - throw new RuntimeException(e.getCause()); + + logger.info("Sending extension request type: " + REQUEST_EXTENSION_ACTION_NAME); + transportService.getThreadPool().generic().execute(new AbstractRunnable() { + @Override + public void onFailure(Exception e) { + if (e.getCause() instanceof TimeoutException || e instanceof ConnectTransportException) { + logger.info("No response from extension to request.", e); + } else if (e.getCause() instanceof RuntimeException) { + throw (RuntimeException) e.getCause(); + } else if (e.getCause() instanceof Error) { + throw (Error) e.getCause(); + } else { + throw new RuntimeException(e.getCause()); + } } - } + + @Override + protected void doRun() throws Exception { + transportService.connectToExtensionNode(extension); + transportService.sendRequest( + extension, + REQUEST_EXTENSION_ACTION_NAME, + new InitializeExtensionRequest(transportService.getLocalNode(), extension), + initializeExtensionResponseHandler + ); + inProgressFuture.orTimeout(EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS).join(); + } + }); } /** @@ -466,84 +436,6 @@ TransportResponse handleExtensionRequest(ExtensionRequest extensionRequest) thro } } - private ExtensionsSettings readFromExtensionsYml(Path filePath) throws IOException { - Yaml yaml = new Yaml(); - try (InputStream inputStream = Files.newInputStream(filePath)) { - Map obj = yaml.load(inputStream); - if (obj == null) { - inputStream.close(); - throw new IOException("extensions.yml is empty"); - } - List> unreadExtensions = new ArrayList<>((Collection>) obj.get("extensions")); - List readExtensions = new ArrayList(); - Set additionalSettingsKeys = additionalSettings.stream().map(s -> s.getKey()).collect(Collectors.toSet()); - for (HashMap extensionMap : unreadExtensions) { - try { - // checking to see whether any required fields are missing from extension.yml file or not - String[] requiredFields = { - "name", - "uniqueId", - "hostAddress", - "port", - "version", - "opensearchVersion", - "minimumCompatibleVersion" }; - List missingFields = Arrays.stream(requiredFields) - .filter(field -> !extensionMap.containsKey(field)) - .collect(Collectors.toList()); - if (!missingFields.isEmpty()) { - throw new IOException("Extension is missing these required fields : " + missingFields); - } - - // Parse extension dependencies - List extensionDependencyList = new ArrayList(); - if (extensionMap.get("dependencies") != null) { - List> extensionDependencies = new ArrayList<>( - (Collection>) extensionMap.get("dependencies") - ); - for (HashMap dependency : extensionDependencies) { - extensionDependencyList.add( - new ExtensionDependency( - dependency.get("uniqueId").toString(), - Version.fromString(dependency.get("version").toString()) - ) - ); - } - } - - ExtensionScopedSettings extAdditionalSettings = new ExtensionScopedSettings(additionalSettings); - Map additionalSettingsMap = extensionMap.entrySet() - .stream() - .filter(kv -> additionalSettingsKeys.contains(kv.getKey())) - .collect(Collectors.toMap(map -> map.getKey(), map -> map.getValue())); - - Settings.Builder output = Settings.builder(); - output.loadFromMap(additionalSettingsMap); - extAdditionalSettings.applySettings(output.build()); - - // Create extension read from yml config - readExtensions.add( - new Extension( - extensionMap.get("name").toString(), - extensionMap.get("uniqueId").toString(), - extensionMap.get("hostAddress").toString(), - extensionMap.get("port").toString(), - extensionMap.get("version").toString(), - extensionMap.get("opensearchVersion").toString(), - extensionMap.get("minimumCompatibleVersion").toString(), - extensionDependencyList, - extAdditionalSettings - ) - ); - } catch (IOException e) { - logger.warn("loading extension has been failed because of exception : " + e.getMessage()); - } - } - inputStream.close(); - return new ExtensionsSettings(readExtensions); - } - } - static String getRequestExtensionActionName() { return REQUEST_EXTENSION_ACTION_NAME; } diff --git a/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java index fb7160bc1bc67..155a0da1a6bc6 100644 --- a/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java @@ -59,11 +59,6 @@ public ExtensionActionResponse handleTransportRequest(ExtensionActionRequest req return new ExtensionActionResponse(new byte[0]); } - @Override - protected void discover() throws IOException { - // no-op - } - @Override public void initialize() { // no-op diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 808c054de4969..268f2ad174625 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -803,7 +803,8 @@ protected Node( circuitBreakerService, usageService, systemIndices, - identityService + identityService, + extensionsManager ); modules.add(actionModule); @@ -1306,7 +1307,6 @@ public Node start() throws NodeValidationException { assert clusterService.localNode().equals(localNodeFactory.getNode()) : "clusterService has a different local node than the factory provided"; transportService.acceptIncomingRequests(); - extensionsManager.initialize(); discovery.startInitialJoin(); final TimeValue initialStateTimeout = DiscoverySettings.INITIAL_STATE_TIMEOUT_SETTING.get(settings()); configureNodeAndClusterIdStateListener(clusterService); diff --git a/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java new file mode 100644 index 0000000000000..0d42f21612924 --- /dev/null +++ b/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java @@ -0,0 +1,123 @@ +/* + * 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.rest.extensions; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.client.node.NodeClient; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.extensions.ExtensionDependency; +import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.extensions.ExtensionsSettings.Extension; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestStatus; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; +import static org.opensearch.rest.RestRequest.Method.POST; + +public class RestInitializeExtensionAction extends BaseRestHandler { + + private final ExtensionsManager extensionsManager; + private static final Logger logger = LogManager.getLogger(RestInitializeExtensionAction.class); + + @Override + public String getName() { + return ExtensionsManager.REQUEST_EXTENSION_ACTION_NAME; + } + + @Override + public List routes() { + return List.of(new Route(POST, "/_extensions/initialize")); + } + + public RestInitializeExtensionAction(ExtensionsManager extensionsManager) { + this.extensionsManager = extensionsManager; + } + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + String name = ""; + String uniqueId = ""; + String hostAddress = ""; + String port = ""; + String version = ""; + String openSearchVersion = ""; + String minimumCompatibleVersion = ""; + List dependencies = new ArrayList<>(); + + if (request.hasContent()) { + try (XContentParser parser = request.contentParser()) { + parser.nextToken(); + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String currentFieldName = parser.currentName(); + parser.nextToken(); + if ("name".equals(currentFieldName)) { + name = parser.text(); + } else if ("uniqueId".equals(currentFieldName)) { + uniqueId = parser.text(); + } else if ("hostAddress".equals(currentFieldName)) { + hostAddress = parser.text(); + } else if ("port".equals(currentFieldName)) { + port = parser.text(); + } else if ("version".equals(currentFieldName)) { + version = parser.text(); + } else if ("opensearchVersion".equals(currentFieldName)) { + openSearchVersion = parser.text(); + } else if ("minimumCompatibleVersion".equals(currentFieldName)) { + minimumCompatibleVersion = parser.text(); + } else if ("dependencies".equals(currentFieldName)) { + ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + dependencies.add(ExtensionDependency.parse(parser)); + } + } + } + } + } + + Extension extension = new Extension( + name, + uniqueId, + hostAddress, + port, + version, + openSearchVersion, + minimumCompatibleVersion, + dependencies, + //TODO create parser for additionalSettings + null + ); + try { + extensionsManager.loadExtension(extension); + extensionsManager.initialize(); + } catch (IOException e) { + logger.error(e); + return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); + + } + + return channel -> { + try (XContentBuilder builder = channel.newBuilder()) { + builder.startObject(); + builder.field("Extension has been initialized"); + builder.endObject(); + channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder)); + } + }; + + } +} From 2a35228856d7d6c697de86ec5d70b8800839e3ba Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Thu, 8 Jun 2023 17:20:52 -0700 Subject: [PATCH 02/12] Cleanup extensions.yml design Signed-off-by: Owais Kazi --- .../org/opensearch/bootstrap/Security.java | 1 - .../java/org/opensearch/env/Environment.java | 4 - .../extensions/ExtensionsManager.java | 76 ++--- .../extensions/NoopExtensionsManager.java | 3 +- .../main/java/org/opensearch/node/Node.java | 2 +- .../RestInitializeExtensionAction.java | 5 +- .../opensearch/action/ActionModuleTests.java | 9 +- .../extensions/ExtensionsManagerTests.java | 290 ++++++++---------- .../rest/RestInitializeExtensionTests.java | 13 + .../rest/RestSendToExtensionActionTests.java | 4 +- 10 files changed, 203 insertions(+), 204 deletions(-) create mode 100644 server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java diff --git a/server/src/main/java/org/opensearch/bootstrap/Security.java b/server/src/main/java/org/opensearch/bootstrap/Security.java index 37c7fd5c0a96c..749c146de4f16 100644 --- a/server/src/main/java/org/opensearch/bootstrap/Security.java +++ b/server/src/main/java/org/opensearch/bootstrap/Security.java @@ -316,7 +316,6 @@ static void addFilePermissions(Permissions policy, Environment environment) thro addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.libDir(), "read,readlink", false); addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.modulesDir(), "read,readlink", false); addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.pluginsDir(), "read,readlink", false); - addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.extensionDir(), "read,readlink", false); addDirectoryPath(policy, "path.conf'", environment.configDir(), "read,readlink", false); // read-write dirs addDirectoryPath(policy, "java.io.tmpdir", environment.tmpDir(), "read,readlink,write,delete", false); diff --git a/server/src/main/java/org/opensearch/env/Environment.java b/server/src/main/java/org/opensearch/env/Environment.java index 938bca58c7081..a1e467ad1ba48 100644 --- a/server/src/main/java/org/opensearch/env/Environment.java +++ b/server/src/main/java/org/opensearch/env/Environment.java @@ -311,10 +311,6 @@ public Path pluginsDir() { return pluginsDir; } - public Path extensionDir() { - return extensionsDir; - } - public Path binDir() { return binDir; } diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index 07b108e65dce9..fad71880765eb 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -10,17 +10,12 @@ import java.io.IOException; import java.net.InetAddress; -import java.nio.file.Files; -import java.net.UnknownHostException; -import java.nio.file.Path; import java.util.HashMap; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; @@ -28,7 +23,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; -import org.opensearch.OpenSearchException; import org.opensearch.Version; import org.opensearch.action.ActionModule; import org.opensearch.action.ActionModule.DynamicActionRegistry; @@ -37,7 +31,7 @@ import org.opensearch.cluster.ClusterSettingsResponse; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Setting; -import org.opensearch.core.util.FileSystemUtils; +import org.opensearch.core.common.Strings; import org.opensearch.common.util.concurrent.AbstractRunnable; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.settings.Settings; @@ -97,7 +91,6 @@ public static enum OpenSearchRequestType { REQUEST_OPENSEARCH_NAMED_WRITEABLE_REGISTRY } - private final Path extensionsPath; private ExtensionTransportActionsHandler extensionTransportActionsHandler; private Map extensionSettingsMap; private Map initializedExtensions; @@ -114,13 +107,11 @@ public static enum OpenSearchRequestType { /** * Instantiate a new ExtensionsManager object to handle requests and responses from extensions. This is called during Node bootstrap. * - * @param extensionsPath Path to a directory containing extensions. * @param additionalSettings Additional settings to read in from extensions.yml * @throws IOException If the extensions discovery file is not properly retrieved. */ - public ExtensionsManager(Path extensionsPath, Set> additionalSettings) throws IOException { + public ExtensionsManager(Set> additionalSettings) throws IOException { logger.info("ExtensionsManager initialized"); - this.extensionsPath = extensionsPath; this.initializedExtensions = new HashMap(); this.extensionIdMap = new HashMap(); this.extensionSettingsMap = new HashMap(); @@ -302,28 +293,47 @@ private void registerRequestHandler(DynamicActionRegistry dynamicActionRegistry) * @param extension The extension to be loaded */ public void loadExtension(Extension extension) throws IOException { - if (extensionIdMap.containsKey(extension.getUniqueId())) { + try { + validateExtension(extension); + DiscoveryExtensionNode discoveryExtensionNode = new DiscoveryExtensionNode( + extension.getName(), + extension.getUniqueId(), + new TransportAddress(InetAddress.getByName(extension.getHostAddress()), Integer.parseInt(extension.getPort())), + new HashMap(), + Version.fromString(extension.getOpensearchVersion()), + Version.fromString(extension.getMinimumCompatibleVersion()), + extension.getDependencies() + ); + extensionIdMap.put(extension.getUniqueId(), discoveryExtensionNode); + extensionSettingsMap.put(extension.getUniqueId(), extension); + logger.info("Loaded extension with uniqueId " + extension.getUniqueId() + ": " + extension); + } catch (IOException e) { + throw e; + } catch (IllegalArgumentException e) { + throw e; + } + + } + + private boolean validateExtension(Extension extension) throws IOException { + if (Strings.isNullOrEmpty(extension.getName())) { + throw new IOException("Required field [name] is missing in the request"); + } else if (Strings.isNullOrEmpty(extension.getUniqueId())) { + throw new IOException("Required field [uniqueId] is missing in the request"); + } else if (Strings.isNullOrEmpty(extension.getHostAddress())) { + throw new IOException("Required field [extension host address] is missing in the request"); + } else if (Strings.isNullOrEmpty(extension.getPort())) { + throw new IOException("Required field [extension port] is missing in the request"); + } else if (Strings.isNullOrEmpty(extension.getVersion())) { + throw new IOException("Required field [extension version] is missing in the request"); + } else if (Strings.isNullOrEmpty(extension.getOpensearchVersion())) { + throw new IOException("Required field [opensearch version] is missing in the request"); + } else if (Strings.isNullOrEmpty(extension.getMinimumCompatibleVersion())) { + throw new IOException("Required field [minimum opensearch version] is missing in the request"); + } else if (extensionIdMap.containsKey(extension.getUniqueId())) { throw new IOException("Duplicate uniqueId " + extension.getUniqueId() + ". Did not load extension: " + extension); - } else { - try { - DiscoveryExtensionNode discoveryExtensionNode = new DiscoveryExtensionNode( - extension.getName(), - extension.getUniqueId(), - new TransportAddress(InetAddress.getByName(extension.getHostAddress()), Integer.parseInt(extension.getPort())), - new HashMap(), - Version.fromString(extension.getOpensearchVersion()), - Version.fromString(extension.getMinimumCompatibleVersion()), - extension.getDependencies() - ); - extensionIdMap.put(extension.getUniqueId(), discoveryExtensionNode); - extensionSettingsMap.put(extension.getUniqueId(), extension); - logger.info("Loaded extension with uniqueId " + extension.getUniqueId() + ": " + extension); - } catch (OpenSearchException e) { - throw new OpenSearchException("Could not load extension with uniqueId " + extension.getUniqueId() + " due to " + e); - } catch (IllegalArgumentException e) { - throw e; - } } + return true; } /** @@ -452,10 +462,6 @@ static Logger getLogger() { return logger; } - Path getExtensionsPath() { - return extensionsPath; - } - TransportService getTransportService() { return transportService; } diff --git a/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java index 155a0da1a6bc6..d434074279041 100644 --- a/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/NoopExtensionsManager.java @@ -9,7 +9,6 @@ package org.opensearch.extensions; import java.io.IOException; -import java.nio.file.Path; import java.util.Optional; import java.util.Set; @@ -32,7 +31,7 @@ public class NoopExtensionsManager extends ExtensionsManager { public NoopExtensionsManager() throws IOException { - super(Path.of(""), Set.of()); + super(Set.of()); } @Override diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 268f2ad174625..688f2d05b203b 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -475,7 +475,7 @@ protected Node( for (ExtensionAwarePlugin extAwarePlugin : extensionAwarePlugins) { additionalSettings.addAll(extAwarePlugin.getExtensionSettings()); } - this.extensionsManager = new ExtensionsManager(initialEnvironment.extensionDir(), additionalSettings); + this.extensionsManager = new ExtensionsManager(additionalSettings); } else { this.extensionsManager = new NoopExtensionsManager(); } diff --git a/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java index 0d42f21612924..b9820ebbb9ab0 100644 --- a/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java +++ b/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java @@ -28,6 +28,9 @@ import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; import static org.opensearch.rest.RestRequest.Method.POST; +/** + * An action that initializes an extension + */ public class RestInitializeExtensionAction extends BaseRestHandler { private final ExtensionsManager extensionsManager; @@ -98,7 +101,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli openSearchVersion, minimumCompatibleVersion, dependencies, - //TODO create parser for additionalSettings + // TODO create parser for additionalSettings null ); try { diff --git a/server/src/test/java/org/opensearch/action/ActionModuleTests.java b/server/src/test/java/org/opensearch/action/ActionModuleTests.java index 66af9ebfd814f..109c60aa1e4f1 100644 --- a/server/src/test/java/org/opensearch/action/ActionModuleTests.java +++ b/server/src/test/java/org/opensearch/action/ActionModuleTests.java @@ -46,6 +46,7 @@ import org.opensearch.common.settings.SettingsFilter; import org.opensearch.common.settings.SettingsModule; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.extensions.ExtensionsManager; import org.opensearch.identity.IdentityService; import org.opensearch.plugins.ActionPlugin; import org.opensearch.plugins.ActionPlugin.ActionHandler; @@ -65,6 +66,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import java.util.function.Supplier; import static java.util.Collections.emptyList; @@ -124,7 +126,7 @@ protected FakeAction() { ); } - public void testSetupRestHandlerContainsKnownBuiltin() { + public void testSetupRestHandlerContainsKnownBuiltin() throws IOException { SettingsModule settings = new SettingsModule(Settings.EMPTY); UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule( @@ -139,7 +141,8 @@ public void testSetupRestHandlerContainsKnownBuiltin() { null, usageService, null, - new IdentityService(Settings.EMPTY, new ArrayList<>()) + new IdentityService(Settings.EMPTY, new ArrayList<>()), + new ExtensionsManager(Set.of()) ); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail @@ -196,6 +199,7 @@ public String getName() { null, usageService, null, + null, null ); Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null)); @@ -246,6 +250,7 @@ public List getRestHandlers( null, usageService, null, + null, null ); actionModule.initRestHandlers(null); diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index 75a1d9ec62c82..9465db5a50af0 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -23,10 +23,7 @@ import java.io.IOException; import java.net.InetAddress; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; import java.nio.file.Path; -import java.security.AccessControlException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -40,6 +37,7 @@ import org.apache.logging.log4j.LogManager; import org.junit.After; import org.junit.Before; +import org.opensearch.OpenSearchException; import org.opensearch.Version; import org.opensearch.action.ActionModule; import org.opensearch.action.ActionModule.DynamicActionRegistry; @@ -52,7 +50,6 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.bytes.BytesReference; -import org.opensearch.common.io.PathUtils; import org.opensearch.common.io.stream.BytesStreamInput; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.io.stream.NamedWriteableRegistry; @@ -69,6 +66,7 @@ import org.opensearch.extensions.proto.ExtensionRequestProto; import org.opensearch.extensions.rest.RegisterRestActionsRequest; import org.opensearch.extensions.settings.RegisterCustomSettingsRequest; +import org.opensearch.extensions.ExtensionsSettings.Extension; import org.opensearch.identity.IdentityService; import org.opensearch.indices.breaker.NoneCircuitBreakerService; import org.opensearch.plugins.ExtensionAwarePlugin; @@ -203,10 +201,37 @@ public void tearDown() throws Exception { ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS); } - public void testDiscover() throws Exception { - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); + public void testLoadExtensions() throws Exception { - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + Set> additionalSettings = extAwarePlugin.getExtensionSettings().stream().collect(Collectors.toSet()); + ExtensionScopedSettings extensionScopedSettings = new ExtensionScopedSettings(additionalSettings); + ExtensionsManager extensionsManager = new ExtensionsManager(additionalSettings); + ExtensionDependency dependentExtension = new ExtensionDependency("uniqueid0", Version.fromString("2.0.0")); + + Extension firstExtension = new Extension( + "firstExtension", + "uniqueid1", + "127.0.0.1", + "9300", + "0.0.7", + "3.0.0", + "3.0.0", + Collections.emptyList(), + extensionScopedSettings + ); + Extension secondExtension = new Extension( + "secondExtension", + "uniqueid2", + "127.0.0.1", + "9301", + "0.0.7", + "2.0.0", + "2.0.0", + List.of(dependentExtension), + extensionScopedSettings + ); + extensionsManager.loadExtension(firstExtension); + extensionsManager.loadExtension(secondExtension); List expectedExtensions = new ArrayList(); @@ -218,7 +243,7 @@ public void testDiscover() throws Exception { new DiscoveryExtensionNode( "firstExtension", "uniqueid1", - new TransportAddress(InetAddress.getByName("127.0.0.0"), 9300), + new TransportAddress(InetAddress.getByName("127.0.0.1"), 9300), new HashMap(), Version.fromString("3.0.0"), Version.fromString("3.0.0"), @@ -252,14 +277,37 @@ public void testDiscover() throws Exception { } } - public void testNonUniqueExtensionsDiscovery() throws Exception { - Path emptyExtensionDir = createTempDir(); - List nonUniqueYmlLines = extensionsYmlLines.stream() - .map(s -> s.replace("uniqueid2", "uniqueid1")) - .collect(Collectors.toList()); - Files.write(emptyExtensionDir.resolve("extensions.yml"), nonUniqueYmlLines, StandardCharsets.UTF_8); + public void testNonUniqueLoadedExtensions() throws Exception { - ExtensionsManager extensionsManager = new ExtensionsManager(emptyExtensionDir, Set.of()); + Extension firstExtension = new Extension( + "firstExtension", + "uniqueid1", + "127.0.0.0", + "9300", + "0.0.7", + "3.0.0", + "3.0.0", + Collections.emptyList(), + null + ); + Extension secondExtension = new Extension( + "secondExtension", + "uniqueid1", + "127.0.0.0", + "9300", + "0.0.7", + "3.0.0", + "3.0.0", + null, + null + ); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); + extensionsManager.loadExtension(firstExtension); + IOException exception = expectThrows(IOException.class, () -> extensionsManager.loadExtension(secondExtension)); + assertEquals( + "Duplicate uniqueId uniqueid1. Did not load extension: Extension [name=secondExtension, uniqueId=uniqueid1, hostAddress=127.0.0.0, port=9300, version=0.0.7, opensearchVersion=3.0.0, minimumCompatibleVersion=3.0.0]", + exception.getMessage() + ); List expectedExtensions = new ArrayList(); @@ -289,56 +337,15 @@ public void testNonUniqueExtensionsDiscovery() throws Exception { assertTrue(expectedExtensions.containsAll(emptyList())); } - public void testMissingRequiredFieldsInExtensionDiscovery() throws Exception { - Path emptyExtensionDir = createTempDir(); - ExtensionsManager extensionsManager; - List requiredFieldMissingYmlLines = extensionsYmlLines.stream() - .map(s -> s.replace(" minimumCompatibleVersion: '2.0.0'", "")) - .collect(Collectors.toList()); - Files.write(emptyExtensionDir.resolve("extensions.yml"), requiredFieldMissingYmlLines, StandardCharsets.UTF_8); + public void testMissingRequiredFieldsWhileLoadingExtension() throws Exception { - try (MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(ExtensionsManager.class))) { + Extension firstExtension = new Extension("firstExtension", "uniqueid1", "127.0.0.0", "9300", "0.0.7", "3.0.0", "", null, null); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); - mockLogAppender.addExpectation( - new MockLogAppender.SeenEventExpectation( - "Required field is missing in extensions.yml", - "org.opensearch.extensions.ExtensionsManager", - Level.WARN, - "loading extension has been failed because of exception : Extension is missing these required fields : [minimumCompatibleVersion]" - ) - ); - - extensionsManager = new ExtensionsManager(emptyExtensionDir, Set.of()); - - mockLogAppender.assertAllExpectationsMatched(); - } + IOException exception = expectThrows(IOException.class, () -> extensionsManager.loadExtension(firstExtension)); + assertEquals("Required field [minimum opensearch version] is missing in the request", exception.getMessage()); - List expectedExtensions = new ArrayList(); - - expectedExtensions.add( - new DiscoveryExtensionNode( - "firstExtension", - "uniqueid1", - new TransportAddress(InetAddress.getByName("127.0.0.0"), 9300), - new HashMap(), - Version.fromString("3.0.0"), - Version.fromString("3.0.0"), - Collections.emptyList() - ) - ); - assertEquals(expectedExtensions.size(), extensionsManager.getExtensionIdMap().values().size()); - for (DiscoveryExtensionNode extension : expectedExtensions) { - DiscoveryExtensionNode initializedExtension = extensionsManager.getExtensionIdMap().get(extension.getId()); - assertEquals(extension.getName(), initializedExtension.getName()); - assertEquals(extension.getId(), initializedExtension.getId()); - assertEquals(extension.getAddress(), initializedExtension.getAddress()); - assertEquals(extension.getAttributes(), initializedExtension.getAttributes()); - assertEquals(extension.getVersion(), initializedExtension.getVersion()); - assertEquals(extension.getMinimumCompatibleVersion(), initializedExtension.getMinimumCompatibleVersion()); - assertEquals(extension.getDependencies(), initializedExtension.getDependencies()); - } - assertTrue(expectedExtensions.containsAll(emptyList())); - assertTrue(expectedExtensions.containsAll(emptyList())); + assertEquals(0, extensionsManager.getExtensionIdMap().values().size()); } public void testDiscoveryExtension() throws Exception { @@ -389,49 +396,8 @@ public void testExtensionDependency() throws Exception { } } - public void testNonAccessibleDirectory() throws Exception { - AccessControlException e = expectThrows( - - AccessControlException.class, - () -> new ExtensionsManager(PathUtils.get(""), Set.of()) - ); - assertEquals("access denied (\"java.io.FilePermission\" \"\" \"read\")", e.getMessage()); - } - - public void testNoExtensionsFile() throws Exception { - Settings settings = Settings.builder().build(); - - try (MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(ExtensionsManager.class))) { - - mockLogAppender.addExpectation( - new MockLogAppender.SeenEventExpectation( - "No Extensions File Present", - "org.opensearch.extensions.ExtensionsManager", - Level.WARN, - "Extensions.yml file is not present. No extensions will be loaded." - ) - ); - - new ExtensionsManager(extensionDir, Set.of()); - - mockLogAppender.assertAllExpectationsMatched(); - } - } - - public void testEmptyExtensionsFile() throws Exception { - Path emptyExtensionDir = createTempDir(); - - List emptyExtensionsYmlLines = Arrays.asList(); - Files.write(emptyExtensionDir.resolve("extensions.yml"), emptyExtensionsYmlLines, StandardCharsets.UTF_8); - - Settings settings = Settings.builder().build(); - - expectThrows(IOException.class, () -> new ExtensionsManager(emptyExtensionDir, Set.of())); - } - public void testInitialize() throws Exception { - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); @@ -472,9 +438,8 @@ public void testInitialize() throws Exception { } public void testHandleRegisterRestActionsRequest() throws Exception { - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); String uniqueIdStr = "uniqueid1"; @@ -488,8 +453,7 @@ public void testHandleRegisterRestActionsRequest() throws Exception { } public void testHandleRegisterSettingsRequest() throws Exception { - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); String uniqueIdStr = "uniqueid1"; @@ -505,7 +469,7 @@ public void testHandleRegisterSettingsRequest() throws Exception { } public void testHandleRegisterRestActionsRequestWithInvalidMethod() throws Exception { - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); String uniqueIdStr = "uniqueid1"; @@ -520,7 +484,7 @@ public void testHandleRegisterRestActionsRequestWithInvalidMethod() throws Excep } public void testHandleRegisterRestActionsRequestWithInvalidDeprecatedMethod() throws Exception { - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); String uniqueIdStr = "uniqueid1"; @@ -535,7 +499,7 @@ public void testHandleRegisterRestActionsRequestWithInvalidDeprecatedMethod() th } public void testHandleRegisterRestActionsRequestWithInvalidUri() throws Exception { - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); String uniqueIdStr = "uniqueid1"; List actionsList = List.of("GET", "PUT /bar", "POST /baz"); @@ -549,7 +513,7 @@ public void testHandleRegisterRestActionsRequestWithInvalidUri() throws Exceptio } public void testHandleRegisterRestActionsRequestWithInvalidDeprecatedUri() throws Exception { - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); String uniqueIdStr = "uniqueid1"; List actionsList = List.of("GET /foo", "PUT /bar", "POST /baz"); @@ -563,7 +527,7 @@ public void testHandleRegisterRestActionsRequestWithInvalidDeprecatedUri() throw } public void testHandleExtensionRequest() throws Exception { - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); ExtensionRequest clusterStateRequest = new ExtensionRequest(ExtensionRequestProto.RequestType.REQUEST_EXTENSION_CLUSTER_STATE); @@ -717,9 +681,7 @@ public void testEnvironmentSettingsDefaultValue() throws Exception { } public void testAddSettingsUpdateConsumerRequest() throws Exception { - Path extensionDir = createTempDir(); - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); List> componentSettings = List.of( @@ -763,10 +725,7 @@ public void testAddSettingsUpdateConsumerRequest() throws Exception { } public void testHandleAddSettingsUpdateConsumerRequest() throws Exception { - - Path extensionDir = createTempDir(); - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); List> componentSettings = List.of( @@ -786,9 +745,7 @@ public void testHandleAddSettingsUpdateConsumerRequest() throws Exception { } public void testUpdateSettingsRequest() throws Exception { - Path extensionDir = createTempDir(); - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); initialize(extensionsManager); Setting componentSetting = Setting.boolSetting("falseSetting", false, Property.Dynamic); @@ -817,7 +774,7 @@ public void testUpdateSettingsRequest() throws Exception { public void testRegisterHandler() throws Exception { - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); TransportService mockTransportService = spy( new TransportService( @@ -842,43 +799,50 @@ public void testRegisterHandler() throws Exception { } - public void testIncompatibleExtensionRegistration() throws IOException, IllegalAccessException { - - try (MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(ExtensionsManager.class))) { - - mockLogAppender.addExpectation( - new MockLogAppender.SeenEventExpectation( - "Could not load extension with uniqueId", - "org.opensearch.extensions.ExtensionsManager", - Level.ERROR, - "Could not load extension with uniqueId uniqueid1 due to OpenSearchException[Extension minimumCompatibleVersion: 3.99.0 is greater than current" - ) - ); - - List incompatibleExtension = Arrays.asList( - "extensions:", - " - name: firstExtension", - " uniqueId: uniqueid1", - " hostAddress: '127.0.0.0'", - " port: '9300'", - " version: '0.0.7'", - " opensearchVersion: '3.0.0'", - " minimumCompatibleVersion: '3.99.0'" - ); - - Files.write(extensionDir.resolve("extensions.yml"), incompatibleExtension, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, Set.of()); - assertEquals(0, extensionsManager.getExtensionIdMap().values().size()); - mockLogAppender.assertAllExpectationsMatched(); - } + public void testIncompatibleExtensionRegistration() throws IOException { + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); + Extension firstExtension = new Extension( + "firstExtension", + "uniqueid1", + "127.0.0.0", + "9300", + "0.0.7", + "3.0.0", + "3.99.0", + List.of(), + null + ); + expectThrows(OpenSearchException.class, () -> extensionsManager.loadExtension(firstExtension)); + assertEquals(0, extensionsManager.getExtensionIdMap().values().size()); } public void testAdditionalExtensionSettingsForExtensionWithCustomSettingSet() throws Exception { - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); + Setting customSetting = Setting.simpleString("custom_extension_setting", "custom_setting", Property.ExtensionScope); + ExtensionAwarePlugin extAwarePlugin = new ExtensionAwarePlugin() { + @Override + public List> getExtensionSettings() { + List> settings = new ArrayList>(); + settings.add(customSetting); + return settings; + } + }; Set> additionalSettings = extAwarePlugin.getExtensionSettings().stream().collect(Collectors.toSet()); + ExtensionScopedSettings extensionScopedSettings = new ExtensionScopedSettings(additionalSettings); + Extension firstExtension = new Extension( + "firstExtension", + "uniqueid1", + "127.0.0.0", + "9300", + "0.0.7", + "3.0.0", + "3.0.0", + List.of(), + extensionScopedSettings + ); - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, additionalSettings); + ExtensionsManager extensionsManager = new ExtensionsManager(additionalSettings); + extensionsManager.loadExtension(firstExtension); DiscoveryExtensionNode extension = new DiscoveryExtensionNode( "firstExtension", @@ -900,11 +864,23 @@ public void testAdditionalExtensionSettingsForExtensionWithCustomSettingSet() th } public void testAdditionalExtensionSettingsForExtensionWithoutCustomSettingSet() throws Exception { - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); Set> additionalSettings = extAwarePlugin.getExtensionSettings().stream().collect(Collectors.toSet()); + ExtensionScopedSettings extensionScopedSettings = new ExtensionScopedSettings(additionalSettings); + Extension firstExtension = new Extension( + "secondExtension", + "uniqueid2", + "127.0.0.0", + "9301", + "0.0.7", + "2.0.0", + "2.0.0", + List.of(), + extensionScopedSettings + ); - ExtensionsManager extensionsManager = new ExtensionsManager(extensionDir, additionalSettings); + ExtensionsManager extensionsManager = new ExtensionsManager(additionalSettings); + extensionsManager.loadExtension(firstExtension); DiscoveryExtensionNode extension = new DiscoveryExtensionNode( "secondExtension", diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java new file mode 100644 index 0000000000000..cc7319415da0e --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java @@ -0,0 +1,13 @@ +/* + * 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.extensions.rest; + +import org.opensearch.test.OpenSearchTestCase; + +public class RestInitializeExtensionTests extends OpenSearchTestCase {} diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java index df047afb677d9..6de73cba3a28c 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java @@ -42,6 +42,7 @@ import org.opensearch.common.util.PageCacheRecycler; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.extensions.DiscoveryExtensionNode; +import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionAction; import org.opensearch.extensions.action.ExtensionTransportAction; import org.opensearch.identity.IdentityService; @@ -118,7 +119,8 @@ public void setup() throws Exception { null, usageService, null, - new IdentityService(Settings.EMPTY, new ArrayList<>()) + new IdentityService(Settings.EMPTY, new ArrayList<>()), + new ExtensionsManager(Set.of()) ); dynamicActionRegistry = actionModule.getDynamicActionRegistry(); } From 9671dae9904b8c1ec21de8b2151e1fb60775042c Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Fri, 9 Jun 2023 17:48:56 -0700 Subject: [PATCH 03/12] Added tests for RestInitializeExtensionAction Signed-off-by: Owais Kazi --- .../RestInitializeExtensionAction.java | 3 +- .../rest/RestInitializeExtensionTests.java | 148 +++++++++++++++++- 2 files changed, 149 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java index b9820ebbb9ab0..055a0932a95f3 100644 --- a/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java +++ b/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java @@ -51,7 +51,7 @@ public RestInitializeExtensionAction(ExtensionsManager extensionsManager) { } @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { String name = ""; String uniqueId = ""; String hostAddress = ""; @@ -113,6 +113,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } + logger.info("Extension has been initialized"); return channel -> { try (XContentBuilder builder = channel.newBuilder()) { builder.startObject(); diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java index cc7319415da0e..e0231f4da8c53 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java @@ -8,6 +8,152 @@ package org.opensearch.extensions.rest; +import java.util.Collections; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import static java.util.Collections.emptyMap; +import static java.util.Collections.emptySet; +import static org.mockito.Mockito.mock; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.junit.After; +import org.junit.Before; +import org.opensearch.Version; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.bytes.BytesArray; +import org.opensearch.common.io.stream.NamedWriteableRegistry; +import org.opensearch.common.network.NetworkService; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.PageCacheRecycler; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.indices.breaker.NoneCircuitBreakerService; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.extensions.RestInitializeExtensionAction; +import org.opensearch.test.MockLogAppender; import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.rest.FakeRestChannel; +import org.opensearch.test.rest.FakeRestRequest; +import org.opensearch.test.transport.MockTransportService; +import org.opensearch.threadpool.TestThreadPool; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportService; +import org.opensearch.transport.nio.MockNioTransport; + +public class RestInitializeExtensionTests extends OpenSearchTestCase { + + private TransportService transportService; + private MockNioTransport transport; + private final ThreadPool threadPool = new TestThreadPool(RestInitializeExtensionTests.class.getSimpleName()); + + @Before + public void setup() throws Exception { + Settings settings = Settings.builder().put("cluster.name", "test").build(); + transport = new MockNioTransport( + settings, + Version.CURRENT, + threadPool, + new NetworkService(Collections.emptyList()), + PageCacheRecycler.NON_RECYCLING_INSTANCE, + new NamedWriteableRegistry(Collections.emptyList()), + new NoneCircuitBreakerService() + ); + transportService = new MockTransportService( + settings, + transport, + threadPool, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + (boundAddress) -> new DiscoveryNode( + "test_node", + "test_node", + boundAddress.publishAddress(), + emptyMap(), + emptySet(), + Version.CURRENT + ), + null, + Collections.emptySet() + ); + + } + + @Override + @After + public void tearDown() throws Exception { + super.tearDown(); + transportService.close(); + ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS); + } + + public void testRestInitializeExtensionActionResponse() throws Exception { + ExtensionsManager extensionsManager = mock(ExtensionsManager.class); + RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); + + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST).build(); + FakeRestChannel channel = new FakeRestChannel(request, false, 0); + restInitializeExtensionAction.handleRequest(request, channel, null); + + assertEquals(1, channel.responses().get()); + assertEquals(0, channel.errors().get()); + assertTrue(channel.capturedResponse().content().utf8ToString().contains("Extension has been initialized")); + + } + + public void testRestInitializeExtensionAction() throws Exception { + ExtensionsManager extensionsManager = mock(ExtensionsManager.class); + RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); + final String content = + "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\",\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\",\"minimumCompatibleVersion\":\"3.0.0\"}"; + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(new BytesArray(content), XContentType.JSON) + .withMethod(RestRequest.Method.POST) + .build(); + + try ( + MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(RestInitializeExtensionAction.class)) + ) { + mockLogAppender.addExpectation( + new MockLogAppender.SeenEventExpectation( + "Extension has been initialized", + "org.opensearch.rest.extensions.RestInitializeExtensionAction", + Level.INFO, + "Extension has been initialized" + ) + ); + restInitializeExtensionAction.prepareRequest(request, null); + mockLogAppender.assertAllExpectationsMatched(); + + } + + } + + public void testRestInitializeExtensionActionFailure() throws Exception { + ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); + RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); + + final String content = + "{\"name\":\"ad-extension\",\"uniqueId\":\"\",\"hostAddress\":\"127.0.0.1\",\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\",\"minimumCompatibleVersion\":\"3.0.0\"}"; + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(new BytesArray(content), XContentType.JSON) + .withMethod(RestRequest.Method.POST) + .build(); + + try ( + MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(RestInitializeExtensionAction.class)) + ) { + mockLogAppender.addExpectation( + new MockLogAppender.SeenEventExpectation( + "Required field is missing in the request", + "org.opensearch.rest.extensions.RestInitializeExtensionAction", + Level.ERROR, + "Required field [uniqueId] is missing in the request" + ) + ); + restInitializeExtensionAction.prepareRequest(request, null); + mockLogAppender.assertAllExpectationsMatched(); + + } + + } -public class RestInitializeExtensionTests extends OpenSearchTestCase {} +} From 09cf97d89e7138cb3989dcfcc8ffa99dc3f24d56 Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Mon, 12 Jun 2023 13:28:58 -0700 Subject: [PATCH 04/12] Pulled extensions REST request in extensions directory Signed-off-by: Owais Kazi --- .../main/java/org/opensearch/action/ActionModule.java | 4 ++-- .../extensions/rest/RestActionsRequestHandler.java | 1 - .../rest}/RestInitializeExtensionAction.java | 2 +- .../rest}/RestSendToExtensionAction.java | 5 +---- .../org/opensearch/rest/extensions/package-info.java | 10 ---------- .../opensearch/action/DynamicActionRegistryTests.java | 2 +- .../extensions/rest/RestInitializeExtensionTests.java | 5 ++--- .../rest/RestSendToExtensionActionTests.java | 1 - 8 files changed, 7 insertions(+), 23 deletions(-) rename server/src/main/java/org/opensearch/{rest/extensions => extensions/rest}/RestInitializeExtensionAction.java (99%) rename server/src/main/java/org/opensearch/{rest/extensions => extensions/rest}/RestSendToExtensionAction.java (98%) delete mode 100644 server/src/main/java/org/opensearch/rest/extensions/package-info.java diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 1581991a64f9d..902ae7cc54e3f 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -297,7 +297,7 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.ExtensionProxyAction; import org.opensearch.extensions.action.ExtensionProxyTransportAction; -import org.opensearch.rest.extensions.RestInitializeExtensionAction; +import org.opensearch.extensions.rest.RestInitializeExtensionAction; import org.opensearch.index.seqno.RetentionLeaseActions; import org.opensearch.identity.IdentityService; import org.opensearch.indices.SystemIndices; @@ -455,7 +455,7 @@ import org.opensearch.rest.action.search.RestPutSearchPipelineAction; import org.opensearch.rest.action.search.RestSearchAction; import org.opensearch.rest.action.search.RestSearchScrollAction; -import org.opensearch.rest.extensions.RestSendToExtensionAction; +import org.opensearch.extensions.rest.RestSendToExtensionAction; import org.opensearch.tasks.Task; import org.opensearch.threadpool.ThreadPool; import org.opensearch.usage.UsageService; diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java b/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java index 37638f2a333d5..d890c1b85bb81 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java @@ -13,7 +13,6 @@ import org.opensearch.extensions.DiscoveryExtensionNode; import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; -import org.opensearch.rest.extensions.RestSendToExtensionAction; import org.opensearch.transport.TransportResponse; import org.opensearch.transport.TransportService; diff --git a/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java similarity index 99% rename from server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java rename to server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java index 055a0932a95f3..0269b357e38d9 100644 --- a/server/src/main/java/org/opensearch/rest/extensions/RestInitializeExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rest.extensions; +package org.opensearch.extensions.rest; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; diff --git a/server/src/main/java/org/opensearch/rest/extensions/RestSendToExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java similarity index 98% rename from server/src/main/java/org/opensearch/rest/extensions/RestSendToExtensionAction.java rename to server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java index 51ff74b1869a0..073b3f3f45818 100644 --- a/server/src/main/java/org/opensearch/rest/extensions/RestSendToExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.rest.extensions; +package org.opensearch.extensions.rest; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -17,9 +17,6 @@ import org.opensearch.common.xcontent.XContentType; import org.opensearch.extensions.DiscoveryExtensionNode; import org.opensearch.extensions.ExtensionsManager; -import org.opensearch.extensions.rest.ExtensionRestRequest; -import org.opensearch.extensions.rest.RegisterRestActionsRequest; -import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.NamedRoute; diff --git a/server/src/main/java/org/opensearch/rest/extensions/package-info.java b/server/src/main/java/org/opensearch/rest/extensions/package-info.java deleted file mode 100644 index 64b92e8b5c149..0000000000000 --- a/server/src/main/java/org/opensearch/rest/extensions/package-info.java +++ /dev/null @@ -1,10 +0,0 @@ -/* - * 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. - */ - -/** REST classes for the extensions package. OpenSearch extensions provide extensibility to OpenSearch.*/ -package org.opensearch.rest.extensions; diff --git a/server/src/test/java/org/opensearch/action/DynamicActionRegistryTests.java b/server/src/test/java/org/opensearch/action/DynamicActionRegistryTests.java index a5b4f91ff1ed5..963d47df3baff 100644 --- a/server/src/test/java/org/opensearch/action/DynamicActionRegistryTests.java +++ b/server/src/test/java/org/opensearch/action/DynamicActionRegistryTests.java @@ -18,7 +18,7 @@ import org.opensearch.extensions.action.ExtensionTransportAction; import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestRequest; -import org.opensearch.rest.extensions.RestSendToExtensionAction; +import org.opensearch.extensions.rest.RestSendToExtensionAction; import org.opensearch.tasks.Task; import org.opensearch.tasks.TaskManager; import org.opensearch.test.OpenSearchTestCase; diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java index e0231f4da8c53..1f27170d076ff 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java @@ -31,7 +31,6 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.indices.breaker.NoneCircuitBreakerService; import org.opensearch.rest.RestRequest; -import org.opensearch.rest.extensions.RestInitializeExtensionAction; import org.opensearch.test.MockLogAppender; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestChannel; @@ -116,7 +115,7 @@ public void testRestInitializeExtensionAction() throws Exception { mockLogAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "Extension has been initialized", - "org.opensearch.rest.extensions.RestInitializeExtensionAction", + "org.opensearch.extensions.rest.RestInitializeExtensionAction", Level.INFO, "Extension has been initialized" ) @@ -144,7 +143,7 @@ public void testRestInitializeExtensionActionFailure() throws Exception { mockLogAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "Required field is missing in the request", - "org.opensearch.rest.extensions.RestInitializeExtensionAction", + "org.opensearch.extensions.rest.RestInitializeExtensionAction", Level.ERROR, "Required field [uniqueId] is missing in the request" ) diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java index 6de73cba3a28c..fe8792b36f048 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java @@ -50,7 +50,6 @@ import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest.Method; -import org.opensearch.rest.extensions.RestSendToExtensionAction; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.transport.MockTransportService; import org.opensearch.threadpool.TestThreadPool; From a86bd3c76f6ff34a9c6acf9ee1fc66e83aef6892 Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Mon, 12 Jun 2023 13:53:55 -0700 Subject: [PATCH 05/12] Removed forbidden APIs from rest action and modified tests Signed-off-by: Owais Kazi --- .../rest/RestInitializeExtensionAction.java | 5 -- .../rest/RestInitializeExtensionTests.java | 55 +++---------------- 2 files changed, 9 insertions(+), 51 deletions(-) diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java index 0269b357e38d9..2601d1e1077ab 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java @@ -8,8 +8,6 @@ package org.opensearch.extensions.rest; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.client.node.NodeClient; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; @@ -34,7 +32,6 @@ public class RestInitializeExtensionAction extends BaseRestHandler { private final ExtensionsManager extensionsManager; - private static final Logger logger = LogManager.getLogger(RestInitializeExtensionAction.class); @Override public String getName() { @@ -108,12 +105,10 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client extensionsManager.loadExtension(extension); extensionsManager.initialize(); } catch (IOException e) { - logger.error(e); return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); } - logger.info("Extension has been initialized"); return channel -> { try (XContentBuilder builder = channel.newBuilder()) { builder.startObject(); diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java index 1f27170d076ff..fb2be7343c15f 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java @@ -16,8 +16,6 @@ import static java.util.Collections.emptySet; import static org.mockito.Mockito.mock; -import org.apache.logging.log4j.Level; -import org.apache.logging.log4j.LogManager; import org.junit.After; import org.junit.Before; import org.opensearch.Version; @@ -31,7 +29,6 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.indices.breaker.NoneCircuitBreakerService; import org.opensearch.rest.RestRequest; -import org.opensearch.test.MockLogAppender; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestChannel; import org.opensearch.test.rest.FakeRestRequest; @@ -89,8 +86,12 @@ public void tearDown() throws Exception { public void testRestInitializeExtensionActionResponse() throws Exception { ExtensionsManager extensionsManager = mock(ExtensionsManager.class); RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); + final String content = + "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\",\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\",\"minimumCompatibleVersion\":\"3.0.0\"}"; + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(new BytesArray(content), XContentType.JSON) + .withMethod(RestRequest.Method.POST) + .build(); - RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST).build(); FakeRestChannel channel = new FakeRestChannel(request, false, 0); restInitializeExtensionAction.handleRequest(request, channel, null); @@ -100,33 +101,6 @@ public void testRestInitializeExtensionActionResponse() throws Exception { } - public void testRestInitializeExtensionAction() throws Exception { - ExtensionsManager extensionsManager = mock(ExtensionsManager.class); - RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); - final String content = - "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\",\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\",\"minimumCompatibleVersion\":\"3.0.0\"}"; - RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(new BytesArray(content), XContentType.JSON) - .withMethod(RestRequest.Method.POST) - .build(); - - try ( - MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(RestInitializeExtensionAction.class)) - ) { - mockLogAppender.addExpectation( - new MockLogAppender.SeenEventExpectation( - "Extension has been initialized", - "org.opensearch.extensions.rest.RestInitializeExtensionAction", - Level.INFO, - "Extension has been initialized" - ) - ); - restInitializeExtensionAction.prepareRequest(request, null); - mockLogAppender.assertAllExpectationsMatched(); - - } - - } - public void testRestInitializeExtensionActionFailure() throws Exception { ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); @@ -137,22 +111,11 @@ public void testRestInitializeExtensionActionFailure() throws Exception { .withMethod(RestRequest.Method.POST) .build(); - try ( - MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(RestInitializeExtensionAction.class)) - ) { - mockLogAppender.addExpectation( - new MockLogAppender.SeenEventExpectation( - "Required field is missing in the request", - "org.opensearch.extensions.rest.RestInitializeExtensionAction", - Level.ERROR, - "Required field [uniqueId] is missing in the request" - ) - ); - restInitializeExtensionAction.prepareRequest(request, null); - mockLogAppender.assertAllExpectationsMatched(); - - } + FakeRestChannel channel = new FakeRestChannel(request, false, 0); + restInitializeExtensionAction.handleRequest(request, channel, null); + assertEquals(1, channel.errors().get()); + assertTrue(channel.capturedResponse().content().utf8ToString().contains("Required field [uniqueId] is missing in the request")); } } From feeb437329aa2f29265794831c94db163c96d197 Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Mon, 12 Jun 2023 14:23:15 -0700 Subject: [PATCH 06/12] Added entry in changelog Signed-off-by: Owais Kazi --- CHANGELOG.md | 1 + .../extensions/rest/RestInitializeExtensionAction.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb67e56c7845f..75287db6b4f07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add descending order search optimization through reverse segment read. ([#7967](https://github.com/opensearch-project/OpenSearch/pull/7967)) - Update components of segrep backpressure to support remote store. ([#8020](https://github.com/opensearch-project/OpenSearch/pull/8020)) - Make remote cluster connection setup in async ([#8038](https://github.com/opensearch-project/OpenSearch/pull/8038)) +- Add API to initialize extensions ([#8029]()https://github.com/opensearch-project/OpenSearch/pull/8029) ### Dependencies - Bump `com.azure:azure-storage-common` from 12.21.0 to 12.21.1 (#7566, #7814) diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java index 2601d1e1077ab..15c80501f7a26 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java @@ -98,7 +98,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client openSearchVersion, minimumCompatibleVersion, dependencies, - // TODO create parser for additionalSettings + // TODO create parser for additionalSettings https://github.com/opensearch-project/OpenSearch/issues/8032 null ); try { From 669b29903212689eb9e47409bee67e16aabe12fc Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Mon, 12 Jun 2023 16:46:28 -0700 Subject: [PATCH 07/12] Added test for parse Signed-off-by: Owais Kazi --- .../extensions/ExtensionsManagerTests.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index 9465db5a50af0..77a2656ccc40e 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -45,6 +45,8 @@ import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.ClusterSettingsResponse; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.env.EnvironmentSettingsResponse; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.node.DiscoveryNode; @@ -396,6 +398,16 @@ public void testExtensionDependency() throws Exception { } } + public void testParseExtensionDependency() throws Exception { + XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"uniqueId\": \"test1\", \"version\": \"2.0.0\"}"); + + assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); + ExtensionDependency dependency = ExtensionDependency.parse(parser); + + assertEquals("test1", dependency.getUniqueId()); + assertEquals(Version.fromString("2.0.0"), dependency.getVersion()); + } + public void testInitialize() throws Exception { ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); From 8906ecc5afe63c95cfb89166278995126f1b7ddc Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Tue, 13 Jun 2023 16:33:35 -0700 Subject: [PATCH 08/12] Addressed PR comments Signed-off-by: Owais Kazi --- .../extensions/ExtensionDependency.java | 27 ++++-- .../extensions/ExtensionsManager.java | 4 +- .../rest/RestInitializeExtensionAction.java | 89 ++++++++++++------- ...> RestInitializeExtensionActionTests.java} | 4 +- 4 files changed, 81 insertions(+), 43 deletions(-) rename server/src/test/java/org/opensearch/extensions/rest/{RestInitializeExtensionTests.java => RestInitializeExtensionActionTests.java} (97%) diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java b/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java index 6205529f8affa..1423a30bbe307 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java @@ -16,6 +16,7 @@ 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.XContentParser; import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; @@ -28,6 +29,8 @@ public class ExtensionDependency implements Writeable { private String uniqueId; private Version version; + private static final String UNIQUE_ID = "uniqueId"; + private static final String VERSION = "version"; public ExtensionDependency(String uniqueId, Version version) { this.uniqueId = uniqueId; @@ -65,12 +68,26 @@ public static ExtensionDependency parse(XContentParser parser) throws IOExceptio String fieldName = parser.currentName(); parser.nextToken(); - if ("uniqueId".equals(fieldName)) { - uniqueId = parser.text(); - } else if ("version".equals(fieldName)) { - version = Version.fromString(parser.text()); + switch (fieldName) { + case UNIQUE_ID: + uniqueId = parser.text(); + break; + case VERSION: + try { + version = Version.fromString(parser.text()); + } catch (IllegalArgumentException e) { + throw e; + } + break; + default: + parser.skipChildren(); + break; } - + } + if (Strings.isNullOrEmpty(uniqueId)) { + throw new IOException("Required field [uniqueId] is missing in the request for the dependent extension"); + } else if (version == null) { + throw new IOException("Required field [version] is missing in the request for the dependent extension"); } return new ExtensionDependency(uniqueId, version); diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index fad71880765eb..778094406697f 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -17,7 +17,6 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; @@ -385,8 +384,9 @@ public String executor() { transportService.getThreadPool().generic().execute(new AbstractRunnable() { @Override public void onFailure(Exception e) { - if (e.getCause() instanceof TimeoutException || e instanceof ConnectTransportException) { + if (e.getCause() instanceof ConnectTransportException) { logger.info("No response from extension to request.", e); + throw (ConnectTransportException) e.getCause(); } else if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); } else if (e.getCause() instanceof Error) { diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java index 15c80501f7a26..078614720f95b 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java @@ -12,16 +12,21 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.extensions.ExtensionDependency; +import org.opensearch.extensions.ExtensionScopedSettings; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.ExtensionsSettings.Extension; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestStatus; +import org.opensearch.transport.ConnectTransportException; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.concurrent.CompletionException; +import java.util.concurrent.TimeoutException; import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; import static org.opensearch.rest.RestRequest.Method.POST; @@ -49,40 +54,42 @@ public RestInitializeExtensionAction(ExtensionsManager extensionsManager) { @Override public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { - String name = ""; - String uniqueId = ""; - String hostAddress = ""; - String port = ""; - String version = ""; - String openSearchVersion = ""; - String minimumCompatibleVersion = ""; + String name = null; + String uniqueId = null; + String hostAddress = null; + String port = null; + String version = null; + String openSearchVersion = null; + String minimumCompatibleVersion = null; List dependencies = new ArrayList<>(); - if (request.hasContent()) { - try (XContentParser parser = request.contentParser()) { + try (XContentParser parser = request.contentParser()) { + parser.nextToken(); + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String currentFieldName = parser.currentName(); parser.nextToken(); - ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); - while (parser.nextToken() != XContentParser.Token.END_OBJECT) { - String currentFieldName = parser.currentName(); - parser.nextToken(); - if ("name".equals(currentFieldName)) { - name = parser.text(); - } else if ("uniqueId".equals(currentFieldName)) { - uniqueId = parser.text(); - } else if ("hostAddress".equals(currentFieldName)) { - hostAddress = parser.text(); - } else if ("port".equals(currentFieldName)) { - port = parser.text(); - } else if ("version".equals(currentFieldName)) { - version = parser.text(); - } else if ("opensearchVersion".equals(currentFieldName)) { - openSearchVersion = parser.text(); - } else if ("minimumCompatibleVersion".equals(currentFieldName)) { - minimumCompatibleVersion = parser.text(); - } else if ("dependencies".equals(currentFieldName)) { - ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); - while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + if ("name".equals(currentFieldName)) { + name = parser.text(); + } else if ("uniqueId".equals(currentFieldName)) { + uniqueId = parser.text(); + } else if ("hostAddress".equals(currentFieldName)) { + hostAddress = parser.text(); + } else if ("port".equals(currentFieldName)) { + port = parser.text(); + } else if ("version".equals(currentFieldName)) { + version = parser.text(); + } else if ("opensearchVersion".equals(currentFieldName)) { + openSearchVersion = parser.text(); + } else if ("minimumCompatibleVersion".equals(currentFieldName)) { + minimumCompatibleVersion = parser.text(); + } else if ("dependencies".equals(currentFieldName)) { + ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + try { dependencies.add(ExtensionDependency.parse(parser)); + } catch (IOException e) { + throw e; } } } @@ -98,13 +105,27 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client openSearchVersion, minimumCompatibleVersion, dependencies, - // TODO create parser for additionalSettings https://github.com/opensearch-project/OpenSearch/issues/8032 - null + // TODO add this to the API (https://github.com/opensearch-project/OpenSearch/issues/8032) + new ExtensionScopedSettings(Collections.emptySet()) ); try { extensionsManager.loadExtension(extension); extensionsManager.initialize(); - } catch (IOException e) { + } catch (CompletionException e) { + Throwable cause = e.getCause(); + if (cause instanceof TimeoutException) { + return channel -> channel.sendResponse( + new BytesRestResponse(RestStatus.REQUEST_TIMEOUT, "No response from extension to request.") + ); + } else if (cause instanceof ConnectTransportException || cause instanceof RuntimeException) { + return channel -> channel.sendResponse( + new BytesRestResponse(RestStatus.REQUEST_TIMEOUT, "Connection failed with the extension.") + ); + } + if (e.getCause() instanceof Error) { + throw (Error) e.getCause(); + } + } catch (Exception e) { return channel -> channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage())); } @@ -112,7 +133,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client return channel -> { try (XContentBuilder builder = channel.newBuilder()) { builder.startObject(); - builder.field("Extension has been initialized"); + builder.field("success", "Extension has been initialized"); builder.endObject(); channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder)); } diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java similarity index 97% rename from server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java rename to server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java index fb2be7343c15f..06bcc9c35085d 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java @@ -38,11 +38,11 @@ import org.opensearch.transport.TransportService; import org.opensearch.transport.nio.MockNioTransport; -public class RestInitializeExtensionTests extends OpenSearchTestCase { +public class RestInitializeExtensionActionTests extends OpenSearchTestCase { private TransportService transportService; private MockNioTransport transport; - private final ThreadPool threadPool = new TestThreadPool(RestInitializeExtensionTests.class.getSimpleName()); + private final ThreadPool threadPool = new TestThreadPool(RestInitializeExtensionActionTests.class.getSimpleName()); @Before public void setup() throws Exception { From a01bc2d65706a37e0f15587ebbd8961508f626df Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Tue, 20 Jun 2023 15:26:42 -0700 Subject: [PATCH 09/12] Addressed PR comments Signed-off-by: Owais Kazi --- .../extensions/ExtensionsManager.java | 33 +++++++++---------- .../rest/RestInitializeExtensionAction.java | 4 +-- .../RestInitializeExtensionActionTests.java | 17 ++++++---- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index 778094406697f..2244439f1386c 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -314,25 +314,23 @@ public void loadExtension(Extension extension) throws IOException { } - private boolean validateExtension(Extension extension) throws IOException { - if (Strings.isNullOrEmpty(extension.getName())) { - throw new IOException("Required field [name] is missing in the request"); - } else if (Strings.isNullOrEmpty(extension.getUniqueId())) { - throw new IOException("Required field [uniqueId] is missing in the request"); - } else if (Strings.isNullOrEmpty(extension.getHostAddress())) { - throw new IOException("Required field [extension host address] is missing in the request"); - } else if (Strings.isNullOrEmpty(extension.getPort())) { - throw new IOException("Required field [extension port] is missing in the request"); - } else if (Strings.isNullOrEmpty(extension.getVersion())) { - throw new IOException("Required field [extension version] is missing in the request"); - } else if (Strings.isNullOrEmpty(extension.getOpensearchVersion())) { - throw new IOException("Required field [opensearch version] is missing in the request"); - } else if (Strings.isNullOrEmpty(extension.getMinimumCompatibleVersion())) { - throw new IOException("Required field [minimum opensearch version] is missing in the request"); - } else if (extensionIdMap.containsKey(extension.getUniqueId())) { + private void validateField(String fieldName, String value) throws IOException { + if (Strings.isNullOrEmpty(value)) { + throw new IOException("Required field [" + fieldName + "] is missing in the request"); + } + } + + private void validateExtension(Extension extension) throws IOException { + validateField("extension name", extension.getName()); + validateField("extension uniqueId", extension.getUniqueId()); + validateField("extension host address", extension.getHostAddress()); + validateField("extension port", extension.getPort()); + validateField("extension version", extension.getVersion()); + validateField("opensearch version", extension.getOpensearchVersion()); + validateField("minimum opensearch version", extension.getMinimumCompatibleVersion()); + if (extensionIdMap.containsKey(extension.getUniqueId())) { throw new IOException("Duplicate uniqueId " + extension.getUniqueId() + ". Did not load extension: " + extension); } - return true; } /** @@ -384,6 +382,7 @@ public String executor() { transportService.getThreadPool().generic().execute(new AbstractRunnable() { @Override public void onFailure(Exception e) { + extensionIdMap.remove(extension.getId()); if (e.getCause() instanceof ConnectTransportException) { logger.info("No response from extension to request.", e); throw (ConnectTransportException) e.getCause(); diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java index 078614720f95b..a29b2904e455a 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java @@ -133,9 +133,9 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client return channel -> { try (XContentBuilder builder = channel.newBuilder()) { builder.startObject(); - builder.field("success", "Extension has been initialized"); + builder.field("success", "A request to initialize an extension has been sent."); builder.endObject(); - channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder)); + channel.sendResponse(new BytesRestResponse(RestStatus.ACCEPTED, builder)); } }; diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java index 06bcc9c35085d..9ddbb79d49807 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java @@ -29,6 +29,7 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.indices.breaker.NoneCircuitBreakerService; import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestStatus; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestChannel; import org.opensearch.test.rest.FakeRestRequest; @@ -87,7 +88,9 @@ public void testRestInitializeExtensionActionResponse() throws Exception { ExtensionsManager extensionsManager = mock(ExtensionsManager.class); RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); final String content = - "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\",\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\",\"minimumCompatibleVersion\":\"3.0.0\"}"; + "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\"," + + "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\"," + + "\"minimumCompatibleVersion\":\"3.0.0\"}"; RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(new BytesArray(content), XContentType.JSON) .withMethod(RestRequest.Method.POST) .build(); @@ -95,10 +98,8 @@ public void testRestInitializeExtensionActionResponse() throws Exception { FakeRestChannel channel = new FakeRestChannel(request, false, 0); restInitializeExtensionAction.handleRequest(request, channel, null); - assertEquals(1, channel.responses().get()); - assertEquals(0, channel.errors().get()); - assertTrue(channel.capturedResponse().content().utf8ToString().contains("Extension has been initialized")); - + assertEquals(channel.capturedResponse().status(), RestStatus.ACCEPTED); + assertTrue(channel.capturedResponse().content().utf8ToString().contains("A request to initialize an extension has been sent.")); } public void testRestInitializeExtensionActionFailure() throws Exception { @@ -106,7 +107,9 @@ public void testRestInitializeExtensionActionFailure() throws Exception { RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); final String content = - "{\"name\":\"ad-extension\",\"uniqueId\":\"\",\"hostAddress\":\"127.0.0.1\",\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\",\"minimumCompatibleVersion\":\"3.0.0\"}"; + "{\"name\":\"ad-extension\",\"uniqueId\":\"\",\"hostAddress\":\"127.0.0.1\"," + + "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\"," + + "\"minimumCompatibleVersion\":\"3.0.0\"}"; RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(new BytesArray(content), XContentType.JSON) .withMethod(RestRequest.Method.POST) .build(); @@ -115,7 +118,7 @@ public void testRestInitializeExtensionActionFailure() throws Exception { restInitializeExtensionAction.handleRequest(request, channel, null); assertEquals(1, channel.errors().get()); - assertTrue(channel.capturedResponse().content().utf8ToString().contains("Required field [uniqueId] is missing in the request")); + assertTrue(channel.capturedResponse().content().utf8ToString().contains("Required field [extension uniqueId] is missing in the request")); } } From 65cb7fbfdf241bbe17f6a8edc899e2fb498f01a3 Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Tue, 20 Jun 2023 16:01:27 -0700 Subject: [PATCH 10/12] Spotless Fixed Signed-off-by: Owais Kazi --- .../RestInitializeExtensionActionTests.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java index 9ddbb79d49807..8d027b7fca9c2 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RestInitializeExtensionActionTests.java @@ -87,10 +87,9 @@ public void tearDown() throws Exception { public void testRestInitializeExtensionActionResponse() throws Exception { ExtensionsManager extensionsManager = mock(ExtensionsManager.class); RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); - final String content = - "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\"," + - "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\"," + - "\"minimumCompatibleVersion\":\"3.0.0\"}"; + final String content = "{\"name\":\"ad-extension\",\"uniqueId\":\"ad-extension\",\"hostAddress\":\"127.0.0.1\"," + + "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\"," + + "\"minimumCompatibleVersion\":\"3.0.0\"}"; RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(new BytesArray(content), XContentType.JSON) .withMethod(RestRequest.Method.POST) .build(); @@ -106,10 +105,9 @@ public void testRestInitializeExtensionActionFailure() throws Exception { ExtensionsManager extensionsManager = new ExtensionsManager(Set.of()); RestInitializeExtensionAction restInitializeExtensionAction = new RestInitializeExtensionAction(extensionsManager); - final String content = - "{\"name\":\"ad-extension\",\"uniqueId\":\"\",\"hostAddress\":\"127.0.0.1\"," + - "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\"," + - "\"minimumCompatibleVersion\":\"3.0.0\"}"; + final String content = "{\"name\":\"ad-extension\",\"uniqueId\":\"\",\"hostAddress\":\"127.0.0.1\"," + + "\"port\":\"4532\",\"version\":\"1.0\",\"opensearchVersion\":\"3.0.0\"," + + "\"minimumCompatibleVersion\":\"3.0.0\"}"; RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(new BytesArray(content), XContentType.JSON) .withMethod(RestRequest.Method.POST) .build(); @@ -118,7 +116,9 @@ public void testRestInitializeExtensionActionFailure() throws Exception { restInitializeExtensionAction.handleRequest(request, channel, null); assertEquals(1, channel.errors().get()); - assertTrue(channel.capturedResponse().content().utf8ToString().contains("Required field [extension uniqueId] is missing in the request")); + assertTrue( + channel.capturedResponse().content().utf8ToString().contains("Required field [extension uniqueId] is missing in the request") + ); } } From 183a157ec6c942e468140b028db4a75df4ab8f6f Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Wed, 21 Jun 2023 10:29:49 -0700 Subject: [PATCH 11/12] Handled exceptions Signed-off-by: Owais Kazi --- .../extensions/ExtensionsManager.java | 37 +++++++------------ .../rest/RestInitializeExtensionAction.java | 8 ++-- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index 2244439f1386c..c0c7c314fece1 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -16,7 +16,6 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; @@ -292,26 +291,19 @@ private void registerRequestHandler(DynamicActionRegistry dynamicActionRegistry) * @param extension The extension to be loaded */ public void loadExtension(Extension extension) throws IOException { - try { - validateExtension(extension); - DiscoveryExtensionNode discoveryExtensionNode = new DiscoveryExtensionNode( - extension.getName(), - extension.getUniqueId(), - new TransportAddress(InetAddress.getByName(extension.getHostAddress()), Integer.parseInt(extension.getPort())), - new HashMap(), - Version.fromString(extension.getOpensearchVersion()), - Version.fromString(extension.getMinimumCompatibleVersion()), - extension.getDependencies() - ); - extensionIdMap.put(extension.getUniqueId(), discoveryExtensionNode); - extensionSettingsMap.put(extension.getUniqueId(), extension); - logger.info("Loaded extension with uniqueId " + extension.getUniqueId() + ": " + extension); - } catch (IOException e) { - throw e; - } catch (IllegalArgumentException e) { - throw e; - } - + validateExtension(extension); + DiscoveryExtensionNode discoveryExtensionNode = new DiscoveryExtensionNode( + extension.getName(), + extension.getUniqueId(), + new TransportAddress(InetAddress.getByName(extension.getHostAddress()), Integer.parseInt(extension.getPort())), + new HashMap(), + Version.fromString(extension.getOpensearchVersion()), + Version.fromString(extension.getMinimumCompatibleVersion()), + extension.getDependencies() + ); + extensionIdMap.put(extension.getUniqueId(), discoveryExtensionNode); + extensionSettingsMap.put(extension.getUniqueId(), extension); + logger.info("Loaded extension with uniqueId " + extension.getUniqueId() + ": " + extension); } private void validateField(String fieldName, String value) throws IOException { @@ -329,7 +321,7 @@ private void validateExtension(Extension extension) throws IOException { validateField("opensearch version", extension.getOpensearchVersion()); validateField("minimum opensearch version", extension.getMinimumCompatibleVersion()); if (extensionIdMap.containsKey(extension.getUniqueId())) { - throw new IOException("Duplicate uniqueId " + extension.getUniqueId() + ". Did not load extension: " + extension); + throw new IOException("Duplicate uniqueId [" + extension.getUniqueId() + "]. Did not load extension: " + extension); } } @@ -404,7 +396,6 @@ protected void doRun() throws Exception { new InitializeExtensionRequest(transportService.getLocalNode(), extension), initializeExtensionResponseHandler ); - inProgressFuture.orTimeout(EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS).join(); } }); } diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java index a29b2904e455a..e0806f8172278 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestInitializeExtensionAction.java @@ -86,14 +86,12 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client } else if ("dependencies".equals(currentFieldName)) { ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - try { - dependencies.add(ExtensionDependency.parse(parser)); - } catch (IOException e) { - throw e; - } + dependencies.add(ExtensionDependency.parse(parser)); } } } + } catch (IOException e) { + throw new IOException("Missing attribute", e); } Extension extension = new Extension( From f6928dd226a448f122b598eec724b48795ce0188 Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Wed, 21 Jun 2023 14:51:51 -0700 Subject: [PATCH 12/12] Handled test failure Signed-off-by: Owais Kazi --- .../java/org/opensearch/extensions/ExtensionsManagerTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index 77a2656ccc40e..f8ec138d8eff2 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -307,7 +307,7 @@ public void testNonUniqueLoadedExtensions() throws Exception { extensionsManager.loadExtension(firstExtension); IOException exception = expectThrows(IOException.class, () -> extensionsManager.loadExtension(secondExtension)); assertEquals( - "Duplicate uniqueId uniqueid1. Did not load extension: Extension [name=secondExtension, uniqueId=uniqueid1, hostAddress=127.0.0.0, port=9300, version=0.0.7, opensearchVersion=3.0.0, minimumCompatibleVersion=3.0.0]", + "Duplicate uniqueId [uniqueid1]. Did not load extension: Extension [name=secondExtension, uniqueId=uniqueid1, hostAddress=127.0.0.0, port=9300, version=0.0.7, opensearchVersion=3.0.0, minimumCompatibleVersion=3.0.0]", exception.getMessage() );