Graduate resource sharing feature out of experimental - #6348
Graduate resource sharing feature out of experimental#6348DarshitChanpura wants to merge 1 commit into
Conversation
PR Reviewer Guide 🔍(Review updated until commit 4954a66)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 4954a66
Previous suggestionsSuggestions up to commit 739106b
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6348 +/- ##
==========================================
- Coverage 75.45% 75.40% -0.05%
==========================================
Files 456 456
Lines 29924 29924
Branches 4530 4530
==========================================
- Hits 22578 22565 -13
- Misses 5251 5264 +13
Partials 2095 2095
🚀 New features to boost your workflow:
|
Renames the resource sharing feature-flag settings, dropping the
'.experimental' segment (breaking change, no deprecated fallback):
plugins.security.experimental.resource_sharing.enabled
-> plugins.security.resource_sharing.enabled
plugins.security.experimental.resource_sharing.protected_types
-> plugins.security.resource_sharing.protected_types
Also removes the @opensearch.experimental annotations from the
resource-sharing SPI and implementation classes, and updates
RESOURCE_SHARING_AND_ACCESS_CONTROL.md accordingly.
Related to opensearch-project#4500
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
739106b to
4954a66
Compare
|
Persistent review updated to latest commit 4954a66 |
|
@DarshitChanpura until there's a clear path with workspaces like proposed in #6119 I don't think this should be promoted out of experimental. Let's get a consistent platform level experience for sharing. I'm very strongly supportive of the ownership model where owner gets to choose sharing and I think the fact that workspaces was developed in a way where workspaces have owners that invite collaborators is testament to that being the desired model. Workspace collaborators being controlled by the frontend is a problem and we need to figure out the path forward that works with workspaces desire to be supported with MDS (Multiple Data Sources). If that problem can be solved then there's clear line-of-sight to graduating from experimental. Edit: @DarshitChanpura can we focus the efforts on adding a share button inside a dashboards plugin (like reporting) that at least with multi-tenancy is tenant aware. idk if reporting plugin is workspace aware atm and I know some teams had this notion of workspace acl (how does that work?) If we can show MDS working with resource sharing where the currently toggled cluster supports resource sharing and share button appears than that would probably be a good place to begin. We certainly are going to want to work backwards from UX so that these share APIs can be conveniently called from the frontend and we should also have something similar to #6107 to power new experiences on Dashboards where we can enable/disable actions based on user permissions. |
Hahaa, here is the centralized share button PR: opensearch-project/security-dashboards-plugin#2491. I've used AD as example but you get the gist.
While I agree that workspaces could leverage from the ownership model, I disagree on not taking this feature out of experimental. We should not delay this feature by tying it with workspaces improvement. |
Description
Graduates the Resource Sharing & Access Control feature out of experimental.
Breaking change (no deprecated fallback): the feature-flag settings are renamed to drop the
.experimentalsegment:plugins.security.experimental.resource_sharing.enabledplugins.security.resource_sharing.enabledplugins.security.experimental.resource_sharing.protected_typesplugins.security.resource_sharing.protected_typesOnly the string values in
ConfigConstantschange — the constant names (OPENSEARCH_RESOURCE_SHARING_ENABLED,OPENSEARCH_RESOURCE_SHARING_PROTECTED_TYPES) are unchanged, so downstream Java references are unaffected.Also:
@opensearch.experimentaljavadoc annotations from the resource-sharing SPI and implementation classes (leaves the unrelatedSecurityConfigExtensionmarker in place).RESOURCE_SHARING_AND_ACCESS_CONTROL.md(setting keys + drops the "experimental" note).Because the rename is breaking and has no fallback, this must ship in 3.8 together with the downstream references. Coordinated changes are being made to:
documentation-website(setting keys + removal of experimental warning callouts)reporting,anomaly-detection,ml-commonsDraft while those companion PRs are prepared and reviewed.
Related Issues
Related to #4500
Check List
--signoffBy 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.