From 6dafed731f412f2fd334acf0a870032a8ed1ffff Mon Sep 17 00:00:00 2001 From: Darren Shepherd Date: Sun, 1 Mar 2020 22:23:36 -0700 Subject: [PATCH] RBAC performance improvements --- pkg/accesscontrol/access_set.go | 22 +++++++ pkg/accesscontrol/access_store.go | 75 ++++++------------------ pkg/accesscontrol/policy_rule_index.go | 44 ++++++++++---- pkg/accesscontrol/role_revision_index.go | 59 +++++++++++++++++++ 4 files changed, 131 insertions(+), 69 deletions(-) create mode 100644 pkg/accesscontrol/role_revision_index.go diff --git a/pkg/accesscontrol/access_set.go b/pkg/accesscontrol/access_set.go index 9be9d85..b221ea1 100644 --- a/pkg/accesscontrol/access_set.go +++ b/pkg/accesscontrol/access_set.go @@ -58,6 +58,28 @@ func (a *AccessSet) Merge(right *AccessSet) { } } +func (a AccessSet) Grants(verb string, gr schema.GroupResource, namespace, name string) bool { + for _, v := range []string{All, verb} { + for _, g := range []string{All, gr.Group} { + for _, r := range []string{All, gr.Resource} { + for k := range a.set[key{ + verb: v, + gr: schema.GroupResource{ + Group: g, + Resource: r, + }, + }] { + if k.Grants(namespace, name) { + return true + } + } + } + } + } + + return false +} + func (a AccessSet) AccessListFor(verb string, gr schema.GroupResource) (result AccessList) { dedup := map[Access]bool{} for _, v := range []string{All, verb} { diff --git a/pkg/accesscontrol/access_store.go b/pkg/accesscontrol/access_store.go index f2f4ddd..cefe3a8 100644 --- a/pkg/accesscontrol/access_store.go +++ b/pkg/accesscontrol/access_store.go @@ -5,12 +5,9 @@ import ( "crypto/sha256" "encoding/hex" "sort" - "sync" "time" v1 "github.com/rancher/wrangler-api/pkg/generated/controllers/rbac/v1" - "github.com/rancher/wrangler/pkg/kv" - k8srbac "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/util/cache" "k8s.io/apiserver/pkg/authentication/user" ) @@ -20,10 +17,9 @@ type AccessSetLookup interface { } type AccessStore struct { - users *policyRuleIndex - groups *policyRuleIndex - roleRevisions sync.Map - cache *cache.LRUExpireCache + users *policyRuleIndex + groups *policyRuleIndex + cache *cache.LRUExpireCache } type roleKey struct { @@ -32,47 +28,17 @@ type roleKey struct { } func NewAccessStore(ctx context.Context, cacheResults bool, rbac v1.Interface) *AccessStore { + revisions := newRoleRevision(ctx, rbac) as := &AccessStore{ - users: newPolicyRuleIndex(true, rbac), - groups: newPolicyRuleIndex(false, rbac), + users: newPolicyRuleIndex(true, revisions, rbac), + groups: newPolicyRuleIndex(false, revisions, rbac), } - rbac.Role().OnChange(ctx, "role-revision-indexer", as.onRoleChanged) - rbac.ClusterRole().OnChange(ctx, "role-revision-indexer", as.onClusterRoleChanged) if cacheResults { - as.cache = cache.NewLRUExpireCache(1000) + as.cache = cache.NewLRUExpireCache(50) } return as } -func (l *AccessStore) onClusterRoleChanged(key string, cr *k8srbac.ClusterRole) (role *k8srbac.ClusterRole, err error) { - if cr == nil { - l.roleRevisions.Delete(roleKey{ - name: key, - }) - } else { - l.roleRevisions.Store(roleKey{ - name: key, - }, cr.ResourceVersion) - } - return cr, nil -} - -func (l *AccessStore) onRoleChanged(key string, cr *k8srbac.Role) (role *k8srbac.Role, err error) { - if cr == nil { - namespace, name := kv.Split(key, "/") - l.roleRevisions.Delete(roleKey{ - name: name, - namespace: namespace, - }) - } else { - l.roleRevisions.Store(roleKey{ - name: cr.Name, - namespace: cr.Namespace, - }, cr.ResourceVersion) - } - return cr, nil -} - func (l *AccessStore) AccessFor(user user.Info) *AccessSet { var cacheKey string if l.cache != nil { @@ -98,23 +64,18 @@ func (l *AccessStore) AccessFor(user user.Info) *AccessSet { } func (l *AccessStore) CacheKey(user user.Info) string { - roles := map[roleKey]struct{}{} - l.users.addRolesToMap(roles, user.GetName()) - for _, group := range user.GetGroups() { - l.groups.addRolesToMap(roles, group) - } - - revs := make([]string, 0, len(roles)) - for roleKey := range roles { - val, _ := l.roleRevisions.Load(roleKey) - rev, _ := val.(string) - revs = append(revs, roleKey.namespace+"/"+roleKey.name+":"+rev) - } - - sort.Strings(revs) d := sha256.New() - for _, rev := range revs { - d.Write([]byte(rev)) + + l.users.addRolesToHash(d, user.GetName()) + + groupBase := user.GetGroups() + groups := make([]string, 0, len(groupBase)) + copy(groups, groupBase) + + sort.Strings(groups) + for _, group := range user.GetGroups() { + l.groups.addRolesToHash(d, group) } + return hex.EncodeToString(d.Sum(nil)) } diff --git a/pkg/accesscontrol/policy_rule_index.go b/pkg/accesscontrol/policy_rule_index.go index 9f4de66..c94bb86 100644 --- a/pkg/accesscontrol/policy_rule_index.go +++ b/pkg/accesscontrol/policy_rule_index.go @@ -1,6 +1,10 @@ package accesscontrol import ( + "hash" + "sort" + "strings" + v1 "github.com/rancher/wrangler-api/pkg/generated/controllers/rbac/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -16,12 +20,13 @@ type policyRuleIndex struct { rCache v1.RoleCache crbCache v1.ClusterRoleBindingCache rbCache v1.RoleBindingCache + revisions *roleRevisionIndex kind string roleIndexKey string clusterRoleIndexKey string } -func newPolicyRuleIndex(user bool, rbac v1.Interface) *policyRuleIndex { +func newPolicyRuleIndex(user bool, revisions *roleRevisionIndex, rbac v1.Interface) *policyRuleIndex { key := "Group" if user { key = "User" @@ -34,6 +39,7 @@ func newPolicyRuleIndex(user bool, rbac v1.Interface) *policyRuleIndex { rbCache: rbac.RoleBinding().Cache(), clusterRoleIndexKey: "crb" + key, roleIndexKey: "rb" + key, + revisions: revisions, } pi.crbCache.AddIndexer(pi.clusterRoleIndexKey, pi.clusterRoleBindingBySubjectIndexer) @@ -60,24 +66,27 @@ func (p *policyRuleIndex) roleBindingBySubject(rb *rbacv1.RoleBinding) (result [ return } -func (p *policyRuleIndex) addRolesToMap(roles map[roleKey]struct{}, subjectName string) { +var null = []byte{'\x00'} + +func (p *policyRuleIndex) addRolesToHash(digest hash.Hash, subjectName string) { for _, crb := range p.getClusterRoleBindings(subjectName) { - roles[roleKey{ - name: crb.RoleRef.Name, - }] = struct{}{} + digest.Write([]byte(crb.RoleRef.Name)) + digest.Write([]byte(p.revisions.roleRevision("", crb.RoleRef.Name))) + digest.Write(null) } for _, rb := range p.getRoleBindings(subjectName) { switch rb.RoleRef.Kind { case "Role": - roles[roleKey{ - name: rb.RoleRef.Name, - namespace: rb.Namespace, - }] = struct{}{} + digest.Write([]byte(rb.RoleRef.Name)) + digest.Write([]byte(rb.Namespace)) + digest.Write([]byte(p.revisions.roleRevision(rb.Namespace, rb.RoleRef.Name))) + digest.Write(null) case "ClusterRole": - roles[roleKey{ - name: rb.RoleRef.Name, - }] = struct{}{} + digest.Write([]byte(rb.RoleRef.Name)) + digest.Write([]byte(rb.Namespace)) + digest.Write([]byte(p.revisions.roleRevision(rb.Namespace, rb.RoleRef.Name))) + digest.Write(null) } } } @@ -145,6 +154,9 @@ func (p *policyRuleIndex) getClusterRoleBindings(subjectName string) []*rbacv1.C if err != nil { return nil } + sort.Slice(result, func(i, j int) bool { + return result[i].Name < result[j].Name + }) return result } @@ -153,5 +165,13 @@ func (p *policyRuleIndex) getRoleBindings(subjectName string) []*rbacv1.RoleBind if err != nil { return nil } + sort.Slice(result, func(i, j int) bool { + if i := strings.Compare(result[i].Namespace, result[j].Namespace); i < 0 { + return true + } else if i > 0 { + return false + } + return result[i].Name < result[j].Name + }) return result } diff --git a/pkg/accesscontrol/role_revision_index.go b/pkg/accesscontrol/role_revision_index.go new file mode 100644 index 0000000..d6c193a --- /dev/null +++ b/pkg/accesscontrol/role_revision_index.go @@ -0,0 +1,59 @@ +package accesscontrol + +import ( + "context" + "sync" + + rbac "github.com/rancher/wrangler-api/pkg/generated/controllers/rbac/v1" + "github.com/rancher/wrangler/pkg/kv" + rbacv1 "k8s.io/api/rbac/v1" +) + +type roleRevisionIndex struct { + roleRevisions sync.Map +} + +func newRoleRevision(ctx context.Context, rbac rbac.Interface) *roleRevisionIndex { + r := &roleRevisionIndex{} + rbac.Role().OnChange(ctx, "role-revision-indexer", r.onRoleChanged) + rbac.ClusterRole().OnChange(ctx, "role-revision-indexer", r.onClusterRoleChanged) + return r +} + +func (r *roleRevisionIndex) roleRevision(namespace, name string) string { + val, _ := r.roleRevisions.Load(roleKey{ + name: name, + namespace: namespace, + }) + revision, _ := val.(string) + return revision +} + +func (r *roleRevisionIndex) onClusterRoleChanged(key string, cr *rbacv1.ClusterRole) (role *rbacv1.ClusterRole, err error) { + if cr == nil { + r.roleRevisions.Delete(roleKey{ + name: key, + }) + } else { + r.roleRevisions.Store(roleKey{ + name: key, + }, cr.ResourceVersion) + } + return cr, nil +} + +func (r *roleRevisionIndex) onRoleChanged(key string, cr *rbacv1.Role) (role *rbacv1.Role, err error) { + if cr == nil { + namespace, name := kv.Split(key, "/") + r.roleRevisions.Delete(roleKey{ + name: name, + namespace: namespace, + }) + } else { + r.roleRevisions.Store(roleKey{ + name: cr.Name, + namespace: cr.Namespace, + }, cr.ResourceVersion) + } + return cr, nil +}