From 4839ef5b79ea79252f87730e41a26aab1fcf8591 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Tue, 11 May 2021 15:44:37 -0700 Subject: [PATCH 1/2] xds: use same format while registering and watching resources For an IPv6 address, the address returned by `listener.Addr().String()` includes the `[]` around the actual literal address, but the host portion returned from a `net.SplitHostPort()` does not. We need to be consistent with what we use for registering and what we use for watching. --- xds/internal/test/xds_server_integration_test.go | 11 +++++++---- xds/internal/test/xds_server_serving_mode_test.go | 6 ++++-- xds/internal/testutils/e2e/clientresources.go | 6 ++---- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/xds/internal/test/xds_server_integration_test.go b/xds/internal/test/xds_server_integration_test.go index 6511a6134cf8..679bf054f0a3 100644 --- a/xds/internal/test/xds_server_integration_test.go +++ b/xds/internal/test/xds_server_integration_test.go @@ -135,7 +135,8 @@ func (s) TestServerSideXDS_Fallback(t *testing.T) { // Create an inbound xDS listener resource for the server side that does not // contain any security configuration. This should force the server-side // xdsCredentials to use fallback. - inboundLis := e2e.DefaultServerListener(host, port, e2e.SecurityLevelNone) + name := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis.Addr().String()) + inboundLis := e2e.DefaultServerListener(name, host, port, e2e.SecurityLevelNone) resources.Listeners = append(resources.Listeners, inboundLis) // Setup the management server with client and server-side resources. @@ -217,7 +218,8 @@ func (s) TestServerSideXDS_FileWatcherCerts(t *testing.T) { // Create an inbound xDS listener resource for the server side that // contains security configuration pointing to the file watcher // plugin. - inboundLis := e2e.DefaultServerListener(host, port, test.secLevel) + name := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis.Addr().String()) + inboundLis := e2e.DefaultServerListener(name, host, port, test.secLevel) resources.Listeners = append(resources.Listeners, inboundLis) // Setup the management server with client and server resources. @@ -282,7 +284,8 @@ func (s) TestServerSideXDS_SecurityConfigChange(t *testing.T) { // Create an inbound xDS listener resource for the server side that does not // contain any security configuration. This should force the xDS credentials // on server to use its fallback. - inboundLis := e2e.DefaultServerListener(host, port, e2e.SecurityLevelNone) + name := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis.Addr().String()) + inboundLis := e2e.DefaultServerListener(name, host, port, e2e.SecurityLevelNone) resources.Listeners = append(resources.Listeners, inboundLis) // Setup the management server with client and server-side resources. @@ -336,7 +339,7 @@ func (s) TestServerSideXDS_SecurityConfigChange(t *testing.T) { Port: port, SecLevel: e2e.SecurityLevelMTLS, }) - inboundLis = e2e.DefaultServerListener(host, port, e2e.SecurityLevelMTLS) + inboundLis = e2e.DefaultServerListener(name, host, port, e2e.SecurityLevelMTLS) resources.Listeners = append(resources.Listeners, inboundLis) if err := managementServer.Update(resources); err != nil { t.Fatal(err) diff --git a/xds/internal/test/xds_server_serving_mode_test.go b/xds/internal/test/xds_server_serving_mode_test.go index 414a559b0982..55c9661cd97f 100644 --- a/xds/internal/test/xds_server_serving_mode_test.go +++ b/xds/internal/test/xds_server_serving_mode_test.go @@ -125,12 +125,14 @@ func (s) TestServerSideXDS_ServingModeChanges(t *testing.T) { if err != nil { t.Fatalf("failed to retrieve host and port of server: %v", err) } - listener1 := e2e.DefaultServerListener(host1, port1, e2e.SecurityLevelNone) + name1 := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis1.Addr().String()) + listener1 := e2e.DefaultServerListener(name1, host1, port1, e2e.SecurityLevelNone) host2, port2, err := hostPortFromListener(lis2) if err != nil { t.Fatalf("failed to retrieve host and port of server: %v", err) } - listener2 := e2e.DefaultServerListener(host2, port2, e2e.SecurityLevelNone) + name2 := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis2.Addr().String()) + listener2 := e2e.DefaultServerListener(name2, host2, port2, e2e.SecurityLevelNone) resources := e2e.UpdateOptions{ NodeID: xdsClientNodeID, Listeners: []*v3listenerpb.Listener{listener1, listener2}, diff --git a/xds/internal/testutils/e2e/clientresources.go b/xds/internal/testutils/e2e/clientresources.go index b521db950558..11aa8b3e87a8 100644 --- a/xds/internal/testutils/e2e/clientresources.go +++ b/xds/internal/testutils/e2e/clientresources.go @@ -19,8 +19,6 @@ package e2e import ( - "fmt" - "github.com/envoyproxy/go-control-plane/pkg/wellknown" "github.com/golang/protobuf/proto" "google.golang.org/grpc/internal/testutils" @@ -122,7 +120,7 @@ func DefaultClientListener(target, routeName string) *v3listenerpb.Listener { // DefaultServerListener returns a basic xds Listener resource to be used on // the server side. -func DefaultServerListener(host string, port uint32, secLevel SecurityLevel) *v3listenerpb.Listener { +func DefaultServerListener(name, host string, port uint32, secLevel SecurityLevel) *v3listenerpb.Listener { var tlsContext *v3tlspb.DownstreamTlsContext switch secLevel { case SecurityLevelNone: @@ -160,7 +158,7 @@ func DefaultServerListener(host string, port uint32, secLevel SecurityLevel) *v3 } } return &v3listenerpb.Listener{ - Name: fmt.Sprintf(ServerListenerResourceNameTemplate, fmt.Sprintf("%s:%d", host, port)), + Name: name, Address: &v3corepb.Address{ Address: &v3corepb.Address_SocketAddress{ SocketAddress: &v3corepb.SocketAddress{ From b2f6f09a6b55ad38b2f2646aa772d76d64899517 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Tue, 11 May 2021 16:03:56 -0700 Subject: [PATCH 2/2] use net.JoinHostPort() instead --- xds/internal/test/xds_server_integration_test.go | 11 ++++------- xds/internal/test/xds_server_serving_mode_test.go | 6 ++---- xds/internal/testutils/e2e/clientresources.go | 8 ++++++-- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/xds/internal/test/xds_server_integration_test.go b/xds/internal/test/xds_server_integration_test.go index 679bf054f0a3..6511a6134cf8 100644 --- a/xds/internal/test/xds_server_integration_test.go +++ b/xds/internal/test/xds_server_integration_test.go @@ -135,8 +135,7 @@ func (s) TestServerSideXDS_Fallback(t *testing.T) { // Create an inbound xDS listener resource for the server side that does not // contain any security configuration. This should force the server-side // xdsCredentials to use fallback. - name := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis.Addr().String()) - inboundLis := e2e.DefaultServerListener(name, host, port, e2e.SecurityLevelNone) + inboundLis := e2e.DefaultServerListener(host, port, e2e.SecurityLevelNone) resources.Listeners = append(resources.Listeners, inboundLis) // Setup the management server with client and server-side resources. @@ -218,8 +217,7 @@ func (s) TestServerSideXDS_FileWatcherCerts(t *testing.T) { // Create an inbound xDS listener resource for the server side that // contains security configuration pointing to the file watcher // plugin. - name := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis.Addr().String()) - inboundLis := e2e.DefaultServerListener(name, host, port, test.secLevel) + inboundLis := e2e.DefaultServerListener(host, port, test.secLevel) resources.Listeners = append(resources.Listeners, inboundLis) // Setup the management server with client and server resources. @@ -284,8 +282,7 @@ func (s) TestServerSideXDS_SecurityConfigChange(t *testing.T) { // Create an inbound xDS listener resource for the server side that does not // contain any security configuration. This should force the xDS credentials // on server to use its fallback. - name := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis.Addr().String()) - inboundLis := e2e.DefaultServerListener(name, host, port, e2e.SecurityLevelNone) + inboundLis := e2e.DefaultServerListener(host, port, e2e.SecurityLevelNone) resources.Listeners = append(resources.Listeners, inboundLis) // Setup the management server with client and server-side resources. @@ -339,7 +336,7 @@ func (s) TestServerSideXDS_SecurityConfigChange(t *testing.T) { Port: port, SecLevel: e2e.SecurityLevelMTLS, }) - inboundLis = e2e.DefaultServerListener(name, host, port, e2e.SecurityLevelMTLS) + inboundLis = e2e.DefaultServerListener(host, port, e2e.SecurityLevelMTLS) resources.Listeners = append(resources.Listeners, inboundLis) if err := managementServer.Update(resources); err != nil { t.Fatal(err) diff --git a/xds/internal/test/xds_server_serving_mode_test.go b/xds/internal/test/xds_server_serving_mode_test.go index 55c9661cd97f..414a559b0982 100644 --- a/xds/internal/test/xds_server_serving_mode_test.go +++ b/xds/internal/test/xds_server_serving_mode_test.go @@ -125,14 +125,12 @@ func (s) TestServerSideXDS_ServingModeChanges(t *testing.T) { if err != nil { t.Fatalf("failed to retrieve host and port of server: %v", err) } - name1 := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis1.Addr().String()) - listener1 := e2e.DefaultServerListener(name1, host1, port1, e2e.SecurityLevelNone) + listener1 := e2e.DefaultServerListener(host1, port1, e2e.SecurityLevelNone) host2, port2, err := hostPortFromListener(lis2) if err != nil { t.Fatalf("failed to retrieve host and port of server: %v", err) } - name2 := fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, lis2.Addr().String()) - listener2 := e2e.DefaultServerListener(name2, host2, port2, e2e.SecurityLevelNone) + listener2 := e2e.DefaultServerListener(host2, port2, e2e.SecurityLevelNone) resources := e2e.UpdateOptions{ NodeID: xdsClientNodeID, Listeners: []*v3listenerpb.Listener{listener1, listener2}, diff --git a/xds/internal/testutils/e2e/clientresources.go b/xds/internal/testutils/e2e/clientresources.go index 11aa8b3e87a8..7c8311a51cc3 100644 --- a/xds/internal/testutils/e2e/clientresources.go +++ b/xds/internal/testutils/e2e/clientresources.go @@ -19,6 +19,10 @@ package e2e import ( + "fmt" + "net" + "strconv" + "github.com/envoyproxy/go-control-plane/pkg/wellknown" "github.com/golang/protobuf/proto" "google.golang.org/grpc/internal/testutils" @@ -120,7 +124,7 @@ func DefaultClientListener(target, routeName string) *v3listenerpb.Listener { // DefaultServerListener returns a basic xds Listener resource to be used on // the server side. -func DefaultServerListener(name, host string, port uint32, secLevel SecurityLevel) *v3listenerpb.Listener { +func DefaultServerListener(host string, port uint32, secLevel SecurityLevel) *v3listenerpb.Listener { var tlsContext *v3tlspb.DownstreamTlsContext switch secLevel { case SecurityLevelNone: @@ -158,7 +162,7 @@ func DefaultServerListener(name, host string, port uint32, secLevel SecurityLeve } } return &v3listenerpb.Listener{ - Name: name, + Name: fmt.Sprintf(ServerListenerResourceNameTemplate, net.JoinHostPort(host, strconv.Itoa(int(port)))), Address: &v3corepb.Address{ Address: &v3corepb.Address_SocketAddress{ SocketAddress: &v3corepb.SocketAddress{