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

Adds a sample resource plugin to demonstrate resource access control in action #4893

Draft
wants to merge 1 commit into
base: feature/resource-permissions
Choose a base branch
from

Conversation

DarshitChanpura
Copy link
Member

Description

This PR introduces a sample plugin aimed at demonstrating the usage of resource access control in action. Resource access control is introduced in #4746

Issues Resolved

[TBD]

Testing

Automated + Manual

Check List

  • New functionality includes testing
    - [ ] New functionality has been documented
    - [ ] New Roles/Permissions have a corresponding security dashboards plugin PR
    - [ ] API changes companion pull request created
  • Commits are signed per the DCO using --signoff

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.

…in action

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.43%. Comparing base (3c635c9) to head (543efbd).

Additional details and impacted files

Impacted file tree graph

@@                      Coverage Diff                      @@
##           feature/resource-permissions    #4893   +/-   ##
=============================================================
  Coverage                         71.43%   71.43%           
=============================================================
  Files                               334      334           
  Lines                             22509    22509           
  Branches                           3582     3582           
=============================================================
+ Hits                              16079    16080    +1     
  Misses                             4638     4638           
+ Partials                           1792     1791    -1     

see 5 files with indirect coverage changes

@@ -0,0 +1 @@
org.opensearch.sample.SampleResourcePlugin
Copy link
Member

Choose a reason for hiding this comment

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

Is there a SPI that this file is associated with?

protected void doExecute(Task task, ListAccessibleResourcesRequest request, ActionListener<ListAccessibleResourcesResponse> listener) {
try {
ResourceService rs = SampleResourcePlugin.GuiceHolder.getResourceService();
List<String> resourceIds = rs.getResourceAccessControlPlugin().listAccessibleResourcesInPlugin(RESOURCE_INDEX_NAME);
Copy link
Member

Choose a reason for hiding this comment

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

Should we consider giving back resource objects here?

@cwperks
Copy link
Member

cwperks commented Nov 11, 2024

Is there any way we can add tests and run them as a CI check for the sample plugin? (preferably w/o relying on core-side changes bc its not clear when core-side changes can be merged)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants