move internals of client into an internal package

Praefect requires a more flexible API to plug in its multiplexing
implementation to when dialing to Gitaly. This implementation should
not be part of the public API interface so we don't have to maintain
backwards compatibility. This commit moves the internals of the dialing
into an internal subpackage. This way we can retain the exisiting public
APIs we offer in `client` package and provide more flexible APIs for
internal use in `ìnternal/gitaly/client`.

1 files changed, 15 insertions, 132 deletions

diff --git a/client/dial.go b/client/dial.go
index 613535a5f..214f69e1e 100644
--- a/client/dial.go
+++ b/client/dial.go
@@ -2,156 +2,39 @@ package client
 
 import (
 	"context"
-	"crypto/tls"
-	"fmt"
-	"net"
-	"net/url"
-	"time"
 
 	"github.com/sirupsen/logrus"
-	"gitlab.com/gitlab-org/gitaly/internal/backchannel"
-	gitaly_x509 "gitlab.com/gitlab-org/gitaly/internal/x509"
-	grpccorrelation "gitlab.com/gitlab-org/labkit/correlation/grpc"
-	grpctracing "gitlab.com/gitlab-org/labkit/tracing/grpc"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/client"
 	"google.golang.org/grpc"
-	"google.golang.org/grpc/credentials"
 	healthpb "google.golang.org/grpc/health/grpc_health_v1"
-	"google.golang.org/grpc/keepalive"
 )
 
 // DefaultDialOpts hold the default DialOptions for connection to Gitaly over UNIX-socket
 var DefaultDialOpts = []grpc.DialOption{}
 
-type connectionType int
-
-const (
-	invalidConnection connectionType = iota
-	tcpConnection
-	tlsConnection
-	unixConnection
-)
-
+// DialContext dials the Gitaly at the given address with the provided options. Valid address formats are
+// 'unix:<socket path>' for Unix sockets, 'tcp://<host:port>' for insecure TCP connections and 'tls://<host:port>'
+// for TCP+TLS connections.
+//
+// The returned ClientConns are configured with tracing and correlation id interceptors to ensure they are propagated
+// correctly. They're also configured to send Keepalives with settings matching what Gitaly expects.
+//
+// connOpts should not contain `grpc.WithInsecure` as DialContext determines whether it is needed or not from the
+// scheme. `grpc.TransportCredentials` should not be provided either as those are handled internally as well.
 func DialContext(ctx context.Context, rawAddress string, connOpts []grpc.DialOption) (*grpc.ClientConn, error) {
-	return dialContext(ctx, rawAddress, connOpts, false, nil)
+	return client.Dial(ctx, rawAddress, connOpts, false, nil)
 }
 
+// Dial calls DialContext with the provided arguments and context.Background. Refer to DialContext's documentation
+// for details.
 func Dial(rawAddress string, connOpts []grpc.DialOption) (*grpc.ClientConn, error) {
-	return dialContext(context.Background(), rawAddress, connOpts, false, nil)
+	return DialContext(context.Background(), rawAddress, connOpts)
 }
 
 // DialWithMux dials with a multiplexed connection to Gitaly. Experimental, this is going to be removed and
 // should not be depended upon.
 func DialWithMux(ctx context.Context, rawAddress string, connOpts []grpc.DialOption, logger *logrus.Entry) (*grpc.ClientConn, error) {
-	return dialContext(ctx, rawAddress, connOpts, true, logger)
-}
-
-func dialContext(ctx context.Context, rawAddress string, connOpts []grpc.DialOption, muxed bool, logger *logrus.Entry) (*grpc.ClientConn, error) {
-	var canonicalAddress string
-	var err error
-	var transportCredentials credentials.TransportCredentials
-
-	switch getConnectionType(rawAddress) {
-	case invalidConnection:
-		return nil, fmt.Errorf("invalid connection string: %q", rawAddress)
-
-	case tlsConnection:
-		canonicalAddress, err = extractHostFromRemoteURL(rawAddress) // Ensure the form: "host:port" ...
-		if err != nil {
-			return nil, fmt.Errorf("failed to extract host for 'tls' connection: %w", err)
-		}
-
-		certPool, err := gitaly_x509.SystemCertPool()
-		if err != nil {
-			return nil, fmt.Errorf("failed to get system certificat pool for 'tls' connection: %w", err)
-		}
-
-		transportCredentials = credentials.NewTLS(&tls.Config{
-			RootCAs:    certPool,
-			MinVersion: tls.VersionTLS12,
-		})
-
-	case tcpConnection:
-		canonicalAddress, err = extractHostFromRemoteURL(rawAddress) // Ensure the form: "host:port" ...
-		if err != nil {
-			return nil, fmt.Errorf("failed to extract host for 'tcp' connection: %w", err)
-		}
-
-	case unixConnection:
-		canonicalAddress = rawAddress // This will be overridden by the custom dialer...
-		connOpts = append(
-			connOpts,
-			// Use a custom dialer to ensure that we don't experience
-			// issues in environments that have proxy configurations
-			// https://gitlab.com/gitlab-org/gitaly/merge_requests/1072#note_140408512
-			grpc.WithContextDialer(func(ctx context.Context, addr string) (conn net.Conn, err error) {
-				path, err := extractPathFromSocketURL(addr)
-				if err != nil {
-					return nil, fmt.Errorf("failed to extract host for 'unix' connection: %w", err)
-				}
-
-				d := net.Dialer{}
-				return d.DialContext(ctx, "unix", path)
-			}),
-		)
-	}
-
-	if muxed {
-		if transportCredentials == nil {
-			transportCredentials = backchannel.Insecure()
-		}
-
-		transportCredentials = backchannel.ServerFactory(
-			func() backchannel.Server { return grpc.NewServer() },
-		).ClientHandshaker(logger, transportCredentials)
-	}
-
-	if transportCredentials == nil {
-		connOpts = append(connOpts, grpc.WithInsecure())
-	} else {
-		connOpts = append(connOpts, grpc.WithTransportCredentials(transportCredentials))
-	}
-
-	connOpts = append(connOpts,
-		// grpc.KeepaliveParams must be specified at least as large as what is allowed by the
-		// server-side grpc.KeepaliveEnforcementPolicy
-		grpc.WithKeepaliveParams(keepalive.ClientParameters{
-			Time:                20 * time.Second,
-			PermitWithoutStream: true,
-		}),
-		grpc.WithChainUnaryInterceptor(
-			grpctracing.UnaryClientTracingInterceptor(),
-			grpccorrelation.UnaryClientCorrelationInterceptor(),
-		),
-		grpc.WithChainStreamInterceptor(
-			grpctracing.StreamClientTracingInterceptor(),
-			grpccorrelation.StreamClientCorrelationInterceptor(),
-		),
-	)
-
-	conn, err := grpc.DialContext(ctx, canonicalAddress, connOpts...)
-	if err != nil {
-		return nil, fmt.Errorf("failed to dial %q connection: %w", canonicalAddress, err)
-	}
-
-	return conn, nil
-}
-
-func getConnectionType(rawAddress string) connectionType {
-	u, err := url.Parse(rawAddress)
-	if err != nil {
-		return invalidConnection
-	}
-
-	switch u.Scheme {
-	case "tls":
-		return tlsConnection
-	case "unix":
-		return unixConnection
-	case "tcp":
-		return tcpConnection
-	default:
-		return invalidConnection
-	}
+	return client.Dial(ctx, rawAddress, connOpts, true, logger)
 }
 
 // FailOnNonTempDialError helps to identify if remote listener is ready to accept new connections.