Merge branch 'jv-upstream-insecure' into 'master'

Use upstream implementation of insecure credentials

See merge request gitlab-org/gitaly!3591

8 files changed, 18 insertions, 52 deletions

diff --git a/internal/backchannel/backchannel_example_test.go b/internal/backchannel/backchannel_example_test.go
index a9f405238..0b06b4779 100644
--- a/internal/backchannel/backchannel_example_test.go
+++ b/internal/backchannel/backchannel_example_test.go
@@ -9,6 +9,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/backchannel"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
 )
 
 func Example() {
@@ -30,7 +31,7 @@ func Example() {
 	// it creates the backchannel connection and stores it into the registry. For each connection,
 	// the ServerHandshaker passes down the peer ID via the context. The peer ID identifies a
 	// backchannel connection.
-	handshaker := backchannel.NewServerHandshaker(logger, backchannel.Insecure(), registry, nil)
+	handshaker := backchannel.NewServerHandshaker(logger, insecure.NewCredentials(), registry, nil)
 
 	// Create the server
 	srv := grpc.NewServer(
@@ -114,7 +115,7 @@ func invokeWithMuxedClient(logger *logrus.Entry, address string) error {
 		}))
 	})
 
-	return invokeWithOpts(address, grpc.WithTransportCredentials(clientHandshaker.ClientHandshake(backchannel.Insecure())))
+	return invokeWithOpts(address, grpc.WithTransportCredentials(clientHandshaker.ClientHandshake(insecure.NewCredentials())))
 }
 
 func invokeWithNormalClient(address string) error {
diff --git a/internal/backchannel/backchannel_test.go b/internal/backchannel/backchannel_test.go
index c8b8d1081..a599d6880 100644
--- a/internal/backchannel/backchannel_test.go
+++ b/internal/backchannel/backchannel_test.go
@@ -17,6 +17,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/status"
 )
 
