fix : IAM Intialization failing with a large number of users/policies (#11701)

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,
+		user := extractPathPrefixAndSuffix(string(kv.Key), prefix, path.Base(string(kv.Key)))
-		// 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))))
 		users.Add(user)
 	}
 	return users
 }
 
-func etcdKvsToSetPolicyDB(prefix string, kvs []*mvccpb.KeyValue) set.StringSet {
+// Extract path string by stripping off the `prefix` value and the suffix,
-	items := set.NewStringSet()
+// value, usually in the following form.
-	for _, kv := range kvs {
+//  s := "config/iam/users/foo/config.json"
-		// Extract user item by stripping off prefix and then
+//  prefix := "config/iam/users/"
-		// stripping of ".json" suffix.
+//  suffix := "config.json"
-		//
+//  result is foo
-		// key := "config/iam/policydb/users/myuser1.json"
+func extractPathPrefixAndSuffix(s string, prefix string, suffix string) string {
-		// prefix := "config/iam/policydb/users/"
+	return path.Clean(strings.TrimSuffix(strings.TrimPrefix(string(s), prefix), suffix))
-		// 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
 }
 
 // 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
 	}
-
+	return getIAMConfig(item, pdata)
-	if globalConfigEncrypted && !utf8.Valid(pdata) {
-		pdata, err = madmin.DecryptData(globalActiveCred.String(), bytes.NewReader(pdata))
-		if err != nil {
-			return err
-		}
-	}
-
-	return json.Unmarshal(pdata, item)
 }
 
 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)
+	// Parse all values to construct the policies data model.
-
+	for _, kvs := range r.Kvs {
-	// Reload config and policies for all policys.
+		if err = ies.getPolicyDoc(ctx, kvs, m); err != nil && err != errNoSuchPolicy {
-	for _, policyName := range policies.ToSlice() {
-		if err = ies.loadPolicyDoc(ctx, policyName, 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)
+	// Parse all users values to create the proper data model
-
+	for _, userKv := range r.Kvs {
-	// Reload config for all users.
+		if err = ies.getUser(ctx, userKv, userType, m, basePrefix); err != nil && err != errNoSuchUser {
-	for _, user := range users.ToSlice() {
-		if err = ies.loadUser(ctx, user, userType, m); 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
 		}
 	}
-
+	// Retrieve all keys and values to avoid too many calls to etcd in case of
-	r, err := ies.client.Get(cctx, basePrefix, etcd.WithPrefix(), etcd.WithKeysOnly())
+	// 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)
+	// Parse all policies mapping to create the proper data model
-
+	for _, kv := range r.Kvs {
-	// Reload config and policies for all users.
+		if err = getMappedPolicy(ctx, kv, userType, isGroup, m, basePrefix); err != nil && err != errNoSuchPolicy {
-	for _, user := range users.ToSlice() {
-		if err = ies.loadMappedPolicy(ctx, user, userType, isGroup, m); err != nil && err != errNoSuchPolicy {
 			return err
 		}
 	}

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)
+		}
+	}
+}

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
 }