Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Extensions]Add query for initialized extensions #5658

Merged
merged 13 commits into from
Jan 5, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add support for ppc64le architecture ([#5459](https://github.com/opensearch-project/OpenSearch/pull/5459))
- Added @gbbafna as an OpenSearch maintainer ([#5668](https://github.com/opensearch-project/OpenSearch/pull/5668))
- Added support for feature flags in opensearch.yml ([#4959](https://github.com/opensearch-project/OpenSearch/pull/4959))
- Add query for initialized extensions ([#5658](https://github.com/opensearch-project/OpenSearch/pull/5658))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@owaiskazi19 Just a heads up, but since this change was to be backported and released in 2.x, this changelog entry should have gone in the [Unreleased 2.x] section of this file. I'm working on cleaning up the changelog on main right now.


### Dependencies
- Bumps `log4j-core` from 2.18.0 to 2.19.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@ public List<ExtensionDependency> getDependencies() {
return dependencies;
}

public boolean dependenciesContain(ExtensionDependency dependency) {
for (ExtensionDependency extensiondependency : this.dependencies) {
if (dependency.getUniqueId().equals(extensiondependency.getUniqueId())
&& dependency.getVersion().equals(extensiondependency.getVersion())) {
return true;
}
}
return false;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* 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.ArrayList;
import java.util.List;
import java.util.Objects;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.transport.TransportResponse;

/**
* The response for getting the Extension Dependency.
*
* @opensearch.internal
*/
public class ExtensionDependencyResponse extends TransportResponse {
private List<DiscoveryExtensionNode> extensionDependencies;

public ExtensionDependencyResponse(List<DiscoveryExtensionNode> extensionDependencies) {
this.extensionDependencies = extensionDependencies;
}

public ExtensionDependencyResponse(StreamInput in) throws IOException {
int size = in.readVInt();
extensionDependencies = new ArrayList<>(size);
for (int i = 0; i < size; i++) {
extensionDependencies.add(new DiscoveryExtensionNode(in));
}
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeVInt(extensionDependencies.size());
for (DiscoveryExtensionNode dependency : extensionDependencies) {
dependency.writeTo(out);
}
}

public List<DiscoveryExtensionNode> getExtensionDependency() {
return extensionDependencies;
}

@Override
public String toString() {
return "ExtensionDependencyResponse{extensiondependency=" + extensionDependencies.toString() + '}';
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ExtensionDependencyResponse that = (ExtensionDependencyResponse) o;
return Objects.equals(extensionDependencies, that.extensionDependencies);
}

@Override
public int hashCode() {
return Objects.hash(extensionDependencies);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.common.Nullable;
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;
import java.util.Optional;

/**
* CLusterService Request for Extensibility
Expand All @@ -24,41 +26,54 @@
*/
public class ExtensionRequest extends TransportRequest {
private static final Logger logger = LogManager.getLogger(ExtensionRequest.class);
private ExtensionsManager.RequestType requestType;
private final ExtensionsManager.RequestType requestType;
private final Optional<String> uniqueId;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there is no test class for ExtensionRequest so you should create one and include a test of this uniqueID, both in the null case (single arg constructor) or value present case (two arg constructor)


public ExtensionRequest(ExtensionsManager.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(StreamInput in) throws IOException {
super(in);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe Requests require the super(in) while responses do not. This should definitely match the writeTo implementation; I don't expect it will pass tests without it matching.

this.requestType = in.readEnum(ExtensionsManager.RequestType.class);
String id = in.readOptionalString();
this.uniqueId = id == null ? Optional.empty() : Optional.of(id);
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeEnum(requestType);
out.writeOptionalString(uniqueId.orElse(null));
}

public ExtensionsManager.RequestType getRequestType() {
return this.requestType;
}

public Optional<String> getUniqueId() {
return uniqueId;
}

public String toString() {
return "ExtensionRequest{" + "requestType=" + requestType + '}';
return "ExtensionRequest{" + "requestType=" + requestType + "uniqueId=" + uniqueId + '}';
}

@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);
return Objects.equals(requestType, that.requestType) && Objects.equals(uniqueId, that.uniqueId);
}

@Override
public int hashCode() {
return Objects.hash(requestType);
return Objects.hash(requestType, uniqueId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -79,6 +80,7 @@ public class ExtensionsManager {
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_DEPENDENCY_INFORMATION = "internal:discovery/dependencyinformation";
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";
Expand All @@ -101,6 +103,7 @@ public static enum RequestType {
REQUEST_EXTENSION_REGISTER_REST_ACTIONS,
REQUEST_EXTENSION_REGISTER_SETTINGS,
REQUEST_EXTENSION_ENVIRONMENT_SETTINGS,
REQUEST_EXTENSION_DEPENDENCY_INFORMATION,
CREATE_COMPONENT,
ON_INDEX_MODULE,
GET_SETTINGS
Expand Down Expand Up @@ -241,6 +244,14 @@ private void registerRequestHandler() {
ExtensionRequest::new,
((request, channel, task) -> channel.sendResponse(handleExtensionRequest(request)))
);
transportService.registerRequestHandler(
REQUEST_EXTENSION_DEPENDENCY_INFORMATION,
ThreadPool.Names.GENERIC,
false,
false,
ExtensionRequest::new,
((request, channel, task) -> channel.sendResponse(handleExtensionRequest(request)))
);
transportService.registerRequestHandler(
REQUEST_EXTENSION_ADD_SETTINGS_UPDATE_CONSUMER,
ThreadPool.Names.GENERIC,
Expand Down Expand Up @@ -417,6 +428,16 @@ TransportResponse handleExtensionRequest(ExtensionRequest extensionRequest) thro
return new ClusterSettingsResponse(clusterService);
case REQUEST_EXTENSION_ENVIRONMENT_SETTINGS:
return new EnvironmentSettingsResponse(this.environmentSettings);
case REQUEST_EXTENSION_DEPENDENCY_INFORMATION:
String uniqueId = extensionRequest.getUniqueId().orElse(null);
if (uniqueId == null) {
return new ExtensionDependencyResponse(extensions);
} else {
ExtensionDependency matchingId = new ExtensionDependency(uniqueId, Version.CURRENT);
return new ExtensionDependencyResponse(
extensions.stream().filter(e -> e.dependenciesContain(matchingId)).collect(Collectors.toList())
);
}
default:
throw new IllegalArgumentException("Handler not present for the provided request");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
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;

Expand Down Expand Up @@ -528,6 +529,88 @@ public void testHandleExtensionRequest() throws Exception {
assertEquals("Handler not present for the provided request", exception.getMessage());
}

public void testExtensionRequest() throws Exception {
ExtensionsManager.RequestType expectedRequestType = ExtensionsManager.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());

// Test ExtensionRequest StreamInput constructor
try (BytesStreamOutput out = new BytesStreamOutput()) {
extensionRequest.writeTo(out);
out.flush();
try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) {
extensionRequest = new ExtensionRequest(in);
assertEquals(expectedRequestType, extensionRequest.getRequestType());
assertEquals(Optional.of(expectedUniqueId), extensionRequest.getUniqueId());
}
}

// Test ExtensionRequest 1 arg constructor
extensionRequest = new ExtensionRequest(expectedRequestType);
assertEquals(expectedRequestType, extensionRequest.getRequestType());
assertEquals(Optional.empty(), extensionRequest.getUniqueId());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test to assertEquals(Optional.empty(), extensionRequest.getUniqueId()); here and below line 562.

// Test ExtensionRequest StreamInput constructor
try (BytesStreamOutput out = new BytesStreamOutput()) {
extensionRequest.writeTo(out);
out.flush();
try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) {
extensionRequest = new ExtensionRequest(in);
assertEquals(expectedRequestType, extensionRequest.getRequestType());
assertEquals(Optional.empty(), extensionRequest.getUniqueId());
}
}
}

public void testExtensionDependencyResponse() throws Exception {
String expectedUniqueId = "test uniqueid";
List<DiscoveryExtensionNode> expectedExtensionsList = new ArrayList<DiscoveryExtensionNode>();
Version expectedVersion = Version.fromString("2.0.0");
ExtensionDependency expectedDependency = new ExtensionDependency(expectedUniqueId, expectedVersion);

expectedExtensionsList.add(
new DiscoveryExtensionNode(
"firstExtension",
"uniqueid1",
"uniqueid1",
"myIndependentPluginHost1",
"127.0.0.0",
new TransportAddress(InetAddress.getByName("127.0.0.0"), 9300),
new HashMap<String, String>(),
Version.fromString("3.0.0"),
new PluginInfo(
"firstExtension",
"Fake description 1",
"0.0.7",
Version.fromString("3.0.0"),
"14",
"fakeClass1",
new ArrayList<String>(),
false
),
List.of(expectedDependency)
)
);

// Test ExtensionDependencyResponse arg constructor
ExtensionDependencyResponse extensionDependencyResponse = new ExtensionDependencyResponse(expectedExtensionsList);
assertEquals(expectedExtensionsList, extensionDependencyResponse.getExtensionDependency());

// Test ExtensionDependencyResponse StreamInput constructor
try (BytesStreamOutput out = new BytesStreamOutput()) {
extensionDependencyResponse.writeTo(out);
out.flush();
try (BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes()))) {
extensionDependencyResponse = new ExtensionDependencyResponse(in);
assertEquals(expectedExtensionsList, extensionDependencyResponse.getExtensionDependency());
}
}
}

public void testEnvironmentSettingsResponse() throws Exception {

// Test EnvironmentSettingsResponse arg constructor
Expand Down Expand Up @@ -711,7 +794,7 @@ public void testRegisterHandler() throws Exception {
settings,
client
);
verify(mockTransportService, times(8)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any());
verify(mockTransportService, times(9)).registerRequestHandler(anyString(), anyString(), anyBoolean(), anyBoolean(), any(), any());

}

Expand Down