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

Create NamedRoute to map extension routes to a shortened name #6870

Merged
merged 59 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
a688fd0
WIP on rest layer authz
cwperks Mar 27, 2023
88955ff
Create PermissibleRoute
cwperks Mar 28, 2023
a7655e5
Update extension handshake
cwperks Mar 28, 2023
de8b8b8
Add connectToNodeAsExtension in TransportService
cwperks Mar 29, 2023
ad68321
Merge branch 'extension-handshake' into move-ext-rest-request
cwperks Mar 29, 2023
0003dcc
Add to CHANGELOG
cwperks Mar 31, 2023
4d87934
Merge branch 'main' into extension-handshake
cwperks Mar 31, 2023
6c41e96
Merge branch 'extension-handshake' into move-ext-rest-request
cwperks Mar 31, 2023
f1d9c71
Add to CHANGELOG
cwperks Mar 31, 2023
64e8667
Update RouteHandler
cwperks Mar 31, 2023
98b1af0
Update java docstrings
cwperks Apr 3, 2023
a7f83b5
Merge branch 'extension-handshake' into move-ext-rest-request
cwperks Apr 3, 2023
e1c754b
Merge branch 'main' into extension-handshake
cwperks Apr 10, 2023
4268341
Merge branch 'extension-handshake' into move-ext-rest-request
cwperks Apr 10, 2023
1ef93fc
Merge branch 'main' into extension-handshake
cwperks Apr 22, 2023
e041cf6
Merge branch 'extension-handshake' into move-ext-rest-request
cwperks Apr 22, 2023
3b4b91b
Run spotlessApply
cwperks Apr 22, 2023
d90d065
Fix merge conflicts
cwperks Apr 22, 2023
38ca9da
Merge branch 'main' into extension-handshake
cwperks Apr 27, 2023
8913f9f
Merge branch 'extension-handshake' into move-ext-rest-request
cwperks Apr 27, 2023
c202da9
Merge branch 'main' into move-ext-rest-request
cwperks Apr 28, 2023
dace95e
Rename to ProtectedRoute
cwperks Apr 28, 2023
bff0ea2
Merge branch 'main' into move-ext-rest-request
cwperks May 1, 2023
13a00a0
Merge branch 'main' into move-ext-rest-request
cwperks May 4, 2023
29284a1
Merge branch 'main' into move-ext-rest-request
cwperks May 5, 2023
634e0b8
Merge branch 'main' into move-ext-rest-request
cwperks May 8, 2023
b2c60d9
Create method to get extension settings from extensions.yml
cwperks May 8, 2023
7d38fee
Add ExtensionsManager.lookupExtensionSettings
cwperks May 8, 2023
f188c34
Small change to name
cwperks May 8, 2023
140440d
Add to CHANGELOG
cwperks May 8, 2023
48264c2
Move extensionSettingsMap.put
cwperks May 9, 2023
00f3ece
Re-run CI
cwperks May 9, 2023
5fc95e4
Merge branch 'lookup-extension-settings' into move-ext-rest-request
cwperks May 9, 2023
955a1de
Merge branch 'main' into move-ext-rest-request
cwperks May 11, 2023
b305e9a
Address review feedback
cwperks May 11, 2023
9f53b37
Add test for ProtectedRoute
cwperks May 11, 2023
91b2753
spotlessApply
cwperks May 11, 2023
097e8d2
Merge branch 'main' into move-ext-rest-request
cwperks May 11, 2023
8155630
Merge branch 'main' into move-ext-rest-request
cwperks May 16, 2023
5e1239a
Add RouteHandlerTests
cwperks May 16, 2023
271b6f6
Switch to NamedRoute and add validation for action naming
cwperks May 16, 2023
8e087b3
Merge branch 'main' into move-ext-rest-request
cwperks May 16, 2023
9e453ec
Avoid magic numbers
cwperks May 16, 2023
1fbde26
Remove @Test annotation
cwperks May 16, 2023
c3f3a0e
Address code review feedback
cwperks May 16, 2023
6cb42fe
Update error message
cwperks May 16, 2023
eceda9c
Check for REST Action name uniqueness across all registered actions
cwperks May 17, 2023
6095f8d
minimize code in the test
cwperks May 17, 2023
52c3f1e
Merge branch 'check-rest-action-name-in-registry' into move-ext-rest-…
cwperks May 17, 2023
300b6a9
Update changelog
cwperks May 17, 2023
2f35272
Add DynamicRouteRegistry
cwperks May 17, 2023
cb1244f
Address code review feedback
cwperks May 17, 2023
5a9c363
Add mock DynamicRouteRegistry.class
cwperks May 17, 2023
9ca3c90
Add RouteRegistry to DynamicActionModule
cwperks May 18, 2023
c7d7c15
Pass around dynamicActionRegistry instead of ActionModule
cwperks May 18, 2023
fd53943
Only pass dynamic action registry
cwperks May 18, 2023
ccbe4b5
Add DynamicActionRegistryTests for tests of dynamic registry
cwperks May 18, 2023
0ad12ee
Move CHANGELOG entry
cwperks May 18, 2023
98a9d44
Merge branch 'main' into move-ext-rest-request
cwperks May 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add events correlation engine plugin ([#6854](https://github.com/opensearch-project/OpenSearch/issues/6854))
- Add connectToNodeAsExtension in TransportService ([#6866](https://github.com/opensearch-project/OpenSearch/pull/6866))
- Adds ExtensionsManager.lookupExtensionSettingsById ([#7466](https://github.com/opensearch-project/OpenSearch/pull/7466))
- Create ProtectedRoute to map extension routes to a shortened name ([#6870](https://github.com/opensearch-project/OpenSearch/pull/6870))
cwperks marked this conversation as resolved.
Show resolved Hide resolved

### Dependencies
- Bump `log4j-core` from 2.18.0 to 2.19.0
Expand Down Expand Up @@ -129,4 +130,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.7...2.x
[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.7...2.x
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ public static enum OpenSearchRequestType {

private final Path extensionsPath;
private ExtensionTransportActionsHandler extensionTransportActionsHandler;

private Map<String, Extension> extensionSettingsMap;
private Map<String, DiscoveryExtensionNode> initializedExtensions;
private Map<String, DiscoveryExtensionNode> extensionIdMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.opensearch.extensions.DiscoveryExtensionNode;
import org.opensearch.rest.RestController;
import org.opensearch.rest.RestHandler;
import org.opensearch.rest.extensions.RestSendToExtensionAction;
cwperks marked this conversation as resolved.
Show resolved Hide resolved
import org.opensearch.transport.TransportResponse;
import org.opensearch.transport.TransportService;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* 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.util.function.Function;

import org.opensearch.rest.RestHandler.Route;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestRequest.Method;

/**
* A subclass of {@link Route} that includes a handler method for that route.
*/
public class RouteHandler extends Route {

private final String name;

private final Function<RestRequest, ExtensionRestResponse> responseHandler;

/**
* Handle the method and path with the specified handler.
*
* @param method The {@link Method} to handle.
* @param path The path to handle.
* @param handler The method which handles the method and path.
*/
public RouteHandler(Method method, String path, Function<RestRequest, ExtensionRestResponse> handler) {
super(method, path);
this.responseHandler = handler;
this.name = null;
}

/**
* Handle the method and path with the specified handler.
*
* @param name The name of the handler.
* @param method The {@link Method} to handle.
* @param path The path to handle.
* @param handler The method which handles the method and path.
*/
public RouteHandler(String name, Method method, String path, Function<RestRequest, ExtensionRestResponse> handler) {
super(method, path);
this.responseHandler = handler;
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
this.name = name;
}

/**
* Executes the handler for this route.
*
* @param request The request to handle
* @return the {@link ExtensionRestResponse} result from the handler for this route.
*/
public ExtensionRestResponse handleRequest(RestRequest request) {
return responseHandler.apply(request);
}

/**
* The name of the RouteHandler. Must be unique across route handlers.
*/
public String name() {
return this.name;
}
}
cwperks marked this conversation as resolved.
Show resolved Hide resolved
31 changes: 31 additions & 0 deletions server/src/main/java/org/opensearch/rest/ProtectedRoute.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.rest;

/**
* A named Route
*
* @opensearch.api
*/
public class ProtectedRoute extends RestHandler.Route {
reta marked this conversation as resolved.
Show resolved Hide resolved

private final String name;

public ProtectedRoute(RestRequest.Method method, String path, String name) {
super(method, path);
this.name = name;
}

/**
* The name of the Route. Must be unique across Route.
*/
public String name() {
return this.name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* compatible open source license.
*/

package org.opensearch.extensions.rest;
package org.opensearch.rest.extensions;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand All @@ -16,8 +16,12 @@
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.extensions.DiscoveryExtensionNode;
import org.opensearch.extensions.ExtensionsManager;
import org.opensearch.extensions.rest.ExtensionRestRequest;
import org.opensearch.extensions.rest.RegisterRestActionsRequest;
import org.opensearch.extensions.rest.RestExecuteOnExtensionResponse;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.ProtectedRoute;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestRequest.Method;
import org.opensearch.rest.RestStatus;
Expand All @@ -33,6 +37,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.concurrent.CompletableFuture;
Expand Down Expand Up @@ -86,15 +91,26 @@ public RestSendToExtensionAction(

List<Route> restActionsAsRoutes = new ArrayList<>();
for (String restAction : restActionsRequest.getRestActions()) {
int delim = restAction.indexOf(' ');
Optional<String> name = Optional.empty();
String[] parts = restAction.split(" ");
if (parts.length < 2) {
throw new IllegalArgumentException("REST action must contain at least a REST method and route");
}
try {
method = RestRequest.Method.valueOf(restAction.substring(0, delim));
path = pathPrefix + restAction.substring(delim).trim();
method = RestRequest.Method.valueOf(parts[0].trim());
path = pathPrefix + parts[1].trim();
if (parts.length > 2) {
name = Optional.of(parts[2].trim());
reta marked this conversation as resolved.
Show resolved Hide resolved
}
} catch (IndexOutOfBoundsException | IllegalArgumentException e) {
throw new IllegalArgumentException(restAction + " does not begin with a valid REST method");
}
logger.info("Registering: " + method + " " + path);
restActionsAsRoutes.add(new Route(method, path));
if (name.isPresent()) {
reta marked this conversation as resolved.
Show resolved Hide resolved
restActionsAsRoutes.add(new ProtectedRoute(method, path, name.get()));
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
} else {
restActionsAsRoutes.add(new Route(method, path));
}
}
this.routes = unmodifiableList(restActionsAsRoutes);

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
*/

/** REST classes for the extensions package. OpenSearch extensions provide extensibility to OpenSearch.*/
package org.opensearch.rest.extensions;
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@
import org.opensearch.common.util.PageCacheRecycler;
import org.opensearch.extensions.DiscoveryExtensionNode;
import org.opensearch.indices.breaker.NoneCircuitBreakerService;
import org.opensearch.rest.ProtectedRoute;
import org.opensearch.rest.RestHandler.Route;
import org.opensearch.rest.RestRequest.Method;
import org.opensearch.rest.extensions.RestSendToExtensionAction;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.test.transport.MockTransportService;
import org.opensearch.threadpool.TestThreadPool;
Expand Down Expand Up @@ -127,6 +129,40 @@ public void testRestSendToExtensionAction() throws Exception {
assertTrue(expectedMethods.containsAll(methods));
}

public void testRestSendToExtensionActionWithProtectedRoute() throws Exception {
RegisterRestActionsRequest registerRestActionRequest = new RegisterRestActionsRequest(
"uniqueid1",
List.of("GET /foo foo", "PUT /bar bar", "POST /baz baz"),
List.of("GET /deprecated/foo foo_deprecated", "It's deprecated!")
);
RestSendToExtensionAction restSendToExtensionAction = new RestSendToExtensionAction(
registerRestActionRequest,
discoveryExtensionNode,
transportService
);

assertEquals("send_to_extension_action", restSendToExtensionAction.getName());
List<ProtectedRoute> expected = new ArrayList<>();
String uriPrefix = "/_extensions/_uniqueid1";
expected.add(new ProtectedRoute(Method.GET, uriPrefix + "/foo", "foo"));
expected.add(new ProtectedRoute(Method.PUT, uriPrefix + "/bar", "bar"));
expected.add(new ProtectedRoute(Method.POST, uriPrefix + "/baz", "baz"));

List<Route> routes = restSendToExtensionAction.routes();
assertEquals(expected.size(), routes.size());
List<String> expectedPaths = expected.stream().map(Route::getPath).collect(Collectors.toList());
List<String> paths = routes.stream().map(Route::getPath).collect(Collectors.toList());
List<Method> expectedMethods = expected.stream().map(Route::getMethod).collect(Collectors.toList());
List<Method> methods = routes.stream().map(Route::getMethod).collect(Collectors.toList());
List<String> expectedNames = expected.stream().map(ProtectedRoute::name).collect(Collectors.toList());
List<String> names = routes.stream().map(r -> ((ProtectedRoute) r).name()).collect(Collectors.toList());
assertTrue(paths.containsAll(expectedPaths));
assertTrue(expectedPaths.containsAll(paths));
assertTrue(methods.containsAll(expectedMethods));
assertTrue(expectedMethods.containsAll(methods));
assertTrue(expectedNames.containsAll(names));
}

public void testRestSendToExtensionActionFilterHeaders() throws Exception {
RegisterRestActionsRequest registerRestActionRequest = new RegisterRestActionsRequest(
"uniqueid1",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* 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.RestRequest;
import org.opensearch.rest.RestStatus;
import org.opensearch.test.OpenSearchTestCase;

public class RouteHandlerTests extends OpenSearchTestCase {
public void testUnnamedRouteHandler() {
RouteHandler rh = new RouteHandler(
RestRequest.Method.GET,
"/foo/bar",
req -> new ExtensionRestResponse(req, RestStatus.OK, "content")
);

assertEquals(null, rh.name());
}

public void testNamedRouteHandler() {
RouteHandler rh = new RouteHandler(
"foo",
RestRequest.Method.GET,
"/foo/bar",
req -> new ExtensionRestResponse(req, RestStatus.OK, "content")
);

assertEquals("foo", rh.name());
}
}