diff --git a/cmd/config/cache/config.go b/cmd/config/cache/config.go index 0f1d56464..8d78145be 100644 --- a/cmd/config/cache/config.go +++ b/cmd/config/cache/config.go @@ -61,22 +61,26 @@ func (cfg *Config) UnmarshalJSON(data []byte) (err error) { return errors.New("config quota value should not be null or negative") } - if _, err = parseCacheDrives(_cfg.Drives); err != nil { - return err - } - if _, err = parseCacheExcludes(_cfg.Exclude); err != nil { - return err - } return nil } // Parses given cacheDrivesEnv and returns a list of cache drives. -func parseCacheDrives(drives []string) ([]string, error) { +func parseCacheDrives(drives string) ([]string, error) { + var drivesSlice []string if len(drives) == 0 { - return drives, nil + return drivesSlice, nil } + + drivesSlice = strings.Split(drives, cacheDelimiterLegacy) + if len(drivesSlice) == 1 && drivesSlice[0] == drives { + drivesSlice = strings.Split(drives, cacheDelimiter) + } + var endpoints []string - for _, d := range drives { + for _, d := range drivesSlice { + if len(d) == 0 { + return nil, config.ErrInvalidCacheDrivesValue(nil).Msg("cache dir cannot be an empty path") + } if ellipses.HasEllipses(d) { s, err := parseCacheDrivePaths(d) if err != nil { @@ -89,9 +93,6 @@ func parseCacheDrives(drives []string) ([]string, error) { } for _, d := range endpoints { - if len(d) == 0 { - return nil, config.ErrInvalidCacheDrivesValue(nil).Msg("cache dir cannot be an empty path") - } if !filepath.IsAbs(d) { return nil, config.ErrInvalidCacheDrivesValue(nil).Msg("cache dir should be absolute path: %s", d) } @@ -114,8 +115,18 @@ func parseCacheDrivePaths(arg string) (ep []string, err error) { } // Parses given cacheExcludesEnv and returns a list of cache exclude patterns. -func parseCacheExcludes(excludes []string) ([]string, error) { - for _, e := range excludes { +func parseCacheExcludes(excludes string) ([]string, error) { + var excludesSlice []string + if len(excludes) == 0 { + return excludesSlice, nil + } + + excludesSlice = strings.Split(excludes, cacheDelimiterLegacy) + if len(excludesSlice) == 1 && excludesSlice[0] == excludes { + excludesSlice = strings.Split(excludes, cacheDelimiter) + } + + for _, e := range excludesSlice { if len(e) == 0 { return nil, config.ErrInvalidCacheExcludesValue(nil).Msg("cache exclude path (%s) cannot be empty", e) } @@ -123,5 +134,6 @@ func parseCacheExcludes(excludes []string) ([]string, error) { return nil, config.ErrInvalidCacheExcludesValue(nil).Msg("cache exclude pattern (%s) cannot start with / as prefix", e) } } - return excludes, nil + + return excludesSlice, nil } diff --git a/cmd/config/cache/config_test.go b/cmd/config/cache/config_test.go index 73e5152b8..b952def8b 100644 --- a/cmd/config/cache/config_test.go +++ b/cmd/config/cache/config_test.go @@ -19,7 +19,6 @@ package cache import ( "reflect" "runtime" - "strings" "testing" ) @@ -61,6 +60,11 @@ func TestParseCacheDrives(t *testing.T) { expectedPatterns []string success bool }{"/home/drive1;/home/drive2;/home/drive3", []string{"/home/drive1", "/home/drive2", "/home/drive3"}, true}) + testCases = append(testCases, struct { + driveStr string + expectedPatterns []string + success bool + }{"/home/drive1,/home/drive2,/home/drive3", []string{"/home/drive1", "/home/drive2", "/home/drive3"}, true}) testCases = append(testCases, struct { driveStr string expectedPatterns []string @@ -73,7 +77,7 @@ func TestParseCacheDrives(t *testing.T) { }{"/home/drive{1..3}", []string{}, false}) } for i, testCase := range testCases { - drives, err := parseCacheDrives(strings.Split(testCase.driveStr, cacheDelimiterLegacy)) + drives, err := parseCacheDrives(testCase.driveStr) if err != nil && testCase.success { t.Errorf("Test %d: Expected success but failed instead %s", i+1, err) } @@ -102,11 +106,12 @@ func TestParseCacheExclude(t *testing.T) { // valid input {"bucket1/*;*.png;images/trip/barcelona/*", []string{"bucket1/*", "*.png", "images/trip/barcelona/*"}, true}, + {"bucket1/*,*.png,images/trip/barcelona/*", []string{"bucket1/*", "*.png", "images/trip/barcelona/*"}, true}, {"bucket1", []string{"bucket1"}, true}, } for i, testCase := range testCases { - excludes, err := parseCacheExcludes(strings.Split(testCase.excludeStr, cacheDelimiterLegacy)) + excludes, err := parseCacheExcludes(testCase.excludeStr) if err != nil && testCase.success { t.Errorf("Test %d: Expected success but failed instead %s", i+1, err) } diff --git a/cmd/config/cache/lookup.go b/cmd/config/cache/lookup.go index 8baac04b1..2d95e0619 100644 --- a/cmd/config/cache/lookup.go +++ b/cmd/config/cache/lookup.go @@ -19,7 +19,6 @@ package cache import ( "errors" "strconv" - "strings" "github.com/minio/minio/cmd/config" "github.com/minio/minio/pkg/env" @@ -84,42 +83,43 @@ func LookupConfig(kvs config.KVS) (Config, error) { return cfg, err } - // Check if cache is explicitly disabled - stateBool, err := config.ParseBool(env.Get(EnvCacheState, kvs.Get(config.State))) - if err != nil { - // Parsing failures happen due to empty KVS, ignore it. - if kvs.Empty() { - return cfg, nil - } - return cfg, err - } - drives := env.Get(EnvCacheDrives, kvs.Get(Drives)) - if stateBool { - if len(drives) == 0 { - return cfg, config.Error("'drives' key cannot be empty if you wish to enable caching") - } - } - if len(drives) == 0 { - return cfg, nil - } - - cfg.Drives, err = parseCacheDrives(strings.Split(drives, cacheDelimiter)) - if err != nil { - cfg.Drives, err = parseCacheDrives(strings.Split(drives, cacheDelimiterLegacy)) + if len(drives) > 0 { + // Drives is not-empty means user wishes to enable this explicitly, but + // check if ENV is set to false to disable caching. + stateBool, err := config.ParseBool(env.Get(EnvCacheState, config.StateOn)) if err != nil { return cfg, err } + if !stateBool { + return cfg, nil + } + } else { + // Check if cache is explicitly disabled + stateBool, err := config.ParseBool(env.Get(EnvCacheState, kvs.Get(config.State))) + if err != nil { + if kvs.Empty() { + return cfg, nil + } + return cfg, err + } + if stateBool { + return cfg, config.Error("'drives' key cannot be empty to enable caching") + } + return cfg, nil + } + + var err error + cfg.Drives, err = parseCacheDrives(drives) + if err != nil { + return cfg, err } cfg.Enabled = true if excludes := env.Get(EnvCacheExclude, kvs.Get(Exclude)); excludes != "" { - cfg.Exclude, err = parseCacheExcludes(strings.Split(excludes, cacheDelimiter)) + cfg.Exclude, err = parseCacheExcludes(excludes) if err != nil { - cfg.Exclude, err = parseCacheExcludes(strings.Split(excludes, cacheDelimiterLegacy)) - if err != nil { - return cfg, err - } + return cfg, err } } diff --git a/cmd/config/config.go b/cmd/config/config.go index 90f407a0e..0ad63d462 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -46,12 +46,12 @@ func (e Error) Error() string { // Default keys const ( Default = madmin.Default - State = "state" - Comment = "comment" + State = madmin.StateKey + Comment = madmin.CommentKey // State values - StateOn = "on" - StateOff = "off" + StateOn = madmin.StateOn + StateOff = madmin.StateOff RegionName = "name" AccessKey = "access_key" @@ -137,7 +137,7 @@ const ( SubSystemSeparator = madmin.SubSystemSeparator KvSeparator = madmin.KvSeparator KvSpaceSeparator = madmin.KvSpaceSeparator - KvComment = `#` + KvComment = madmin.KvComment KvNewline = madmin.KvNewline KvDoubleQuote = madmin.KvDoubleQuote KvSingleQuote = madmin.KvSingleQuote @@ -232,9 +232,11 @@ type Config map[string]map[string]KVS func (c Config) String() string { var s strings.Builder - for k, v := range c { + hkvs := HelpSubSysMap[""] + for _, hkv := range hkvs { + v := c[hkv.Key] for target, kv := range v { - s.WriteString(k) + s.WriteString(hkv.Key) if target != Default { s.WriteString(SubSystemSeparator) s.WriteString(target) @@ -252,10 +254,11 @@ func (c Config) DelFrom(r io.Reader) error { scanner := bufio.NewScanner(r) for scanner.Scan() { // Skip any empty lines, or comment like characters - if scanner.Text() == "" || strings.HasPrefix(scanner.Text(), KvComment) { + text := scanner.Text() + if text == "" || strings.HasPrefix(text, KvComment) { continue } - if err := c.DelKVS(scanner.Text()); err != nil { + if err := c.DelKVS(text); err != nil { return err } } @@ -271,13 +274,14 @@ func (c Config) ReadFrom(r io.Reader) (int64, error) { scanner := bufio.NewScanner(r) for scanner.Scan() { // Skip any empty lines, or comment like characters - if scanner.Text() == "" || strings.HasPrefix(scanner.Text(), KvComment) { + text := scanner.Text() + if text == "" || strings.HasPrefix(text, KvComment) { continue } - if err := c.SetKVS(scanner.Text(), DefaultKVS); err != nil { + if err := c.SetKVS(text, DefaultKVS); err != nil { return 0, err } - n += len(scanner.Text()) + n += len(text) } if err := scanner.Err(); err != nil { return 0, err @@ -411,6 +415,7 @@ func New() Config { srvCfg := make(Config) for _, k := range SubSystems.ToSlice() { srvCfg[k] = map[string]KVS{} + srvCfg[k][Default] = DefaultKVS[k] } return srvCfg } @@ -502,12 +507,6 @@ func (c Config) DelKVS(s string) error { return nil } -// This function is needed, to trim off single or double quotes, creeping into the values. -func sanitizeValue(v string) string { - v = strings.TrimSuffix(strings.TrimPrefix(strings.TrimSpace(v), KvDoubleQuote), KvDoubleQuote) - return strings.TrimSuffix(strings.TrimPrefix(v, KvSingleQuote), KvSingleQuote) -} - // Clone - clones a config map entirely. func (c Config) Clone() Config { cp := New() @@ -551,8 +550,11 @@ func (c Config) SetKVS(s string, defaultKVS map[string]KVS) error { } if len(kv) == 1 && prevK != "" { kvs = append(kvs, KV{ - Key: prevK, - Value: strings.Join([]string{kvs.Get(prevK), sanitizeValue(kv[0])}, KvSpaceSeparator), + Key: prevK, + Value: strings.Join([]string{ + kvs.Get(prevK), + madmin.SanitizeValue(kv[0]), + }, KvSpaceSeparator), }) continue } @@ -562,7 +564,7 @@ func (c Config) SetKVS(s string, defaultKVS map[string]KVS) error { prevK = kv[0] kvs = append(kvs, KV{ Key: kv[0], - Value: sanitizeValue(kv[1]), + Value: madmin.SanitizeValue(kv[1]), }) } @@ -574,7 +576,6 @@ func (c Config) SetKVS(s string, defaultKVS map[string]KVS) error { // Save client sent kvs c[subSys][tgt] = kvs - _, ok := c[subSys][tgt].Lookup(State) if !ok { // implicit state "on" if not specified. diff --git a/cmd/config/storageclass/help.go b/cmd/config/storageclass/help.go index d12d71c0e..f4e2e90ba 100644 --- a/cmd/config/storageclass/help.go +++ b/cmd/config/storageclass/help.go @@ -22,14 +22,14 @@ import "github.com/minio/minio/cmd/config" var ( Help = config.HelpKVS{ config.HelpKV{ - Key: ClassRRS, - Description: `Set reduced redundancy storage class parity ratio. eg: "EC:2"`, + Key: ClassStandard, + Description: `Set standard storage class parity ratio. eg: "EC:4"`, Optional: true, Type: "string", }, config.HelpKV{ - Key: ClassStandard, - Description: `Set standard storage class parity ratio. eg: "EC:4"`, + Key: ClassRRS, + Description: `Set reduced redundancy storage class parity ratio. eg: "EC:2"`, Optional: true, Type: "string", }, diff --git a/pkg/madmin/config-kv-commands.go b/pkg/madmin/config-kv-commands.go index 330fac6fc..54e9e344b 100644 --- a/pkg/madmin/config-kv-commands.go +++ b/pkg/madmin/config-kv-commands.go @@ -51,11 +51,7 @@ func (adm *AdminClient) DelConfigKV(k string) (err error) { // SetConfigKV - set key value config to server. func (adm *AdminClient) SetConfigKV(kv string) (err error) { - targets, err := ParseSubSysTarget([]byte(kv)) - if err != nil { - return err - } - econfigBytes, err := EncryptData(adm.secretAccessKey, []byte(targets.String())) + econfigBytes, err := EncryptData(adm.secretAccessKey, []byte(kv)) if err != nil { return err } diff --git a/pkg/madmin/parse-kv.go b/pkg/madmin/parse-kv.go index 679e29225..4f226150b 100644 --- a/pkg/madmin/parse-kv.go +++ b/pkg/madmin/parse-kv.go @@ -59,22 +59,33 @@ func (kvs KVS) Lookup(key string) (string, bool) { return "", false } -// Targets sub-system targets -type Targets map[string]map[string]KVS +// Target signifies an individual target +type Target struct { + SubSystem string `json:"subSys"` + KVS KVS `json:"kvs"` +} +// Targets sub-system targets +type Targets []Target + +// Standard config keys and values. const ( - stateKey = "state" - commentKey = "comment" + StateKey = "state" + CommentKey = "comment" + + // State values + StateOn = "on" + StateOff = "off" ) func (kvs KVS) String() string { var s strings.Builder for _, kv := range kvs { - // Do not need to print state - if kv.Key == stateKey { + // Do not need to print state which is on. + if kv.Key == StateKey && kv.Value == StateOn { continue } - if kv.Key == commentKey && kv.Value == "" { + if kv.Key == CommentKey && kv.Value == "" { continue } s.WriteString(kv.Key) @@ -94,13 +105,7 @@ func (kvs KVS) String() string { // Count - returns total numbers of target func (t Targets) Count() int { - var count int - for _, targetKV := range t { - for range targetKV { - count++ - } - } - return count + return len(t) } func hasSpace(s string) bool { @@ -115,19 +120,15 @@ func hasSpace(s string) bool { func (t Targets) String() string { var s strings.Builder count := t.Count() - for subSys, targetKV := range t { - for target, kv := range targetKV { - count-- - s.WriteString(subSys) - if target != Default { - s.WriteString(SubSystemSeparator) - s.WriteString(target) - } - s.WriteString(KvSpaceSeparator) - s.WriteString(kv.String()) - if (len(t) > 1 || len(targetKV) > 1) && count > 0 { - s.WriteString(KvNewline) - } + // Print all "on" states entries + for _, targetKV := range t { + kv := targetKV.KVS + count-- + s.WriteString(targetKV.SubSystem) + s.WriteString(KvSpaceSeparator) + s.WriteString(kv.String()) + if len(t) > 1 && count > 0 { + s.WriteString(KvNewline) } } return s.String() @@ -137,6 +138,7 @@ func (t Targets) String() string { const ( SubSystemSeparator = `:` KvSeparator = `=` + KvComment = `#` KvSpaceSeparator = ` ` KvNewline = "\n" KvDoubleQuote = `"` @@ -145,13 +147,14 @@ const ( Default = `_` ) -// This function is needed, to trim off single or double quotes, creeping into the values. -func sanitizeValue(v string) string { +// SanitizeValue - this function is needed, to trim off single or double quotes, creeping into the values. +func SanitizeValue(v string) string { v = strings.TrimSuffix(strings.TrimPrefix(strings.TrimSpace(v), KvDoubleQuote), KvDoubleQuote) return strings.TrimSuffix(strings.TrimPrefix(v, KvSingleQuote), KvSingleQuote) } -func convertTargets(s string, targets Targets) error { +// AddTarget - adds new targets, by parsing the input string s. +func (t *Targets) AddTarget(s string) error { inputs := strings.SplitN(s, KvSpaceSeparator, 2) if len(inputs) <= 1 { return fmt.Errorf("invalid number of arguments '%s'", s) @@ -170,7 +173,7 @@ func convertTargets(s string, targets Targets) error { if len(kv) == 1 && prevK != "" { kvs = append(kvs, KV{ Key: prevK, - Value: strings.Join([]string{kvs.Get(prevK), sanitizeValue(kv[0])}, KvSpaceSeparator), + Value: strings.Join([]string{kvs.Get(prevK), SanitizeValue(kv[0])}, KvSpaceSeparator), }) continue } @@ -180,28 +183,24 @@ func convertTargets(s string, targets Targets) error { prevK = kv[0] kvs = append(kvs, KV{ Key: kv[0], - Value: sanitizeValue(kv[1]), + Value: SanitizeValue(kv[1]), }) } - _, ok := targets[subSystemValue[0]] - if !ok { - targets[subSystemValue[0]] = map[string]KVS{} - } - if len(subSystemValue) == 2 { - targets[subSystemValue[0]][subSystemValue[1]] = kvs - } else { - targets[subSystemValue[0]][Default] = kvs - } + *t = append(*t, Target{ + SubSystem: inputs[0], + KVS: kvs, + }) + return nil } // ParseSubSysTarget - parse sub-system target func ParseSubSysTarget(buf []byte) (Targets, error) { - targets := make(map[string]map[string]KVS) + var targets Targets bio := bufio.NewScanner(bytes.NewReader(buf)) for bio.Scan() { - if err := convertTargets(bio.Text(), targets); err != nil { + if err := targets.AddTarget(bio.Text()); err != nil { return nil, err } }