diff --git a/CREATE_YOUR_FIRST_EXTENSION.md b/CREATE_YOUR_FIRST_EXTENSION.md index 53d49fc1..643efc07 100644 --- a/CREATE_YOUR_FIRST_EXTENSION.md +++ b/CREATE_YOUR_FIRST_EXTENSION.md @@ -164,28 +164,28 @@ import org.opensearch.sdk.rest.BaseExtensionRestHandler; public class CrudAction extends BaseExtensionRestHandler { @Override - protected List routeHandlers() { + public List routes() { return List.of( - new RouteHandler(Method.PUT, "/sample", createHandler), - new RouteHandler(Method.GET, "/sample/{id}", readHandler), - new RouteHandler(Method.POST, "/sample/{id}", updateHandler), - new RouteHandler(Method.DELETE, "/sample/{id}", deleteHandler) + new NamedRoute.Builder().method(Method.PUT).path("/sample").uniqueName("extension1:sample/create").handler(createHandler).build(), + new NamedRoute.Builder().method(Method.GET).path("/sample/{id}").uniqueName("extension1:sample/get").handler(readHandler).build(), + new NamedRoute.Builder().method(Method.POST).path("/sample/{id}").uniqueName("extension1:sample/post").handler(updateHandler).build(), + new NamedRoute.Builder().method(Method.DELETE).path("/sample/{id}").uniqueName("extension1:sample/delete").handler(deleteHandler).build() ); } - Function createHandler = (request) -> { + Function createHandler = (request) -> { return new ExtensionRestResponse(request, RestStatus.OK, "To be implemented"); }; - Function readHandler = (request) -> { + Function readHandler = (request) -> { return new ExtensionRestResponse(request, RestStatus.OK, "To be implemented"); }; - Function updateHandler = (request) -> { + Function updateHandler = (request) -> { return new ExtensionRestResponse(request, RestStatus.OK, "To be implemented"); }; - Function deleteHandler = (request) -> { + Function deleteHandler = (request) -> { return new ExtensionRestResponse(request, RestStatus.OK, "To be implemented"); }; } @@ -248,7 +248,7 @@ return createJsonResponse(request, RestStatus.OK, "_id", response.id()); Finally, you have the following code: ```java -Function createHandler = (request) -> { +Function createHandler = (request) -> { IndexResponse response; try { BooleanResponse exists = client.indices().exists(new ExistsRequest.Builder().index("crudsample").build()); diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 021ebc7a..e9957f47 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -146,14 +146,14 @@ To **run OpenSearch from a compiled binary**, follow these steps: - Start OpenSearch using `./bin/opensearch`. - Send the below sample REST API to initialize an extension ```bash -curl -XPOST "localhost:9200/_extensions/initialize" -H "Content-Type:application/json" --data '{ \ -"name":"hello-world", \ -"uniqueId":"hello-world", \ -"hostAddress":"127.0.0.1", \ -"port":"4532", \ -"version":"1.0", \ -"opensearchVersion":"3.0.0", \ -"minimumCompatibleVersion":"3.0.0", \ +curl -XPOST "localhost:9200/_extensions/initialize" -H "Content-Type:application/json" --data '{ +"name":"hello-world", +"uniqueId":"hello-world", +"hostAddress":"127.0.0.1", +"port":"4532", +"version":"1.0", +"opensearchVersion":"3.0.0", +"minimumCompatibleVersion":"3.0.0", "dependencies":[{"uniqueId":"test1","version":"2.0.0"},{"uniqueId":"test2","version":"3.0.0"}] \ }' ``` @@ -162,18 +162,20 @@ To **run OpenSearch from Gradle**, follow these steps: - Run `./gradlew run` to start OpenSearch. - Send the below sample REST API to initialize an extension ```bash -curl -XPOST "localhost:9200/_extensions/initialize" -H "Content-Type:application/json" --data '{ \ -"name":"hello-world", \ -"uniqueId":"hello-world", \ -"hostAddress":"127.0.0.1", \ -"port":"4532", \ -"version":"1.0", \ -"opensearchVersion":"3.0.0", \ -"minimumCompatibleVersion":"3.0.0", \ -"dependencies":[{"uniqueId":"test1","version":"2.0.0"},{"uniqueId":"test2","version":"3.0.0"}] \ +curl -XPOST "localhost:9200/_extensions/initialize" -H "Content-Type:application/json" --data '{ +"name":"hello-world", +"uniqueId":"hello-world", +"hostAddress":"127.0.0.1", +"port":"4532", +"version":"1.0", +"opensearchVersion":"3.0.0", +"minimumCompatibleVersion":"3.0.0", +"dependencies":[{"uniqueId":"test1","version":"2.0.0"},{"uniqueId":"test2","version":"3.0.0"}] }' ``` +Note: If the Security plugin is initialized in OpenSearch, use admin credentials to send extension initialization request. + In response to the REST `/initialize` request, `ExtensionsManager` discovers the extension listening on a predefined port and executes the TCP handshake protocol to establish a data transfer connection. Then OpenSearch sends a request to the OpenSearch SDK for Java and, upon acknowledgment, the extension responds with its name. This name is logged in the terminal where OpenSearch is running: ```bash diff --git a/PLUGIN_MIGRATION.md b/PLUGIN_MIGRATION.md index bfb134c3..5be87b61 100644 --- a/PLUGIN_MIGRATION.md +++ b/PLUGIN_MIGRATION.md @@ -67,14 +67,38 @@ XContentParser parser = XContentType.JSON Other potential initialization values are: ```java this.environmentSettings = extensionsRunner.getEnvironmentSettings(); -this.transportService = extensionsRunner.getExtensionTransportService(); +this.transportService = extensionsRunner.getSdkTransportService().getTransportService(); this.restClient = anomalyDetectorExtension.getRestClient(); this.sdkClusterService = new SDKClusterService(extensionsRunner); ``` Many of these components are also available via Guice injection. -Optionally, change the `routes()` to `routeHandlers()`. Change `prepareRequest()` to `handleRequest()`. +### Replace `Route` with `NamedRoute` +Change `routes()` to be NamedRoutes. Here is a sample of an existing route converted to a named route: +Before: +``` +public List routes() { + return ImmutableList.of( + new Route(GET, "/uri") + ); +} +``` +With new scheme: +``` +private Function uriHandler = () -> {}; +public List routes() { + return ImmutableList.of( + new NamedRoute.Builder().method(GET).path("/uri").uniqueName("extension:uri").handler(uriHandler).build() + ); +} +``` + +You can optionally also add `actionNames()` to this route. These should correspond to any current actions defined as permissions in roles. +`actionNames()` serve as a valuable tool for converting plugins into extensions while maintaining compatibility with pre-defined reserved roles. +Ensure that these name-to-route mappings are easily accessible to the cluster admins to allow granting access to these APIs. + +Change `prepareRequest()` to `handleRequest()`. ### Replace `BytesRestResponse` with `ExtensionRestResponse` diff --git a/build.gradle b/build.gradle index 35588e9b..550e2f0c 100644 --- a/build.gradle +++ b/build.gradle @@ -189,16 +189,18 @@ dependencies { testRuntimeOnly("org.junit.platform:junit-platform-launcher:${junitPlatform}") configurations.all { - resolutionStrategy.force("jakarta.json:jakarta.json-api:${jakartaVersion}") - resolutionStrategy.force("com.fasterxml.jackson.core:jackson-databind:${jacksonDatabindVersion}") - resolutionStrategy.force("com.fasterxml.jackson.core:jackson-core:${jacksonDatabindVersion}") - resolutionStrategy.force("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:${jacksonDatabindVersion}") - resolutionStrategy.force("com.fasterxml.jackson.dataformat:jackson-dataformat-smile:${jacksonDatabindVersion}") - resolutionStrategy.force("com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${jacksonDatabindVersion}") - resolutionStrategy.force("org.apache.logging.log4j:log4j-api:${log4jVersion}") - resolutionStrategy.force("org.apache.logging.log4j:log4j-core:${log4jVersion}") - resolutionStrategy.force("org.apache.logging.log4j:log4j-jul:${log4jVersion}") - resolutionStrategy.force("org.slf4j:slf4j-api:${slf4jVersion}") + resolutionStrategy { + force("jakarta.json:jakarta.json-api:${jakartaVersion}") + force("com.fasterxml.jackson.core:jackson-databind:${jacksonDatabindVersion}") + force("com.fasterxml.jackson.core:jackson-core:${jacksonDatabindVersion}") + force("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:${jacksonDatabindVersion}") + force("com.fasterxml.jackson.dataformat:jackson-dataformat-smile:${jacksonDatabindVersion}") + force("com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:${jacksonDatabindVersion}") + force("org.apache.logging.log4j:log4j-api:${log4jVersion}") + force("org.apache.logging.log4j:log4j-core:${log4jVersion}") + force("org.apache.logging.log4j:log4j-jul:${log4jVersion}") + force("org.slf4j:slf4j-api:${slf4jVersion}") + } } } @@ -322,7 +324,7 @@ task closeTestExtension (type: Exec) { tasks.named("integTest").configure { finalizedBy(closeTestExtension) } testClusters.integTest { - extension(new ExtensionsProperties("${testExtensionYml.name}", "${testExtensionYml.uniqueId}", "${testExtensionYml.hostAddress}", "${testExtensionYml.port}", "${testExtensionYml.version}", "${testExtensionYml.opensearchVersion}", "${testExtensionYml.minimumCompatibleVersion}")) + extension(true) testDistribution = "ARCHIVE" // Cluster shrink exception thrown if we try to set numberOfNodes to 1, so only apply if > 1 if (_numNodes > 1) numberOfNodes = _numNodes diff --git a/config/certs/cert-gen.sh b/config/certs/cert-gen.sh new file mode 100755 index 00000000..2547511b --- /dev/null +++ b/config/certs/cert-gen.sh @@ -0,0 +1,34 @@ +#! /bin/bash + +openssl genrsa -out root-ca-key.pem 2048 +openssl req -new -x509 -sha256 -key root-ca-key.pem -subj "/C=US/ST=NEW YORK/L=BROOKLYN/O=OPENSEARCH/OU=SECURITY/CN=ROOT" -out root-ca.pem -days 730 + +openssl genrsa -out extension-01-key-temp.pem 2048 +openssl pkcs8 -inform PEM -outform PEM -in extension-01-key-temp.pem -topk8 -nocrypt -v1 PBE-SHA1-3DES -out extension-01-key.pem +openssl req -new -key extension-01-key.pem -subj "/C=US/ST=NEW YORK/L=BROOKLYN/O=OPENSEARCH/OU=SECURITY/CN=extension-01" -out extension-01.csr +echo 'subjectAltName=DNS:extension-01' | tee -a extension-01.ext +echo 'subjectAltName=IP:172.20.0.11' | tee -a extension-01.ext +openssl x509 -req -in extension-01.csr -CA root-ca.pem -CAkey root-ca-key.pem -CAcreateserial -sha256 -out extension-01.pem -days 730 -extfile extension-01.ext + +rm extension-01-key-temp.pem +rm extension-01.csr +rm extension-01.ext +rm root-ca.srl + +openssl genrsa -out admin-key-temp.pem 2048 +openssl pkcs8 -inform PEM -outform PEM -in admin-key-temp.pem -topk8 -nocrypt -v1 PBE-SHA1-3DES -out admin-key.pem +openssl req -new -key admin-key.pem -subj "/C=US/ST=NEW YORK/L=BROOKLYN/O=OPENSEARCH/OU=SECURITY/CN=A" -out admin.csr +openssl x509 -req -in admin.csr -CA root-ca.pem -CAkey root-ca-key.pem -CAcreateserial -sha256 -out admin.pem -days 730 +openssl genrsa -out os-node-01-key-temp.pem 2048 +openssl pkcs8 -inform PEM -outform PEM -in os-node-01-key-temp.pem -topk8 -nocrypt -v1 PBE-SHA1-3DES -out os-node-01-key.pem +openssl req -new -key os-node-01-key.pem -subj "/C=US/ST=NEW YORK/L=BROOKLYN/O=OPENSEARCH/OU=SECURITY/CN=os-node-01" -out os-node-01.csr +echo 'subjectAltName=DNS:os-node-01' | tee -a os-node-01.ext +echo 'subjectAltName=IP:172.20.0.11' | tee -a os-node-01.ext +openssl x509 -req -in os-node-01.csr -CA root-ca.pem -CAkey root-ca-key.pem -CAcreateserial -sha256 -out os-node-01.pem -days 730 -extfile os-node-01.ext + +rm admin-key-temp.pem +rm admin.csr +rm os-node-01-key-temp.pem +rm os-node-01.csr +rm os-node-01.ext +rm root-ca.srl \ No newline at end of file diff --git a/src/main/java/org/opensearch/sdk/ExtensionSettings.java b/src/main/java/org/opensearch/sdk/ExtensionSettings.java index 5d93663c..53ae7668 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionSettings.java +++ b/src/main/java/org/opensearch/sdk/ExtensionSettings.java @@ -53,6 +53,7 @@ public class ExtensionSettings { private String hostPort; private String opensearchAddress; private String opensearchPort; + private Map securitySettings; /** * A set of keys for security settings related to SSL transport, keystore and truststore files, and hostname verification. @@ -83,8 +84,6 @@ public class ExtensionSettings { SSL_TRANSPORT_TRUSTSTORE_TYPE ); - private Map securitySettings; - /** * Jackson requires a no-arg constructor. */ diff --git a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java index 34216457..68f60930 100644 --- a/src/main/java/org/opensearch/sdk/ExtensionsRunner.java +++ b/src/main/java/org/opensearch/sdk/ExtensionsRunner.java @@ -36,6 +36,7 @@ import org.opensearch.sdk.handlers.ExtensionsInitRequestHandler; import org.opensearch.sdk.handlers.ExtensionsRestRequestHandler; import org.opensearch.sdk.handlers.UpdateSettingsRequestHandler; +import org.opensearch.sdk.rest.BaseExtensionRestHandler; import org.opensearch.sdk.rest.ExtensionRestHandler; import org.opensearch.sdk.rest.ExtensionRestPathRegistry; import org.opensearch.tasks.TaskManager; @@ -233,6 +234,9 @@ protected ExtensionsRunner(Extension extension) throws IOException { if (extension instanceof ActionExtension) { // store REST handlers in the registry for (ExtensionRestHandler extensionRestHandler : ((ActionExtension) extension).getExtensionRestHandlers()) { + if (extensionRestHandler instanceof BaseExtensionRestHandler) { + ((BaseExtensionRestHandler) extensionRestHandler).setExtensionName(extensionSettings.getExtensionName()); + } extensionRestPathRegistry.registerHandler(extensionRestHandler); } } diff --git a/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java b/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java index bb616bf9..b6fc7c8a 100644 --- a/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java +++ b/src/main/java/org/opensearch/sdk/rest/BaseExtensionRestHandler.java @@ -14,7 +14,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; -import org.opensearch.rest.BaseRestHandler; + import static org.opensearch.rest.RestStatus.INTERNAL_SERVER_ERROR; import static org.opensearch.rest.RestStatus.NOT_FOUND; @@ -26,35 +26,31 @@ import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.extensions.rest.ExtensionRestResponse; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.DeprecationRestHandler; +import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestHandler.DeprecatedRoute; import org.opensearch.rest.RestHandler.ReplacedRoute; -import org.opensearch.rest.RestHandler.Route; -import org.opensearch.rest.RestRequest.Method; -import org.opensearch.rest.DeprecationRestHandler; import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestRequest.Method; +import org.opensearch.rest.RestResponse; import org.opensearch.rest.RestStatus; /** * Provides convenience methods to reduce boilerplate code in an {@link ExtensionRestHandler} implementation. */ public abstract class BaseExtensionRestHandler implements ExtensionRestHandler { + + private static String extensionName; + /** * Constant for JSON content type */ public static final String JSON_CONTENT_TYPE = APPLICATION_JSON.withCharset(StandardCharsets.UTF_8).toString(); - /** - * Defines a list of methods which handle each rest {@link Route}. Override this in a subclass to use the functional syntax. - * - * @return a list of {@link RouteHandler} with corresponding methods to each route. - */ - protected List routeHandlers() { - return Collections.emptyList(); - } - @Override - public List routes() { - return List.copyOf(routeHandlers()); + public List routes() { + return Collections.emptyList(); } /** @@ -80,6 +76,19 @@ protected List replacedRouteHandlers() { return Collections.emptyList(); } + public void setExtensionName(String extensionName) { + BaseExtensionRestHandler.extensionName = extensionName; + } + + /** + * Generates a name for the handler prepended with the extension's name + * @param name The human-readable name for a route registered by this extension + * @return Returns a name prepended with the extension's name + */ + protected static String routePrefix(String name) { + return extensionName + ":" + name; + } + @Override public List replacedRoutes() { return List.copyOf(replacedRouteHandlers()); @@ -87,12 +96,12 @@ public List replacedRoutes() { @Override public ExtensionRestResponse handleRequest(RestRequest request) { - Optional handler = routeHandlers().stream() + Optional route = routes().stream() .filter(rh -> rh.getMethod().equals(request.method())) .filter(rh -> restPathMatches(request.path(), rh.getPath())) .findFirst(); - if (handler.isPresent()) { - return handler.get().handleRequest(request); + if (route.isPresent() && route.get().handler() != null) { + return (ExtensionRestResponse) route.get().handler().apply(request); } Optional deprecatedHandler = deprecatedRouteHandlers().stream() .filter(rh -> rh.getMethod().equals(request.method())) @@ -122,7 +131,7 @@ public ExtensionRestResponse handleRequest(RestRequest request) { * Determines if the request's path is a match for the configured handler path. * * @param requestPath The path from the {@link RestRequest} - * @param handlerPath The path from the {@link RouteHandler} or {@link DeprecatedRouteHandler} + * @param handlerPath The path from the {@link NamedRoute} or {@link DeprecatedRouteHandler} or {@link ReplacedRouteHandler} * @return true if the request path matches the route */ private boolean restPathMatches(String requestPath, String handlerPath) { @@ -223,42 +232,12 @@ protected ExtensionRestResponse createEmptyJsonResponse(RestRequest request, Res return new ExtensionRestResponse(request, status, JSON_CONTENT_TYPE, "{}"); } - /** - * A subclass of {@link Route} that includes a handler method for that route. - */ - public static class RouteHandler extends Route { - - private final Function 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 handler) { - super(method, path); - this.responseHandler = handler; - } - - /** - * 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); - } - } - /** * A subclass of {@link DeprecatedRoute} that includes a handler method for that route. */ public static class DeprecatedRouteHandler extends DeprecatedRoute { - private final Function responseHandler; + private final Function responseHandler; /** * Handle the method and path with the specified handler. @@ -268,12 +247,7 @@ public static class DeprecatedRouteHandler extends DeprecatedRoute { * @param deprecationMessage The message to log with the deprecation logger * @param handler The method which handles the method and path. */ - public DeprecatedRouteHandler( - Method method, - String path, - String deprecationMessage, - Function handler - ) { + public DeprecatedRouteHandler(Method method, String path, String deprecationMessage, Function handler) { super(method, path, deprecationMessage); this.responseHandler = handler; } @@ -285,7 +259,7 @@ public DeprecatedRouteHandler( * @return the {@link ExtensionRestResponse} result from the handler for this route. */ public ExtensionRestResponse handleRequest(RestRequest request) { - return responseHandler.apply(request); + return (ExtensionRestResponse) responseHandler.apply(request); } } diff --git a/src/main/java/org/opensearch/sdk/rest/ExtensionRestHandler.java b/src/main/java/org/opensearch/sdk/rest/ExtensionRestHandler.java index ed4d3c87..5cb2e502 100644 --- a/src/main/java/org/opensearch/sdk/rest/ExtensionRestHandler.java +++ b/src/main/java/org/opensearch/sdk/rest/ExtensionRestHandler.java @@ -14,6 +14,7 @@ import org.opensearch.extensions.rest.ExtensionRestResponse; import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestHandler.DeprecatedRoute; import org.opensearch.rest.RestHandler.ReplacedRoute; @@ -42,7 +43,7 @@ public interface ExtensionRestHandler { * * @return The routes this handler will handle. */ - default List routes() { + default List routes() { return Collections.emptyList(); } diff --git a/src/main/java/org/opensearch/sdk/rest/ExtensionRestPathRegistry.java b/src/main/java/org/opensearch/sdk/rest/ExtensionRestPathRegistry.java index 3dd76950..0a8cc12c 100644 --- a/src/main/java/org/opensearch/sdk/rest/ExtensionRestPathRegistry.java +++ b/src/main/java/org/opensearch/sdk/rest/ExtensionRestPathRegistry.java @@ -11,10 +11,12 @@ import java.util.ArrayList; import java.util.List; +import java.util.Set; import org.opensearch.common.Nullable; import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.path.PathTrie; + import org.opensearch.rest.RestRequest.Method; import org.opensearch.rest.RestUtils; import org.opensearch.sdk.rest.BaseExtensionRestHandler.ExtensionDeprecationRestHandler; @@ -38,9 +40,18 @@ public class ExtensionRestPathRegistry { * @param restHandler The RestHandler to register routes. */ public void registerHandler(ExtensionRestHandler restHandler) { - restHandler.routes().forEach(route -> registerHandler(route.getMethod(), route.getPath(), restHandler)); + restHandler.routes().forEach(route -> { + String routeActionName = route.name(); + if (routeActionName == null) { + throw new IllegalArgumentException("Route handler must have a name associated with it."); + } + Set associatedActions = route.actionNames(); + registerHandler(route.getMethod(), route.getPath(), routeActionName, associatedActions, restHandler); + }); + restHandler.deprecatedRoutes() .forEach(route -> registerAsDeprecatedHandler(route.getMethod(), route.getPath(), restHandler, route.getDeprecationMessage())); + restHandler.replacedRoutes() .forEach( route -> registerWithDeprecatedHandler( @@ -57,20 +68,28 @@ public void registerHandler(ExtensionRestHandler restHandler) { * Registers a REST handler to be executed when one of the provided methods and path match the request. * * @param path Path to handle (e.g., "/{index}/{type}/_bulk") - * @param handler The handler to actually execute + * @param extensionRestHandler The handler to actually execute + * @param name The name corresponding to this handler + * @param actionNames The set of actions to be registered with this handler * @param method GET, POST, etc. */ - private void registerHandler(Method method, String path, ExtensionRestHandler extensionRestHandler) { + public void registerHandler( + Method method, + String path, + String name, + Set actionNames, + ExtensionRestHandler extensionRestHandler + ) { pathTrie.insertOrUpdate( path, new SDKMethodHandlers(path, extensionRestHandler, method), (mHandlers, newMHandler) -> mHandlers.addMethods(extensionRestHandler, method) ); if (extensionRestHandler instanceof ExtensionDeprecationRestHandler) { - registeredDeprecatedPaths.add(restPathToString(method, path)); + registeredDeprecatedPaths.add(restPathToString(method, path, name, actionNames)); registeredDeprecatedPaths.add(((ExtensionDeprecationRestHandler) extensionRestHandler).getDeprecationMessage()); } else { - registeredPaths.add(restPathToString(method, path)); + registeredPaths.add(restPathToString(method, path, name, actionNames)); } } @@ -85,7 +104,7 @@ private void registerHandler(Method method, String path, ExtensionRestHandler ex private void registerAsDeprecatedHandler(Method method, String path, ExtensionRestHandler handler, String deprecationMessage) { assert (handler instanceof ExtensionDeprecationRestHandler) == false; - registerHandler(method, path, new ExtensionDeprecationRestHandler(handler, deprecationMessage, deprecationLogger)); + registerHandler(method, path, null, null, new ExtensionDeprecationRestHandler(handler, deprecationMessage, deprecationLogger)); } /** @@ -129,7 +148,7 @@ private void registerWithDeprecatedHandler( + path + "] instead."; - registerHandler(method, path, handler); + registerHandler(method, path, null, null, handler); registerAsDeprecatedHandler(deprecatedMethod, deprecatedPath, handler, deprecationMessage); } @@ -171,9 +190,18 @@ public List getRegisteredDeprecatedPaths() { * * @param method the method. * @param path the path. + * @param name the name corresponding to this route. + * @param actionNames the set of actions registered with this route * @return A string appending the method and path. */ - public static String restPathToString(Method method, String path) { - return method.name() + " " + path; + public static String restPathToString(Method method, String path, String name, Set actionNames) { + StringBuilder sb = new StringBuilder(method.name() + " " + path + " "); + if (name != null && !name.isEmpty()) { + sb.append(name).append(" "); + } + if (actionNames != null) { + actionNames.forEach(act -> sb.append(act).append(",")); + } + return sb.toString(); } } diff --git a/src/main/java/org/opensearch/sdk/rest/ReplacedRouteHandler.java b/src/main/java/org/opensearch/sdk/rest/ReplacedRouteHandler.java index 99d4c5ab..1fdab92b 100644 --- a/src/main/java/org/opensearch/sdk/rest/ReplacedRouteHandler.java +++ b/src/main/java/org/opensearch/sdk/rest/ReplacedRouteHandler.java @@ -16,13 +16,14 @@ import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; +import org.opensearch.rest.RestResponse; /** * A subclass of {@link ReplacedRoute} that includes a handler method for that route. */ public class ReplacedRouteHandler extends ReplacedRoute { - private final Function responseHandler; + private final Function responseHandler; /** * Handle replaced routes using new and deprecated methods and new and deprecated paths. @@ -31,14 +32,14 @@ public class ReplacedRouteHandler extends ReplacedRoute { * @param path new route path * @param deprecatedMethod deprecated method * @param deprecatedPath deprecated path - * @param handler The method which handles the method and path. + * @param handler The method which handles the REST method and path. */ public ReplacedRouteHandler( Method method, String path, Method deprecatedMethod, String deprecatedPath, - Function handler + Function handler ) { super(method, path, deprecatedMethod, deprecatedPath); this.responseHandler = handler; @@ -51,9 +52,9 @@ public ReplacedRouteHandler( * @param method route method * @param path new route path * @param deprecatedPath deprecated path - * @param handler The method which handles the method and path. + * @param handler The method which handles the REST method and path. */ - public ReplacedRouteHandler(Method method, String path, String deprecatedPath, Function handler) { + public ReplacedRouteHandler(Method method, String path, String deprecatedPath, Function handler) { this(method, path, method, deprecatedPath, handler); } @@ -63,9 +64,9 @@ public ReplacedRouteHandler(Method method, String path, String deprecatedPath, F * @param route route * @param prefix new route prefix * @param deprecatedPrefix deprecated prefix - * @param handler The method which handles the method and path. + * @param handler The method which handles the REST method and path. */ - public ReplacedRouteHandler(Route route, String prefix, String deprecatedPrefix, Function handler) { + public ReplacedRouteHandler(Route route, String prefix, String deprecatedPrefix, Function handler) { this(route.getMethod(), prefix + route.getPath(), deprecatedPrefix + route.getPath(), handler); } @@ -76,6 +77,6 @@ public ReplacedRouteHandler(Route route, String prefix, String deprecatedPrefix, * @return the {@link ExtensionRestResponse} result from the handler for this route. */ public ExtensionRestResponse handleRequest(RestRequest request) { - return responseHandler.apply(request); + return (ExtensionRestResponse) responseHandler.apply(request); } } diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java index 8673d48c..17a36c04 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestHelloAction.java @@ -15,16 +15,21 @@ import org.opensearch.common.xcontent.XContentType; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.extensions.rest.ExtensionRestResponse; +import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestResponse; import org.opensearch.sdk.rest.BaseExtensionRestHandler; import org.opensearch.sdk.rest.ExtensionRestHandler; + import java.io.IOException; import java.net.URLDecoder; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Random; +import java.util.Set; import java.util.function.Function; import static org.opensearch.rest.RestRequest.Method.DELETE; @@ -50,24 +55,49 @@ public class RestHelloAction extends BaseExtensionRestHandler { private List worldAdjectives = new ArrayList<>(); private Random rand = new Random(); + /** + * Instantiate this action + */ + public RestHelloAction() {} + @Override - public List routeHandlers() { + public List routes() { return List.of( - new RouteHandler(GET, "/hello", handleGetRequest), - new RouteHandler(POST, "/hello", handlePostRequest), - new RouteHandler(PUT, "/hello/{name}", handlePutRequest), - new RouteHandler(DELETE, "/goodbye", handleDeleteRequest) + new NamedRoute.Builder().method(GET) + .path("/hello") + .handler(handleGetRequest) + .uniqueName(routePrefix("greet")) + .legacyActionNames(Set.of("cluster:admin/opensearch/hw/greet")) + .build(), + new NamedRoute.Builder().method(POST) + .path("/hello") + .handler(handlePostRequest) + .uniqueName(routePrefix("greet_with_adjective")) + .legacyActionNames(Collections.emptySet()) + .build(), + new NamedRoute.Builder().method(PUT) + .path("/hello/{name}") + .handler(handlePutRequest) + .uniqueName(routePrefix("greet_with_name")) + .legacyActionNames(Set.of("cluster:admin/opensearch/hw/greet_with_name")) + .build(), + new NamedRoute.Builder().method(DELETE) + .path("/goodbye") + .handler(handleDeleteRequest) + .uniqueName(routePrefix("goodbye")) + .legacyActionNames(Collections.emptySet()) + .build() ); } - private Function handleGetRequest = (request) -> { + private Function handleGetRequest = (request) -> { String worldNameWithRandomAdjective = worldAdjectives.isEmpty() ? worldName : String.join(" ", worldAdjectives.get(rand.nextInt(worldAdjectives.size())), worldName); return new ExtensionRestResponse(request, OK, String.format(GREETING, worldNameWithRandomAdjective)); }; - private Function handlePostRequest = (request) -> { + private Function handlePostRequest = (request) -> { if (request.hasContent()) { String adjective = ""; XContentType contentType = request.getXContentType(); @@ -115,7 +145,7 @@ public List routeHandlers() { return new ExtensionRestResponse(request, BAD_REQUEST, TEXT_CONTENT_TYPE, noContent); }; - private Function handlePutRequest = (request) -> { + private Function handlePutRequest = (request) -> { String name = request.param("name"); try { worldName = URLDecoder.decode(name, StandardCharsets.UTF_8); @@ -125,7 +155,7 @@ public List routeHandlers() { return new ExtensionRestResponse(request, OK, "Updated the world's name to " + worldName); }; - private Function handleDeleteRequest = (request) -> { + private Function handleDeleteRequest = (request) -> { this.worldName = DEFAULT_NAME; this.worldAdjectives.clear(); return new ExtensionRestResponse(request, OK, "Goodbye, cruel world! Restored default values."); diff --git a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java index cf4952f7..d3c12e42 100644 --- a/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java +++ b/src/main/java/org/opensearch/sdk/sample/helloworld/rest/RestRemoteHelloAction.java @@ -14,7 +14,9 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.extensions.action.RemoteExtensionActionResponse; import org.opensearch.extensions.rest.ExtensionRestResponse; +import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestResponse; import org.opensearch.sdk.ExtensionsRunner; import org.opensearch.sdk.SDKClient; import org.opensearch.sdk.action.RemoteExtensionAction; @@ -24,6 +26,7 @@ import org.opensearch.sdk.sample.helloworld.transport.SampleRequest; import org.opensearch.sdk.sample.helloworld.transport.SampleResponse; +import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -49,11 +52,19 @@ public RestRemoteHelloAction(ExtensionsRunner runner) { } @Override - public List routeHandlers() { - return List.of(new RouteHandler(GET, "/hello/{name}", handleRemoteGetRequest)); + public List routes() { + return List.of( + + new NamedRoute.Builder().method(GET) + .path("/hello/{name}") + .handler(handleRemoteGetRequest) + .uniqueName(routePrefix("remote_greet_with_name")) + .legacyActionNames(Collections.emptySet()) + .build() + ); } - private Function handleRemoteGetRequest = (request) -> { + private Function handleRemoteGetRequest = (request) -> { SDKClient client = extensionsRunner.getSdkClient(); String name = request.param("name"); @@ -80,7 +91,7 @@ public List routeHandlers() { TimeUnit.SECONDS ).get(); if (!response.isSuccess()) { - return new ExtensionRestResponse(request, OK, "Remote extension reponse failed: " + response.getResponseBytesAsString()); + return new ExtensionRestResponse(request, OK, "Remote extension response failed: " + response.getResponseBytesAsString()); } // Parse out the expected response class from the bytes SampleResponse sampleResponse = new SampleResponse(StreamInput.wrap(response.getResponseBytes())); diff --git a/src/test/java/org/opensearch/sdk/rest/TestBaseExtensionRestHandler.java b/src/test/java/org/opensearch/sdk/rest/TestBaseExtensionRestHandler.java index 7fa02c83..3c375331 100644 --- a/src/test/java/org/opensearch/sdk/rest/TestBaseExtensionRestHandler.java +++ b/src/test/java/org/opensearch/sdk/rest/TestBaseExtensionRestHandler.java @@ -17,35 +17,56 @@ import org.junit.jupiter.api.Test; import org.opensearch.common.bytes.BytesArray; import org.opensearch.extensions.rest.ExtensionRestResponse; +import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; +import org.opensearch.rest.RestResponse; import org.opensearch.rest.RestStatus; import org.opensearch.test.OpenSearchTestCase; +import static org.opensearch.rest.RestRequest.Method.GET; + public class TestBaseExtensionRestHandler extends OpenSearchTestCase { private final BaseExtensionRestHandler handler = new BaseExtensionRestHandler() { @Override - public List routeHandlers() { - return List.of(new RouteHandler(Method.GET, "/foo", handleFoo)); + public List routes() { + return List.of( + new NamedRoute.Builder().method(GET) + .path("/foo") + .handler(handleFoo) + .uniqueName("foo") + .legacyActionNames(Collections.emptySet()) + .build() + ); } @Override public List deprecatedRouteHandlers() { - return List.of(new DeprecatedRouteHandler(Method.GET, "/deprecated/foo", "It's deprecated", handleFoo)); + return List.of(new DeprecatedRouteHandler(GET, "/deprecated/foo", "It's deprecated", handleBar)); } @Override public List replacedRouteHandlers() { return List.of( - new ReplacedRouteHandler(Method.GET, "/new/foo", Method.GET, "/old/foo", handleFoo), - new ReplacedRouteHandler(Method.PUT, "/new/put/foo", "/old/put/foo", handleFoo), - new ReplacedRouteHandler(new Route(Method.POST, "/foo"), "/new", "/old", handleFoo) + new ReplacedRouteHandler(GET, "/new/foo", GET, "/old/foo", handleBar), + new ReplacedRouteHandler(Method.PUT, "/new/put/foo", "/old/put/foo", handleBar), + new ReplacedRouteHandler(new Route(Method.POST, "/foo"), "/new", "/old", handleBar) ); } - private final Function handleFoo = (request) -> { + private final Function handleFoo = (request) -> { + try { + if ("foo".equals(request.content().utf8ToString())) { + return createJsonResponse(request, RestStatus.OK, "success", "named foo"); + } + throw new IllegalArgumentException("no foo"); + } catch (Exception e) { + return exceptionalRequest(request, e); + } + }; + private final Function handleBar = (request) -> { try { if ("bar".equals(request.content().utf8ToString())) { return createJsonResponse(request, RestStatus.OK, "success", "bar"); @@ -62,13 +83,12 @@ public void testHandlerDefaultRoutes() { BaseExtensionRestHandler defaultHandler = new BaseExtensionRestHandler() { }; assertTrue(defaultHandler.routes().isEmpty()); - assertTrue(defaultHandler.routeHandlers().isEmpty()); } @Test public void testJsonResponse() { RestRequest successfulRequest = TestSDKRestRequest.createTestRestRequest( - Method.GET, + GET, "/foo", "/foo", Collections.emptyMap(), @@ -86,7 +106,7 @@ public void testJsonResponse() { @Test public void testJsonDeprecatedResponse() { RestRequest successfulRequest = TestSDKRestRequest.createTestRestRequest( - Method.GET, + GET, "/deprecated/foo", "/deprecated/foo", Collections.emptyMap(), @@ -104,7 +124,7 @@ public void testJsonDeprecatedResponse() { @Test public void testJsonReplacedResponseGet() { RestRequest successfulRequestOld = TestSDKRestRequest.createTestRestRequest( - Method.GET, + GET, "/old/foo", "/old/foo", Collections.emptyMap(), @@ -115,7 +135,7 @@ public void testJsonReplacedResponseGet() { null ); RestRequest successfulRequestNew = TestSDKRestRequest.createTestRestRequest( - Method.GET, + GET, "/new/foo", "/new/foo", Collections.emptyMap(), @@ -203,7 +223,7 @@ public void testJsonReplacedResponsePost() { @Test public void testErrorResponseOnException() { RestRequest exceptionalRequest = TestSDKRestRequest.createTestRestRequest( - Method.GET, + GET, "/foo", "/foo", Collections.emptyMap(), @@ -243,7 +263,7 @@ public void testErrorResponseOnUnhandled() { ); RestRequest unhandledRequestPath = TestSDKRestRequest.createTestRestRequest( - Method.GET, + GET, "foobar", "foobar", Collections.emptyMap(), @@ -269,18 +289,25 @@ public void testErrorResponseOnUnhandled() { public void testCreateEmptyJsonResponse() { BaseExtensionRestHandler handlerWithEmptyJsonResponse = new BaseExtensionRestHandler() { @Override - public List routeHandlers() { - return List.of(new RouteHandler(Method.GET, "/emptyJsonResponse", handleEmptyJsonResponse)); + public List routes() { + return List.of( + new NamedRoute.Builder().method(GET) + .path("/emptyJsonResponse") + .handler(handleEmptyJsonResponse) + .uniqueName("emptyresponse") + .legacyActionNames(Collections.emptySet()) + .build() + ); } - private final Function handleEmptyJsonResponse = (request) -> createEmptyJsonResponse( + private final Function handleEmptyJsonResponse = (request) -> createEmptyJsonResponse( request, RestStatus.OK ); }; RestRequest emptyJsonResponseRequest = TestSDKRestRequest.createTestRestRequest( - Method.GET, + GET, "/emptyJsonResponse", "/emptyJsonResponse", Collections.emptyMap(), diff --git a/src/test/java/org/opensearch/sdk/rest/TestExtensionRestPathRegistry.java b/src/test/java/org/opensearch/sdk/rest/TestExtensionRestPathRegistry.java index 8d794cb7..008214d0 100644 --- a/src/test/java/org/opensearch/sdk/rest/TestExtensionRestPathRegistry.java +++ b/src/test/java/org/opensearch/sdk/rest/TestExtensionRestPathRegistry.java @@ -9,14 +9,16 @@ package org.opensearch.sdk.rest; +import java.util.Collections; import java.util.List; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.opensearch.extensions.rest.ExtensionRestResponse; +import org.opensearch.rest.NamedRoute; +import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestHandler.DeprecatedRoute; import org.opensearch.rest.RestHandler.ReplacedRoute; -import org.opensearch.rest.RestHandler.Route; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; import org.opensearch.sdk.rest.BaseExtensionRestHandler.ExtensionDeprecationRestHandler; @@ -28,8 +30,8 @@ public class TestExtensionRestPathRegistry extends OpenSearchTestCase { private ExtensionRestHandler fooHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of(new Route(Method.GET, "/foo")); + public List routes() { + return List.of(new NamedRoute.Builder().method(Method.GET).path("/foo").uniqueName("foo").build()); } @Override @@ -46,18 +48,16 @@ public ExtensionRestResponse handleRequest(RestRequest request) { @Override public List replacedRoutes() { return List.of( - new ReplacedRouteHandler(Method.GET, "/new/foo", Method.GET, "/old/foo", r -> { return null; }), - new ReplacedRouteHandler(Method.PUT, "/new/put/foo", "/old/put/foo", r -> { - return null; - }), - new ReplacedRouteHandler(new Route(Method.POST, "/foo"), "/new", "/old", r -> { return null; }) + new ReplacedRouteHandler(Method.GET, "/new/foo", Method.GET, "/old/foo", r -> null), + new ReplacedRouteHandler(Method.PUT, "/new/put/foo", "/old/put/foo", r -> null), + new ReplacedRouteHandler(new Route(Method.POST, "/foo"), "/new", "/old", r -> null) ); } }; private ExtensionRestHandler barHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of(new Route(Method.PUT, "/bar/{planet}")); + public List routes() { + return List.of(new NamedRoute.Builder().method(Method.PUT).path("/bar/{planet}").uniqueName("bar_planet").build()); } @Override @@ -67,8 +67,11 @@ public ExtensionRestResponse handleRequest(RestRequest request) { }; private ExtensionRestHandler bazHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of(new Route(Method.POST, "/baz/{moon}/qux"), new Route(Method.PUT, "/bar/baz")); + public List routes() { + return List.of( + new NamedRoute.Builder().method(Method.POST).path("/baz/{moon}/qux").uniqueName("bar_qux_for_moon").build(), + new NamedRoute.Builder().method(Method.PUT).path("/bar/baz").uniqueName("bar_baz").build() + ); } @Override @@ -92,8 +95,8 @@ public void testRegisterConflicts() { // Can't register same exact name ExtensionRestHandler duplicateFooHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of(new Route(Method.GET, "/foo")); + public List routes() { + return List.of(new NamedRoute.Builder().method(Method.GET).path("/foo").uniqueName("foo").build()); } @Override @@ -105,8 +108,8 @@ public ExtensionRestResponse handleRequest(RestRequest request) { // Can't register conflicting named wildcards, even if method is different ExtensionRestHandler barNoneHandler = new ExtensionRestHandler() { @Override - public List routes() { - return List.of(new Route(Method.GET, "/bar/{none}")); + public List routes() { + return List.of(new NamedRoute.Builder().method(Method.GET).path("/bar/{none}").uniqueName("bar_none").build()); } @Override @@ -181,6 +184,11 @@ public void testGetRegisteredReplacedPaths() { @Test public void testRestPathToString() { - assertEquals("GET /foo", ExtensionRestPathRegistry.restPathToString(Method.GET, "/foo")); + assertEquals("GET /foo", ExtensionRestPathRegistry.restPathToString(Method.GET, "/foo", "", Collections.emptySet())); + } + + @Test + public void testRestPathWithNameToString() { + assertEquals("GET /foo foo", ExtensionRestPathRegistry.restPathToString(Method.GET, "/foo", "foo", Collections.emptySet())); } } diff --git a/src/test/java/org/opensearch/sdk/rest/TestNamedRouteHandler.java b/src/test/java/org/opensearch/sdk/rest/TestNamedRouteHandler.java new file mode 100644 index 00000000..cf557d11 --- /dev/null +++ b/src/test/java/org/opensearch/sdk/rest/TestNamedRouteHandler.java @@ -0,0 +1,45 @@ +/* + * 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.sdk.rest; + +import org.opensearch.extensions.rest.ExtensionRestResponse; +import org.opensearch.rest.NamedRoute; +import org.opensearch.rest.RestStatus; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Collections; + +import static org.opensearch.rest.RestRequest.Method.GET; + +public class TestNamedRouteHandler extends OpenSearchTestCase { + public void testUnnamedRouteHandler() { + assertThrows( + IllegalArgumentException.class, + () -> new NamedRoute.Builder().method(GET) + .path("/foo/bar") + .handler(req -> new ExtensionRestResponse(req, RestStatus.OK, "content")) + .uniqueName("") + .legacyActionNames(Collections.emptySet()) + .build() + ); + } + + public void testNamedRouteHandler() { + NamedRoute nr = new NamedRoute.Builder().method(GET) + .path("/foo/bar") + .handler(req -> new ExtensionRestResponse(req, RestStatus.OK, "content")) + .uniqueName("") + .legacyActionNames(Collections.emptySet()) + .build(); + + assertEquals("foo", nr.name()); + assertNotNull(nr.handler()); + } +} diff --git a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java index 8c4ee0a2..f45930f8 100644 --- a/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java +++ b/src/test/java/org/opensearch/sdk/sample/helloworld/rest/TestRestHelloAction.java @@ -16,7 +16,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.opensearch.rest.RestHandler.Route; +import org.opensearch.rest.NamedRoute; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; import org.opensearch.common.bytes.BytesArray; @@ -47,7 +47,7 @@ public void setUp() throws Exception { @Test public void testRoutes() { - List routes = restHelloAction.routes(); + List routes = restHelloAction.routes(); assertEquals(4, routes.size()); assertEquals(Method.GET, routes.get(0).getMethod()); assertEquals("/hello", routes.get(0).getPath());