From e66fb4bd7b83008ac2baa430329152cb082267fe Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Wed, 21 Sep 2016 17:44:57 +0100 Subject: [PATCH] configMigrate() returns errors + tests (#2735) --- cmd/config-migrate.go | 172 ++++++++++++++++++++++----------- cmd/config-migrate_test.go | 188 +++++++++++++++++++++++++++++++++++++ cmd/config.go | 15 ++- cmd/main.go | 11 ++- 4 files changed, 326 insertions(+), 60 deletions(-) create mode 100644 cmd/config-migrate_test.go diff --git a/cmd/config-migrate.go b/cmd/config-migrate.go index 87ecf0591..f77abe6a9 100644 --- a/cmd/config-migrate.go +++ b/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 } diff --git a/cmd/config-migrate_test.go b/cmd/config-migrate_test.go new file mode 100644 index 000000000..4aa6ef801 --- /dev/null +++ b/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") + } + +} diff --git a/cmd/config.go b/cmd/config.go index 77cf3150f..86912b5e0 100644 --- a/cmd/config.go +++ b/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 } diff --git a/cmd/main.go b/cmd/main.go index 2d7d0fe1a..0235065d7 100644 --- a/cmd/main.go +++ b/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)