Bring back a custom dialler for Gitaly Ruby

This is a second fix for the GRPC bug related to using unix sockets
alongside unix sockets. Previously we fixed incoming clients. This
change fixes the connection between Gitaly and Gitaly-Ruby, which
always relies on unix sockets, and which currently fail when http
proxies are setup.

A (too) long history of this change:

* Unix sockets have never been officially supported by gRPC
* Originally unix sockets didn't work at all, and you had to use a
  dialer
* Attempted to upgrade from gRPC 1.9 to 1.16 and found that our dialer
  broke connectivity:
  https://gitlab.com/gitlab-org/gitaly/merge_requests/972
* Isolated the problem to this commit:
  https://github.com/grpc/grpc-go/commit/90dca43332f6cc944c37e16f32a82c41639e7705
* Fixed the problem by switching to the default dialer for unix: URLs
* (note that it's still not officially supported, but without
  integration tests with proxy configurations, it seems to work)
* In #1447, discovered that the default dialer breaks if one has http
  proxies configured (there was a also a second failure in
  https://gitlab.com/gitlab-org/gitaly/merge_requests/1032 which
  involved clients talking to Gitaly)
* This change partially reverts
  https://gitlab.com/gitlab-org/gitaly/merge_requests/972 while keeping
  the fix for the original connectivity issue

4 files changed, 24 insertions, 3 deletions

diff --git a/changelogs/unreleased/an-fix-gitaly-ruby-proxy-issues.yml b/changelogs/unreleased/an-fix-gitaly-ruby-proxy-issues.yml
new file mode 100644
index 000000000..0ea487a21
--- /dev/null
+++ b/changelogs/unreleased/an-fix-gitaly-ruby-proxy-issues.yml
@@ -0,0 +1,5 @@
+---
+title: Bring back a custom dialer for Gitaly Ruby
+merge_request: 1072
+author:
+type: fixed
diff --git a/client/dial.go b/client/dial.go
index fe4a3e683..1874cc6e1 100644
--- a/client/dial.go
+++ b/client/dial.go
@@ -59,6 +59,9 @@ func Dial(rawAddress string, connOpts []grpc.DialOption) (*grpc.ClientConn, erro
 		connOpts = append(
 			connOpts,
 			grpc.WithInsecure(),
+			// 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.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) {
 				path, err := extractPathFromSocketURL(addr)
 				if err != nil {
diff --git a/internal/rubyserver/health.go b/internal/rubyserver/health.go
index cc6005f89..ffab0c779 100644
--- a/internal/rubyserver/health.go
+++ b/internal/rubyserver/health.go
@@ -2,6 +2,7 @@ package rubyserver
 
 import (
 	"context"
+	"net"
 	"time"
 
 	"google.golang.org/grpc"
@@ -12,6 +13,12 @@ func ping(address string) error {
 	conn, err := grpc.Dial(
 		address,
 		grpc.WithInsecure(),
+		// 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.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) {
+			return net.DialTimeout("unix", addr, timeout)
+		}),
 	)
 	if err != nil {
 		return err
diff --git a/internal/rubyserver/rubyserver.go b/internal/rubyserver/rubyserver.go
index 8eab03025..615444287 100644
--- a/internal/rubyserver/rubyserver.go
+++ b/internal/rubyserver/rubyserver.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
+	"net"
 	"os"
 	"path"
 	"path/filepath"
@@ -128,7 +129,6 @@ func Start() (*Server, error) {
 	for i := 0; i < numWorkers; i++ {
 		name := fmt.Sprintf("gitaly-ruby.%d", i)
 		socketPath := socketPath(i)
-		address := "unix://" + socketPath
 
 		// Use 'ruby-cd' to make sure gitaly-ruby has the same working directory
 		// as the current process. This is a hack to sort-of support relative
@@ -136,13 +136,13 @@ func Start() (*Server, error) {
 		args := []string{"bundle", "exec", "bin/ruby-cd", wd, gitalyRuby, strconv.Itoa(os.Getpid()), socketPath}
 
 		events := make(chan supervisor.Event)
-		check := func() error { return ping(address) }
+		check := func() error { return ping(socketPath) }
 		p, err := supervisor.New(name, env, args, cfg.Ruby.Dir, cfg.Ruby.MaxRSS, events, check)
 		if err != nil {
 			return nil, err
 		}
 
-		s.workers = append(s.workers, newWorker(p, address, events, false))
+		s.workers = append(s.workers, newWorker(p, socketPath, events, false))
 	}
 
 	return s, nil
@@ -256,6 +256,12 @@ func dialOptions() []grpc.DialOption {
 	return []grpc.DialOption{
 		grpc.WithBlock(), // With this we get retries. Without, connections fail fast.
 		grpc.WithInsecure(),
+		// 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.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) {
+			return net.DialTimeout("unix", addr, timeout)
+		}),
 		grpc.WithUnaryInterceptor(
 			grpc_middleware.ChainUnaryClient(
 				grpc_prometheus.UnaryClientInterceptor,