From ce816b23bb4a0917efd663afb9f3a47cad258c57 Mon Sep 17 00:00:00 2001 From: Chao Chen Date: Thu, 27 Jul 2023 15:18:12 -0700 Subject: [PATCH] endpoints.Interpret returns Host:port as ServerName Signed-off-by: Chao Chen --- client/v3/internal/endpoint/endpoint.go | 18 ++++----- client/v3/internal/endpoint/endpoint_test.go | 24 ++++++------ tests/e2e/ctl_v3_grpc_test.go | 40 +++++++++++--------- tests/integration/grpc_test.go | 13 ++++--- 4 files changed, 49 insertions(+), 46 deletions(-) diff --git a/client/v3/internal/endpoint/endpoint.go b/client/v3/internal/endpoint/endpoint.go index f6674235cd9e..35a3fe8c3374 100644 --- a/client/v3/internal/endpoint/endpoint.go +++ b/client/v3/internal/endpoint/endpoint.go @@ -41,10 +41,6 @@ func extractHostFromHostPort(ep string) string { return host } -func extractHostFromPath(pathStr string) string { - return extractHostFromHostPort(path.Base(pathStr)) -} - // mustSplit2 returns the values from strings.SplitN(s, sep, 2). // If sep is not found, it returns ("", "", false) instead. func mustSplit2(s, sep string) (string, string) { @@ -96,29 +92,29 @@ func translateEndpoint(ep string) (addr string, serverName string, requireCreds if strings.HasPrefix(ep, "unix:///") || strings.HasPrefix(ep, "unixs:///") { // absolute path case schema, absolutePath := mustSplit2(ep, "://") - return "unix://" + absolutePath, extractHostFromPath(absolutePath), schemeToCredsRequirement(schema) + return "unix://" + absolutePath, path.Base(absolutePath), schemeToCredsRequirement(schema) } if strings.HasPrefix(ep, "unix://") || strings.HasPrefix(ep, "unixs://") { // legacy etcd local path schema, localPath := mustSplit2(ep, "://") - return "unix:" + localPath, extractHostFromPath(localPath), schemeToCredsRequirement(schema) + return "unix:" + localPath, path.Base(localPath), schemeToCredsRequirement(schema) } schema, localPath := mustSplit2(ep, ":") - return "unix:" + localPath, extractHostFromPath(localPath), schemeToCredsRequirement(schema) + return "unix:" + localPath, path.Base(localPath), schemeToCredsRequirement(schema) } if strings.Contains(ep, "://") { url, err := url.Parse(ep) if err != nil { - return ep, extractHostFromHostPort(ep), CREDS_OPTIONAL + return ep, ep, CREDS_OPTIONAL } if url.Scheme == "http" || url.Scheme == "https" { - return url.Host, url.Hostname(), schemeToCredsRequirement(url.Scheme) + return url.Host, url.Host, schemeToCredsRequirement(url.Scheme) } - return ep, url.Hostname(), schemeToCredsRequirement(url.Scheme) + return ep, url.Host, schemeToCredsRequirement(url.Scheme) } // Handles plain addresses like 10.0.0.44:437. - return ep, extractHostFromHostPort(ep), CREDS_OPTIONAL + return ep, ep, CREDS_OPTIONAL } // RequiresCredentials returns whether given endpoint requires diff --git a/client/v3/internal/endpoint/endpoint_test.go b/client/v3/internal/endpoint/endpoint_test.go index bc6cd71399cc..7de9226ff19a 100644 --- a/client/v3/internal/endpoint/endpoint_test.go +++ b/client/v3/internal/endpoint/endpoint_test.go @@ -27,35 +27,35 @@ func Test_interpret(t *testing.T) { }{ {"127.0.0.1", "127.0.0.1", "127.0.0.1", CREDS_OPTIONAL}, {"localhost", "localhost", "localhost", CREDS_OPTIONAL}, - {"localhost:8080", "localhost:8080", "localhost", CREDS_OPTIONAL}, + {"localhost:8080", "localhost:8080", "localhost:8080", CREDS_OPTIONAL}, {"unix:127.0.0.1", "unix:127.0.0.1", "127.0.0.1", CREDS_OPTIONAL}, - {"unix:127.0.0.1:8080", "unix:127.0.0.1:8080", "127.0.0.1", CREDS_OPTIONAL}, + {"unix:127.0.0.1:8080", "unix:127.0.0.1:8080", "127.0.0.1:8080", CREDS_OPTIONAL}, {"unix://127.0.0.1", "unix:127.0.0.1", "127.0.0.1", CREDS_OPTIONAL}, - {"unix://127.0.0.1:8080", "unix:127.0.0.1:8080", "127.0.0.1", CREDS_OPTIONAL}, + {"unix://127.0.0.1:8080", "unix:127.0.0.1:8080", "127.0.0.1:8080", CREDS_OPTIONAL}, {"unixs:127.0.0.1", "unix:127.0.0.1", "127.0.0.1", CREDS_REQUIRE}, - {"unixs:127.0.0.1:8080", "unix:127.0.0.1:8080", "127.0.0.1", CREDS_REQUIRE}, + {"unixs:127.0.0.1:8080", "unix:127.0.0.1:8080", "127.0.0.1:8080", CREDS_REQUIRE}, {"unixs://127.0.0.1", "unix:127.0.0.1", "127.0.0.1", CREDS_REQUIRE}, - {"unixs://127.0.0.1:8080", "unix:127.0.0.1:8080", "127.0.0.1", CREDS_REQUIRE}, + {"unixs://127.0.0.1:8080", "unix:127.0.0.1:8080", "127.0.0.1:8080", CREDS_REQUIRE}, {"http://127.0.0.1", "127.0.0.1", "127.0.0.1", CREDS_DROP}, - {"http://127.0.0.1:8080", "127.0.0.1:8080", "127.0.0.1", CREDS_DROP}, + {"http://127.0.0.1:8080", "127.0.0.1:8080", "127.0.0.1:8080", CREDS_DROP}, {"https://127.0.0.1", "127.0.0.1", "127.0.0.1", CREDS_REQUIRE}, - {"https://127.0.0.1:8080", "127.0.0.1:8080", "127.0.0.1", CREDS_REQUIRE}, - {"https://localhost:20000", "localhost:20000", "localhost", CREDS_REQUIRE}, + {"https://127.0.0.1:8080", "127.0.0.1:8080", "127.0.0.1:8080", CREDS_REQUIRE}, + {"https://localhost:20000", "localhost:20000", "localhost:20000", CREDS_REQUIRE}, {"unix:///tmp/abc", "unix:///tmp/abc", "abc", CREDS_OPTIONAL}, {"unixs:///tmp/abc", "unix:///tmp/abc", "abc", CREDS_REQUIRE}, - {"unix:///tmp/abc:1234", "unix:///tmp/abc:1234", "abc", CREDS_OPTIONAL}, - {"unixs:///tmp/abc:1234", "unix:///tmp/abc:1234", "abc", CREDS_REQUIRE}, + {"unix:///tmp/abc:1234", "unix:///tmp/abc:1234", "abc:1234", CREDS_OPTIONAL}, + {"unixs:///tmp/abc:1234", "unix:///tmp/abc:1234", "abc:1234", CREDS_REQUIRE}, {"etcd.io", "etcd.io", "etcd.io", CREDS_OPTIONAL}, {"http://etcd.io/abc", "etcd.io", "etcd.io", CREDS_DROP}, {"dns://something-other", "dns://something-other", "something-other", CREDS_OPTIONAL}, - {"http://[2001:db8:1f70::999:de8:7648:6e8]:100/", "[2001:db8:1f70::999:de8:7648:6e8]:100", "2001:db8:1f70::999:de8:7648:6e8", CREDS_DROP}, - {"[2001:db8:1f70::999:de8:7648:6e8]:100", "[2001:db8:1f70::999:de8:7648:6e8]:100", "2001:db8:1f70::999:de8:7648:6e8", CREDS_OPTIONAL}, + {"http://[2001:db8:1f70::999:de8:7648:6e8]:100/", "[2001:db8:1f70::999:de8:7648:6e8]:100", "[2001:db8:1f70::999:de8:7648:6e8]:100", CREDS_DROP}, + {"[2001:db8:1f70::999:de8:7648:6e8]:100", "[2001:db8:1f70::999:de8:7648:6e8]:100", "[2001:db8:1f70::999:de8:7648:6e8]:100", CREDS_OPTIONAL}, {"unix:unexpected-file_name#123$456", "unix:unexpected-file_name#123$456", "unexpected-file_name#123$456", CREDS_OPTIONAL}, } for _, tt := range tests { diff --git a/tests/e2e/ctl_v3_grpc_test.go b/tests/e2e/ctl_v3_grpc_test.go index 7059017dbfac..ad7e28dd79b3 100644 --- a/tests/e2e/ctl_v3_grpc_test.go +++ b/tests/e2e/ctl_v3_grpc_test.go @@ -19,13 +19,13 @@ package e2e import ( "context" "fmt" + "net/url" "strings" "testing" "time" "github.com/stretchr/testify/assert" - "go.etcd.io/etcd/tests/v3/framework/config" "go.etcd.io/etcd/tests/v3/framework/e2e" "go.etcd.io/etcd/tests/v3/framework/testutils" ) @@ -127,20 +127,20 @@ func TestAuthority(t *testing.T) { t.Fatalf("could not start etcd process cluster (%v)", err) } defer epc.Close() - endpoints := templateEndpoints(t, tc.clientURLPattern, epc) - client, err := e2e.NewEtcdctl(cfg.Client, endpoints) - assert.NoError(t, err) - err = client.Put(ctx, "foo", "bar", config.PutOptions{}) - if err != nil { - t.Fatal(err) + endpoints := templateEndpoints(t, tc.clientURLPattern, epc) + client := newClient(t, endpoints, cfg.Client) + for i := 0; i < 100; i++ { + _, err = client.Put(ctx, "foo", "bar") + if err != nil { + t.Fatal(err) + } } testutils.ExecuteWithTimeout(t, 5*time.Second, func() { - assertAuthority(t, strings.ReplaceAll(tc.expectAuthorityPattern, "${MEMBER_PORT}", "20000"), epc) + assertAuthority(t, tc.expectAuthorityPattern, epc) }) }) - } } } @@ -156,16 +156,20 @@ func templateEndpoints(t *testing.T, pattern string, clus *e2e.EtcdProcessCluste return endpoints } -func assertAuthority(t *testing.T, expectAurhority string, clus *e2e.EtcdProcessCluster) { - var logs []e2e.LogsExpect - for _, proc := range clus.Procs { - logs = append(logs, proc.Logs()) +func assertAuthority(t *testing.T, expectAuthorityPattern string, clus *e2e.EtcdProcessCluster) { + for i := range clus.Procs { + line := firstMatch(t, `http2: decoded hpack field header field ":authority"`, clus.Procs[i].Logs()) + line = strings.TrimSuffix(line, "\n") + line = strings.TrimSuffix(line, "\r") + + u, err := url.Parse(clus.Procs[i].EndpointsGRPC()[0]) + if err != nil { + t.Fatal(err) + } + expectAuthority := strings.ReplaceAll(expectAuthorityPattern, "${MEMBER_PORT}", u.Port()) + expectLine := fmt.Sprintf(`http2: decoded hpack field header field ":authority" = %q`, expectAuthority) + assert.True(t, strings.HasSuffix(line, expectLine), fmt.Sprintf("Got %q expected suffix %q", line, expectLine)) } - line := firstMatch(t, `http2: decoded hpack field header field ":authority"`, logs...) - line = strings.TrimSuffix(line, "\n") - line = strings.TrimSuffix(line, "\r") - expectLine := fmt.Sprintf(`http2: decoded hpack field header field ":authority" = %q`, expectAurhority) - assert.True(t, strings.HasSuffix(line, expectLine), fmt.Sprintf("Got %q expected suffix %q", line, expectLine)) } func firstMatch(t *testing.T, expectLine string, logs ...e2e.LogsExpect) string { diff --git a/tests/integration/grpc_test.go b/tests/integration/grpc_test.go index 618c63f71943..cbb8ef4712ce 100644 --- a/tests/integration/grpc_test.go +++ b/tests/integration/grpc_test.go @@ -103,12 +103,14 @@ func TestAuthority(t *testing.T) { defer kv.Close() putRequestMethod := "/etcdserverpb.KV/Put" - _, err := kv.Put(context.TODO(), "foo", "bar") - if err != nil { - t.Fatal(err) + for i := 0; i < 100; i++ { + _, err := kv.Put(context.TODO(), "foo", "bar") + if err != nil { + t.Fatal(err) + } } - assertAuthority(t, templateAuthority(t, tc.expectAuthorityPattern, clus.Members[0]), clus, putRequestMethod) + assertAuthority(t, tc.expectAuthorityPattern, clus, putRequestMethod) }) } } @@ -162,10 +164,11 @@ func templateAuthority(t *testing.T, pattern string, m *integration.Member) stri return authority } -func assertAuthority(t *testing.T, expectedAuthority string, clus *integration.Cluster, filterMethod string) { +func assertAuthority(t *testing.T, expectedAuthorityPattern string, clus *integration.Cluster, filterMethod string) { t.Helper() requestsFound := 0 for _, m := range clus.Members { + expectedAuthority := templateAuthority(t, expectedAuthorityPattern, m) for _, r := range m.RecordedRequests() { if filterMethod != "" && r.FullMethod != filterMethod { continue