[Extensions] Moving ExtensionsRequest to use Protobuf

CHANGELOG.md

@@ -95,6 +95,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Enable sort optimization for all NumericTypes ([#6464](https://github.com/opensearch-project/OpenSearch/pull/6464)
 - Remove 'cluster_manager' role attachment when using 'node.master' deprecated setting ([#6331](https://github.com/opensearch-project/OpenSearch/pull/6331))
 - Add new cluster settings to ignore weighted round-robin routing and fallback to default behaviour. ([#6834](https://github.com/opensearch-project/OpenSearch/pull/6834))
+- [Extensions] Moving Extensions APIs to protobuf serialization. ([#6960](https://github.com/opensearch-project/OpenSearch/pull/6960))
 - Add experimental support for ZSTD compression. ([#3577](https://github.com/opensearch-project/OpenSearch/pull/3577))
 - [Segment Replication] Add point in time and scroll query compatibility. ([#6644](https://github.com/opensearch-project/OpenSearch/pull/6644))
 
@@ -136,4 +137,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Security
 
 [Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.x...HEAD
-[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.5...2.x
+[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.5...2.x

modules/transport-netty4/build.gradle

@@ -131,12 +131,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',
@@ -191,8 +185,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',

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',

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.2'
   api "commons-logging:commons-logging:${versions.commonslogging}"
   api 'commons-cli:commons-cli:1.5.0'
   api "commons-codec:commons-codec:${versions.commonscodec}"
@@ -116,14 +115,7 @@ tasks.named("dependencyLicenses").configure {
 
 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'
+
   )
 }
 

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',

server/build.gradle

@@ -30,6 +30,10 @@
 
 import org.opensearch.gradle.info.BuildParams
 
+plugins {
+  id('com.google.protobuf') version '0.8.19'
+}
+
 apply plugin: 'opensearch.build'
 apply plugin: 'com.netflix.nebula.optional-base'
 apply plugin: 'opensearch.publish'
@@ -45,6 +49,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 +147,9 @@ dependencies {
   // jna
   api "net.java.dev.jna:jna:${versions.jna}"
 
+  // protobuf
+  api 'com.google.protobuf:protobuf-java:3.22.2'
+
   testImplementation(project(":test:framework")) {
     // tests use the locally compiled version of server
     exclude group: 'org.opensearch', module: 'server'
@@ -203,6 +217,12 @@ def generatePluginsList = tasks.register("generatePluginsList") {
     }
 }
 
+protobuf {
+  protoc {
+    artifact = "com.google.protobuf:protoc:3.22.2"
+  }
+}
+
 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 {

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,54 +26,54 @@
  */
 public class ExtensionRequest extends TransportRequest {
     private static final Logger logger = LogManager.getLogger(ExtensionRequest.class);
-    private final ExtensionsManager.RequestType requestType;
-    private final Optional<String> 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<String> getUniqueId() {
-        return uniqueId;
+    public String getUniqueId() {
+        return request.getUniqueId();
     }
 
     public String toString() {
-        return "ExtensionRequest{" + "requestType=" + requestType + "uniqueId=" + uniqueId + '}';
+        return "ExtensionRequest{" + request.toString() + '}';
     }
 
     @Override
     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());
     }
 }

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;
     }

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;
+}

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());