Honor etcd legacy v/s new config settings properly (#8510)

This PR also fixes issues related to

- Add proper newline for `mc admin config get` output
  for more than one targets
- Fixes issue of temporary user credentials to have
  consistent output
- Fixes a crash when setting a key with empty values
- Fixes a parsing issue with `mc admin config history`
- Fixes gateway ENV handling for etcd server and gateway

cmd/config/config.go

@@ -405,6 +405,9 @@ func (c Config) SetKVS(s string, defaultKVS map[string]KVS) error {
 			kvs[prevK] = strings.Join([]string{kvs[prevK], sanitizeValue(kv[0])}, KvSpaceSeparator)
 			continue
 		}
+		if len(kv) == 1 {
+			return Error(fmt.Sprintf("key '%s', cannot have empty value", kv[0]))
+		}
 		prevK = kv[0]
 		kvs[kv[0]] = sanitizeValue(kv[1])
 	}

cmd/config/etcd/etcd.go

@@ -95,6 +95,39 @@ func parseEndpoints(endpoints string) ([]string, bool, error) {
 	return etcdEndpoints, etcdSecure, nil
 }
 
+func lookupLegacyConfig(rootCAs *x509.CertPool) (Config, error) {
+	cfg := Config{}
+	endpoints := env.Get(EnvEtcdEndpoints, "")
+	if endpoints == "" {
+		return cfg, nil
+	}
+	etcdEndpoints, etcdSecure, err := parseEndpoints(endpoints)
+	if err != nil {
+		return cfg, err
+	}
+	cfg.Enabled = true
+	cfg.DialTimeout = defaultDialTimeout
+	cfg.DialKeepAliveTime = defaultDialKeepAlive
+	cfg.Endpoints = etcdEndpoints
+	cfg.CoreDNSPath = "/skydns"
+	if etcdSecure {
+		cfg.TLS = &tls.Config{
+			RootCAs: rootCAs,
+		}
+		// This is only to support client side certificate authentication
+		// https://coreos.com/etcd/docs/latest/op-guide/security.html
+		etcdClientCertFile := env.Get(EnvEtcdClientCert, "")
+		etcdClientCertKey := env.Get(EnvEtcdClientCertKey, "")
+		if etcdClientCertFile != "" && etcdClientCertKey != "" {
+			cfg.TLS.GetClientCertificate = func(unused *tls.CertificateRequestInfo) (*tls.Certificate, error) {
+				cert, err := tls.LoadX509KeyPair(etcdClientCertFile, etcdClientCertKey)
+				return &cert, err
+			}
+		}
+	}
+	return cfg, nil
+}
+
 // LookupConfig - Initialize new etcd config.
 func LookupConfig(kv config.KVS, rootCAs *x509.CertPool) (Config, error) {
 	cfg := Config{}
@@ -102,20 +135,37 @@ func LookupConfig(kv config.KVS, rootCAs *x509.CertPool) (Config, error) {
 		return cfg, err
 	}
 
-	stateBool, err := config.ParseBool(env.Get(EnvEtcdState, kv.Get(config.State)))
+	stateBool, err := config.ParseBool(env.Get(EnvEtcdState, config.StateOn))
 	if err != nil {
 		return cfg, err
 	}
 
-	endpoints := env.Get(EnvEtcdEndpoints, kv.Get(Endpoints))
-	if stateBool && len(endpoints) == 0 {
-		return cfg, config.Error("'endpoints' key cannot be empty if you wish to enable etcd")
+	if stateBool {
+		// By default state is 'on' to honor legacy config.
+		cfg, err = lookupLegacyConfig(rootCAs)
+		if err != nil {
+			return cfg, err
+		}
+		// If old legacy config is enabled honor it.
+		if cfg.Enabled {
+			return cfg, nil
+		}
 	}
 
-	if len(endpoints) == 0 && !stateBool {
+	stateBool, err = config.ParseBool(env.Get(EnvEtcdState, kv.Get(config.State)))
+	if err != nil {
+		return cfg, err
+	}
+
+	if !stateBool {
 		return cfg, nil
 	}
 
+	endpoints := env.Get(EnvEtcdEndpoints, kv.Get(Endpoints))
+	if endpoints == "" {
+		return cfg, config.Error("'endpoints' key cannot be empty to enable etcd")
+	}
+
 	etcdEndpoints, etcdSecure, err := parseEndpoints(endpoints)
 	if err != nil {
 		return cfg, err

cmd/gateway-main.go

@@ -147,6 +147,25 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 	// Set when gateway is enabled
 	globalIsGateway = true
 
+	enableConfigOps := gatewayName == "nas"
+
+	// TODO: We need to move this code with globalConfigSys.Init()
+	// for now keep it here such that "s3" gateway layer initializes
+	// itself properly when KMS is set.
+
+	// Initialize server config.
+	srvCfg := newServerConfig()
+
+	// Override any values from ENVs.
+	if err := lookupConfigs(srvCfg); err != nil {
+		logger.FatalIf(err, "Unable to initialize server config")
+	}
+
+	// hold the mutex lock before a new config is assigned.
+	globalServerConfigMu.Lock()
+	globalServerConfig = srvCfg
+	globalServerConfigMu.Unlock()
+
 	router := mux.NewRouter().SkipClean(true)
 
 	if globalEtcdClient != nil {
@@ -156,8 +175,6 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 
 	// Initialize globalConsoleSys system
 	globalConsoleSys = NewConsoleLogger(context.Background(), globalEndpoints)
-
-	enableConfigOps := gatewayName == "nas"
 	enableIAMOps := globalEtcdClient != nil
 
 	// Enable IAM admin APIs if etcd is enabled, if not just enable basic
@@ -187,32 +204,14 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 		getCert = globalTLSCerts.GetCertificate
 	}
 
-	globalHTTPServer = xhttp.NewServer([]string{globalCLIContext.Addr}, criticalErrorHandler{registerHandlers(router, globalHandlers...)}, getCert)
+	globalHTTPServer = xhttp.NewServer([]string{globalCLIContext.Addr},
+		criticalErrorHandler{registerHandlers(router, globalHandlers...)}, getCert)
 	go func() {
 		globalHTTPServerErrorCh <- globalHTTPServer.Start()
 	}()
 
 	signal.Notify(globalOSSignalCh, os.Interrupt, syscall.SIGTERM)
 
-	if !enableConfigOps {
-		// TODO: We need to move this code with globalConfigSys.Init()
-		// for now keep it here such that "s3" gateway layer initializes
-		// itself properly when KMS is set.
-
-		// Initialize server config.
-		srvCfg := newServerConfig()
-
-		// Override any values from ENVs.
-		if err := lookupConfigs(srvCfg); err != nil {
-			logger.FatalIf(err, "Unable to initialize server config")
-		}
-
-		// hold the mutex lock before a new config is assigned.
-		globalServerConfigMu.Lock()
-		globalServerConfig = srvCfg
-		globalServerConfigMu.Unlock()
-	}
-
 	newObject, err := gw.NewGatewayLayer(globalActiveCred)
 	if err != nil {
 		// Stop watching for any certificate changes.
@@ -252,7 +251,6 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 	// ****  WARNING ****
 	// Migrating to encrypted backend should happen before initialization of any
 	// sub-systems, make sure that we do not move the above codeblock elsewhere.
-
 	if enableConfigOps {
 		logger.FatalIf(globalConfigSys.Init(newObject), "Unable to initialize config system")
 

cmd/iam.go

@@ -578,7 +578,12 @@ func (sys *IAMSys) ListUsers() (map[string]madmin.UserInfo, error) {
 	for k, v := range sys.iamUsersMap {
 		users[k] = madmin.UserInfo{
 			PolicyName: sys.iamUserPolicyMap[k].Policy,
-			Status:     madmin.AccountStatus(v.Status),
+			Status: func() madmin.AccountStatus {
+				if v.IsValid() {
+					return madmin.AccountEnabled
+				}
+				return madmin.AccountDisabled
+			}(),
 		}
 	}
 
@@ -609,8 +614,13 @@ func (sys *IAMSys) GetUserInfo(name string) (u madmin.UserInfo, err error) {
 
 	u = madmin.UserInfo{
 		PolicyName: sys.iamUserPolicyMap[name].Policy,
-		Status:     madmin.AccountStatus(creds.Status),
-		MemberOf:   sys.iamUserGroupMemberships[name].ToSlice(),
+		Status: func() madmin.AccountStatus {
+			if creds.IsValid() {
+				return madmin.AccountEnabled
+			}
+			return madmin.AccountDisabled
+		}(),
+		MemberOf: sys.iamUserGroupMemberships[name].ToSlice(),
 	}
 	return u, nil
 }

cmd/server-main.go

@@ -235,15 +235,6 @@ func initSafeModeInit(buckets []BucketInfo) (err error) {
 		return err
 	}
 
-	if globalEtcdClient != nil {
-		// ****  WARNING ****
-		// Migrating to encrypted backend on etcd should happen before initialization of
-		// IAM sub-systems, make sure that we do not move the above codeblock elsewhere.
-		if err = migrateIAMConfigsEtcdToEncrypted(globalEtcdClient); err != nil {
-			return fmt.Errorf("Unable to handle encrypted backend for iam and policies: %v", err)
-		}
-	}
-
 	return nil
 }
 
@@ -257,6 +248,15 @@ func initAllSubsystems(buckets []BucketInfo, newObject ObjectLayer) (err error)
 		return fmt.Errorf("Unable to initialize notification target(s) from config: %w", err)
 	}
 
+	if globalEtcdClient != nil {
+		// ****  WARNING ****
+		// Migrating to encrypted backend on etcd should happen before initialization of
+		// IAM sub-systems, make sure that we do not move the above codeblock elsewhere.
+		if err = migrateIAMConfigsEtcdToEncrypted(globalEtcdClient); err != nil {
+			return fmt.Errorf("Unable to handle encrypted backend for iam and policies: %v", err)
+		}
+	}
+
 	if err = globalIAMSys.Init(newObject); err != nil {
 		return fmt.Errorf("Unable to initialize IAM system: %w", err)
 	}

pkg/madmin/parse-kv.go

@@ -37,10 +37,23 @@ const (
 	commentKey = "comment"
 )
 
+// Count - returns total numbers of target
+func (t Targets) Count() int {
+	var count int
+	for _, targetKV := range t {
+		for range targetKV {
+			count++
+		}
+	}
+	return count
+}
+
 func (t Targets) String() string {
 	var s strings.Builder
+	count := t.Count()
 	for subSys, targetKV := range t {
 		for target, kv := range targetKV {
+			count--
 			c := kv[commentKey]
 			data, err := base64.RawStdEncoding.DecodeString(c)
 			if err == nil {
@@ -73,7 +86,7 @@ func (t Targets) String() string {
 				s.WriteString(KvDoubleQuote)
 				s.WriteString(KvSpaceSeparator)
 			}
-			if len(t) > 1 {
+			if (len(t) > 1 || len(targetKV) > 1) && count > 0 {
 				s.WriteString(KvNewline)
 				s.WriteString(KvNewline)
 			}
@@ -121,7 +134,7 @@ func convertTargets(s string, targets Targets) error {
 			kvs[prevK] = strings.Join([]string{kvs[prevK], sanitizeValue(kv[0])}, KvSpaceSeparator)
 			continue
 		}
-		if len(kv[1]) == 0 {
+		if len(kv) == 1 {
 			return fmt.Errorf("value for key '%s' cannot be empty", kv[0])
 		}
 		prevK = kv[0]