From ad8e5acb16f2bfd3365eacaf779eefc36dc155eb Mon Sep 17 00:00:00 2001 From: Sarat Vemulapalli Date: Thu, 20 Apr 2023 18:14:57 -0700 Subject: [PATCH] [Extensions] Moving ExtensionsRequest to use Protobuf (#6960) --- CHANGELOG.md | 1 + buildSrc/version.properties | 1 + modules/transport-netty4/build.gradle | 8 --- plugins/repository-gcs/build.gradle | 13 ++-- .../protobuf-java-util-3.22.2.jar.sha1 | 1 - plugins/repository-hdfs/build.gradle | 14 ---- .../licenses/protobuf-java-3.22.3.jar.sha1 | 1 - .../licenses/protobuf-java-LICENSE.txt | 10 --- .../licenses/protobuf-java-NOTICE.txt | 2 - plugins/transport-nio/build.gradle | 8 --- protobuf-java-NOTICE.txt | 0 server/build.gradle | 66 +++++++++++++++++-- .../licenses/protobuf-java-3.22.2.jar.sha1 | 0 .../licenses/protobuf-java-LICENSE.txt | 0 .../licenses/protobuf-java-NOTICE.txt | 0 .../extensions/ExtensionRequest.java | 38 +++++------ .../extensions/ExtensionsManager.java | 27 +------- .../extensions/ExtensionRequestProto.proto | 32 +++++++++ .../allocation/BalanceConfigurationTests.java | 2 +- .../extensions/ExtensionsManagerTests.java | 20 +++--- 20 files changed, 132 insertions(+), 112 deletions(-) delete mode 100644 plugins/repository-gcs/licenses/protobuf-java-util-3.22.2.jar.sha1 delete mode 100644 plugins/repository-hdfs/licenses/protobuf-java-3.22.3.jar.sha1 delete mode 100644 plugins/repository-hdfs/licenses/protobuf-java-LICENSE.txt delete mode 100644 plugins/repository-hdfs/licenses/protobuf-java-NOTICE.txt create mode 100644 protobuf-java-NOTICE.txt rename {plugins/repository-gcs => server}/licenses/protobuf-java-3.22.2.jar.sha1 (100%) rename plugins/repository-gcs/licenses/protobuf-LICENSE.txt => server/licenses/protobuf-java-LICENSE.txt (100%) rename plugins/repository-gcs/licenses/protobuf-NOTICE.txt => server/licenses/protobuf-java-NOTICE.txt (100%) create mode 100644 server/src/main/proto/extensions/ExtensionRequestProto.proto diff --git a/CHANGELOG.md b/CHANGELOG.md index c407c632fecfb..79ee28aaf7d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased 2.x] ### Added +- [Extensions] Moving Extensions APIs to protobuf serialization. ([#6960](https://github.com/opensearch-project/OpenSearch/pull/6960)) ### Dependencies diff --git a/buildSrc/version.properties b/buildSrc/version.properties index 1562a5aecfd35..fc8d64f6f07dc 100644 --- a/buildSrc/version.properties +++ b/buildSrc/version.properties @@ -22,6 +22,7 @@ woodstox = 6.4.0 kotlin = 1.7.10 antlr4 = 4.11.1 guava = 31.1-jre +protobuf = 3.22.2 # when updating the JNA version, also update the version in buildSrc/build.gradle jna = 5.5.0 diff --git a/modules/transport-netty4/build.gradle b/modules/transport-netty4/build.gradle index 4c245ec517bbd..ba3ea044139a8 100644 --- a/modules/transport-netty4/build.gradle +++ b/modules/transport-netty4/build.gradle @@ -132,12 +132,6 @@ thirdPartyAudit { 'com.aayushatharva.brotli4j.encoder.Encoder$Parameters', // classes are missing - // from io.netty.handler.codec.protobuf.ProtobufDecoder (netty) - 'com.google.protobuf.ExtensionRegistry', - 'com.google.protobuf.MessageLite$Builder', - 'com.google.protobuf.MessageLite', - 'com.google.protobuf.Parser', - // from io.netty.logging.CommonsLoggerFactory (netty) 'org.apache.commons.logging.Log', 'org.apache.commons.logging.LogFactory', @@ -192,8 +186,6 @@ thirdPartyAudit { 'org.slf4j.spi.LocationAwareLogger', 'com.github.luben.zstd.Zstd', - 'com.google.protobuf.ExtensionRegistryLite', - 'com.google.protobuf.MessageLiteOrBuilder', 'com.google.protobuf.nano.CodedOutputByteBufferNano', 'com.google.protobuf.nano.MessageNano', 'com.jcraft.jzlib.Deflater', diff --git a/plugins/repository-gcs/build.gradle b/plugins/repository-gcs/build.gradle index 5ef92e2ca2465..8ed235af6cab9 100644 --- a/plugins/repository-gcs/build.gradle +++ b/plugins/repository-gcs/build.gradle @@ -65,8 +65,6 @@ dependencies { api 'com.google.api:api-common:1.8.1' api 'com.google.api:gax:2.17.0' api 'org.threeten:threetenbp:1.4.4' - api 'com.google.protobuf:protobuf-java-util:3.22.2' - api 'com.google.protobuf:protobuf-java:3.22.2' api 'com.google.code.gson:gson:2.9.0' api 'com.google.api.grpc:proto-google-common-protos:2.10.0' api 'com.google.api.grpc:proto-google-iam-v1:0.12.0' @@ -105,13 +103,6 @@ tasks.named("dependencyLicenses").configure { thirdPartyAudit { ignoreViolations( // uses internal java api: sun.misc.Unsafe - 'com.google.protobuf.UnsafeUtil', - 'com.google.protobuf.UnsafeUtil$1', - 'com.google.protobuf.UnsafeUtil$JvmMemoryAccessor', - 'com.google.protobuf.UnsafeUtil$MemoryAccessor', - 'com.google.protobuf.MessageSchema', - 'com.google.protobuf.UnsafeUtil$Android32MemoryAccessor', - 'com.google.protobuf.UnsafeUtil$Android64MemoryAccessor', 'com.google.common.cache.Striped64', 'com.google.common.cache.Striped64$1', 'com.google.common.cache.Striped64$Cell', @@ -151,6 +142,10 @@ thirdPartyAudit { 'com.google.appengine.api.urlfetch.URLFetchService', 'com.google.appengine.api.urlfetch.URLFetchServiceFactory', 'com.oracle.svm.core.configure.ResourcesRegistry', + 'com.google.protobuf.util.JsonFormat', + 'com.google.protobuf.util.JsonFormat$Parser', + 'com.google.protobuf.util.JsonFormat$Printer', + 'com.google.protobuf.util.Timestamps', // commons-logging optional dependencies 'org.apache.avalon.framework.logger.Logger', 'org.apache.log.Hierarchy', diff --git a/plugins/repository-gcs/licenses/protobuf-java-util-3.22.2.jar.sha1 b/plugins/repository-gcs/licenses/protobuf-java-util-3.22.2.jar.sha1 deleted file mode 100644 index 2a5707254b8cf..0000000000000 --- a/plugins/repository-gcs/licenses/protobuf-java-util-3.22.2.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -749cd4fe8ab52f37bc186193802ba19f5b284647 \ No newline at end of file diff --git a/plugins/repository-hdfs/build.gradle b/plugins/repository-hdfs/build.gradle index c1aaa184d59db..e2f95fec52d43 100644 --- a/plugins/repository-hdfs/build.gradle +++ b/plugins/repository-hdfs/build.gradle @@ -69,7 +69,6 @@ dependencies { api 'org.apache.avro:avro:1.11.1' api 'com.google.code.gson:gson:2.10.1' runtimeOnly "com.google.guava:guava:${versions.guava}" - api 'com.google.protobuf:protobuf-java:3.22.3' api "commons-logging:commons-logging:${versions.commonslogging}" api 'commons-cli:commons-cli:1.5.0' api "commons-codec:commons-codec:${versions.commonscodec}" @@ -114,19 +113,6 @@ tasks.named("dependencyLicenses").configure { mapping from: /hadoop-.*/, to: 'hadoop' } -thirdPartyAudit { - ignoreViolations( - // uses internal java api: sun.misc.Unsafe - 'com.google.protobuf.MessageSchema', - 'com.google.protobuf.UnsafeUtil', - 'com.google.protobuf.UnsafeUtil$1', - 'com.google.protobuf.UnsafeUtil$Android32MemoryAccessor', - 'com.google.protobuf.UnsafeUtil$Android64MemoryAccessor', - 'com.google.protobuf.UnsafeUtil$JvmMemoryAccessor', - 'com.google.protobuf.UnsafeUtil$MemoryAccessor' - ) -} - tasks.named("integTest").configure { it.dependsOn(project.tasks.named("bundlePlugin")) } diff --git a/plugins/repository-hdfs/licenses/protobuf-java-3.22.3.jar.sha1 b/plugins/repository-hdfs/licenses/protobuf-java-3.22.3.jar.sha1 deleted file mode 100644 index 80feeec023e7b..0000000000000 --- a/plugins/repository-hdfs/licenses/protobuf-java-3.22.3.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -fdee98b8f6abab73f146a4edb4c09e56f8278d03 \ No newline at end of file diff --git a/plugins/repository-hdfs/licenses/protobuf-java-LICENSE.txt b/plugins/repository-hdfs/licenses/protobuf-java-LICENSE.txt deleted file mode 100644 index 49e7019ac5a1b..0000000000000 --- a/plugins/repository-hdfs/licenses/protobuf-java-LICENSE.txt +++ /dev/null @@ -1,10 +0,0 @@ -Copyright (c) , -All rights reserved. - -Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: - -1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. - -2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. - -THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/plugins/repository-hdfs/licenses/protobuf-java-NOTICE.txt b/plugins/repository-hdfs/licenses/protobuf-java-NOTICE.txt deleted file mode 100644 index 139597f9cb07c..0000000000000 --- a/plugins/repository-hdfs/licenses/protobuf-java-NOTICE.txt +++ /dev/null @@ -1,2 +0,0 @@ - - diff --git a/plugins/transport-nio/build.gradle b/plugins/transport-nio/build.gradle index 7f7c75b8e2142..2e58cf3b87b3e 100644 --- a/plugins/transport-nio/build.gradle +++ b/plugins/transport-nio/build.gradle @@ -66,12 +66,6 @@ thirdPartyAudit { 'com.aayushatharva.brotli4j.encoder.Encoder$Mode', 'com.aayushatharva.brotli4j.encoder.Encoder$Parameters', - // from io.netty.handler.codec.protobuf.ProtobufDecoder (netty) - 'com.google.protobuf.ExtensionRegistry', - 'com.google.protobuf.MessageLite$Builder', - 'com.google.protobuf.MessageLite', - 'com.google.protobuf.Parser', - // from io.netty.logging.CommonsLoggerFactory (netty) 'org.apache.commons.logging.Log', 'org.apache.commons.logging.LogFactory', @@ -118,8 +112,6 @@ thirdPartyAudit { 'org.slf4j.spi.LocationAwareLogger', 'com.github.luben.zstd.Zstd', - 'com.google.protobuf.ExtensionRegistryLite', - 'com.google.protobuf.MessageLiteOrBuilder', 'com.google.protobuf.nano.CodedOutputByteBufferNano', 'com.google.protobuf.nano.MessageNano', 'com.jcraft.jzlib.Deflater', diff --git a/protobuf-java-NOTICE.txt b/protobuf-java-NOTICE.txt new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/server/build.gradle b/server/build.gradle index 2eea312699798..235570d1778ff 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -30,10 +30,13 @@ import org.opensearch.gradle.info.BuildParams -apply plugin: 'opensearch.build' -apply plugin: 'com.netflix.nebula.optional-base' -apply plugin: 'opensearch.publish' -apply plugin: 'opensearch.internal-cluster-test' +plugins { + id('com.google.protobuf') version 'latest.release' + id('opensearch.build') + id('com.netflix.nebula.optional-base') + id('opensearch.publish') + id('opensearch.internal-cluster-test') +} publishing { publications { @@ -45,6 +48,13 @@ publishing { archivesBaseName = 'opensearch' +sourceSets { + main { + java { + srcDir "${buildDir}/generated/source/proto/main/java" + } + } +} // we want to keep the JDKs in our IDEs set to JDK 8 until minimum JDK is bumped to 11 so we do not include this source set in our IDEs if (!isEclipse) { sourceSets { @@ -136,6 +146,9 @@ dependencies { // jna api "net.java.dev.jna:jna:${versions.jna}" + // protobuf + api "com.google.protobuf:protobuf-java:${versions.protobuf}" + testImplementation(project(":test:framework")) { // tests use the locally compiled version of server exclude group: 'org.opensearch', module: 'server' @@ -157,6 +170,7 @@ tasks.named("internalClusterTest").configure { } tasks.named("forbiddenPatterns").configure { + dependsOn("generateProto") exclude '**/*.json' exclude '**/*.jmx' exclude '**/*.dic' @@ -203,6 +217,12 @@ def generatePluginsList = tasks.register("generatePluginsList") { } } +protobuf { + protoc { + artifact = "com.google.protobuf:protoc:${versions.protobuf}" + } +} + tasks.named("processResources").configure { dependsOn generateModulesList, generatePluginsList } @@ -303,6 +323,15 @@ tasks.named("thirdPartyAudit").configure { 'com.google.common.geometry.S2$Metric', 'com.google.common.geometry.S2LatLng' ) + ignoreViolations( + 'com.google.protobuf.MessageSchema', + 'com.google.protobuf.UnsafeUtil', + 'com.google.protobuf.UnsafeUtil$1', + 'com.google.protobuf.UnsafeUtil$Android32MemoryAccessor', + 'com.google.protobuf.UnsafeUtil$Android64MemoryAccessor', + 'com.google.protobuf.UnsafeUtil$JvmMemoryAccessor', + 'com.google.protobuf.UnsafeUtil$MemoryAccessor' + ) } tasks.named("dependencyLicenses").configure { @@ -315,13 +344,35 @@ tasks.named("dependencyLicenses").configure { } } +tasks.named("missingJavadoc").configure { + /* + * Generated code doesn't follow javadocs formats. + * Unfortunately the missingJavadoc task doesnt take *. + * TODO: Add support to missingJavadoc task to ignore all generated source code + * https://github.com/opensearch-project/OpenSearch/issues/7264 + */ + dependsOn("generateProto") + javadocMissingIgnore = [ + "org.opensearch.extensions.proto.ExtensionRequestProto", + "org.opensearch.extensions.proto.ExtensionRequestProto.ExtensionRequestOrBuilder", + "org.opensearch.extensions.proto" + ] +} + +tasks.named("filepermissions").configure { + mustRunAfter("generateProto") +} + tasks.named("licenseHeaders").configure { + dependsOn("generateProto") // Ignore our vendored version of Google Guice excludes << 'org/opensearch/common/inject/**/*' // Ignore temporary copies of impending 8.7 Lucene classes excludes << 'org/apache/lucene/search/RegExp87*' excludes << 'org/apache/lucene/search/RegexpQuery87*' excludes << 'org/opensearch/client/documentation/placeholder.txt' + // Ignore for protobuf generated code + excludes << 'org/opensearch/extensions/proto/*' } tasks.test { @@ -329,3 +380,10 @@ tasks.test { jvmArgs += ["--add-opens", "java.base/java.nio.file=ALL-UNNAMED"] } } + +tasks.named("sourcesJar").configure { + // Ignore duplicates for protobuf generated code (main and generatedSources). + filesMatching("**/proto/*") { + duplicatesStrategy = DuplicatesStrategy.EXCLUDE + } +} diff --git a/plugins/repository-gcs/licenses/protobuf-java-3.22.2.jar.sha1 b/server/licenses/protobuf-java-3.22.2.jar.sha1 similarity index 100% rename from plugins/repository-gcs/licenses/protobuf-java-3.22.2.jar.sha1 rename to server/licenses/protobuf-java-3.22.2.jar.sha1 diff --git a/plugins/repository-gcs/licenses/protobuf-LICENSE.txt b/server/licenses/protobuf-java-LICENSE.txt similarity index 100% rename from plugins/repository-gcs/licenses/protobuf-LICENSE.txt rename to server/licenses/protobuf-java-LICENSE.txt diff --git a/plugins/repository-gcs/licenses/protobuf-NOTICE.txt b/server/licenses/protobuf-java-NOTICE.txt similarity index 100% rename from plugins/repository-gcs/licenses/protobuf-NOTICE.txt rename to server/licenses/protobuf-java-NOTICE.txt diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java b/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java index bfe53bee27cd6..07d055bf54d97 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionRequest.java @@ -13,11 +13,11 @@ import org.opensearch.common.Nullable; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; +import org.opensearch.extensions.proto.ExtensionRequestProto; import org.opensearch.transport.TransportRequest; import java.io.IOException; import java.util.Objects; -import java.util.Optional; /** * CLusterService Request for Extensibility @@ -26,42 +26,41 @@ */ public class ExtensionRequest extends TransportRequest { private static final Logger logger = LogManager.getLogger(ExtensionRequest.class); - private final ExtensionsManager.RequestType requestType; - private final Optional uniqueId; + private final ExtensionRequestProto.ExtensionRequest request; - public ExtensionRequest(ExtensionsManager.RequestType requestType) { + public ExtensionRequest(ExtensionRequestProto.RequestType requestType) { this(requestType, null); } - public ExtensionRequest(ExtensionsManager.RequestType requestType, @Nullable String uniqueId) { - this.requestType = requestType; - this.uniqueId = uniqueId == null ? Optional.empty() : Optional.of(uniqueId); + public ExtensionRequest(ExtensionRequestProto.RequestType requestType, @Nullable String uniqueId) { + ExtensionRequestProto.ExtensionRequest.Builder builder = ExtensionRequestProto.ExtensionRequest.newBuilder(); + if (uniqueId != null) { + builder.setUniqueId(uniqueId); + } + this.request = builder.setRequestType(requestType).build(); } public ExtensionRequest(StreamInput in) throws IOException { super(in); - this.requestType = in.readEnum(ExtensionsManager.RequestType.class); - String id = in.readOptionalString(); - this.uniqueId = id == null ? Optional.empty() : Optional.of(id); + this.request = ExtensionRequestProto.ExtensionRequest.parseFrom(in.readByteArray()); } @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - out.writeEnum(requestType); - out.writeOptionalString(uniqueId.orElse(null)); + out.writeByteArray(request.toByteArray()); } - public ExtensionsManager.RequestType getRequestType() { - return this.requestType; + public ExtensionRequestProto.RequestType getRequestType() { + return this.request.getRequestType(); } - public Optional getUniqueId() { - return uniqueId; + public String getUniqueId() { + return request.getUniqueId(); } public String toString() { - return "ExtensionRequest{" + "requestType=" + requestType + "uniqueId=" + uniqueId + '}'; + return "ExtensionRequest{" + request.toString() + '}'; } @Override @@ -69,11 +68,12 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; ExtensionRequest that = (ExtensionRequest) o; - return Objects.equals(requestType, that.requestType) && Objects.equals(uniqueId, that.uniqueId); + return Objects.equals(request.getRequestType(), that.request.getRequestType()) + && Objects.equals(request.getUniqueId(), that.request.getUniqueId()); } @Override public int hashCode() { - return Objects.hash(requestType, uniqueId); + return Objects.hash(request.getRequestType(), request.getUniqueId()); } } diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index 8488743051b6b..2d432f28bd014 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -89,32 +89,13 @@ public class ExtensionsManager { 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_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 REQUEST_EXTENSION_HANDLE_REMOTE_TRANSPORT_ACTION = "internal:extensions/handle-remote-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); - /** - * Enum for Extension Requests - * - * @opensearch.internal - */ - public static enum RequestType { - REQUEST_EXTENSION_CLUSTER_STATE, - REQUEST_EXTENSION_CLUSTER_SETTINGS, - REQUEST_EXTENSION_REGISTER_REST_ACTIONS, - REQUEST_EXTENSION_REGISTER_SETTINGS, - REQUEST_EXTENSION_ENVIRONMENT_SETTINGS, - REQUEST_EXTENSION_DEPENDENCY_INFORMATION, - CREATE_COMPONENT, - ON_INDEX_MODULE, - GET_SETTINGS - }; - /** * Enum for OpenSearch Requests * @@ -430,7 +411,7 @@ public String executor() { /** * Handles an {@link ExtensionRequest}. * - * @param extensionRequest The request to handle, of a type defined in the {@link RequestType} enum. + * @param extensionRequest The request to handle, of a type defined in the {@link org.opensearch.extensions.proto.ExtensionRequestProto.RequestType} enum. * @return an Response matching the request. * @throws Exception if the request is not handled properly. */ @@ -443,7 +424,7 @@ TransportResponse handleExtensionRequest(ExtensionRequest extensionRequest) thro case REQUEST_EXTENSION_ENVIRONMENT_SETTINGS: return new EnvironmentSettingsResponse(this.environmentSettings); case REQUEST_EXTENSION_DEPENDENCY_INFORMATION: - String uniqueId = extensionRequest.getUniqueId().orElse(null); + String uniqueId = extensionRequest.getUniqueId(); if (uniqueId == null) { return new ExtensionDependencyResponse(extensions); } else { @@ -686,10 +667,6 @@ public static String getRequestExtensionRegisterRestActions() { return REQUEST_EXTENSION_REGISTER_REST_ACTIONS; } - public static String getRequestOpensearchParseNamedWriteable() { - return REQUEST_OPENSEARCH_PARSE_NAMED_WRITEABLE; - } - public static String getRequestRestExecuteOnExtensionAction() { return REQUEST_REST_EXECUTE_ON_EXTENSION_ACTION; } diff --git a/server/src/main/proto/extensions/ExtensionRequestProto.proto b/server/src/main/proto/extensions/ExtensionRequestProto.proto new file mode 100644 index 0000000000000..a49e413d37ca9 --- /dev/null +++ b/server/src/main/proto/extensions/ExtensionRequestProto.proto @@ -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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +syntax = "proto3"; +package org.opensearch.extensions.proto; + +option java_outer_classname = "ExtensionRequestProto"; + +enum RequestType { + REQUEST_EXTENSION_CLUSTER_STATE = 0; + REQUEST_EXTENSION_CLUSTER_SETTINGS = 1; + REQUEST_EXTENSION_REGISTER_REST_ACTIONS = 2; + REQUEST_EXTENSION_REGISTER_SETTINGS = 3; + REQUEST_EXTENSION_ENVIRONMENT_SETTINGS = 4; + REQUEST_EXTENSION_DEPENDENCY_INFORMATION = 5; + CREATE_COMPONENT = 6; + ON_INDEX_MODULE = 7; + GET_SETTINGS = 8; +} + +message ExtensionRequest { + RequestType requestType = 1; + string uniqueId = 2; +} diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/BalanceConfigurationTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/BalanceConfigurationTests.java index e1f5eba735904..7bef00edd18d2 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/BalanceConfigurationTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/BalanceConfigurationTests.java @@ -364,7 +364,7 @@ public void testPrimaryBalanceWithContrainstBreaching() { /** * This test verifies global balance by creating indices iteratively and verify primary shards do not pile up on one - * node. + * @throws Exception generic exception */ public void testGlobalPrimaryBalance() throws Exception { AllocationService strategy = createAllocationService(getSettingsBuilderForPrimaryBalance().build(), new TestGatewayAllocator()); diff --git a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java index 6d38e22f28bab..9d60e8ad31873 100644 --- a/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java +++ b/server/src/test/java/org/opensearch/extensions/ExtensionsManagerTests.java @@ -32,7 +32,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; -import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -68,6 +67,7 @@ import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.env.Environment; import org.opensearch.env.TestEnvironment; +import org.opensearch.extensions.proto.ExtensionRequestProto; import org.opensearch.extensions.rest.RegisterRestActionsRequest; import org.opensearch.extensions.settings.RegisterCustomSettingsRequest; import org.opensearch.index.IndexModule; @@ -554,18 +554,18 @@ public void testHandleExtensionRequest() throws Exception { ExtensionsManager extensionsManager = new ExtensionsManager(settings, extensionDir); initialize(extensionsManager); - ExtensionRequest clusterStateRequest = new ExtensionRequest(ExtensionsManager.RequestType.REQUEST_EXTENSION_CLUSTER_STATE); + ExtensionRequest clusterStateRequest = new ExtensionRequest(ExtensionRequestProto.RequestType.REQUEST_EXTENSION_CLUSTER_STATE); assertEquals(ClusterStateResponse.class, extensionsManager.handleExtensionRequest(clusterStateRequest).getClass()); - ExtensionRequest clusterSettingRequest = new ExtensionRequest(ExtensionsManager.RequestType.REQUEST_EXTENSION_CLUSTER_SETTINGS); + ExtensionRequest clusterSettingRequest = new ExtensionRequest(ExtensionRequestProto.RequestType.REQUEST_EXTENSION_CLUSTER_SETTINGS); assertEquals(ClusterSettingsResponse.class, extensionsManager.handleExtensionRequest(clusterSettingRequest).getClass()); ExtensionRequest environmentSettingsRequest = new ExtensionRequest( - ExtensionsManager.RequestType.REQUEST_EXTENSION_ENVIRONMENT_SETTINGS + ExtensionRequestProto.RequestType.REQUEST_EXTENSION_ENVIRONMENT_SETTINGS ); assertEquals(EnvironmentSettingsResponse.class, extensionsManager.handleExtensionRequest(environmentSettingsRequest).getClass()); - ExtensionRequest exceptionRequest = new ExtensionRequest(ExtensionsManager.RequestType.GET_SETTINGS); + ExtensionRequest exceptionRequest = new ExtensionRequest(ExtensionRequestProto.RequestType.GET_SETTINGS); Exception exception = expectThrows( IllegalArgumentException.class, () -> extensionsManager.handleExtensionRequest(exceptionRequest) @@ -574,13 +574,13 @@ public void testHandleExtensionRequest() throws Exception { } public void testExtensionRequest() throws Exception { - ExtensionsManager.RequestType expectedRequestType = ExtensionsManager.RequestType.REQUEST_EXTENSION_DEPENDENCY_INFORMATION; + ExtensionRequestProto.RequestType expectedRequestType = ExtensionRequestProto.RequestType.REQUEST_EXTENSION_DEPENDENCY_INFORMATION; // Test ExtensionRequest 2 arg constructor String expectedUniqueId = "test uniqueid"; ExtensionRequest extensionRequest = new ExtensionRequest(expectedRequestType, expectedUniqueId); assertEquals(expectedRequestType, extensionRequest.getRequestType()); - assertEquals(Optional.of(expectedUniqueId), extensionRequest.getUniqueId()); + assertEquals(expectedUniqueId, extensionRequest.getUniqueId()); // Test ExtensionRequest StreamInput constructor try (BytesStreamOutput out = new BytesStreamOutput()) { @@ -589,14 +589,14 @@ public void testExtensionRequest() throws Exception { try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { extensionRequest = new ExtensionRequest(in); assertEquals(expectedRequestType, extensionRequest.getRequestType()); - assertEquals(Optional.of(expectedUniqueId), extensionRequest.getUniqueId()); + assertEquals(expectedUniqueId, extensionRequest.getUniqueId()); } } // Test ExtensionRequest 1 arg constructor extensionRequest = new ExtensionRequest(expectedRequestType); assertEquals(expectedRequestType, extensionRequest.getRequestType()); - assertEquals(Optional.empty(), extensionRequest.getUniqueId()); + assertTrue(extensionRequest.getUniqueId().isEmpty()); // Test ExtensionRequest StreamInput constructor try (BytesStreamOutput out = new BytesStreamOutput()) { @@ -605,7 +605,7 @@ public void testExtensionRequest() throws Exception { try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) { extensionRequest = new ExtensionRequest(in); assertEquals(expectedRequestType, extensionRequest.getRequestType()); - assertEquals(Optional.empty(), extensionRequest.getUniqueId()); + assertTrue(extensionRequest.getUniqueId().isEmpty()); } } }