-
Notifications
You must be signed in to change notification settings - Fork 981
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] Fix find saved object within a workspace without queryparams return all the saved objects #9420
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. |
❌ 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. |
❌ Entry Too LongEntry is 111 characters long, which is 11 characters longer than the maximum allowed length of 100 characters. Please revise your entry to be within the maximum length. |
) | ||
); | ||
}; | ||
|
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.
Just refactoring repeated code
it('should handle workspace filtering', async () => { | ||
registerFindRoute(router, Promise.resolve(managementService)); | ||
|
||
req.query.workspaces = ['workspace1']; |
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.
nit: should we have a case for workspace is empty?
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.
I think the no workspace case is already been covered by the above "basic find" test case. For a find request without workspace filtering then the saved object is always opt-in so that it somehow pointless to explicitly test this case.
workspaces: req.query.workspaces ? Array<string>().concat(req.query.workspaces) : undefined, | ||
...(req.query.workspaces | ||
? { | ||
workspaces: Array<string>().concat(req.query.workspaces), | ||
} | ||
: {}), |
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.
We have another PR to fix the * issue: #9409. I think it should be fine for this PR to leave *.
Signed-off-by: Owen Wang <owenwyk@amazon.com>
Signed-off-by: Owen Wang <owenwyk@amazon.com>
Description
Given we have two workspaces (namely
first
andsecond
), and each has a saved object.We retrieve the saved objects using the following URL (using workspace prefix but no query parameters):
/w/first/api/opensearch-dashboards/management/saved_objects/_find?type=index-pattern
Previously, this request would return both saved objects because when no
workspace
query parameter was provided, our endpoint would passworkspace: undefined
to the actual query, bypassing the workspace check. As a result, the backend would return all saved objects even when the original request was made under thefirst
workspace context.This PR removes the logic of passing
workspace: undefined
when there is noworkspace
query parameter. Now, the wrapper for the_find
endpoint automatically retrieves the current workspace context, ensuring the backend only returns saved objects associated with the current workspace.Issues Resolved
NA
Screenshot
NA
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration