fix(ACL): The Acl cache should be updated on restart and restore #7926
Conversation
| // ResetAcls is an empty method since ACL is only supported in the enterprise version. | ||
| func RefreshAcls(closer *z.Closer) { | ||
| // SubscribeForAclUpdates is an empty method since ACL is only supported in the enterprise version. | ||
| func SubscribeForAclUpdates(closer *z.Closer) { | ||
| // do nothing |
There was a problem hiding this comment.
return. Why need to wait
minhaj-shakeel
changed the title
There was a problem hiding this comment.
Reviewed 1 of 3 files at r1, 6 of 7 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ahsanbarkati)
worker/acl_cache.go, line 1 at r3 (raw file):
package worker
Keep the file under the same license. So, this file should stay as enterprise.
Also, if we were to compile with oss flag, would this then build?
There was a problem hiding this comment.
Got a few comments. Rest looks good to me. Please address them before merging.
Reviewed 4 of 11 files at r4.
Reviewable status: 7 of 14 files reviewed, 8 unresolved discussions (waiting on @ahsanbarkati and @manishrjain)
edgraph/access_ee.go, line 372 at r4 (raw file):
} maxRefreshTs = refreshTs ctx := x.AttachNamespace(closer.Ctx(), ns)
Seems like you probably don't need to attach namespace over here as we are passing the namespace in refreshAclCache.
edgraph/access_ee.go, line 376 at r4 (raw file):
if !worker.AclCachePtr.Loaded() { updaters := z.NewCloser(1) RefreshACLs(updaters.Ctx())
Why do we need to call this? This will load the ACLs for all the namespaces 🤔
worker/acl_cache.go, line 43 at r4 (raw file):
} func (cache *AclCache) Set() {
Don't we need lock over here and in the above functions?
worker/acl_cache.go, line 54 at r4 (raw file):
func (cache *AclCache) GetUserPredPerms(userId string) map[string]int32 { return cache.userPredPerms[userId]
I think it would be good to move the lock/unlock from the caller to GetUserPredPerms.
(cherry picked from commit 9759fb0)
This change is