Add Extension API interfaces and implement in sample Hello World extension

[dbwiddis]

Description

This PR creates new interfaces Extension and ExtensionRestHandler which developers will implement when writing new Extensions, and demonstrates their use in a sample HelloWorldExtension which responds as expected to its registered OpenSearch REST request:

[~/git/OpenSearch] % curl -X GET localhost:9200/_extensions/_opensearch-sdk-1/hello
Hello, World!%

The Extension interface requires two methods to be implemented by developers.

• getExtensionSettings() which the ExtensionsRunner needs to bind the host and port.
• getExtensionRestHandlers() which obtains a list of objects implementing the ExtensionRestHandler interface, which will handle the REST requests registered with OpenSearch.

The ExtensionRestHandler interface requires two methods to be implemented by developers.

• routes() which corresponds directly to the routes() method required by the RestHandler interface implemented by plugins.
• handleRequest() which is analogous to the logic eventually called by the RestHandler's handleRequest() method, implemented in plugins with the prepareRequest() method.

#102 discussed possibly implementing the RestHandler interface in extensions to speed migrating plugins, but presented the following challenges:

• The full method signature of prepareRequest() is public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client). This involves a Channel and Client to handle a RestRequest:
  • The channel and end user are on the OpenSearch side of the transport mechanism so can't be used directly
  • The RestRequest isn't yet configured to be deserialized to send over transport
    • Even if we add that, it includes HTTP headers, potentially including basic authentication (user/password). For security purposes these are not being sent to extensions, and it's difficult or impossible to remove or filter them from an existing object.
• We could create new Channel, Client, and RestRequest objects with appropriate information, but that introduces a lot of complexity and overhead (and duplication) when instead we can just send and return only the information that we really need to.

The ExtensionRestHandler interface still corresponds very closely to the Plugin action implementation of BaseRestHandler.

Presently the action method takes only the method and URI and returns a String, as the scope of this PR is to implement that functionality. This signature is envisioned to change over time as we add new features. For example:

• We will probably add a new field for communicating a lookup key for an Identity object as discussed in Add support for using User Identity in extensions #37
• We will need to handle URL queries with parameters. I've added [FEATURE] Pass REST params to extensions #111 to address this.
• We will eventually need to change the return type to an object encapsulating both a message and a REST response code as we implement more actions. I've added [FEATURE] Return HTTP Status from Extensions #112 to address this.
• We will possibly need some support for NamedWriteables added (not sure yet).
• Other items from the RestRequest object (excluding the HTML headers) may be relevant (not sure yet).

In addition to the interface implementations and sample extension, this PR also:

• Makes multiple changes in ExtensionsRunner to:
  • Add a run() method to instantiate using the settings of an Extension.
  • Use the Extension's settings to configure its hostaddress/port
• Removes the ExtensionRestPaths class (and yml and test files) as it is replaced by the Extension API implementation (and tests).
• Moves the extension.yml file to the classpath at /sample/extension-settings.yml as part of the sample extension implementation. This may make [BUG] Package extension.yml file #86 no longer needed.
• Updates DESIGN with details
• Changes build.gradle to execute the main() method of the sample extension rather than running ExtensionsRunner. I believe this is closer to the intent of the SDK.
• Updates the DEVELOPER_GUIDE appropriately for the additional configuration

Issues Resolved

Closes #102.

Removes the need for #86.

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.

[ryanbogan]

I agree that #86 is no longer necessary with these changes.

[joshpalis]

Besides the test name change, LGTM! Thanks for adding to the design document, the ExtensionAction diagram definitely helped me gain a better understanding of the workflow. I also agree that this change makes #86 obsolete, should I go ahead and close this bug out now?

[dbwiddis]

@owaiskazi19 and @saratvemulapalli, after reviewing #80 and #105 I can already see a need for name changes in my implementation.

What I have implemented in the ExtensionAction interface directly correlates to a RestHandler implementation, and the code enables a the equivalent of getRestHandlers(). I chose the Action name because of classes like RestIndexAnomalyDetectorAction. However, I see this is separate from getActions() which returns classes like IndexAnomalyDetectorAction <insert pun about "No Rest for the weary.">. Clearly just 'Action' is not clear enough. Some options:

• Change the interface name to ExtensionRestAction
  • Closer to the implementation naming but possibly still confusing
• Change the Interface name to ExtensionRestHandler
  • I kind of prefer this one, and also would rename getExtensionResponse() (corresponding to prepareRequest() in existing Rest*Actions) to handleRequest to align (mostly) with the RestHandler interface.

[saratvemulapalli]

@owaiskazi19 and @saratvemulapalli, after reviewing #80 and #105 I can already see a need for name changes in my implementation.

What I have implemented in the ExtensionAction interface directly correlates to a RestHandler implementation, and the code enables a the equivalent of getRestHandlers(). I chose the Action name because of classes like RestIndexAnomalyDetectorAction. However, I see this is separate from getActions() which returns classes like IndexAnomalyDetectorAction <insert pun about "No Rest for the weary.">. Clearly just 'Action' is not clear enough. Some options:

• Change the interface name to ExtensionRestAction

  • Closer to the implementation naming but possibly still confusing

• Change the Interface name to ExtensionRestHandler

  • I kind of prefer this one, and also would rename getExtensionResponse() (corresponding to prepareRequest() in existing Rest*Actions) to handleRequest to align (mostly) with the RestHandler interface.

Thanks @dbwiddis. I love these changes. Its a great start of helping extension authors write simple extensions.

Action is used in multiple ways in OpenSearch but I like the second option ExtensionRestHandler.
This keeps it simple and for all actions we can assume its always transport.

[saratvemulapalli]

Changes look good to me. I'll do another pass once we merge #101 and opensearch-project/OpenSearch#4282

[peternied]

Helloworld API awesome!

Even if we add that, it includes HTTP headers, potentially including basic authentication (user/password). For security purposes these are not being sent to extensions, and it's difficult or impossible to remove or filter them from an existing object.

Could you create an issue for this to be followed up on? One way we could manage this more securely is to deep copy then remove/unset all headers from the request before its passed along to the extension.

Might be worthwhile to ask, besides the method, path, body - do extensions need any other functionality to support a given REST request?

[peternied]

ExtensionsRunner is getting big, could we compartmentalize this functionality? Maybe there is a pattern like configuration/registration/request handling?

[saratvemulapalli]

A good problem to have :)

[dbwiddis]

Could you create an issue for this to be followed up on? One way we could manage this more securely is to deep copy then remove/unset all headers from the request before its passed along to the extension.

@peternied already created with #111 and comments on #37.

The challenge with a "deep copy" is we still have to send it across Transport, so we're stuck "disassembling" the RestRequest in any case. The only question is whether to reassemble it on the Extension end before passing it to the Extension action, or to just pass the component pieces.

For now, passing the pieces seems easiest but we can always change it (up through version 0.9.99.999) as we iterate on new extensions and see what's needed.

Might be worthwhile to ask, besides the method, path, body - do extensions need any other functionality to support a given REST request?

I think as we migrate plugins to extensions this will answer itself.

[dbwiddis]

@saratvemulapalli @ryanbogan @joshpalis @owaiskazi19

Have rebased this PR on main after merge of #101 which invalidated earlier reviews. No changes, though, would like to get this merged today before I submit yet another PR building on this, addressing #112