Skip to content

Commit

Permalink
Merge pull request #13224 from sakateka/issue-7798-netutil-fix
Browse files Browse the repository at this point in the history 
netutil: add url comparison without resolver to URLStringsEqual
  • Loading branch information
@ptabor
ptabor authored Oct 7, 2021
2 parents 3f639e5 + b272b98 commit 33623c3
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 36 deletions.
53 changes: 34 additions & 19 deletions pkg/netutil/netutil.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,31 @@ func urlsEqual(ctx context.Context, lg *zap.Logger, a []url.URL, b []url.URL) (b
if len(a) != len(b) {
return false, fmt.Errorf("len(%q) != len(%q)", urlsToStrings(a), urlsToStrings(b))
}

sort.Sort(types.URLs(a))
sort.Sort(types.URLs(b))
var needResolve bool
for i := range a {
if !reflect.DeepEqual(a[i], b[i]) {
needResolve = true
break
}
}
if !needResolve {
return true, nil
}

// If URLs are not equal, try to resolve it and compare again.
urls, err := resolveTCPAddrs(ctx, lg, [][]url.URL{a, b})
if err != nil {
return false, err
}
preva, prevb := a, b
a, b = urls[0], urls[1]
sort.Sort(types.URLs(a))
sort.Sort(types.URLs(b))
for i := range a {
if !reflect.DeepEqual(a[i], b[i]) {
return false, fmt.Errorf("%q(resolved from %q) != %q(resolved from %q)",
a[i].String(), preva[i].String(),
b[i].String(), prevb[i].String(),
)
return false, fmt.Errorf("resolved urls: %q != %q", a[i].String(), b[i].String())
}
}
return true, nil
Expand All @@ -174,21 +185,13 @@ func URLStringsEqual(ctx context.Context, lg *zap.Logger, a []string, b []string
if len(a) != len(b) {
return false, fmt.Errorf("len(%q) != len(%q)", a, b)
}
urlsA := make([]url.URL, 0)
for _, str := range a {
u, err := url.Parse(str)
if err != nil {
return false, fmt.Errorf("failed to parse %q", str)
}
urlsA = append(urlsA, *u)
urlsA, err := stringsToURLs(a)
if err != nil {
return false, err
}
urlsB := make([]url.URL, 0)
for _, str := range b {
u, err := url.Parse(str)
if err != nil {
return false, fmt.Errorf("failed to parse %q", str)
}
urlsB = append(urlsB, *u)
urlsB, err := stringsToURLs(b)
if err != nil {
return false, err
}
if lg == nil {
lg, _ = zap.NewProduction()
Expand All @@ -207,6 +210,18 @@ func urlsToStrings(us []url.URL) []string {
return rs
}

func stringsToURLs(us []string) ([]url.URL, error) {
urls := make([]url.URL, 0, len(us))
for _, str := range us {
u, err := url.Parse(str)
if err != nil {
return nil, fmt.Errorf("failed to parse string to URL: %q", str)
}
urls = append(urls, *u)
}
return urls, nil
}

func IsNetworkTimeoutError(err error) bool {
nerr, ok := err.(net.Error)
return ok && nerr.Timeout()
Expand Down
77 changes: 60 additions & 17 deletions pkg/netutil/netutil_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package netutil
import (
"context"
"errors"
"fmt"
"net"
"net/url"
"reflect"
Expand Down Expand Up @@ -166,113 +167,133 @@ func TestURLsEqual(t *testing.T) {
}

tests := []struct {
n int
a []url.URL
b []url.URL
expect bool
err error
}{
{
n: 0,
a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}},
b: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}},
expect: true,
},
{
n: 1,
a: []url.URL{{Scheme: "http", Host: "example.com:2379"}},
b: []url.URL{{Scheme: "http", Host: "10.0.10.1:2379"}},
expect: true,
},
{
n: 2,
a: []url.URL{{Scheme: "http", Host: "example.com:2379"}},
b: []url.URL{{Scheme: "https", Host: "10.0.10.1:2379"}},
expect: false,
err: errors.New(`"http://10.0.10.1:2379"(resolved from "http://example.com:2379") != "https://10.0.10.1:2379"(resolved from "https://10.0.10.1:2379")`),
err: errors.New(`resolved urls: "http://10.0.10.1:2379" != "https://10.0.10.1:2379"`),
},
{
n: 3,
a: []url.URL{{Scheme: "https", Host: "example.com:2379"}},
b: []url.URL{{Scheme: "http", Host: "10.0.10.1:2379"}},
expect: false,
err: errors.New(`"https://10.0.10.1:2379"(resolved from "https://example.com:2379") != "http://10.0.10.1:2379"(resolved from "http://10.0.10.1:2379")`),
err: errors.New(`resolved urls: "https://10.0.10.1:2379" != "http://10.0.10.1:2379"`),
},
{
n: 4,
a: []url.URL{{Scheme: "unix", Host: "abc:2379"}},
b: []url.URL{{Scheme: "unix", Host: "abc:2379"}},
expect: true,
},
{
n: 5,
a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
b: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
expect: true,
},
{
n: 6,
a: []url.URL{{Scheme: "http", Host: "example.com:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
b: []url.URL{{Scheme: "http", Host: "example.com:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
expect: true,
},
{
n: 7,
a: []url.URL{{Scheme: "http", Host: "10.0.10.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
b: []url.URL{{Scheme: "http", Host: "example.com:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
expect: true,
},
{
n: 8,
a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}},
b: []url.URL{{Scheme: "http", Host: "127.0.0.1:2380"}},
expect: false,
err: errors.New(`"http://127.0.0.1:2379"(resolved from "http://127.0.0.1:2379") != "http://127.0.0.1:2380"(resolved from "http://127.0.0.1:2380")`),
err: errors.New(`resolved urls: "http://127.0.0.1:2379" != "http://127.0.0.1:2380"`),
},
{
n: 9,
a: []url.URL{{Scheme: "http", Host: "example.com:2380"}},
b: []url.URL{{Scheme: "http", Host: "10.0.10.1:2379"}},
expect: false,
err: errors.New(`"http://10.0.10.1:2380"(resolved from "http://example.com:2380") != "http://10.0.10.1:2379"(resolved from "http://10.0.10.1:2379")`),
err: errors.New(`resolved urls: "http://10.0.10.1:2380" != "http://10.0.10.1:2379"`),
},
{
n: 10,
a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}},
b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}},
expect: false,
err: errors.New(`"http://127.0.0.1:2379"(resolved from "http://127.0.0.1:2379") != "http://10.0.0.1:2379"(resolved from "http://10.0.0.1:2379")`),
err: errors.New(`resolved urls: "http://127.0.0.1:2379" != "http://10.0.0.1:2379"`),
},
{
n: 11,
a: []url.URL{{Scheme: "http", Host: "example.com:2379"}},
b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}},
expect: false,
err: errors.New(`"http://10.0.10.1:2379"(resolved from "http://example.com:2379") != "http://10.0.0.1:2379"(resolved from "http://10.0.0.1:2379")`),
err: errors.New(`resolved urls: "http://10.0.10.1:2379" != "http://10.0.0.1:2379"`),
},
{
n: 12,
a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
b: []url.URL{{Scheme: "http", Host: "127.0.0.1:2380"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
expect: false,
err: errors.New(`"http://127.0.0.1:2379"(resolved from "http://127.0.0.1:2379") != "http://127.0.0.1:2380"(resolved from "http://127.0.0.1:2380")`),
err: errors.New(`resolved urls: "http://127.0.0.1:2379" != "http://127.0.0.1:2380"`),
},
{
n: 13,
a: []url.URL{{Scheme: "http", Host: "example.com:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
b: []url.URL{{Scheme: "http", Host: "127.0.0.1:2380"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
expect: false,
err: errors.New(`"http://10.0.10.1:2379"(resolved from "http://example.com:2379") != "http://127.0.0.1:2380"(resolved from "http://127.0.0.1:2380")`),
err: errors.New(`resolved urls: "http://10.0.10.1:2379" != "http://127.0.0.1:2380"`),
},
{
n: 14,
a: []url.URL{{Scheme: "http", Host: "127.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
expect: false,
err: errors.New(`"http://127.0.0.1:2379"(resolved from "http://127.0.0.1:2379") != "http://10.0.0.1:2379"(resolved from "http://10.0.0.1:2379")`),
err: errors.New(`resolved urls: "http://127.0.0.1:2379" != "http://10.0.0.1:2379"`),
},
{
n: 15,
a: []url.URL{{Scheme: "http", Host: "example.com:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
expect: false,
err: errors.New(`"http://10.0.10.1:2379"(resolved from "http://example.com:2379") != "http://10.0.0.1:2379"(resolved from "http://10.0.0.1:2379")`),
err: errors.New(`resolved urls: "http://10.0.10.1:2379" != "http://10.0.0.1:2379"`),
},
{
n: 16,
a: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}},
b: []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "127.0.0.1:2380"}},
expect: false,
err: errors.New(`len(["http://10.0.0.1:2379"]) != len(["http://10.0.0.1:2379" "http://127.0.0.1:2380"])`),
},
{
n: 17,
a: []url.URL{{Scheme: "http", Host: "first.com:2379"}, {Scheme: "http", Host: "second.com:2380"}},
b: []url.URL{{Scheme: "http", Host: "10.0.11.1:2379"}, {Scheme: "http", Host: "10.0.11.2:2380"}},
expect: true,
},
{
n: 18,
a: []url.URL{{Scheme: "http", Host: "second.com:2380"}, {Scheme: "http", Host: "first.com:2379"}},
b: []url.URL{{Scheme: "http", Host: "10.0.11.1:2379"}, {Scheme: "http", Host: "10.0.11.2:2380"}},
expect: true,
Expand All @@ -282,21 +303,43 @@ func TestURLsEqual(t *testing.T) {
for i, test := range tests {
result, err := urlsEqual(context.TODO(), zap.NewExample(), test.a, test.b)
if result != test.expect {
t.Errorf("#%d: a:%v b:%v, expected %v but %v", i, test.a, test.b, test.expect, result)
t.Errorf("idx=%d #%d: a:%v b:%v, expected %v but %v", i, test.n, test.a, test.b, test.expect, result)
}
if test.err != nil {
if err.Error() != test.err.Error() {
t.Errorf("#%d: err expected %v but %v", i, test.err, err)
t.Errorf("idx=%d #%d: err expected %v but %v", i, test.n, test.err, err)
}
}
}
}
func TestURLStringsEqual(t *testing.T) {
result, err := URLStringsEqual(context.TODO(), zap.NewExample(), []string{"http://127.0.0.1:8080"}, []string{"http://127.0.0.1:8080"})
if !result {
t.Errorf("unexpected result %v", result)
defer func() { resolveTCPAddr = resolveTCPAddrDefault }()
errOnResolve := func(ctx context.Context, addr string) (*net.TCPAddr, error) {
return nil, fmt.Errorf("unexpected attempt to resolve: %q", addr)
}
cases := []struct {
urlsA []string
urlsB []string
resolver func(ctx context.Context, addr string) (*net.TCPAddr, error)
}{
{[]string{"http://127.0.0.1:8080"}, []string{"http://127.0.0.1:8080"}, resolveTCPAddrDefault},
{[]string{
"http://host1:8080",
"http://host2:8080",
}, []string{
"http://host1:8080",
"http://host2:8080",
}, errOnResolve},
}
if err != nil {
t.Errorf("unexpected error %v", err)
for idx, c := range cases {
t.Logf("TestURLStringsEqual, case #%d", idx)
resolveTCPAddr = c.resolver
result, err := URLStringsEqual(context.TODO(), zap.NewExample(), c.urlsA, c.urlsB)
if !result {
t.Errorf("unexpected result %v", result)
}
if err != nil {
t.Errorf("unexpected error %v", err)
}
}
}

0 comments on commit 33623c3

Please sign in to comment.