From c08050a3aa5790ed91a1acb6768103ecaed6b705 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Sun, 18 Dec 2022 22:29:56 +0000 Subject: [PATCH 01/10] Added getSettings() support, ActionListener onFailure(), and initial createComponents support for extensions Signed-off-by: Ryan Bogan --- .../common/settings/WriteableSetting.java | 279 ++++++++++ .../env/EnvironmentSettingsResponse.java | 86 ++++ .../AddSettingsUpdateConsumerRequest.java | 98 ++++ ...dSettingsUpdateConsumerRequestHandler.java | 91 ++++ .../EnvironmentSettingsRequest.java | 79 +++ .../EnvironmentSettingsRequestHandler.java | 43 ++ .../extensions/ExtensionActionListener.java | 50 ++ .../ExtensionActionListenerHandler.java | 43 ++ ...tensionActionListenerOnFailureRequest.java | 72 +++ .../extensions/ExtensionRequest.java | 2 - ...onse.java => ExtensionStringResponse.java} | 10 +- .../extensions/ExtensionsManager.java | 161 +++++- .../extensions/UpdateSettingsRequest.java | 93 ++++ .../UpdateSettingsResponseHandler.java | 47 ++ .../rest/RestActionsRequestHandler.java | 5 +- .../rest/RestSendToExtensionAction.java | 2 + .../CustomSettingsRequestHandler.java | 57 ++ .../RegisterCustomSettingsRequest.java | 83 +++ .../extensions/settings/package-info.java | 10 + .../main/java/org/opensearch/node/Node.java | 8 +- .../settings/WriteableSettingTests.java | 487 ++++++++++++++++++ .../extensions/ExtensionResponseTests.java | 51 ++ .../extensions/ExtensionsManagerTests.java | 353 ++++++++++--- .../rest/RegisterRestActionsTests.java | 17 - .../settings/RegisterCustomSettingsTests.java | 56 ++ 25 files changed, 2177 insertions(+), 106 deletions(-) create mode 100644 server/src/main/java/org/opensearch/common/settings/WriteableSetting.java create mode 100644 server/src/main/java/org/opensearch/env/EnvironmentSettingsResponse.java create mode 100644 server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequest.java create mode 100644 server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java create mode 100644 server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequest.java create mode 100644 server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequestHandler.java create mode 100644 server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java create mode 100644 server/src/main/java/org/opensearch/extensions/ExtensionActionListenerHandler.java create mode 100644 server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java rename server/src/main/java/org/opensearch/extensions/{rest/RegisterRestActionsResponse.java => ExtensionStringResponse.java} (69%) create mode 100644 server/src/main/java/org/opensearch/extensions/UpdateSettingsRequest.java create mode 100644 server/src/main/java/org/opensearch/extensions/UpdateSettingsResponseHandler.java create mode 100644 server/src/main/java/org/opensearch/extensions/settings/CustomSettingsRequestHandler.java create mode 100644 server/src/main/java/org/opensearch/extensions/settings/RegisterCustomSettingsRequest.java create mode 100644 server/src/main/java/org/opensearch/extensions/settings/package-info.java create mode 100644 server/src/test/java/org/opensearch/common/settings/WriteableSettingTests.java create mode 100644 server/src/test/java/org/opensearch/extensions/ExtensionResponseTests.java create mode 100644 server/src/test/java/org/opensearch/extensions/settings/RegisterCustomSettingsTests.java diff --git a/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java b/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java new file mode 100644 index 0000000000000..0909224892c7a --- /dev/null +++ b/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java @@ -0,0 +1,279 @@ +/* + * 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.common.settings; + +import org.opensearch.Version; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.common.io.stream.Writeable; +import org.opensearch.common.settings.Setting.Property; +import org.opensearch.common.unit.ByteSizeValue; +import org.opensearch.common.unit.TimeValue; +import java.io.IOException; +import java.util.Arrays; +import java.util.EnumSet; +import java.util.concurrent.TimeUnit; + +/** + * Wrapper for {@link Setting} with {@link #writeTo(StreamOutput)} implementation dependent on the setting type. + * + * @opensearch.internal + */ +public class WriteableSetting implements Writeable { + + /** + * The Generic Types which this class can serialize. + */ + public enum SettingType { + Boolean, + Integer, + Long, + Float, + Double, + String, + TimeValue, // long + TimeUnit + ByteSizeValue, // long + ByteSizeUnit + Version + } + + private Setting setting; + private SettingType type; + + /** + * Wrap a {@link Setting}. The generic type is determined from the type of the default value. + * + * @param setting The setting to wrap. The default value must not be null. + * @throws IllegalArgumentException if the setting has a null default value. + */ + public WriteableSetting(Setting setting) { + this(setting, getGenericTypeFromDefault(setting)); + } + + /** + * Wrap a {@link Setting} with a specified generic type. + * + * @param setting The setting to wrap. + * @param type The Generic type of the setting. + */ + public WriteableSetting(Setting setting, SettingType type) { + this.setting = setting; + this.type = type; + } + + /** + * Wrap a {@link Setting} read from a stream. + * + * @param in Input to read the value from. + * @throws IOException if there is an error reading the values + */ + public WriteableSetting(StreamInput in) throws IOException { + // Read the type + this.type = in.readEnum(SettingType.class); + // Read the key + String key = in.readString(); + // Read the default value + Object defaultValue = readDefaultValue(in); + // Read a boolean specifying whether the fallback settings is null + WriteableSetting fallback = null; + boolean hasFallback = in.readBoolean(); + if (hasFallback) { + fallback = new WriteableSetting(in); + } + // We are using known types so don't need the parser + // We are not using validator + // Read properties + EnumSet propSet = in.readEnumSet(Property.class); + // Put it all in a setting object + this.setting = createSetting(type, key, defaultValue, fallback, propSet.toArray(Property[]::new)); + } + + /** + * Due to type erasure, it is impossible to determine the generic type of a Setting at runtime. + * All settings have a non-null default, however, so the type of the default can be used to determine the setting type. + * + * @param setting The setting with a generic type. + * @return The corresponding {@link SettingType} for the default value. + */ + private static SettingType getGenericTypeFromDefault(Setting setting) { + String typeStr = null; + try { + // This throws NPE on null default + typeStr = setting.getDefault(Settings.EMPTY).getClass().getSimpleName(); + // This throws IAE if not in enum + return SettingType.valueOf(typeStr); + } catch (NullPointerException e) { + throw new IllegalArgumentException("Unable to determine the generic type of this setting with a null default value."); + } catch (IllegalArgumentException e) { + throw new UnsupportedOperationException( + "This class is not yet set up to handle the generic type: " + + typeStr + + ". Supported types are " + + Arrays.toString(SettingType.values()) + ); + } + } + + /** + * Gets the wrapped setting. Use {@link #getType()} to determine its generic type. + * + * @return The wrapped setting. + */ + public Setting getSetting() { + return this.setting; + } + + /** + * Gets the generic type of the wrapped setting. + * + * @return The wrapped setting's generic type. + */ + public SettingType getType() { + return this.type; + } + + @SuppressWarnings("unchecked") + private Setting createSetting( + SettingType type, + String key, + Object defaultValue, + WriteableSetting fallback, + Property[] propertyArray + ) { + switch (type) { + case Boolean: + return fallback == null + ? Setting.boolSetting(key, (boolean) defaultValue, propertyArray) + : Setting.boolSetting(key, (Setting) fallback.getSetting(), propertyArray); + case Integer: + return fallback == null + ? Setting.intSetting(key, (int) defaultValue, propertyArray) + : Setting.intSetting(key, (Setting) fallback.getSetting(), propertyArray); + case Long: + return fallback == null + ? Setting.longSetting(key, (long) defaultValue, propertyArray) + : Setting.longSetting(key, (Setting) fallback.getSetting(), propertyArray); + case Float: + return fallback == null + ? Setting.floatSetting(key, (float) defaultValue, propertyArray) + : Setting.floatSetting(key, (Setting) fallback.getSetting(), propertyArray); + case Double: + return fallback == null + ? Setting.doubleSetting(key, (double) defaultValue, propertyArray) + : Setting.doubleSetting(key, (Setting) fallback.getSetting(), propertyArray); + case String: + return fallback == null + ? Setting.simpleString(key, (String) defaultValue, propertyArray) + : Setting.simpleString(key, (Setting) fallback.getSetting(), propertyArray); + case TimeValue: + return fallback == null + ? Setting.timeSetting(key, (TimeValue) defaultValue, propertyArray) + : Setting.timeSetting(key, (Setting) fallback.getSetting(), propertyArray); + case ByteSizeValue: + return fallback == null + ? Setting.byteSizeSetting(key, (ByteSizeValue) defaultValue, propertyArray) + : Setting.byteSizeSetting(key, (Setting) fallback.getSetting(), propertyArray); + case Version: + // No fallback option on this method + return Setting.versionSetting(key, (Version) defaultValue, propertyArray); + default: + // This Should Never Happen (TM) + throw new UnsupportedOperationException("A SettingType has been added to the enum and not handled here."); + } + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + // Write the type + out.writeEnum(type); + // Write the key + out.writeString(setting.getKey()); + // Write the default value + writeDefaultValue(out, setting.getDefault(Settings.EMPTY)); + // Write a boolean specifying whether the fallback settings is null + boolean hasFallback = setting.fallbackSetting != null; + out.writeBoolean(hasFallback); + if (hasFallback) { + new WriteableSetting(setting.fallbackSetting, type).writeTo(out); + } + // We are using known types so don't need the parser + // We are not using validator + // Write properties + out.writeEnumSet(setting.getProperties()); + } + + private void writeDefaultValue(StreamOutput out, Object defaultValue) throws IOException { + switch (type) { + case Boolean: + out.writeBoolean((boolean) defaultValue); + break; + case Integer: + out.writeInt((int) defaultValue); + break; + case Long: + out.writeLong((long) defaultValue); + break; + case Float: + out.writeFloat((float) defaultValue); + break; + case Double: + out.writeDouble((double) defaultValue); + break; + case String: + out.writeString((String) defaultValue); + break; + case TimeValue: + TimeValue tv = (TimeValue) defaultValue; + out.writeLong(tv.duration()); + out.writeString(tv.timeUnit().name()); + break; + case ByteSizeValue: + ((ByteSizeValue) defaultValue).writeTo(out); + break; + case Version: + Version.writeVersion((Version) defaultValue, out); + break; + default: + // This Should Never Happen (TM) + throw new UnsupportedOperationException("A SettingType has been added to the enum and not handled here."); + } + } + + private Object readDefaultValue(StreamInput in) throws IOException { + switch (type) { + case Boolean: + return in.readBoolean(); + case Integer: + return in.readInt(); + case Long: + return in.readLong(); + case Float: + return in.readFloat(); + case Double: + return in.readDouble(); + case String: + return in.readString(); + case TimeValue: + long duration = in.readLong(); + TimeUnit unit = TimeUnit.valueOf(in.readString()); + return new TimeValue(duration, unit); + case ByteSizeValue: + return new ByteSizeValue(in); + case Version: + return Version.readVersion(in); + default: + // This Should Never Happen (TM) + throw new UnsupportedOperationException("A SettingType has been added to the enum and not handled here."); + } + } + + @Override + public String toString() { + return "WriteableSettings{type=Setting<" + type + ">, setting=" + setting + "}"; + } +} diff --git a/server/src/main/java/org/opensearch/env/EnvironmentSettingsResponse.java b/server/src/main/java/org/opensearch/env/EnvironmentSettingsResponse.java new file mode 100644 index 0000000000000..0f541ed8ce51b --- /dev/null +++ b/server/src/main/java/org/opensearch/env/EnvironmentSettingsResponse.java @@ -0,0 +1,86 @@ +/* + * 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.env; + +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.transport.TransportResponse; +import org.opensearch.common.settings.WriteableSetting; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * Environment Settings Response for Extensibility + * + * @opensearch.internal + */ +public class EnvironmentSettingsResponse extends TransportResponse { + private Map, Object> componentSettingValues; + + public EnvironmentSettingsResponse(Settings environmentSettings, List> componentSettings) { + Map, Object> componentSettingValues = new HashMap<>(); + for (Setting componentSetting : componentSettings) { + + // Retrieve component setting value from enviornment settings, or default value if it does not exist + Object componentSettingValue = componentSetting.get(environmentSettings); + componentSettingValues.put(componentSetting, componentSettingValue); + } + this.componentSettingValues = componentSettingValues; + } + + public EnvironmentSettingsResponse(StreamInput in) throws IOException { + super(in); + Map, Object> componentSettingValues = new HashMap<>(); + int componentSettingValuesCount = in.readVInt(); + for (int i = 0; i < componentSettingValuesCount; i++) { + Setting componentSetting = new WriteableSetting(in).getSetting(); + Object componentSettingValue = in.readGenericValue(); + componentSettingValues.put(componentSetting, componentSettingValue); + } + this.componentSettingValues = componentSettingValues; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeVInt(componentSettingValues.size()); + for (Map.Entry, Object> entry : componentSettingValues.entrySet()) { + new WriteableSetting(entry.getKey()).writeTo(out); + out.writeGenericValue(entry.getValue()); + } + } + + public Map, Object> getComponentSettingValues() { + return Collections.unmodifiableMap(this.componentSettingValues); + } + + @Override + public String toString() { + return "EnvironmentSettingsResponse{componentSettingValues=" + componentSettingValues.toString() + '}'; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + EnvironmentSettingsResponse that = (EnvironmentSettingsResponse) o; + return Objects.equals(componentSettingValues, that.componentSettingValues); + } + + @Override + public int hashCode() { + return Objects.hash(componentSettingValues); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequest.java b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequest.java new file mode 100644 index 0000000000000..687e3a07e3108 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequest.java @@ -0,0 +1,98 @@ +/* + * 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; + +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.transport.TransportRequest; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.WriteableSetting; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +/** + * Add Settings Update Consumer Request for Extensibility + * + * @opensearch.internal + */ +public class AddSettingsUpdateConsumerRequest extends TransportRequest { + private final DiscoveryExtensionNode extensionNode; + private final List componentSettings; + + public AddSettingsUpdateConsumerRequest(DiscoveryExtensionNode extensionNode, List> componentSettings) { + this.extensionNode = extensionNode; + this.componentSettings = new ArrayList<>(componentSettings.size()); + for (Setting setting : componentSettings) { + this.componentSettings.add(new WriteableSetting(setting)); + } + } + + public AddSettingsUpdateConsumerRequest(StreamInput in) throws IOException { + super(in); + + // Set extension node to send update settings request to + this.extensionNode = new DiscoveryExtensionNode(in); + + // Read in component setting list + int componentSettingsCount = in.readVInt(); + List componentSettings = new ArrayList<>(componentSettingsCount); + for (int i = 0; i < componentSettingsCount; i++) { + componentSettings.add(new WriteableSetting(in)); + } + this.componentSettings = componentSettings; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + + // Write extension node to stream output + this.extensionNode.writeTo(out); + + // Write component setting list to stream output + out.writeVInt(this.componentSettings.size()); + for (WriteableSetting componentSetting : this.componentSettings) { + componentSetting.writeTo(out); + } + } + + public List getComponentSettings() { + return new ArrayList<>(this.componentSettings); + } + + public DiscoveryExtensionNode getExtensionNode() { + return this.extensionNode; + } + + @Override + public String toString() { + return "AddSettingsUpdateConsumerRequest{extensionNode=" + + this.extensionNode.toString() + + ", componentSettings=" + + this.componentSettings.toString() + + "}"; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + AddSettingsUpdateConsumerRequest that = (AddSettingsUpdateConsumerRequest) obj; + return Objects.equals(extensionNode, that.extensionNode) && Objects.equals(componentSettings, that.componentSettings); + } + + @Override + public int hashCode() { + return Objects.hash(extensionNode, componentSettings); + } + +} diff --git a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java new file mode 100644 index 0000000000000..791482aad0432 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java @@ -0,0 +1,91 @@ +/* + * 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; + +import java.util.List; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.WriteableSetting; +import org.opensearch.transport.TransportResponse; +import org.opensearch.transport.TransportService; + +/** + * Handles requests to add setting update consumers + * + * @opensearch.internal + */ +public class AddSettingsUpdateConsumerRequestHandler { + + private static final Logger logger = LogManager.getLogger(AddSettingsUpdateConsumerRequestHandler.class); + + private final ClusterService clusterService; + private final TransportService transportService; + private final String updateSettingsRequestType; + + /** + * Instantiates a new Add Settings Update Consumer Request Handler with the {@link ClusterService} and {@link TransportService} + * + * @param clusterService the cluster service used to extract cluster settings + * @param transportService the node's transport service + * @param updateSettingsRequestType the update settings request type + */ + public AddSettingsUpdateConsumerRequestHandler( + ClusterService clusterService, + TransportService transportService, + String updateSettingsRequestType + ) { + this.clusterService = clusterService; + this.transportService = transportService; + this.updateSettingsRequestType = updateSettingsRequestType; + } + + /** + * Handles a {@link AddSettingsUpdateConsumerRequest}. + * + * @param addSettingsUpdateConsumerRequest The request to handle. + * @return A {@link AcknowledgedResponse} indicating success. + * @throws Exception if the request is not handled properly. + */ + TransportResponse handleAddSettingsUpdateConsumerRequest(AddSettingsUpdateConsumerRequest addSettingsUpdateConsumerRequest) + throws Exception { + + boolean status = true; + List extensionComponentSettings = addSettingsUpdateConsumerRequest.getComponentSettings(); + DiscoveryExtensionNode extensionNode = addSettingsUpdateConsumerRequest.getExtensionNode(); + + try { + for (WriteableSetting extensionComponentSetting : extensionComponentSettings) { + + // Extract setting and type from writeable setting + Setting setting = extensionComponentSetting.getSetting(); + WriteableSetting.SettingType settingType = extensionComponentSetting.getType(); + + // Register setting update consumer with callback method to extension + clusterService.getClusterSettings().addSettingsUpdateConsumer(setting, (data) -> { + logger.info("Sending extension request type: " + updateSettingsRequestType); + UpdateSettingsResponseHandler updateSettingsResponseHandler = new UpdateSettingsResponseHandler(); + transportService.sendRequest( + extensionNode, + updateSettingsRequestType, + new UpdateSettingsRequest(settingType, setting, data), + updateSettingsResponseHandler + ); + }); + } + } catch (IllegalArgumentException e) { + logger.error(e.toString()); + status = false; + } + + return new AcknowledgedResponse(status); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequest.java b/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequest.java new file mode 100644 index 0000000000000..ab470087f8ec9 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequest.java @@ -0,0 +1,79 @@ +/* + * 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; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.transport.TransportRequest; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.WriteableSetting; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +/** + * Environment Settings Request for Extensibility + * + * @opensearch.internal + */ +public class EnvironmentSettingsRequest extends TransportRequest { + private static final Logger logger = LogManager.getLogger(EnvironmentSettingsRequest.class); + private List> componentSettings; + + public EnvironmentSettingsRequest(List> componentSettings) { + this.componentSettings = new ArrayList<>(componentSettings); + } + + public EnvironmentSettingsRequest(StreamInput in) throws IOException { + super(in); + int componentSettingsCount = in.readVInt(); + List> componentSettings = new ArrayList<>(componentSettingsCount); + for (int i = 0; i < componentSettingsCount; i++) { + WriteableSetting writeableSetting = new WriteableSetting(in); + componentSettings.add(writeableSetting.getSetting()); + } + this.componentSettings = componentSettings; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeVInt(this.componentSettings.size()); + for (Setting componentSetting : componentSettings) { + new WriteableSetting(componentSetting).writeTo(out); + } + } + + public List> getComponentSettings() { + return new ArrayList<>(componentSettings); + } + + @Override + public String toString() { + return "EnvironmentSettingsRequest{componentSettings=" + componentSettings.toString() + "}"; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + EnvironmentSettingsRequest that = (EnvironmentSettingsRequest) obj; + return Objects.equals(componentSettings, that.componentSettings); + } + + @Override + public int hashCode() { + return Objects.hash(componentSettings); + } + +} diff --git a/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequestHandler.java b/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequestHandler.java new file mode 100644 index 0000000000000..723eb482d7c44 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequestHandler.java @@ -0,0 +1,43 @@ +/* + * 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; + +import org.opensearch.common.settings.Settings; +import org.opensearch.env.EnvironmentSettingsResponse; +import org.opensearch.transport.TransportResponse; + +/** + * Handles requests to retrieve environment settings. + * + * @opensearch.internal + */ +public class EnvironmentSettingsRequestHandler { + + private final Settings initialEnvironmentSettings; + + /** + * Instantiates a new Environment Settings Request Handler with the environment settings + * + * @param initialEnvironmentSettings the finalized view of environment {@link Settings} + */ + public EnvironmentSettingsRequestHandler(Settings initialEnvironmentSettings) { + this.initialEnvironmentSettings = initialEnvironmentSettings; + } + + /** + * Handles a {@link EnvironmentSettingsRequest}. + * + * @param environmentSettingsRequest The request to handle. + * @return A {@link EnvironmentSettingsResponse} + * @throws Exception if the request is not handled properly. + */ + TransportResponse handleEnvironmentSettingsRequest(EnvironmentSettingsRequest environmentSettingsRequest) throws Exception { + return new EnvironmentSettingsResponse(this.initialEnvironmentSettings, environmentSettingsRequest.getComponentSettings()); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java b/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java new file mode 100644 index 0000000000000..53d7e887e4814 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java @@ -0,0 +1,50 @@ +/* + * 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; + +import java.util.ArrayList; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.ActionListener; +import org.opensearch.action.admin.indices.analyze.AnalyzeAction.Response; + +/** + * ActionListener for ExtensionsManager + * + * @opensearch.internal + */ +public class ExtensionActionListener implements ActionListener { + + private static final Logger logger = LogManager.getLogger(ExtensionActionListener.class); + private ArrayList exceptionList; + + public ExtensionActionListener() { + exceptionList = new ArrayList(); + } + + @Override + public void onResponse(Response response) { + logger.info("response {}", response); + } + + @Override + public void onFailure(Exception e) { + exceptionList.add(e); + logger.error(e.getMessage()); + } + + public static Logger getLogger() { + return logger; + } + + public ArrayList getExceptionList() { + return exceptionList; + } +} diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerHandler.java b/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerHandler.java new file mode 100644 index 0000000000000..ceba1e1f65000 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerHandler.java @@ -0,0 +1,43 @@ +/* + * 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; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.OpenSearchException; + +/** + * Handles ActionListener requests from extensions + * + * @opensearch.internal + */ +public class ExtensionActionListenerHandler { + + private static final Logger logger = LogManager.getLogger(ExtensionActionListener.class); + private ExtensionActionListener listener; + + public ExtensionActionListenerHandler(ExtensionActionListener listener) { + this.listener = listener; + } + + /** + * Handles a {@link ExtensionActionListenerOnFailureRequest}. + * + * @param request The request to handle. + * @return A {@link AcknowledgedResponse} indicating success or failure. + */ + public AcknowledgedResponse handleExtensionActionListenerOnFailureRequest(ExtensionActionListenerOnFailureRequest request) { + try { + listener.onFailure(new OpenSearchException(request.getFailureException())); + return new AcknowledgedResponse(true); + } catch (Exception e) { + logger.error(e.getMessage()); + return new AcknowledgedResponse(false); + } + } +} diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java b/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java new file mode 100644 index 0000000000000..22a12433f06d3 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java @@ -0,0 +1,72 @@ +/* + * 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; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.transport.TransportRequest; + +import java.io.IOException; +import java.util.Objects; + +/** + * ClusterService Request for Action Listener onFailure + * + * @opensearch.internal + */ +public class ExtensionActionListenerOnFailureRequest extends TransportRequest { + private static final Logger logger = LogManager.getLogger(ExtensionRequest.class); + private String failureExceptionMessage; + + /** + * Instantiates a request for ActionListener onFailure from an extension + * + * @param failureExceptionMessage A String that contains both the Exception type and message + */ + public ExtensionActionListenerOnFailureRequest(String failureExceptionMessage) { + super(); + this.failureExceptionMessage = failureExceptionMessage; + } + + public ExtensionActionListenerOnFailureRequest(StreamInput in) throws IOException { + super(in); + this.failureExceptionMessage = in.readString(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(failureExceptionMessage); + } + + public String toString() { + return "ExtensionActionListenerOnFailureRequest{" + "failureExceptionMessage= " + failureExceptionMessage + " }"; + } + + @Override + public boolean equals(Object o) { + + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ExtensionActionListenerOnFailureRequest that = (ExtensionActionListenerOnFailureRequest) o; + return Objects.equals(failureExceptionMessage, that.failureExceptionMessage); + } + + @Override + public int hashCode() { + return Objects.hash(failureExceptionMessage); + } + + public String getFailureException() { + return failureExceptionMessage; + } + +} diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java b/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java index 924fce49a5dc2..44d59f0815975 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java @@ -51,7 +51,6 @@ public String toString() { @Override public boolean equals(Object o) { - if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; ExtensionRequest that = (ExtensionRequest) o; @@ -62,5 +61,4 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(requestType); } - } diff --git a/server/src/main/java/org/opensearch/extensions/rest/RegisterRestActionsResponse.java b/server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java similarity index 69% rename from server/src/main/java/org/opensearch/extensions/rest/RegisterRestActionsResponse.java rename to server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java index c0a79ad32ce89..5c9c4c3ef784b 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RegisterRestActionsResponse.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.extensions.rest; +package org.opensearch.extensions; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; @@ -15,18 +15,18 @@ import java.io.IOException; /** - * Response to register REST Actions request. + * Generic string response indicating the status of some previous request sent to the SDK * * @opensearch.internal */ -public class RegisterRestActionsResponse extends TransportResponse { +public class ExtensionStringResponse extends TransportResponse { private String response; - public RegisterRestActionsResponse(String response) { + public ExtensionStringResponse(String response) { this.response = response; } - public RegisterRestActionsResponse(StreamInput in) throws IOException { + public ExtensionStringResponse(StreamInput in) throws IOException { response = in.readString(); } diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index be843fe35a5f9..cda8b40ef1b2d 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -33,6 +33,7 @@ import org.opensearch.common.io.FileSystemUtils; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsModule; import org.opensearch.common.transport.TransportAddress; import org.opensearch.discovery.InitializeExtensionRequest; @@ -40,6 +41,8 @@ import org.opensearch.extensions.ExtensionsSettings.Extension; import org.opensearch.extensions.rest.RegisterRestActionsRequest; import org.opensearch.extensions.rest.RestActionsRequestHandler; +import org.opensearch.extensions.settings.CustomSettingsRequestHandler; +import org.opensearch.extensions.settings.RegisterCustomSettingsRequest; import org.opensearch.index.IndexModule; import org.opensearch.index.IndexService; import org.opensearch.index.IndicesModuleRequest; @@ -69,11 +72,15 @@ public class ExtensionsManager { public static final String REQUEST_EXTENSION_CLUSTER_STATE = "internal:discovery/clusterstate"; public static final String REQUEST_EXTENSION_LOCAL_NODE = "internal:discovery/localnode"; public static final String REQUEST_EXTENSION_CLUSTER_SETTINGS = "internal:discovery/clustersettings"; + public static final String REQUEST_EXTENSION_ENVIRONMENT_SETTINGS = "internal:discovery/enviornmentsettings"; + public static final String REQUEST_EXTENSION_ADD_SETTINGS_UPDATE_CONSUMER = "internal:discovery/addsettingsupdateconsumer"; + public static final String REQUEST_EXTENSION_UPDATE_SETTINGS = "internal:discovery/updatesettings"; + public static final String REQUEST_EXTENSION_REGISTER_CUSTOM_SETTINGS = "internal:discovery/registercustomsettings"; public static final String REQUEST_EXTENSION_REGISTER_REST_ACTIONS = "internal:discovery/registerrestactions"; - public static final String REQUEST_OPENSEARCH_NAMED_WRITEABLE_REGISTRY = "internal:discovery/namedwriteableregistry"; + public static final String REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS = "internal:discovery/registertransportactions"; public static final String REQUEST_OPENSEARCH_PARSE_NAMED_WRITEABLE = "internal:discovery/parsenamedwriteable"; + public static final String REQUEST_EXTENSION_ACTION_LISTENER_ON_FAILURE = "internal:extensions/actionlisteneronfailure"; public static final String REQUEST_REST_EXECUTE_ON_EXTENSION_ACTION = "internal:extensions/restexecuteonextensiontaction"; - public static final String REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS = "internal:discovery/registertransportactions"; private static final Logger logger = LogManager.getLogger(ExtensionsManager.class); @@ -86,7 +93,9 @@ public static enum RequestType { REQUEST_EXTENSION_CLUSTER_STATE, REQUEST_EXTENSION_LOCAL_NODE, REQUEST_EXTENSION_CLUSTER_SETTINGS, + REQUEST_EXTENSION_ACTION_LISTENER_ON_FAILURE, REQUEST_EXTENSION_REGISTER_REST_ACTIONS, + REQUEST_EXTENSION_REGISTER_SETTINGS, CREATE_COMPONENT, ON_INDEX_MODULE, GET_SETTINGS @@ -106,8 +115,13 @@ public static enum OpenSearchRequestType { private List extensions; private Map extensionIdMap; private RestActionsRequestHandler restActionsRequestHandler; + private CustomSettingsRequestHandler customSettingsRequestHandler; private TransportService transportService; private ClusterService clusterService; + private ExtensionActionListener listener; + private ExtensionActionListenerHandler listenerHandler; + private EnvironmentSettingsRequestHandler environmentSettingsRequestHandler; + private AddSettingsUpdateConsumerRequestHandler addSettingsUpdateConsumerRequestHandler; public ExtensionsManager() { this.extensionsPath = Path.of(""); @@ -127,6 +141,7 @@ public ExtensionsManager(Settings settings, Path extensionsPath) throws IOExcept this.extensions = new ArrayList(); this.extensionIdMap = new HashMap(); this.clusterService = null; + this.listener = new ExtensionActionListener(); /* * Now Discover extensions @@ -136,21 +151,33 @@ public ExtensionsManager(Settings settings, Path extensionsPath) throws IOExcept } /** - * Initializes the {@link RestActionsRequestHandler}, {@link TransportService} and {@link ClusterService}. This is called during Node bootstrap. + * Initializes the {@link RestActionsRequestHandler}, {@link TransportService}, {@link ClusterService} and environment settings. This is called during Node bootstrap. * Lists/maps of extensions have already been initialized but not yet populated. * * @param restController The RestController on which to register Rest Actions. + * @param settingsModule The module that binds the provided settings to interface. * @param transportService The Node's transport service. * @param clusterService The Node's cluster service. + * @param initialEnvironmentSettings The finalized view of settings for the Environment */ public void initializeServicesAndRestHandler( RestController restController, + SettingsModule settingsModule, TransportService transportService, - ClusterService clusterService + ClusterService clusterService, + Settings initialEnvironmentSettings ) { this.restActionsRequestHandler = new RestActionsRequestHandler(restController, extensionIdMap, transportService); + this.listenerHandler = new ExtensionActionListenerHandler(listener); + this.customSettingsRequestHandler = new CustomSettingsRequestHandler(settingsModule); this.transportService = transportService; this.clusterService = clusterService; + this.environmentSettingsRequestHandler = new EnvironmentSettingsRequestHandler(initialEnvironmentSettings); + this.addSettingsUpdateConsumerRequestHandler = new AddSettingsUpdateConsumerRequestHandler( + clusterService, + transportService, + REQUEST_EXTENSION_UPDATE_SETTINGS + ); registerRequestHandler(); } @@ -163,6 +190,14 @@ private void registerRequestHandler() { RegisterRestActionsRequest::new, ((request, channel, task) -> channel.sendResponse(restActionsRequestHandler.handleRegisterRestActionsRequest(request))) ); + transportService.registerRequestHandler( + REQUEST_EXTENSION_REGISTER_CUSTOM_SETTINGS, + ThreadPool.Names.GENERIC, + false, + false, + RegisterCustomSettingsRequest::new, + ((request, channel, task) -> channel.sendResponse(customSettingsRequestHandler.handleRegisterCustomSettingsRequest(request))) + ); transportService.registerRequestHandler( REQUEST_EXTENSION_CLUSTER_STATE, ThreadPool.Names.GENERIC, @@ -187,6 +222,32 @@ private void registerRequestHandler() { ExtensionRequest::new, ((request, channel, task) -> channel.sendResponse(handleExtensionRequest(request))) ); + transportService.registerRequestHandler( + REQUEST_EXTENSION_ACTION_LISTENER_ON_FAILURE, + ThreadPool.Names.GENERIC, + false, + false, + ExtensionActionListenerOnFailureRequest::new, + ((request, channel, task) -> channel.sendResponse(listenerHandler.handleExtensionActionListenerOnFailureRequest(request))) + ); + transportService.registerRequestHandler( + REQUEST_EXTENSION_ENVIRONMENT_SETTINGS, + ThreadPool.Names.GENERIC, + false, + false, + EnvironmentSettingsRequest::new, + ((request, channel, task) -> channel.sendResponse(environmentSettingsRequestHandler.handleEnvironmentSettingsRequest(request))) + ); + transportService.registerRequestHandler( + REQUEST_EXTENSION_ADD_SETTINGS_UPDATE_CONSUMER, + ThreadPool.Names.GENERIC, + false, + false, + AddSettingsUpdateConsumerRequest::new, + ((request, channel, task) -> channel.sendResponse( + addSettingsUpdateConsumerRequestHandler.handleAddSettingsUpdateConsumerRequest(request) + )) + ); transportService.registerRequestHandler( REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS, ThreadPool.Names.GENERIC, @@ -357,7 +418,7 @@ TransportResponse handleExtensionRequest(ExtensionRequest extensionRequest) thro case REQUEST_EXTENSION_CLUSTER_SETTINGS: return new ClusterSettingsResponse(clusterService); default: - throw new IllegalStateException("Handler not present for the provided request"); + throw new IllegalArgumentException("Handler not present for the provided request"); } } @@ -528,10 +589,6 @@ public static String getRequestExtensionRegisterRestActions() { return REQUEST_EXTENSION_REGISTER_REST_ACTIONS; } - public static String getRequestOpensearchNamedWriteableRegistry() { - return REQUEST_OPENSEARCH_NAMED_WRITEABLE_REGISTRY; - } - public static String getRequestOpensearchParseNamedWriteable() { return REQUEST_OPENSEARCH_PARSE_NAMED_WRITEABLE; } @@ -548,4 +605,90 @@ public RestActionsRequestHandler getRestActionsRequestHandler() { return restActionsRequestHandler; } + public void setExtensions(List extensions) { + this.extensions = extensions; + } + + public void setExtensionIdMap(Map extensionIdMap) { + this.extensionIdMap = extensionIdMap; + } + + public void setRestActionsRequestHandler(RestActionsRequestHandler restActionsRequestHandler) { + this.restActionsRequestHandler = restActionsRequestHandler; + } + + public void setTransportService(TransportService transportService) { + this.transportService = transportService; + } + + public void setClusterService(ClusterService clusterService) { + this.clusterService = clusterService; + } + + public static String getRequestExtensionRegisterTransportActions() { + return REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS; + } + + public static String getRequestExtensionActionListenerOnFailure() { + return REQUEST_EXTENSION_ACTION_LISTENER_ON_FAILURE; + } + + public ExtensionActionListener getListener() { + return listener; + } + + public static String getRequestExtensionRegisterCustomSettings() { + return REQUEST_EXTENSION_REGISTER_CUSTOM_SETTINGS; + } + + public CustomSettingsRequestHandler getCustomSettingsRequestHandler() { + return customSettingsRequestHandler; + } + + public void setCustomSettingsRequestHandler(CustomSettingsRequestHandler customSettingsRequestHandler) { + this.customSettingsRequestHandler = customSettingsRequestHandler; + } + + public static String getRequestExtensionEnvironmentSettings() { + return REQUEST_EXTENSION_ENVIRONMENT_SETTINGS; + } + + public static String getRequestExtensionAddSettingsUpdateConsumer() { + return REQUEST_EXTENSION_ADD_SETTINGS_UPDATE_CONSUMER; + } + + public static String getRequestExtensionUpdateSettings() { + return REQUEST_EXTENSION_UPDATE_SETTINGS; + } + + public EnvironmentSettingsRequestHandler getEnvironmentSettingsRequestHandler() { + return environmentSettingsRequestHandler; + } + + public void setEnvironmentSettingsRequestHandler(EnvironmentSettingsRequestHandler environmentSettingsRequestHandler) { + this.environmentSettingsRequestHandler = environmentSettingsRequestHandler; + } + + public AddSettingsUpdateConsumerRequestHandler getAddSettingsUpdateConsumerRequestHandler() { + return addSettingsUpdateConsumerRequestHandler; + } + + public void setAddSettingsUpdateConsumerRequestHandler( + AddSettingsUpdateConsumerRequestHandler addSettingsUpdateConsumerRequestHandler + ) { + this.addSettingsUpdateConsumerRequestHandler = addSettingsUpdateConsumerRequestHandler; + } + + public void setListener(ExtensionActionListener listener) { + this.listener = listener; + } + + public ExtensionActionListenerHandler getListenerHandler() { + return listenerHandler; + } + + public void setListenerHandler(ExtensionActionListenerHandler listenerHandler) { + this.listenerHandler = listenerHandler; + } + } diff --git a/server/src/main/java/org/opensearch/extensions/UpdateSettingsRequest.java b/server/src/main/java/org/opensearch/extensions/UpdateSettingsRequest.java new file mode 100644 index 0000000000000..3191f189ac18b --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/UpdateSettingsRequest.java @@ -0,0 +1,93 @@ +/* + * 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; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.WriteableSetting; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.transport.TransportRequest; + +import java.io.IOException; +import java.util.Objects; + +/** + * Update Settings Request for Extensibility + * + * @opensearch.internal + */ +public class UpdateSettingsRequest extends TransportRequest { + private static final Logger logger = LogManager.getLogger(EnvironmentSettingsRequest.class); + + private WriteableSetting.SettingType settingType; + private Setting componentSetting; + private Object data; + + public UpdateSettingsRequest(WriteableSetting.SettingType settingType, Setting componentSetting, Object data) { + this.settingType = settingType; + this.componentSetting = componentSetting; + this.data = data; + } + + public UpdateSettingsRequest(StreamInput in) throws IOException { + super(in); + this.settingType = in.readEnum(WriteableSetting.SettingType.class); + this.componentSetting = new WriteableSetting(in).getSetting(); + this.data = in.readGenericValue(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeEnum(settingType); + new WriteableSetting(componentSetting).writeTo(out); + out.writeGenericValue(this.data); + } + + public WriteableSetting.SettingType getSettingType() { + return this.settingType; + } + + public Setting getComponentSetting() { + return this.componentSetting; + } + + public Object getData() { + return this.data; + } + + @Override + public String toString() { + return "UpdateSettingRequest{settingType=" + + this.settingType.toString() + + "componentSetting=" + + this.componentSetting.toString() + + ", data=" + + this.data.toString() + + "}"; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + UpdateSettingsRequest that = (UpdateSettingsRequest) obj; + return Objects.equals(settingType, that.settingType) + && Objects.equals(componentSetting, that.componentSetting) + && Objects.equals(data, that.data); + } + + @Override + public int hashCode() { + return Objects.hash(settingType, componentSetting, data); + } + +} diff --git a/server/src/main/java/org/opensearch/extensions/UpdateSettingsResponseHandler.java b/server/src/main/java/org/opensearch/extensions/UpdateSettingsResponseHandler.java new file mode 100644 index 0000000000000..be8f43b5cfce6 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/UpdateSettingsResponseHandler.java @@ -0,0 +1,47 @@ +/* + * 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; + +import java.io.IOException; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.TransportException; +import org.opensearch.transport.TransportResponseHandler; + +/** + * Response handler for {@link UpdateSettingsRequest} + * + * @opensearch.internal + */ +public class UpdateSettingsResponseHandler implements TransportResponseHandler { + private static final Logger logger = LogManager.getLogger(UpdateSettingsResponseHandler.class); + + @Override + public AcknowledgedResponse read(StreamInput in) throws IOException { + return new AcknowledgedResponse(in); + } + + @Override + public void handleResponse(AcknowledgedResponse response) { + logger.info("response {}", response.getStatus()); + } + + @Override + public void handleException(TransportException exp) { + logger.error(new ParameterizedMessage("UpdateSettingsRequest failed"), exp); + } + + @Override + public String executor() { + return ThreadPool.Names.GENERIC; + } +} 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 e24f5d519bf81..8c935b1dc87a6 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java @@ -9,6 +9,7 @@ package org.opensearch.extensions.rest; import org.opensearch.extensions.DiscoveryExtensionNode; +import org.opensearch.extensions.ExtensionStringResponse; import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; import org.opensearch.transport.TransportResponse; @@ -48,14 +49,14 @@ public RestActionsRequestHandler( * Handles a {@link RegisterRestActionsRequest}. * * @param restActionsRequest The request to handle. - * @return A {@link RegisterRestActionsResponse} indicating success. + * @return A {@link ExtensionStringResponse} indicating success. * @throws Exception if the request is not handled properly. */ public TransportResponse handleRegisterRestActionsRequest(RegisterRestActionsRequest restActionsRequest) throws Exception { DiscoveryExtensionNode discoveryExtensionNode = extensionIdMap.get(restActionsRequest.getUniqueId()); RestHandler handler = new RestSendToExtensionAction(restActionsRequest, discoveryExtensionNode, transportService); restController.registerHandler(handler); - return new RegisterRestActionsResponse( + return new ExtensionStringResponse( "Registered extension " + restActionsRequest.getUniqueId() + " to handle REST Actions " + restActionsRequest.getRestActions() ); } diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java index 8f5a2e6b1c8a5..d08a74c0ba314 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java @@ -156,6 +156,8 @@ public String executor() { transportService.sendRequest( discoveryExtensionNode, ExtensionsManager.REQUEST_REST_EXECUTE_ON_EXTENSION_ACTION, + // HERE BE DRAGONS - DO NOT INCLUDE HEADERS + // SEE https://github.com/opensearch-project/OpenSearch/issues/4429 new RestExecuteOnExtensionRequest(method, uri), restExecuteOnExtensionResponseHandler ); diff --git a/server/src/main/java/org/opensearch/extensions/settings/CustomSettingsRequestHandler.java b/server/src/main/java/org/opensearch/extensions/settings/CustomSettingsRequestHandler.java new file mode 100644 index 0000000000000..83d34facb35f6 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/settings/CustomSettingsRequestHandler.java @@ -0,0 +1,57 @@ +/* + * 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.settings; + +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.SettingsModule; +import org.opensearch.extensions.ExtensionStringResponse; +import org.opensearch.transport.TransportResponse; + +import java.util.ArrayList; +import java.util.List; + +/** + * Handles requests to register a list of custom extension settings. + * + * @opensearch.internal + */ +public class CustomSettingsRequestHandler { + + private final SettingsModule settingsModule; + + /** + * Instantiates a new Settings Request Handler using the Node's SettingsModule. + * + * @param settingsModule The Node's {@link SettingsModule}. + */ + public CustomSettingsRequestHandler(SettingsModule settingsModule) { + this.settingsModule = settingsModule; + } + + /** + * Handles a {@link RegisterCustomSettingsRequest}. + * + * @param customSettingsRequest The request to handle. + * @return A {@link ExtensionStringResponse} indicating success. + * @throws Exception if the request is not handled properly. + */ + public TransportResponse handleRegisterCustomSettingsRequest(RegisterCustomSettingsRequest customSettingsRequest) throws Exception { + // TODO: How do we prevent key collisions in settings registration? + // we have settingsRequest.getUniqueId() available or could enforce reverse DNS naming + // See https://github.com/opensearch-project/opensearch-sdk-java/issues/142 + List registeredCustomSettings = new ArrayList<>(); + for (Setting setting : customSettingsRequest.getSettings()) { + settingsModule.registerDynamicSetting(setting); + registeredCustomSettings.add(setting.getKey()); + } + return new ExtensionStringResponse( + "Registered settings from extension " + customSettingsRequest.getUniqueId() + ": " + String.join(", ", registeredCustomSettings) + ); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/settings/RegisterCustomSettingsRequest.java b/server/src/main/java/org/opensearch/extensions/settings/RegisterCustomSettingsRequest.java new file mode 100644 index 0000000000000..b8217972767f9 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/settings/RegisterCustomSettingsRequest.java @@ -0,0 +1,83 @@ +/* + * 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.settings; + +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.WriteableSetting; +import org.opensearch.transport.TransportRequest; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +/** + * Request to register a list of custom extension settings + * + * @opensearch.internal + */ +public class RegisterCustomSettingsRequest extends TransportRequest { + private String uniqueId; + private List> settings; + + public RegisterCustomSettingsRequest(String uniqueId, List> settings) { + this.uniqueId = uniqueId; + this.settings = new ArrayList<>(settings); + } + + public RegisterCustomSettingsRequest(StreamInput in) throws IOException { + super(in); + this.uniqueId = in.readString(); + int size = in.readVInt(); + List> settingsList = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + WriteableSetting ws = new WriteableSetting(in); + settingsList.add(ws.getSetting()); + } + this.settings = settingsList; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(uniqueId); + out.writeVInt(settings.size()); + for (Setting setting : settings) { + new WriteableSetting(setting).writeTo(out); + } + } + + public String getUniqueId() { + return uniqueId; + } + + public List> getSettings() { + return new ArrayList<>(settings); + } + + @Override + public String toString() { + return "RegisterSettingsRequest{uniqueId=" + uniqueId + ", settings=" + settings + "}"; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + RegisterCustomSettingsRequest that = (RegisterCustomSettingsRequest) obj; + return Objects.equals(uniqueId, that.uniqueId) && Objects.equals(settings, that.settings); + } + + @Override + public int hashCode() { + return Objects.hash(uniqueId, settings); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/settings/package-info.java b/server/src/main/java/org/opensearch/extensions/settings/package-info.java new file mode 100644 index 0000000000000..4ae82baba9bbb --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/settings/package-info.java @@ -0,0 +1,10 @@ +/* + * 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. + */ + +/** Settings classes for the extensions package. OpenSearch extensions provide extensibility to OpenSearch.*/ +package org.opensearch.extensions.settings; diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 25b821430ce4e..e127826921fe9 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -828,7 +828,13 @@ protected Node( taskHeaders ); if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) { - this.extensionsManager.initializeServicesAndRestHandler(restController, transportService, clusterService); + this.extensionsManager.initializeServicesAndRestHandler( + restController, + settingsModule, + transportService, + clusterService, + environment.settings() + ); } final GatewayMetaState gatewayMetaState = new GatewayMetaState(); final ResponseCollectorService responseCollectorService = new ResponseCollectorService(clusterService); diff --git a/server/src/test/java/org/opensearch/common/settings/WriteableSettingTests.java b/server/src/test/java/org/opensearch/common/settings/WriteableSettingTests.java new file mode 100644 index 0000000000000..98eb81ef23957 --- /dev/null +++ b/server/src/test/java/org/opensearch/common/settings/WriteableSettingTests.java @@ -0,0 +1,487 @@ +/* + * 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.common.settings; + +import org.junit.Before; +import org.opensearch.Version; +import org.opensearch.common.SuppressForbidden; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.unit.ByteSizeUnit; +import org.opensearch.common.unit.ByteSizeValue; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.util.EnumMap; +import java.util.EnumSet; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; + +import static org.opensearch.common.settings.Setting.Property; +import static org.opensearch.common.settings.WriteableSetting.SettingType; + +public class WriteableSettingTests extends OpenSearchTestCase { + + // These settings have a default value and null fallback + private final Map> settingMap = new EnumMap<>(SettingType.class); + // These settings have a fallback setting instead of a default + private final Map> settingWithFallbackMap = new EnumMap<>(SettingType.class); + + @SuppressWarnings("unchecked") + @Before + public void setup() throws Exception { + super.setUp(); + settingMap.put(SettingType.Boolean, Setting.boolSetting("boolSettingBase", false, Property.NodeScope, Property.Dynamic)); + settingMap.put(SettingType.Integer, Setting.intSetting("intSettingBase", 6, Property.NodeScope, Property.Dynamic)); + settingMap.put(SettingType.Long, Setting.longSetting("longSettingBase", 42L, Property.NodeScope, Property.Dynamic)); + settingMap.put(SettingType.Float, Setting.floatSetting("floatSettingBase", 6.2f, Property.NodeScope, Property.Dynamic)); + settingMap.put(SettingType.Double, Setting.doubleSetting("doubleSettingBase", 42.2d, Property.NodeScope, Property.Dynamic)); + settingMap.put(SettingType.String, Setting.simpleString("simpleStringBase", "foo", Property.NodeScope, Property.Dynamic)); + settingMap.put( + SettingType.TimeValue, + Setting.timeSetting("timeSettingBase", new TimeValue(5, TimeUnit.MILLISECONDS), Property.NodeScope, Property.Dynamic) + ); + settingMap.put( + SettingType.ByteSizeValue, + Setting.byteSizeSetting("byteSizeSettingBase", new ByteSizeValue(10, ByteSizeUnit.KB), Property.NodeScope, Property.Dynamic) + ); + settingMap.put( + SettingType.Version, + Setting.versionSetting("versionSettingBase", Version.CURRENT, Property.NodeScope, Property.Dynamic) + ); + + settingWithFallbackMap.put( + SettingType.Boolean, + Setting.boolSetting("boolSetting", (Setting) settingMap.get(SettingType.Boolean), Property.NodeScope, Property.Dynamic) + ); + settingWithFallbackMap.put( + SettingType.Integer, + Setting.intSetting("intSetting", (Setting) settingMap.get(SettingType.Integer), Property.NodeScope, Property.Dynamic) + ); + settingWithFallbackMap.put( + SettingType.Long, + Setting.longSetting("longSetting", (Setting) settingMap.get(SettingType.Long), Property.NodeScope, Property.Dynamic) + ); + settingWithFallbackMap.put( + SettingType.Float, + Setting.floatSetting("floatSetting", (Setting) settingMap.get(SettingType.Float), Property.NodeScope, Property.Dynamic) + ); + settingWithFallbackMap.put( + SettingType.Double, + Setting.doubleSetting( + "doubleSetting", + (Setting) settingMap.get(SettingType.Double), + Property.NodeScope, + Property.Dynamic + ) + ); + settingWithFallbackMap.put( + SettingType.String, + Setting.simpleString("simpleString", (Setting) settingMap.get(SettingType.String), Property.NodeScope, Property.Dynamic) + ); + settingWithFallbackMap.put( + SettingType.TimeValue, + Setting.timeSetting( + "timeSetting", + (Setting) settingMap.get(SettingType.TimeValue), + Property.NodeScope, + Property.Dynamic + ) + ); + settingWithFallbackMap.put( + SettingType.ByteSizeValue, + Setting.byteSizeSetting( + "byteSizeSetting", + (Setting) settingMap.get(SettingType.ByteSizeValue), + Property.NodeScope, + Property.Dynamic + ) + ); + // No fallback for versionSetting + + } + + @SuppressWarnings("unchecked") + public void testBooleanSetting() throws IOException { + WriteableSetting ws = new WriteableSetting(settingMap.get(SettingType.Boolean)); + assertEquals(SettingType.Boolean, ws.getType()); + Setting setting = (Setting) ws.getSetting(); + assertEquals("boolSettingBase", setting.getKey()); + assertFalse(setting.getDefault(Settings.EMPTY)); + EnumSet props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + WriteableSetting wsfb = new WriteableSetting(settingWithFallbackMap.get(SettingType.Boolean)); + assertEquals(SettingType.Boolean, wsfb.getType()); + setting = (Setting) wsfb.getSetting(); + assertEquals("boolSetting", setting.getKey()); + assertFalse(setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + wsfb.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + WriteableSetting wsIn = new WriteableSetting(in); + + assertEquals(SettingType.Boolean, wsIn.getType()); + setting = (Setting) wsIn.getSetting(); + assertEquals("boolSetting", setting.getKey()); + assertFalse(setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + } + } + + } + + @SuppressWarnings("unchecked") + public void testIntegerSetting() throws IOException { + WriteableSetting ws = new WriteableSetting(settingMap.get(SettingType.Integer)); + assertEquals(SettingType.Integer, ws.getType()); + Setting setting = (Setting) ws.getSetting(); + assertEquals("intSettingBase", setting.getKey()); + assertEquals(6, (int) setting.getDefault(Settings.EMPTY)); + EnumSet props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + WriteableSetting wsfb = new WriteableSetting(settingWithFallbackMap.get(SettingType.Integer)); + assertEquals(SettingType.Integer, wsfb.getType()); + setting = (Setting) wsfb.getSetting(); + assertEquals("intSetting", setting.getKey()); + assertEquals(6, (int) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + wsfb.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + WriteableSetting wsIn = new WriteableSetting(in); + + assertEquals(SettingType.Integer, wsIn.getType()); + setting = (Setting) wsIn.getSetting(); + assertEquals("intSetting", setting.getKey()); + assertEquals(6, (int) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + } + } + } + + @SuppressWarnings("unchecked") + public void testLongSetting() throws IOException { + WriteableSetting ws = new WriteableSetting(settingMap.get(SettingType.Long)); + assertEquals(SettingType.Long, ws.getType()); + Setting setting = (Setting) ws.getSetting(); + assertEquals("longSettingBase", setting.getKey()); + assertEquals(42L, (long) setting.getDefault(Settings.EMPTY)); + EnumSet props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + WriteableSetting wsfb = new WriteableSetting(settingWithFallbackMap.get(SettingType.Long)); + assertEquals(SettingType.Long, wsfb.getType()); + setting = (Setting) wsfb.getSetting(); + assertEquals("longSetting", setting.getKey()); + assertEquals(42L, (long) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + wsfb.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + WriteableSetting wsIn = new WriteableSetting(in); + + assertEquals(SettingType.Long, wsIn.getType()); + setting = (Setting) wsIn.getSetting(); + assertEquals("longSetting", setting.getKey()); + assertEquals(42L, (long) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + } + } + } + + @SuppressWarnings("unchecked") + public void testFloatSetting() throws IOException { + WriteableSetting ws = new WriteableSetting(settingMap.get(SettingType.Float)); + assertEquals(SettingType.Float, ws.getType()); + Setting setting = (Setting) ws.getSetting(); + assertEquals("floatSettingBase", setting.getKey()); + assertEquals(6.2f, (float) setting.getDefault(Settings.EMPTY), Float.MIN_NORMAL); + EnumSet props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + WriteableSetting wsfb = new WriteableSetting(settingWithFallbackMap.get(SettingType.Float)); + assertEquals(SettingType.Float, wsfb.getType()); + setting = (Setting) wsfb.getSetting(); + assertEquals("floatSetting", setting.getKey()); + assertEquals(6.2f, (float) setting.getDefault(Settings.EMPTY), Float.MIN_NORMAL); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + wsfb.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + WriteableSetting wsIn = new WriteableSetting(in); + + assertEquals(SettingType.Float, wsIn.getType()); + setting = (Setting) wsIn.getSetting(); + assertEquals("floatSetting", setting.getKey()); + assertEquals(6.2f, (Float) setting.getDefault(Settings.EMPTY), Float.MIN_NORMAL); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + } + } + } + + @SuppressWarnings("unchecked") + public void testDoubleSetting() throws IOException { + WriteableSetting ws = new WriteableSetting(settingMap.get(SettingType.Double)); + assertEquals(SettingType.Double, ws.getType()); + Setting setting = (Setting) ws.getSetting(); + assertEquals("doubleSettingBase", setting.getKey()); + assertEquals(42.2d, (double) setting.getDefault(Settings.EMPTY), Double.MIN_NORMAL); + EnumSet props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + WriteableSetting wsfb = new WriteableSetting(settingWithFallbackMap.get(SettingType.Double)); + assertEquals(SettingType.Double, wsfb.getType()); + setting = (Setting) wsfb.getSetting(); + assertEquals("doubleSetting", setting.getKey()); + assertEquals(42.2d, (double) setting.getDefault(Settings.EMPTY), Double.MIN_NORMAL); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + wsfb.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + WriteableSetting wsIn = new WriteableSetting(in); + + assertEquals(SettingType.Double, wsIn.getType()); + setting = (Setting) wsIn.getSetting(); + assertEquals("doubleSetting", setting.getKey()); + assertEquals(42.2d, (double) setting.getDefault(Settings.EMPTY), Double.MIN_NORMAL); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + } + } + } + + @SuppressWarnings("unchecked") + public void testStringSetting() throws IOException { + WriteableSetting ws = new WriteableSetting(settingMap.get(SettingType.String)); + assertEquals(SettingType.String, ws.getType()); + Setting setting = (Setting) ws.getSetting(); + assertEquals("simpleStringBase", setting.getKey()); + assertEquals("foo", (String) setting.getDefault(Settings.EMPTY)); + EnumSet props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + WriteableSetting wsfb = new WriteableSetting(settingWithFallbackMap.get(SettingType.String)); + assertEquals(SettingType.String, wsfb.getType()); + setting = (Setting) wsfb.getSetting(); + assertEquals("simpleString", setting.getKey()); + assertEquals("foo", (String) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + wsfb.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + WriteableSetting wsIn = new WriteableSetting(in); + + assertEquals(SettingType.String, wsIn.getType()); + setting = (Setting) wsIn.getSetting(); + assertEquals("simpleString", setting.getKey()); + assertEquals("foo", (String) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + } + } + } + + @SuppressWarnings("unchecked") + public void testTimeValueSetting() throws IOException { + WriteableSetting ws = new WriteableSetting(settingMap.get(SettingType.TimeValue)); + assertEquals(SettingType.TimeValue, ws.getType()); + Setting setting = (Setting) ws.getSetting(); + assertEquals("timeSettingBase", setting.getKey()); + assertEquals(new TimeValue(5, TimeUnit.MILLISECONDS), (TimeValue) setting.getDefault(Settings.EMPTY)); + EnumSet props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + WriteableSetting wsfb = new WriteableSetting(settingWithFallbackMap.get(SettingType.TimeValue)); + assertEquals(SettingType.TimeValue, wsfb.getType()); + setting = (Setting) wsfb.getSetting(); + assertEquals("timeSetting", setting.getKey()); + assertEquals(new TimeValue(5, TimeUnit.MILLISECONDS), (TimeValue) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + wsfb.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + WriteableSetting wsIn = new WriteableSetting(in); + + assertEquals(SettingType.TimeValue, wsIn.getType()); + setting = (Setting) wsIn.getSetting(); + assertEquals("timeSetting", setting.getKey()); + assertEquals(new TimeValue(5, TimeUnit.MILLISECONDS), (TimeValue) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + } + } + } + + @SuppressWarnings("unchecked") + public void testByteSizeValueSetting() throws IOException { + WriteableSetting ws = new WriteableSetting(settingMap.get(SettingType.ByteSizeValue)); + assertEquals(SettingType.ByteSizeValue, ws.getType()); + Setting setting = (Setting) ws.getSetting(); + assertEquals("byteSizeSettingBase", setting.getKey()); + assertEquals(new ByteSizeValue(10, ByteSizeUnit.KB), (ByteSizeValue) setting.getDefault(Settings.EMPTY)); + EnumSet props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + WriteableSetting wsfb = new WriteableSetting(settingWithFallbackMap.get(SettingType.ByteSizeValue)); + assertEquals(SettingType.ByteSizeValue, wsfb.getType()); + setting = (Setting) wsfb.getSetting(); + assertEquals("byteSizeSetting", setting.getKey()); + assertEquals(new ByteSizeValue(10, ByteSizeUnit.KB), (ByteSizeValue) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + wsfb.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + WriteableSetting wsIn = new WriteableSetting(in); + + assertEquals(SettingType.ByteSizeValue, wsIn.getType()); + setting = (Setting) wsIn.getSetting(); + assertEquals("byteSizeSetting", setting.getKey()); + assertEquals(new ByteSizeValue(10, ByteSizeUnit.KB), (ByteSizeValue) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + } + } + } + + @SuppressWarnings("unchecked") + public void testVersionSetting() throws IOException { + WriteableSetting ws = new WriteableSetting(settingMap.get(SettingType.Version)); + assertEquals(SettingType.Version, ws.getType()); + Setting setting = (Setting) ws.getSetting(); + assertEquals("versionSettingBase", setting.getKey()); + assertEquals(Version.CURRENT, (Version) setting.getDefault(Settings.EMPTY)); + EnumSet props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + ws.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + WriteableSetting wsIn = new WriteableSetting(in); + + assertEquals(SettingType.Version, wsIn.getType()); + setting = (Setting) wsIn.getSetting(); + assertEquals("versionSettingBase", setting.getKey()); + assertEquals(Version.CURRENT, (Version) setting.getDefault(Settings.EMPTY)); + props = setting.getProperties(); + assertEquals(2, props.size()); + assertTrue(props.contains(Property.NodeScope)); + assertTrue(props.contains(Property.Dynamic)); + } + } + } + + @SuppressForbidden(reason = "The only way to test these is via reflection") + public void testExceptionHandling() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { + // abuse reflection to change default value, no way to do this with given Setting class + Setting setting = Setting.simpleString(""); + Field dv = setting.getClass().getDeclaredField("defaultValue"); + dv.setAccessible(true); + Field p = setting.getClass().getDeclaredField("parser"); + p.setAccessible(true); + + // test null default value + dv.set(setting, null); + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> new WriteableSetting(setting)); + assertTrue(iae.getMessage().contains("null default value")); + + // test default value type not in enum + Function dvfi = s -> ""; + dv.set(setting, dvfi); + Function pfi = s -> new WriteableSettingTests(); + p.set(setting, pfi); + UnsupportedOperationException uoe = expectThrows(UnsupportedOperationException.class, () -> new WriteableSetting(setting)); + assertTrue(uoe.getMessage().contains("generic type: WriteableSettingTests")); + } +} diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionResponseTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionResponseTests.java new file mode 100644 index 0000000000000..3b2993cb164c0 --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/ExtensionResponseTests.java @@ -0,0 +1,51 @@ +/* + * 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; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.test.OpenSearchTestCase; + +public class ExtensionResponseTests extends OpenSearchTestCase { + + public void testAcknowledgedResponse() throws Exception { + boolean response = true; + AcknowledgedResponse booleanResponse = new AcknowledgedResponse(response); + + assertEquals(response, booleanResponse.getStatus()); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + booleanResponse.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + booleanResponse = new AcknowledgedResponse(in); + + assertEquals(response, booleanResponse.getStatus()); + } + } + } + + public void testExtensionStringResponse() throws Exception { + String response = "This is a response"; + ExtensionStringResponse stringResponse = new ExtensionStringResponse(response); + + assertEquals(response, stringResponse.getResponse()); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + stringResponse.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + stringResponse = new ExtensionStringResponse(in); + + assertEquals(response, stringResponse.getResponse()); + } + } + } +} diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index d45e51ea2bbc8..ae4f61ffbec99 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -8,6 +8,7 @@ package org.opensearch.extensions; +import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; import static org.mockito.ArgumentMatchers.any; @@ -30,7 +31,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; -import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -43,17 +43,23 @@ import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.ClusterSettingsResponse; import org.opensearch.cluster.LocalNodeResponse; +import org.opensearch.env.EnvironmentSettingsResponse; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; 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.NamedWriteable; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.io.stream.NamedWriteableRegistry; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.network.NetworkService; +import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.WriteableSetting; +import org.opensearch.common.settings.Setting.Property; +import org.opensearch.common.settings.WriteableSetting.SettingType; +import org.opensearch.common.settings.SettingsModule; import org.opensearch.common.transport.TransportAddress; import org.opensearch.common.util.FeatureFlagTests; import org.opensearch.common.util.PageCacheRecycler; @@ -61,7 +67,7 @@ import org.opensearch.env.Environment; import org.opensearch.env.TestEnvironment; import org.opensearch.extensions.rest.RegisterRestActionsRequest; -import org.opensearch.extensions.rest.RegisterRestActionsResponse; +import org.opensearch.extensions.settings.RegisterCustomSettingsRequest; import org.opensearch.index.IndexModule; import org.opensearch.index.IndexSettings; import org.opensearch.index.analysis.AnalysisRegistry; @@ -86,6 +92,7 @@ public class ExtensionsManagerTests extends OpenSearchTestCase { private TransportService transportService; private RestController restController; + private SettingsModule settingsModule; private ClusterService clusterService; private MockNioTransport transport; private Path extensionDir; @@ -122,6 +129,8 @@ public class ExtensionsManagerTests extends OpenSearchTestCase { " hasNativeController: true" ); + private DiscoveryExtensionNode extensionNode; + @Before public void setup() throws Exception { FeatureFlagTests.enableFeature(); @@ -158,9 +167,31 @@ public void setup() throws Exception { new NoneCircuitBreakerService(), new UsageService() ); + settingsModule = new SettingsModule(Settings.EMPTY, emptyList(), emptyList(), emptySet()); clusterService = createClusterService(threadPool); extensionDir = createTempDir(); + + extensionNode = new DiscoveryExtensionNode( + "firstExtension", + "uniqueid1", + "uniqueid1", + "myIndependentPluginHost1", + "127.0.0.0", + new TransportAddress(InetAddress.getByName("127.0.0.0"), 9300), + new HashMap(), + Version.fromString("3.0.0"), + new PluginInfo( + "firstExtension", + "Fake description 1", + "0.0.7", + Version.fromString("3.0.0"), + "14", + "fakeClass1", + new ArrayList(), + false + ) + ); } @Override @@ -172,8 +203,6 @@ public void tearDown() throws Exception { } public void testDiscover() throws Exception { - Path extensionDir = createTempDir(); - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); @@ -231,14 +260,13 @@ public void testDiscover() throws Exception { } public void testNonUniqueExtensionsDiscovery() throws Exception { - Path extensionDir = createTempDir(); - + Path emptyExtensionDir = createTempDir(); List nonUniqueYmlLines = extensionsYmlLines.stream() .map(s -> s.replace("uniqueid2", "uniqueid1")) .collect(Collectors.toList()); - Files.write(extensionDir.resolve("extensions.yml"), nonUniqueYmlLines, StandardCharsets.UTF_8); + Files.write(emptyExtensionDir.resolve("extensions.yml"), nonUniqueYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + ExtensionsManager extensionsManager = new ExtensionsManager(settings, emptyExtensionDir); List expectedUninitializedExtensions = new ArrayList(); @@ -299,26 +327,24 @@ public void testNoExtensionsFile() throws Exception { } public void testEmptyExtensionsFile() throws Exception { - Path extensionDir = createTempDir(); + Path emptyExtensionDir = createTempDir(); List emptyExtensionsYmlLines = Arrays.asList(); - Files.write(extensionDir.resolve("extensions.yml"), emptyExtensionsYmlLines, StandardCharsets.UTF_8); + Files.write(emptyExtensionDir.resolve("extensions.yml"), emptyExtensionsYmlLines, StandardCharsets.UTF_8); Settings settings = Settings.builder().build(); - expectThrows(IOException.class, () -> new ExtensionsManager(settings, extensionDir)); + expectThrows(IOException.class, () -> new ExtensionsManager(settings, emptyExtensionDir)); } public void testInitialize() throws Exception { - Path extensionDir = createTempDir(); - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); transportService.start(); transportService.acceptIncomingRequests(); - extensionsManager.initializeServicesAndRestHandler(restController, transportService, clusterService); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); try (MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(ExtensionsManager.class))) { @@ -350,31 +376,45 @@ public void testInitialize() throws Exception { } public void testHandleRegisterRestActionsRequest() throws Exception { - - Path extensionDir = createTempDir(); - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - extensionsManager.initializeServicesAndRestHandler(restController, transportService, clusterService); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); String uniqueIdStr = "uniqueid1"; List actionsList = List.of("GET /foo", "PUT /bar", "POST /baz"); RegisterRestActionsRequest registerActionsRequest = new RegisterRestActionsRequest(uniqueIdStr, actionsList); TransportResponse response = extensionsManager.getRestActionsRequestHandler() .handleRegisterRestActionsRequest(registerActionsRequest); - assertEquals(RegisterRestActionsResponse.class, response.getClass()); - assertTrue(((RegisterRestActionsResponse) response).getResponse().contains(uniqueIdStr)); - assertTrue(((RegisterRestActionsResponse) response).getResponse().contains(actionsList.toString())); + assertEquals(ExtensionStringResponse.class, response.getClass()); + assertTrue(((ExtensionStringResponse) response).getResponse().contains(uniqueIdStr)); + assertTrue(((ExtensionStringResponse) response).getResponse().contains(actionsList.toString())); } - public void testHandleRegisterRestActionsRequestWithInvalidMethod() throws Exception { + public void testHandleRegisterSettingsRequest() throws Exception { + Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - Path extensionDir = createTempDir(); + ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + String uniqueIdStr = "uniqueid1"; + List> settingsList = List.of( + Setting.boolSetting("index.falseSetting", false, Property.IndexScope, Property.Dynamic), + Setting.simpleString("fooSetting", "foo", Property.NodeScope, Property.Final) + ); + RegisterCustomSettingsRequest registerCustomSettingsRequest = new RegisterCustomSettingsRequest(uniqueIdStr, settingsList); + TransportResponse response = extensionsManager.getCustomSettingsRequestHandler() + .handleRegisterCustomSettingsRequest(registerCustomSettingsRequest); + assertEquals(ExtensionStringResponse.class, response.getClass()); + assertTrue(((ExtensionStringResponse) response).getResponse().contains(uniqueIdStr)); + assertTrue(((ExtensionStringResponse) response).getResponse().contains("falseSetting")); + assertTrue(((ExtensionStringResponse) response).getResponse().contains("fooSetting")); + } + + public void testHandleRegisterRestActionsRequestWithInvalidMethod() throws Exception { ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - extensionsManager.initializeServicesAndRestHandler(restController, transportService, clusterService); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); String uniqueIdStr = "uniqueid1"; List actionsList = List.of("FOO /foo", "PUT /bar", "POST /baz"); RegisterRestActionsRequest registerActionsRequest = new RegisterRestActionsRequest(uniqueIdStr, actionsList); @@ -390,7 +430,7 @@ public void testHandleRegisterRestActionsRequestWithInvalidUri() throws Exceptio ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - extensionsManager.initializeServicesAndRestHandler(restController, transportService, clusterService); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); String uniqueIdStr = "uniqueid1"; List actionsList = List.of("GET", "PUT /bar", "POST /baz"); RegisterRestActionsRequest registerActionsRequest = new RegisterRestActionsRequest(uniqueIdStr, actionsList); @@ -404,7 +444,7 @@ public void testHandleExtensionRequest() throws Exception { ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - extensionsManager.initializeServicesAndRestHandler(restController, transportService, clusterService); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); ExtensionRequest clusterStateRequest = new ExtensionRequest(ExtensionsManager.RequestType.REQUEST_EXTENSION_CLUSTER_STATE); assertEquals(ClusterStateResponse.class, extensionsManager.handleExtensionRequest(clusterStateRequest).getClass()); @@ -415,67 +455,240 @@ public void testHandleExtensionRequest() throws Exception { assertEquals(LocalNodeResponse.class, extensionsManager.handleExtensionRequest(localNodeRequest).getClass()); ExtensionRequest exceptionRequest = new ExtensionRequest(ExtensionsManager.RequestType.GET_SETTINGS); - Exception exception = expectThrows(IllegalStateException.class, () -> extensionsManager.handleExtensionRequest(exceptionRequest)); + Exception exception = expectThrows( + IllegalArgumentException.class, + () -> extensionsManager.handleExtensionRequest(exceptionRequest) + ); assertEquals("Handler not present for the provided request", exception.getMessage()); } - public void testRegisterHandler() throws Exception { + public void testHandleActionListenerOnFailureRequest() throws Exception { + + Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - TransportService mockTransportService = spy( - new TransportService( - Settings.EMPTY, - mock(Transport.class), - null, - TransportService.NOOP_TRANSPORT_INTERCEPTOR, - x -> null, - null, - Collections.emptySet() - ) - ); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); - extensionsManager.initializeServicesAndRestHandler(restController, mockTransportService, clusterService); - verify(mockTransportService, times(5)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any()); + ExtensionActionListenerOnFailureRequest listenerFailureRequest = new ExtensionActionListenerOnFailureRequest("Test failure"); + assertEquals( + AcknowledgedResponse.class, + extensionsManager.getListenerHandler().handleExtensionActionListenerOnFailureRequest(listenerFailureRequest).getClass() + ); + assertEquals("Test failure", extensionsManager.getListener().getExceptionList().get(0).getMessage()); } - private static class Example implements NamedWriteable { - public static final String INVALID_NAME = "invalid_name"; - public static final String NAME = "example"; - private final String message; + public void testEnvironmentSettingsRequest() throws Exception { - Example(String message) { - this.message = message; - } + Path extensionDir = createTempDir(); + Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); + ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); - Example(StreamInput in) throws IOException { - this.message = in.readString(); - } + List> componentSettings = List.of( + Setting.boolSetting("falseSetting", false, Property.IndexScope, Property.NodeScope), + Setting.simpleString("fooSetting", "foo", Property.Dynamic) + ); - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeString(message); + // Test EnvironmentSettingsRequest arg constructor + EnvironmentSettingsRequest environmentSettingsRequest = new EnvironmentSettingsRequest(componentSettings); + List> requestComponentSettings = environmentSettingsRequest.getComponentSettings(); + assertEquals(componentSettings.size(), requestComponentSettings.size()); + assertTrue(requestComponentSettings.containsAll(componentSettings)); + assertTrue(componentSettings.containsAll(requestComponentSettings)); + + // Test EnvironmentSettingsRequest StreamInput constructor + try (BytesStreamOutput out = new BytesStreamOutput()) { + environmentSettingsRequest.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + environmentSettingsRequest = new EnvironmentSettingsRequest(in); + requestComponentSettings = environmentSettingsRequest.getComponentSettings(); + assertEquals(componentSettings.size(), requestComponentSettings.size()); + assertTrue(requestComponentSettings.containsAll(componentSettings)); + assertTrue(componentSettings.containsAll(requestComponentSettings)); + } } - @Override - public String getWriteableName() { - return NAME; + } + + public void testEnvironmentSettingsResponse() throws Exception { + + List> componentSettings = List.of( + Setting.boolSetting("falseSetting", false, Property.IndexScope, Property.NodeScope), + Setting.simpleString("fooSetting", "foo", Property.Dynamic) + ); + + // Test EnvironmentSettingsResponse arg constructor + EnvironmentSettingsResponse environmentSettingsResponse = new EnvironmentSettingsResponse(settings, componentSettings); + assertEquals(componentSettings.size(), environmentSettingsResponse.getComponentSettingValues().size()); + + List> responseSettings = new ArrayList<>(); + responseSettings.addAll(environmentSettingsResponse.getComponentSettingValues().keySet()); + assertTrue(responseSettings.containsAll(componentSettings)); + assertTrue(componentSettings.containsAll(responseSettings)); + + // Test EnvironmentSettingsResponse StreamInput constrcutor + try (BytesStreamOutput out = new BytesStreamOutput()) { + environmentSettingsResponse.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + + environmentSettingsResponse = new EnvironmentSettingsResponse(in); + assertEquals(componentSettings.size(), environmentSettingsResponse.getComponentSettingValues().size()); + + responseSettings = new ArrayList<>(); + responseSettings.addAll(environmentSettingsResponse.getComponentSettingValues().keySet()); + assertTrue(responseSettings.containsAll(componentSettings)); + assertTrue(componentSettings.containsAll(responseSettings)); + } } + } + + public void testHandleEnvironmentSettingsRequest() throws Exception { + + Path extensionDir = createTempDir(); + Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); + ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); - @Override - public boolean equals(Object o) { - if (o == null || getClass() != o.getClass()) { - return false; + List> componentSettings = List.of( + Setting.boolSetting("falseSetting", false, Property.Dynamic), + Setting.boolSetting("trueSetting", true, Property.Dynamic) + ); + + EnvironmentSettingsRequest environmentSettingsRequest = new EnvironmentSettingsRequest(componentSettings); + TransportResponse response = extensionsManager.getEnvironmentSettingsRequestHandler() + .handleEnvironmentSettingsRequest(environmentSettingsRequest); + + assertEquals(EnvironmentSettingsResponse.class, response.getClass()); + assertEquals(componentSettings.size(), ((EnvironmentSettingsResponse) response).getComponentSettingValues().size()); + + List> responseSettings = new ArrayList<>(); + responseSettings.addAll(((EnvironmentSettingsResponse) response).getComponentSettingValues().keySet()); + assertTrue(responseSettings.containsAll(componentSettings)); + assertTrue(componentSettings.containsAll(responseSettings)); + } + + public void testAddSettingsUpdateConsumerRequest() throws Exception { + Path extensionDir = createTempDir(); + Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); + ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + + List> componentSettings = List.of( + Setting.boolSetting("falseSetting", false, Property.IndexScope, Property.NodeScope), + Setting.simpleString("fooSetting", "foo", Property.Dynamic) + ); + + // Test AddSettingsUpdateConsumerRequest arg constructor + AddSettingsUpdateConsumerRequest addSettingsUpdateConsumerRequest = new AddSettingsUpdateConsumerRequest( + extensionNode, + componentSettings + ); + assertEquals(extensionNode, addSettingsUpdateConsumerRequest.getExtensionNode()); + assertEquals(componentSettings.size(), addSettingsUpdateConsumerRequest.getComponentSettings().size()); + + List> requestComponentSettings = new ArrayList<>(); + for (WriteableSetting writeableSetting : addSettingsUpdateConsumerRequest.getComponentSettings()) { + requestComponentSettings.add(writeableSetting.getSetting()); + } + assertTrue(requestComponentSettings.containsAll(componentSettings)); + assertTrue(componentSettings.containsAll(requestComponentSettings)); + + // Test AddSettingsUpdateConsumerRequest StreamInput constructor + try (BytesStreamOutput out = new BytesStreamOutput()) { + addSettingsUpdateConsumerRequest.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + addSettingsUpdateConsumerRequest = new AddSettingsUpdateConsumerRequest(in); + assertEquals(extensionNode, addSettingsUpdateConsumerRequest.getExtensionNode()); + assertEquals(componentSettings.size(), addSettingsUpdateConsumerRequest.getComponentSettings().size()); + + requestComponentSettings = new ArrayList<>(); + for (WriteableSetting writeableSetting : addSettingsUpdateConsumerRequest.getComponentSettings()) { + requestComponentSettings.add(writeableSetting.getSetting()); + } + assertTrue(requestComponentSettings.containsAll(componentSettings)); + assertTrue(componentSettings.containsAll(requestComponentSettings)); } - Example that = (Example) o; - return Objects.equals(message, that.message); } - @Override - public int hashCode() { - return Objects.hash(message); + } + + public void testHandleAddSettingsUpdateConsumerRequest() throws Exception { + + Path extensionDir = createTempDir(); + Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); + ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + + List> componentSettings = List.of( + Setting.boolSetting("falseSetting", false, Property.Dynamic), + Setting.boolSetting("trueSetting", true, Property.Dynamic) + ); + + AddSettingsUpdateConsumerRequest addSettingsUpdateConsumerRequest = new AddSettingsUpdateConsumerRequest( + extensionNode, + componentSettings + ); + TransportResponse response = extensionsManager.getAddSettingsUpdateConsumerRequestHandler() + .handleAddSettingsUpdateConsumerRequest(addSettingsUpdateConsumerRequest); + assertEquals(AcknowledgedResponse.class, response.getClass()); + // Should fail as component settings are not registered within cluster settings + assertEquals(false, ((AcknowledgedResponse) response).getStatus()); + } + + public void testUpdateSettingsRequest() throws Exception { + Path extensionDir = createTempDir(); + Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); + ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + + Setting componentSetting = Setting.boolSetting("falseSetting", false, Property.Dynamic); + SettingType settingType = SettingType.Boolean; + boolean data = true; + + // Test UpdateSettingRequest arg constructor + UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(settingType, componentSetting, data); + assertEquals(componentSetting, updateSettingsRequest.getComponentSetting()); + assertEquals(settingType, updateSettingsRequest.getSettingType()); + assertEquals(data, updateSettingsRequest.getData()); + + // Test UpdateSettingRequest StreamInput constructor + try (BytesStreamOutput out = new BytesStreamOutput()) { + updateSettingsRequest.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + updateSettingsRequest = new UpdateSettingsRequest(in); + assertEquals(componentSetting, updateSettingsRequest.getComponentSetting()); + assertEquals(settingType, updateSettingsRequest.getSettingType()); + assertEquals(data, updateSettingsRequest.getData()); + } } + + } + + public void testRegisterHandler() throws Exception { + + ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + + TransportService mockTransportService = spy( + new TransportService( + Settings.EMPTY, + mock(Transport.class), + null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + x -> null, + null, + Collections.emptySet() + ) + ); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, mockTransportService, clusterService, settings); + verify(mockTransportService, times(9)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any()); + } public void testOnIndexModule() throws Exception { @@ -485,7 +698,7 @@ public void testOnIndexModule() throws Exception { transportService.start(); transportService.acceptIncomingRequests(); - extensionsManager.initializeServicesAndRestHandler(restController, transportService, clusterService); + extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); Environment environment = TestEnvironment.newEnvironment(settings); AnalysisRegistry emptyAnalysisRegistry = new AnalysisRegistry( diff --git a/server/src/test/java/org/opensearch/extensions/rest/RegisterRestActionsTests.java b/server/src/test/java/org/opensearch/extensions/rest/RegisterRestActionsTests.java index a8f1739ce82f2..4929394fd18bb 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RegisterRestActionsTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RegisterRestActionsTests.java @@ -42,21 +42,4 @@ public void testRegisterRestActionsRequest() throws Exception { } } } - - public void testRegisterRestActionsResponse() throws Exception { - String response = "This is a response"; - RegisterRestActionsResponse registerRestActionsResponse = new RegisterRestActionsResponse(response); - - assertEquals(response, registerRestActionsResponse.getResponse()); - - try (BytesStreamOutput out = new BytesStreamOutput()) { - registerRestActionsResponse.writeTo(out); - out.flush(); - try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { - registerRestActionsResponse = new RegisterRestActionsResponse(in); - - assertEquals(response, registerRestActionsResponse.getResponse()); - } - } - } } diff --git a/server/src/test/java/org/opensearch/extensions/settings/RegisterCustomSettingsTests.java b/server/src/test/java/org/opensearch/extensions/settings/RegisterCustomSettingsTests.java new file mode 100644 index 0000000000000..68f32672871ad --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/settings/RegisterCustomSettingsTests.java @@ -0,0 +1,56 @@ +/* + * 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.settings; + +import java.util.List; +import java.util.concurrent.TimeUnit; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Setting.Property; +import org.opensearch.common.unit.ByteSizeUnit; +import org.opensearch.common.unit.ByteSizeValue; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.test.OpenSearchTestCase; + +public class RegisterCustomSettingsTests extends OpenSearchTestCase { + + public void testRegisterCustomSettingsRequest() throws Exception { + String uniqueIdStr = "uniqueid1"; + List> expected = List.of( + Setting.boolSetting("falseSetting", false, Property.IndexScope, Property.NodeScope), + Setting.simpleString("fooSetting", "foo", Property.Dynamic), + Setting.timeSetting("timeSetting", new TimeValue(5, TimeUnit.MILLISECONDS), Property.Dynamic), + Setting.byteSizeSetting("byteSizeSetting", new ByteSizeValue(10, ByteSizeUnit.KB), Property.Dynamic) + ); + RegisterCustomSettingsRequest registerCustomSettingsRequest = new RegisterCustomSettingsRequest(uniqueIdStr, expected); + + assertEquals(uniqueIdStr, registerCustomSettingsRequest.getUniqueId()); + List> settings = registerCustomSettingsRequest.getSettings(); + assertEquals(expected.size(), settings.size()); + assertTrue(settings.containsAll(expected)); + assertTrue(expected.containsAll(settings)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + registerCustomSettingsRequest.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + registerCustomSettingsRequest = new RegisterCustomSettingsRequest(in); + + assertEquals(uniqueIdStr, registerCustomSettingsRequest.getUniqueId()); + settings = registerCustomSettingsRequest.getSettings(); + assertEquals(expected.size(), settings.size()); + assertTrue(settings.containsAll(expected)); + assertTrue(expected.containsAll(settings)); + } + } + } +} From 1a75ced143bcb232f66e8e3bfb820d74a1c9b3a3 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Sun, 18 Dec 2022 22:36:49 +0000 Subject: [PATCH 02/10] Update CHANGELOG Signed-off-by: Ryan Bogan --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97643061cb097..335b49d965738 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add feature flag for extensions ([#5211](https://github.com/opensearch-project/OpenSearch/pull/5211)) - Added jackson dependency to server ([#5366] (https://github.com/opensearch-project/OpenSearch/pull/5366)) - Adding support to register settings dynamically ([#5495](https://github.com/opensearch-project/OpenSearch/pull/5495)) -- Added experimental support for extensions ([#5347](https://github.com/opensearch-project/OpenSearch/pull/5347)), ([#5518](https://github.com/opensearch-project/OpenSearch/pull/5518)) +- Added experimental support for extensions ([#5347](https://github.com/opensearch-project/OpenSearch/pull/5347)), ([#5518](https://github.com/opensearch-project/OpenSearch/pull/5518), ([#5597](https://github.com/opensearch-project/OpenSearch/pull/5597))) - Add CI bundle pattern to distribution download ([#5348](https://github.com/opensearch-project/OpenSearch/pull/5348)) From aab130613d8c98ca3547902948357965779bf93d Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Tue, 20 Dec 2022 21:27:23 +0000 Subject: [PATCH 03/10] Rework EnvironmentSettings API Signed-off-by: Ryan Bogan --- .../env/EnvironmentSettingsResponse.java | 45 ++------ .../EnvironmentSettingsRequest.java | 79 -------------- .../EnvironmentSettingsRequestHandler.java | 43 -------- .../extensions/ExtensionsManager.java | 28 ++--- .../extensions/UpdateSettingsRequest.java | 2 +- .../extensions/ExtensionsManagerTests.java | 102 +++++++----------- 6 files changed, 65 insertions(+), 234 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequest.java delete mode 100644 server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequestHandler.java diff --git a/server/src/main/java/org/opensearch/env/EnvironmentSettingsResponse.java b/server/src/main/java/org/opensearch/env/EnvironmentSettingsResponse.java index 0f541ed8ce51b..3fbe636803e66 100644 --- a/server/src/main/java/org/opensearch/env/EnvironmentSettingsResponse.java +++ b/server/src/main/java/org/opensearch/env/EnvironmentSettingsResponse.java @@ -10,16 +10,10 @@ import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.transport.TransportResponse; -import org.opensearch.common.settings.WriteableSetting; import java.io.IOException; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; import java.util.Objects; /** @@ -28,47 +22,28 @@ * @opensearch.internal */ public class EnvironmentSettingsResponse extends TransportResponse { - private Map, Object> componentSettingValues; + private final Settings environmentSettings; - public EnvironmentSettingsResponse(Settings environmentSettings, List> componentSettings) { - Map, Object> componentSettingValues = new HashMap<>(); - for (Setting componentSetting : componentSettings) { - - // Retrieve component setting value from enviornment settings, or default value if it does not exist - Object componentSettingValue = componentSetting.get(environmentSettings); - componentSettingValues.put(componentSetting, componentSettingValue); - } - this.componentSettingValues = componentSettingValues; + public EnvironmentSettingsResponse(Settings environmentSettings) { + this.environmentSettings = environmentSettings; } public EnvironmentSettingsResponse(StreamInput in) throws IOException { - super(in); - Map, Object> componentSettingValues = new HashMap<>(); - int componentSettingValuesCount = in.readVInt(); - for (int i = 0; i < componentSettingValuesCount; i++) { - Setting componentSetting = new WriteableSetting(in).getSetting(); - Object componentSettingValue = in.readGenericValue(); - componentSettingValues.put(componentSetting, componentSettingValue); - } - this.componentSettingValues = componentSettingValues; + this.environmentSettings = Settings.readSettingsFromStream(in); } @Override public void writeTo(StreamOutput out) throws IOException { - out.writeVInt(componentSettingValues.size()); - for (Map.Entry, Object> entry : componentSettingValues.entrySet()) { - new WriteableSetting(entry.getKey()).writeTo(out); - out.writeGenericValue(entry.getValue()); - } + Settings.writeSettingsToStream(this.environmentSettings, out); } - public Map, Object> getComponentSettingValues() { - return Collections.unmodifiableMap(this.componentSettingValues); + public Settings getEnvironmentSettings() { + return environmentSettings; } @Override public String toString() { - return "EnvironmentSettingsResponse{componentSettingValues=" + componentSettingValues.toString() + '}'; + return "EnvironmentSettingsResponse{environmentSettings=" + environmentSettings.toString() + '}'; } @Override @@ -76,11 +51,11 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; EnvironmentSettingsResponse that = (EnvironmentSettingsResponse) o; - return Objects.equals(componentSettingValues, that.componentSettingValues); + return Objects.equals(environmentSettings, that.environmentSettings); } @Override public int hashCode() { - return Objects.hash(componentSettingValues); + return Objects.hash(environmentSettings); } } diff --git a/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequest.java b/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequest.java deleted file mode 100644 index ab470087f8ec9..0000000000000 --- a/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequest.java +++ /dev/null @@ -1,79 +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. - */ - -package org.opensearch.extensions; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.transport.TransportRequest; -import org.opensearch.common.settings.Setting; -import org.opensearch.common.settings.WriteableSetting; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; - -/** - * Environment Settings Request for Extensibility - * - * @opensearch.internal - */ -public class EnvironmentSettingsRequest extends TransportRequest { - private static final Logger logger = LogManager.getLogger(EnvironmentSettingsRequest.class); - private List> componentSettings; - - public EnvironmentSettingsRequest(List> componentSettings) { - this.componentSettings = new ArrayList<>(componentSettings); - } - - public EnvironmentSettingsRequest(StreamInput in) throws IOException { - super(in); - int componentSettingsCount = in.readVInt(); - List> componentSettings = new ArrayList<>(componentSettingsCount); - for (int i = 0; i < componentSettingsCount; i++) { - WriteableSetting writeableSetting = new WriteableSetting(in); - componentSettings.add(writeableSetting.getSetting()); - } - this.componentSettings = componentSettings; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeVInt(this.componentSettings.size()); - for (Setting componentSetting : componentSettings) { - new WriteableSetting(componentSetting).writeTo(out); - } - } - - public List> getComponentSettings() { - return new ArrayList<>(componentSettings); - } - - @Override - public String toString() { - return "EnvironmentSettingsRequest{componentSettings=" + componentSettings.toString() + "}"; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null || getClass() != obj.getClass()) return false; - EnvironmentSettingsRequest that = (EnvironmentSettingsRequest) obj; - return Objects.equals(componentSettings, that.componentSettings); - } - - @Override - public int hashCode() { - return Objects.hash(componentSettings); - } - -} diff --git a/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequestHandler.java b/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequestHandler.java deleted file mode 100644 index 723eb482d7c44..0000000000000 --- a/server/src/main/java/org/opensearch/extensions/EnvironmentSettingsRequestHandler.java +++ /dev/null @@ -1,43 +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. - */ - -package org.opensearch.extensions; - -import org.opensearch.common.settings.Settings; -import org.opensearch.env.EnvironmentSettingsResponse; -import org.opensearch.transport.TransportResponse; - -/** - * Handles requests to retrieve environment settings. - * - * @opensearch.internal - */ -public class EnvironmentSettingsRequestHandler { - - private final Settings initialEnvironmentSettings; - - /** - * Instantiates a new Environment Settings Request Handler with the environment settings - * - * @param initialEnvironmentSettings the finalized view of environment {@link Settings} - */ - public EnvironmentSettingsRequestHandler(Settings initialEnvironmentSettings) { - this.initialEnvironmentSettings = initialEnvironmentSettings; - } - - /** - * Handles a {@link EnvironmentSettingsRequest}. - * - * @param environmentSettingsRequest The request to handle. - * @return A {@link EnvironmentSettingsResponse} - * @throws Exception if the request is not handled properly. - */ - TransportResponse handleEnvironmentSettingsRequest(EnvironmentSettingsRequest environmentSettingsRequest) throws Exception { - return new EnvironmentSettingsResponse(this.initialEnvironmentSettings, environmentSettingsRequest.getComponentSettings()); - } -} diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index cda8b40ef1b2d..334ded61cae7c 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -56,6 +56,7 @@ import org.opensearch.transport.TransportResponse; import org.opensearch.transport.TransportResponseHandler; import org.opensearch.transport.TransportService; +import org.opensearch.env.EnvironmentSettingsResponse; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; @@ -96,6 +97,7 @@ public static enum RequestType { REQUEST_EXTENSION_ACTION_LISTENER_ON_FAILURE, REQUEST_EXTENSION_REGISTER_REST_ACTIONS, REQUEST_EXTENSION_REGISTER_SETTINGS, + REQUEST_EXTENSION_ENVIRONMENT_SETTINGS, CREATE_COMPONENT, ON_INDEX_MODULE, GET_SETTINGS @@ -120,7 +122,7 @@ public static enum OpenSearchRequestType { private ClusterService clusterService; private ExtensionActionListener listener; private ExtensionActionListenerHandler listenerHandler; - private EnvironmentSettingsRequestHandler environmentSettingsRequestHandler; + private Settings environmentSettings; private AddSettingsUpdateConsumerRequestHandler addSettingsUpdateConsumerRequestHandler; public ExtensionsManager() { @@ -172,7 +174,7 @@ public void initializeServicesAndRestHandler( this.customSettingsRequestHandler = new CustomSettingsRequestHandler(settingsModule); this.transportService = transportService; this.clusterService = clusterService; - this.environmentSettingsRequestHandler = new EnvironmentSettingsRequestHandler(initialEnvironmentSettings); + this.environmentSettings = initialEnvironmentSettings; this.addSettingsUpdateConsumerRequestHandler = new AddSettingsUpdateConsumerRequestHandler( clusterService, transportService, @@ -235,8 +237,8 @@ private void registerRequestHandler() { ThreadPool.Names.GENERIC, false, false, - EnvironmentSettingsRequest::new, - ((request, channel, task) -> channel.sendResponse(environmentSettingsRequestHandler.handleEnvironmentSettingsRequest(request))) + ExtensionRequest::new, + ((request, channel, task) -> channel.sendResponse(handleExtensionRequest(request))) ); transportService.registerRequestHandler( REQUEST_EXTENSION_ADD_SETTINGS_UPDATE_CONSUMER, @@ -417,6 +419,8 @@ TransportResponse handleExtensionRequest(ExtensionRequest extensionRequest) thro return new LocalNodeResponse(clusterService); case REQUEST_EXTENSION_CLUSTER_SETTINGS: return new ClusterSettingsResponse(clusterService); + case REQUEST_EXTENSION_ENVIRONMENT_SETTINGS: + return new EnvironmentSettingsResponse(this.environmentSettings); default: throw new IllegalArgumentException("Handler not present for the provided request"); } @@ -661,14 +665,6 @@ public static String getRequestExtensionUpdateSettings() { return REQUEST_EXTENSION_UPDATE_SETTINGS; } - public EnvironmentSettingsRequestHandler getEnvironmentSettingsRequestHandler() { - return environmentSettingsRequestHandler; - } - - public void setEnvironmentSettingsRequestHandler(EnvironmentSettingsRequestHandler environmentSettingsRequestHandler) { - this.environmentSettingsRequestHandler = environmentSettingsRequestHandler; - } - public AddSettingsUpdateConsumerRequestHandler getAddSettingsUpdateConsumerRequestHandler() { return addSettingsUpdateConsumerRequestHandler; } @@ -691,4 +687,12 @@ public void setListenerHandler(ExtensionActionListenerHandler listenerHandler) { this.listenerHandler = listenerHandler; } + public Settings getEnvironmentSettings() { + return environmentSettings; + } + + public void setEnvironmentSettings(Settings environmentSettings) { + this.environmentSettings = environmentSettings; + } + } diff --git a/server/src/main/java/org/opensearch/extensions/UpdateSettingsRequest.java b/server/src/main/java/org/opensearch/extensions/UpdateSettingsRequest.java index 3191f189ac18b..6ed7b9a5a6d36 100644 --- a/server/src/main/java/org/opensearch/extensions/UpdateSettingsRequest.java +++ b/server/src/main/java/org/opensearch/extensions/UpdateSettingsRequest.java @@ -25,7 +25,7 @@ * @opensearch.internal */ public class UpdateSettingsRequest extends TransportRequest { - private static final Logger logger = LogManager.getLogger(EnvironmentSettingsRequest.class); + private static final Logger logger = LogManager.getLogger(UpdateSettingsRequest.class); private WriteableSetting.SettingType settingType; private Setting componentSetting; diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index ae4f61ffbec99..6ffa683cf6349 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -454,6 +454,11 @@ public void testHandleExtensionRequest() throws Exception { ExtensionRequest localNodeRequest = new ExtensionRequest(ExtensionsManager.RequestType.REQUEST_EXTENSION_LOCAL_NODE); assertEquals(LocalNodeResponse.class, extensionsManager.handleExtensionRequest(localNodeRequest).getClass()); + ExtensionRequest environmentSettingsRequest = new ExtensionRequest( + ExtensionsManager.RequestType.REQUEST_EXTENSION_ENVIRONMENT_SETTINGS + ); + assertEquals(EnvironmentSettingsResponse.class, extensionsManager.handleExtensionRequest(environmentSettingsRequest).getClass()); + ExtensionRequest exceptionRequest = new ExtensionRequest(ExtensionsManager.RequestType.GET_SETTINGS); Exception exception = expectThrows( IllegalArgumentException.class, @@ -479,96 +484,65 @@ public void testHandleActionListenerOnFailureRequest() throws Exception { assertEquals("Test failure", extensionsManager.getListener().getExceptionList().get(0).getMessage()); } - public void testEnvironmentSettingsRequest() throws Exception { - - Path extensionDir = createTempDir(); - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); - - List> componentSettings = List.of( - Setting.boolSetting("falseSetting", false, Property.IndexScope, Property.NodeScope), - Setting.simpleString("fooSetting", "foo", Property.Dynamic) - ); + public void testEnvironmentSettingsResponse() throws Exception { - // Test EnvironmentSettingsRequest arg constructor - EnvironmentSettingsRequest environmentSettingsRequest = new EnvironmentSettingsRequest(componentSettings); - List> requestComponentSettings = environmentSettingsRequest.getComponentSettings(); - assertEquals(componentSettings.size(), requestComponentSettings.size()); - assertTrue(requestComponentSettings.containsAll(componentSettings)); - assertTrue(componentSettings.containsAll(requestComponentSettings)); + // Test EnvironmentSettingsResponse arg constructor + EnvironmentSettingsResponse environmentSettingsResponse = new EnvironmentSettingsResponse(settings); + assertEquals(settings, environmentSettingsResponse.getEnvironmentSettings()); - // Test EnvironmentSettingsRequest StreamInput constructor + // Test EnvironmentSettingsResponse StreamInput constructor try (BytesStreamOutput out = new BytesStreamOutput()) { - environmentSettingsRequest.writeTo(out); + environmentSettingsResponse.writeTo(out); out.flush(); try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { - environmentSettingsRequest = new EnvironmentSettingsRequest(in); - requestComponentSettings = environmentSettingsRequest.getComponentSettings(); - assertEquals(componentSettings.size(), requestComponentSettings.size()); - assertTrue(requestComponentSettings.containsAll(componentSettings)); - assertTrue(componentSettings.containsAll(requestComponentSettings)); + environmentSettingsResponse = new EnvironmentSettingsResponse(in); + assertEquals(settings, environmentSettingsResponse.getEnvironmentSettings()); } } } - public void testEnvironmentSettingsResponse() throws Exception { + public void testEnvironmentSettingsRegisteredValue() throws Exception { + // Create setting with value false + Setting boolSetting = Setting.boolSetting("boolSetting", false, Property.Dynamic); - List> componentSettings = List.of( - Setting.boolSetting("falseSetting", false, Property.IndexScope, Property.NodeScope), - Setting.simpleString("fooSetting", "foo", Property.Dynamic) - ); - - // Test EnvironmentSettingsResponse arg constructor - EnvironmentSettingsResponse environmentSettingsResponse = new EnvironmentSettingsResponse(settings, componentSettings); - assertEquals(componentSettings.size(), environmentSettingsResponse.getComponentSettingValues().size()); - - List> responseSettings = new ArrayList<>(); - responseSettings.addAll(environmentSettingsResponse.getComponentSettingValues().keySet()); - assertTrue(responseSettings.containsAll(componentSettings)); - assertTrue(componentSettings.containsAll(responseSettings)); + // Create Settings with registered bool setting with value true + Settings environmentSettings = Settings.builder().put("boolSetting", "true").build(); - // Test EnvironmentSettingsResponse StreamInput constrcutor + EnvironmentSettingsResponse environmentSettingsResponse = new EnvironmentSettingsResponse(environmentSettings); try (BytesStreamOutput out = new BytesStreamOutput()) { environmentSettingsResponse.writeTo(out); out.flush(); try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { environmentSettingsResponse = new EnvironmentSettingsResponse(in); - assertEquals(componentSettings.size(), environmentSettingsResponse.getComponentSettingValues().size()); + assertEquals(environmentSettings, environmentSettingsResponse.getEnvironmentSettings()); - responseSettings = new ArrayList<>(); - responseSettings.addAll(environmentSettingsResponse.getComponentSettingValues().keySet()); - assertTrue(responseSettings.containsAll(componentSettings)); - assertTrue(componentSettings.containsAll(responseSettings)); + // bool setting is registered in Settings object, thus the expected return value is the registered setting value + assertEquals(true, boolSetting.get(environmentSettingsResponse.getEnvironmentSettings())); } } } - public void testHandleEnvironmentSettingsRequest() throws Exception { - - Path extensionDir = createTempDir(); - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); - - List> componentSettings = List.of( - Setting.boolSetting("falseSetting", false, Property.Dynamic), - Setting.boolSetting("trueSetting", true, Property.Dynamic) - ); + public void testEnvironmentSettingsDefaultValue() throws Exception { + // Create setting with value false + Setting boolSetting = Setting.boolSetting("boolSetting", false, Property.Dynamic); - EnvironmentSettingsRequest environmentSettingsRequest = new EnvironmentSettingsRequest(componentSettings); - TransportResponse response = extensionsManager.getEnvironmentSettingsRequestHandler() - .handleEnvironmentSettingsRequest(environmentSettingsRequest); + // Create settings object without registered bool setting + Settings environmentSettings = Settings.builder().put("testSetting", "true").build(); - assertEquals(EnvironmentSettingsResponse.class, response.getClass()); - assertEquals(componentSettings.size(), ((EnvironmentSettingsResponse) response).getComponentSettingValues().size()); + EnvironmentSettingsResponse environmentSettingsResponse = new EnvironmentSettingsResponse(environmentSettings); + try (BytesStreamOutput out = new BytesStreamOutput()) { + environmentSettingsResponse.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { - List> responseSettings = new ArrayList<>(); - responseSettings.addAll(((EnvironmentSettingsResponse) response).getComponentSettingValues().keySet()); - assertTrue(responseSettings.containsAll(componentSettings)); - assertTrue(componentSettings.containsAll(responseSettings)); + environmentSettingsResponse = new EnvironmentSettingsResponse(in); + assertEquals(environmentSettings, environmentSettingsResponse.getEnvironmentSettings()); + // bool setting is not registered in Settings object, thus the expected return value is the default setting value + assertEquals(false, boolSetting.get(environmentSettingsResponse.getEnvironmentSettings())); + } + } } public void testAddSettingsUpdateConsumerRequest() throws Exception { From b3497621852527ad64f3eded0fedf788dbe62a5b Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Tue, 20 Dec 2022 23:56:13 +0000 Subject: [PATCH 04/10] Addressed PR Comments Signed-off-by: Ryan Bogan --- .../opensearch/common/settings/WriteableSetting.java | 8 ++------ .../extensions/AddSettingsUpdateConsumerRequest.java | 11 +++-------- .../AddSettingsUpdateConsumerRequestHandler.java | 2 +- .../extensions/ExtensionActionListener.java | 12 +++++------- .../ExtensionActionListenerOnFailureRequest.java | 3 --- .../settings/RegisterCustomSettingsRequest.java | 5 ++--- .../common/settings/WriteableSettingTests.java | 9 ++------- 7 files changed, 15 insertions(+), 35 deletions(-) diff --git a/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java b/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java index 0909224892c7a..3cf28ef38f245 100644 --- a/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java +++ b/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java @@ -101,16 +101,12 @@ public WriteableSetting(StreamInput in) throws IOException { * @return The corresponding {@link SettingType} for the default value. */ private static SettingType getGenericTypeFromDefault(Setting setting) { - String typeStr = null; + String typeStr = setting.getDefault(Settings.EMPTY).getClass().getSimpleName(); try { - // This throws NPE on null default - typeStr = setting.getDefault(Settings.EMPTY).getClass().getSimpleName(); // This throws IAE if not in enum return SettingType.valueOf(typeStr); - } catch (NullPointerException e) { - throw new IllegalArgumentException("Unable to determine the generic type of this setting with a null default value."); } catch (IllegalArgumentException e) { - throw new UnsupportedOperationException( + throw new IllegalArgumentException( "This class is not yet set up to handle the generic type: " + typeStr + ". Supported types are " diff --git a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequest.java b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequest.java index 687e3a07e3108..0ecf841e49b9e 100644 --- a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequest.java +++ b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequest.java @@ -44,11 +44,10 @@ public AddSettingsUpdateConsumerRequest(StreamInput in) throws IOException { // Read in component setting list int componentSettingsCount = in.readVInt(); - List componentSettings = new ArrayList<>(componentSettingsCount); + this.componentSettings = new ArrayList<>(componentSettingsCount); for (int i = 0; i < componentSettingsCount; i++) { - componentSettings.add(new WriteableSetting(in)); + this.componentSettings.add(new WriteableSetting(in)); } - this.componentSettings = componentSettings; } @Override @@ -75,11 +74,7 @@ public DiscoveryExtensionNode getExtensionNode() { @Override public String toString() { - return "AddSettingsUpdateConsumerRequest{extensionNode=" - + this.extensionNode.toString() - + ", componentSettings=" - + this.componentSettings.toString() - + "}"; + return "AddSettingsUpdateConsumerRequest{extensionNode=" + this.extensionNode.toString() + "}"; } @Override diff --git a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java index 791482aad0432..cb05a4210b483 100644 --- a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java +++ b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java @@ -71,7 +71,7 @@ TransportResponse handleAddSettingsUpdateConsumerRequest(AddSettingsUpdateConsum // Register setting update consumer with callback method to extension clusterService.getClusterSettings().addSettingsUpdateConsumer(setting, (data) -> { - logger.info("Sending extension request type: " + updateSettingsRequestType); + logger.debug("Sending extension request type: " + updateSettingsRequestType); UpdateSettingsResponseHandler updateSettingsResponseHandler = new UpdateSettingsResponseHandler(); transportService.sendRequest( extensionNode, diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java b/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java index 53d7e887e4814..42cab8f0cdd66 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java @@ -9,6 +9,8 @@ package org.opensearch.extensions; import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -23,7 +25,7 @@ public class ExtensionActionListener implements ActionListener { private static final Logger logger = LogManager.getLogger(ExtensionActionListener.class); - private ArrayList exceptionList; + private List exceptionList; public ExtensionActionListener() { exceptionList = new ArrayList(); @@ -40,11 +42,7 @@ public void onFailure(Exception e) { logger.error(e.getMessage()); } - public static Logger getLogger() { - return logger; - } - - public ArrayList getExceptionList() { - return exceptionList; + public List getExceptionList() { + return Collections.unmodifiableList(exceptionList); } } diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java b/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java index 22a12433f06d3..ff53d23cddb72 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java @@ -8,8 +8,6 @@ package org.opensearch.extensions; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.transport.TransportRequest; @@ -23,7 +21,6 @@ * @opensearch.internal */ public class ExtensionActionListenerOnFailureRequest extends TransportRequest { - private static final Logger logger = LogManager.getLogger(ExtensionRequest.class); private String failureExceptionMessage; /** diff --git a/server/src/main/java/org/opensearch/extensions/settings/RegisterCustomSettingsRequest.java b/server/src/main/java/org/opensearch/extensions/settings/RegisterCustomSettingsRequest.java index b8217972767f9..1f7e23544ebdf 100644 --- a/server/src/main/java/org/opensearch/extensions/settings/RegisterCustomSettingsRequest.java +++ b/server/src/main/java/org/opensearch/extensions/settings/RegisterCustomSettingsRequest.java @@ -37,12 +37,11 @@ public RegisterCustomSettingsRequest(StreamInput in) throws IOException { super(in); this.uniqueId = in.readString(); int size = in.readVInt(); - List> settingsList = new ArrayList<>(size); + this.settings = new ArrayList<>(size); for (int i = 0; i < size; i++) { WriteableSetting ws = new WriteableSetting(in); - settingsList.add(ws.getSetting()); + this.settings.add(ws.getSetting()); } - this.settings = settingsList; } @Override diff --git a/server/src/test/java/org/opensearch/common/settings/WriteableSettingTests.java b/server/src/test/java/org/opensearch/common/settings/WriteableSettingTests.java index 98eb81ef23957..5e34f68539798 100644 --- a/server/src/test/java/org/opensearch/common/settings/WriteableSettingTests.java +++ b/server/src/test/java/org/opensearch/common/settings/WriteableSettingTests.java @@ -471,17 +471,12 @@ public void testExceptionHandling() throws NoSuchFieldException, SecurityExcepti Field p = setting.getClass().getDeclaredField("parser"); p.setAccessible(true); - // test null default value - dv.set(setting, null); - IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> new WriteableSetting(setting)); - assertTrue(iae.getMessage().contains("null default value")); - // test default value type not in enum Function dvfi = s -> ""; dv.set(setting, dvfi); Function pfi = s -> new WriteableSettingTests(); p.set(setting, pfi); - UnsupportedOperationException uoe = expectThrows(UnsupportedOperationException.class, () -> new WriteableSetting(setting)); - assertTrue(uoe.getMessage().contains("generic type: WriteableSettingTests")); + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> new WriteableSetting(setting)); + assertTrue(iae.getMessage().contains("generic type: WriteableSettingTests")); } } From d80cfd49641f684fa3fb05723254486a663d4d9f Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Wed, 21 Dec 2022 23:59:48 +0000 Subject: [PATCH 05/10] Removed ExtensionActionListener and changed exception types Signed-off-by: Ryan Bogan --- .../common/settings/WriteableSetting.java | 6 +- .../extensions/ExtensionActionListener.java | 48 ------------- .../ExtensionActionListenerHandler.java | 43 ------------ ...tensionActionListenerOnFailureRequest.java | 69 ------------------- .../extensions/ExtensionsManager.java | 34 --------- .../extensions/ExtensionsManagerTests.java | 19 +---- 6 files changed, 4 insertions(+), 215 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java delete mode 100644 server/src/main/java/org/opensearch/extensions/ExtensionActionListenerHandler.java delete mode 100644 server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java diff --git a/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java b/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java index 3cf28ef38f245..2dd0d85ab0cd7 100644 --- a/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java +++ b/server/src/main/java/org/opensearch/common/settings/WriteableSetting.java @@ -179,7 +179,7 @@ private Setting createSetting( return Setting.versionSetting(key, (Version) defaultValue, propertyArray); default: // This Should Never Happen (TM) - throw new UnsupportedOperationException("A SettingType has been added to the enum and not handled here."); + throw new IllegalArgumentException("A SettingType has been added to the enum and not handled here."); } } @@ -236,7 +236,7 @@ private void writeDefaultValue(StreamOutput out, Object defaultValue) throws IOE break; default: // This Should Never Happen (TM) - throw new UnsupportedOperationException("A SettingType has been added to the enum and not handled here."); + throw new IllegalArgumentException("A SettingType has been added to the enum and not handled here."); } } @@ -264,7 +264,7 @@ private Object readDefaultValue(StreamInput in) throws IOException { return Version.readVersion(in); default: // This Should Never Happen (TM) - throw new UnsupportedOperationException("A SettingType has been added to the enum and not handled here."); + throw new IllegalArgumentException("A SettingType has been added to the enum and not handled here."); } } diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java b/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java deleted file mode 100644 index 42cab8f0cdd66..0000000000000 --- a/server/src/main/java/org/opensearch/extensions/ExtensionActionListener.java +++ /dev/null @@ -1,48 +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. - */ - -package org.opensearch.extensions; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.action.ActionListener; -import org.opensearch.action.admin.indices.analyze.AnalyzeAction.Response; - -/** - * ActionListener for ExtensionsManager - * - * @opensearch.internal - */ -public class ExtensionActionListener implements ActionListener { - - private static final Logger logger = LogManager.getLogger(ExtensionActionListener.class); - private List exceptionList; - - public ExtensionActionListener() { - exceptionList = new ArrayList(); - } - - @Override - public void onResponse(Response response) { - logger.info("response {}", response); - } - - @Override - public void onFailure(Exception e) { - exceptionList.add(e); - logger.error(e.getMessage()); - } - - public List getExceptionList() { - return Collections.unmodifiableList(exceptionList); - } -} diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerHandler.java b/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerHandler.java deleted file mode 100644 index ceba1e1f65000..0000000000000 --- a/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerHandler.java +++ /dev/null @@ -1,43 +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. - */ -package org.opensearch.extensions; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.OpenSearchException; - -/** - * Handles ActionListener requests from extensions - * - * @opensearch.internal - */ -public class ExtensionActionListenerHandler { - - private static final Logger logger = LogManager.getLogger(ExtensionActionListener.class); - private ExtensionActionListener listener; - - public ExtensionActionListenerHandler(ExtensionActionListener listener) { - this.listener = listener; - } - - /** - * Handles a {@link ExtensionActionListenerOnFailureRequest}. - * - * @param request The request to handle. - * @return A {@link AcknowledgedResponse} indicating success or failure. - */ - public AcknowledgedResponse handleExtensionActionListenerOnFailureRequest(ExtensionActionListenerOnFailureRequest request) { - try { - listener.onFailure(new OpenSearchException(request.getFailureException())); - return new AcknowledgedResponse(true); - } catch (Exception e) { - logger.error(e.getMessage()); - return new AcknowledgedResponse(false); - } - } -} diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java b/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java deleted file mode 100644 index ff53d23cddb72..0000000000000 --- a/server/src/main/java/org/opensearch/extensions/ExtensionActionListenerOnFailureRequest.java +++ /dev/null @@ -1,69 +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. - */ - -package org.opensearch.extensions; - -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.transport.TransportRequest; - -import java.io.IOException; -import java.util.Objects; - -/** - * ClusterService Request for Action Listener onFailure - * - * @opensearch.internal - */ -public class ExtensionActionListenerOnFailureRequest extends TransportRequest { - private String failureExceptionMessage; - - /** - * Instantiates a request for ActionListener onFailure from an extension - * - * @param failureExceptionMessage A String that contains both the Exception type and message - */ - public ExtensionActionListenerOnFailureRequest(String failureExceptionMessage) { - super(); - this.failureExceptionMessage = failureExceptionMessage; - } - - public ExtensionActionListenerOnFailureRequest(StreamInput in) throws IOException { - super(in); - this.failureExceptionMessage = in.readString(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeString(failureExceptionMessage); - } - - public String toString() { - return "ExtensionActionListenerOnFailureRequest{" + "failureExceptionMessage= " + failureExceptionMessage + " }"; - } - - @Override - public boolean equals(Object o) { - - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ExtensionActionListenerOnFailureRequest that = (ExtensionActionListenerOnFailureRequest) o; - return Objects.equals(failureExceptionMessage, that.failureExceptionMessage); - } - - @Override - public int hashCode() { - return Objects.hash(failureExceptionMessage); - } - - public String getFailureException() { - return failureExceptionMessage; - } - -} diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index 334ded61cae7c..e42b9d1e755a5 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -80,7 +80,6 @@ public class ExtensionsManager { public static final String REQUEST_EXTENSION_REGISTER_REST_ACTIONS = "internal:discovery/registerrestactions"; public static final String REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS = "internal:discovery/registertransportactions"; public static final String REQUEST_OPENSEARCH_PARSE_NAMED_WRITEABLE = "internal:discovery/parsenamedwriteable"; - public static final String REQUEST_EXTENSION_ACTION_LISTENER_ON_FAILURE = "internal:extensions/actionlisteneronfailure"; public static final String REQUEST_REST_EXECUTE_ON_EXTENSION_ACTION = "internal:extensions/restexecuteonextensiontaction"; private static final Logger logger = LogManager.getLogger(ExtensionsManager.class); @@ -94,7 +93,6 @@ public static enum RequestType { REQUEST_EXTENSION_CLUSTER_STATE, REQUEST_EXTENSION_LOCAL_NODE, REQUEST_EXTENSION_CLUSTER_SETTINGS, - REQUEST_EXTENSION_ACTION_LISTENER_ON_FAILURE, REQUEST_EXTENSION_REGISTER_REST_ACTIONS, REQUEST_EXTENSION_REGISTER_SETTINGS, REQUEST_EXTENSION_ENVIRONMENT_SETTINGS, @@ -120,8 +118,6 @@ public static enum OpenSearchRequestType { private CustomSettingsRequestHandler customSettingsRequestHandler; private TransportService transportService; private ClusterService clusterService; - private ExtensionActionListener listener; - private ExtensionActionListenerHandler listenerHandler; private Settings environmentSettings; private AddSettingsUpdateConsumerRequestHandler addSettingsUpdateConsumerRequestHandler; @@ -143,7 +139,6 @@ public ExtensionsManager(Settings settings, Path extensionsPath) throws IOExcept this.extensions = new ArrayList(); this.extensionIdMap = new HashMap(); this.clusterService = null; - this.listener = new ExtensionActionListener(); /* * Now Discover extensions @@ -170,7 +165,6 @@ public void initializeServicesAndRestHandler( Settings initialEnvironmentSettings ) { this.restActionsRequestHandler = new RestActionsRequestHandler(restController, extensionIdMap, transportService); - this.listenerHandler = new ExtensionActionListenerHandler(listener); this.customSettingsRequestHandler = new CustomSettingsRequestHandler(settingsModule); this.transportService = transportService; this.clusterService = clusterService; @@ -224,14 +218,6 @@ private void registerRequestHandler() { ExtensionRequest::new, ((request, channel, task) -> channel.sendResponse(handleExtensionRequest(request))) ); - transportService.registerRequestHandler( - REQUEST_EXTENSION_ACTION_LISTENER_ON_FAILURE, - ThreadPool.Names.GENERIC, - false, - false, - ExtensionActionListenerOnFailureRequest::new, - ((request, channel, task) -> channel.sendResponse(listenerHandler.handleExtensionActionListenerOnFailureRequest(request))) - ); transportService.registerRequestHandler( REQUEST_EXTENSION_ENVIRONMENT_SETTINGS, ThreadPool.Names.GENERIC, @@ -633,14 +619,6 @@ public static String getRequestExtensionRegisterTransportActions() { return REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS; } - public static String getRequestExtensionActionListenerOnFailure() { - return REQUEST_EXTENSION_ACTION_LISTENER_ON_FAILURE; - } - - public ExtensionActionListener getListener() { - return listener; - } - public static String getRequestExtensionRegisterCustomSettings() { return REQUEST_EXTENSION_REGISTER_CUSTOM_SETTINGS; } @@ -675,18 +653,6 @@ public void setAddSettingsUpdateConsumerRequestHandler( this.addSettingsUpdateConsumerRequestHandler = addSettingsUpdateConsumerRequestHandler; } - public void setListener(ExtensionActionListener listener) { - this.listener = listener; - } - - public ExtensionActionListenerHandler getListenerHandler() { - return listenerHandler; - } - - public void setListenerHandler(ExtensionActionListenerHandler listenerHandler) { - this.listenerHandler = listenerHandler; - } - public Settings getEnvironmentSettings() { return environmentSettings; } diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index 6ffa683cf6349..5847276d6adab 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -467,23 +467,6 @@ public void testHandleExtensionRequest() throws Exception { assertEquals("Handler not present for the provided request", exception.getMessage()); } - public void testHandleActionListenerOnFailureRequest() throws Exception { - - Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - - ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); - - ExtensionActionListenerOnFailureRequest listenerFailureRequest = new ExtensionActionListenerOnFailureRequest("Test failure"); - - assertEquals( - AcknowledgedResponse.class, - extensionsManager.getListenerHandler().handleExtensionActionListenerOnFailureRequest(listenerFailureRequest).getClass() - ); - assertEquals("Test failure", extensionsManager.getListener().getExceptionList().get(0).getMessage()); - } - public void testEnvironmentSettingsResponse() throws Exception { // Test EnvironmentSettingsResponse arg constructor @@ -661,7 +644,7 @@ public void testRegisterHandler() throws Exception { ) ); extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, mockTransportService, clusterService, settings); - verify(mockTransportService, times(9)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any()); + verify(mockTransportService, times(8)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any()); } From 0e12953b69b06661447b12f83955ff81d9e257e1 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Thu, 22 Dec 2022 08:01:30 +0000 Subject: [PATCH 06/10] Added TransportActions support and removed LocalNodeResponse for extensions Signed-off-by: Ryan Bogan --- .../org/opensearch/action/ActionModule.java | 7 + .../opensearch/cluster/LocalNodeResponse.java | 60 ------ .../extensions/DiscoveryExtensionNode.java | 21 +- .../extensions/ExtensionDependency.java | 89 ++++++++ .../extensions/ExtensionStringResponse.java | 2 +- .../extensions/ExtensionsManager.java | 114 +++++++---- .../extensions/ExtensionsSettings.java | 6 + .../RegisterTransportActionsRequest.java | 36 +++- .../action/ExtensionActionRequest.java | 76 +++++++ .../action/ExtensionActionResponse.java | 59 ++++++ .../ExtensionHandleTransportRequest.java | 89 ++++++++ .../action/ExtensionProxyAction.java | 25 +++ .../action/ExtensionTransportAction.java | 55 +++++ .../ExtensionTransportActionsHandler.java | 193 ++++++++++++++++++ .../TransportActionRequestFromExtension.java | 102 +++++++++ .../TransportActionResponseToExtension.java | 58 ++++++ .../extensions/action/package-info.java | 10 + .../rest/RestSendToExtensionAction.java | 2 +- .../main/java/org/opensearch/node/Node.java | 6 +- .../extensions/ExtensionsManagerTests.java | 146 ++++++++++--- .../RegisterTransportActionsRequestTests.java | 8 +- .../action/ExtensionActionRequestTests.java | 37 ++++ .../action/ExtensionActionResponseTests.java | 32 +++ .../ExtensionHandleTransportRequestTests.java | 35 ++++ .../action/ExtensionProxyActionTests.java | 18 ++ ...ExtensionTransportActionsHandlerTests.java | 181 ++++++++++++++++ ...nsportActionRequestFromExtensionTests.java | 42 ++++ ...ansportActionResponseToExtensionTests.java | 43 ++++ .../rest/RestSendToExtensionActionTests.java | 3 +- .../src/test/resources/config/extensions.yml | 5 + 30 files changed, 1410 insertions(+), 150 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/cluster/LocalNodeResponse.java create mode 100644 server/src/main/java/org/opensearch/extensions/ExtensionDependency.java create mode 100644 server/src/main/java/org/opensearch/extensions/action/ExtensionActionRequest.java create mode 100644 server/src/main/java/org/opensearch/extensions/action/ExtensionActionResponse.java create mode 100644 server/src/main/java/org/opensearch/extensions/action/ExtensionHandleTransportRequest.java create mode 100644 server/src/main/java/org/opensearch/extensions/action/ExtensionProxyAction.java create mode 100644 server/src/main/java/org/opensearch/extensions/action/ExtensionTransportAction.java create mode 100644 server/src/main/java/org/opensearch/extensions/action/ExtensionTransportActionsHandler.java create mode 100644 server/src/main/java/org/opensearch/extensions/action/TransportActionRequestFromExtension.java create mode 100644 server/src/main/java/org/opensearch/extensions/action/TransportActionResponseToExtension.java create mode 100644 server/src/main/java/org/opensearch/extensions/action/package-info.java create mode 100644 server/src/test/java/org/opensearch/extensions/action/ExtensionActionRequestTests.java create mode 100644 server/src/test/java/org/opensearch/extensions/action/ExtensionActionResponseTests.java create mode 100644 server/src/test/java/org/opensearch/extensions/action/ExtensionHandleTransportRequestTests.java create mode 100644 server/src/test/java/org/opensearch/extensions/action/ExtensionProxyActionTests.java create mode 100644 server/src/test/java/org/opensearch/extensions/action/ExtensionTransportActionsHandlerTests.java create mode 100644 server/src/test/java/org/opensearch/extensions/action/TransportActionRequestFromExtensionTests.java create mode 100644 server/src/test/java/org/opensearch/extensions/action/TransportActionResponseToExtensionTests.java diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java index 84bc9b395c5dc..bba3aabdd61f9 100644 --- a/server/src/main/java/org/opensearch/action/ActionModule.java +++ b/server/src/main/java/org/opensearch/action/ActionModule.java @@ -280,6 +280,8 @@ import org.opensearch.common.inject.TypeLiteral; import org.opensearch.common.inject.multibindings.MapBinder; import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.extensions.action.ExtensionProxyAction; +import org.opensearch.extensions.action.ExtensionTransportAction; import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsFilter; @@ -703,6 +705,11 @@ public void reg // Remote Store actions.register(RestoreRemoteStoreAction.INSTANCE, TransportRestoreRemoteStoreAction.class); + if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) { + // ExtensionProxyAction + actions.register(ExtensionProxyAction.INSTANCE, ExtensionTransportAction.class); + } + // Decommission actions actions.register(DecommissionAction.INSTANCE, TransportDecommissionAction.class); actions.register(GetDecommissionStateAction.INSTANCE, TransportGetDecommissionStateAction.class); diff --git a/server/src/main/java/org/opensearch/cluster/LocalNodeResponse.java b/server/src/main/java/org/opensearch/cluster/LocalNodeResponse.java deleted file mode 100644 index ef1ef4a49ad62..0000000000000 --- a/server/src/main/java/org/opensearch/cluster/LocalNodeResponse.java +++ /dev/null @@ -1,60 +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. - */ - -package org.opensearch.cluster; - -import org.opensearch.cluster.node.DiscoveryNode; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.transport.TransportResponse; - -import java.io.IOException; -import java.util.Objects; - -/** - * LocalNode Response for Extensibility - * - * @opensearch.internal - */ -public class LocalNodeResponse extends TransportResponse { - private final DiscoveryNode localNode; - - public LocalNodeResponse(ClusterService clusterService) { - this.localNode = clusterService.localNode(); - } - - public LocalNodeResponse(StreamInput in) throws IOException { - super(in); - this.localNode = new DiscoveryNode(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - this.localNode.writeTo(out); - } - - @Override - public String toString() { - return "LocalNodeResponse{" + "localNode=" + localNode + '}'; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - LocalNodeResponse that = (LocalNodeResponse) o; - return Objects.equals(localNode, that.localNode); - } - - @Override - public int hashCode() { - return Objects.hash(localNode); - } - -} diff --git a/server/src/main/java/org/opensearch/extensions/DiscoveryExtensionNode.java b/server/src/main/java/org/opensearch/extensions/DiscoveryExtensionNode.java index e4fa0d74f78f0..1d9e8b768be33 100644 --- a/server/src/main/java/org/opensearch/extensions/DiscoveryExtensionNode.java +++ b/server/src/main/java/org/opensearch/extensions/DiscoveryExtensionNode.java @@ -20,6 +20,9 @@ import org.opensearch.plugins.PluginInfo; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Map; /** @@ -30,6 +33,7 @@ public class DiscoveryExtensionNode extends DiscoveryNode implements Writeable, ToXContentFragment { private final PluginInfo pluginInfo; + private List dependencies = Collections.emptyList(); public DiscoveryExtensionNode( String name, @@ -40,16 +44,22 @@ public DiscoveryExtensionNode( TransportAddress address, Map attributes, Version version, - PluginInfo pluginInfo + PluginInfo pluginInfo, + List dependencies ) { super(name, id, ephemeralId, hostName, hostAddress, address, attributes, DiscoveryNodeRole.BUILT_IN_ROLES, version); this.pluginInfo = pluginInfo; + this.dependencies = dependencies; } @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); pluginInfo.writeTo(out); + out.writeVInt(dependencies.size()); + for (ExtensionDependency dependency : dependencies) { + dependency.writeTo(out); + } } /** @@ -61,6 +71,15 @@ public void writeTo(StreamOutput out) throws IOException { public DiscoveryExtensionNode(final StreamInput in) throws IOException { super(in); this.pluginInfo = new PluginInfo(in); + int size = in.readVInt(); + dependencies = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + dependencies.add(new ExtensionDependency(in)); + } + } + + public List getDependencies() { + return dependencies; } @Override diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java b/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java new file mode 100644 index 0000000000000..5e7fd651edfac --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/ExtensionDependency.java @@ -0,0 +1,89 @@ +/* +* Copyright OpenSearch Contributors + * 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; + +import java.io.IOException; +import java.util.Objects; + +import org.opensearch.Version; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.common.io.stream.Writeable; + +/** + * This class handles the dependent extensions information + * + * @opensearch.internal + */ +public class ExtensionDependency implements Writeable { + private String uniqueId; + private Version version; + + public ExtensionDependency(String uniqueId, Version version) { + this.uniqueId = uniqueId; + this.version = version; + } + + /** + * Jackson requires a no-arg constructor. + * + */ + @SuppressWarnings("unused") + private ExtensionDependency() {} + + /** + * Reads the extension dependency information + * + * @throws IOException if an I/O exception occurred reading the extension dependency information + */ + public ExtensionDependency(StreamInput in) throws IOException { + uniqueId = in.readString(); + version = Version.readVersion(in); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(uniqueId); + Version.writeVersion(version, out); + } + + /** + * The uniqueId of the dependency extension + * + * @return the extension uniqueId + */ + public String getUniqueId() { + return uniqueId; + } + + /** + * The minimum version of the dependency extension + * + * @return the extension version + */ + public Version getVersion() { + return version; + } + + public String toString() { + return "ExtensionDependency:{uniqueId=" + uniqueId + ", version=" + version + "}"; + } + + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + ExtensionDependency that = (ExtensionDependency) obj; + return Objects.equals(uniqueId, that.uniqueId) && Objects.equals(version, that.version); + } + + public int hashCode() { + return Objects.hash(uniqueId, version); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java b/server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java index 5c9c4c3ef784b..91fb5752e012d 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java @@ -17,7 +17,7 @@ /** * Generic string response indicating the status of some previous request sent to the SDK * - * @opensearch.internal + * @opensearch.api */ public class ExtensionStringResponse extends TransportResponse { private String response; diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index e42b9d1e755a5..e638faab3a747 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -26,8 +26,8 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.opensearch.Version; import org.opensearch.action.admin.cluster.state.ClusterStateResponse; +import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.ClusterSettingsResponse; -import org.opensearch.cluster.LocalNodeResponse; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.io.FileSystemUtils; @@ -39,6 +39,10 @@ import org.opensearch.discovery.InitializeExtensionRequest; import org.opensearch.discovery.InitializeExtensionResponse; import org.opensearch.extensions.ExtensionsSettings.Extension; +import org.opensearch.extensions.action.ExtensionActionRequest; +import org.opensearch.extensions.action.ExtensionActionResponse; +import org.opensearch.extensions.action.ExtensionTransportActionsHandler; +import org.opensearch.extensions.action.TransportActionRequestFromExtension; import org.opensearch.extensions.rest.RegisterRestActionsRequest; import org.opensearch.extensions.rest.RestActionsRequestHandler; import org.opensearch.extensions.settings.CustomSettingsRequestHandler; @@ -71,7 +75,6 @@ public class ExtensionsManager { public static final String INDICES_EXTENSION_POINT_ACTION_NAME = "indices:internal/extensions"; public static final String INDICES_EXTENSION_NAME_ACTION_NAME = "indices:internal/name"; public static final String REQUEST_EXTENSION_CLUSTER_STATE = "internal:discovery/clusterstate"; - public static final String REQUEST_EXTENSION_LOCAL_NODE = "internal:discovery/localnode"; public static final String REQUEST_EXTENSION_CLUSTER_SETTINGS = "internal:discovery/clustersettings"; public static final String REQUEST_EXTENSION_ENVIRONMENT_SETTINGS = "internal:discovery/enviornmentsettings"; public static final String REQUEST_EXTENSION_ADD_SETTINGS_UPDATE_CONSUMER = "internal:discovery/addsettingsupdateconsumer"; @@ -81,6 +84,9 @@ public class ExtensionsManager { public static final String REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS = "internal:discovery/registertransportactions"; public static final String REQUEST_OPENSEARCH_PARSE_NAMED_WRITEABLE = "internal:discovery/parsenamedwriteable"; public static final String REQUEST_REST_EXECUTE_ON_EXTENSION_ACTION = "internal:extensions/restexecuteonextensiontaction"; + public static final String REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION = "internal:extensions/handle-transportaction"; + public static final String TRANSPORT_ACTION_REQUEST_FROM_EXTENSION = "internal:extensions/request-transportaction-from-extension"; + public static final int EXTENSION_REQUEST_WAIT_TIMEOUT = 10; private static final Logger logger = LogManager.getLogger(ExtensionsManager.class); @@ -91,7 +97,6 @@ public class ExtensionsManager { */ public static enum RequestType { REQUEST_EXTENSION_CLUSTER_STATE, - REQUEST_EXTENSION_LOCAL_NODE, REQUEST_EXTENSION_CLUSTER_SETTINGS, REQUEST_EXTENSION_REGISTER_REST_ACTIONS, REQUEST_EXTENSION_REGISTER_SETTINGS, @@ -111,6 +116,7 @@ public static enum OpenSearchRequestType { } private final Path extensionsPath; + private ExtensionTransportActionsHandler extensionTransportActionsHandler; // A list of initialized extensions, a subset of the values of map below which includes all extensions private List extensions; private Map extensionIdMap; @@ -120,6 +126,7 @@ public static enum OpenSearchRequestType { private ClusterService clusterService; private Settings environmentSettings; private AddSettingsUpdateConsumerRequestHandler addSettingsUpdateConsumerRequestHandler; + private NodeClient client; public ExtensionsManager() { this.extensionsPath = Path.of(""); @@ -135,10 +142,13 @@ public ExtensionsManager() { public ExtensionsManager(Settings settings, Path extensionsPath) throws IOException { logger.info("ExtensionsManager initialized"); this.extensionsPath = extensionsPath; - this.transportService = null; this.extensions = new ArrayList(); this.extensionIdMap = new HashMap(); + // will be initialized in initializeServicesAndRestHandler which is called after the Node is initialized + this.transportService = null; this.clusterService = null; + this.client = null; + this.extensionTransportActionsHandler = null; /* * Now Discover extensions @@ -156,13 +166,15 @@ public ExtensionsManager(Settings settings, Path extensionsPath) throws IOExcept * @param transportService The Node's transport service. * @param clusterService The Node's cluster service. * @param initialEnvironmentSettings The finalized view of settings for the Environment + * @param client The client used to make transport requests */ public void initializeServicesAndRestHandler( RestController restController, SettingsModule settingsModule, TransportService transportService, ClusterService clusterService, - Settings initialEnvironmentSettings + Settings initialEnvironmentSettings, + NodeClient client ) { this.restActionsRequestHandler = new RestActionsRequestHandler(restController, extensionIdMap, transportService); this.customSettingsRequestHandler = new CustomSettingsRequestHandler(settingsModule); @@ -174,9 +186,20 @@ public void initializeServicesAndRestHandler( transportService, REQUEST_EXTENSION_UPDATE_SETTINGS ); + this.client = client; + this.extensionTransportActionsHandler = new ExtensionTransportActionsHandler(extensionIdMap, transportService, client); registerRequestHandler(); } + /** + * Handles Transport Request from {@link org.opensearch.extensions.action.ExtensionTransportAction} which was invoked by an extension via {@link ExtensionTransportActionsHandler}. + * + * @param request which was sent by an extension. + */ + public ExtensionActionResponse handleTransportRequest(ExtensionActionRequest request) throws InterruptedException { + return extensionTransportActionsHandler.sendTransportRequestToExtension(request); + } + private void registerRequestHandler() { transportService.registerRequestHandler( REQUEST_EXTENSION_REGISTER_REST_ACTIONS, @@ -202,14 +225,6 @@ private void registerRequestHandler() { ExtensionRequest::new, ((request, channel, task) -> channel.sendResponse(handleExtensionRequest(request))) ); - transportService.registerRequestHandler( - REQUEST_EXTENSION_LOCAL_NODE, - ThreadPool.Names.GENERIC, - false, - false, - ExtensionRequest::new, - ((request, channel, task) -> channel.sendResponse(handleExtensionRequest(request))) - ); transportService.registerRequestHandler( REQUEST_EXTENSION_CLUSTER_SETTINGS, ThreadPool.Names.GENERIC, @@ -242,7 +257,19 @@ private void registerRequestHandler() { false, false, RegisterTransportActionsRequest::new, - ((request, channel, task) -> channel.sendResponse(handleRegisterTransportActionsRequest(request))) + ((request, channel, task) -> channel.sendResponse( + extensionTransportActionsHandler.handleRegisterTransportActionsRequest(request) + )) + ); + transportService.registerRequestHandler( + TRANSPORT_ACTION_REQUEST_FROM_EXTENSION, + ThreadPool.Names.GENERIC, + false, + false, + TransportActionRequestFromExtension::new, + ((request, channel, task) -> channel.sendResponse( + extensionTransportActionsHandler.handleTransportActionRequestFromExtension(request) + )) ); } @@ -301,7 +328,8 @@ private void loadExtension(Extension extension) throws IOException { extension.getClassName(), new ArrayList(), Boolean.parseBoolean(extension.hasNativeController()) - ) + ), + extension.getDependencies() ); extensionIdMap.put(extension.getUniqueId(), discoveryExtensionNode); logger.info("Loaded extension with uniqueId " + extension.getUniqueId() + ": " + extension); @@ -364,7 +392,7 @@ public String executor() { initializeExtensionResponseHandler ); // TODO: make asynchronous - inProgressFuture.get(100, TimeUnit.SECONDS); + inProgressFuture.get(EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS); } catch (Exception e) { try { throw e; @@ -374,22 +402,6 @@ public String executor() { } } - /** - * Handles a {@link RegisterTransportActionsRequest}. - * - * @param transportActionsRequest The request to handle. - * @return A {@link AcknowledgedResponse} indicating success. - * @throws Exception if the request is not handled properly. - */ - TransportResponse handleRegisterTransportActionsRequest(RegisterTransportActionsRequest transportActionsRequest) throws Exception { - /* - * TODO: https://github.com/opensearch-project/opensearch-sdk-java/issues/107 - * Register these new Transport Actions with ActionModule - * and add support for NodeClient to recognise these actions when making transport calls. - */ - return new AcknowledgedResponse(true); - } - /** * Handles an {@link ExtensionRequest}. * @@ -401,8 +413,6 @@ TransportResponse handleExtensionRequest(ExtensionRequest extensionRequest) thro switch (extensionRequest.getRequestType()) { case REQUEST_EXTENSION_CLUSTER_STATE: return new ClusterStateResponse(clusterService.getClusterName(), clusterService.state(), false); - case REQUEST_EXTENSION_LOCAL_NODE: - return new LocalNodeResponse(clusterService); case REQUEST_EXTENSION_CLUSTER_SETTINGS: return new ClusterSettingsResponse(clusterService); case REQUEST_EXTENSION_ENVIRONMENT_SETTINGS: @@ -477,7 +487,7 @@ public void beforeIndexRemoved( acknowledgedResponseHandler ); // TODO: make asynchronous - inProgressIndexNameFuture.get(100, TimeUnit.SECONDS); + inProgressIndexNameFuture.get(EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS); logger.info("Received ack response from Extension"); } catch (Exception e) { try { @@ -513,7 +523,7 @@ public String executor() { indicesModuleResponseHandler ); // TODO: make asynchronous - inProgressFuture.get(100, TimeUnit.SECONDS); + inProgressFuture.get(EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS); logger.info("Received response from Extension"); } catch (Exception e) { try { @@ -547,10 +557,6 @@ public static String getRequestExtensionClusterState() { return REQUEST_EXTENSION_CLUSTER_STATE; } - public static String getRequestExtensionLocalNode() { - return REQUEST_EXTENSION_LOCAL_NODE; - } - public static String getRequestExtensionClusterSettings() { return REQUEST_EXTENSION_CLUSTER_SETTINGS; } @@ -661,4 +667,32 @@ public void setEnvironmentSettings(Settings environmentSettings) { this.environmentSettings = environmentSettings; } + public static String getRequestExtensionHandleTransportAction() { + return REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION; + } + + public static String getTransportActionRequestFromExtension() { + return TRANSPORT_ACTION_REQUEST_FROM_EXTENSION; + } + + public static int getExtensionRequestWaitTimeout() { + return EXTENSION_REQUEST_WAIT_TIMEOUT; + } + + public ExtensionTransportActionsHandler getExtensionTransportActionsHandler() { + return extensionTransportActionsHandler; + } + + public void setExtensionTransportActionsHandler(ExtensionTransportActionsHandler extensionTransportActionsHandler) { + this.extensionTransportActionsHandler = extensionTransportActionsHandler; + } + + public NodeClient getClient() { + return client; + } + + public void setClient(NodeClient client) { + this.client = client; + } + } diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsSettings.java b/server/src/main/java/org/opensearch/extensions/ExtensionsSettings.java index 8b6226e578ea3..61ab481bc0b76 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsSettings.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsSettings.java @@ -9,6 +9,7 @@ package org.opensearch.extensions; import java.util.ArrayList; +import java.util.Collections; import java.util.List; /** @@ -43,6 +44,7 @@ public static class Extension { private String className; private String customFolderName; private String hasNativeController; + private List dependencies = Collections.emptyList(); public Extension() { name = ""; @@ -184,6 +186,10 @@ public void setHasNativeController(String hasNativeController) { this.hasNativeController = hasNativeController; } + public List getDependencies() { + return dependencies; + } + } public List getExtensions() { diff --git a/server/src/main/java/org/opensearch/extensions/RegisterTransportActionsRequest.java b/server/src/main/java/org/opensearch/extensions/RegisterTransportActionsRequest.java index a3603aaf22dd0..47061f94dee83 100644 --- a/server/src/main/java/org/opensearch/extensions/RegisterTransportActionsRequest.java +++ b/server/src/main/java/org/opensearch/extensions/RegisterTransportActionsRequest.java @@ -8,6 +8,9 @@ package org.opensearch.extensions; +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionResponse; +import org.opensearch.action.support.TransportAction; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.transport.TransportRequest; @@ -16,6 +19,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Map.Entry; /** * Request to register extension Transport actions @@ -23,20 +27,28 @@ * @opensearch.internal */ public class RegisterTransportActionsRequest extends TransportRequest { - private Map transportActions; + private String uniqueId; + private Map>> transportActions; - public RegisterTransportActionsRequest(Map transportActions) { + public RegisterTransportActionsRequest( + String uniqueId, + Map>> transportActions + ) { + this.uniqueId = uniqueId; this.transportActions = new HashMap<>(transportActions); } public RegisterTransportActionsRequest(StreamInput in) throws IOException { super(in); - Map actions = new HashMap<>(); + this.uniqueId = in.readString(); + Map>> actions = new HashMap<>(); int actionCount = in.readVInt(); for (int i = 0; i < actionCount; i++) { try { String actionName = in.readString(); - Class transportAction = Class.forName(in.readString()); + @SuppressWarnings("unchecked") + Class> transportAction = (Class< + ? extends TransportAction>) Class.forName(in.readString()); actions.put(actionName, transportAction); } catch (ClassNotFoundException e) { throw new IllegalArgumentException("Could not read transport action"); @@ -45,15 +57,21 @@ public RegisterTransportActionsRequest(StreamInput in) throws IOException { this.transportActions = actions; } - public Map getTransportActions() { + public String getUniqueId() { + return uniqueId; + } + + public Map>> getTransportActions() { return transportActions; } @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); + out.writeString(uniqueId); out.writeVInt(this.transportActions.size()); - for (Map.Entry action : transportActions.entrySet()) { + for (Entry>> action : transportActions + .entrySet()) { out.writeString(action.getKey()); out.writeString(action.getValue().getName()); } @@ -61,7 +79,7 @@ public void writeTo(StreamOutput out) throws IOException { @Override public String toString() { - return "TransportActionsRequest{actions=" + transportActions + "}"; + return "TransportActionsRequest{uniqueId=" + uniqueId + ", actions=" + transportActions + "}"; } @Override @@ -69,11 +87,11 @@ public boolean equals(Object obj) { if (this == obj) return true; if (obj == null || getClass() != obj.getClass()) return false; RegisterTransportActionsRequest that = (RegisterTransportActionsRequest) obj; - return Objects.equals(transportActions, that.transportActions); + return Objects.equals(uniqueId, that.uniqueId) && Objects.equals(transportActions, that.transportActions); } @Override public int hashCode() { - return Objects.hash(transportActions); + return Objects.hash(uniqueId, transportActions); } } diff --git a/server/src/main/java/org/opensearch/extensions/action/ExtensionActionRequest.java b/server/src/main/java/org/opensearch/extensions/action/ExtensionActionRequest.java new file mode 100644 index 0000000000000..801b40e847d21 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/action/ExtensionActionRequest.java @@ -0,0 +1,76 @@ +/* + * 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.action; + +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; + +import java.io.IOException; + +/** + * This class translates Extension transport request to ActionRequest + * which is internally used to make transport action call. + * + * @opensearch.internal + */ +public class ExtensionActionRequest extends ActionRequest { + /** + * action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + */ + private final String action; + /** + * requestBytes is the raw bytes being transported between extensions. + */ + private final byte[] requestBytes; + + /** + * ExtensionActionRequest constructor. + * + * @param action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + * @param requestBytes is the raw bytes being transported between extensions. + */ + public ExtensionActionRequest(String action, byte[] requestBytes) { + this.action = action; + this.requestBytes = requestBytes; + } + + /** + * ExtensionActionRequest constructor from {@link StreamInput}. + * + * @param in bytes stream input used to de-serialize the message. + * @throws IOException when message de-serialization fails. + */ + ExtensionActionRequest(StreamInput in) throws IOException { + super(in); + action = in.readString(); + requestBytes = in.readByteArray(); + } + + public String getAction() { + return action; + } + + public byte[] getRequestBytes() { + return requestBytes; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(action); + out.writeByteArray(requestBytes); + } + + @Override + public ActionRequestValidationException validate() { + return null; + } +} diff --git a/server/src/main/java/org/opensearch/extensions/action/ExtensionActionResponse.java b/server/src/main/java/org/opensearch/extensions/action/ExtensionActionResponse.java new file mode 100644 index 0000000000000..68729ada48c25 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/action/ExtensionActionResponse.java @@ -0,0 +1,59 @@ +/* + * 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.action; + +import org.opensearch.action.ActionResponse; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; + +import java.io.IOException; + +/** + * This class encapsulates the transport response from extension + * + * @opensearch.internal + */ +public class ExtensionActionResponse extends ActionResponse { + /** + * responseBytes is the raw bytes being transported between extensions. + */ + private byte[] responseBytes; + + /** + * ExtensionActionResponse constructor. + * + * @param responseBytes is the raw bytes being transported between extensions. + */ + public ExtensionActionResponse(byte[] responseBytes) { + this.responseBytes = responseBytes; + } + + /** + * ExtensionActionResponse constructor from {@link StreamInput}. + * + * @param in bytes stream input used to de-serialize the message. + * @throws IOException when message de-serialization fails. + */ + public ExtensionActionResponse(StreamInput in) throws IOException { + responseBytes = in.readByteArray(); + } + + public byte[] getResponseBytes() { + return responseBytes; + } + + public void setResponseBytes(byte[] responseBytes) { + this.responseBytes = responseBytes; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeByteArray(responseBytes); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/action/ExtensionHandleTransportRequest.java b/server/src/main/java/org/opensearch/extensions/action/ExtensionHandleTransportRequest.java new file mode 100644 index 0000000000000..1b946d08f0459 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/action/ExtensionHandleTransportRequest.java @@ -0,0 +1,89 @@ +/* + * 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.action; + +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.transport.TransportRequest; + +import java.io.IOException; +import java.util.Objects; + +/** + * This class encapsulates a transport request to extension + * + * @opensearch.api + */ +public class ExtensionHandleTransportRequest extends TransportRequest { + /** + * action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + */ + private final String action; + /** + * requestBytes is the raw bytes being transported between extensions. + */ + private final byte[] requestBytes; + + /** + * ExtensionHandleTransportRequest constructor. + * + * @param action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + * @param requestBytes is the raw bytes being transported between extensions. + */ + public ExtensionHandleTransportRequest(String action, byte[] requestBytes) { + this.action = action; + this.requestBytes = requestBytes; + } + + /** + * ExtensionHandleTransportRequest constructor from {@link StreamInput}. + * + * @param in bytes stream input used to de-serialize the message. + * @throws IOException when message de-serialization fails. + */ + public ExtensionHandleTransportRequest(StreamInput in) throws IOException { + super(in); + this.action = in.readString(); + this.requestBytes = in.readByteArray(); + } + + public String getAction() { + return this.action; + } + + public byte[] getRequestBytes() { + return this.requestBytes; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(action); + out.writeByteArray(requestBytes); + } + + @Override + public String toString() { + return "ExtensionHandleTransportRequest{action=" + action + ", requestBytes=" + requestBytes + "}"; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + ExtensionHandleTransportRequest that = (ExtensionHandleTransportRequest) obj; + return Objects.equals(action, that.action) && Objects.equals(requestBytes, that.requestBytes); + } + + @Override + public int hashCode() { + return Objects.hash(action, requestBytes); + } + +} diff --git a/server/src/main/java/org/opensearch/extensions/action/ExtensionProxyAction.java b/server/src/main/java/org/opensearch/extensions/action/ExtensionProxyAction.java new file mode 100644 index 0000000000000..7345cf44e007f --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/action/ExtensionProxyAction.java @@ -0,0 +1,25 @@ +/* + * 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.action; + +import org.opensearch.action.ActionType; + +/** + * The main proxy action for all extensions + * + * @opensearch.internal + */ +public class ExtensionProxyAction extends ActionType { + public static final String NAME = "cluster:internal/extensions"; + public static final ExtensionProxyAction INSTANCE = new ExtensionProxyAction(); + + public ExtensionProxyAction() { + super(NAME, ExtensionActionResponse::new); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportAction.java b/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportAction.java new file mode 100644 index 0000000000000..5976db78002eb --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportAction.java @@ -0,0 +1,55 @@ +/* + * 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.action; + +import org.opensearch.action.ActionListener; +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.HandledTransportAction; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.inject.Inject; +import org.opensearch.common.settings.Settings; +import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.node.Node; +import org.opensearch.tasks.Task; +import org.opensearch.transport.TransportService; + +/** + * The main proxy transport action used to proxy a transport request from extension to another extension + * + * @opensearch.internal + */ +public class ExtensionTransportAction extends HandledTransportAction { + + private final String nodeName; + private final ClusterService clusterService; + private final ExtensionsManager extensionsManager; + + @Inject + public ExtensionTransportAction( + Settings settings, + TransportService transportService, + ActionFilters actionFilters, + ClusterService clusterService, + ExtensionsManager extensionsManager + ) { + super(ExtensionProxyAction.NAME, transportService, actionFilters, ExtensionActionRequest::new); + this.nodeName = Node.NODE_NAME_SETTING.get(settings); + this.clusterService = clusterService; + this.extensionsManager = extensionsManager; + } + + @Override + protected void doExecute(Task task, ExtensionActionRequest request, ActionListener listener) { + try { + listener.onResponse(extensionsManager.handleTransportRequest(request)); + } catch (Exception e) { + listener.onFailure(e); + } + } +} diff --git a/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportActionsHandler.java b/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportActionsHandler.java new file mode 100644 index 0000000000000..ac3ec6630634a --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/action/ExtensionTransportActionsHandler.java @@ -0,0 +1,193 @@ +/* + * 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.action; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.ActionListener; +import org.opensearch.client.node.NodeClient; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.extensions.DiscoveryExtensionNode; +import org.opensearch.extensions.AcknowledgedResponse; +import org.opensearch.extensions.ExtensionsManager; +import org.opensearch.extensions.RegisterTransportActionsRequest; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.ActionNotFoundTransportException; +import org.opensearch.transport.TransportException; +import org.opensearch.transport.TransportResponse; +import org.opensearch.transport.TransportResponseHandler; +import org.opensearch.transport.TransportService; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +/** + * This class manages TransportActions for extensions + * + * @opensearch.internal + */ +public class ExtensionTransportActionsHandler { + private static final Logger logger = LogManager.getLogger(ExtensionTransportActionsHandler.class); + private Map actionsMap; + private final Map extensionIdMap; + private final TransportService transportService; + private final NodeClient client; + + public ExtensionTransportActionsHandler( + Map extensionIdMap, + TransportService transportService, + NodeClient client + ) { + this.actionsMap = new HashMap<>(); + this.extensionIdMap = extensionIdMap; + this.transportService = transportService; + this.client = client; + } + + /** + * Method to register actions for extensions. + * + * @param action to be registered. + * @param extension for which action is being registered. + * @throws IllegalArgumentException when action being registered already is registered. + */ + void registerAction(String action, DiscoveryExtensionNode extension) throws IllegalArgumentException { + if (actionsMap.containsKey(action)) { + throw new IllegalArgumentException("The " + action + " you are trying to register is already registered"); + } + actionsMap.putIfAbsent(action, extension); + } + + /** + * Method to get extension for a given action. + * + * @param action for which to get the registered extension. + * @return the extension. + */ + public DiscoveryExtensionNode getExtension(String action) { + return actionsMap.get(action); + } + + /** + * Handles a {@link RegisterTransportActionsRequest}. + * + * @param transportActionsRequest The request to handle. + * @return A {@link AcknowledgedResponse} indicating success. + */ + public TransportResponse handleRegisterTransportActionsRequest(RegisterTransportActionsRequest transportActionsRequest) { + /* + * We are proxying the transport Actions through ExtensionProxyAction, so we really dont need to register dynamic actions for now. + */ + logger.debug("Register Transport Actions request recieved {}", transportActionsRequest); + DiscoveryExtensionNode extension = extensionIdMap.get(transportActionsRequest.getUniqueId()); + try { + for (String action : transportActionsRequest.getTransportActions().keySet()) { + registerAction(action, extension); + } + } catch (Exception e) { + logger.error("Could not register Transport Action " + e); + return new AcknowledgedResponse(false); + } + return new AcknowledgedResponse(true); + } + + /** + * Method which handles transport action request from an extension. + * + * @param request from extension. + * @return {@link TransportResponse} which is sent back to the transport action invoker. + * @throws InterruptedException when message transport fails. + */ + public TransportResponse handleTransportActionRequestFromExtension(TransportActionRequestFromExtension request) + throws InterruptedException { + DiscoveryExtensionNode extension = extensionIdMap.get(request.getUniqueId()); + final CountDownLatch inProgressLatch = new CountDownLatch(1); + final TransportActionResponseToExtension response = new TransportActionResponseToExtension(new byte[0]); + client.execute( + ExtensionProxyAction.INSTANCE, + new ExtensionActionRequest(request.getAction(), request.getRequestBytes()), + new ActionListener() { + @Override + public void onResponse(ExtensionActionResponse actionResponse) { + response.setResponseBytes(actionResponse.getResponseBytes()); + inProgressLatch.countDown(); + } + + @Override + public void onFailure(Exception exp) { + logger.debug("Transport request failed", exp); + byte[] responseBytes = ("Request failed: " + exp.getMessage()).getBytes(StandardCharsets.UTF_8); + response.setResponseBytes(responseBytes); + inProgressLatch.countDown(); + } + } + ); + inProgressLatch.await(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS); + return response; + } + + /** + * Method to send transport action request to an extension to handle. + * + * @param request to extension to handle transport request. + * @return {@link ExtensionActionResponse} which encapsulates the transport response from the extension. + * @throws InterruptedException when message transport fails. + */ + public ExtensionActionResponse sendTransportRequestToExtension(ExtensionActionRequest request) throws InterruptedException { + DiscoveryExtensionNode extension = actionsMap.get(request.getAction()); + if (extension == null) { + throw new ActionNotFoundTransportException(request.getAction()); + } + final CountDownLatch inProgressLatch = new CountDownLatch(1); + final ExtensionActionResponse extensionActionResponse = new ExtensionActionResponse(new byte[0]); + final TransportResponseHandler extensionActionResponseTransportResponseHandler = + new TransportResponseHandler() { + + @Override + public ExtensionActionResponse read(StreamInput in) throws IOException { + return new ExtensionActionResponse(in); + } + + @Override + public void handleResponse(ExtensionActionResponse response) { + extensionActionResponse.setResponseBytes(response.getResponseBytes()); + inProgressLatch.countDown(); + } + + @Override + public void handleException(TransportException exp) { + logger.debug("Transport request failed", exp); + byte[] responseBytes = ("Request failed: " + exp.getMessage()).getBytes(StandardCharsets.UTF_8); + extensionActionResponse.setResponseBytes(responseBytes); + inProgressLatch.countDown(); + } + + @Override + public String executor() { + return ThreadPool.Names.GENERIC; + } + }; + try { + transportService.sendRequest( + extension, + ExtensionsManager.REQUEST_EXTENSION_HANDLE_TRANSPORT_ACTION, + new ExtensionHandleTransportRequest(request.getAction(), request.getRequestBytes()), + extensionActionResponseTransportResponseHandler + ); + } catch (Exception e) { + logger.info("Failed to send transport action to extension " + extension.getName(), e); + } + inProgressLatch.await(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS); + return extensionActionResponse; + } +} diff --git a/server/src/main/java/org/opensearch/extensions/action/TransportActionRequestFromExtension.java b/server/src/main/java/org/opensearch/extensions/action/TransportActionRequestFromExtension.java new file mode 100644 index 0000000000000..df494297559b3 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/action/TransportActionRequestFromExtension.java @@ -0,0 +1,102 @@ +/* + * 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.action; + +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.transport.TransportRequest; + +import java.io.IOException; +import java.util.Objects; + +/** + * Transport Action Request from Extension + * + * @opensearch.api + */ +public class TransportActionRequestFromExtension extends TransportRequest { + /** + * action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + */ + private final String action; + /** + * requestBytes is the raw bytes being transported between extensions. + */ + private final byte[] requestBytes; + /** + * uniqueId to identify which extension is making a transport request call. + */ + private final String uniqueId; + + /** + * TransportActionRequestFromExtension constructor. + * + * @param action is the transport action intended to be invoked which is registered by an extension via {@link ExtensionTransportActionsHandler}. + * @param requestBytes is the raw bytes being transported between extensions. + * @param uniqueId to identify which extension is making a transport request call. + */ + public TransportActionRequestFromExtension(String action, byte[] requestBytes, String uniqueId) { + this.action = action; + this.requestBytes = requestBytes; + this.uniqueId = uniqueId; + } + + /** + * TransportActionRequestFromExtension constructor from {@link StreamInput}. + * + * @param in bytes stream input used to de-serialize the message. + * @throws IOException when message de-serialization fails. + */ + public TransportActionRequestFromExtension(StreamInput in) throws IOException { + super(in); + this.action = in.readString(); + this.requestBytes = in.readByteArray(); + this.uniqueId = in.readString(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeString(action); + out.writeByteArray(requestBytes); + out.writeString(uniqueId); + } + + public String getAction() { + return this.action; + } + + public byte[] getRequestBytes() { + return this.requestBytes; + } + + public String getUniqueId() { + return this.uniqueId; + } + + @Override + public String toString() { + return "TransportActionRequestFromExtension{action=" + action + ", requestBytes=" + requestBytes + ", uniqueId=" + uniqueId + "}"; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + TransportActionRequestFromExtension that = (TransportActionRequestFromExtension) obj; + return Objects.equals(action, that.action) + && Objects.equals(requestBytes, that.requestBytes) + && Objects.equals(uniqueId, that.uniqueId); + } + + @Override + public int hashCode() { + return Objects.hash(action, requestBytes, uniqueId); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/action/TransportActionResponseToExtension.java b/server/src/main/java/org/opensearch/extensions/action/TransportActionResponseToExtension.java new file mode 100644 index 0000000000000..2913402bcd5e1 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/action/TransportActionResponseToExtension.java @@ -0,0 +1,58 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.extensions.action; + +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.transport.TransportResponse; + +import java.io.IOException; + +/** + * This class encapsulates transport response to extension. + * + * @opensearch.api + */ +public class TransportActionResponseToExtension extends TransportResponse { + /** + * responseBytes is the raw bytes being transported between extensions. + */ + private byte[] responseBytes; + + /** + * TransportActionResponseToExtension constructor. + * + * @param responseBytes is the raw bytes being transported between extensions. + */ + public TransportActionResponseToExtension(byte[] responseBytes) { + this.responseBytes = responseBytes; + } + + /** + * TransportActionResponseToExtension constructor from {@link StreamInput} + * @param in bytes stream input used to de-serialize the message. + * @throws IOException when message de-serialization fails. + */ + public TransportActionResponseToExtension(StreamInput in) throws IOException { + this.responseBytes = in.readByteArray(); + } + + public void setResponseBytes(byte[] responseBytes) { + this.responseBytes = responseBytes; + } + + public byte[] getResponseBytes() { + return responseBytes; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeByteArray(responseBytes); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/action/package-info.java b/server/src/main/java/org/opensearch/extensions/action/package-info.java new file mode 100644 index 0000000000000..9bad08eaeb921 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/action/package-info.java @@ -0,0 +1,10 @@ +/* + * 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. + */ + +/** Actions classes for the extensions package. OpenSearch extensions provide extensibility to OpenSearch.*/ +package org.opensearch.extensions.action; diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java index d08a74c0ba314..9fd0bd73f5e5f 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java @@ -163,7 +163,7 @@ public String executor() { ); try { // TODO: make asynchronous - inProgressFuture.get(5, TimeUnit.SECONDS); + inProgressFuture.get(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS); } catch (InterruptedException e) { return channel -> channel.sendResponse( new BytesRestResponse(RestStatus.REQUEST_TIMEOUT, "No response from extension to request.") diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index e127826921fe9..46270230ccf27 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -833,7 +833,8 @@ protected Node( settingsModule, transportService, clusterService, - environment.settings() + environment.settings(), + client ); } final GatewayMetaState gatewayMetaState = new GatewayMetaState(); @@ -1001,6 +1002,9 @@ protected Node( b.bind(Client.class).toInstance(client); b.bind(NodeClient.class).toInstance(client); b.bind(Environment.class).toInstance(this.environment); + if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) { + b.bind(ExtensionsManager.class).toInstance(this.extensionsManager); + } b.bind(ThreadPool.class).toInstance(threadPool); b.bind(NodeEnvironment.class).toInstance(nodeEnvironment); b.bind(ResourceWatcherService.class).toInstance(resourceWatcherService); diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index 5847276d6adab..a216b4926c066 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -42,7 +42,6 @@ import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.ClusterSettingsResponse; -import org.opensearch.cluster.LocalNodeResponse; import org.opensearch.env.EnvironmentSettingsResponse; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; @@ -79,6 +78,7 @@ import org.opensearch.test.IndexSettingsModule; import org.opensearch.test.MockLogAppender; import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.client.NoOpNodeClient; import org.opensearch.test.transport.MockTransportService; import org.opensearch.threadpool.TestThreadPool; import org.opensearch.threadpool.ThreadPool; @@ -94,6 +94,7 @@ public class ExtensionsManagerTests extends OpenSearchTestCase { private RestController restController; private SettingsModule settingsModule; private ClusterService clusterService; + private NodeClient client; private MockNioTransport transport; private Path extensionDir; private final ThreadPool threadPool = new TestThreadPool(ExtensionsManagerTests.class.getSimpleName()); @@ -126,7 +127,10 @@ public class ExtensionsManagerTests extends OpenSearchTestCase { " javaVersion: '17'", " className: fakeClass2", " customFolderName: fakeFolder2", - " hasNativeController: true" + " hasNativeController: true", + " dependencies:", + " - uniqueId: 'uniqueid0'", + " - version: '2.0.0'" ); private DiscoveryExtensionNode extensionNode; @@ -190,8 +194,10 @@ public void setup() throws Exception { "fakeClass1", new ArrayList(), false - ) + ), + Collections.emptyList() ); + client = new NoOpNodeClient(this.getTestName()); } @Override @@ -199,6 +205,7 @@ public void setup() throws Exception { public void tearDown() throws Exception { super.tearDown(); transportService.close(); + client.close(); ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS); } @@ -209,6 +216,10 @@ public void testDiscover() throws Exception { List expectedUninitializedExtensions = new ArrayList(); + String expectedUniqueId = "uniqueid0"; + Version expectedVersion = Version.fromString("2.0.0"); + ExtensionDependency expectedDependency = new ExtensionDependency(expectedUniqueId, expectedVersion); + expectedUninitializedExtensions.add( new DiscoveryExtensionNode( "firstExtension", @@ -228,7 +239,8 @@ public void testDiscover() throws Exception { "fakeClass1", new ArrayList(), false - ) + ), + Collections.emptyList() ) ); @@ -251,10 +263,12 @@ public void testDiscover() throws Exception { "fakeClass2", new ArrayList(), true - ) + ), + List.of(expectedDependency) ) ); assertEquals(expectedUninitializedExtensions.size(), extensionsManager.getExtensionIdMap().values().size()); + assertEquals(List.of(expectedDependency), expectedUninitializedExtensions.get(1).getDependencies()); assertTrue(expectedUninitializedExtensions.containsAll(extensionsManager.getExtensionIdMap().values())); assertTrue(extensionsManager.getExtensionIdMap().values().containsAll(expectedUninitializedExtensions)); } @@ -289,12 +303,74 @@ public void testNonUniqueExtensionsDiscovery() throws Exception { "fakeClass1", new ArrayList(), false - ) + ), + Collections.emptyList() ) ); assertEquals(expectedUninitializedExtensions.size(), extensionsManager.getExtensionIdMap().values().size()); assertTrue(expectedUninitializedExtensions.containsAll(extensionsManager.getExtensionIdMap().values())); assertTrue(extensionsManager.getExtensionIdMap().values().containsAll(expectedUninitializedExtensions)); + assertTrue(expectedUninitializedExtensions.containsAll(emptyList())); + } + + public void testDiscoveryExtension() throws Exception { + String expectedId = "test id"; + Version expectedVersion = Version.fromString("2.0.0"); + ExtensionDependency expectedDependency = new ExtensionDependency(expectedId, expectedVersion); + + DiscoveryExtensionNode discoveryExtensionNode = new DiscoveryExtensionNode( + "firstExtension", + "uniqueid1", + "uniqueid1", + "myIndependentPluginHost1", + "127.0.0.0", + new TransportAddress(InetAddress.getByName("127.0.0.0"), 9300), + new HashMap(), + Version.fromString("3.0.0"), + new PluginInfo( + "firstExtension", + "Fake description 1", + "0.0.7", + Version.fromString("3.0.0"), + "14", + "fakeClass1", + new ArrayList(), + false + ), + List.of(expectedDependency) + ); + + assertEquals(List.of(expectedDependency), discoveryExtensionNode.getDependencies()); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + discoveryExtensionNode.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + discoveryExtensionNode = new DiscoveryExtensionNode(in); + + assertEquals(List.of(expectedDependency), discoveryExtensionNode.getDependencies()); + } + } + } + + public void testExtensionDependency() throws Exception { + String expectedUniqueId = "Test uniqueId"; + Version expectedVersion = Version.fromString("3.0.0"); + + ExtensionDependency dependency = new ExtensionDependency(expectedUniqueId, expectedVersion); + + assertEquals(expectedUniqueId, dependency.getUniqueId()); + assertEquals(expectedVersion, dependency.getVersion()); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + dependency.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + dependency = new ExtensionDependency(in); + assertEquals(expectedUniqueId, dependency.getUniqueId()); + assertEquals(expectedVersion, dependency.getVersion()); + } + } } public void testNonAccessibleDirectory() throws Exception { @@ -339,12 +415,9 @@ public void testEmptyExtensionsFile() throws Exception { public void testInitialize() throws Exception { Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - transportService.start(); - transportService.acceptIncomingRequests(); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + initialize(extensionsManager); try (MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(ExtensionsManager.class))) { @@ -379,8 +452,8 @@ public void testHandleRegisterRestActionsRequest() throws Exception { Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + initialize(extensionsManager); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); String uniqueIdStr = "uniqueid1"; List actionsList = List.of("GET /foo", "PUT /bar", "POST /baz"); RegisterRestActionsRequest registerActionsRequest = new RegisterRestActionsRequest(uniqueIdStr, actionsList); @@ -393,10 +466,9 @@ public void testHandleRegisterRestActionsRequest() throws Exception { public void testHandleRegisterSettingsRequest() throws Exception { Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + initialize(extensionsManager); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); String uniqueIdStr = "uniqueid1"; List> settingsList = List.of( Setting.boolSetting("index.falseSetting", false, Property.IndexScope, Property.Dynamic), @@ -413,8 +485,8 @@ public void testHandleRegisterSettingsRequest() throws Exception { public void testHandleRegisterRestActionsRequestWithInvalidMethod() throws Exception { ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + initialize(extensionsManager); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); String uniqueIdStr = "uniqueid1"; List actionsList = List.of("FOO /foo", "PUT /bar", "POST /baz"); RegisterRestActionsRequest registerActionsRequest = new RegisterRestActionsRequest(uniqueIdStr, actionsList); @@ -425,12 +497,8 @@ public void testHandleRegisterRestActionsRequestWithInvalidMethod() throws Excep } public void testHandleRegisterRestActionsRequestWithInvalidUri() throws Exception { - - Path extensionDir = createTempDir(); - ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + initialize(extensionsManager); String uniqueIdStr = "uniqueid1"; List actionsList = List.of("GET", "PUT /bar", "POST /baz"); RegisterRestActionsRequest registerActionsRequest = new RegisterRestActionsRequest(uniqueIdStr, actionsList); @@ -441,19 +509,15 @@ public void testHandleRegisterRestActionsRequestWithInvalidUri() throws Exceptio } public void testHandleExtensionRequest() throws Exception { - ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); + initialize(extensionsManager); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); ExtensionRequest clusterStateRequest = new ExtensionRequest(ExtensionsManager.RequestType.REQUEST_EXTENSION_CLUSTER_STATE); assertEquals(ClusterStateResponse.class, extensionsManager.handleExtensionRequest(clusterStateRequest).getClass()); ExtensionRequest clusterSettingRequest = new ExtensionRequest(ExtensionsManager.RequestType.REQUEST_EXTENSION_CLUSTER_SETTINGS); assertEquals(ClusterSettingsResponse.class, extensionsManager.handleExtensionRequest(clusterSettingRequest).getClass()); - ExtensionRequest localNodeRequest = new ExtensionRequest(ExtensionsManager.RequestType.REQUEST_EXTENSION_LOCAL_NODE); - assertEquals(LocalNodeResponse.class, extensionsManager.handleExtensionRequest(localNodeRequest).getClass()); - ExtensionRequest environmentSettingsRequest = new ExtensionRequest( ExtensionsManager.RequestType.REQUEST_EXTENSION_ENVIRONMENT_SETTINGS ); @@ -532,7 +596,7 @@ public void testAddSettingsUpdateConsumerRequest() throws Exception { Path extensionDir = createTempDir(); Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + initialize(extensionsManager); List> componentSettings = List.of( Setting.boolSetting("falseSetting", false, Property.IndexScope, Property.NodeScope), @@ -579,8 +643,7 @@ public void testHandleAddSettingsUpdateConsumerRequest() throws Exception { Path extensionDir = createTempDir(); Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + initialize(extensionsManager); List> componentSettings = List.of( Setting.boolSetting("falseSetting", false, Property.Dynamic), @@ -602,7 +665,7 @@ public void testUpdateSettingsRequest() throws Exception { Path extensionDir = createTempDir(); Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + initialize(extensionsManager); Setting componentSetting = Setting.boolSetting("falseSetting", false, Property.Dynamic); SettingType settingType = SettingType.Boolean; @@ -643,19 +706,22 @@ public void testRegisterHandler() throws Exception { Collections.emptySet() ) ); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, mockTransportService, clusterService, settings); + extensionsManager.initializeServicesAndRestHandler( + restController, + settingsModule, + mockTransportService, + clusterService, + settings, + client + ); verify(mockTransportService, times(8)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any()); } public void testOnIndexModule() throws Exception { Files.write(extensionDir.resolve("extensions.yml"), extensionsYmlLines, StandardCharsets.UTF_8); - ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); - - transportService.start(); - transportService.acceptIncomingRequests(); - extensionsManager.initializeServicesAndRestHandler(restController, settingsModule, transportService, clusterService, settings); + initialize(extensionsManager); Environment environment = TestEnvironment.newEnvironment(settings); AnalysisRegistry emptyAnalysisRegistry = new AnalysisRegistry( @@ -699,4 +765,16 @@ public void testOnIndexModule() throws Exception { } } + private void initialize(ExtensionsManager extensionsManager) { + transportService.start(); + transportService.acceptIncomingRequests(); + extensionsManager.initializeServicesAndRestHandler( + restController, + settingsModule, + transportService, + clusterService, + settings, + client + ); + } } diff --git a/server/src/test/java/org/opensearch/extensions/RegisterTransportActionsRequestTests.java b/server/src/test/java/org/opensearch/extensions/RegisterTransportActionsRequestTests.java index ed36cc5290bb1..eb59c80ac6461 100644 --- a/server/src/test/java/org/opensearch/extensions/RegisterTransportActionsRequestTests.java +++ b/server/src/test/java/org/opensearch/extensions/RegisterTransportActionsRequestTests.java @@ -9,6 +9,7 @@ package org.opensearch.extensions; import org.junit.Before; +import org.opensearch.action.admin.indices.create.AutoCreateAction.TransportAction; import org.opensearch.common.collect.Map; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.io.stream.StreamInput; @@ -21,7 +22,7 @@ public class RegisterTransportActionsRequestTests extends OpenSearchTestCase { @Before public void setup() { - this.originalRequest = new RegisterTransportActionsRequest(Map.of("testAction", Map.class)); + this.originalRequest = new RegisterTransportActionsRequest("extension-uniqueId", Map.of("testAction", TransportAction.class)); } public void testRegisterTransportActionsRequest() throws IOException { @@ -37,6 +38,9 @@ public void testRegisterTransportActionsRequest() throws IOException { } public void testToString() { - assertEquals(originalRequest.toString(), "TransportActionsRequest{actions={testAction=class org.opensearch.common.collect.Map}}"); + assertEquals( + originalRequest.toString(), + "TransportActionsRequest{uniqueId=extension-uniqueId, actions={testAction=class org.opensearch.action.admin.indices.create.AutoCreateAction$TransportAction}}" + ); } } diff --git a/server/src/test/java/org/opensearch/extensions/action/ExtensionActionRequestTests.java b/server/src/test/java/org/opensearch/extensions/action/ExtensionActionRequestTests.java new file mode 100644 index 0000000000000..2d4f2b5d8aa66 --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/action/ExtensionActionRequestTests.java @@ -0,0 +1,37 @@ +/* + * 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.action; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.test.OpenSearchTestCase; + +import java.nio.charset.StandardCharsets; + +public class ExtensionActionRequestTests extends OpenSearchTestCase { + + public void testExtensionActionRequest() throws Exception { + String expectedAction = "test-action"; + byte[] expectedRequestBytes = "request-bytes".getBytes(StandardCharsets.UTF_8); + ExtensionActionRequest request = new ExtensionActionRequest(expectedAction, expectedRequestBytes); + + assertEquals(expectedAction, request.getAction()); + assertEquals(expectedRequestBytes, request.getRequestBytes()); + + BytesStreamOutput out = new BytesStreamOutput(); + request.writeTo(out); + BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes())); + request = new ExtensionActionRequest(in); + + assertEquals(expectedAction, request.getAction()); + assertArrayEquals(expectedRequestBytes, request.getRequestBytes()); + assertNull(request.validate()); + } +} diff --git a/server/src/test/java/org/opensearch/extensions/action/ExtensionActionResponseTests.java b/server/src/test/java/org/opensearch/extensions/action/ExtensionActionResponseTests.java new file mode 100644 index 0000000000000..5ec8c16027da2 --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/action/ExtensionActionResponseTests.java @@ -0,0 +1,32 @@ +/* + * 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.action; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.test.OpenSearchTestCase; + +import java.nio.charset.StandardCharsets; + +public class ExtensionActionResponseTests extends OpenSearchTestCase { + + public void testExtensionActionResponse() throws Exception { + byte[] expectedResponseBytes = "response-bytes".getBytes(StandardCharsets.UTF_8); + ExtensionActionResponse response = new ExtensionActionResponse(expectedResponseBytes); + + assertEquals(expectedResponseBytes, response.getResponseBytes()); + + BytesStreamOutput out = new BytesStreamOutput(); + response.writeTo(out); + BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes())); + response = new ExtensionActionResponse(in); + assertArrayEquals(expectedResponseBytes, response.getResponseBytes()); + } +} diff --git a/server/src/test/java/org/opensearch/extensions/action/ExtensionHandleTransportRequestTests.java b/server/src/test/java/org/opensearch/extensions/action/ExtensionHandleTransportRequestTests.java new file mode 100644 index 0000000000000..15e7320ba7556 --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/action/ExtensionHandleTransportRequestTests.java @@ -0,0 +1,35 @@ +/* + * 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.action; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.test.OpenSearchTestCase; + +import java.nio.charset.StandardCharsets; + +public class ExtensionHandleTransportRequestTests extends OpenSearchTestCase { + public void testExtensionHandleTransportRequest() throws Exception { + String expectedAction = "test-action"; + byte[] expectedRequestBytes = "request-bytes".getBytes(StandardCharsets.UTF_8); + ExtensionHandleTransportRequest request = new ExtensionHandleTransportRequest(expectedAction, expectedRequestBytes); + + assertEquals(expectedAction, request.getAction()); + assertEquals(expectedRequestBytes, request.getRequestBytes()); + + BytesStreamOutput out = new BytesStreamOutput(); + request.writeTo(out); + BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes())); + request = new ExtensionHandleTransportRequest(in); + + assertEquals(expectedAction, request.getAction()); + assertArrayEquals(expectedRequestBytes, request.getRequestBytes()); + } +} diff --git a/server/src/test/java/org/opensearch/extensions/action/ExtensionProxyActionTests.java b/server/src/test/java/org/opensearch/extensions/action/ExtensionProxyActionTests.java new file mode 100644 index 0000000000000..3719c29090287 --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/action/ExtensionProxyActionTests.java @@ -0,0 +1,18 @@ +/* + * 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.action; + +import org.opensearch.test.OpenSearchTestCase; + +public class ExtensionProxyActionTests extends OpenSearchTestCase { + public void testExtensionProxyAction() { + assertEquals("cluster:internal/extensions", ExtensionProxyAction.NAME); + assertEquals(ExtensionProxyAction.class, ExtensionProxyAction.INSTANCE.getClass()); + } +} diff --git a/server/src/test/java/org/opensearch/extensions/action/ExtensionTransportActionsHandlerTests.java b/server/src/test/java/org/opensearch/extensions/action/ExtensionTransportActionsHandlerTests.java new file mode 100644 index 0000000000000..c3d6372a4f6b8 --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/action/ExtensionTransportActionsHandlerTests.java @@ -0,0 +1,181 @@ +/* + * 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.action; + +import org.junit.After; +import org.junit.Before; +import org.opensearch.Version; +import org.opensearch.action.admin.indices.create.AutoCreateAction.TransportAction; +import org.opensearch.client.node.NodeClient; +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.common.io.stream.NamedWriteableRegistry; +import org.opensearch.common.network.NetworkService; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.transport.TransportAddress; +import org.opensearch.common.util.PageCacheRecycler; +import org.opensearch.extensions.DiscoveryExtensionNode; +import org.opensearch.extensions.AcknowledgedResponse; +import org.opensearch.extensions.RegisterTransportActionsRequest; +import org.opensearch.extensions.rest.RestSendToExtensionActionTests; +import org.opensearch.indices.breaker.NoneCircuitBreakerService; +import org.opensearch.plugins.PluginInfo; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.test.client.NoOpNodeClient; +import org.opensearch.test.transport.MockTransportService; +import org.opensearch.threadpool.TestThreadPool; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.ActionNotFoundTransportException; +import org.opensearch.transport.TransportService; +import org.opensearch.transport.nio.MockNioTransport; + +import java.net.InetAddress; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import static java.util.Collections.emptyMap; +import static java.util.Collections.emptySet; + +public class ExtensionTransportActionsHandlerTests extends OpenSearchTestCase { + private TransportService transportService; + private MockNioTransport transport; + private DiscoveryExtensionNode discoveryExtensionNode; + private ExtensionTransportActionsHandler extensionTransportActionsHandler; + private NodeClient client; + private final ThreadPool threadPool = new TestThreadPool(RestSendToExtensionActionTests.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() + ); + discoveryExtensionNode = new DiscoveryExtensionNode( + "firstExtension", + "uniqueid1", + "uniqueid1", + "myIndependentPluginHost1", + "127.0.0.0", + new TransportAddress(InetAddress.getByName("127.0.0.0"), 9300), + new HashMap(), + Version.fromString("3.0.0"), + new PluginInfo( + "firstExtension", + "Fake description 1", + "0.0.7", + Version.fromString("3.0.0"), + "14", + "fakeClass1", + new ArrayList(), + false + ), + Collections.emptyList() + ); + client = new NoOpNodeClient(this.getTestName()); + extensionTransportActionsHandler = new ExtensionTransportActionsHandler( + Map.of("uniqueid1", discoveryExtensionNode), + transportService, + client + ); + } + + @Override + @After + public void tearDown() throws Exception { + super.tearDown(); + transportService.close(); + client.close(); + ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS); + } + + public void testRegisterAction() { + String action = "test-action"; + extensionTransportActionsHandler.registerAction(action, discoveryExtensionNode); + assertEquals(discoveryExtensionNode, extensionTransportActionsHandler.getExtension(action)); + + // Test duplicate action registration + expectThrows(IllegalArgumentException.class, () -> extensionTransportActionsHandler.registerAction(action, discoveryExtensionNode)); + assertEquals(discoveryExtensionNode, extensionTransportActionsHandler.getExtension(action)); + } + + public void testRegisterTransportActionsRequest() { + String action = "test-action"; + RegisterTransportActionsRequest request = new RegisterTransportActionsRequest("uniqueid1", Map.of(action, TransportAction.class)); + AcknowledgedResponse response = (AcknowledgedResponse) extensionTransportActionsHandler.handleRegisterTransportActionsRequest( + request + ); + assertTrue(response.getStatus()); + assertEquals(discoveryExtensionNode, extensionTransportActionsHandler.getExtension(action)); + + // Test duplicate action registration + response = (AcknowledgedResponse) extensionTransportActionsHandler.handleRegisterTransportActionsRequest(request); + assertFalse(response.getStatus()); + } + + public void testTransportActionRequestFromExtension() throws InterruptedException { + String action = "test-action"; + byte[] requestBytes = "requestBytes".getBytes(StandardCharsets.UTF_8); + TransportActionRequestFromExtension request = new TransportActionRequestFromExtension(action, requestBytes, "uniqueid1"); + // NoOpNodeClient returns null as response + expectThrows(NullPointerException.class, () -> extensionTransportActionsHandler.handleTransportActionRequestFromExtension(request)); + } + + public void testSendTransportRequestToExtension() throws InterruptedException { + String action = "test-action"; + byte[] requestBytes = "request-bytes".getBytes(StandardCharsets.UTF_8); + ExtensionActionRequest request = new ExtensionActionRequest(action, requestBytes); + + // Action not registered, expect exception + expectThrows( + ActionNotFoundTransportException.class, + () -> extensionTransportActionsHandler.sendTransportRequestToExtension(request) + ); + + // Register Action + RegisterTransportActionsRequest registerRequest = new RegisterTransportActionsRequest( + "uniqueid1", + Map.of(action, TransportAction.class) + ); + AcknowledgedResponse response = (AcknowledgedResponse) extensionTransportActionsHandler.handleRegisterTransportActionsRequest( + registerRequest + ); + assertTrue(response.getStatus()); + + ExtensionActionResponse extensionResponse = extensionTransportActionsHandler.sendTransportRequestToExtension(request); + assertEquals( + "Request failed: [firstExtension][127.0.0.0:9300] Node not connected", + new String(extensionResponse.getResponseBytes(), StandardCharsets.UTF_8) + ); + } +} diff --git a/server/src/test/java/org/opensearch/extensions/action/TransportActionRequestFromExtensionTests.java b/server/src/test/java/org/opensearch/extensions/action/TransportActionRequestFromExtensionTests.java new file mode 100644 index 0000000000000..a8ef5372800d9 --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/action/TransportActionRequestFromExtensionTests.java @@ -0,0 +1,42 @@ +/* + * 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.action; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.test.OpenSearchTestCase; + +import java.nio.charset.StandardCharsets; + +public class TransportActionRequestFromExtensionTests extends OpenSearchTestCase { + public void testTransportActionRequestFromExtension() throws Exception { + String expectedAction = "test-action"; + byte[] expectedRequestBytes = "request-bytes".getBytes(StandardCharsets.UTF_8); + String uniqueId = "test-uniqueId"; + TransportActionRequestFromExtension request = new TransportActionRequestFromExtension( + expectedAction, + expectedRequestBytes, + uniqueId + ); + + assertEquals(expectedAction, request.getAction()); + assertEquals(expectedRequestBytes, request.getRequestBytes()); + assertEquals(uniqueId, request.getUniqueId()); + + BytesStreamOutput out = new BytesStreamOutput(); + request.writeTo(out); + BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes())); + request = new TransportActionRequestFromExtension(in); + + assertEquals(expectedAction, request.getAction()); + assertArrayEquals(expectedRequestBytes, request.getRequestBytes()); + assertEquals(uniqueId, request.getUniqueId()); + } +} diff --git a/server/src/test/java/org/opensearch/extensions/action/TransportActionResponseToExtensionTests.java b/server/src/test/java/org/opensearch/extensions/action/TransportActionResponseToExtensionTests.java new file mode 100644 index 0000000000000..070feaa240d98 --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/action/TransportActionResponseToExtensionTests.java @@ -0,0 +1,43 @@ +/* + * 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.action; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +public class TransportActionResponseToExtensionTests extends OpenSearchTestCase { + public void testTransportActionRequestToExtension() throws IOException { + byte[] expectedResponseBytes = "response-bytes".getBytes(StandardCharsets.UTF_8); + TransportActionResponseToExtension response = new TransportActionResponseToExtension(expectedResponseBytes); + + assertEquals(expectedResponseBytes, response.getResponseBytes()); + + BytesStreamOutput out = new BytesStreamOutput(); + response.writeTo(out); + BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes())); + response = new TransportActionResponseToExtension(in); + + assertArrayEquals(expectedResponseBytes, response.getResponseBytes()); + } + + public void testSetBytes() { + byte[] expectedResponseBytes = "response-bytes".getBytes(StandardCharsets.UTF_8); + byte[] expectedEmptyBytes = new byte[0]; + TransportActionResponseToExtension response = new TransportActionResponseToExtension(expectedEmptyBytes); + assertArrayEquals(expectedEmptyBytes, response.getResponseBytes()); + + response.setResponseBytes(expectedResponseBytes); + assertArrayEquals(expectedResponseBytes, response.getResponseBytes()); + } +} 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 2a593a8d251e9..97eeae8fb95af 100644 --- a/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java +++ b/server/src/test/java/org/opensearch/extensions/rest/RestSendToExtensionActionTests.java @@ -93,7 +93,8 @@ public void setup() throws Exception { "fakeClass1", new ArrayList(), false - ) + ), + Collections.emptyList() ); } diff --git a/server/src/test/resources/config/extensions.yml b/server/src/test/resources/config/extensions.yml index 6264e9630ad60..e02a2913385d9 100644 --- a/server/src/test/resources/config/extensions.yml +++ b/server/src/test/resources/config/extensions.yml @@ -7,6 +7,11 @@ extensions: version: '3.0.0' - name: "secondExtension" uniqueId: 'uniqueid2' + dependencies: + - name: 'uniqueid0' + version: '2.0.0' + - name: 'uniqueid1' + version: '3.0.0' hostName: 'myIndependentPluginHost2' hostAddress: '127.0.0.1' port: '9301' From f97430414e6ccb3c307547c13e8fc63871291c69 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Thu, 22 Dec 2022 08:05:18 +0000 Subject: [PATCH 07/10] Update CHANGELOG Signed-off-by: Ryan Bogan --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7255c83bbd0e..254b91e1c494c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add feature flag for extensions ([#5211](https://github.com/opensearch-project/OpenSearch/pull/5211)) - Added jackson dependency to server ([#5366] (https://github.com/opensearch-project/OpenSearch/pull/5366)) - Adding support to register settings dynamically ([#5495](https://github.com/opensearch-project/OpenSearch/pull/5495)) -- Added experimental support for extensions ([#5347](https://github.com/opensearch-project/OpenSearch/pull/5347)), ([#5518](https://github.com/opensearch-project/OpenSearch/pull/5518), ([#5597](https://github.com/opensearch-project/OpenSearch/pull/5597))) +- Added experimental support for extensions ([#5347](https://github.com/opensearch-project/OpenSearch/pull/5347)), ([#5518](https://github.com/opensearch-project/OpenSearch/pull/5518), ([#5597](https://github.com/opensearch-project/OpenSearch/pull/5597)), ([#5615](https://github.com/opensearch-project/OpenSearch/pull/5615))) - Add CI bundle pattern to distribution download ([#5348](https://github.com/opensearch-project/OpenSearch/pull/5348)) - Add support for ppc64le architecture ([#5459](https://github.com/opensearch-project/OpenSearch/pull/5459)) From f46fc9ce82edb25cf6b8633e4e06541d4ded1ee0 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Thu, 22 Dec 2022 16:10:41 +0000 Subject: [PATCH 08/10] Update failure handling in UpdateSettingsResponseHandler Signed-off-by: Ryan Bogan --- .../opensearch/extensions/UpdateSettingsResponseHandler.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/org/opensearch/extensions/UpdateSettingsResponseHandler.java b/server/src/main/java/org/opensearch/extensions/UpdateSettingsResponseHandler.java index be8f43b5cfce6..9bf53812c223e 100644 --- a/server/src/main/java/org/opensearch/extensions/UpdateSettingsResponseHandler.java +++ b/server/src/main/java/org/opensearch/extensions/UpdateSettingsResponseHandler.java @@ -33,6 +33,9 @@ public AcknowledgedResponse read(StreamInput in) throws IOException { @Override public void handleResponse(AcknowledgedResponse response) { logger.info("response {}", response.getStatus()); + if (!response.getStatus()) { + handleException(new TransportException("Request was not completed successfully")); + } } @Override From 82a8365a847988acb069435ca93ee8441ea96006 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Thu, 22 Dec 2022 21:09:12 +0000 Subject: [PATCH 09/10] Added REST updates Signed-off-by: Ryan Bogan --- .../extensions/ExtensionStringResponse.java | 41 --- .../extensions/rest/ExtensionRestRequest.java | 292 ++++++++++++++++++ .../rest/ExtensionRestResponse.java | 113 +++++++ .../rest/RestActionsRequestHandler.java | 8 +- .../rest/RestExecuteOnExtensionRequest.java | 77 ----- .../rest/RestExecuteOnExtensionResponse.java | 56 +++- .../rest/RestSendToExtensionAction.java | 90 +++--- .../CustomSettingsRequestHandler.java | 8 +- .../extensions/ExtensionResponseTests.java | 17 - .../extensions/ExtensionsManagerTests.java | 11 +- .../rest/ExtensionRestRequestTests.java | 262 ++++++++++++++++ .../rest/ExtensionRestResponseTests.java | 132 ++++++++ .../rest/RestExecuteOnExtensionTests.java | 94 ------ 13 files changed, 898 insertions(+), 303 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java create mode 100644 server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java create mode 100644 server/src/main/java/org/opensearch/extensions/rest/ExtensionRestResponse.java delete mode 100644 server/src/main/java/org/opensearch/extensions/rest/RestExecuteOnExtensionRequest.java create mode 100644 server/src/test/java/org/opensearch/extensions/rest/ExtensionRestRequestTests.java create mode 100644 server/src/test/java/org/opensearch/extensions/rest/ExtensionRestResponseTests.java delete mode 100644 server/src/test/java/org/opensearch/extensions/rest/RestExecuteOnExtensionTests.java diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java b/server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java deleted file mode 100644 index 91fb5752e012d..0000000000000 --- a/server/src/main/java/org/opensearch/extensions/ExtensionStringResponse.java +++ /dev/null @@ -1,41 +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. - */ - -package org.opensearch.extensions; - -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.transport.TransportResponse; - -import java.io.IOException; - -/** - * Generic string response indicating the status of some previous request sent to the SDK - * - * @opensearch.api - */ -public class ExtensionStringResponse extends TransportResponse { - private String response; - - public ExtensionStringResponse(String response) { - this.response = response; - } - - public ExtensionStringResponse(StreamInput in) throws IOException { - response = in.readString(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeString(response); - } - - public String getResponse() { - return response; - } -} diff --git a/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java b/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java new file mode 100644 index 0000000000000..da59578b4917e --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestRequest.java @@ -0,0 +1,292 @@ +/* + * 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.OpenSearchParseException; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.NamedXContentRegistry; +import org.opensearch.common.xcontent.XContentParser; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestRequest.Method; +import org.opensearch.transport.TransportRequest; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +/** + * Request to execute REST actions on extension node. + * This contains necessary portions of a {@link RestRequest} object, but does not pass the full request for security concerns. + * + * @opensearch.api + */ +public class ExtensionRestRequest extends TransportRequest { + + private Method method; + private String path; + private Map params; + private XContentType xContentType = null; + private BytesReference content; + // The owner of this request object + // Will be replaced with PrincipalIdentifierToken class from feature/identity + private String principalIdentifierToken; + + // Tracks consumed parameters and content + private final Set consumedParams = new HashSet<>(); + private boolean contentConsumed = false; + + /** + * This object can be instantiated given method, uri, params, content and identifier + * + * @param method of type {@link Method} + * @param path the REST path string (excluding the query) + * @param params the REST params + * @param xContentType the content type, or null for plain text or no content + * @param content the REST request content + * @param principalIdentifier the owner of this request + */ + public ExtensionRestRequest( + Method method, + String path, + Map params, + XContentType xContentType, + BytesReference content, + String principalIdentifier + ) { + this.method = method; + this.path = path; + this.params = params; + this.xContentType = xContentType; + this.content = content; + this.principalIdentifierToken = principalIdentifier; + } + + /** + * Instantiate this request from input stream + * + * @param in Input stream + * @throws IOException on failure to read the stream + */ + public ExtensionRestRequest(StreamInput in) throws IOException { + super(in); + method = in.readEnum(RestRequest.Method.class); + path = in.readString(); + params = in.readMap(StreamInput::readString, StreamInput::readString); + if (in.readBoolean()) { + xContentType = in.readEnum(XContentType.class); + } + content = in.readBytesReference(); + principalIdentifierToken = in.readString(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeEnum(method); + out.writeString(path); + out.writeMap(params, StreamOutput::writeString, StreamOutput::writeString); + out.writeBoolean(xContentType != null); + if (xContentType != null) { + out.writeEnum(xContentType); + } + out.writeBytesReference(content); + out.writeString(principalIdentifierToken); + } + + /** + * Gets the REST method + * + * @return This REST request {@link Method} type + */ + public Method method() { + return method; + } + + /** + * Gets the REST path + * + * @return This REST request's path + */ + public String path() { + return path; + } + + /** + * Gets the full map of params without consuming them. Rest Handlers should use {@link #param(String)} or {@link #param(String, String)} + * to get parameter values. + * + * @return This REST request's params + */ + public Map params() { + return params; + } + + /** + * Tests whether a parameter named {@code key} exists. + * + * @param key The parameter to test. + * @return True if there is a value for this parameter. + */ + public boolean hasParam(String key) { + return params.containsKey(key); + } + + /** + * Gets the value of a parameter, consuming it in the process. + * + * @param key The parameter key + * @return The parameter value if it exists, or null. + */ + public String param(String key) { + consumedParams.add(key); + return params.get(key); + } + + /** + * Gets the value of a parameter, consuming it in the process. + * + * @param key The parameter key + * @param defaultValue A value to return if the parameter value doesn't exist. + * @return The parameter value if it exists, or the default value. + */ + public String param(String key, String defaultValue) { + consumedParams.add(key); + return params.getOrDefault(key, defaultValue); + } + + /** + * Gets the value of a parameter as a long, consuming it in the process. + * + * @param key The parameter key + * @param defaultValue A value to return if the parameter value doesn't exist. + * @return The parameter value if it exists, or the default value. + */ + public long paramAsLong(String key, long defaultValue) { + String value = param(key); + if (value == null) { + return defaultValue; + } + try { + return Long.parseLong(value); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Unable to parse param '" + key + "' value '" + value + "' to a long.", e); + } + } + + /** + * Returns parameters consumed by {@link #param(String)} or {@link #param(String, String)}. + * + * @return a list of consumed parameters. + */ + public List consumedParams() { + return new ArrayList<>(consumedParams); + } + + /** + * Gets the content type, if any. + * + * @return the content type of the {@link #content()}, or null if the context is plain text or if there is no content. + */ + public XContentType getXContentType() { + return xContentType; + } + + /** + * Gets the content. + * + * @return This REST request's content + */ + public BytesReference content() { + contentConsumed = true; + return content; + } + + /** + * Tests whether content exists. + * + * @return True if there is non-empty content. + */ + public boolean hasContent() { + return content.length() > 0; + } + + /** + * Tests whether content has been consumed. + * + * @return True if the content was consumed. + */ + public boolean isContentConsumed() { + return contentConsumed; + } + + /** + * Gets a parser for the contents of this request if there is content and an xContentType. + * + * @param xContentRegistry The extension's xContentRegistry + * @return A parser for the given content and content type. + * @throws OpenSearchParseException on missing body or xContentType. + * @throws IOException on a failure creating the parser. + */ + public final XContentParser contentParser(NamedXContentRegistry xContentRegistry) throws IOException { + if (!hasContent() || getXContentType() == null) { + throw new OpenSearchParseException("There is no request body or the ContentType is invalid."); + } + return getXContentType().xContent().createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput()); + } + + /** + * @return This REST request issuer's identity token + */ + public String getRequestIssuerIdentity() { + return principalIdentifierToken; + } + + @Override + public String toString() { + return "ExtensionRestRequest{method=" + + method + + ", path=" + + path + + ", params=" + + params + + ", xContentType=" + + xContentType + + ", contentLength=" + + content.length() + + ", requester=" + + principalIdentifierToken + + "}"; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + ExtensionRestRequest that = (ExtensionRestRequest) obj; + return Objects.equals(method, that.method) + && Objects.equals(path, that.path) + && Objects.equals(params, that.params) + && Objects.equals(xContentType, that.xContentType) + && Objects.equals(content, that.content) + && Objects.equals(principalIdentifierToken, that.principalIdentifierToken); + } + + @Override + public int hashCode() { + return Objects.hash(method, path, params, xContentType, content, principalIdentifierToken); + } +} diff --git a/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestResponse.java b/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestResponse.java new file mode 100644 index 0000000000000..0eb59823bee93 --- /dev/null +++ b/server/src/main/java/org/opensearch/extensions/rest/ExtensionRestResponse.java @@ -0,0 +1,113 @@ +/* + * 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.common.bytes.BytesReference; +import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestStatus; + +import java.util.List; + +/** + * A subclass of {@link BytesRestResponse} which also tracks consumed parameters and content. + * + * @opensearch.api + */ +public class ExtensionRestResponse extends BytesRestResponse { + + private final List consumedParams; + private final boolean contentConsumed; + + /** + * Creates a new response based on {@link XContentBuilder}. + * + * @param request the REST request being responded to. + * @param status The REST status. + * @param builder The builder for the response. + */ + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, XContentBuilder builder) { + super(status, builder); + this.consumedParams = request.consumedParams(); + this.contentConsumed = request.isContentConsumed(); + } + + /** + * Creates a new plain text response. + * + * @param request the REST request being responded to. + * @param status The REST status. + * @param content A plain text response string. + */ + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String content) { + super(status, content); + this.consumedParams = request.consumedParams(); + this.contentConsumed = request.isContentConsumed(); + } + + /** + * Creates a new plain text response. + * + * @param request the REST request being responded to. + * @param status The REST status. + * @param contentType The content type of the response string. + * @param content A response string. + */ + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String contentType, String content) { + super(status, contentType, content); + this.consumedParams = request.consumedParams(); + this.contentConsumed = request.isContentConsumed(); + } + + /** + * Creates a binary response. + * + * @param request the REST request being responded to. + * @param status The REST status. + * @param contentType The content type of the response bytes. + * @param content Response bytes. + */ + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String contentType, byte[] content) { + super(status, contentType, content); + this.consumedParams = request.consumedParams(); + this.contentConsumed = request.isContentConsumed(); + } + + /** + * Creates a binary response. + * + * @param request the REST request being responded to. + * @param status The REST status. + * @param contentType The content type of the response bytes. + * @param content Response bytes. + */ + public ExtensionRestResponse(ExtensionRestRequest request, RestStatus status, String contentType, BytesReference content) { + super(status, contentType, content); + this.consumedParams = request.consumedParams(); + this.contentConsumed = request.isContentConsumed(); + } + + /** + * Gets the list of consumed parameters. These are needed to consume the parameters of the original request. + * + * @return the list of consumed params. + */ + public List getConsumedParams() { + return consumedParams; + } + + /** + * Reports whether content was consumed. + * + * @return true if the content was consumed, false otherwise. + */ + public boolean isContentConsumed() { + return contentConsumed; + } +} 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 8c935b1dc87a6..790beaef0a969 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestActionsRequestHandler.java @@ -8,8 +8,8 @@ package org.opensearch.extensions.rest; +import org.opensearch.extensions.AcknowledgedResponse; import org.opensearch.extensions.DiscoveryExtensionNode; -import org.opensearch.extensions.ExtensionStringResponse; import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; import org.opensearch.transport.TransportResponse; @@ -49,15 +49,13 @@ public RestActionsRequestHandler( * Handles a {@link RegisterRestActionsRequest}. * * @param restActionsRequest The request to handle. - * @return A {@link ExtensionStringResponse} indicating success. + * @return A {@link AcknowledgedResponse} indicating success. * @throws Exception if the request is not handled properly. */ public TransportResponse handleRegisterRestActionsRequest(RegisterRestActionsRequest restActionsRequest) throws Exception { DiscoveryExtensionNode discoveryExtensionNode = extensionIdMap.get(restActionsRequest.getUniqueId()); RestHandler handler = new RestSendToExtensionAction(restActionsRequest, discoveryExtensionNode, transportService); restController.registerHandler(handler); - return new ExtensionStringResponse( - "Registered extension " + restActionsRequest.getUniqueId() + " to handle REST Actions " + restActionsRequest.getRestActions() - ); + return new AcknowledgedResponse(true); } } diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestExecuteOnExtensionRequest.java b/server/src/main/java/org/opensearch/extensions/rest/RestExecuteOnExtensionRequest.java deleted file mode 100644 index 128dad2645b42..0000000000000 --- a/server/src/main/java/org/opensearch/extensions/rest/RestExecuteOnExtensionRequest.java +++ /dev/null @@ -1,77 +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. - */ - -package org.opensearch.extensions.rest; - -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.rest.RestRequest; -import org.opensearch.rest.RestRequest.Method; -import org.opensearch.transport.TransportRequest; - -import java.io.IOException; -import java.util.Objects; - -/** - * Request to execute REST actions on extension node - * - * @opensearch.internal - */ -public class RestExecuteOnExtensionRequest extends TransportRequest { - - private Method method; - private String uri; - - public RestExecuteOnExtensionRequest(Method method, String uri) { - this.method = method; - this.uri = uri; - } - - public RestExecuteOnExtensionRequest(StreamInput in) throws IOException { - super(in); - try { - method = RestRequest.Method.valueOf(in.readString()); - } catch (IllegalArgumentException e) { - throw new IOException(e); - } - uri = in.readString(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeString(method.name()); - out.writeString(uri); - } - - public Method getMethod() { - return method; - } - - public String getUri() { - return uri; - } - - @Override - public String toString() { - return "RestExecuteOnExtensionRequest{method=" + method + ", uri=" + uri + "}"; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null || getClass() != obj.getClass()) return false; - RestExecuteOnExtensionRequest that = (RestExecuteOnExtensionRequest) obj; - return Objects.equals(method, that.method) && Objects.equals(uri, that.uri); - } - - @Override - public int hashCode() { - return Objects.hash(method, uri); - } -} diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestExecuteOnExtensionResponse.java b/server/src/main/java/org/opensearch/extensions/rest/RestExecuteOnExtensionResponse.java index b7d7aae3faaab..e2625105e705c 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestExecuteOnExtensionResponse.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestExecuteOnExtensionResponse.java @@ -10,14 +10,11 @@ import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestResponse; import org.opensearch.rest.RestStatus; import org.opensearch.transport.TransportResponse; import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.Collections; import java.util.List; import java.util.Map; @@ -27,20 +24,13 @@ * @opensearch.internal */ public class RestExecuteOnExtensionResponse extends TransportResponse { + private RestStatus status; private String contentType; private byte[] content; private Map> headers; - - /** - * Instantiate this object with a status and response string. - * - * @param status The REST status. - * @param responseString The response content as a String. - */ - public RestExecuteOnExtensionResponse(RestStatus status, String responseString) { - this(status, BytesRestResponse.TEXT_CONTENT_TYPE, responseString.getBytes(StandardCharsets.UTF_8), Collections.emptyMap()); - } + private List consumedParams; + private boolean contentConsumed; /** * Instantiate this object with the components of a {@link RestResponse}. @@ -49,33 +39,49 @@ public RestExecuteOnExtensionResponse(RestStatus status, String responseString) * @param contentType The type of the content. * @param content The content. * @param headers The headers. + * @param consumedParams The consumed params. + * @param contentConsumed Whether content was consumed. */ - public RestExecuteOnExtensionResponse(RestStatus status, String contentType, byte[] content, Map> headers) { + public RestExecuteOnExtensionResponse( + RestStatus status, + String contentType, + byte[] content, + Map> headers, + List consumedParams, + boolean contentConsumed + ) { + super(); setStatus(status); setContentType(contentType); setContent(content); setHeaders(headers); + setConsumedParams(consumedParams); + setContentConsumed(contentConsumed); } /** - * Instantiate this object from a Transport Stream + * Instantiate this object from a Transport Stream. * * @param in The stream input. * @throws IOException on transport failure. */ public RestExecuteOnExtensionResponse(StreamInput in) throws IOException { - setStatus(RestStatus.readFrom(in)); + setStatus(in.readEnum(RestStatus.class)); setContentType(in.readString()); setContent(in.readByteArray()); setHeaders(in.readMapOfLists(StreamInput::readString, StreamInput::readString)); + setConsumedParams(in.readStringList()); + setContentConsumed(in.readBoolean()); } @Override public void writeTo(StreamOutput out) throws IOException { - RestStatus.writeTo(out, status); + out.writeEnum(status); out.writeString(contentType); out.writeByteArray(content); out.writeMapOfLists(headers, StreamOutput::writeString, StreamOutput::writeString); + out.writeStringCollection(consumedParams); + out.writeBoolean(contentConsumed); } public RestStatus getStatus() { @@ -109,4 +115,20 @@ public Map> getHeaders() { public void setHeaders(Map> headers) { this.headers = Map.copyOf(headers); } + + public List getConsumedParams() { + return consumedParams; + } + + public void setConsumedParams(List consumedParams) { + this.consumedParams = consumedParams; + } + + public boolean isContentConsumed() { + return contentConsumed; + } + + public void setContentConsumed(boolean contentConsumed) { + this.contentConsumed = contentConsumed; + } } diff --git a/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java b/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java index 9fd0bd73f5e5f..38e92ed604a09 100644 --- a/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java +++ b/server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java @@ -11,7 +11,9 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.client.node.NodeClient; +import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.extensions.DiscoveryExtensionNode; import org.opensearch.extensions.ExtensionsManager; import org.opensearch.rest.BaseRestHandler; @@ -26,14 +28,13 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.security.Principal; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Map.Entry; -import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; - +import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableList; @@ -44,17 +45,23 @@ public class RestSendToExtensionAction extends BaseRestHandler { private static final String SEND_TO_EXTENSION_ACTION = "send_to_extension_action"; private static final Logger logger = LogManager.getLogger(RestSendToExtensionAction.class); - private static final String CONSUMED_PARAMS_KEY = "extension.consumed.parameters"; + // To replace with user identity see https://github.com/opensearch-project/OpenSearch/pull/4247 + private static final Principal DEFAULT_PRINCIPAL = new Principal() { + @Override + public String getName() { + return "OpenSearchUser"; + } + }; private final List routes; - private final String uriPrefix; + private final String pathPrefix; private final DiscoveryExtensionNode discoveryExtensionNode; private final TransportService transportService; /** * Instantiates this object using a {@link RegisterRestActionsRequest} to populate the routes. * - * @param restActionsRequest A request encapsulating a list of Strings with the API methods and URIs. + * @param restActionsRequest A request encapsulating a list of Strings with the API methods and paths. * @param transportService The OpenSearch transport service * @param discoveryExtensionNode The extension node to which to send actions */ @@ -63,20 +70,20 @@ public RestSendToExtensionAction( DiscoveryExtensionNode discoveryExtensionNode, TransportService transportService ) { - this.uriPrefix = "/_extensions/_" + restActionsRequest.getUniqueId(); + this.pathPrefix = "/_extensions/_" + restActionsRequest.getUniqueId(); List restActionsAsRoutes = new ArrayList<>(); for (String restAction : restActionsRequest.getRestActions()) { RestRequest.Method method; - String uri; + String path; try { int delim = restAction.indexOf(' '); method = RestRequest.Method.valueOf(restAction.substring(0, delim)); - uri = uriPrefix + restAction.substring(delim).trim(); + path = pathPrefix + restAction.substring(delim).trim(); } catch (IndexOutOfBoundsException | IllegalArgumentException e) { throw new IllegalArgumentException(restAction + " does not begin with a valid REST method"); } - logger.info("Registering: " + method + " " + uri); - restActionsAsRoutes.add(new Route(method, uri)); + logger.info("Registering: " + method + " " + path); + restActionsAsRoutes.add(new Route(method, path)); } this.routes = unmodifiableList(restActionsAsRoutes); this.discoveryExtensionNode = discoveryExtensionNode; @@ -95,21 +102,27 @@ public List routes() { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - Method method = request.getHttpRequest().method(); - String uri = request.getHttpRequest().uri(); - if (uri.startsWith(uriPrefix)) { - uri = uri.substring(uriPrefix.length()); + Method method = request.method(); + String path = request.path(); + Map params = request.params(); + XContentType contentType = request.getXContentType(); + BytesReference content = request.content(); + + if (path.startsWith(pathPrefix)) { + path = path.substring(pathPrefix.length()); } - String message = "Forwarding the request " + method + " " + uri + " to " + discoveryExtensionNode; + String message = "Forwarding the request " + method + " " + path + " to " + discoveryExtensionNode; logger.info(message); // Initialize response. Values will be changed in the handler. final RestExecuteOnExtensionResponse restExecuteOnExtensionResponse = new RestExecuteOnExtensionResponse( RestStatus.INTERNAL_SERVER_ERROR, BytesRestResponse.TEXT_CONTENT_TYPE, message.getBytes(StandardCharsets.UTF_8), - emptyMap() + emptyMap(), + emptyList(), + false ); - final CompletableFuture inProgressFuture = new CompletableFuture<>(); + final CountDownLatch inProgressLatch = new CountDownLatch(1); final TransportResponseHandler restExecuteOnExtensionResponseHandler = new TransportResponseHandler< RestExecuteOnExtensionResponse>() { @@ -124,27 +137,21 @@ public void handleResponse(RestExecuteOnExtensionResponse response) { restExecuteOnExtensionResponse.setStatus(response.getStatus()); restExecuteOnExtensionResponse.setContentType(response.getContentType()); restExecuteOnExtensionResponse.setContent(response.getContent()); - // Extract the consumed parameters from the header - Map> headers = response.getHeaders(); - List consumedParams = headers.get(CONSUMED_PARAMS_KEY); - if (consumedParams != null) { - consumedParams.stream().forEach(p -> request.param(p)); + restExecuteOnExtensionResponse.setHeaders(response.getHeaders()); + // Consume parameters and content + response.getConsumedParams().stream().forEach(p -> request.param(p)); + if (response.isContentConsumed()) { + request.content(); } - Map> headersWithoutConsumedParams = headers.entrySet() - .stream() - .filter(e -> !e.getKey().equals(CONSUMED_PARAMS_KEY)) - .collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue())); - restExecuteOnExtensionResponse.setHeaders(headersWithoutConsumedParams); - inProgressFuture.complete(response); } @Override public void handleException(TransportException exp) { - logger.error("REST request failed", exp); + logger.debug("REST request failed", exp); // Status is already defaulted to 500 (INTERNAL_SERVER_ERROR) byte[] responseBytes = ("Request failed: " + exp.getMessage()).getBytes(StandardCharsets.UTF_8); restExecuteOnExtensionResponse.setContent(responseBytes); - inProgressFuture.completeExceptionally(exp); + inProgressLatch.countDown(); } @Override @@ -153,17 +160,20 @@ public String executor() { } }; try { + // Will be replaced with ExtensionTokenProcessor and PrincipalIdentifierToken classes from feature/identity + final String extensionTokenProcessor = "placeholder_token_processor"; + final String requestIssuerIdentity = "placeholder_request_issuer_identity"; + transportService.sendRequest( discoveryExtensionNode, ExtensionsManager.REQUEST_REST_EXECUTE_ON_EXTENSION_ACTION, // HERE BE DRAGONS - DO NOT INCLUDE HEADERS // SEE https://github.com/opensearch-project/OpenSearch/issues/4429 - new RestExecuteOnExtensionRequest(method, uri), + new ExtensionRestRequest(method, path, params, contentType, content, requestIssuerIdentity), restExecuteOnExtensionResponseHandler ); try { - // TODO: make asynchronous - inProgressFuture.get(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS); + inProgressLatch.await(5, TimeUnit.SECONDS); } catch (InterruptedException e) { return channel -> channel.sendResponse( new BytesRestResponse(RestStatus.REQUEST_TIMEOUT, "No response from extension to request.") @@ -177,11 +187,11 @@ public String executor() { restExecuteOnExtensionResponse.getContentType(), restExecuteOnExtensionResponse.getContent() ); - for (Entry> headerEntry : restExecuteOnExtensionResponse.getHeaders().entrySet()) { - for (String value : headerEntry.getValue()) { - restResponse.addHeader(headerEntry.getKey(), value); - } - } + // No constructor that includes headers so we roll our own + restExecuteOnExtensionResponse.getHeaders() + .entrySet() + .stream() + .forEach(e -> { e.getValue().stream().forEach(v -> restResponse.addHeader(e.getKey(), v)); }); return channel -> channel.sendResponse(restResponse); } diff --git a/server/src/main/java/org/opensearch/extensions/settings/CustomSettingsRequestHandler.java b/server/src/main/java/org/opensearch/extensions/settings/CustomSettingsRequestHandler.java index 83d34facb35f6..980dcf67c3128 100644 --- a/server/src/main/java/org/opensearch/extensions/settings/CustomSettingsRequestHandler.java +++ b/server/src/main/java/org/opensearch/extensions/settings/CustomSettingsRequestHandler.java @@ -10,7 +10,7 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.SettingsModule; -import org.opensearch.extensions.ExtensionStringResponse; +import org.opensearch.extensions.AcknowledgedResponse; import org.opensearch.transport.TransportResponse; import java.util.ArrayList; @@ -38,7 +38,7 @@ public CustomSettingsRequestHandler(SettingsModule settingsModule) { * Handles a {@link RegisterCustomSettingsRequest}. * * @param customSettingsRequest The request to handle. - * @return A {@link ExtensionStringResponse} indicating success. + * @return A {@link AcknowledgedResponse} indicating success. * @throws Exception if the request is not handled properly. */ public TransportResponse handleRegisterCustomSettingsRequest(RegisterCustomSettingsRequest customSettingsRequest) throws Exception { @@ -50,8 +50,6 @@ public TransportResponse handleRegisterCustomSettingsRequest(RegisterCustomSetti settingsModule.registerDynamicSetting(setting); registeredCustomSettings.add(setting.getKey()); } - return new ExtensionStringResponse( - "Registered settings from extension " + customSettingsRequest.getUniqueId() + ": " + String.join(", ", registeredCustomSettings) - ); + return new AcknowledgedResponse(true); } } diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionResponseTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionResponseTests.java index 3b2993cb164c0..de8c677e5bcc6 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionResponseTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionResponseTests.java @@ -31,21 +31,4 @@ public void testAcknowledgedResponse() throws Exception { } } } - - public void testExtensionStringResponse() throws Exception { - String response = "This is a response"; - ExtensionStringResponse stringResponse = new ExtensionStringResponse(response); - - assertEquals(response, stringResponse.getResponse()); - - try (BytesStreamOutput out = new BytesStreamOutput()) { - stringResponse.writeTo(out); - out.flush(); - try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { - stringResponse = new ExtensionStringResponse(in); - - assertEquals(response, stringResponse.getResponse()); - } - } - } } diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index a216b4926c066..47820ee739c49 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -459,9 +459,8 @@ public void testHandleRegisterRestActionsRequest() throws Exception { RegisterRestActionsRequest registerActionsRequest = new RegisterRestActionsRequest(uniqueIdStr, actionsList); TransportResponse response = extensionsManager.getRestActionsRequestHandler() .handleRegisterRestActionsRequest(registerActionsRequest); - assertEquals(ExtensionStringResponse.class, response.getClass()); - assertTrue(((ExtensionStringResponse) response).getResponse().contains(uniqueIdStr)); - assertTrue(((ExtensionStringResponse) response).getResponse().contains(actionsList.toString())); + assertEquals(AcknowledgedResponse.class, response.getClass()); + assertTrue(((AcknowledgedResponse) response).getStatus()); } public void testHandleRegisterSettingsRequest() throws Exception { @@ -477,10 +476,8 @@ public void testHandleRegisterSettingsRequest() throws Exception { RegisterCustomSettingsRequest registerCustomSettingsRequest = new RegisterCustomSettingsRequest(uniqueIdStr, settingsList); TransportResponse response = extensionsManager.getCustomSettingsRequestHandler() .handleRegisterCustomSettingsRequest(registerCustomSettingsRequest); - assertEquals(ExtensionStringResponse.class, response.getClass()); - assertTrue(((ExtensionStringResponse) response).getResponse().contains(uniqueIdStr)); - assertTrue(((ExtensionStringResponse) response).getResponse().contains("falseSetting")); - assertTrue(((ExtensionStringResponse) response).getResponse().contains("fooSetting")); + assertEquals(AcknowledgedResponse.class, response.getClass()); + assertTrue(((AcknowledgedResponse) response).getStatus()); } public void testHandleRegisterRestActionsRequestWithInvalidMethod() throws Exception { diff --git a/server/src/test/java/org/opensearch/extensions/rest/ExtensionRestRequestTests.java b/server/src/test/java/org/opensearch/extensions/rest/ExtensionRestRequestTests.java new file mode 100644 index 0000000000000..9f09735dbe38d --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/rest/ExtensionRestRequestTests.java @@ -0,0 +1,262 @@ +/* + * 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.rest.RestStatus; +import org.opensearch.OpenSearchParseException; +import org.opensearch.common.bytes.BytesArray; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.io.stream.BytesStreamInput; +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.io.stream.NamedWriteableRegistry; +import org.opensearch.common.xcontent.NamedXContentRegistry; +import org.opensearch.common.xcontent.XContentParser; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.common.io.stream.NamedWriteableAwareStreamInput; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestRequest.Method; +import org.opensearch.test.OpenSearchTestCase; + +import java.nio.charset.StandardCharsets; +import java.security.Principal; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import static java.util.Map.entry; + +public class ExtensionRestRequestTests extends OpenSearchTestCase { + + private Method expectedMethod; + private String expectedPath; + Map expectedParams; + XContentType expectedContentType; + BytesReference expectedContent; + String extensionUniqueId1; + Principal userPrincipal; + // Will be replaced with ExtensionTokenProcessor and PrincipalIdentifierToken classes from feature/identity + String extensionTokenProcessor; + String expectedRequestIssuerIdentity; + NamedWriteableRegistry registry; + + public void setUp() throws Exception { + super.setUp(); + expectedMethod = Method.GET; + expectedPath = "/test/uri"; + expectedParams = Map.ofEntries(entry("foo", "bar"), entry("baz", "42")); + expectedContentType = XContentType.JSON; + expectedContent = new BytesArray("{\"key\": \"value\"}".getBytes(StandardCharsets.UTF_8)); + extensionUniqueId1 = "ext_1"; + userPrincipal = () -> "user1"; + extensionTokenProcessor = "placeholder_extension_token_processor"; + expectedRequestIssuerIdentity = "placeholder_request_issuer_identity"; + } + + public void testExtensionRestRequest() throws Exception { + ExtensionRestRequest request = new ExtensionRestRequest( + expectedMethod, + expectedPath, + expectedParams, + expectedContentType, + expectedContent, + expectedRequestIssuerIdentity + ); + + assertEquals(expectedMethod, request.method()); + assertEquals(expectedPath, request.path()); + + assertEquals(expectedParams, request.params()); + assertEquals(Collections.emptyList(), request.consumedParams()); + assertTrue(request.hasParam("foo")); + assertFalse(request.hasParam("bar")); + assertEquals("bar", request.param("foo")); + assertEquals("baz", request.param("bar", "baz")); + assertEquals(42L, request.paramAsLong("baz", 0L)); + assertEquals(0L, request.paramAsLong("bar", 0L)); + assertTrue(request.consumedParams().contains("foo")); + assertTrue(request.consumedParams().contains("baz")); + + assertEquals(expectedContentType, request.getXContentType()); + assertTrue(request.hasContent()); + assertFalse(request.isContentConsumed()); + assertEquals(expectedContent, request.content()); + assertTrue(request.isContentConsumed()); + + XContentParser parser = request.contentParser(NamedXContentRegistry.EMPTY); + Map contentMap = parser.mapStrings(); + assertEquals("value", contentMap.get("key")); + + assertEquals(expectedRequestIssuerIdentity, request.getRequestIssuerIdentity()); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + request.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + try (NamedWriteableAwareStreamInput nameWritableAwareIn = new NamedWriteableAwareStreamInput(in, registry)) { + request = new ExtensionRestRequest(nameWritableAwareIn); + assertEquals(expectedMethod, request.method()); + assertEquals(expectedPath, request.path()); + assertEquals(expectedParams, request.params()); + assertEquals(expectedContent, request.content()); + assertEquals(expectedRequestIssuerIdentity, request.getRequestIssuerIdentity()); + } + } + } + } + + public void testExtensionRestRequestWithNoContent() throws Exception { + ExtensionRestRequest request = new ExtensionRestRequest( + expectedMethod, + expectedPath, + expectedParams, + null, + new BytesArray(new byte[0]), + expectedRequestIssuerIdentity + ); + + assertEquals(expectedMethod, request.method()); + assertEquals(expectedPath, request.path()); + assertEquals(expectedParams, request.params()); + assertNull(request.getXContentType()); + assertEquals(0, request.content().length()); + assertEquals(expectedRequestIssuerIdentity, request.getRequestIssuerIdentity()); + + final ExtensionRestRequest requestWithNoContent = request; + assertThrows(OpenSearchParseException.class, () -> requestWithNoContent.contentParser(NamedXContentRegistry.EMPTY)); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + request.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + try (NamedWriteableAwareStreamInput nameWritableAwareIn = new NamedWriteableAwareStreamInput(in, registry)) { + request = new ExtensionRestRequest(nameWritableAwareIn); + assertEquals(expectedMethod, request.method()); + assertEquals(expectedPath, request.path()); + assertEquals(expectedParams, request.params()); + assertNull(request.getXContentType()); + assertEquals(0, request.content().length()); + assertEquals(expectedRequestIssuerIdentity, request.getRequestIssuerIdentity()); + + final ExtensionRestRequest requestWithNoContentType = request; + assertThrows(OpenSearchParseException.class, () -> requestWithNoContentType.contentParser(NamedXContentRegistry.EMPTY)); + } + } + } + } + + public void testExtensionRestRequestWithPlainTextContent() throws Exception { + BytesReference expectedText = new BytesArray("Plain text"); + + ExtensionRestRequest request = new ExtensionRestRequest( + expectedMethod, + expectedPath, + expectedParams, + null, + expectedText, + expectedRequestIssuerIdentity + ); + + assertEquals(expectedMethod, request.method()); + assertEquals(expectedPath, request.path()); + assertEquals(expectedParams, request.params()); + assertNull(request.getXContentType()); + assertEquals(expectedText, request.content()); + assertEquals(expectedRequestIssuerIdentity, request.getRequestIssuerIdentity()); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + request.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + try (NamedWriteableAwareStreamInput nameWritableAwareIn = new NamedWriteableAwareStreamInput(in, registry)) { + request = new ExtensionRestRequest(nameWritableAwareIn); + assertEquals(expectedMethod, request.method()); + assertEquals(expectedPath, request.path()); + assertEquals(expectedParams, request.params()); + assertNull(request.getXContentType()); + assertEquals(expectedText, request.content()); + assertEquals(expectedRequestIssuerIdentity, request.getRequestIssuerIdentity()); + } + } + } + } + + public void testRestExecuteOnExtensionResponse() throws Exception { + RestStatus expectedStatus = RestStatus.OK; + String expectedContentType = BytesRestResponse.TEXT_CONTENT_TYPE; + String expectedResponse = "Test response"; + byte[] expectedResponseBytes = expectedResponse.getBytes(StandardCharsets.UTF_8); + + RestExecuteOnExtensionResponse response = new RestExecuteOnExtensionResponse( + expectedStatus, + expectedContentType, + expectedResponseBytes, + Collections.emptyMap(), + Collections.emptyList(), + false + ); + + assertEquals(expectedStatus, response.getStatus()); + assertEquals(expectedContentType, response.getContentType()); + assertArrayEquals(expectedResponseBytes, response.getContent()); + assertEquals(0, response.getHeaders().size()); + assertEquals(0, response.getConsumedParams().size()); + assertFalse(response.isContentConsumed()); + + String headerKey = "foo"; + List headerValueList = List.of("bar", "baz"); + Map> expectedHeaders = Map.of(headerKey, headerValueList); + List expectedConsumedParams = List.of("foo", "bar"); + + response = new RestExecuteOnExtensionResponse( + expectedStatus, + expectedContentType, + expectedResponseBytes, + expectedHeaders, + expectedConsumedParams, + true + ); + + assertEquals(expectedStatus, response.getStatus()); + assertEquals(expectedContentType, response.getContentType()); + assertArrayEquals(expectedResponseBytes, response.getContent()); + + assertEquals(1, response.getHeaders().keySet().size()); + assertTrue(response.getHeaders().containsKey(headerKey)); + + List fooList = response.getHeaders().get(headerKey); + assertEquals(2, fooList.size()); + assertTrue(fooList.containsAll(headerValueList)); + + assertEquals(2, response.getConsumedParams().size()); + assertTrue(response.getConsumedParams().containsAll(expectedConsumedParams)); + assertTrue(response.isContentConsumed()); + + try (BytesStreamOutput out = new BytesStreamOutput()) { + response.writeTo(out); + out.flush(); + try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { + response = new RestExecuteOnExtensionResponse(in); + + assertEquals(expectedStatus, response.getStatus()); + assertEquals(expectedContentType, response.getContentType()); + assertArrayEquals(expectedResponseBytes, response.getContent()); + + assertEquals(1, response.getHeaders().keySet().size()); + assertTrue(response.getHeaders().containsKey(headerKey)); + + fooList = response.getHeaders().get(headerKey); + assertEquals(2, fooList.size()); + assertTrue(fooList.containsAll(headerValueList)); + + assertEquals(2, response.getConsumedParams().size()); + assertTrue(response.getConsumedParams().containsAll(expectedConsumedParams)); + assertTrue(response.isContentConsumed()); + } + } + } +} diff --git a/server/src/test/java/org/opensearch/extensions/rest/ExtensionRestResponseTests.java b/server/src/test/java/org/opensearch/extensions/rest/ExtensionRestResponseTests.java new file mode 100644 index 0000000000000..82ae61b02cb32 --- /dev/null +++ b/server/src/test/java/org/opensearch/extensions/rest/ExtensionRestResponseTests.java @@ -0,0 +1,132 @@ +/* + * 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 java.io.IOException; +import java.nio.ByteBuffer; +import java.util.Collections; + +import org.opensearch.common.bytes.BytesArray; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.rest.RestRequest.Method; +import org.opensearch.test.OpenSearchTestCase; + +import static org.opensearch.rest.BytesRestResponse.TEXT_CONTENT_TYPE; +import static org.opensearch.rest.RestStatus.ACCEPTED; +import static org.opensearch.rest.RestStatus.OK; + +public class ExtensionRestResponseTests extends OpenSearchTestCase { + + private static final String OCTET_CONTENT_TYPE = "application/octet-stream"; + private static final String JSON_CONTENT_TYPE = "application/json; charset=UTF-8"; + + private String testText; + private byte[] testBytes; + + @Override + public void setUp() throws Exception { + super.setUp(); + testText = "plain text"; + testBytes = new byte[] { 1, 2 }; + } + + private ExtensionRestRequest generateTestRequest() { + ExtensionRestRequest request = new ExtensionRestRequest( + Method.GET, + "/foo", + Collections.emptyMap(), + null, + new BytesArray("Text Content"), + null + ); + // consume params "foo" and "bar" + request.param("foo"); + request.param("bar"); + // consume content + request.content(); + return request; + } + + public void testConstructorWithBuilder() throws IOException { + XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent()); + builder.startObject(); + builder.field("status", ACCEPTED); + builder.endObject(); + ExtensionRestRequest request = generateTestRequest(); + ExtensionRestResponse response = new ExtensionRestResponse(request, OK, builder); + + assertEquals(OK, response.status()); + assertEquals(JSON_CONTENT_TYPE, response.contentType()); + assertEquals("{\"status\":\"ACCEPTED\"}", response.content().utf8ToString()); + for (String param : response.getConsumedParams()) { + assertTrue(request.consumedParams().contains(param)); + } + assertTrue(request.isContentConsumed()); + } + + public void testConstructorWithPlainText() { + ExtensionRestRequest request = generateTestRequest(); + ExtensionRestResponse response = new ExtensionRestResponse(request, OK, testText); + + assertEquals(OK, response.status()); + assertEquals(TEXT_CONTENT_TYPE, response.contentType()); + assertEquals(testText, response.content().utf8ToString()); + for (String param : response.getConsumedParams()) { + assertTrue(request.consumedParams().contains(param)); + } + assertTrue(request.isContentConsumed()); + } + + public void testConstructorWithText() { + ExtensionRestRequest request = generateTestRequest(); + ExtensionRestResponse response = new ExtensionRestResponse(request, OK, TEXT_CONTENT_TYPE, testText); + + assertEquals(OK, response.status()); + assertEquals(TEXT_CONTENT_TYPE, response.contentType()); + assertEquals(testText, response.content().utf8ToString()); + + for (String param : response.getConsumedParams()) { + assertTrue(request.consumedParams().contains(param)); + } + assertTrue(request.isContentConsumed()); + } + + public void testConstructorWithByteArray() { + ExtensionRestRequest request = generateTestRequest(); + ExtensionRestResponse response = new ExtensionRestResponse(request, OK, OCTET_CONTENT_TYPE, testBytes); + + assertEquals(OK, response.status()); + assertEquals(OCTET_CONTENT_TYPE, response.contentType()); + assertArrayEquals(testBytes, BytesReference.toBytes(response.content())); + for (String param : response.getConsumedParams()) { + assertTrue(request.consumedParams().contains(param)); + } + assertTrue(request.isContentConsumed()); + } + + public void testConstructorWithBytesReference() { + ExtensionRestRequest request = generateTestRequest(); + ExtensionRestResponse response = new ExtensionRestResponse( + request, + OK, + OCTET_CONTENT_TYPE, + BytesReference.fromByteBuffer(ByteBuffer.wrap(testBytes, 0, 2)) + ); + + assertEquals(OK, response.status()); + assertEquals(OCTET_CONTENT_TYPE, response.contentType()); + assertArrayEquals(testBytes, BytesReference.toBytes(response.content())); + for (String param : response.getConsumedParams()) { + assertTrue(request.consumedParams().contains(param)); + } + assertTrue(request.isContentConsumed()); + } +} diff --git a/server/src/test/java/org/opensearch/extensions/rest/RestExecuteOnExtensionTests.java b/server/src/test/java/org/opensearch/extensions/rest/RestExecuteOnExtensionTests.java deleted file mode 100644 index 98521ddcf1e26..0000000000000 --- a/server/src/test/java/org/opensearch/extensions/rest/RestExecuteOnExtensionTests.java +++ /dev/null @@ -1,94 +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. - */ - -package org.opensearch.extensions.rest; - -import org.opensearch.rest.RestStatus; -import org.opensearch.common.bytes.BytesReference; -import org.opensearch.common.io.stream.BytesStreamInput; -import org.opensearch.common.io.stream.BytesStreamOutput; -import org.opensearch.rest.BytesRestResponse; -import org.opensearch.rest.RestRequest.Method; -import org.opensearch.test.OpenSearchTestCase; - -import java.nio.charset.StandardCharsets; -import java.util.List; -import java.util.Map; - -public class RestExecuteOnExtensionTests extends OpenSearchTestCase { - - public void testRestExecuteOnExtensionRequest() throws Exception { - Method expectedMethod = Method.GET; - String expectedUri = "/test/uri"; - RestExecuteOnExtensionRequest request = new RestExecuteOnExtensionRequest(expectedMethod, expectedUri); - - assertEquals(expectedMethod, request.getMethod()); - assertEquals(expectedUri, request.getUri()); - - try (BytesStreamOutput out = new BytesStreamOutput()) { - request.writeTo(out); - out.flush(); - try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { - request = new RestExecuteOnExtensionRequest(in); - - assertEquals(expectedMethod, request.getMethod()); - assertEquals(expectedUri, request.getUri()); - } - } - } - - public void testRestExecuteOnExtensionResponse() throws Exception { - RestStatus expectedStatus = RestStatus.OK; - String expectedContentType = BytesRestResponse.TEXT_CONTENT_TYPE; - String expectedResponse = "Test response"; - byte[] expectedResponseBytes = expectedResponse.getBytes(StandardCharsets.UTF_8); - - RestExecuteOnExtensionResponse response = new RestExecuteOnExtensionResponse(expectedStatus, expectedResponse); - - assertEquals(expectedStatus, response.getStatus()); - assertEquals(expectedContentType, response.getContentType()); - assertArrayEquals(expectedResponseBytes, response.getContent()); - assertEquals(0, response.getHeaders().size()); - - String headerKey = "foo"; - List headerValueList = List.of("bar", "baz"); - Map> expectedHeaders = Map.of(headerKey, headerValueList); - - response = new RestExecuteOnExtensionResponse(expectedStatus, expectedContentType, expectedResponseBytes, expectedHeaders); - - assertEquals(expectedStatus, response.getStatus()); - assertEquals(expectedContentType, response.getContentType()); - assertArrayEquals(expectedResponseBytes, response.getContent()); - - assertEquals(1, expectedHeaders.keySet().size()); - assertTrue(expectedHeaders.containsKey(headerKey)); - - List fooList = expectedHeaders.get(headerKey); - assertEquals(2, fooList.size()); - assertTrue(fooList.containsAll(headerValueList)); - - try (BytesStreamOutput out = new BytesStreamOutput()) { - response.writeTo(out); - out.flush(); - try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { - response = new RestExecuteOnExtensionResponse(in); - - assertEquals(expectedStatus, response.getStatus()); - assertEquals(expectedContentType, response.getContentType()); - assertArrayEquals(expectedResponseBytes, response.getContent()); - - assertEquals(1, expectedHeaders.keySet().size()); - assertTrue(expectedHeaders.containsKey(headerKey)); - - fooList = expectedHeaders.get(headerKey); - assertEquals(2, fooList.size()); - assertTrue(fooList.containsAll(headerValueList)); - } - } - } -} From 5119e4dd1a036b00c24bdfcf6854dab47f7f9405 Mon Sep 17 00:00:00 2001 From: Ryan Bogan Date: Fri, 23 Dec 2022 17:10:00 +0000 Subject: [PATCH 10/10] Fix merge conflict Signed-off-by: Ryan Bogan --- CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a623ecc487c92..42cd569c9fb49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,11 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add feature flag for extensions ([#5211](https://github.com/opensearch-project/OpenSearch/pull/5211)) - Added jackson dependency to server ([#5366] (https://github.com/opensearch-project/OpenSearch/pull/5366)) - Adding support to register settings dynamically ([#5495](https://github.com/opensearch-project/OpenSearch/pull/5495)) -<<<<<<< HEAD - Added experimental support for extensions ([#5347](https://github.com/opensearch-project/OpenSearch/pull/5347)), ([#5518](https://github.com/opensearch-project/OpenSearch/pull/5518), ([#5597](https://github.com/opensearch-project/OpenSearch/pull/5597)), ([#5615](https://github.com/opensearch-project/OpenSearch/pull/5615))) -======= -- Added experimental support for extensions ([#5347](https://github.com/opensearch-project/OpenSearch/pull/5347)), ([#5518](https://github.com/opensearch-project/OpenSearch/pull/5518), ([#5597](https://github.com/opensearch-project/OpenSearch/pull/5597))) ->>>>>>> main - Add CI bundle pattern to distribution download ([#5348](https://github.com/opensearch-project/OpenSearch/pull/5348)) - Add support for ppc64le architecture ([#5459](https://github.com/opensearch-project/OpenSearch/pull/5459))