Register new routes via SDK as named routes

[DarshitChanpura]

Description

Continuation of #622 This change allows registering extension routes as NamedRoute (new, deprecated, replaced), and updates sample helloworld extension to conform to the new route registration mechanism to optionally add legacy permission actions (i.e. cluster:admin/opensearch/.....)

Related PRs:

• Blocked by: https://github.com/opensearch-project/OpenSearch/pull/7957
• https://github.com/opensearch-project/security/pull/2753

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[codecov[bot]]

Codecov Report

Merging #827 (8863e29) into main (eff7635) will decrease coverage by 43.58%. The diff coverage is 0.00%.

:exclamation: Current head 8863e29 differs from pull request most recent head 56a0946. Consider uploading reports for the commit 56a0946 to get more accurate results

@@             Coverage Diff              @@
##               main    #827       +/-   ##
============================================
- Coverage     43.57%   0.00%   -43.58%     
============================================
  Files            69      71        +2     
  Lines          1983    2034       +51     
  Branches        141     149        +8     
============================================
- Hits            864       0      -864     
- Misses         1101    2034      +933     
+ Partials         18       0       -18     

Impacted Files	Coverage Δ
...ain/java/org/opensearch/sdk/ExtensionSettings.java	0.00% <0.00%> (-84.10%)	:arrow_down:
...main/java/org/opensearch/sdk/ExtensionsRunner.java	0.00% <0.00%> (-75.73%)	:arrow_down:
src/main/java/org/opensearch/sdk/SDKClient.java	0.00% <0.00%> (-90.23%)	:arrow_down:
...ain/java/org/opensearch/sdk/SDKClusterService.java	0.00% <0.00%> (-100.00%)	:arrow_down:
...n/java/org/opensearch/sdk/SDKTransportService.java	0.00% <0.00%> (-47.62%)	:arrow_down:
...h/sdk/handlers/ClusterSettingsResponseHandler.java	0.00% <ø> (-25.00%)	:arrow_down:
...arch/sdk/handlers/ClusterStateResponseHandler.java	0.00% <ø> (-35.30%)	:arrow_down:
...k/handlers/EnvironmentSettingsResponseHandler.java	0.00% <ø> (-52.95%)	:arrow_down:
...k/handlers/ExtensionDependencyResponseHandler.java	0.00% <ø> (ø)
...rch/sdk/handlers/ExtensionsInitRequestHandler.java	0.00% <0.00%> (-96.88%)	:arrow_down:
... and 8 more

... and 36 files with indirect coverage changes

[DarshitChanpura]

Gradle check compileJava fails as it needs core artifact with changes from this: https://github.com/opensearch-project/OpenSearch/pull/7957 merged

[DarshitChanpura]

@dbwiddis I've removed references to named handlers for deprecated and replaced routes.

[DarshitChanpura]

@dbwiddis Would you please re-review?

The build seems to broken due to this change: https://github.com/opensearch-project/OpenSearch/commit/52a5e3f6e0ca599e3193807134ea42660ecdd195

[DarshitChanpura]

@dbwiddis Would you please re-review?

The build seems to broken due to this change: opensearch-project/OpenSearch@52a5e3f

Added a fix.

[dbwiddis]

@dbwiddis Would you please re-review? The build seems to broken due to this change: opensearch-project/OpenSearch@52a5e3f

Added a fix.

Yep, I'll re-review and thank you for fixing that! I was playing around off and on fixing up the sample extension that this documents, and encountered that same problem. I just deleted the offending lines. ;)

BTW, updated sample at https://github.com/dbwiddis/CRUDExtension I am working on updating the docs starting with your version in this PR

[DarshitChanpura]

@dbwiddis @owaiskazi19 Would you please review this again?

[opensearch-trigger-bot[bot]]

The backport to 1.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/opensearch-sdk-java/backport-1.x 1.x
# Navigate to the new working tree
pushd ../.worktrees/opensearch-sdk-java/backport-1.x
# Create a new branch
git switch --create backport/backport-827-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 09c22b08d10b70b036e9176345d18f645e509e7e
# Push it to GitHub
git push --set-upstream origin backport/backport-827-to-1.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/opensearch-sdk-java/backport-1.x

Then, create a pull request where the base branch is 1.x and the compare/head branch is backport/backport-827-to-1.x.

[dbwiddis]

@DarshitChanpura can you please create a backport PR?

[joshpalis]

@DarshitChanpura Can you confirm that the hello world extension can be started with these new changes. I am seeing errors when running ./gradlew helloWorld

19:15:52.052 [main] DEBUG org.opensearch.threadpool.ThreadPool - created thread pool: name [write], size [8], queue size [10k]
19:15:52.052 [main] DEBUG org.opensearch.threadpool.ThreadPool - created thread pool: name [snapshot], core [1], max [4], keep alive [5m]
19:15:52.053 [main] DEBUG org.opensearch.threadpool.ThreadPool - created thread pool: name [search_throttled], size [1], queue size [100]
Exception in thread "main" OpenSearchException[Invalid route name specified. The route name may include the following characters 'a-z', 'A-Z', '0-9', ':', '/', '*', '_' and be less than 250 characters]
        at org.opensearch.rest.NamedRoute.<init>(NamedRoute.java:143)
        at org.opensearch.rest.NamedRoute$Builder.build(NamedRoute.java:112)
        at org.opensearch.sdk.sample.helloworld.rest.RestHelloAction.routes(RestHelloAction.java:71)
        at org.opensearch.sdk.rest.ExtensionRestPathRegistry.registerHandler(ExtensionRestPathRegistry.java:43)
        at org.opensearch.sdk.ExtensionsRunner.<init>(ExtensionsRunner.java:240)
        at org.opensearch.sdk.ExtensionsRunner.run(ExtensionsRunner.java:583)
        at org.opensearch.sdk.sample.helloworld.HelloWorldExtension.main(HelloWorldExtension.java:82)

[dbwiddis]

@DarshitChanpura Can you confirm that the hello world extension can be started with these new changes. I am seeing errors when running ./gradlew helloWorld

The problem is the route prefix code:

protected static String routePrefix(String name) {
    return extensionName + ":" + name;
}

It uses the extension name without checking it; the hello world extension name (per the .yml file) is

extensionName: hello-world

The - is not allowed.

[DarshitChanpura]

Aah. I've tested it using the short hand hw and didn't run into this error. I can create a PR to rename this to hello_world which will work.

[owaiskazi19]

Aah. I've tested it using the short hand hw and didn't run into this error. I can create a PR to rename this to hello_world which will work.

The correct way of adding a route would be helloWorld rather with a - or _. Can you rename it with the same @DarshitChanpura?

[DarshitChanpura]

Aah. I've tested it using the short hand hw and didn't run into this error. I can create a PR to rename this to hello_world which will work.

The correct way of adding a route would be helloWorld rather with a - or _. Can you rename it with the same @DarshitChanpura?

Are you suggesting renaming the demo extension name to helloWorld?

[DarshitChanpura]

@dbwiddis @joshpalis This PR https://github.com/opensearch-project/opensearch-sdk-java/pull/868 will resolve the issue.