diff options
author | Andrew Newdigate <andrew@gitlab.com> | 2019-01-09 17:47:26 +0300 |
---|---|---|
committer | Jacob Vosmaer <jacob@gitlab.com> | 2019-01-14 17:44:20 +0300 |
commit | 73dda503412a5bd9a5cf1ecab03550dfb140df1e (patch) | |
tree | 314adf3fbdbcae932b3742717def3a4db0e968f4 | |
parent | f39b2d6bc632e77b1772c41a781576988c23dc12 (diff) |
Adds GRPC integration tests for client.Dial
client.Dial is the recommended way for Golang Gitaly clients to dial the
Gitaly service, given a connection string. The method takes a URL which
can refer to an insecure TCP connection, a secure TLS connection or an
(insecure) Unix socket connection. Additionally, it supports several
forms of URL for some of the URLs.
Unfortunately, we've found this interface to be fragile, having broken
during refactoring and GRPC library upgrades. This change is an
attempt to build some integration tests to catch these failures early,
rather than later on in the process.
-rw-r--r-- | client/dial_test.go | 250 | ||||
-rw-r--r-- | client/testdata/gitalycert.pem | 30 | ||||
-rw-r--r-- | client/testdata/gitalykey.pem | 52 | ||||
-rw-r--r-- | cmd/gitaly-ssh/auth_test.go | 65 |
4 files changed, 381 insertions, 16 deletions
diff --git a/client/dial_test.go b/client/dial_test.go new file mode 100644 index 000000000..a7edd5598 --- /dev/null +++ b/client/dial_test.go @@ -0,0 +1,250 @@ +package client + +import ( + "context" + "crypto/tls" + "fmt" + "net" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/testhelper" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials" + healthpb "google.golang.org/grpc/health/grpc_health_v1" + "google.golang.org/grpc/status" +) + +var proxyEnvironmentKeys = []string{"http_proxy", "https_proxy", "no_proxy"} + +func doDialAndExecuteCall(addr string) error { + conn, err := Dial(addr, nil) + if err != nil { + return fmt.Errorf("dial: %v", err) + } + + client := healthpb.NewHealthClient(conn) + _, err = client.Check(context.Background(), &healthpb.HealthCheckRequest{}) + return err +} + +func TestDial(t *testing.T) { + if emitProxyWarning() { + t.Log("WARNING. Proxy configuration detected from environment settings. This test failure may be related to proxy configuration. Please process with caution") + } + + stop, connectionMap, err := startListeners() + require.NoError(t, err, "start listeners: %v. %s", err) + defer stop() + + wd, err := os.Getwd() + require.NoError(t, err, "getwd: %v. %s", err) + + unixSocketAbsPath := connectionMap["unix"] + unixSocketRelPath, err := filepath.Rel(wd, unixSocketAbsPath) + require.NoError(t, err, "relative path failure: %v. %s", err) + + tests := []struct { + name string + rawAddress string + envSSLCertFile string + expectFailure bool + }{ + { + name: "tcp localhost with prefix", + rawAddress: "tcp://localhost:" + connectionMap["tcp"], // "tcp://localhost:1234" + expectFailure: false, + }, + { + name: "tls localhost", + rawAddress: "tls://localhost:" + connectionMap["tls"], // "tls://localhost:1234" + envSSLCertFile: "./testdata/gitalycert.pem", + expectFailure: false, + }, + { + name: "unix absolute", + rawAddress: "unix:" + unixSocketAbsPath, // "unix:/tmp/temp-socket" + expectFailure: false, + }, + { + name: "unix relative", + rawAddress: "unix:" + unixSocketRelPath, // "unix:../../tmp/temp-socket" + expectFailure: false, + }, + { + name: "unix absolute does not exist", + rawAddress: "unix:" + unixSocketAbsPath + ".does_not_exist", // "unix:/tmp/temp-socket.does_not_exist" + expectFailure: true, + }, + { + name: "unix relative does not exist", + rawAddress: "unix:" + unixSocketRelPath + ".does_not_exist", // "unix:../../tmp/temp-socket.does_not_exist" + expectFailure: true, + }, + { + // Gitaly does not support connections that do not have a scheme. + name: "tcp localhost no prefix", + rawAddress: "localhost:" + connectionMap["tcp"], // "localhost:1234" + expectFailure: true, + }, + { + name: "invalid", + rawAddress: ".", + expectFailure: true, + }, + { + name: "empty", + rawAddress: "", + expectFailure: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if emitProxyWarning() { + t.Log("WARNING. Proxy configuration detected from environment settings. This test failure may be related to proxy configuration. Please process with caution") + } + + if tt.envSSLCertFile != "" { + restore := modifyEnvironment("SSL_CERT_FILE", tt.envSSLCertFile) + defer restore() + } + + err := doDialAndExecuteCall(tt.rawAddress) + if tt.expectFailure { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} + +// healthServer provide a basic GRPC health service endpoint for testing purposes +type healthServer struct { +} + +func (*healthServer) Check(context.Context, *healthpb.HealthCheckRequest) (*healthpb.HealthCheckResponse, error) { + return &healthpb.HealthCheckResponse{Status: healthpb.HealthCheckResponse_SERVING}, nil +} + +func (*healthServer) Watch(*healthpb.HealthCheckRequest, healthpb.Health_WatchServer) error { + return status.Errorf(codes.Unimplemented, "Not implemented") +} + +// startTCPListener will start a insecure TCP listener on a random unused port +func startTCPListener() (func(), string, error) { + listener, err := net.Listen("tcp", ":0") + if err != nil { + return nil, "", err + } + tcpPort := listener.Addr().(*net.TCPAddr).Port + address := fmt.Sprintf("%d", tcpPort) + + grpcServer := grpc.NewServer() + healthpb.RegisterHealthServer(grpcServer, &healthServer{}) + go grpcServer.Serve(listener) + + return func() { + grpcServer.Stop() + }, address, nil +} + +// startUnixListener will start a unix socket listener using a temporary file +func startUnixListener() (func(), string, error) { + serverSocketPath := testhelper.GetTemporaryGitalySocketFileName() + + listener, err := net.Listen("unix", serverSocketPath) + if err != nil { + return nil, "", err + } + + grpcServer := grpc.NewServer() + healthpb.RegisterHealthServer(grpcServer, &healthServer{}) + go grpcServer.Serve(listener) + + return func() { + grpcServer.Stop() + }, serverSocketPath, nil +} + +// startTLSListener will start a secure TLS listener on a random unused port +func startTLSListener() (func(), string, error) { + listener, err := net.Listen("tcp", ":0") + if err != nil { + return nil, "", err + } + tcpPort := listener.Addr().(*net.TCPAddr).Port + address := fmt.Sprintf("%d", tcpPort) + + cert, err := tls.LoadX509KeyPair("./testdata/gitalycert.pem", "./testdata/gitalykey.pem") + if err != nil { + return nil, "", err + } + + grpcServer := grpc.NewServer(grpc.Creds(credentials.NewServerTLSFromCert(&cert))) + healthpb.RegisterHealthServer(grpcServer, &healthServer{}) + go grpcServer.Serve(listener) + + return func() { + grpcServer.Stop() + }, address, nil +} + +var listeners = map[string]func() (func(), string, error){ + "tcp": startTCPListener, + "unix": startUnixListener, + "tls": startTLSListener, +} + +// startListeners will start all the different listeners used in this test +func startListeners() (func(), map[string]string, error) { + var closers []func() + connectionMap := map[string]string{} + for k, v := range listeners { + closer, address, err := v() + if err != nil { + return nil, nil, err + } + closers = append(closers, closer) + connectionMap[k] = address + } + + return func() { + for _, v := range closers { + v() + } + }, connectionMap, nil +} + +// modifyEnvironment will change an environment variable and return a func suitable +// for `defer` to change the value back. +func modifyEnvironment(key string, value string) func() { + oldValue, hasOldValue := os.LookupEnv(key) + os.Setenv(key, value) + return func() { + if hasOldValue { + os.Setenv(key, oldValue) + } else { + os.Unsetenv(key) + } + } +} + +func emitProxyWarning() bool { + for _, key := range proxyEnvironmentKeys { + value := os.Getenv(key) + if value != "" { + return true + } + value = os.Getenv(strings.ToUpper(key)) + if value != "" { + return true + } + } + return false +} diff --git a/client/testdata/gitalycert.pem b/client/testdata/gitalycert.pem new file mode 100644 index 000000000..8b1514548 --- /dev/null +++ b/client/testdata/gitalycert.pem @@ -0,0 +1,30 @@ +-----BEGIN CERTIFICATE----- +MIIFODCCAyACCQDpPfNtveVc8TANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJV +UzELMAkGA1UECAwCVVMxCzAJBgNVBAcMAlVTMQ8wDQYDVQQKDAZHaXRMYWIxDzAN +BgNVBAsMBmdpdGFseTESMBAGA1UEAwwJbG9jYWxob3N0MCAXDTE4MTEwMjA5MDIx +MloYDzIxMTgxMDA5MDkwMjEyWjBdMQswCQYDVQQGEwJVUzELMAkGA1UECAwCVVMx +CzAJBgNVBAcMAlVTMQ8wDQYDVQQKDAZHaXRMYWIxDzANBgNVBAsMBmdpdGFseTES +MBAGA1UEAwwJbG9jYWxob3N0MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKC +AgEApJXJOWpUkV32v8gRXLWn6TEsQmy2WeilQXg96V6VOQjGZAGMEJLEjH9WHBNe +Zi4V+W+j1FB8vWTNRGTcOcpSEmDFuewoBJVA8dFtNF4jj7QQymmnKeDuOW4fWLeU +YcyGxyjlpkm2+DUg5CavT4bMZILqbsAavxJ8SKCdJpMtW3sxklnGuTHcAckHldab +9ZxH/qYqLxc5Ek2BK4OibBxA84h1RUsqe2EdzZUOoet3xpwG3Vr8bGPqR7Psghs6 +TDdWU8hYYHlReCWezgZHiYDoRqY9HCZrHSpUZ1lbRo++2j4bvdFHOAUm4BEQ6fFc +sgtW+xkNK8bxj9XTcpuDrEVscv3fyBlCMSvD+HpNbr2k1oZSOFhxISIwBLKWQBjq +5muvMRbmrG5RgWqMWjXb+g0UmlyMa2YWAWsBgSuUSjJePgbUZWHuxp/dM8CQ4lHJ +ADvfSI9ysJQM/trqjRu5BRhxiKWR72QSi1qpDPT0nKWlzQ58zs3RSuOJbWm8oOqr +XL9G/XmvgzK1qwToI/WmXBeaqmfpkagYZm+TJW0GVnDqTC+EoXdFKW7aWIjlcb4p +tYoiRA/2jjq5OqeV6iKnxz7mEJQR1xDebm6+AWgFy4zyB/QvzanaUTvNiLhyBy6Q +YwXJHkNh+KrVszBlXxkARrGesXgqOznmDeErkOKDjxzQv+cCAwEAATANBgkqhkiG +9w0BAQsFAAOCAgEAk83b9wY9iwRrx5Yep3DA3xZkVu3GJcKf0tTL8apP1MzVBSUK +5tkvW2Z4D41jpZWgJDRF8/nT2lvVwvd5xQ8/oTUerFeG/ZZ+AiBagkBKl8piPHqD +cefAO8N2SKoYHV4xBeoVU6InUuJ7xu7BLF6tY3xKvx0XsjGC7B621xmq+E56dPZg +sQwekkxODbUw4NekqYFY21BT4xiWVrTRLIGY9AfV9Ry4gqQTxda7yst4ykWh1a9e +O+426uz3jshzpQTjZwk8kCZquJKa8Qzqfdlevns0FQDP5jck4BH/YkMNsa/g9XCd +ZHSB7gqAfNoNTB1rqNKIfPUF4mTu/RWMVwxb8f6h0TfywHZ4q/4R3Zfu3jUyeVVY +ziJu2CJpcoR9SESKFbN4WFzk91nIhf2pCGo/qNO5f+n5ZPnS2jrrWL5h64e1rz2h +rVKIYLfeM2M8lVzSL1V0aJ+POcruTRsmlrFT5f7na/5YFt5N+5Z5fzixCLr1MK2w +4gFw+KhN7CAhKGzHq3NBdWpRFFMR53hyeYsb1vvwFu07JTRh+NbaePk/sk07WtCo +u2w6pD7xlayTAWcR9WRBv7c3lDejN80U8DONb8fLwtI5oIrkSuwOqvmlDOeFpKiT +MwTB6oC81Ar39P0R53247w7u9plhPUrmDn/A5KphW633UvgbkH6VmB4Isiw= +-----END CERTIFICATE----- diff --git a/client/testdata/gitalykey.pem b/client/testdata/gitalykey.pem new file mode 100644 index 000000000..8df87e633 --- /dev/null +++ b/client/testdata/gitalykey.pem @@ -0,0 +1,52 @@ +-----BEGIN PRIVATE KEY----- +MIIJRAIBADANBgkqhkiG9w0BAQEFAASCCS4wggkqAgEAAoICAQCklck5alSRXfa/ +yBFctafpMSxCbLZZ6KVBeD3pXpU5CMZkAYwQksSMf1YcE15mLhX5b6PUUHy9ZM1E +ZNw5ylISYMW57CgElUDx0W00XiOPtBDKaacp4O45bh9Yt5RhzIbHKOWmSbb4NSDk +Jq9PhsxkgupuwBq/EnxIoJ0mky1bezGSWca5MdwByQeV1pv1nEf+piovFzkSTYEr +g6JsHEDziHVFSyp7YR3NlQ6h63fGnAbdWvxsY+pHs+yCGzpMN1ZTyFhgeVF4JZ7O +BkeJgOhGpj0cJmsdKlRnWVtGj77aPhu90Uc4BSbgERDp8VyyC1b7GQ0rxvGP1dNy +m4OsRWxy/d/IGUIxK8P4ek1uvaTWhlI4WHEhIjAEspZAGOrma68xFuasblGBaoxa +Ndv6DRSaXIxrZhYBawGBK5RKMl4+BtRlYe7Gn90zwJDiUckAO99Ij3KwlAz+2uqN +G7kFGHGIpZHvZBKLWqkM9PScpaXNDnzOzdFK44ltabyg6qtcv0b9ea+DMrWrBOgj +9aZcF5qqZ+mRqBhmb5MlbQZWcOpML4Shd0UpbtpYiOVxvim1iiJED/aOOrk6p5Xq +IqfHPuYQlBHXEN5ubr4BaAXLjPIH9C/NqdpRO82IuHIHLpBjBckeQ2H4qtWzMGVf +GQBGsZ6xeCo7OeYN4SuQ4oOPHNC/5wIDAQABAoICAHjlPeZa4LvXFcVSJM7A8RIt ++KDiUiBA8ALjXDbsLxiyBWi4ajZSWOYLMyl0YMcV2zZadzEh3j8QqGcw30PkBd1S +EGu9uLeFGyuF9n2dGOoaDqtgaFYuz06IQaZdUzVzkx0AQZCgXTJ9dCei8uurzL+Y +GrQ3kG4CGiEPOeB4A71LBOLH511p7n2xOU0rU2xa29eGHz5wBJAZNmTMUKaxKlS5 +S8sWp6HxeH7mmtT9rgHJ4pD+oKTNz+3TkEsRzQTnMRZh9+kFtH5YxAn6OtoaQoSC +4CipX80QpuczkASI2lxdeus3quTPg/rbDl2J2dk+0ymnATHC9PX+z09ERLhqVnoD +QBY+vIw8Opj7GB/viWm/IiF0wseM+qEgr+f1qgl8pRe4N+EeD2OCnB++kslOhol+ +50KJbMJ/bfHo/3NKCqAHoKd/gk4HAiqmEKHRgSaXXjvE6bBRoFeL20zFitzCWm8z +H3CexwDRY0qJqy80Qahg+NQX58MkYskOA42fFMzuvUxfYIu/mpTTDvRK5jxsDffe +cPU2BTcbxi5hCJjo7ertid5JGp51jr6XvViuDYf77hhw8Cm5KdeavHcF1XgksRa3 +SHTtDv/Um1RvOqMzINy1Z5mFdBN8TdzEA+9gPCm+pqpD0FTDiu/IkggYeszk0syf +AIhoEJI8PkBKqQj0DxoBAoIBAQDZ96DL+fzwXN13aqhvYNTfGxZ3RCP40KCuCSrQ +gjcGcGavFOR21Y5CHaFmIFNmrtXTj3P950N+a8/KNAmm9zFx6060HEHyR6rN5b0h +BMMlp7ezyPY+VCWJWCEi3TD+4LseAynyP05rWdm2+nsGBxaXWB8yAq0CwuyYTQdZ +IZHGKeGI9irv+5mIe7bVRCVEug22u6gHmOLERXCqO70Mzi/c/UFxjGmF0LR4d7TY +VIQ5r/PPvXJzf72PVIPwJQzRsaOXnZvD1UzSahyaB3fABB6I1y6OfXyU8BukwBQ5 +J0qVRFpzMc46PFULQC5KTUIzPcnrW73UWEu5XGiAooN5SysnAoIBAQDBTaGC/5xO +755EdZggpx+05LlmoW0ijeDnuRcKPlwap4dPFUSQS2EOWxjmm4K8sNAOdiQt7Jwf +gX/S6jdY79V0rkyktWk1uCfiwu1c/rvyLdl2Jg1RW6HByL7MTBuASfuCgJsfzhev +yu+HzPJMQNFhgQTcLL9LYHp5moGKCbJzIp+FXOeFokjjlynrrmPoHV0A8To6s7q2 +yH5qu9OaOu75kWqDul5b9cXO+isxUBZbEjUy7OEa3Zezuhq8XgZVE5t2QgJOnDI/ +NNp5d16N7GcDpaEZzSNag95F0+wsFIgT29LL2zSF6h3+VjeqKTxwbRtFsMboN9TZ +QHIgBB+2ib1BAoIBAQDC31r6ovFacJxsZIZctcT8BzrJvLkwfk356yZFLvZFIn8b +r2EnQX0jbVxcczA9kLiJoirA6V91iqxHCslKZpzlTcyayNzI4Pw7g1fZSmmyo8Vg +zp4hUZgRuCJACmQArCl/BrMc6y6QWc+FgWI2HGY9P0L8slm+K0neTJfyP0oWUmFa +00PGNTqqRHlNKNTtIi6aniH3UOAFPFQjTq+R4FH4kNBO1YuOYO7I+bVM6BsjfEVO +CQFnc+ClYZloPae9XsV1Cys1JeG+CbKyn1SX7tbh3wi3ykd03UrJvBUYmCFdXLRF +Y1UOydv66BG6ymISb/60FtycGajx+0VPJHzJF8RnAoIBAQCVVyyY0HIqaeWUbmWB +lJxiXPL/32c5cvN3EwBB4bu2vAdFieDWueXZ+Xdbcnmm3dNf2NZKxKo5jQr8IAdy +ppf69U4xUhZeclAeWQqY9hSuHc4MAYn4eRqXZEhD/eihTIcLY+B0yfxyzA4SlLv9 +PXaGJe9jSw7fZUI6AKxjwOolGXK0zfnwvFgjvP2eH7T/9u+LctLR11lBLdS9ES+B +0FYgacAo1SthUJfqOEx2ZLFg2shO98NRxjEVoYpWTS4HPIa27nhp0zLesi63+QkM +DL/piWTVUi8mFwr6V6f2xkX7UbGh3VDOxPk3LdUDmaggE6smRFTnw3ql/awuIAGA +PRoBAoIBAQCwIkbP/Py8qXkrUil/ZW2+7yPXy4DeOkduLBrLBXH5fxAPUoyOywjl +CFOVcHNuioRZnN22M64VzlCgRhY4gD+ypyeFmVR4fHBWZuQObwt6jkaGXxvcGl+U +cDrMYt1xjJbjEdvX4+VjkLwJzIAzBG09agk3eLwcVAH8w5uteyKNdi0Kg9mClFJm +LRJNjI6fp5KfVitMEthx9WEe5Zu9phBLCtqYNYQoH+VY3yp8aD9NB0X5sgHYKCaK +jgQUpEnGU9zSnKeK8MglhWson3a6NEjPufsAjHgGHbTAfGEXLkiHZee3gAB2BJdk +eM9aMpgdAlLOJrfZHS3kK3968ZclB4GB +-----END PRIVATE KEY----- diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go index 432c8f207..33f76ef21 100644 --- a/cmd/gitaly-ssh/auth_test.go +++ b/cmd/gitaly-ssh/auth_test.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path" + "path/filepath" "strconv" "strings" "testing" @@ -44,6 +45,9 @@ func TestConnectivity(t *testing.T) { socketPath := testhelper.GetTemporaryGitalySocketFileName() + relativeSocketPath, err := filepath.Rel(cwd, socketPath) + require.NoError(t, err, "relative path failure: %v. %s", err) + tcpServer, tcpPort := runServer(t, server.NewInsecure, "tcp", "localhost:0") defer tcpServer.Stop() @@ -54,15 +58,35 @@ func TestConnectivity(t *testing.T) { defer unixServer.Stop() testCases := []struct { - addr string + name string + addr string + proxy bool }{ { + name: "tcp", addr: fmt.Sprintf("tcp://localhost:%d", tcpPort), }, { - addr: fmt.Sprintf("unix://%s", socketPath), + name: "unix absolute", + addr: fmt.Sprintf("unix:%s", socketPath), + }, + { + name: "unix abs with proxy", + addr: fmt.Sprintf("unix:%s", socketPath), + proxy: true, + }, + { + name: "unix relative", + addr: fmt.Sprintf("unix:%s", relativeSocketPath), + }, + { + name: "unix relative with proxy", + addr: fmt.Sprintf("unix:%s", relativeSocketPath), + proxy: true, }, + { + name: "tls", addr: fmt.Sprintf("tls://localhost:%d", tlsPort), }, } @@ -74,20 +98,29 @@ func TestConnectivity(t *testing.T) { require.NoError(t, err) for _, testcase := range testCases { - cmd := exec.Command("git", "ls-remote", "git@localhost:test/test.git", "refs/heads/master") - cmd.Stderr = os.Stderr - cmd.Env = []string{ - fmt.Sprintf("GITALY_PAYLOAD=%s", payload), - fmt.Sprintf("GITALY_ADDRESS=%s", testcase.addr), - fmt.Sprintf("PATH=.:%s", os.Getenv("PATH")), - fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", gitalySSHPath), - fmt.Sprintf("SSL_CERT_DIR=%s", certPoolPath), - } - - output, err := cmd.Output() - - require.NoError(t, err, "git ls-remote exit status") - require.True(t, strings.HasSuffix(strings.TrimSpace(string(output)), "refs/heads/master")) + t.Run(testcase.name, func(t *testing.T) { + cmd := exec.Command("git", "ls-remote", "git@localhost:test/test.git", "refs/heads/master") + cmd.Stderr = os.Stderr + cmd.Env = []string{ + fmt.Sprintf("GITALY_PAYLOAD=%s", payload), + fmt.Sprintf("GITALY_ADDRESS=%s", testcase.addr), + fmt.Sprintf("PATH=.:%s", os.Getenv("PATH")), + fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", gitalySSHPath), + fmt.Sprintf("SSL_CERT_DIR=%s", certPoolPath), + } + + if testcase.proxy { + cmd.Env = append(cmd.Env, + "http_proxy=http://invalid:1234", + "https_proxy=https://invalid:1234", + ) + } + + output, err := cmd.Output() + + require.NoError(t, err, "git ls-remote exit status") + require.True(t, strings.HasSuffix(strings.TrimSpace(string(output)), "refs/heads/master")) + }) } } |