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

[Backport 2.x] Create NamedRoute to map extension routes to a shortened name #7631

Merged
merged 1 commit into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Adds ExtensionsManager.lookupExtensionSettingsById ([#7466](https://github.com/opensearch-project/OpenSearch/pull/7466))
- Provide mechanism to configure XContent parsing constraints (after update to Jackson 2.15.0 and above) ([#7550](https://github.com/opensearch-project/OpenSearch/pull/7550))
- Support to clear filecache using clear indices cache API ([#7498](https://github.com/opensearch-project/OpenSearch/pull/7498))
- Create NamedRoute to map extension routes to a shortened name ([#6870](https://github.com/opensearch-project/OpenSearch/pull/6870))

### Dependencies
- Bump `com.netflix.nebula:gradle-info-plugin` from 12.0.0 to 12.1.3 (#7564)
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) {
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());
}
}

/**
* 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 @@ -32,6 +32,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 @@ -107,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 Expand Up @@ -181,7 +181,7 @@ public void initializeServicesAndRestHandler(
actionModule,
this
);
registerRequestHandler();
registerRequestHandler(actionModule.getDynamicActionRegistry());
}

/**
Expand Down Expand Up @@ -222,14 +222,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;
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;
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;
}
}
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;
}
return routeName.matches(VALID_ACTION_NAME_PATTERN);
}

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

/**
Expand Down
Loading