From 6ed190e6f44a70bf8970569a9f784212cf0a817f Mon Sep 17 00:00:00 2001 From: Nikhil Vasan Date: Thu, 18 Jul 2024 09:01:45 -0700 Subject: [PATCH 1/4] account for protocols that aren't tcp --- pkg/grpc/client.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/grpc/client.go b/pkg/grpc/client.go index 612210f40..2e7ef3e93 100644 --- a/pkg/grpc/client.go +++ b/pkg/grpc/client.go @@ -1,7 +1,8 @@ package grpc import ( - "strings" + "fmt" + "net/url" grpc "google.golang.org/grpc" ) @@ -13,12 +14,15 @@ func NewClient( target string, opts ...grpc.DialOption, ) (conn *grpc.ClientConn, err error) { - // strip the scheme from the target - target = strings.TrimPrefix(strings.TrimPrefix(target, "http://"), "https://") + // We need to strip the protocol / scheme from the URL + ip, err := url.Parse(target) + if err != nil { + return nil, err + } // create a new client return grpc.NewClient( - target, + fmt.Sprintf("%s:%s", ip.Hostname(), ip.Port()), opts..., ) } From c5dcaa43620856c49a2576a8b638e72438563810 Mon Sep 17 00:00:00 2001 From: Nikhil Vasan Date: Thu, 18 Jul 2024 10:29:00 -0700 Subject: [PATCH 2/4] account for schemes when dialing over grpc --- pkg/grpc/client.go | 17 +++++++++--- pkg/grpc/client_test.go | 60 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 pkg/grpc/client_test.go diff --git a/pkg/grpc/client.go b/pkg/grpc/client.go index 2e7ef3e93..ad615e6a1 100644 --- a/pkg/grpc/client.go +++ b/pkg/grpc/client.go @@ -3,6 +3,7 @@ package grpc import ( "fmt" "net/url" + "net" grpc "google.golang.org/grpc" ) @@ -14,15 +15,23 @@ func NewClient( target string, opts ...grpc.DialOption, ) (conn *grpc.ClientConn, err error) { - // We need to strip the protocol / scheme from the URL - ip, err := url.Parse(target) + // check if this is a host:port URI, if so continue, + // otherwise, parse the URL and extract the host and port + host, port, err := net.SplitHostPort(target) if err != nil { - return nil, err + // parse the URL + ip, err := url.Parse(target) + if err != nil { + return nil, err + } + + // extract the host and port + host, port = ip.Hostname(), ip.Port() } // create a new client return grpc.NewClient( - fmt.Sprintf("%s:%s", ip.Hostname(), ip.Port()), + fmt.Sprintf("%s:%s", host, port), opts..., ) } diff --git a/pkg/grpc/client_test.go b/pkg/grpc/client_test.go new file mode 100644 index 000000000..a2ef18365 --- /dev/null +++ b/pkg/grpc/client_test.go @@ -0,0 +1,60 @@ +package grpc_test + +import ( + "fmt" + "net" + "testing" + "context" + + slinkygrpc "github.com/skip-mev/slinky/pkg/grpc" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/reflection" + reflectionpb "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" +) + +func TestClient(t *testing.T) { + // spin up a mock grpc-server + test connecting to it via diff addresses + srv := grpc.NewServer() + + // listen on a random open port + lis, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + // register reflection service on the server + reflection.Register(srv) + + // start the server + go func() { + if err := srv.Serve(lis); err != nil { + t.Fatalf("failed to serve: %v", err) + } + }() + + _, port, err := net.SplitHostPort(lis.Addr().String()) + if err != nil { + t.Fatalf("failed to parse address: %v", err) + } + + t.Run("try dialing via non supported GRPC target URL (i.e tcp prefix)", func(t *testing.T) { + // try dialing via non supported GRPC target URL (i.e tcp prefix) + client, err := slinkygrpc.NewClient(fmt.Sprintf("tcp://localhost:%s", port), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + + // ping the server + _, err = reflectionpb.NewServerReflectionClient(client).ServerReflectionInfo(context.Background()) + require.NoError(t, err) + }) + + t.Run("try dialing via supported GRPC target URL (i.e host:port)", func(t *testing.T) { + // try dialing via supported GRPC target URL (i.e host:port) + client, err := slinkygrpc.NewClient(fmt.Sprintf("localhost:%s", port), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + + // ping the server + _, err = reflectionpb.NewServerReflectionClient(client).ServerReflectionInfo(context.Background()) + require.NoError(t, err) + }) +} From eed3ca1aec45656eebd36ec28ecaacb38ab601c5 Mon Sep 17 00:00:00 2001 From: Nikhil Vasan Date: Thu, 18 Jul 2024 12:09:59 -0700 Subject: [PATCH 3/4] lint --- pkg/grpc/client.go | 2 +- pkg/grpc/client_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/grpc/client.go b/pkg/grpc/client.go index ad615e6a1..f6952c11b 100644 --- a/pkg/grpc/client.go +++ b/pkg/grpc/client.go @@ -2,8 +2,8 @@ package grpc import ( "fmt" - "net/url" "net" + "net/url" grpc "google.golang.org/grpc" ) diff --git a/pkg/grpc/client_test.go b/pkg/grpc/client_test.go index a2ef18365..19a955993 100644 --- a/pkg/grpc/client_test.go +++ b/pkg/grpc/client_test.go @@ -1,10 +1,10 @@ package grpc_test import ( + "context" "fmt" "net" "testing" - "context" slinkygrpc "github.com/skip-mev/slinky/pkg/grpc" "github.com/stretchr/testify/require" From 2c28de5908d67747850450b157497edf9080495e Mon Sep 17 00:00:00 2001 From: Nikhil Vasan Date: Thu, 18 Jul 2024 13:28:19 -0700 Subject: [PATCH 4/4] linting --- pkg/grpc/client_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/grpc/client_test.go b/pkg/grpc/client_test.go index 19a955993..0bb98b191 100644 --- a/pkg/grpc/client_test.go +++ b/pkg/grpc/client_test.go @@ -28,9 +28,7 @@ func TestClient(t *testing.T) { // start the server go func() { - if err := srv.Serve(lis); err != nil { - t.Fatalf("failed to serve: %v", err) - } + srv.Serve(lis) }() _, port, err := net.SplitHostPort(lis.Addr().String())