From 124816f6a617fefa2c7816a0ff0c4fceafc08980 Mon Sep 17 00:00:00 2001 From: sgandon Date: Fri, 5 Mar 2021 17:36:16 +0100 Subject: [PATCH] fix : IAM Intialization failing with a large number of users/policies (#11701) --- cmd/iam-etcd-store.go | 137 +++++++++++++++++++++++-------------- cmd/iam-etcd-store_test.go | 39 +++++++++++ cmd/iam.go | 1 + 3 files changed, 124 insertions(+), 53 deletions(-) create mode 100644 cmd/iam-etcd-store_test.go diff --git a/cmd/iam-etcd-store.go b/cmd/iam-etcd-store.go index 6c6c98c50..0d08e0380 100644 --- a/cmd/iam-etcd-store.go +++ b/cmd/iam-etcd-store.go @@ -42,34 +42,20 @@ var defaultContextTimeout = 30 * time.Second func etcdKvsToSet(prefix string, kvs []*mvccpb.KeyValue) set.StringSet { users := set.NewStringSet() for _, kv := range kvs { - // Extract user by stripping off the `prefix` value as suffix, - // then strip off the remaining basename to obtain the prefix - // value, usually in the following form. - // - // key := "config/iam/users/newuser/identity.json" - // prefix := "config/iam/users/" - // v := trim(trim(key, prefix), base(key)) == "newuser" - // - user := path.Clean(strings.TrimSuffix(strings.TrimPrefix(string(kv.Key), prefix), path.Base(string(kv.Key)))) + user := extractPathPrefixAndSuffix(string(kv.Key), prefix, path.Base(string(kv.Key))) users.Add(user) } return users } -func etcdKvsToSetPolicyDB(prefix string, kvs []*mvccpb.KeyValue) set.StringSet { - items := set.NewStringSet() - for _, kv := range kvs { - // Extract user item by stripping off prefix and then - // stripping of ".json" suffix. - // - // key := "config/iam/policydb/users/myuser1.json" - // prefix := "config/iam/policydb/users/" - // v := trimSuffix(trimPrefix(key, prefix), ".json") - key := string(kv.Key) - item := path.Clean(strings.TrimSuffix(strings.TrimPrefix(key, prefix), ".json")) - items.Add(item) - } - return items +// Extract path string by stripping off the `prefix` value and the suffix, +// value, usually in the following form. +// s := "config/iam/users/foo/config.json" +// prefix := "config/iam/users/" +// suffix := "config.json" +// result is foo +func extractPathPrefixAndSuffix(s string, prefix string, suffix string) string { + return path.Clean(strings.TrimSuffix(strings.TrimPrefix(string(s), prefix), suffix)) } // IAMEtcdStore implements IAMStorageAPI @@ -113,20 +99,24 @@ func (ies *IAMEtcdStore) saveIAMConfig(ctx context.Context, item interface{}, pa return saveKeyEtcd(ctx, ies.client, path, data, opts...) } +func getIAMConfig(item interface{}, value []byte) error { + conf := value + var err error + if globalConfigEncrypted && !utf8.Valid(value) { + conf, err = madmin.DecryptData(globalActiveCred.String(), bytes.NewReader(conf)) + if err != nil { + return err + } + } + return json.Unmarshal(conf, item) +} + func (ies *IAMEtcdStore) loadIAMConfig(ctx context.Context, item interface{}, path string) error { pdata, err := readKeyEtcd(ctx, ies.client, path) if err != nil { return err } - - if globalConfigEncrypted && !utf8.Valid(pdata) { - pdata, err = madmin.DecryptData(globalActiveCred.String(), bytes.NewReader(pdata)) - if err != nil { - return err - } - } - - return json.Unmarshal(pdata, item) + return getIAMConfig(item, pdata) } func (ies *IAMEtcdStore) deleteIAMConfig(ctx context.Context, path string) error { @@ -273,35 +263,53 @@ func (ies *IAMEtcdStore) loadPolicyDoc(ctx context.Context, policy string, m map return nil } +func (ies *IAMEtcdStore) getPolicyDoc(ctx context.Context, kvs *mvccpb.KeyValue, m map[string]iampolicy.Policy) error { + var p iampolicy.Policy + err := getIAMConfig(&p, kvs.Value) + if err != nil { + if err == errConfigNotFound { + return errNoSuchPolicy + } + return err + } + policy := extractPathPrefixAndSuffix(string(kvs.Key), iamConfigPoliciesPrefix, path.Base(string(kvs.Key))) + m[policy] = p + return nil +} + func (ies *IAMEtcdStore) loadPolicyDocs(ctx context.Context, m map[string]iampolicy.Policy) error { ctx, cancel := context.WithTimeout(ctx, defaultContextTimeout) defer cancel() - r, err := ies.client.Get(ctx, iamConfigPoliciesPrefix, etcd.WithPrefix(), etcd.WithKeysOnly()) + // Retrieve all keys and values to avoid too many calls to etcd in case of + // a large number of policies + r, err := ies.client.Get(ctx, iamConfigPoliciesPrefix, etcd.WithPrefix()) if err != nil { return err } - policies := etcdKvsToSet(iamConfigPoliciesPrefix, r.Kvs) - - // Reload config and policies for all policys. - for _, policyName := range policies.ToSlice() { - if err = ies.loadPolicyDoc(ctx, policyName, m); err != nil && err != errNoSuchPolicy { + // Parse all values to construct the policies data model. + for _, kvs := range r.Kvs { + if err = ies.getPolicyDoc(ctx, kvs, m); err != nil && err != errNoSuchPolicy { return err } } return nil } -func (ies *IAMEtcdStore) loadUser(ctx context.Context, user string, userType IAMUserType, m map[string]auth.Credentials) error { +func (ies *IAMEtcdStore) getUser(ctx context.Context, userkv *mvccpb.KeyValue, userType IAMUserType, m map[string]auth.Credentials, basePrefix string) error { var u UserIdentity - err := ies.loadIAMConfig(ctx, &u, getUserIdentityPath(user, userType)) + err := getIAMConfig(&u, userkv.Value) if err != nil { if err == errConfigNotFound { return errNoSuchUser } return err } + user := extractPathPrefixAndSuffix(string(userkv.Key), basePrefix, path.Base(string(userkv.Key))) + return ies.addUser(ctx, user, userType, u, m) +} +func (ies *IAMEtcdStore) addUser(ctx context.Context, user string, userType IAMUserType, u UserIdentity, m map[string]auth.Credentials) error { if u.Credentials.IsExpired() { // Delete expired identity. deleteKeyEtcd(ctx, ies.client, getUserIdentityPath(user, userType)) @@ -334,7 +342,18 @@ func (ies *IAMEtcdStore) loadUser(ctx context.Context, user string, userType IAM } m[user] = u.Credentials return nil +} +func (ies *IAMEtcdStore) loadUser(ctx context.Context, user string, userType IAMUserType, m map[string]auth.Credentials) error { + var u UserIdentity + err := ies.loadIAMConfig(ctx, &u, getUserIdentityPath(user, userType)) + if err != nil { + if err == errConfigNotFound { + return errNoSuchUser + } + return err + } + return ies.addUser(ctx, user, userType, u, m) } func (ies *IAMEtcdStore) loadUsers(ctx context.Context, userType IAMUserType, m map[string]auth.Credentials) error { @@ -351,16 +370,16 @@ func (ies *IAMEtcdStore) loadUsers(ctx context.Context, userType IAMUserType, m cctx, cancel := context.WithTimeout(ctx, defaultContextTimeout) defer cancel() - r, err := ies.client.Get(cctx, basePrefix, etcd.WithPrefix(), etcd.WithKeysOnly()) + // Retrieve all keys and values to avoid too many calls to etcd in case of + // a large number of users + r, err := ies.client.Get(cctx, basePrefix, etcd.WithPrefix()) if err != nil { return err } - users := etcdKvsToSet(basePrefix, r.Kvs) - - // Reload config for all users. - for _, user := range users.ToSlice() { - if err = ies.loadUser(ctx, user, userType, m); err != nil && err != errNoSuchUser { + // Parse all users values to create the proper data model + for _, userKv := range r.Kvs { + if err = ies.getUser(ctx, userKv, userType, m, basePrefix); err != nil && err != errNoSuchUser { return err } } @@ -413,7 +432,20 @@ func (ies *IAMEtcdStore) loadMappedPolicy(ctx context.Context, name string, user } m[name] = p return nil +} +func getMappedPolicy(ctx context.Context, kv *mvccpb.KeyValue, userType IAMUserType, isGroup bool, m map[string]MappedPolicy, basePrefix string) error { + var p MappedPolicy + err := getIAMConfig(&p, kv.Value) + if err != nil { + if err == errConfigNotFound { + return errNoSuchPolicy + } + return err + } + name := extractPathPrefixAndSuffix(string(kv.Key), basePrefix, ".json") + m[name] = p + return nil } func (ies *IAMEtcdStore) loadMappedPolicies(ctx context.Context, userType IAMUserType, isGroup bool, m map[string]MappedPolicy) error { @@ -432,17 +464,16 @@ func (ies *IAMEtcdStore) loadMappedPolicies(ctx context.Context, userType IAMUse basePrefix = iamConfigPolicyDBUsersPrefix } } - - r, err := ies.client.Get(cctx, basePrefix, etcd.WithPrefix(), etcd.WithKeysOnly()) + // Retrieve all keys and values to avoid too many calls to etcd in case of + // a large number of policy mappings + r, err := ies.client.Get(cctx, basePrefix, etcd.WithPrefix()) if err != nil { return err } - users := etcdKvsToSetPolicyDB(basePrefix, r.Kvs) - - // Reload config and policies for all users. - for _, user := range users.ToSlice() { - if err = ies.loadMappedPolicy(ctx, user, userType, isGroup, m); err != nil && err != errNoSuchPolicy { + // Parse all policies mapping to create the proper data model + for _, kv := range r.Kvs { + if err = getMappedPolicy(ctx, kv, userType, isGroup, m, basePrefix); err != nil && err != errNoSuchPolicy { return err } } diff --git a/cmd/iam-etcd-store_test.go b/cmd/iam-etcd-store_test.go new file mode 100644 index 000000000..bfbea2bfd --- /dev/null +++ b/cmd/iam-etcd-store_test.go @@ -0,0 +1,39 @@ +/* + * MinIO Cloud Storage, (C) 2016 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "testing" +) + +func TestExtractPrefixAndSuffix(t *testing.T) { + specs := []struct { + path, prefix, suffix string + expected string + }{ + {"config/iam/groups/foo.json", "config/iam/groups/", ".json", "foo"}, + {"config/iam/groups/./foo.json", "config/iam/groups/", ".json", "foo"}, + {"config/iam/groups/foo/config.json", "config/iam/groups/", "/config.json", "foo"}, + {"config/iam/groups/foo/config.json", "config/iam/groups/", "config.json", "foo"}, + } + for i, test := range specs { + result := extractPathPrefixAndSuffix(test.path, test.prefix, test.suffix) + if result != test.expected { + t.Errorf("unexpected result on test[%v]: expected[%s] but had [%s]", i, test.expected, result) + } + } +} diff --git a/cmd/iam.go b/cmd/iam.go index b7e8a3bdc..9af4f810a 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -561,6 +561,7 @@ func (sys *IAMSys) Load(ctx context.Context, store IAMStorageAPI) error { default: close(sys.configLoaded) } + logger.Info("IAM initialization complete") return nil }