@@ -40,7 +41,7 @@ func TestBackchannel_concurrentRequestsFromMultipleClients(t *testing.T) {
 	registry := NewRegistry()
 	handshaker := NewServerHandshaker(
 		newLogger(),
-		Insecure(),
+		insecure.NewCredentials(),
 		registry,
 		[]grpc.DialOption{
 			grpc.WithUnaryInterceptor(func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
@@ -123,7 +124,7 @@ func TestBackchannel_concurrentRequestsFromMultipleClients(t *testing.T) {
 
 			<-start
 			client, err := grpc.Dial(ln.Addr().String(),
-				grpc.WithTransportCredentials(clientHandshaker.ClientHandshake(Insecure())),
+				grpc.WithTransportCredentials(clientHandshaker.ClientHandshake(insecure.NewCredentials())),
 			)
 			if !assert.NoError(t, err) {
 				return
@@ -182,7 +183,7 @@ func Benchmark(b *testing.B) {
 					var serverOpts []grpc.ServerOption
 					if tc.multiplexed {
 						serverOpts = []grpc.ServerOption{
-							grpc.Creds(NewServerHandshaker(newLogger(), Insecure(), NewRegistry(), nil)),
+							grpc.Creds(NewServerHandshaker(newLogger(), insecure.NewCredentials(), NewRegistry(), nil)),
 						}
 					}
 
@@ -213,7 +214,7 @@ func Benchmark(b *testing.B) {
 						clientHandshaker := NewClientHandshaker(newLogger(), func() Server { return grpc.NewServer() })
 						opts = []grpc.DialOption{
 							grpc.WithBlock(),
-							grpc.WithTransportCredentials(clientHandshaker.ClientHandshake(Insecure())),
+							grpc.WithTransportCredentials(clientHandshaker.ClientHandshake(insecure.NewCredentials())),
 						}
 					}
 
diff --git a/internal/backchannel/insecure.go b/internal/backchannel/insecure.go
deleted file mode 100644
index 678a90527..000000000
--- a/internal/backchannel/insecure.go
+++ /dev/null
@@ -1,40 +0,0 @@
-package backchannel
-
-import (
-	"context"
-	"net"
-
-	"google.golang.org/grpc/credentials"
-)
-
-type insecureAuthInfo struct{ credentials.CommonAuthInfo }
-
-func (insecureAuthInfo) AuthType() string { return "insecure" }
-
-type insecure struct{}
-
-func (insecure) ServerHandshake(conn net.Conn) (net.Conn, credentials.AuthInfo, error) {
-	return conn, insecureAuthInfo{credentials.CommonAuthInfo{SecurityLevel: credentials.NoSecurity}}, nil
-}
-
-func (insecure) ClientHandshake(_ context.Context, _ string, conn net.Conn) (net.Conn, credentials.AuthInfo, error) {
-	return conn, insecureAuthInfo{credentials.CommonAuthInfo{SecurityLevel: credentials.NoSecurity}}, nil
-}
-
-func (insecure) Info() credentials.ProtocolInfo {
-	return credentials.ProtocolInfo{SecurityProtocol: "insecure"}
-}
-
-func (insecure) Clone() credentials.TransportCredentials { return Insecure() }
-
-func (insecure) OverrideServerName(string) error { return nil }
-
-// Insecure can be used in place of transport credentials when no transport security is configured.
-// Its handshakes simply return the passed in connection.
-//
-// Similar credentials are already implemented in gRPC:
-// https://github.com/grpc/grpc-go/blob/702608ffae4d03a6821b96d3e2311973d34b96dc/credentials/insecure/insecure.go
-// We've reimplemented these here as upgrading our gRPC version was very involved. Once
-// we've upgrade to a version that contains the insecure credentials, this implementation can be removed and
-// substituted by the official implementation.
-func Insecure() credentials.TransportCredentials { return insecure{} }
diff --git a/internal/gitaly/client/dial.go b/internal/gitaly/client/dial.go
index 1f70475a0..e0990671f 100644
--- a/internal/gitaly/client/dial.go
+++ b/internal/gitaly/client/dial.go
@@ -8,12 +8,12 @@ import (
 	"net/url"
 	"time"
 
-	"gitlab.com/gitlab-org/gitaly/v14/internal/backchannel"
 	gitaly_x509 "gitlab.com/gitlab-org/gitaly/v14/internal/x509"
 	grpccorrelation "gitlab.com/gitlab-org/labkit/correlation/grpc"
 	grpctracing "gitlab.com/gitlab-org/labkit/tracing/grpc"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
+	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/keepalive"
 )
 
@@ -108,7 +108,7 @@ func Dial(ctx context.Context, rawAddress string, connOpts []grpc.DialOption, ha
 
 	if handshaker != nil {
 		if transportCredentials == nil {
-			transportCredentials = backchannel.Insecure()
+			transportCredentials = insecure.NewCredentials()
 		}
 
 		transportCredentials = handshaker.ClientHandshake(transportCredentials)
diff --git a/internal/gitaly/client/dial_test.go b/internal/gitaly/client/dial_test.go
index 87b996ceb..7db99c2ff 100644
--- a/internal/gitaly/client/dial_test.go
+++ b/internal/gitaly/client/dial_test.go
@@ -11,6 +11,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/status"
 )
 
@@ -21,7 +22,7 @@ func TestDial(t *testing.T) {
 	logger := testhelper.DiscardTestEntry(t)
 
 	srv := grpc.NewServer(
-		grpc.Creds(backchannel.NewServerHandshaker(logger, backchannel.Insecure(), backchannel.NewRegistry(), nil)),
+		grpc.Creds(backchannel.NewServerHandshaker(logger, insecure.NewCredentials(), backchannel.NewRegistry(), nil)),
 		grpc.UnknownServiceHandler(func(srv interface{}, stream grpc.ServerStream) error {
 			_, err := backchannel.GetPeerID(stream.Context())
 			if err == backchannel.ErrNonMultiplexedConnection {
diff --git a/internal/gitaly/server/server.go b/internal/gitaly/server/server.go
index ed8e3184a..a2540761b 100644
--- a/internal/gitaly/server/server.go
+++ b/internal/gitaly/server/server.go
@@ -30,6 +30,7 @@ import (
 	grpctracing "gitlab.com/gitlab-org/labkit/tracing/grpc"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
+	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/keepalive"
 )
 
@@ -78,7 +79,7 @@ func New(
 
 	lh := limithandler.New(concurrencyKeyFn)
 
-	transportCredentials := backchannel.Insecure()
+	transportCredentials := insecure.NewCredentials()
 	// If tls config is specified attempt to extract tls options and use it
 	// as a grpc.ServerOption
 	if secure {
diff --git a/internal/praefect/nodes/sql_elector_test.go b/internal/praefect/nodes/sql_elector_test.go
index ef788580c..882f744fb 100644
--- a/internal/praefect/nodes/sql_elector_test.go
+++ b/internal/praefect/nodes/sql_elector_test.go
@@ -20,6 +20,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/health"
 	"google.golang.org/grpc/health/grpc_health_v1"
 	"google.golang.org/grpc/status"
@@ -430,7 +431,7 @@ func TestConnectionMultiplexing(t *testing.T) {
 	logger := testhelper.DiscardTestEntry(t)
 
 	srv := grpc.NewServer(
-		grpc.Creds(backchannel.NewServerHandshaker(logger, backchannel.Insecure(), backchannel.NewRegistry(), nil)),
+		grpc.Creds(backchannel.NewServerHandshaker(logger, insecure.NewCredentials(), backchannel.NewRegistry(), nil)),
 		grpc.UnknownServiceHandler(func(srv interface{}, stream grpc.ServerStream) error {
 			_, err := backchannel.GetPeerID(stream.Context())
 			if err == backchannel.ErrNonMultiplexedConnection {
diff --git a/internal/praefect/server_test.go b/internal/praefect/server_test.go
index 1dc764fa7..4ce342ee0 100644
--- a/internal/praefect/server_test.go
+++ b/internal/praefect/server_test.go
@@ -46,6 +46,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/health/grpc_health_v1"
 	grpc_metadata "google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
@@ -57,7 +58,7 @@ func TestNewBackchannelServerFactory(t *testing.T) {
 	logger := testhelper.DiscardTestEntry(t)
 	registry := backchannel.NewRegistry()
 	server := grpc.NewServer(
-		grpc.Creds(backchannel.NewServerHandshaker(logger, backchannel.Insecure(), registry, nil)),
+		grpc.Creds(backchannel.NewServerHandshaker(logger, insecure.NewCredentials(), registry, nil)),
 		grpc.UnknownServiceHandler(func(srv interface{}, stream grpc.ServerStream) error {
 			id, err := backchannel.GetPeerID(stream.Context())
 			if !assert.NoError(t, err) {