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

[Workspace]Optimize workspace permission validation for bulk operations #7516

Conversation

wanglam
Copy link
Contributor

@wanglam wanglam commented Jul 26, 2024

Description

This PR is for optimizing saved objects's workspace permission validation logic for bulk operations. It mainly includes below three changes:

  1. Add saved objects cache related methods to permission control client to enable saved objects cache.
  2. Cache workspace saved objects when executing saved object client bulk create, bulk update and bulk get.
  3. Clear saved objects cache when saved objects cache active before repsonse

Issues Resolved

#7520

Screenshot

No UI Changes

Testing the changes

Can refer test files:

  • src/plugins/workspace/server/plugin.test.ts
  • src/plugins/workspace/server/permission_control/client.test.ts

Changelog

  • feat: [Workspace]Optimize workspace permission validation for bulk operations

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

opensearch-changeset-bot bot added a commit to wanglam/OpenSearch-Dashboards that referenced this pull request Jul 26, 2024
@wanglam wanglam force-pushed the optimize-workspace-permission-validation branch from 5c62c6b to a6521a2 Compare July 26, 2024 09:39
Copy link

codecov bot commented Jul 26, 2024

Codecov Report

Attention: Patch coverage is 93.18182% with 3 lines in your changes missing coverage. Please review.

Project coverage is 63.67%. Comparing base (352d2e9) to head (14e388b).
Report is 225 commits behind head on main.

Files with missing lines Patch % Lines
...gins/workspace/server/permission_control/client.ts 90.62% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7516      +/-   ##
==========================================
+ Coverage   63.65%   63.67%   +0.01%     
==========================================
  Files        3632     3632              
  Lines       80060    80101      +41     
  Branches    12683    12691       +8     
==========================================
+ Hits        50965    51004      +39     
  Misses      25990    25990              
- Partials     3105     3107       +2     
Flag Coverage Δ
Linux_1 30.59% <93.18%> (+0.04%) ⬆️
Linux_2 55.58% <ø> (ø)
Linux_3 40.56% <ø> (-0.01%) ⬇️
Linux_4 31.30% <ø> (ø)
Windows_1 30.61% <93.18%> (+0.04%) ⬆️
Windows_2 55.53% <ø> (ø)
Windows_3 40.56% <ø> (ø)
Windows_4 31.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wanglam wanglam force-pushed the optimize-workspace-permission-validation branch from a6521a2 to e8b4b17 Compare July 26, 2024 10:49
@wanglam wanglam changed the title [Workspace]Optimize workspace permission validate for bulk [Workspace]Optimize workspace permission validation for bulk operations Jul 26, 2024
opensearch-changeset-bot bot added a commit to wanglam/OpenSearch-Dashboards that referenced this pull request Jul 26, 2024
Signed-off-by: Lin Wang <wonglam@amazon.com>
@wanglam wanglam force-pushed the optimize-workspace-permission-validation branch from 98d108d to cc91fd4 Compare July 26, 2024 14:16
this._savedObjectCache.delete(requestKey);
}

public isSavedObjectsCacheActive(request: OpenSearchDashboardsRequest) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to call isSavedObjectsCacheActive before clearSavedObjectsCache? Seems it can call clearSavedObjectsCache directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, since this delete operation will be fired in every request response. Not sure if it too expensive to do the delete.

Copy link
Member

Choose a reason for hiding this comment

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

We can encapsulate the check in clearSavedObjectsCache

@ruanyl ruanyl added the v2.17.0 label Jul 29, 2024
Signed-off-by: Lin Wang <wonglam@amazon.com>
wanglam and others added 2 commits July 30, 2024 14:17
Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
@wanglam wanglam requested a review from joshuali925 as a code owner July 30, 2024 06:33
@SuZhou-Joe SuZhou-Joe merged commit ef47b5f into opensearch-project:main Aug 7, 2024
68 of 69 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 8, 2024
…ns (#7516)

* Optimize workspace permission validation for bulk operations

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Changeset file for PR #7516 created/updated

* Remove isSavedObjectsCacheActive in permission control client

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Update src/plugins/workspace/server/permission_control/client.ts

Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>

* Rename cacheSavedObjects to addToCacheAllowlist

Signed-off-by: Lin Wang <wonglam@amazon.com>

* Use request uuid as cache

Signed-off-by: Lin Wang <wonglam@amazon.com>

---------

Signed-off-by: Lin Wang <wonglam@amazon.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
(cherry picked from commit ef47b5f)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
SuZhou-Joe added a commit that referenced this pull request Aug 9, 2024
…ns (#7516) (#7660)

* Optimize workspace permission validation for bulk operations



* Changeset file for PR #7516 created/updated

* Remove isSavedObjectsCacheActive in permission control client



* Update src/plugins/workspace/server/permission_control/client.ts




* Rename cacheSavedObjects to addToCacheAllowlist



* Use request uuid as cache



---------




(cherry picked from commit ef47b5f)

Signed-off-by: Lin Wang <wonglam@amazon.com>
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>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants