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

Create NamedRoute to map extension routes to a shortened name #6870

Merged
merged 59 commits into from
May 18, 2023

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Mar 29, 2023

Description

This PR will allow the naming of a Route to something that can be placed in a role definition.

i.e. GET /_extensions/_hw/hello becomes hw:greet. I have been thinking about this as REST action names which would be analogous to naming Transport actions.

A role definition in the security plugin currently lists action names granted to a role, but for extensions there is no transport action that is executed so there is currently no short name that can be permitted in a roles definition. This PR creates a concept of a ProtecteedRoute (a named route) with the purpose of creating a shortened name for a route that can be granted in a roles definition similar to:

extension_hw_full:
  reserved: true
  cluster_permissions:
    - 'hw:greet'
    - 'hw:greet_with_adjective'
    - 'hw:great_with_name'
    - 'hw:goodbye'

Issues Resolved

Related to: opensearch-project/security#2589

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testSearchAggregationWithNetworkDisruption_FailOpenEnabled

@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2023

Codecov Report

Merging #6870 (ccbe4b5) into main (ff138cb) will decrease coverage by 0.14%.
The diff coverage is 82.27%.

❗ Current head ccbe4b5 differs from pull request most recent head 98a9d44. Consider uploading reports for the commit 98a9d44 to get more accurate results

@@             Coverage Diff              @@
##               main    #6870      +/-   ##
============================================
- Coverage     70.74%   70.60%   -0.14%     
- Complexity    56023    59837    +3814     
============================================
  Files          4670     4898     +228     
  Lines        265699   286970   +21271     
  Branches      39015    41364    +2349     
============================================
+ Hits         187958   202615   +14657     
- Misses        61807    67656    +5849     
- Partials      15934    16699     +765     
Impacted Files Coverage Δ
...a/org/opensearch/extensions/ExtensionsManager.java 47.69% <33.33%> (-0.19%) ⬇️
...src/main/java/org/opensearch/rest/RestHandler.java 62.26% <45.45%> (-4.41%) ⬇️
.../main/java/org/opensearch/action/ActionModule.java 94.97% <82.14%> (-0.74%) ⬇️
...a/org/opensearch/extensions/rest/RouteHandler.java 90.00% <90.00%> (ø)
...rch/extensions/rest/RestActionsRequestHandler.java 100.00% <100.00%> (ø)
.../src/main/java/org/opensearch/rest/NamedRoute.java 100.00% <100.00%> (ø)
...rch/rest/extensions/RestSendToExtensionAction.java 47.45% <100.00%> (ø)

... and 736 files with indirect coverage changes

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.index.remote.RemoteRefreshSegmentPressureServiceTests.testValidateSegmentUploadLag

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@cwperks
Copy link
Member Author

cwperks commented May 18, 2023

Thank you for all of the help on this one @reta!

Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM! Minor comments. Thanks @cwperks for the patience and for addressing the comments.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@cwperks
Copy link
Member Author

cwperks commented May 18, 2023

@owaiskazi19 Final 👀 ?

@owaiskazi19 owaiskazi19 added the backport 2.x Backport to 2.x branch label May 18, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      3 org.opensearch.remotestore.RemoteStoreRefreshListenerIT.testRemoteRefreshRetryOnFailure

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@owaiskazi19 owaiskazi19 merged commit 8470df6 into opensearch-project:main May 18, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 18, 2023
* WIP on rest layer authz

Signed-off-by: Craig Perkins <[email protected]>

* Create PermissibleRoute

Signed-off-by: Craig Perkins <[email protected]>

* Update extension handshake

Signed-off-by: Craig Perkins <[email protected]>

* Add connectToNodeAsExtension in TransportService

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Update RouteHandler

Signed-off-by: Craig Perkins <[email protected]>

* Update java docstrings

Signed-off-by: Craig Perkins <[email protected]>

* Run spotlessApply

Signed-off-by: Craig Perkins <[email protected]>

* Fix merge conflicts

Signed-off-by: Craig Perkins <[email protected]>

* Rename to ProtectedRoute

Signed-off-by: Craig Perkins <[email protected]>

* Create method to get extension settings from extensions.yml

