feat: update to ParseClusterURL and use addr param

cluster.go

@@ -133,12 +133,15 @@ func (opt *ClusterOptions) init() {
 	}
 }
 
-// ParseClusterURLs parses an array of URLs into ClusterOptions that can be used to connect to Redis.
+// ParseClusterURL parses a URL into ClusterOptions that can be used to connect to Redis.
-// The strings in the array must be in the form:
+// The URL must be in the form:
 //		redis://<user>:<password>@<host>:<port>
 //		or
 //		rediss://<user>:<password>@<host>:<port>
-// All strings in the array must use the same scheme, username, and password.
+// To add additional addresses, specify the query parameter, "addr" one or more times. e.g:
+//		redis://<user>:<password>@<host>:<port>?addr=<host2>:<port2>&addr=<host3>:<port3>
+//		or
+//		rediss://<user>:<password>@<host>:<port>?addr=<host2>:<port2>&addr=<host3>:<port3>
 //
 // Most Option fields can be set using query parameters, with the following restrictions:
 //	- field names are mapped using snake-case conversion: to set MaxRetries, use max_retries
@@ -152,32 +155,25 @@ func (opt *ClusterOptions) init() {
 //	  URL attributes (scheme, host, userinfo, resp.), query paremeters using these
 //	  names will be treated as unknown parameters
 //	- unknown parameter names will result in an error
-// 	- if query parameters differ between urls, the last one in the array will be used
+// Example:
-// Examples:
+//		redis://user:password@localhost:6789?dial_timeout=3&read_timeout=6s&max_retries=2&addr=localhost:6790&addr=localhost:6791
-//		[
-//			redis://user:password@localhost:6789?dial_timeout=3&read_timeout=6s&max_retries=2,
-//			redis://user:password@localhost:6790?dial_timeout=3&read_timeout=6s&max_retries=2,
-//			redis://user:password@localhost:6791?dial_timeout=3&read_timeout=6s&max_retries=5,
-//		]
 //		is equivalent to:
 //		&ClusterOptions{
 //			Addr:        ["localhost:6789", "localhost:6790", "localhost:6791"]
 //			DialTimeout: 3 * time.Second, // no time unit = seconds
 //			ReadTimeout: 6 * time.Second,
-//			MaxRetries:  5, // last one in the array is used
+//			MaxRetries:  2,
 //		}
-func ParseClusterURLs(redisURLs []string) (*ClusterOptions, error) {
+func ParseClusterURL(redisURL string) (*ClusterOptions, error) {
 	o := &ClusterOptions{}
-	previousScheme := ""
 
-	// loop through all the URLs and retrieve the addresses as well as the
-	// cluster options
-	for _, redisURL := range redisURLs {
 	u, err := url.Parse(redisURL)
 	if err != nil {
 		return nil, err
 	}
 
+	// add base URL to the array of addresses
+	// more addresses may be added through the URL params
 	h, p, err := net.SplitHostPort(u.Host)
 	if err != nil {
 		h = u.Host
@@ -188,52 +184,37 @@ func ParseClusterURLs(redisURLs []string) (*ClusterOptions, error) {
 	if p == "" {
 		p = "6379"
 	}
-		o.Addrs = append(o.Addrs, net.JoinHostPort(h, p))
 
-		// all URLS must use the same scheme
+	o.Addrs = append(o.Addrs, net.JoinHostPort(h, p))
-		if previousScheme != "" && u.Scheme != previousScheme {
-			return nil, fmt.Errorf("redis: mismatch schemes: %s and %s", previousScheme, u.Scheme)
-		}
-		previousScheme = u.Scheme
 
 	// setup username, password, and other configurations
 	o, err = setupClusterConn(u, h, o)
 	if err != nil {
 		return nil, err
 	}
-	}
+
 	return o, nil
 }
 
 // setupClusterConn gets the username and password from the URL and the query parameters.
 func setupClusterConn(u *url.URL, host string, o *ClusterOptions) (*ClusterOptions, error) {
+	switch u.Scheme {
+	case "rediss":
+		o.TLSConfig = &tls.Config{ServerName: host}
+		fallthrough
+	case "redis":
+		o.Username, o.Password = getUserPassword(u)
+	default:
+		return nil, fmt.Errorf("redis: invalid URL scheme: %s", u.Scheme)
+	}
+
 	// retrieve the configuration from the query parameters
 	o, err := setupClusterQueryParams(u, o)
 	if err != nil {
 		return nil, err
 	}
 
-	switch u.Scheme {
-	case "rediss":
-		o.TLSConfig = &tls.Config{ServerName: host}
-		fallthrough
-	case "redis":
-		// get the username & password - they must be consistent across urls
-		u, p := getUserPassword(u)
-
-		if o.Username != "" && o.Username != u {
-			return nil, fmt.Errorf("redis: mismatch usernames: %s and %s", o.Username, u)
-		}
-		if o.Password != "" && o.Password != p {
-			return nil, fmt.Errorf("redis: mismatch passwords")
-		}
-
-		o.Username, o.Password = u, p
-
 	return o, nil
-	default:
-		return nil, fmt.Errorf("redis: invalid URL scheme: %s", u.Scheme)
-	}
 }
 
 // setupClusterQueryParams converts query parameters in u to option value in o.
@@ -262,6 +243,22 @@ func setupClusterQueryParams(u *url.URL, o *ClusterOptions) (*ClusterOptions, er
 		return nil, q.err
 	}
 
+	// addr can be specified as many times as needed
+	addr := q.string("addr")
+	for addr != "" {
+		h, p, err := net.SplitHostPort(addr)
+		if err != nil || h == "" || p == "" {
+			return nil, fmt.Errorf("redis: unable to parse addr param: %s", addr)
+		}
+
+		o.Addrs = append(o.Addrs, net.JoinHostPort(h, p))
+
+		addr = q.string("addr")
+		if q.err != nil {
+			return nil, q.err
+		}
+	}
+
 	// any parameters left?
 	if r := q.remaining(); len(r) > 0 {
 		return nil, fmt.Errorf("redis: unexpected option: %s", strings.Join(r, ", "))

cluster_test.go

@@ -1286,116 +1286,92 @@ var _ = Describe("ClusterClient timeout", func() {
 	})
 })
 
-func TestParseClusterURLs(t *testing.T) {
+func TestParseClusterURL(t *testing.T) {
 	cases := []struct {
 		test string
-		urls []string
+		url  string
 		o    *redis.ClusterOptions // expected value
 		err  error
 	}{
 		{
 			test: "ParseRedisURL",
-			urls: []string{"redis://localhost:123"},
+			url:  "redis://localhost:123",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}},
 		}, {
 			test: "ParseRedissURL",
-			urls: []string{"rediss://localhost:123"},
+			url:  "rediss://localhost:123",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}, TLSConfig: &tls.Config{ /* no deep comparison */ }},
 		}, {
 			test: "MissingRedisPort",
-			urls: []string{"redis://localhost"},
+			url:  "redis://localhost",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:6379"}},
 		}, {
 			test: "MissingRedissPort",
-			urls: []string{"rediss://localhost"},
+			url:  "rediss://localhost",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:6379"}, TLSConfig: &tls.Config{ /* no deep comparison */ }},
 		}, {
 			test: "MultipleRedisURLs",
-			urls: []string{"redis://localhost:123", "redis://localhost:1234"},
+			url:  "redis://localhost:123?addr=localhost:1234&addr=localhost:12345",
-			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}},
+			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:12345", "localhost:1234"}},
 		}, {
 			test: "MultipleRedissURLs",
-			urls: []string{"rediss://localhost:123", "rediss://localhost:1234"},
+			url:  "rediss://localhost:123?addr=localhost:1234&addr=localhost:12345",
-			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}, TLSConfig: &tls.Config{ /* no deep comparison */ }},
+			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:12345", "localhost:1234"}, TLSConfig: &tls.Config{ /* no deep comparison */ }},
 		}, {
 			test: "OnlyPassword",
-			urls: []string{"redis://:bar@localhost:123"},
+			url:  "redis://:bar@localhost:123",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}, Password: "bar"},
 		}, {
 			test: "OnlyUser",
-			urls: []string{"redis://foo@localhost:123"},
+			url:  "redis://foo@localhost:123",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}, Username: "foo"},
 		}, {
 			test: "RedisUsernamePassword",
-			urls: []string{"redis://foo:bar@localhost:123"},
+			url:  "redis://foo:bar@localhost:123",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}, Username: "foo", Password: "bar"},
 		}, {
 			test: "RedissUsernamePassword",
-			urls: []string{"rediss://foo:bar@localhost:123", "rediss://foo:bar@localhost:1234"},
+			url:  "rediss://foo:bar@localhost:123?addr=localhost:1234",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}, Username: "foo", Password: "bar", TLSConfig: &tls.Config{ /* no deep comparison */ }},
 		}, {
 			test: "QueryParameters",
-			urls: []string{"redis://localhost:123?read_timeout=2&pool_fifo=true"},
+			url:  "redis://localhost:123?read_timeout=2&pool_fifo=true&addr=localhost:1234",
-			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}, ReadTimeout: 2 * time.Second, PoolFIFO: true},
+			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}, ReadTimeout: 2 * time.Second, PoolFIFO: true},
-		}, {
-			test: "UseFinalQueryParameters",
-			urls: []string{"redis://localhost:123?read_timeout=2&pool_fifo=true", "redis://localhost:1234?read_timeout=3&pool_fifo=true"},
-			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}, ReadTimeout: 3 * time.Second, PoolFIFO: true},
 		}, {
 			test: "DisabledTimeout",
-			urls: []string{"redis://localhost:123?idle_timeout=0"},
+			url:  "redis://localhost:123?idle_timeout=0",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}, IdleTimeout: -1},
 		}, {
 			test: "DisabledTimeoutNeg",
-			urls: []string{"redis://localhost:123?idle_timeout=-1"},
+			url:  "redis://localhost:123?idle_timeout=-1",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}, IdleTimeout: -1},
 		}, {
 			test: "UseDefault",
-			urls: []string{"redis://localhost:123?idle_timeout="},
+			url:  "redis://localhost:123?idle_timeout=",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}, IdleTimeout: 0},
 		}, {
 			test: "UseDefaultMissing=",
-			urls: []string{"redis://localhost:123?idle_timeout"},
+			url:  "redis://localhost:123?idle_timeout",
 			o:    &redis.ClusterOptions{Addrs: []string{"localhost:123"}, IdleTimeout: 0},
 		}, {
-			test: "RedisPasswordMismatch",
+			test: "InvalidQueryAddr",
-			urls: []string{"redis://foo:bar@localhost:123", "redis://foo:barr@localhost:1234"},
+			url:  "rediss://foo:bar@localhost:123?addr=rediss://foo:barr@localhost:1234",
-			err:  errors.New(`redis: mismatch passwords`),
+			err:  errors.New(`redis: unable to parse addr param: rediss://foo:barr@localhost:1234`),
-		}, {
-			test: "RedisUsernameMismatch",
-			urls: []string{"redis://fooo:bar@localhost:123", "redis://foo:bar@localhost:1234"},
-			err:  errors.New(`redis: mismatch usernames: fooo and foo`),
-		}, {
-			test: "RedissPasswordMismatch",
-			urls: []string{"rediss://foo:bar@localhost:123", "rediss://foo:barr@localhost:1234"},
-			err:  errors.New(`redis: mismatch passwords`),
-		}, {
-			test: "RedissUsernameMismatch",
-			urls: []string{"rediss://foo:bar@localhost:123", "rediss://fooo:bar@localhost:1234"},
-			err:  errors.New(`redis: mismatch usernames: foo and fooo`),
-		}, {
-			test: "SchemeMismatch",
-			urls: []string{"rediss://foo:bar@localhost:123", "redis://foo:bar@localhost:1234"},
-			err:  errors.New(`redis: mismatch schemes: rediss and redis`),
-		}, {
-			test: "SchemeMismatch",
-			urls: []string{"redis://foo:bar@localhost:123", "localhost:1234"},
-			err:  errors.New(`redis: mismatch schemes: redis and localhost`),
 		}, {
 			test: "InvalidInt",
-			urls: []string{"redis://localhost?pool_size=five"},
+			url:  "redis://localhost?pool_size=five",
 			err:  errors.New(`redis: invalid pool_size number: strconv.Atoi: parsing "five": invalid syntax`),
 		}, {
 			test: "InvalidBool",
-			urls: []string{"redis://localhost?pool_fifo=yes"},
+			url:  "redis://localhost?pool_fifo=yes",
 			err:  errors.New(`redis: invalid pool_fifo boolean: expected true/false/1/0 or an empty string, got "yes"`),
 		}, {
 			test: "UnknownParam",
-			urls: []string{"redis://localhost?abc=123"},
+			url:  "redis://localhost?abc=123",
 			err:  errors.New("redis: unexpected option: abc"),
 		}, {
 			test: "InvalidScheme",
-			urls: []string{"https://google.com"},
+			url:  "https://google.com",
 			err:  errors.New("redis: invalid URL scheme: https"),
 		},
 	}
@@ -1405,11 +1381,15 @@ func TestParseClusterURLs(t *testing.T) {
 		t.Run(tc.test, func(t *testing.T) {
 			t.Parallel()
 
-			actual, err := redis.ParseClusterURLs(tc.urls)
+			actual, err := redis.ParseClusterURL(tc.url)
 			if tc.err == nil && err != nil {
 				t.Fatalf("unexpected error: %q", err)
 				return
 			}
+			if tc.err != nil && err == nil {
+				t.Fatalf("expected error: got %+v", actual)
+				return
+			}
 			if tc.err != nil && err != nil {
 				if tc.err.Error() != err.Error() {
 					t.Fatalf("got %q, expected %q", err, tc.err)

options.go

@@ -296,7 +296,14 @@ func (o *queryOptions) string(name string) string {
 	if len(vs) == 0 {
 		return ""
 	}
-	delete(o.q, name) // enable detection of unknown parameters
+
+	// enable detection of unknown parameters
+	if len(vs) > 1 {
+		o.q[name] = o.q[name][:len(vs)-1]
+	} else {
+		delete(o.q, name)
+	}
+
 	return vs[len(vs)-1]
 }