configMigrate() returns errors + tests (#2735)

cmd/config-migrate.go

@@ -17,7 +17,7 @@
 package cmd
 
 import (
-	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 
@@ -25,51 +25,72 @@ import (
 	"github.com/minio/minio/pkg/quick"
 )
 
-func migrateConfig() {
+func migrateConfig() error {
 	// Purge all configs with version '1'.
-	purgeV1()
+	if err := purgeV1(); err != nil {
+		return err
+	}
 	// Migrate version '2' to '3'.
-	migrateV2ToV3()
+	if err := migrateV2ToV3(); err != nil {
+		return err
+	}
 	// Migrate version '3' to '4'.
-	migrateV3ToV4()
+	if err := migrateV3ToV4(); err != nil {
+		return err
+	}
 	// Migrate version '4' to '5'.
-	migrateV4ToV5()
+	if err := migrateV4ToV5(); err != nil {
+		return err
+	}
 	// Migrate version '5' to '6.
-	migrateV5ToV6()
+	if err := migrateV5ToV6(); err != nil {
+		return err
+	}
 	// Migrate version '6' to '7'.
-	migrateV6ToV7()
+	if err := migrateV6ToV7(); err != nil {
+		return err
+	}
+	return nil
 }
 
 // Version '1' is not supported anymore and deprecated, safe to delete.
-func purgeV1() {
+func purgeV1() error {
 	cv1, err := loadConfigV1()
-	if err != nil && os.IsNotExist(err) {
-		return
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("Unable to load config version ‘1’. %v", err)
+
 	}
-	fatalIf(err, "Unable to load config version ‘1’.")
 
 	if cv1.Version == "1" {
 		console.Println("Removed unsupported config version ‘1’.")
 		/// Purge old fsUsers.json file
 		configPath, err := getConfigPath()
-		fatalIf(err, "Unable to retrieve config path.")
+		if err != nil {
+			return fmt.Errorf("Unable to retrieve config path. %v", err)
+		}
 
 		configFile := filepath.Join(configPath, "fsUsers.json")
 		removeAll(configFile)
+		return nil
 	}
-	fatalIf(errors.New(""), "Failed to migrate unrecognized config version ‘"+cv1.Version+"’.")
+	return fmt.Errorf("Failed to migrate unrecognized config version ‘" + cv1.Version + "’.")
 }
 
 // Version '2' to '3' config migration adds new fields and re-orders
 // previous fields. Simplifies config for future additions.
-func migrateV2ToV3() {
+func migrateV2ToV3() error {
 	cv2, err := loadConfigV2()
-	if err != nil && os.IsNotExist(err) {
-		return
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("Unable to load config version ‘2’. %v", err)
 	}
-	fatalIf(err, "Unable to load config version ‘2’.")
 	if cv2.Version != "2" {
-		return
+		return nil
 	}
 	srvConfig := &configV3{}
 	srvConfig.Version = "3"
@@ -104,29 +125,38 @@ func migrateV2ToV3() {
 	srvConfig.Logger.Syslog = slogger
 
 	qc, err := quick.New(srvConfig)
-	fatalIf(err, "Unable to initialize config.")
+	if err != nil {
+		return fmt.Errorf("Unable to initialize config. %v", err)
+	}
 
 	configFile, err := getConfigFile()
-	fatalIf(err, "Unable to get config file.")
+	if err != nil {
+		return fmt.Errorf("Unable to get config file. %v", err)
+	}
 
 	// Migrate the config.
 	err = qc.Save(configFile)
-	fatalIf(err, "Failed to migrate config from ‘"+cv2.Version+"’ to ‘"+srvConfig.Version+"’ failed.")
+	if err != nil {
+		return fmt.Errorf("Failed to migrate config from ‘"+cv2.Version+"’ to ‘"+srvConfig.Version+"’ failed. %v", err)
+	}
 
 	console.Println("Migration from version ‘" + cv2.Version + "’ to ‘" + srvConfig.Version + "’ completed successfully.")
+	return nil
 }
 
 // Version '3' to '4' migrates config, removes previous fields related
 // to backend types and server address. This change further simplifies
 // the config for future additions.
-func migrateV3ToV4() {
+func migrateV3ToV4() error {
 	cv3, err := loadConfigV3()
-	if err != nil && os.IsNotExist(err) {
-		return
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("Unable to load config version ‘3’. %v", err)
 	}
-	fatalIf(err, "Unable to load config version ‘3’.")
 	if cv3.Version != "3" {
-		return
+		return nil
 	}
 
 	// Save only the new fields, ignore the rest.
@@ -143,27 +173,36 @@ func migrateV3ToV4() {
 	srvConfig.Logger.Syslog = cv3.Logger.Syslog
 
 	qc, err := quick.New(srvConfig)
-	fatalIf(err, "Unable to initialize the quick config.")
+	if err != nil {
+		return fmt.Errorf("Unable to initialize the quick config. %v", err)
+	}
 	configFile, err := getConfigFile()
-	fatalIf(err, "Unable to get config file.")
+	if err != nil {
+		return fmt.Errorf("Unable to get config file. %v", err)
+	}
 
 	err = qc.Save(configFile)
-	fatalIf(err, "Failed to migrate config from ‘"+cv3.Version+"’ to ‘"+srvConfig.Version+"’ failed.")
+	if err != nil {
+		return fmt.Errorf("Failed to migrate config from ‘"+cv3.Version+"’ to ‘"+srvConfig.Version+"’ failed. %v", err)
+	}
 
 	console.Println("Migration from version ‘" + cv3.Version + "’ to ‘" + srvConfig.Version + "’ completed successfully.")
+	return nil
 }
 
 // Version '4' to '5' migrates config, removes previous fields related
 // to backend types and server address. This change further simplifies
 // the config for future additions.
-func migrateV4ToV5() {
+func migrateV4ToV5() error {
 	cv4, err := loadConfigV4()
-	if err != nil && os.IsNotExist(err) {
-		return
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("Unable to load config version ‘4’. %v", err)
 	}
-	fatalIf(err, "Unable to load config version ‘4’.")
 	if cv4.Version != "4" {
-		return
+		return nil
 	}
 
 	// Save only the new fields, ignore the rest.
@@ -183,27 +222,36 @@ func migrateV4ToV5() {
 	srvConfig.Logger.Redis.Enable = false
 
 	qc, err := quick.New(srvConfig)
-	fatalIf(err, "Unable to initialize the quick config.")
+	if err != nil {
+		return fmt.Errorf("Unable to initialize the quick config. %v", err)
+	}
 	configFile, err := getConfigFile()
-	fatalIf(err, "Unable to get config file.")
+	if err != nil {
+		return fmt.Errorf("Unable to get config file. %v", err)
+	}
 
 	err = qc.Save(configFile)
-	fatalIf(err, "Failed to migrate config from ‘"+cv4.Version+"’ to ‘"+srvConfig.Version+"’ failed.")
+	if err != nil {
+		return fmt.Errorf("Failed to migrate config from ‘"+cv4.Version+"’ to ‘"+srvConfig.Version+"’ failed. %v", err)
+	}
 
 	console.Println("Migration from version ‘" + cv4.Version + "’ to ‘" + srvConfig.Version + "’ completed successfully.")
+	return nil
 }
 
 // Version '5' to '6' migrates config, removes previous fields related
 // to backend types and server address. This change further simplifies
 // the config for future additions.
-func migrateV5ToV6() {
+func migrateV5ToV6() error {
 	cv5, err := loadConfigV5()
-	if err != nil && os.IsNotExist(err) {
-		return
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("Unable to load config version ‘5’. %v", err)
 	}
-	fatalIf(err, "Unable to load config version ‘5’.")
 	if cv5.Version != "5" {
-		return
+		return nil
 	}
 
 	// Save only the new fields, ignore the rest.
@@ -250,27 +298,36 @@ func migrateV5ToV6() {
 	}
 
 	qc, err := quick.New(srvConfig)
-	fatalIf(err, "Unable to initialize the quick config.")
+	if err != nil {
+		return fmt.Errorf("Unable to initialize the quick config. %v", err)
+	}
 	configFile, err := getConfigFile()
-	fatalIf(err, "Unable to get config file.")
+	if err != nil {
+		return fmt.Errorf("Unable to get config file. %v", err)
+	}
 
 	err = qc.Save(configFile)
-	fatalIf(err, "Failed to migrate config from ‘"+cv5.Version+"’ to ‘"+srvConfig.Version+"’ failed.")
+	if err != nil {
+		return fmt.Errorf("Failed to migrate config from ‘"+cv5.Version+"’ to ‘"+srvConfig.Version+"’ failed. %v", err)
+	}
 
 	console.Println("Migration from version ‘" + cv5.Version + "’ to ‘" + srvConfig.Version + "’ completed successfully.")
+	return nil
 }
 
 // Version '6' to '7' migrates config, removes previous fields related
 // to backend types and server address. This change further simplifies
 // the config for future additions.
-func migrateV6ToV7() {
+func migrateV6ToV7() error {
 	cv6, err := loadConfigV6()
-	if err != nil && os.IsNotExist(err) {
-		return
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("Unable to load config version ‘6’. %v", err)
 	}
-	fatalIf(err, "Unable to load config version ‘6’.")
 	if cv6.Version != "6" {
-		return
+		return nil
 	}
 
 	// Save only the new fields, ignore the rest.
@@ -305,12 +362,19 @@ func migrateV6ToV7() {
 	}
 
 	qc, err := quick.New(srvConfig)
-	fatalIf(err, "Unable to initialize the quick config.")
+	if err != nil {
+		return fmt.Errorf("Unable to initialize the quick config. %v", err)
+	}
 	configFile, err := getConfigFile()
-	fatalIf(err, "Unable to get config file.")
+	if err != nil {
+		return fmt.Errorf("Unable to get config file. %v", err)
+	}
 
 	err = qc.Save(configFile)
-	fatalIf(err, "Failed to migrate config from ‘"+cv6.Version+"’ to ‘"+srvConfig.Version+"’ failed.")
+	if err != nil {
+		return fmt.Errorf("Failed to migrate config from ‘"+cv6.Version+"’ to ‘"+srvConfig.Version+"’ failed. %v", err)
+	}
 
 	console.Println("Migration from version ‘" + cv6.Version + "’ to ‘" + srvConfig.Version + "’ completed successfully.")
+	return nil
 }

cmd/config-migrate_test.go

@@ -0,0 +1,188 @@
+/*
+ * 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 (
+	"io/ioutil"
+	"os"
+	"strconv"
+	"testing"
+)
+
+const lastConfigVersion = 7
+
+// TestServerConfigMigrateV1 - tests if a config v1 is purged
+func TestServerConfigMigrateV1(t *testing.T) {
+	rootPath, err := newTestConfig("us-east-1")
+	if err != nil {
+		t.Fatalf("Init Test config failed")
+	}
+	// remove the root folder after the test ends.
+	defer removeAll(rootPath)
+
+	setGlobalConfigPath(rootPath)
+
+	// Create a V1 config json file and store it
+	configJSON := "{ \"version\":\"1\", \"accessKeyId\":\"abcde\", \"secretAccessKey\":\"abcdefgh\"}"
+	configPath := rootPath + "/fsUsers.json"
+	if err := ioutil.WriteFile(configPath, []byte(configJSON), 0644); err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
+
+	// Fire a migrateConfig()
+	if err := migrateConfig(); err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
+	// Check if config v1 is removed from filesystem
+	if _, err := os.Stat(configPath); err == nil || !os.IsNotExist(err) {
+		t.Fatal("Config V1 file is not purged")
+	}
+
+	// Initialize server config and check again if everything is fine
+	if err := initConfig(); err != nil {
+		t.Fatalf("Unable to initialize from updated config file %s", err)
+	}
+}
+
+// TestServerConfigMigrateV1 - tests if all migrate code return nil when config file is not existent
+func TestServerConfigMigrateInexistentConfig(t *testing.T) {
+	rootPath, err := newTestConfig("us-east-1")
+	if err != nil {
+		t.Fatalf("Init Test config failed")
+	}
+	// remove the root folder after the test ends.
+	defer removeAll(rootPath)
+
+	setGlobalConfigPath(rootPath)
+	configPath := rootPath + "/" + globalMinioConfigFile
+
+	// Remove config file
+	if err := os.Remove(configPath); err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
+
+	if err := migrateV2ToV3(); err != nil {
+		t.Fatal("migrate v2 to v3 should succeed when no config file is found")
+	}
+	if err := migrateV3ToV4(); err != nil {
+		t.Fatal("migrate v3 to v4 should succeed when no config file is found")
+	}
+	if err := migrateV4ToV5(); err != nil {
+		t.Fatal("migrate v4 to v5 should succeed when no config file is found")
+	}
+	if err := migrateV5ToV6(); err != nil {
+		t.Fatal("migrate v5 to v6 should succeed when no config file is found")
+	}
+	if err := migrateV6ToV7(); err != nil {
+		t.Fatal("migrate v6 to v7 should succeed when no config file is found")
+	}
+}
+
+// TestServerConfigMigrateV2toV7 - tests if a config from v2 to v7 is successfully done
+func TestServerConfigMigrateV2toV7(t *testing.T) {
+	rootPath, err := newTestConfig("us-east-1")
+	if err != nil {
+		t.Fatalf("Init Test config failed")
+	}
+	// remove the root folder after the test ends.
+	defer removeAll(rootPath)
+
+	setGlobalConfigPath(rootPath)
+	configPath := rootPath + "/" + globalMinioConfigFile
+
+	// Create a corrupted config file
+	if err := ioutil.WriteFile(configPath, []byte("{ \"version\":\"2\","), 0644); err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
+	// Fire a migrateConfig()
+	if err := migrateConfig(); err == nil {
+		t.Fatal("migration should fail with corrupted config file")
+	}
+
+	accessKey := "accessfoo"
+	secretKey := "secretfoo"
+
+	// Create a V2 config json file and store it
+	configJSON := "{ \"version\":\"2\", \"credentials\": {\"accessKeyId\":\"" + accessKey + "\", \"secretAccessKey\":\"" + secretKey + "\", \"region\":\"us-east-1\"}, \"mongoLogger\":{\"addr\":\"127.0.0.1:3543\", \"db\":\"foodb\", \"collection\":\"foo\"}, \"syslogLogger\":{\"network\":\"127.0.0.1:543\", \"addr\":\"addr\"}, \"fileLogger\":{\"filename\":\"log.out\"}}"
+	if err := ioutil.WriteFile(configPath, []byte(configJSON), 0644); err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
+	// Fire a migrateConfig()
+	if err := migrateConfig(); err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
+
+	// Initialize server config and check again if everything is fine
+	if err := initConfig(); err != nil {
+		t.Fatalf("Unable to initialize from updated config file %s", err)
+	}
+
+	// Check the version number in the upgraded config file
+	expectedVersion := strconv.Itoa(lastConfigVersion)
+	if serverConfig.Version != expectedVersion {
+		t.Fatalf("Expect version "+expectedVersion+", found: %v", serverConfig.Version)
+	}
+
+	// Check if accessKey and secretKey are not altered during migration
+	if serverConfig.Credential.AccessKeyID != accessKey {
+		t.Fatalf("Access key lost during migration, expected: %v, found:%v", accessKey, serverConfig.Credential.AccessKeyID)
+	}
+	if serverConfig.Credential.SecretAccessKey != secretKey {
+		t.Fatalf("Secret key lost during migration, expected: %v, found: %v", secretKey, serverConfig.Credential.SecretAccessKey)
+	}
+
+	// Initialize server config and check again if everything is fine
+	if err := initConfig(); err != nil {
+		t.Fatalf("Unable to initialize from updated config file %s", err)
+	}
+}
+
+// TestServerConfigMigrateFaultyConfig - checks if all migrate code return errors with corrupted config files
+func TestServerConfigMigrateFaultyConfig(t *testing.T) {
+	rootPath, err := newTestConfig("us-east-1")
+	if err != nil {
+		t.Fatalf("Init Test config failed")
+	}
+	// remove the root folder after the test ends.
+	defer removeAll(rootPath)
+
+	setGlobalConfigPath(rootPath)
+	configPath := rootPath + "/" + globalMinioConfigFile
+
+	// Create a corrupted config file
+	if err := ioutil.WriteFile(configPath, []byte("{ \"version\":\""), 0644); err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
+
+	// Test different migrate versions and be sure they are returning an error
+	if err := migrateV2ToV3(); err == nil {
+		t.Fatal("migrateConfigV2ToV3() should fail with a corrupted json")
+	}
+	if err := migrateV3ToV4(); err == nil {
+		t.Fatal("migrateConfigV3ToV4() should fail with a corrupted json")
+	}
+	if err := migrateV4ToV5(); err == nil {
+		t.Fatal("migrateConfigV4ToV5() should fail with a corrupted json")
+	}
+	if err := migrateV5ToV6(); err == nil {
+		t.Fatal("migrateConfigV5ToV6() should fail with a corrupted json")
+	}
+	if err := migrateV6ToV7(); err == nil {
+		t.Fatal("migrateConfigV6ToV7() should fail with a corrupted json")
+	}
+
+}

cmd/config.go

@@ -54,7 +54,9 @@ func getConfigPath() (string, error) {
 // mustGetConfigPath must get server config path.
 func mustGetConfigPath() string {
 	configPath, err := getConfigPath()
-	fatalIf(err, "Unable to get config path.")
+	if err != nil {
+		return ""
+	}
 	return configPath
 }
 
@@ -69,7 +71,11 @@ func createConfigPath() error {
 
 // isConfigFileExists - returns true if config file exists.
 func isConfigFileExists() bool {
-	st, err := os.Stat(mustGetConfigFile())
+	path, err := getConfigFile()
+	if err != nil {
+		return false
+	}
+	st, err := os.Stat(path)
 	// If file exists and is regular return true.
 	if err == nil && st.Mode().IsRegular() {
 		return true
@@ -80,8 +86,9 @@ func isConfigFileExists() bool {
 // mustGetConfigFile must get server config file.
 func mustGetConfigFile() string {
 	configFile, err := getConfigFile()
-	fatalIf(err, "Unable to get config file.")
-
+	if err != nil {
+		return ""
+	}
 	return configFile
 }
 

cmd/main.go

@@ -17,6 +17,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"sort"
@@ -79,7 +80,8 @@ func init() {
 
 func migrate() {
 	// Migrate config file
-	migrateConfig()
+	err := migrateConfig()
+	fatalIf(err, "Config migration failed.")
 
 	// Migrate other configs here.
 }
@@ -157,8 +159,13 @@ func checkMainSyntax(c *cli.Context) {
 func Main() {
 	app := registerApp()
 	app.Before = func(c *cli.Context) error {
+
+		configDir := c.GlobalString("config-dir")
+		if configDir == "" {
+			fatalIf(errors.New("Config directory is empty"), "Unable to get config file.")
+		}
 		// Sets new config folder.
-		setGlobalConfigPath(c.GlobalString("config-dir"))
+		setGlobalConfigPath(configDir)
 
 		// Valid input arguments to main.
 		checkMainSyntax(c)