diff --git a/cmd/browser-flag.go b/cmd/browser-flag.go new file mode 100644 index 000000000..d835d35be --- /dev/null +++ b/cmd/browser-flag.go @@ -0,0 +1,68 @@ +/* + * Minio Cloud Storage, (C) 2017 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 ( + "encoding/json" + "fmt" +) + +// BrowserFlag - wrapper bool type. +type BrowserFlag bool + +// String - returns string of BrowserFlag. +func (bf BrowserFlag) String() string { + if bf { + return "on" + } + + return "off" +} + +// MarshalJSON - converts BrowserFlag into JSON data. +func (bf BrowserFlag) MarshalJSON() ([]byte, error) { + return json.Marshal(bf.String()) +} + +// UnmarshalJSON - parses given data into BrowserFlag. +func (bf *BrowserFlag) UnmarshalJSON(data []byte) (err error) { + var s string + if err = json.Unmarshal(data, &s); err == nil { + b := BrowserFlag(true) + if s == "" { + // Empty string is treated as valid. + *bf = b + } else if b, err = ParseBrowserFlag(s); err == nil { + *bf = b + } + } + + return err +} + +// ParseBrowserFlag - parses string into BrowserFlag. +func ParseBrowserFlag(s string) (bf BrowserFlag, err error) { + if s == "on" { + bf = true + } else if s == "off" { + bf = false + } else { + err = fmt.Errorf("invalid value ‘%s’ for BrowserFlag", s) + } + + return bf, err +} diff --git a/cmd/browser-flag_test.go b/cmd/browser-flag_test.go new file mode 100644 index 000000000..68b58f14d --- /dev/null +++ b/cmd/browser-flag_test.go @@ -0,0 +1,137 @@ +/* + * Minio Cloud Storage, (C) 2017 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 ( + "errors" + "testing" +) + +// Test BrowserFlag.String() +func TestBrowserFlagString(t *testing.T) { + var bf BrowserFlag + + testCases := []struct { + flag BrowserFlag + expectedResult string + }{ + {bf, "off"}, + {BrowserFlag(true), "on"}, + {BrowserFlag(false), "off"}, + } + + for _, testCase := range testCases { + str := testCase.flag.String() + if testCase.expectedResult != str { + t.Fatalf("expected: %v, got: %v", testCase.expectedResult, str) + } + } +} + +// Test BrowserFlag.MarshalJSON() +func TestBrowserFlagMarshalJSON(t *testing.T) { + var bf BrowserFlag + + testCases := []struct { + flag BrowserFlag + expectedResult string + }{ + {bf, `"off"`}, + {BrowserFlag(true), `"on"`}, + {BrowserFlag(false), `"off"`}, + } + + for _, testCase := range testCases { + data, _ := testCase.flag.MarshalJSON() + if testCase.expectedResult != string(data) { + t.Fatalf("expected: %v, got: %v", testCase.expectedResult, string(data)) + } + } +} + +// Test BrowserFlag.UnmarshalJSON() +func TestBrowserFlagUnmarshalJSON(t *testing.T) { + testCases := []struct { + data []byte + expectedResult BrowserFlag + expectedErr error + }{ + {[]byte(`{}`), BrowserFlag(false), errors.New("json: cannot unmarshal object into Go value of type string")}, + {[]byte(`["on"]`), BrowserFlag(false), errors.New("json: cannot unmarshal array into Go value of type string")}, + {[]byte(`"junk"`), BrowserFlag(false), errors.New("invalid value ‘junk’ for BrowserFlag")}, + {[]byte(`"true"`), BrowserFlag(false), errors.New("invalid value ‘true’ for BrowserFlag")}, + {[]byte(`"false"`), BrowserFlag(false), errors.New("invalid value ‘false’ for BrowserFlag")}, + {[]byte(`"ON"`), BrowserFlag(false), errors.New("invalid value ‘ON’ for BrowserFlag")}, + {[]byte(`"OFF"`), BrowserFlag(false), errors.New("invalid value ‘OFF’ for BrowserFlag")}, + {[]byte(`""`), BrowserFlag(true), nil}, + {[]byte(`"on"`), BrowserFlag(true), nil}, + {[]byte(`"off"`), BrowserFlag(false), nil}, + } + + for _, testCase := range testCases { + var flag BrowserFlag + err := (&flag).UnmarshalJSON(testCase.data) + if testCase.expectedErr == nil { + if err != nil { + t.Fatalf("error: expected = , got = %v", err) + } + } else if err == nil { + t.Fatalf("error: expected = %v, got = ", testCase.expectedErr) + } else if testCase.expectedErr.Error() != err.Error() { + t.Fatalf("error: expected = %v, got = %v", testCase.expectedErr, err) + } + + if err == nil && testCase.expectedResult != flag { + t.Fatalf("result: expected: %v, got: %v", testCase.expectedResult, flag) + } + } +} + +// Test ParseBrowserFlag() +func TestParseBrowserFlag(t *testing.T) { + testCases := []struct { + flagStr string + expectedResult BrowserFlag + expectedErr error + }{ + {"", BrowserFlag(false), errors.New("invalid value ‘’ for BrowserFlag")}, + {"junk", BrowserFlag(false), errors.New("invalid value ‘junk’ for BrowserFlag")}, + {"true", BrowserFlag(false), errors.New("invalid value ‘true’ for BrowserFlag")}, + {"false", BrowserFlag(false), errors.New("invalid value ‘false’ for BrowserFlag")}, + {"ON", BrowserFlag(false), errors.New("invalid value ‘ON’ for BrowserFlag")}, + {"OFF", BrowserFlag(false), errors.New("invalid value ‘OFF’ for BrowserFlag")}, + {"on", BrowserFlag(true), nil}, + {"off", BrowserFlag(false), nil}, + } + + for _, testCase := range testCases { + bf, err := ParseBrowserFlag(testCase.flagStr) + if testCase.expectedErr == nil { + if err != nil { + t.Fatalf("error: expected = , got = %v", err) + } + } else if err == nil { + t.Fatalf("error: expected = %v, got = ", testCase.expectedErr) + } else if testCase.expectedErr.Error() != err.Error() { + t.Fatalf("error: expected = %v, got = %v", testCase.expectedErr, err) + } + + if err == nil && testCase.expectedResult != bf { + t.Fatalf("result: expected: %v, got: %v", testCase.expectedResult, bf) + } + } +} diff --git a/cmd/config-migrate.go b/cmd/config-migrate.go index 869383ac5..9fbd5b3cd 100644 --- a/cmd/config-migrate.go +++ b/cmd/config-migrate.go @@ -886,7 +886,7 @@ func migrateV13ToV14() error { } // Set the new browser parameter to true by default - srvConfig.Browser = "on" + srvConfig.Browser = true if err = quick.Save(configFile, srvConfig); err != nil { return fmt.Errorf("Failed to migrate config from ‘%s’ to ‘%s’. %v", cv13.Version, srvConfig.Version, err) diff --git a/cmd/config-old.go b/cmd/config-old.go index 5eed563bf..41cb64440 100644 --- a/cmd/config-old.go +++ b/cmd/config-old.go @@ -386,9 +386,9 @@ type serverConfigV14 struct { Version string `json:"version"` // S3 API configuration. - Credential credential `json:"credential"` - Region string `json:"region"` - Browser string `json:"browser"` + Credential credential `json:"credential"` + Region string `json:"region"` + Browser BrowserFlag `json:"browser"` // Additional error logging configuration. Logger *loggerV7 `json:"logger"` @@ -403,9 +403,9 @@ type serverConfigV15 struct { Version string `json:"version"` // S3 API configuration. - Credential credential `json:"credential"` - Region string `json:"region"` - Browser string `json:"browser"` + Credential credential `json:"credential"` + Region string `json:"region"` + Browser BrowserFlag `json:"browser"` // Additional error logging configuration. Logger *loggerV7 `json:"logger"` diff --git a/cmd/config-v16.go b/cmd/config-v16.go index 2a68d7863..aee62356a 100644 --- a/cmd/config-v16.go +++ b/cmd/config-v16.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "io/ioutil" - "strings" "sync" "github.com/minio/minio/pkg/quick" @@ -30,7 +29,7 @@ import ( // Read Write mutex for safe access to ServerConfig. var serverConfigMu sync.RWMutex -var v16 = "16" +const v16 = "16" // serverConfigV16 server configuration version '16' which is like // version '15' except it removes log level field and renames `fileName` @@ -39,9 +38,9 @@ type serverConfigV16 struct { Version string `json:"version"` // S3 API configuration. - Credential credential `json:"credential"` - Region string `json:"region"` - Browser string `json:"browser"` + Credential credential `json:"credential"` + Region string `json:"region"` + Browser BrowserFlag `json:"browser"` // Additional error logging configuration. Logger *loggers `json:"logger"` @@ -52,13 +51,14 @@ type serverConfigV16 struct { func newServerConfigV16() *serverConfigV16 { srvCfg := &serverConfigV16{ - Version: v16, - Region: globalMinioDefaultRegion, - Logger: &loggers{}, - Notify: ¬ifier{}, + Version: v16, + Credential: mustGetNewCredential(), + Region: globalMinioDefaultRegion, + Browser: true, + Logger: &loggers{}, + Notify: ¬ifier{}, } - srvCfg.SetCredential(mustGetNewCredential()) - srvCfg.SetBrowser("on") + // Enable console logger by default on a fresh run. srvCfg.Logger.Console = NewConsoleLogger() @@ -117,10 +117,12 @@ func newConfig(envParams envParams) error { // loadConfig - loads a new config from disk, overrides params from env // if found and valid func loadConfig(envParams envParams) error { - configFile := getConfigFile() + srvCfg := &serverConfigV16{ + Region: globalMinioDefaultRegion, + Browser: true, + } - srvCfg := &serverConfigV16{} - if _, err := quick.Load(configFile, srvCfg); err != nil { + if _, err := quick.Load(getConfigFile(), srvCfg); err != nil { return err } if srvCfg.Version != v16 { @@ -136,7 +138,7 @@ func loadConfig(envParams envParams) error { srvCfg.SetBrowser(envParams.browser) } - if strings.ToLower(srvCfg.GetBrowser()) == "off" { + if !srvCfg.GetBrowser() { globalIsBrowserEnabled = false } @@ -201,9 +203,12 @@ func checkDupJSONKeys(json string) error { // validateConfig checks for func validateConfig() error { - configFile := getConfigFile() + srvCfg := &serverConfigV16{ + Region: globalMinioDefaultRegion, + Browser: true, + } - srvCfg := &serverConfigV16{} + configFile := getConfigFile() if _, err := quick.Load(configFile, srvCfg); err != nil { return err } @@ -227,11 +232,6 @@ func validateConfig() error { return errors.New("Region config value cannot be empty") } - // Validate browser field - if b := strings.ToLower(srvCfg.GetBrowser()); b != "on" && b != "off" { - return fmt.Errorf("Browser config value %s is invalid", b) - } - // Validate credential fields only when // they are not set via the environment if !globalIsEnvCreds { @@ -307,30 +307,20 @@ func (s serverConfigV16) GetCredential() credential { } // SetBrowser set if browser is enabled. -func (s *serverConfigV16) SetBrowser(v string) { +func (s *serverConfigV16) SetBrowser(b BrowserFlag) { serverConfigMu.Lock() defer serverConfigMu.Unlock() - // Set browser param - if v == "" { - v = "on" // Browser is on by default. - } - // Set the new value. - s.Browser = v + s.Browser = b } // GetCredentials get current credentials. -func (s serverConfigV16) GetBrowser() string { +func (s serverConfigV16) GetBrowser() BrowserFlag { serverConfigMu.RLock() defer serverConfigMu.RUnlock() - if s.Browser != "" { - return s.Browser - } // empty browser. - - // Empty browser means "on" by default. - return "on" + return s.Browser } // Save config. diff --git a/cmd/config-v16_test.go b/cmd/config-v16_test.go index 7b92d571a..a3c9612a2 100644 --- a/cmd/config-v16_test.go +++ b/cmd/config-v16_test.go @@ -159,7 +159,7 @@ func TestServerConfigWithEnvs(t *testing.T) { defer removeAll(rootPath) // Check if serverConfig has - if serverConfig.GetBrowser() != "off" { + if serverConfig.GetBrowser() { t.Errorf("Expecting browser `off` found %s", serverConfig.GetBrowser()) } diff --git a/cmd/server-main.go b/cmd/server-main.go index 04f3e8c7d..c60bf57fa 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -96,7 +96,7 @@ func checkUpdate(mode string) { // envParams holds all env parameters type envParams struct { creds credential - browser string + browser BrowserFlag } func migrate() { @@ -139,20 +139,21 @@ func initConfig() { globalIsEnvCreds = true } - browser := os.Getenv("MINIO_BROWSER") - if browser != "" { - if !(strings.EqualFold(browser, "off") || strings.EqualFold(browser, "on")) { - fatalIf(errors.New("invalid value"), "‘%s’ in MINIO_BROWSER environment variable.", browser) - } - - // browser Envs are set globally, this doesn't represent - // if browser is turned off or on. - globalIsEnvBrowser = true - } - envs := envParams{ creds: cred, - browser: browser, + browser: BrowserFlag(true), + } + + if browser := os.Getenv("MINIO_BROWSER"); browser != "" { + browserFlag, err := ParseBrowserFlag(browser) + if err != nil { + fatalIf(errors.New("invalid value"), "Unknown value ‘%s’ in MINIO_BROWSER environment variable.", browser) + } + + // browser Envs are set globally, this does not represent + // if browser is turned off or on. + globalIsEnvBrowser = true + envs.browser = browserFlag } // Config file does not exist, we create it fresh and return upon success.