Allow action prefixes to be registered from plugins#20913
Allow action prefixes to be registered from plugins#20913cwperks wants to merge 8 commits intoopensearch-project:mainfrom
Conversation
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
PR Code Analyzer ❗AI-powered 'Code-Diff-Analyzer' found issues on commit d258baa.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
PR Code Analyzer ❗AI-powered 'Code-Diff-Analyzer' found issues on commit 28aa469.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
PR Code Analyzer ❗AI-powered 'Code-Diff-Analyzer' found issues on commit 28aa469.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
PR Code Analyzer ❗AI-powered 'Code-Diff-Analyzer' found issues on commit a91ccd2.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
PR Reviewer Guide 🔍(Review updated until commit e4cccc0)Here are some key observations to aid the review process:
|
| for (Route route : handler.routes()) { | ||
| if (route instanceof NamedRoute namedRoute) { | ||
| for (String actionName : namedRoute.actionNames()) { | ||
| if (!TransportService.isValidActionName(actionName)) { |
There was a problem hiding this comment.
This logic retains the validation currently in NamedRoute but enforces on node bootstrap.
This line is equivalent to this.legacyActionNames.addAll(validateLegacyActionNames(legacyActionNames));
PR Code Suggestions ✨Latest suggestions up to e4cccc0
Previous suggestionsSuggestions up to commit a91ccd2
|
|
❌ Gradle check result for a91ccd2: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for a91ccd2: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Latest failure |
|
❌ Gradle check result for a91ccd2: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for a91ccd2: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Persistent review updated to latest commit e4cccc0 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #20913 +/- ##
============================================
+ Coverage 73.24% 73.29% +0.05%
- Complexity 72510 72555 +45
============================================
Files 5819 5819
Lines 331373 331391 +18
Branches 47882 47887 +5
============================================
+ Hits 242707 242887 +180
+ Misses 69201 68979 -222
- Partials 19465 19525 +60 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Companion PR in Security plugin to showcase how this can be used: opensearch-project/security#6020
The changes in this PR allow action prefixes to be registered from plugins. This gives more flexibility in action naming to help make more readable action-names (especially in the context of resource sharing).
Action names with custom prefix would be used for actions pertaining to a single resource (a single doc containing metadata in a system index)
Take these examples proposed for the reporting plugin:
cluster:admin/opendistro/reports/instance/get->report_instance:getcluster:admin/opendistro/reports/instance/list-> this would stay the same as a cluster_permission as it does not pertain to a single resourcecluster:admin/opendistro/reports/menu/download->report_instance:downloadBy adding custom prefixes, we get more flexibility with action naming in an aim to simplify how our current permissions system works.
Related Issues
Related to opensearch-project/security#4500
Check List
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.