Skip to content
Merged
Show file tree
Hide file tree
Changes from 57 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
1 change: 1 addition & 0 deletions 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 NamedRoute to map extension routes to a shortened name ([#6870](https://github.com/opensearch-project/OpenSearch/pull/6870))
Comment thread
owaiskazi19 marked this conversation as resolved.
Outdated

### Dependencies
- Bump `log4j-core` from 2.18.0 to 2.19.0
Expand Down
73 changes: 73 additions & 0 deletions server/src/main/java/org/opensearch/action/ActionModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@
import org.opensearch.persistent.UpdatePersistentTaskStatusAction;
import org.opensearch.plugins.ActionPlugin;
import org.opensearch.plugins.ActionPlugin.ActionHandler;
import org.opensearch.rest.NamedRoute;
import org.opensearch.rest.RestController;
import org.opensearch.rest.RestHandler;
import org.opensearch.rest.RestHeaderDefinition;
Expand Down Expand Up @@ -449,6 +450,7 @@
import org.opensearch.rest.action.search.RestPutSearchPipelineAction;
import org.opensearch.rest.action.search.RestSearchAction;
import org.opensearch.rest.action.search.RestSearchScrollAction;
import org.opensearch.rest.extensions.RestSendToExtensionAction;
import org.opensearch.tasks.Task;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.usage.UsageService;
Expand All @@ -457,8 +459,10 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentSkipListSet;
import java.util.function.Consumer;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;
Expand Down Expand Up @@ -1023,13 +1027,22 @@ public static class DynamicActionRegistry {
// at times other than node bootstrap.
private final Map<ActionType<?>, TransportAction<?, ?>> registry = new ConcurrentHashMap<>();

// A dynamic registry to add or remove Route / RestSendToExtensionAction pairs
// at times other than node bootstrap.
private final Map<RestHandler.Route, RestSendToExtensionAction> routeRegistry = new ConcurrentHashMap<>();

private final Set<String> registeredActionNames = new ConcurrentSkipListSet<>();

/**
* Register the immutable actions in the registry.
*
* @param actions The injected map of {@link ActionType} to {@link TransportAction}
*/
public void registerUnmodifiableActionMap(Map<ActionType, TransportAction> actions) {
this.actions = actions;
for (ActionType action : actions.keySet()) {
registeredActionNames.add(action.name());
}
}

/**
Expand All @@ -1044,6 +1057,7 @@ public void registerDynamicAction(ActionType<?> action, TransportAction<?, ?> tr
if (actions.containsKey(action) || registry.putIfAbsent(action, transportAction) != null) {
throw new IllegalArgumentException("action [" + action.name() + "] already registered");
}
registeredActionNames.add(action.name());
}

/**
Expand All @@ -1056,6 +1070,16 @@ public void unregisterDynamicAction(ActionType<?> action) {
if (registry.remove(action) == null) {
throw new IllegalArgumentException("action [" + action.name() + "] was not registered");
}
registeredActionNames.remove(action.name());
}

/**
* Checks to see if an action is registered provided an action name
*
* @param actionName The name of the action to check
*/
public boolean isActionRegistered(String actionName) {
return registeredActionNames.contains(actionName);
}

/**
Expand All @@ -1071,5 +1095,54 @@ public void unregisterDynamicAction(ActionType<?> action) {
}
return registry.get(action);
}

/**
* Add a dynamic action to the registry.
*
* @param route The route instance to add
* @param action The corresponding instance of RestSendToExtensionAction to execute
*/
public void registerDynamicRoute(RestHandler.Route route, RestSendToExtensionAction action) {
requireNonNull(route, "route is required");
requireNonNull(action, "action is required");
Optional<String> routeName = Optional.empty();
if (route instanceof NamedRoute) {
routeName = Optional.of(((NamedRoute) route).name());
if (isActionRegistered(routeName.get()) || registeredActionNames.contains(routeName.get())) {
throw new IllegalArgumentException("route [" + route + "] already registered");
}
}
if (routeRegistry.containsKey(route)) {
throw new IllegalArgumentException("route [" + route + "] already registered");
}
routeRegistry.put(route, action);
routeName.ifPresent(registeredActionNames::add);
}

/**
* Remove a dynamic route from the registry.
*
* @param route The route to remove
*/
public void unregisterDynamicRoute(RestHandler.Route route) {
Comment thread
owaiskazi19 marked this conversation as resolved.
requireNonNull(route, "route is required");
if (routeRegistry.remove(route) == null) {
throw new IllegalArgumentException("action [" + route + "] was not registered");
}
if (route instanceof NamedRoute) {
registeredActionNames.remove(((NamedRoute) route).name());
}
}
Comment thread
owaiskazi19 marked this conversation as resolved.

/**
* Gets the {@link RestSendToExtensionAction} instance corresponding to the {@link RestHandler.Route} instance.
*
* @param route The {@link RestHandler.Route}.
* @return the corresponding {@link RestSendToExtensionAction} if it is registered, null otherwise.
*/
@SuppressWarnings("unchecked")
public RestSendToExtensionAction get(RestHandler.Route route) {
return routeRegistry.get(route);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.opensearch.OpenSearchException;
import org.opensearch.Version;
import org.opensearch.action.ActionModule;
import org.opensearch.action.ActionModule.DynamicActionRegistry;
import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
import org.opensearch.client.node.NodeClient;
import org.opensearch.cluster.ClusterSettingsResponse;
Expand Down Expand Up @@ -108,7 +109,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 Expand Up @@ -182,7 +182,7 @@ public void initializeServicesAndRestHandler(
actionModule,
this
);
registerRequestHandler();
registerRequestHandler(actionModule.getDynamicActionRegistry());
}

/**
Expand Down Expand Up @@ -223,14 +223,16 @@ public ExtensionActionResponse handleTransportRequest(ExtensionActionRequest req
return extensionTransportActionsHandler.sendTransportRequestToExtension(request);
}

private void registerRequestHandler() {
private void registerRequestHandler(DynamicActionRegistry dynamicActionRegistry) {
transportService.registerRequestHandler(
REQUEST_EXTENSION_REGISTER_REST_ACTIONS,
ThreadPool.Names.GENERIC,
false,
false,
RegisterRestActionsRequest::new,
((request, channel, task) -> channel.sendResponse(restActionsRequestHandler.handleRegisterRestActionsRequest(request)))
((request, channel, task) -> channel.sendResponse(
restActionsRequestHandler.handleRegisterRestActionsRequest(request, dynamicActionRegistry)
))
);
transportService.registerRequestHandler(
REQUEST_EXTENSION_REGISTER_CUSTOM_SETTINGS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@

package org.opensearch.extensions.rest;

import org.opensearch.action.ActionModule.DynamicActionRegistry;
import org.opensearch.extensions.AcknowledgedResponse;
import org.opensearch.extensions.DiscoveryExtensionNode;
import org.opensearch.rest.RestController;
import org.opensearch.rest.RestHandler;
import org.opensearch.rest.extensions.RestSendToExtensionAction;
Comment thread
cwperks marked this conversation as resolved.
import org.opensearch.transport.TransportResponse;
import org.opensearch.transport.TransportService;

Expand Down Expand Up @@ -52,9 +54,17 @@ public RestActionsRequestHandler(
* @return A {@link AcknowledgedResponse} indicating success.
* @throws Exception if the request is not handled properly.
*/
public TransportResponse handleRegisterRestActionsRequest(RegisterRestActionsRequest restActionsRequest) throws Exception {
public TransportResponse handleRegisterRestActionsRequest(
RegisterRestActionsRequest restActionsRequest,
DynamicActionRegistry dynamicActionRegistry
) throws Exception {
DiscoveryExtensionNode discoveryExtensionNode = extensionIdMap.get(restActionsRequest.getUniqueId());
RestHandler handler = new RestSendToExtensionAction(restActionsRequest, discoveryExtensionNode, transportService);
RestHandler handler = new RestSendToExtensionAction(
restActionsRequest,
discoveryExtensionNode,
transportService,
dynamicActionRegistry
);
restController.registerHandler(handler);
return new AcknowledgedResponse(true);
}
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;
Comment thread
owaiskazi19 marked this conversation as 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;
}
}
Comment thread
cwperks marked this conversation as resolved.
55 changes: 55 additions & 0 deletions server/src/main/java/org/opensearch/rest/NamedRoute.java
Original file line number Diff line number Diff line change
@@ -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.rest;

import org.opensearch.OpenSearchException;

/**
* A named Route
*
* @opensearch.internal
*/
public class NamedRoute extends RestHandler.Route {
private static final String VALID_ACTION_NAME_PATTERN = "^[a-zA-Z0-9:/*_]*$";
static final int MAX_LENGTH_OF_ACTION_NAME = 250;

private final String name;

public boolean isValidRouteName(String routeName) {
if (routeName == null || routeName.isBlank() || routeName.length() > MAX_LENGTH_OF_ACTION_NAME) {
return false;
}
Comment thread
owaiskazi19 marked this conversation as resolved.
return routeName.matches(VALID_ACTION_NAME_PATTERN);
}
Comment thread
owaiskazi19 marked this conversation as resolved.

public NamedRoute(RestRequest.Method method, String path, String name) {
super(method, path);
if (!isValidRouteName(name)) {
throw new OpenSearchException(
"Invalid route name specified. The route name may include the following characters"
+ " 'a-z', 'A-Z', '0-9', ':', '/', '*', '_' and be less than "
+ MAX_LENGTH_OF_ACTION_NAME
+ " characters"
);
}
this.name = name;
}

/**
* The name of the Route. Must be unique across Route.
*/
public String name() {
return this.name;
}

@Override
public String toString() {
return "NamedRoute [method=" + method + ", path=" + path + ", name=" + name + "]";
}
}
32 changes: 30 additions & 2 deletions server/src/main/java/org/opensearch/rest/RestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,8 @@ public boolean allowSystemIndexAccessByDefault() {
*/
class Route {

private final String path;
private final Method method;
protected final String path;
protected final Method method;

public Route(Method method, String path) {
this.path = path;
Expand All @@ -196,9 +196,37 @@ public String getPath() {
return path;
}

public String getPathWithPathParamsReplaced() {
return path.replaceAll("(?<=\\{).*?(?=\\})", "path_param");
}

public Method getMethod() {
return method;
}

@Override
public int hashCode() {
String routeStr = "Route [method=" + method + ", path=" + getPathWithPathParamsReplaced() + "]";
return routeStr.hashCode();
}

@Override
public String toString() {
return "Route [method=" + method + ", path=" + path + "]";
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
Route that = (Route) o;
return Objects.equals(method, that.method)
&& Objects.equals(getPathWithPathParamsReplaced(), that.getPathWithPathParamsReplaced());
Comment thread
cwperks marked this conversation as resolved.
}
}

/**
Expand Down
Loading