From 175d0d81fcef65aec8cee8e814b2e8e429971805 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Mon, 18 Oct 2021 09:07:36 -0500 Subject: [PATCH] feat: update to ParseClusterURL and use addr param --- cluster.go | 127 +++++++++++++++++++++++------------------------- cluster_test.go | 84 ++++++++++++-------------------- options.go | 9 +++- 3 files changed, 102 insertions(+), 118 deletions(-) diff --git a/cluster.go b/cluster.go index 5ecde2e..f7777e8 100644 --- a/cluster.go +++ b/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. -// The strings in the array must be in the form: +// ParseClusterURL parses a URL into ClusterOptions that can be used to connect to Redis. +// The URL must be in the form: // redis://:@: // or // rediss://:@: -// 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://:@:?addr=:&addr=: +// or +// rediss://:@:?addr=:&addr=: // // 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,88 +155,66 @@ 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 -// Examples: -// [ -// 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, -// ] +// Example: +// redis://user:password@localhost:6789?dial_timeout=3&read_timeout=6s&max_retries=2&addr=localhost:6790&addr=localhost:6791 // 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 - } - - h, p, err := net.SplitHostPort(u.Host) - if err != nil { - h = u.Host - } - if h == "" { - h = "localhost" - } - if p == "" { - p = "6379" - } - o.Addrs = append(o.Addrs, net.JoinHostPort(h, p)) - - // all URLS must use the same scheme - 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 - } + 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 + } + if h == "" { + h = "localhost" + } + if p == "" { + p = "6379" + } + + o.Addrs = append(o.Addrs, net.JoinHostPort(h, p)) + + // 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) - } + return o, nil } // 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, ", ")) diff --git a/cluster_test.go b/cluster_test.go index 92ca085..f923112 100644 --- a/cluster_test.go +++ b/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"}, - o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}}, + url: "redis://localhost:123?addr=localhost:1234&addr=localhost:12345", + o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:12345", "localhost:1234"}}, }, { test: "MultipleRedissURLs", - urls: []string{"rediss://localhost:123", "rediss://localhost:1234"}, - o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}, TLSConfig: &tls.Config{ /* no deep comparison */ }}, + url: "rediss://localhost:123?addr=localhost:1234&addr=localhost:12345", + 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"}, - o: &redis.ClusterOptions{Addrs: []string{"localhost:123"}, 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}, + url: "redis://localhost:123?read_timeout=2&pool_fifo=true&addr=localhost:1234", + o: &redis.ClusterOptions{Addrs: []string{"localhost:123", "localhost:1234"}, ReadTimeout: 2 * 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", - urls: []string{"redis://foo:bar@localhost:123", "redis://foo:barr@localhost:1234"}, - err: errors.New(`redis: mismatch passwords`), - }, { - 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: "InvalidQueryAddr", + url: "rediss://foo:bar@localhost:123?addr=rediss://foo:barr@localhost:1234", + err: errors.New(`redis: unable to parse addr param: rediss://foo:barr@localhost:1234`), }, { 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) diff --git a/options.go b/options.go index a4abe32..ed20c51 100644 --- a/options.go +++ b/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] }