opensearch-project / opensearch-sdk-java

OpenSearch SDK to build and run extensions
Apache License 2.0
28 stars 58 forks source link

[FEATURE] Add the ability to use identical REST paths for plugins vs. extensions #355

Open dbwiddis opened 1 year ago

dbwiddis commented 1 year ago

Is your feature request related to a problem?

Users migrating from a plugin to an extension should ideally not have to change anything.

Currently plugin rest paths include hardcoded names such as _plugins/_anomaly_detection/foo.

Extension rest paths include the ability to specify the unique ID (e.g., ad) and change the prefix, so the same request would be _extensions/_ad/foo.

If users want to run both the plugin and extension side by side, this does not conflict. However in a migration context, we want the ability for plugin to redirect to extension.

What solution would you like?

During Extension initialization, Extensions register their paths with the RestController. This logic should be updated:

This will allow the following configurations:

What alternatives have you considered?

Do you have any additional context?

stephen-crawford commented 1 year ago

I am not sure how helpful of advice this will be, but I wanted to mention for whoever tackles this that it may be worthwhile to look at some of the principal resolution that is happening in the Identity project. Reading this description, it is somewhat similar in that you are treating the plugin name like an alias for the equivalent extension. You should be able treat it as two principals for the same subject in the case where there is only the extension running but not the plugin.

dbwiddis commented 1 year ago

Looks like we can accomplish this by implementing the replacedRoutes() extension point:

https://github.com/opensearch-project/anomaly-detection/blob/main/src/main/java/org/opensearch/ad/rest/RestAnomalyDetectorJobAction.java#L104

So we should revise rest handler registration to have the full path(s) generated on the SDK side.

saratvemulapalli commented 1 year ago

Ignore me @dbwiddis is way faster than I am :) For seamless migration for plugins RestHandler[1] interface supports replaced paths to help deprecate old APIs and migrate to new API Paths.

[1] https://github.com/opensearch-project/OpenSearch/blob/ee305d0b95e00e66dd20ab0a4ce6687df6cf8875/server/src/main/java/org/opensearch/rest/RestHandler.java#L106

dblock commented 1 year ago

Why would one want to run a plugin and an extension side-by-side on the same REST path?

One possible way to solve this is that all extensions have a configurable REST prefix and by default always register routes behind _extensions/prefix by default. Thus I could install an extension with a default _extensions/ad or _plugins/ad by supplying a --prefix parameter at installation time.

dbwiddis commented 1 year ago

Why would one want to run a plugin and an extension side-by-side on the same REST path?

They wouldn't. A single REST path goes to exactly one handler.

They might want to run them side by side during transition to evaluate them, but in that case they would use different rest paths.

One possible way to solve this is that all extensions have a configurable REST prefix and by default always register routes behind _extensions/prefix by default.

That is how it is currently implemented.

There are three scenarios:

  1. Plugin only. Status quo. /_plugin/foo path.
  2. Extension and plugin side by side (to evaluate, perhaps). Plugin keeps old path. Extension path is /_extensions/foo.
  3. Extension only. Extension path /_extensions/foo works.
    • In addition, we automatically redirect /_plugin/foo to the same handler.

This last part (the sub bullet under 3) is the only real change, in order to provide backwards compatibility with scripts / dashboards / etc. that use the old route, similarly to how we have replaced routes going to the same handler as the old one; both work.