Signed-off-by: Craig Perkins <[email protected]>

* Add ExtensionsManager.lookupExtensionSettings

Signed-off-by: Craig Perkins <[email protected]>

* Small change to name

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Move extensionSettingsMap.put

Signed-off-by: Craig Perkins <[email protected]>

* Re-run CI

Signed-off-by: Craig Perkins <[email protected]>

* Address review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Add test for ProtectedRoute

Signed-off-by: Craig Perkins <[email protected]>

* spotlessApply

Signed-off-by: Craig Perkins <[email protected]>

* Add RouteHandlerTests

Signed-off-by: Craig Perkins <[email protected]>

* Switch to NamedRoute and add validation for action naming

Signed-off-by: Craig Perkins <[email protected]>

* Avoid magic numbers

Signed-off-by: Craig Perkins <[email protected]>

* Remove @test annotation

Signed-off-by: Craig Perkins <[email protected]>

* Address code review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Update error message

Signed-off-by: Craig Perkins <[email protected]>

* Check for REST Action name uniqueness across all registered actions

Signed-off-by: Craig Perkins <[email protected]>

* minimize code in the test

Signed-off-by: Craig Perkins <[email protected]>

* Update changelog

Signed-off-by: Craig Perkins <[email protected]>

* Add DynamicRouteRegistry

Signed-off-by: Craig Perkins <[email protected]>

* Address code review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Add mock DynamicRouteRegistry.class

Signed-off-by: Craig Perkins <[email protected]>

* Add RouteRegistry to DynamicActionModule

Signed-off-by: Craig Perkins <[email protected]>

* Pass around dynamicActionRegistry instead of ActionModule

Signed-off-by: Craig Perkins <[email protected]>

* Only pass dynamic action registry

Signed-off-by: Craig Perkins <[email protected]>

* Add DynamicActionRegistryTests for tests of dynamic registry

Signed-off-by: Craig Perkins <[email protected]>

* Move CHANGELOG entry

Signed-off-by: Craig Perkins <[email protected]>

---------

Signed-off-by: Craig Perkins <[email protected]>
(cherry picked from commit 8470df6)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta pushed a commit that referenced this pull request May 18, 2023
…#7631)

* WIP on rest layer authz



* Create PermissibleRoute



* Update extension handshake



* Add connectToNodeAsExtension in TransportService



* Add to CHANGELOG



* Add to CHANGELOG



* Update RouteHandler



* Update java docstrings



* Run spotlessApply



* Fix merge conflicts



* Rename to ProtectedRoute



* Create method to get extension settings from extensions.yml



* Add ExtensionsManager.lookupExtensionSettings



* Small change to name



* Add to CHANGELOG



* Move extensionSettingsMap.put



* Re-run CI



* Address review feedback



* Add test for ProtectedRoute



* spotlessApply



* Add RouteHandlerTests



* Switch to NamedRoute and add validation for action naming



* Avoid magic numbers



* Remove @test annotation



* Address code review feedback



* Update error message



* Check for REST Action name uniqueness across all registered actions



* minimize code in the test



* Update changelog



* Add DynamicRouteRegistry



* Address code review feedback



* Add mock DynamicRouteRegistry.class



* Add RouteRegistry to DynamicActionModule



* Pass around dynamicActionRegistry instead of ActionModule



* Only pass dynamic action registry



* Add DynamicActionRegistryTests for tests of dynamic registry



* Move CHANGELOG entry



---------


(cherry picked from commit 8470df6)

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
bharath-techie pushed a commit to bharath-techie/OpenSearch that referenced this pull request May 23, 2023
…arch-project#6870)

* WIP on rest layer authz

Signed-off-by: Craig Perkins <[email protected]>

* Create PermissibleRoute

Signed-off-by: Craig Perkins <[email protected]>

* Update extension handshake

Signed-off-by: Craig Perkins <[email protected]>

* Add connectToNodeAsExtension in TransportService

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Update RouteHandler

Signed-off-by: Craig Perkins <[email protected]>

* Update java docstrings

Signed-off-by: Craig Perkins <[email protected]>

* Run spotlessApply

Signed-off-by: Craig Perkins <[email protected]>

