fix(ACL): Prevents permissions overrride and merges acl cache to persist permissions across different namespaces (#8418) #8506
Conversation
…ist permissions across different namespaces (#8418) In the current implementation, when you update ACL rules across different namespaces, the previous rules are overwritten. This results in incorrect results for other namespaces (the ones which were created earlier). To reproduce: 1. Create a `namespace-1` with acl rules configured 2. Create a `namespace-2` with a different set of acl rules configured 3. Login to `namespace-1` and do a query. It will not respect the rules configured in step-1. Rather than overwriting the rules, we do a merge of map that holds the rules. This helps in persisting the rules configured for different namespaces. i.e ``` AclCachePtr.predPerms = predPerms ``` is modified to ``` map.copy(AclCachePtr.predPerms, predPerms) ``` `map` is only available in `go>=1.18`, therefore this PR also bumps the go version in `go.mod`. 1. `TestTwoPermissionSetsInNameSpacesWithAcl` (cherry picked from commit de758ac)
github-actions
bot
added
area/enterprise
| Op: "AND", | ||
| Child: []gql.FilterTree{ | ||
| {filter, newFilter} | ||
| } |
There was a problem hiding this comment.
[nitpick ~ no need to fix] I notice the tabbing & related aesthetic changes coming in from your PRs - possibly from your IDE formatter. I personally think we should avoid introducing these changes, when the PR is targeting an older branch.
There was a problem hiding this comment.
So the original change went into main and it had the formatting changes like you mentioned. Since this PR is only a cherry-pick, I brought in all the changes from the other merge. Fixing (removing) these would require changes on top of cherry-pick PR.
There was a problem hiding this comment.
[nitpick] we should try to keep the diff's tightened ONLY to the issue at hand. It would be nice to raise a separate PR for these aesthetic changes. I am guessing this happened because of your IDE, and you should probably minimize changes for easier review in these side-releases. (no action is needed)
This is merged in main. This is only a cherry-pick to slash branch for one customer who is facing issues. If you have any urgent comments, can you file it on the original PR or as a separate issue. I'll fix them in main. |
In the current implementation, when you update ACL rules across
different namespaces, the previous rules are overwritten. This results
in incorrect results for other namespaces (the ones which were created
earlier).
To reproduce:
Create a
namespace-1with acl rules configuredCreate a
namespace-2with a different set of acl rules configuredLogin to
namespace-1and do a query. It will not respect the rulesconfigured in step-1.
TestTwoPermissionSetsInNameSpacesWithAcl(cherry picked from commit de758ac)