From f3334159a486417bfdaf1e18c6b52b7f7a8e2b61 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 16 Mar 2017 11:06:17 -0700 Subject: [PATCH] config: Relax browser and region to be empty. (#3912) - If browser field is missing or empty then default to "on". - If region field is empty or missing then default to "us-east-1" (S3 spec behavior) --- cmd/config-v14.go | 28 +++++++++++++++++++++++---- cmd/config-v14_test.go | 43 +++++++++++++++++++++++------------------- cmd/server-main.go | 6 ++++-- 3 files changed, 52 insertions(+), 25 deletions(-) diff --git a/cmd/config-v14.go b/cmd/config-v14.go index a2986091e..e17d2b787 100644 --- a/cmd/config-v14.go +++ b/cmd/config-v14.go @@ -18,6 +18,7 @@ package cmd import ( "errors" + "fmt" "io/ioutil" "os" "strings" @@ -244,12 +245,12 @@ func validateConfig() error { // Validate region field if srvCfg.GetRegion() == "" { - return errors.New("region config is empty") + return errors.New("Region config value cannot be empty") } // Validate browser field if b := strings.ToLower(srvCfg.GetBrowser()); b != "on" && b != "off" { - return errors.New("invalid browser config") + return fmt.Errorf("Browser config value %s is invalid", b) } // Validate credential field @@ -286,6 +287,10 @@ func (s *serverConfigV14) SetRegion(region string) { serverConfigMu.Lock() defer serverConfigMu.Unlock() + // Empty region means "us-east-1" by default from S3 spec. + if region == "" { + region = "us-east-1" + } s.Region = region } @@ -294,7 +299,12 @@ func (s serverConfigV14) GetRegion() string { serverConfigMu.RLock() defer serverConfigMu.RUnlock() - return s.Region + if s.Region != "" { + return s.Region + } // region empty + + // Empty region means "us-east-1" by default from S3 spec. + return "us-east-1" } // SetCredentials set new credentials. @@ -320,6 +330,11 @@ func (s *serverConfigV14) SetBrowser(v string) { defer serverConfigMu.Unlock() // Set browser param + if v == "" { + v = "on" // Browser is on by default. + } + + // Set the new value. s.Browser = v } @@ -328,7 +343,12 @@ func (s serverConfigV14) GetBrowser() string { serverConfigMu.RLock() defer serverConfigMu.RUnlock() - return s.Browser + if s.Browser != "" { + return s.Browser + } // empty browser. + + // Empty browser means "on" by default. + return "on" } // Save config. diff --git a/cmd/config-v14_test.go b/cmd/config-v14_test.go index eadb5966a..718b6276d 100644 --- a/cmd/config-v14_test.go +++ b/cmd/config-v14_test.go @@ -200,11 +200,13 @@ func TestCheckDupJSONKeys(t *testing.T) { } +// Tests config validator.. func TestValidateConfig(t *testing.T) { rootPath, err := newTestConfig(globalMinioDefaultRegion) if err != nil { t.Fatalf("Init Test config failed") } + // remove the root directory after the test ends. defer removeAll(rootPath) @@ -234,52 +236,55 @@ func TestValidateConfig(t *testing.T) { // Test 6 - missing secret key {`{"version": "` + v + `", "browser": "on", "credential" : {"accessKey":"minio", "secretKey":""}}`, false}, - // Test 7 - missing region - {`{"version": "` + v + `", "browser": "on", "credential" : {"accessKey":"minio", "secretKey":"minio123"}}`, false}, + // Test 7 - missing region should pass, defaults to 'us-east-1'. + {`{"version": "` + v + `", "browser": "on", "credential" : {"accessKey":"minio", "secretKey":"minio123"}}`, true}, + + // Test 8 - missing browser should pass, defaults to 'on'. + {`{"version": "` + v + `", "region": "us-east-1", "credential" : {"accessKey":"minio", "secretKey":"minio123"}}`, true}, - // Test 8 - success + // Test 9 - success {`{"version": "` + v + `", "browser": "on", "region":"us-east-1", "credential" : {"accessKey":"minio", "secretKey":"minio123"}}`, true}, - // Test 9 - duplicated json keys + // Test 10 - duplicated json keys {`{"version": "` + v + `", "browser": "on", "browser": "on", "region":"us-east-1", "credential" : {"accessKey":"minio", "secretKey":"minio123"}}`, false}, - // Test 10 - Wrong Console logger level + // Test 11 - Wrong Console logger level {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "logger": { "console": { "enable": true, "level": "foo" } }}`, false}, - // Test 11 - Wrong File logger level + // Test 12 - Wrong File logger level {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "logger": { "file": { "enable": true, "level": "foo" } }}`, false}, - // Test 12 - Test AMQP + // Test 13 - Test AMQP {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "amqp": { "1": { "enable": true, "url": "", "exchange": "", "routingKey": "", "exchangeType": "", "mandatory": false, "immediate": false, "durable": false, "internal": false, "noWait": false, "autoDeleted": false }}}}`, false}, - // Test 13 - Test NATS + // Test 14 - Test NATS {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "nats": { "1": { "enable": true, "address": "", "subject": "", "username": "", "password": "", "token": "", "secure": false, "pingInterval": 0, "streaming": { "enable": false, "clusterID": "", "clientID": "", "async": false, "maxPubAcksInflight": 0 } } }}}`, false}, - // Test 14 - Test ElasticSearch + // Test 15 - Test ElasticSearch {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "elasticsearch": { "1": { "enable": true, "url": "", "index": "" } }}}`, false}, - // Test 15 - Test Redis + // Test 16 - Test Redis {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "redis": { "1": { "enable": true, "address": "", "password": "", "key": "" } }}}`, false}, - // Test 16 - Test PostgreSQL + // Test 17 - Test PostgreSQL {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "postgresql": { "1": { "enable": true, "connectionString": "", "table": "", "host": "", "port": "", "user": "", "password": "", "database": "" }}}}`, false}, - // Test 17 - Test Kafka + // Test 18 - Test Kafka {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "kafka": { "1": { "enable": true, "brokers": null, "topic": "" } }}}`, false}, - // Test 18 - Test Webhook + // Test 19 - Test Webhook {`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "webhook": { "1": { "enable": true, "endpoint": "" } }}}`, false}, } for i, testCase := range testCases { - if err := ioutil.WriteFile(configPath, []byte(testCase.configData), 0700); err != nil { - t.Error(err) + if werr := ioutil.WriteFile(configPath, []byte(testCase.configData), 0700); werr != nil { + t.Fatal(werr) } - err := validateConfig() - if testCase.shouldPass && err != nil { - t.Errorf("Test %d, should pass but it failed with err = %v", i+1, err) + verr := validateConfig() + if testCase.shouldPass && verr != nil { + t.Errorf("Test %d, should pass but it failed with err = %v", i+1, verr) } - if !testCase.shouldPass && err == nil { + if !testCase.shouldPass && verr == nil { t.Errorf("Test %d, should fail but it succeed.", i+1) } } diff --git a/cmd/server-main.go b/cmd/server-main.go index b45c52e22..61223af23 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -128,7 +128,7 @@ func initConfig() { console.Fatalf("Invalid access/secret Key set in environment. Err: %s.\n", err) } - // Envs are set globally. + // credential Envs are set globally. globalIsEnvCreds = true } @@ -138,7 +138,9 @@ func initConfig() { console.Fatalf("Invalid value ‘%s’ in MINIO_BROWSER environment variable.", browser) } - globalIsEnvBrowser = strings.EqualFold(browser, "off") + // browser Envs are set globally, this doesn't represent + // if browser is turned off or on. + globalIsEnvBrowser = true } envs := envParams{