* Fix merge conflicts

Signed-off-by: Craig Perkins <[email protected]>

* Rename to ProtectedRoute

Signed-off-by: Craig Perkins <[email protected]>

* Create method to get extension settings from extensions.yml

Signed-off-by: Craig Perkins <[email protected]>

* Add ExtensionsManager.lookupExtensionSettings

Signed-off-by: Craig Perkins <[email protected]>

* Small change to name

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Move extensionSettingsMap.put

Signed-off-by: Craig Perkins <[email protected]>

* Re-run CI

Signed-off-by: Craig Perkins <[email protected]>

* Address review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Add test for ProtectedRoute

Signed-off-by: Craig Perkins <[email protected]>

* spotlessApply

Signed-off-by: Craig Perkins <[email protected]>

* Add RouteHandlerTests

Signed-off-by: Craig Perkins <[email protected]>

* Switch to NamedRoute and add validation for action naming

Signed-off-by: Craig Perkins <[email protected]>

* Avoid magic numbers

Signed-off-by: Craig Perkins <[email protected]>

* Remove @test annotation

Signed-off-by: Craig Perkins <[email protected]>

* Address code review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Update error message

Signed-off-by: Craig Perkins <[email protected]>

* Check for REST Action name uniqueness across all registered actions

Signed-off-by: Craig Perkins <[email protected]>

* minimize code in the test

Signed-off-by: Craig Perkins <[email protected]>

* Update changelog

Signed-off-by: Craig Perkins <[email protected]>

* Add DynamicRouteRegistry

Signed-off-by: Craig Perkins <[email protected]>

* Address code review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Add mock DynamicRouteRegistry.class

Signed-off-by: Craig Perkins <[email protected]>

* Add RouteRegistry to DynamicActionModule

Signed-off-by: Craig Perkins <[email protected]>

* Pass around dynamicActionRegistry instead of ActionModule

Signed-off-by: Craig Perkins <[email protected]>

* Only pass dynamic action registry

Signed-off-by: Craig Perkins <[email protected]>

* Add DynamicActionRegistryTests for tests of dynamic registry

Signed-off-by: Craig Perkins <[email protected]>

* Move CHANGELOG entry

Signed-off-by: Craig Perkins <[email protected]>

---------

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Bharathwaj G <[email protected]>
stephen-crawford pushed a commit to stephen-crawford/OpenSearch that referenced this pull request May 31, 2023
…arch-project#6870)

* WIP on rest layer authz

Signed-off-by: Craig Perkins <[email protected]>

* Create PermissibleRoute

Signed-off-by: Craig Perkins <[email protected]>

* Update extension handshake

Signed-off-by: Craig Perkins <[email protected]>

* Add connectToNodeAsExtension in TransportService

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Update RouteHandler

Signed-off-by: Craig Perkins <[email protected]>

* Update java docstrings

Signed-off-by: Craig Perkins <[email protected]>

* Run spotlessApply

Signed-off-by: Craig Perkins <[email protected]>

* Fix merge conflicts

Signed-off-by: Craig Perkins <[email protected]>

* Rename to ProtectedRoute

Signed-off-by: Craig Perkins <[email protected]>

* Create method to get extension settings from extensions.yml

Signed-off-by: Craig Perkins <[email protected]>

* Add ExtensionsManager.lookupExtensionSettings

Signed-off-by: Craig Perkins <[email protected]>

* Small change to name

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Move extensionSettingsMap.put

Signed-off-by: Craig Perkins <[email protected]>

* Re-run CI

Signed-off-by: Craig Perkins <[email protected]>

* Address review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Add test for ProtectedRoute

Signed-off-by: Craig Perkins <[email protected]>

* spotlessApply

Signed-off-by: Craig Perkins <[email protected]>

* Add RouteHandlerTests

Signed-off-by: Craig Perkins <[email protected]>

* Switch to NamedRoute and add validation for action naming

Signed-off-by: Craig Perkins <[email protected]>

* Avoid magic numbers

Signed-off-by: Craig Perkins <[email protected]>

* Remove @test annotation

Signed-off-by: Craig Perkins <[email protected]>

* Address code review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Update error message

