fix: groups lookup performance issue with users with lots of groups (#20740)

fixes https://github.com/minio/minio/issues/20717
This commit is contained in:
Harshavardhana 2024-12-11 16:23:28 +05:30 committed by GitHub
parent d56ef8dbe1
commit 7b3eb9f7f8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 94 additions and 13 deletions

View File

@ -26,6 +26,7 @@ import (
"path"
"sort"
"strings"
"sync"
"time"
jsoniter "github.com/json-iterator/go"
@ -37,6 +38,7 @@ import (
"github.com/minio/minio/internal/jwt"
"github.com/minio/pkg/v3/env"
"github.com/minio/pkg/v3/policy"
"github.com/minio/pkg/v3/sync/errgroup"
"github.com/puzpuzpuz/xsync/v3"
"golang.org/x/sync/singleflight"
)
@ -357,6 +359,68 @@ func (c *iamCache) removeGroupFromMembershipsMap(group string) {
}
}
func (c *iamCache) policyDBGetGroups(store *IAMStoreSys, userPolicyPresent bool, groups ...string) ([]string, error) {
var policies []string
for _, group := range groups {
if store.getUsersSysType() == MinIOUsersSysType {
g, ok := c.iamGroupsMap[group]
if !ok {
continue
}
// Group is disabled, so we return no policy - this
// ensures the request is denied.
if g.Status == statusDisabled {
continue
}
}
policy, ok := c.iamGroupPolicyMap.Load(group)
if !ok {
continue
}
policies = append(policies, policy.toSlice()...)
}
found := len(policies) > 0
if found {
return policies, nil
}
if userPolicyPresent {
// if user mapping present and no group policies found
// rely on user policy for access, instead of fallback.
return nil, nil
}
var mu sync.Mutex
// no mappings found, fallback for all groups.
g := errgroup.WithNErrs(len(groups)).WithConcurrency(10) // load like 10 groups at a time.
for index := range groups {
index := index
g.Go(func() error {
err := store.loadMappedPolicy(context.TODO(), groups[index], regUser, true, c.iamGroupPolicyMap)
if err != nil && !errors.Is(err, errNoSuchPolicy) {
return err
}
if errors.Is(err, errNoSuchPolicy) {
return nil
}
policy, _ := c.iamGroupPolicyMap.Load(groups[index])
mu.Lock()
policies = append(policies, policy.toSlice()...)
mu.Unlock()
return nil
}, index)
}
err := errors.Join(g.Wait()...)
return policies, err
}
// policyDBGet - lower-level helper; does not take locks.
//
// If a group is passed, it returns policies associated with the group.
@ -676,7 +740,8 @@ func (store *IAMStoreSys) LoadIAMCache(ctx context.Context, firstTime bool) erro
type IAMStoreSys struct {
IAMStorageAPI
group *singleflight.Group
group *singleflight.Group
policy *singleflight.Group
}
// HasWatcher - returns if the storage system has a watcher.
@ -757,21 +822,32 @@ func (store *IAMStoreSys) PolicyDBGet(name string, groups ...string) ([]string,
cache := store.rlock()
defer store.runlock()
policies, _, err := cache.policyDBGet(store, name, false, false)
if err != nil {
return nil, err
}
userPolicyPresent := len(policies) > 0
for _, group := range groups {
ps, _, err := cache.policyDBGet(store, group, true, userPolicyPresent)
getPolicies := func() ([]string, error) {
policies, _, err := cache.policyDBGet(store, name, false, false)
if err != nil {
return nil, err
}
policies = append(policies, ps...)
}
return policies, nil
userPolicyPresent := len(policies) > 0
groupPolicies, err := cache.policyDBGetGroups(store, userPolicyPresent, groups...)
if err != nil {
return nil, err
}
policies = append(policies, groupPolicies...)
return policies, nil
}
if store.policy != nil {
val, err, _ := store.policy.Do(name, func() (interface{}, error) {
return getPolicies()
})
if err != nil {
return nil, err
}
return val.([]string), nil
}
return getPolicies()
}
// AddUsersToGroup - adds users to group, creating the group if needed.

View File

@ -178,13 +178,18 @@ func (sys *IAMSys) initStore(objAPI ObjectLayer, etcdClient *etcd.Client) {
}
if etcdClient == nil {
var group *singleflight.Group
var (
group *singleflight.Group
policy *singleflight.Group
)
if env.Get("_MINIO_IAM_SINGLE_FLIGHT", config.EnableOn) == config.EnableOn {
group = &singleflight.Group{}
policy = &singleflight.Group{}
}
sys.store = &IAMStoreSys{
IAMStorageAPI: newIAMObjectStore(objAPI, sys.usersSysType),
group: group,
policy: policy,
}
} else {
sys.store = &IAMStoreSys{IAMStorageAPI: newIAMEtcdStore(etcdClient, sys.usersSysType)}