Refactor and simplify etcd helpers used in IAM subsystem (#7980)

cmd/config-common.go

@@ -20,9 +20,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
-	"fmt"
 
-	etcd "github.com/coreos/etcd/clientv3"
 	"github.com/minio/minio/cmd/logger"
 	"github.com/minio/minio/pkg/hash"
 )
@@ -51,38 +49,10 @@ func readConfig(ctx context.Context, objAPI ObjectLayer, configFile string) ([]b
 	return buffer.Bytes(), nil
 }
 
-func deleteConfigEtcd(ctx context.Context, client *etcd.Client, configFile string) error {
-	timeoutCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
-	defer cancel()
-
-	_, err := client.Delete(timeoutCtx, configFile)
-	if err != nil {
-		if err == context.DeadlineExceeded {
-			return fmt.Errorf("etcd setup is unreachable, please check your endpoints %s",
-				client.Endpoints())
-		}
-		return fmt.Errorf("unexpected error %s returned by etcd setup, please check your endpoints %s",
-			err, client.Endpoints())
-	}
-	return nil
-}
-
 func deleteConfig(ctx context.Context, objAPI ObjectLayer, configFile string) error {
 	return objAPI.DeleteObject(ctx, minioMetaBucket, configFile)
 }
 
-func saveConfigEtcd(ctx context.Context, client *etcd.Client, configFile string, data []byte) error {
-	timeoutCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
-	defer cancel()
-	_, err := client.Put(timeoutCtx, configFile, string(data))
-	if err == context.DeadlineExceeded {
-		return fmt.Errorf("etcd setup is unreachable, please check your endpoints %s", client.Endpoints())
-	} else if err != nil {
-		return fmt.Errorf("unexpected error %s returned by etcd setup, please check your endpoints %s", err, client.Endpoints())
-	}
-	return nil
-}
-
 func saveConfig(ctx context.Context, objAPI ObjectLayer, configFile string, data []byte) error {
 	hashReader, err := hash.NewReader(bytes.NewReader(data), int64(len(data)), "", getSHA256Hash(data), int64(len(data)), globalCLIContext.StrictS3Compat)
 	if err != nil {
@@ -93,29 +63,6 @@ func saveConfig(ctx context.Context, objAPI ObjectLayer, configFile string, data
 	return err
 }
 
-func readConfigEtcd(ctx context.Context, client *etcd.Client, configFile string) ([]byte, error) {
-	timeoutCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
-	defer cancel()
-	resp, err := client.Get(timeoutCtx, configFile)
-	if err != nil {
-		if err == context.DeadlineExceeded {
-			return nil, fmt.Errorf("etcd setup is unreachable, please check your endpoints %s",
-				client.Endpoints())
-		}
-		return nil, fmt.Errorf("unexpected error %s returned by etcd setup, please check your endpoints %s",
-			err, client.Endpoints())
-	}
-	if resp.Count == 0 {
-		return nil, errConfigNotFound
-	}
-	for _, ev := range resp.Kvs {
-		if string(ev.Key) == configFile {
-			return ev.Value, nil
-		}
-	}
-	return nil, errConfigNotFound
-}
-
 func checkConfig(ctx context.Context, objAPI ObjectLayer, configFile string) error {
 	if _, err := objAPI.GetObjectInfo(ctx, minioMetaBucket, configFile, ObjectOptions{}); err != nil {
 		// Treat object not found as config not found.

cmd/config-migrate.go

@@ -2426,7 +2426,7 @@ func migrateConfigToMinioSys(objAPI ObjectLayer) (err error) {
 	defer func() {
 		if err == nil {
 			if globalEtcdClient != nil {
-				deleteConfigEtcd(context.Background(), globalEtcdClient, configFile)
+				deleteKeyEtcd(context.Background(), globalEtcdClient, configFile)
 			} else {
 				// Rename config.json to config.json.deprecated only upon
 				// success of this function.

cmd/etcd.go

@@ -0,0 +1,72 @@
+/*
+ * MinIO Cloud Storage, (C) 2019 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 (
+	"context"
+	"errors"
+	"fmt"
+
+	etcd "github.com/coreos/etcd/clientv3"
+)
+
+var errEtcdUnreachable = errors.New("etcd is unreachable, please check your endpoints")
+
+func etcdErrToErr(err error, etcdEndpoints []string) error {
+	if err == nil {
+		return nil
+	}
+	switch err {
+	case context.DeadlineExceeded:
+		return fmt.Errorf("%s %s", errEtcdUnreachable, etcdEndpoints)
+	default:
+		return fmt.Errorf("unexpected error %s from etcd, please check your endpoints %s", err, etcdEndpoints)
+	}
+}
+
+func saveKeyEtcd(ctx context.Context, client *etcd.Client, key string, data []byte) error {
+	timeoutCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
+	defer cancel()
+	_, err := client.Put(timeoutCtx, key, string(data))
+	return etcdErrToErr(err, client.Endpoints())
+}
+
+func deleteKeyEtcd(ctx context.Context, client *etcd.Client, key string) error {
+	timeoutCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
+	defer cancel()
+
+	_, err := client.Delete(timeoutCtx, key)
+	return etcdErrToErr(err, client.Endpoints())
+}
+
+func readKeyEtcd(ctx context.Context, client *etcd.Client, key string) ([]byte, error) {
+	timeoutCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
+	defer cancel()
+	resp, err := client.Get(timeoutCtx, key)
+	if err != nil {
+		return nil, etcdErrToErr(err, client.Endpoints())
+	}
+	if resp.Count == 0 {
+		return nil, errConfigNotFound
+	}
+	for _, ev := range resp.Kvs {
+		if string(ev.Key) == key {
+			return ev.Value, nil
+		}
+	}
+	return nil, errConfigNotFound
+}

cmd/iam.go

@@ -194,7 +194,7 @@ func listIAMConfigItems(objectAPI ObjectLayer, pathPrefix string, dirs bool,
 }
 
 func loadIAMConfigItemEtcd(ctx context.Context, item interface{}, path string) error {
-	pdata, err := readConfigEtcd(ctx, globalEtcdClient, path)
+	pdata, err := readKeyEtcd(ctx, globalEtcdClient, path)
 	if err != nil {
 		return err
 	}
@@ -206,7 +206,7 @@ func saveIAMConfigItemEtcd(ctx context.Context, item interface{}, path string) e
 	if err != nil {
 		return err
 	}
-	return saveConfigEtcd(context.Background(), globalEtcdClient, path, data)
+	return saveKeyEtcd(ctx, globalEtcdClient, path, data)
 }
 
 // IAMSys - config system.
@@ -601,7 +601,7 @@ func migrateUsersConfigEtcdToV1(isSTS bool) error {
 			}
 
 			// 3. delete policy file in old loc.
-			deleteConfigEtcd(ctx, globalEtcdClient, oldPolicyPath)
+			deleteKeyEtcd(ctx, globalEtcdClient, oldPolicyPath)
 		}
 
 	next:
@@ -793,7 +793,7 @@ func (sys *IAMSys) DeletePolicy(policyName string) error {
 	var err error
 	pFile := getPolicyDocPath(policyName)
 	if globalEtcdClient != nil {
-		err = deleteConfigEtcd(context.Background(), globalEtcdClient, pFile)
+		err = deleteKeyEtcd(context.Background(), globalEtcdClient, pFile)
 	} else {
 		err = deleteConfig(context.Background(), objectAPI, pFile)
 	}
@@ -875,8 +875,8 @@ func (sys *IAMSys) DeleteUser(accessKey string) error {
 	idPath := getUserIdentityPath(accessKey, false)
 	if globalEtcdClient != nil {
 		// It is okay to ignore errors when deleting policy.json for the user.
-		deleteConfigEtcd(context.Background(), globalEtcdClient, mappingPath)
+		deleteKeyEtcd(context.Background(), globalEtcdClient, mappingPath)
-		err = deleteConfigEtcd(context.Background(), globalEtcdClient, idPath)
+		err = deleteKeyEtcd(context.Background(), globalEtcdClient, idPath)
 	} else {
 		// It is okay to ignore errors when deleting policy.json for the user.
 		_ = deleteConfig(context.Background(), objectAPI, mappingPath)
@@ -1338,10 +1338,9 @@ func loadEtcdUser(ctx context.Context, user string, isSTS bool, m map[string]aut
 	}
 
 	if u.Credentials.IsExpired() {
-		idPath := getUserIdentityPath(user, isSTS)
 		// Delete expired identity.
-		deleteConfigEtcd(ctx, globalEtcdClient, idPath)
+		deleteKeyEtcd(ctx, globalEtcdClient, getUserIdentityPath(user, isSTS))
-		deleteConfigEtcd(ctx, globalEtcdClient, getMappedPolicyPath(user, isSTS))
+		deleteKeyEtcd(ctx, globalEtcdClient, getMappedPolicyPath(user, isSTS))
 		return nil
 	}