Signed-off-by: Craig Perkins <[email protected]>

* Check for REST Action name uniqueness across all registered actions

Signed-off-by: Craig Perkins <[email protected]>

* minimize code in the test

Signed-off-by: Craig Perkins <[email protected]>

* Update changelog

Signed-off-by: Craig Perkins <[email protected]>

* Add DynamicRouteRegistry

Signed-off-by: Craig Perkins <[email protected]>

* Address code review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Add mock DynamicRouteRegistry.class

Signed-off-by: Craig Perkins <[email protected]>

* Add RouteRegistry to DynamicActionModule

Signed-off-by: Craig Perkins <[email protected]>

* Pass around dynamicActionRegistry instead of ActionModule

Signed-off-by: Craig Perkins <[email protected]>

* Only pass dynamic action registry

Signed-off-by: Craig Perkins <[email protected]>

* Add DynamicActionRegistryTests for tests of dynamic registry

Signed-off-by: Craig Perkins <[email protected]>

* Move CHANGELOG entry

Signed-off-by: Craig Perkins <[email protected]>

---------

Signed-off-by: Craig Perkins <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…arch-project#6870)

* WIP on rest layer authz

Signed-off-by: Craig Perkins <[email protected]>

* Create PermissibleRoute

Signed-off-by: Craig Perkins <[email protected]>

* Update extension handshake

Signed-off-by: Craig Perkins <[email protected]>

* Add connectToNodeAsExtension in TransportService

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Update RouteHandler

Signed-off-by: Craig Perkins <[email protected]>

* Update java docstrings

Signed-off-by: Craig Perkins <[email protected]>

* Run spotlessApply

Signed-off-by: Craig Perkins <[email protected]>

* Fix merge conflicts

Signed-off-by: Craig Perkins <[email protected]>

* Rename to ProtectedRoute

Signed-off-by: Craig Perkins <[email protected]>

* Create method to get extension settings from extensions.yml

Signed-off-by: Craig Perkins <[email protected]>

* Add ExtensionsManager.lookupExtensionSettings

Signed-off-by: Craig Perkins <[email protected]>

* Small change to name

Signed-off-by: Craig Perkins <[email protected]>

* Add to CHANGELOG

Signed-off-by: Craig Perkins <[email protected]>

* Move extensionSettingsMap.put

Signed-off-by: Craig Perkins <[email protected]>

* Re-run CI

Signed-off-by: Craig Perkins <[email protected]>

* Address review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Add test for ProtectedRoute

Signed-off-by: Craig Perkins <[email protected]>

* spotlessApply

Signed-off-by: Craig Perkins <[email protected]>

* Add RouteHandlerTests

Signed-off-by: Craig Perkins <[email protected]>

* Switch to NamedRoute and add validation for action naming

Signed-off-by: Craig Perkins <[email protected]>

* Avoid magic numbers

Signed-off-by: Craig Perkins <[email protected]>

* Remove @test annotation

Signed-off-by: Craig Perkins <[email protected]>

* Address code review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Update error message

Signed-off-by: Craig Perkins <[email protected]>

* Check for REST Action name uniqueness across all registered actions

Signed-off-by: Craig Perkins <[email protected]>

* minimize code in the test

Signed-off-by: Craig Perkins <[email protected]>

* Update changelog

Signed-off-by: Craig Perkins <[email protected]>

* Add DynamicRouteRegistry

Signed-off-by: Craig Perkins <[email protected]>

* Address code review feedback

Signed-off-by: Craig Perkins <[email protected]>

* Add mock DynamicRouteRegistry.class

Signed-off-by: Craig Perkins <[email protected]>

* Add RouteRegistry to DynamicActionModule

Signed-off-by: Craig Perkins <[email protected]>

* Pass around dynamicActionRegistry instead of ActionModule

Signed-off-by: Craig Perkins <[email protected]>

* Only pass dynamic action registry

Signed-off-by: Craig Perkins <[email protected]>

* Add DynamicActionRegistryTests for tests of dynamic registry

Signed-off-by: Craig Perkins <[email protected]>

* Move CHANGELOG entry

Signed-off-by: Craig Perkins <[email protected]>

---------

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants