-
Notifications
You must be signed in to change notification settings - Fork 12
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
ID-897 filter resources endpoint #1236
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I'm going to change this from v3 to v2 endpoint version |
tlangs
commented
Oct 31, 2023
src/main/scala/org/broadinstitute/dsde/workbench/sam/api/ResourceRoutes.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala
Outdated
Show resolved
Hide resolved
sjkobori
approved these changes
Oct 31, 2023
Ghost-in-a-Jar
approved these changes
Oct 31, 2023
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
mlilynolting
reviewed
Nov 1, 2023
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.
this looks great.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket: https://broadworkbench.atlassian.net/browse/ID-897
GET /api/resources/v2/{resource_type_name}
is just too darn slow. The DB query to generate the response is one of the most expensive queries in Sam, and it recently caused us to DDoS ourselves when a change was made in Terra-UI that ultimately hit that endpoint millions of times.The current endpoint returns all the resources of a resource type that the calling user has access to, including direct and inherited roles and actions. It also returns all public resources of that type. The query backing this runs extraordinarily slowly, often .5-1 seconds, and takes a lot of DB CPU resources.
I’m adding a new endpoint:
GET /api/resources/v3/filter?resourceTypes=resourceType1,resourceType2&policies=policyName1,policyName2&roles=roleName1,roleName2&actions=actionName1,actionName2&includePublic=false
This “filter” query runs an order-of-magnitude faster through some specific optimizations. The filter parameters are all lists (except includePublic). Results will be the intersection of all filters, excluding empty ones. For example:
GET /api/resources/v3/filter?resourceTypes=workspace
would get all the workspaces available to the user, and include all the policies, roles, and actions they have on those workspaces.GET /api/resources/v3/filter?resourceTypes=workspace&roles=reader,writer
would get all workspaces for the user where they are in either the reader OR the writer role.The response will look like this:
In the initial implementation, getting public resources for a user through this query will remain unimplemented, and throw an exception if includePublic=true is set in the request. Some special attention needs to be paid to how we get public resources for a user, since that actually makes up the bulk of the query latency in /api/resource/v2/{resource_type_name}.Ended up implementing it, and its 4x faster than the list resources endpointThis endpoint is extremely useful for services like Leo, which only want to get the resources for a user for which they have a specific type of access. Currently, Leo is forced to get all resources of a type for a user and filter client-side, but this query will let Leo ask Sam only for what it needs. Efficiencies!
In initial testing, the DB query supporting the filtering runs 10x faster than the standard list resources query!
PR checklist