-
Notifications
You must be signed in to change notification settings - Fork 867
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]feat: bypass permission control when construct ui settings client #8053
Conversation
❌ Empty Changelog SectionThe Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8053 +/- ##
==========================================
- Coverage 60.59% 60.58% -0.01%
==========================================
Files 3738 3738
Lines 88678 88680 +2
Branches 13787 13788 +1
==========================================
- Hits 53732 53731 -1
- Misses 31661 31663 +2
- Partials 3285 3286 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
a620f9c
to
ede386e
Compare
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
ede386e
to
f6208eb
Compare
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
const uiSettingsClient = coreStart.uiSettings.asScopedToClient( | ||
coreStart.savedObjects.getScopedClient(request) | ||
coreStart.savedObjects.getScopedClient(request, { | ||
excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], | ||
}) | ||
); | ||
const defaultWorkspaceId = await uiSettingsClient.get(DEFAULT_WORKSPACE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about call await uiSettingsClient.get(DEFAULT_WORKSPACE);
no matter what's the path after the user authenticated, after this call setting object should been initialized which include upgrade logic. With this, we don't need to put exclude workspace wrapper logic in other place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get your point, yeah let me see if I can find a centralized place that will trigger the get
method of ui settings client.
Description
In 2.16 we refactored the way how we construct the find query based on workspace and permission control, which makes normal user unable to
find
global objects. And this behavior breaks the upgrade flow of advanced settings as it is using find to list all of the global configsThis PR is to fix this issue by constructing a saved objects client that will bypass workspace permission control wrapper so that the upgrade flow can work as it was.
Issues Resolved
Screenshot
Testing the changes
savedObjects.permission.enabled: true
and config a user as the dashboard admin.config:2.9.0
with some customized attributes and delete the existing config whose doc id isconfig:3.0.0
config:3.0.0
should be created, with the customized attributes upgraded fromconfig:2.9.0
Changelog
Check List
yarn test:jest
yarn test:jest_integration