fix: config to support keys with special values (#9304)

This PR adds context-based `k=v` splits based
on the sub-system which was obtained, if the
keys are not provided an error will be thrown
during parsing, if keys are provided with wrong
values an error will be thrown. Keys can now
have values which are of a much more complex
form such as `k="v=v"` or `k=" v = v"`
and other variations.

additionally, deprecate unnecessary postgres/mysql
configuration styles, support only

- connection_string for Postgres
- dsn_string for MySQL

All other parameters are removed.

cmd/config/config.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"io"
 	"regexp"
+	"sort"
 	"strings"
 
 	"github.com/minio/minio-go/v6/pkg/set"
@@ -196,6 +197,15 @@ func (kvs KVS) Empty() bool {
 	return len(kvs) == 0
 }
 
+// Keys returns the list of keys for the current KVS
+func (kvs KVS) Keys() []string {
+	var keys = make([]string, len(kvs))
+	for i := range kvs {
+		keys[i] = kvs[i].Key
+	}
+	return keys
+}
+
 func (kvs KVS) String() string {
 	var s strings.Builder
 	for _, kv := range kvs {
@@ -559,6 +569,33 @@ func (c Config) Clone() Config {
 	return cp
 }
 
+// Converts an input string of form "k1=v1 k2=v2" into fields
+// of ["k1=v1", "k2=v2"], the tokenization of each `k=v`
+// happens with the right number of input keys, if keys
+// input is empty returned value is empty slice as well.
+func kvFields(input string, keys []string) []string {
+	var valueIndexes []int
+	for _, key := range keys {
+		i := strings.Index(input, key+KvSeparator)
+		if i == -1 {
+			continue
+		}
+		valueIndexes = append(valueIndexes, i)
+	}
+
+	sort.Ints(valueIndexes)
+	var fields = make([]string, len(valueIndexes))
+	for i := range valueIndexes {
+		j := i + 1
+		if j < len(valueIndexes) {
+			fields[i] = strings.TrimSpace(input[valueIndexes[i]:valueIndexes[j]])
+		} else {
+			fields[i] = strings.TrimSpace(input[valueIndexes[i]:])
+		}
+	}
+	return fields
+}
+
 // SetKVS - set specific key values per sub-system.
 func (c Config) SetKVS(s string, defaultKVS map[string]KVS) error {
 	if len(s) == 0 {
@@ -581,9 +618,20 @@ func (c Config) SetKVS(s string, defaultKVS map[string]KVS) error {
 		return Errorf("sub-system '%s' only supports single target", subSystemValue[0])
 	}
 
+	tgt := Default
+	subSys := subSystemValue[0]
+	if len(subSystemValue) == 2 {
+		tgt = subSystemValue[1]
+	}
+
+	fields := kvFields(inputs[1], defaultKVS[subSys].Keys())
+	if len(fields) == 0 {
+		return Errorf("sub-system '%s' cannot have empty keys", subSys)
+	}
+
 	var kvs = KVS{}
 	var prevK string
-	for _, v := range strings.Fields(inputs[1]) {
+	for _, v := range fields {
 		kv := strings.SplitN(v, KvSeparator, 2)
 		if len(kv) == 0 {
 			continue
@@ -604,12 +652,6 @@ func (c Config) SetKVS(s string, defaultKVS map[string]KVS) error {
 		return Errorf("key '%s', cannot have empty value", kv[0])
 	}
 
-	tgt := Default
-	subSys := subSystemValue[0]
-	if len(subSystemValue) == 2 {
-		tgt = subSystemValue[1]
-	}
-
 	_, ok := kvs.Lookup(Enable)
 	// Check if state is required
 	_, enableRequired := defaultKVS[subSys].Lookup(Enable)

cmd/config/config_test.go

@@ -17,7 +17,94 @@
 
 package config
 
-import "testing"
+import (
+	"testing"
+)
+
+func TestKVFields(t *testing.T) {
+	tests := []struct {
+		input          string
+		keys           []string
+		expectedFields map[string]struct{}
+	}{
+		// No keys present
+		{
+			input:          "",
+			keys:           []string{"comment"},
+			expectedFields: map[string]struct{}{},
+		},
+		// No keys requested for tokenizing
+		{
+			input:          `comment="Hi this is my comment ="`,
+			keys:           []string{},
+			expectedFields: map[string]struct{}{},
+		},
+		// Single key requested and present
+		{
+			input:          `comment="Hi this is my comment ="`,
+			keys:           []string{"comment"},
+			expectedFields: map[string]struct{}{`comment="Hi this is my comment ="`: {}},
+		},
+		// Keys and input order of k=v is same.
+		{
+			input: `connection_string="host=localhost port=2832" comment="really long comment"`,
+			keys:  []string{"connection_string", "comment"},
+			expectedFields: map[string]struct{}{
+				`connection_string="host=localhost port=2832"`: {},
+				`comment="really long comment"`:                {},
+			},
+		},
+		// Keys with spaces in between
+		{
+			input: `enable=on format=namespace connection_string=" host=localhost port=5432 dbname = cesnietor sslmode=disable" table=holicrayoli`,
+			keys:  []string{"enable", "connection_string", "comment", "format", "table"},
+			expectedFields: map[string]struct{}{
+				`enable=on`:        {},
+				`format=namespace`: {},
+				`connection_string=" host=localhost port=5432 dbname = cesnietor sslmode=disable"`: {},
+				`table=holicrayoli`: {},
+			},
+		},
+		// One of the keys is not present and order of input has changed.
+		{
+			input: `comment="really long comment" connection_string="host=localhost port=2832"`,
+			keys:  []string{"connection_string", "comment", "format"},
+			expectedFields: map[string]struct{}{
+				`connection_string="host=localhost port=2832"`: {},
+				`comment="really long comment"`:                {},
+			},
+		},
+		// Incorrect delimiter, expected fields should be empty.
+		{
+			input:          `comment:"really long comment" connection_string:"host=localhost port=2832"`,
+			keys:           []string{"connection_string", "comment"},
+			expectedFields: map[string]struct{}{},
+		},
+		// Incorrect type of input v/s required keys.
+		{
+			input:          `comme="really long comment" connection_str="host=localhost port=2832"`,
+			keys:           []string{"connection_string", "comment"},
+			expectedFields: map[string]struct{}{},
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run("", func(t *testing.T) {
+			gotFields := kvFields(test.input, test.keys)
+			if len(gotFields) != len(test.expectedFields) {
+				t.Errorf("Expected keys %d, found %d", len(test.expectedFields), len(gotFields))
+			}
+			found := true
+			for _, field := range gotFields {
+				_, ok := test.expectedFields[field]
+				found = found && ok
+			}
+			if !found {
+				t.Errorf("Expected %s, got %s", test.expectedFields, gotFields)
+			}
+		})
+	}
+}
 
 func TestValidRegion(t *testing.T) {
 	tests := []struct {

cmd/config/notify/help.go

@@ -297,7 +297,7 @@ var (
 	HelpPostgres = config.HelpKVS{
 		config.HelpKV{
 			Key:         target.PostgresConnectionString,
-			Description: "Postgres server connection-string",
+			Description: `Postgres server connection-string e.g. "host=localhost port=5432 dbname=minio_events user=postgres password=password sslmode=disable"`,
 			Type:        "string",
 		},
 		config.HelpKV{
@@ -310,36 +310,6 @@ var (
 			Description: formatComment,
 			Type:        "namespace*|access",
 		},
-		config.HelpKV{
-			Key:         target.PostgresHost,
-			Description: "Postgres server hostname (used only if `connection_string` is empty)",
-			Optional:    true,
-			Type:        "hostname",
-		},
-		config.HelpKV{
-			Key:         target.PostgresPort,
-			Description: "Postgres server port, defaults to `5432` (used only if `connection_string` is empty)",
-			Optional:    true,
-			Type:        "port",
-		},
-		config.HelpKV{
-			Key:         target.PostgresUsername,
-			Description: "database username (used only if `connection_string` is empty)",
-			Optional:    true,
-			Type:        "string",
-		},
-		config.HelpKV{
-			Key:         target.PostgresPassword,
-			Description: "database password (used only if `connection_string` is empty)",
-			Optional:    true,
-			Type:        "string",
-		},
-		config.HelpKV{
-			Key:         target.PostgresDatabase,
-			Description: "database name (used only if `connection_string` is empty)",
-			Optional:    true,
-			Type:        "string",
-		},
 		config.HelpKV{
 			Key:         target.PostgresQueueDir,
 			Description: queueDirComment,
@@ -363,7 +333,7 @@ var (
 	HelpMySQL = config.HelpKVS{
 		config.HelpKV{
 			Key:         target.MySQLDSNString,
-			Description: "MySQL data-source-name connection string",
+			Description: `MySQL data-source-name connection string e.g. "<user>:<password>@tcp(<host>:<port>)/<database>"`,
 			Optional:    true,
 			Type:        "string",
 		},
@@ -377,36 +347,6 @@ var (
 			Description: formatComment,
 			Type:        "namespace*|access",
 		},
-		config.HelpKV{
-			Key:         target.MySQLHost,
-			Description: "MySQL server hostname (used only if `dsn_string` is empty)",
-			Optional:    true,
-			Type:        "hostname",
-		},
-		config.HelpKV{
-			Key:         target.MySQLPort,
-			Description: "MySQL server port (used only if `dsn_string` is empty)",
-			Optional:    true,
-			Type:        "port",
-		},
-		config.HelpKV{
-			Key:         target.MySQLUsername,
-			Description: "database username (used only if `dsn_string` is empty)",
-			Optional:    true,
-			Type:        "string",
-		},
-		config.HelpKV{
-			Key:         target.MySQLPassword,
-			Description: "database password (used only if `dsn_string` is empty)",
-			Optional:    true,
-			Type:        "string",
-		},
-		config.HelpKV{
-			Key:         target.MySQLDatabase,
-			Description: "database name (used only if `dsn_string` is empty)",
-			Optional:    true,
-			Type:        "string",
-		},
 		config.HelpKV{
 			Key:         target.MySQLQueueDir,
 			Description: queueDirComment,

cmd/config/notify/parse.go

@@ -306,8 +306,10 @@ func checkValidNotificationKeys(cfg config.Config) error {
 			if tname != config.Default {
 				subSysTarget = subSys + config.SubSystemSeparator + tname
 			}
-			if err := config.CheckValidKeys(subSysTarget, kv, validKVS); err != nil {
-				return err
+			if v, ok := kv.Lookup(config.Enable); ok && v == config.EnableOn {
+				if err := config.CheckValidKeys(subSysTarget, kv, validKVS); err != nil {
+					return err
+				}
 			}
 		}
 	}
@@ -696,26 +698,6 @@ var (
 			Key:   target.MySQLFormat,
 			Value: formatNamespace,
 		},
-		config.KV{
-			Key:   target.MySQLHost,
-			Value: "",
-		},
-		config.KV{
-			Key:   target.MySQLPort,
-			Value: "",
-		},
-		config.KV{
-			Key:   target.MySQLUsername,
-			Value: "",
-		},
-		config.KV{
-			Key:   target.MySQLPassword,
-			Value: "",
-		},
-		config.KV{
-			Key:   target.MySQLDatabase,
-			Value: "",
-		},
 		config.KV{
 			Key:   target.MySQLDSNString,
 			Value: "",
@@ -752,16 +734,6 @@ func GetNotifyMySQL(mysqlKVS map[string]config.KVS) (map[string]target.MySQLArgs
 			continue
 		}
 
-		hostEnv := target.EnvMySQLHost
-		if k != config.Default {
-			hostEnv = hostEnv + config.Default + k
-		}
-
-		host, err := xnet.ParseURL(env.Get(hostEnv, kv.Get(target.MySQLHost)))
-		if err != nil {
-			return nil, err
-		}
-
 		queueLimitEnv := target.EnvMySQLQueueLimit
 		if k != config.Default {
 			queueLimitEnv = queueLimitEnv + config.Default + k
@@ -775,30 +747,17 @@ func GetNotifyMySQL(mysqlKVS map[string]config.KVS) (map[string]target.MySQLArgs
 		if k != config.Default {
 			formatEnv = formatEnv + config.Default + k
 		}
+
 		dsnStringEnv := target.EnvMySQLDSNString
 		if k != config.Default {
 			dsnStringEnv = dsnStringEnv + config.Default + k
 		}
+
 		tableEnv := target.EnvMySQLTable
 		if k != config.Default {
 			tableEnv = tableEnv + config.Default + k
 		}
-		portEnv := target.EnvMySQLPort
-		if k != config.Default {
-			portEnv = portEnv + config.Default + k
-		}
-		usernameEnv := target.EnvMySQLUsername
-		if k != config.Default {
-			usernameEnv = usernameEnv + config.Default + k
-		}
-		passwordEnv := target.EnvMySQLPassword
-		if k != config.Default {
-			passwordEnv = passwordEnv + config.Default + k
-		}
-		databaseEnv := target.EnvMySQLDatabase
-		if k != config.Default {
-			databaseEnv = databaseEnv + config.Default + k
-		}
+
 		queueDirEnv := target.EnvMySQLQueueDir
 		if k != config.Default {
 			queueDirEnv = queueDirEnv + config.Default + k
@@ -808,11 +767,6 @@ func GetNotifyMySQL(mysqlKVS map[string]config.KVS) (map[string]target.MySQLArgs
 			Format:     env.Get(formatEnv, kv.Get(target.MySQLFormat)),
 			DSN:        env.Get(dsnStringEnv, kv.Get(target.MySQLDSNString)),
 			Table:      env.Get(tableEnv, kv.Get(target.MySQLTable)),
-			Host:       *host,
-			Port:       env.Get(portEnv, kv.Get(target.MySQLPort)),
-			User:       env.Get(usernameEnv, kv.Get(target.MySQLUsername)),
-			Password:   env.Get(passwordEnv, kv.Get(target.MySQLPassword)),
-			Database:   env.Get(databaseEnv, kv.Get(target.MySQLDatabase)),
 			QueueDir:   env.Get(queueDirEnv, kv.Get(target.MySQLQueueDir)),
 			QueueLimit: queueLimit,
 		}
@@ -1180,26 +1134,6 @@ var (
 			Key:   target.PostgresTable,
 			Value: "",
 		},
-		config.KV{
-			Key:   target.PostgresHost,
-			Value: "",
-		},
-		config.KV{
-			Key:   target.PostgresPort,
-			Value: "",
-		},
-		config.KV{
-			Key:   target.PostgresUsername,
-			Value: "",
-		},
-		config.KV{
-			Key:   target.PostgresPassword,
-			Value: "",
-		},
-		config.KV{
-			Key:   target.PostgresDatabase,
-			Value: "",
-		},
 		config.KV{
 			Key:   target.PostgresQueueDir,
 			Value: "",
@@ -1228,16 +1162,6 @@ func GetNotifyPostgres(postgresKVS map[string]config.KVS) (map[string]target.Pos
 			continue
 		}
 
-		hostEnv := target.EnvPostgresHost
-		if k != config.Default {
-			hostEnv = hostEnv + config.Default + k
-		}
-
-		host, err := xnet.ParseHost(env.Get(hostEnv, kv.Get(target.PostgresHost)))
-		if err != nil {
-			return nil, err
-		}
-
 		queueLimitEnv := target.EnvPostgresQueueLimit
 		if k != config.Default {
 			queueLimitEnv = queueLimitEnv + config.Default + k
@@ -1263,26 +1187,6 @@ func GetNotifyPostgres(postgresKVS map[string]config.KVS) (map[string]target.Pos
 			tableEnv = tableEnv + config.Default + k
 		}
 
-		portEnv := target.EnvPostgresPort
-		if k != config.Default {
-			portEnv = portEnv + config.Default + k
-		}
-
-		usernameEnv := target.EnvPostgresUsername
-		if k != config.Default {
-			usernameEnv = usernameEnv + config.Default + k
-		}
-
-		passwordEnv := target.EnvPostgresPassword
-		if k != config.Default {
-			passwordEnv = passwordEnv + config.Default + k
-		}
-
-		databaseEnv := target.EnvPostgresDatabase
-		if k != config.Default {
-			databaseEnv = databaseEnv + config.Default + k
-		}
-
 		queueDirEnv := target.EnvPostgresQueueDir
 		if k != config.Default {
 			queueDirEnv = queueDirEnv + config.Default + k
@@ -1293,11 +1197,6 @@ func GetNotifyPostgres(postgresKVS map[string]config.KVS) (map[string]target.Pos
 			Format:           env.Get(formatEnv, kv.Get(target.PostgresFormat)),
 			ConnectionString: env.Get(connectionStringEnv, kv.Get(target.PostgresConnectionString)),
 			Table:            env.Get(tableEnv, kv.Get(target.PostgresTable)),
-			Host:             *host,
-			Port:             env.Get(portEnv, kv.Get(target.PostgresPort)),
-			User:             env.Get(usernameEnv, kv.Get(target.PostgresUsername)),
-			Password:         env.Get(passwordEnv, kv.Get(target.PostgresPassword)),
-			Database:         env.Get(databaseEnv, kv.Get(target.PostgresDatabase)),
 			QueueDir:         env.Get(queueDirEnv, kv.Get(target.PostgresQueueDir)),
 			QueueLimit:       uint64(queueLimit),
 		}