Skip to content

Commit

Permalink
endpoints.Interpret returns Host:port as ServerName
Browse files Browse the repository at this point in the history 
Signed-off-by: Chao Chen <chaochn@amazon.com>
  • Loading branch information
@chaochn47
chaochn47 committed Jul 28, 2023
1 parent e59e3d7 commit bf5735f
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 60 deletions.
18 changes: 7 additions & 11 deletions client/v3/internal/endpoint/endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
24 changes: 12 additions & 12 deletions client/v3/internal/endpoint/endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
46 changes: 20 additions & 26 deletions tests/e2e/ctl_v3_grpc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package e2e
import (
"context"
"fmt"
"net/url"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -127,20 +128,21 @@ func TestAuthority(t *testing.T) {
t.Fatalf("could not start etcd process cluster (%v)", err)
}
defer epc.Close()
endpoints := templateEndpoints(t, tc.clientURLPattern, epc)

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)
for i := 0; i < 100; i++ {
err = client.Put(ctx, "foo", "bar", config.PutOptions{})
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)
})
})

}
}
}
Expand All @@ -156,26 +158,18 @@ 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())
}
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 assertAuthority(t *testing.T, expectAuthorityPattern string, clus *e2e.EtcdProcessCluster) {
for i := range clus.Procs {
line, _ := clus.Procs[i].Logs().ExpectWithContext(context.TODO(), `http2: decoded hpack field header field ":authority"`)
line = strings.TrimSuffix(line, "\n")
line = strings.TrimSuffix(line, "\r")

func firstMatch(t *testing.T, expectLine string, logs ...e2e.LogsExpect) string {
t.Helper()
match := make(chan string, len(logs))
for i := range logs {
go func(l e2e.LogsExpect) {
line, _ := l.ExpectWithContext(context.TODO(), expectLine)
match <- line
}(logs[i])
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))
}
return <-match
}
26 changes: 15 additions & 11 deletions tests/integration/grpc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Expand Down Expand Up @@ -162,21 +164,23 @@ 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 {
requestsFound := 0
expectedAuthority := templateAuthority(t, expectedAuthorityPattern, m)
for _, r := range m.RecordedRequests() {
if filterMethod != "" && r.FullMethod != filterMethod {
continue
}
requestsFound++
if r.Authority != expectedAuthority {
if r.Authority == expectedAuthority {
requestsFound++
} else {
t.Errorf("Got unexpected authority header, member: %q, request: %q, got authority: %q, expected %q", m.Name, r.FullMethod, r.Authority, expectedAuthority)
}
}
}
if requestsFound == 0 {
t.Errorf("Expected at least one request")
if requestsFound == 0 {
t.Errorf("Expect at least one request with matched authority header value was recorded by the server intercepter on member %s but got 0", m.Name)
}
}
}

0 comments on commit bf5735f

Please sign in to comment.