CORE-137 Optimize filter resources

[dvoet]

Ticket: https://broadworkbench.atlassian.net/browse/CORE-137

What:

The query implemented in PostgresAccessPolicyDAO.filterPrivateResources has been highlighted by query insights as far and away the biggest load by time on the database. It is called on average 1.1 million times per day! It is the query behind the api to list user resources. Around the beginning of October, we also noticed in dev that this query was causing stability issues because the AoU user had access to too many google-project resources. Those were cleaned up but the vulnerability remains.

How:

There were 2 main observations:

1. including actions associated to roles for each resource in the query results multiplied the number of rows returned, for the AoU case there were millions. But the role to action mapping is static from Sam config and is already loaded in memory. Moving all this extra data around is wasteful and is probably leading to excess garbage collection.
2. removing the joins to figure out auth domain information sped up the AoU queries. Prior versions of this query loaded auth domains separately especially because not all resource types are affected by them. Revert to that.

Overall, the workhorse query is much smaller in terms of joins.

---

PR checklist

• I've followed the instructions if I've made any changes to the API, especially if they're breaking changes
• I've filled out the Security Risk Assessment (requires Broad Internal network access) and attached the result to the JIRA ticket

[dvoet]

I switched to a concurrent hashmap implementation because synchronized maps do not allow concurrent reads. Even if the reads are fast, synchronized can lead to jam ups and interference between different resource types.

[davidangb]

including actions associated to roles for each resource in the query results multiplied the number of rows …. But the role to action mapping is static from Sam config and is already loaded in memory.

If the role:action association exists both as static config and in the db, does this change lead to a possible inconsistency?

1. A user is granted a role on some resource; the role's actions are persisted to the db for this user
2. A DSP engineer modifies the role:action mapping in the static config
3. This list-resources API now returns the updated actions for the role, but some other APIs still rely on the db for actions and return a different result

[davidangb]

This looks ok to me. I have to admit I am too unfamiliar with it to be very confident. I get the high-level design change and that sounds good.

Does the contract test failure indicate a real change in functionality?

[davidangb]

is this intentional or a leftover from development?

[dvoet]

If the role:action association exists both as static config and in the db, does this change lead to a possible inconsistency?

1. A user is granted a role on some resource; the role's actions are persisted to the db for this user
2. A DSP engineer modifies the role:action mapping in the static config
3. This list-resources API now returns the updated actions for the role, but some other APIs still rely on the db for actions and return a different result

No, step 1 in your list is not how it works. The role is assigned to the user, not the role's actions. The role:action association stored in the database is only update on startup to match what is in config.

[davidangb]

No, step 1 in your list is not how it works. The role is assigned to the user, not the role's actions. The role:action association stored in the database is only update on startup to match what is in config.

Got it - thank you!

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
9.1% Duplication on New Code

See analysis details on SonarCloud