Fix console logger crash in gateway mode (#8525)

This PR also fixes config migration only
for credentials and region which are valid
and set.

Also fix implicit `state="on"` behavior

cmd/config/config.go

@@ -19,6 +19,7 @@ package config
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/minio/minio-go/pkg/set"
@@ -233,6 +234,8 @@ func LookupCreds(kv KVS) (auth.Credentials, error) {
 	return auth.CreateCredentials(accessKey, secretKey)
 }
 
+var validRegionRegex = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9-_-]+$")
+
 // LookupRegion - get current region.
 func LookupRegion(kv KVS) (string, error) {
 	if err := CheckValidKeys(RegionSubSys, kv, DefaultRegionKVS); err != nil {
@@ -242,7 +245,14 @@ func LookupRegion(kv KVS) (string, error) {
 	if region == "" {
 		region = env.Get(EnvRegionName, kv.Get(RegionName))
 	}
-	return region, nil
+	if region != "" {
+		if validRegionRegex.MatchString(region) {
+			return region, nil
+		}
+		return "", Errorf("region '%s' is invalid, expected simple characters such as [us-east-1, myregion...]",
+			region)
+	}
+	return "", nil
 }
 
 // CheckValidKeys - checks if inputs KVS has the necessary keys,
@@ -299,9 +309,8 @@ func (c Config) GetKVS(s string) (map[string]KVS, error) {
 	}
 	found := SubSystems.Contains(subSystemValue[0])
 	if !found {
-		// Check for sub-prefix only if the input value
+		// Check for sub-prefix only if the input value is only a
-		// is only a single value, this rejects invalid
+		// single value, this rejects invalid inputs if any.
-		// inputs if any.
 		found = !SubSystems.FuncMatch(strings.HasPrefix, subSystemValue[0]).IsEmpty() && len(subSystemValue) == 1
 	}
 	if !found {
@@ -431,17 +440,20 @@ func (c Config) SetKVS(s string) error {
 	if len(subSystemValue) == 2 {
 		tgt = subSystemValue[1]
 	}
+
 	_, ok := c[subSystemValue[0]][tgt]
 	if !ok {
 		c[subSystemValue[0]][tgt] = KVS{}
 	}
 
 	for k, v := range kvs {
-		if len(subSystemValue) == 2 {
+		c[subSystemValue[0]][tgt][k] = v
-			c[subSystemValue[0]][subSystemValue[1]][k] = v
+	}
-		} else {
+
-			c[subSystemValue[0]][Default][k] = v
+	_, ok = c[subSystemValue[0]][tgt][State]
-		}
+	if !ok {
+		// implicit state "on" if not specified.
+		c[subSystemValue[0]][tgt][State] = StateOn
 	}
 
 	return nil

cmd/config/config_test.go

@@ -0,0 +1,44 @@
+/*
+ * MinIO Cloud Storage, (C) 2019 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 config
+
+import "testing"
+
+func TestValidRegion(t *testing.T) {
+	tests := []struct {
+		name    string
+		success bool
+	}{
+		{name: "us-east-1", success: true},
+		{name: "us_east", success: true},
+		{name: "helloWorld", success: true},
+		{name: "-fdslka", success: false},
+		{name: "^00[", success: false},
+		{name: "my region", success: false},
+		{name: "%%$#!", success: false},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ok := validRegionRegex.MatchString(test.name)
+			if test.success != ok {
+				t.Errorf("Expected %t, got %t", test.success, ok)
+			}
+		})
+	}
+}

cmd/config/legacy.go

@@ -23,6 +23,13 @@ import "github.com/minio/minio/pkg/auth"
 
 // SetCredentials - One time migration code needed, for migrating from older config to new for server credentials.
 func SetCredentials(c Config, cred auth.Credentials) {
+	creds, err := auth.CreateCredentials(cred.AccessKey, cred.SecretKey)
+	if err != nil {
+		return
+	}
+	if !creds.IsValid() {
+		return
+	}
 	c[CredentialsSubSys][Default] = KVS{
 		State:     StateOn,
 		Comment:   "Settings for credentials, after migrating config",
@@ -33,6 +40,9 @@ func SetCredentials(c Config, cred auth.Credentials) {
 
 // SetRegion - One time migration code needed, for migrating from older config to new for server Region.
 func SetRegion(c Config, name string) {
+	if name == "" {
+		return
+	}
 	c[RegionSubSys][Default] = KVS{
 		RegionName: name,
 		State:      StateOn,

cmd/gateway-main.go

@@ -145,6 +145,9 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 	// Set when gateway is enabled
 	globalIsGateway = true
 
+	// Initialize globalConsoleSys system
+	globalConsoleSys = NewConsoleLogger(context.Background(), globalEndpoints)
+
 	enableConfigOps := gatewayName == "nas"
 
 	// TODO: We need to move this code with globalConfigSys.Init()
@@ -171,8 +174,6 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 		registerSTSRouter(router)
 	}
 
-	// Initialize globalConsoleSys system
-	globalConsoleSys = NewConsoleLogger(context.Background(), globalEndpoints)
 	enableIAMOps := globalEtcdClient != nil
 
 	// Enable IAM admin APIs if etcd is enabled, if not just enable basic

pkg/madmin/config-kv-commands.go

@@ -55,16 +55,6 @@ func (adm *AdminClient) SetConfigKV(kv string) (err error) {
 	if err != nil {
 		return err
 	}
-	for subSys, targetKV := range targets {
-		for target := range targetKV {
-			_, ok := targets[subSys][target][stateKey]
-			if !ok {
-				// If client asked for state preserve.
-				// otherwise implicitly add state to "on"
-				targets[subSys][target][stateKey] = stateOn
-			}
-		}
-	}
 	econfigBytes, err := EncryptData(adm.secretAccessKey, []byte(targets.String()))
 	if err != nil {
 		return err