grpc: Enable dns lookup by default with NewClient

Gitaly uses the deprecated grpc.DialContext [1] function to instantiate new client connections. This function relies on being provided a target address starting with the dns: scheme in order to configure DNS resolution.

Before we provide the target address, we perform some formatting depending on the scheme provided by the user [2]. Gitaly addresses starting with dns: remain as-is, whereas addresses using the tcp: and tls: schemes are stripped of the scheme.

For example:

  • dns://mygitalynode.test remains as so. This represents a non-TLS connection.
  • tls://127.0.0.1:1234 is transformed to 127.0.0.1:1234.
  • tls://mygitalynode.test:1234 is transformed in a similar way.

This transformed address is then provided to grpc.DialContext(). When the dns: prefixed address abovce is provided, it will cause the DNS resolver to be configured and used to resolve mygitalynode.test to the IP of the node. When the second, non-prefixed IP is provided, no DNS resolver is configured, and it's in fact not needed. When the third non-prefixed address is provided, despite requiring DNS resolution, no DNS resolver is configured due to the lack of the dns: scheme.

The problem is that while plain TCP connections to a hostname can be configured via the dns: scheme, TLS connections to a hostname cannot, since that requires using the tls: prefix instead of dns:.

The new grpc.NewClient() constructor resolves this problem by enabling DNS resolution by default. It's no longer required to use the dns: prefix; you can simply provide a tls: prefixed address with a hostname. For example, tls://mygitalynode.test:1234 transforms into mygitalynode.test:1234 before being provided to grpc.NewClient(). Despite the lack of an explicit dns: prefix, the DNS resolver is still used, and the IP will be resolved correctly.

The one caveat is that addresses containing a DNS authority must still use the dns: prefix. tls://mydnsserver.test/mygitalynode.test:1234 is invalid for example.

Change the invocation of grpc.DialContext to grpc.NewClient. We wrap a lot of the gRPC connection code, so there's only one place we need to update the call.

Also add missing integration tests to the gitaly-ssh command to exercise the various schemes we support.

[1] https://pkg.go.dev/google.golang.org/grpc#DialContext [2] https://gitlab.com/gitlab-org/gitaly/-/blob/922659f937508f6e92c5d0f5a2f456bbf866c6d6/internal/grpc/client/dial.go#L113

Edited by James Liu

Merge request reports

Loading