SSH Upload/ReceivePack Integration Tests

14 files changed, 633 insertions, 20 deletions

diff --git a/.gitignore b/.gitignore
index 0aba6d488..e2f23dc67 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,3 +6,4 @@
 /internal/service/smarthttp/testdata
 /internal/testhelper/testdata
 /config.toml
+/internal/service/ssh/gitaly-*-pack
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 44e46fc33..3112c7a2b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Gitaly changelog
 
+UNRELEASED
+
+- Added integration test for SSH, and a client package
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/178/
+
 v0.13.0
 
 - Added usage and version flags to the command line interface
diff --git a/client/dial.go b/client/dial.go
new file mode 100644
index 000000000..87e288da7
--- /dev/null
+++ b/client/dial.go
@@ -0,0 +1,61 @@
+package client
+
+import (
+	"fmt"
+	"net"
+	"net/url"
+	"strings"
+	"time"
+
+	"google.golang.org/grpc"
+)
+
+// DefaultDialOpts hold the default DialOptions for connection to Gitaly over UNIX-socket
+var DefaultDialOpts = []grpc.DialOption{
+	grpc.WithInsecure(),
+}
+
+// Dial gitaly
+func Dial(rawAddress string, connOpts []grpc.DialOption) (*grpc.ClientConn, error) {
+	network, addr, err := parseAddress(rawAddress)
+	if err != nil {
+		return nil, err
+	}
+
+	connOpts = append(connOpts,
+		grpc.WithDialer(func(a string, _ time.Duration) (net.Conn, error) {
+			return net.Dial(network, a)
+		}))
+	conn, err := grpc.Dial(addr, connOpts...)
+	if err != nil {
+		return nil, err
+	}
+
+	return conn, nil
+}
+
+func parseAddress(rawAddress string) (network, addr string, err error) {
+	// Parsing unix:// URL's with url.Parse does not give the result we want
+	// so we do it manually.
+	for _, prefix := range []string{"unix://", "unix:"} {
+		if strings.HasPrefix(rawAddress, prefix) {
+			return "unix", strings.TrimPrefix(rawAddress, prefix), nil
+		}
+	}
+
+	u, err := url.Parse(rawAddress)
+	if err != nil {
+		return "", "", err
+	}
+
+	if u.Scheme != "tcp" {
+		return "", "", fmt.Errorf("unknown scheme: %q", rawAddress)
+	}
+	if u.Host == "" {
+		return "", "", fmt.Errorf("network tcp requires host: %q", rawAddress)
+	}
+	if u.Path != "" {
+		return "", "", fmt.Errorf("network tcp should have no path: %q", rawAddress)
+	}
+	return "tcp", u.Host, nil
+}
diff --git a/client/dial_test.go b/client/dial_test.go
new file mode 100644
index 000000000..d0513fc81
--- /dev/null
+++ b/client/dial_test.go
@@ -0,0 +1,50 @@
+package client
+
+import (
+	"testing"
+)
+
+func TestParseAddress(t *testing.T) {
+	testCases := []struct {
+		raw     string
+		network string
+		addr    string
+		invalid bool
+	}{
+		{raw: "unix:/foo/bar.socket", network: "unix", addr: "/foo/bar.socket"},
+		{raw: "unix:///foo/bar.socket", network: "unix", addr: "/foo/bar.socket"},
+		// Mainly for test purposes we explicitly want to support relative paths
+		{raw: "unix://foo/bar.socket", network: "unix", addr: "foo/bar.socket"},
+		{raw: "unix:foo/bar.socket", network: "unix", addr: "foo/bar.socket"},
+		{raw: "tcp://1.2.3.4", network: "tcp", addr: "1.2.3.4"},
+		{raw: "tcp://1.2.3.4:567", network: "tcp", addr: "1.2.3.4:567"},
+		{raw: "tcp://foobar", network: "tcp", addr: "foobar"},
+		{raw: "tcp://foobar:567", network: "tcp", addr: "foobar:567"},
+		{raw: "tcp://1.2.3.4/foo/bar.socket", invalid: true},
+		{raw: "tcp:///foo/bar.socket", invalid: true},
+		{raw: "tcp:/foo/bar.socket", invalid: true},
+	}
+
+	for _, tc := range testCases {
+		network, addr, err := parseAddress(tc.raw)
+
+		if err == nil && tc.invalid {
+			t.Errorf("%v: expected error, got none", tc)
+		} else if err != nil && !tc.invalid {
+			t.Errorf("%v: parse error: %v", tc, err)
+			continue
+		}
+
+		if tc.invalid {
+			continue
+		}
+
+		if tc.network != network {
+			t.Errorf("%v: expected %q, got %q", tc, tc.network, network)
+		}
+
+		if tc.addr != addr {
+			t.Errorf("%v: expected %q, got %q", tc, tc.addr, addr)
+		}
+	}
+}
diff --git a/client/receive_pack.go b/client/receive_pack.go
new file mode 100644
index 000000000..39ba7ae5e
--- /dev/null
+++ b/client/receive_pack.go
@@ -0,0 +1,40 @@
+package client
+
+import (
+	"io"
+
+	"google.golang.org/grpc"
+
+	"golang.org/x/net/context"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	pbhelper "gitlab.com/gitlab-org/gitaly-proto/go/helper"
+)
+
+// ReceivePack proxies an SSH git-receive-pack (git push) session to Gitaly
+func ReceivePack(ctx context.Context, conn *grpc.ClientConn, stdin io.Reader, stdout, stderr io.Writer, req *pb.SSHReceivePackRequest) (int32, error) {
+	ctx2, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	ssh := pb.NewSSHClient(conn)
+	stream, err := ssh.SSHReceivePack(ctx2)
+	if err != nil {
+		return 0, err
+	}
+
+	if err = stream.Send(req); err != nil {
+		return 0, err
+	}
+
+	inWriter := pbhelper.NewSendWriter(func(p []byte) error {
+		return stream.Send(&pb.SSHReceivePackRequest{Stdin: p})
+	})
+
+	return streamHandler(func() (stdoutStderrResponse, error) {
+		return stream.Recv()
+	}, func(errC chan error) {
+		_, errRecv := io.Copy(inWriter, stdin)
+		stream.CloseSend()
+		errC <- errRecv
+	}, stdout, stderr)
+}
diff --git a/client/std_stream.go b/client/std_stream.go
new file mode 100644
index 000000000..83d2589bf
--- /dev/null
+++ b/client/std_stream.go
@@ -0,0 +1,66 @@
+package client
+
+import (
+	"fmt"
+	"io"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+)
+
+type stdoutStderrResponse interface {
+	GetExitStatus() *pb.ExitStatus
+	GetStderr() []byte
+	GetStdout() []byte
+}
+
+func streamHandler(recv func() (stdoutStderrResponse, error), send func(chan error), stdout, stderr io.Writer) (int32, error) {
+	var (
+		exitStatus int32
+		err        error
+		resp       stdoutStderrResponse
+	)
+
+	errC := make(chan error, 1)
+
+	go send(errC)
+
+	for {
+		resp, err = recv()
+		if err != nil {
+			break
+		}
+		if resp.GetExitStatus() != nil {
+			exitStatus = resp.GetExitStatus().GetValue()
+		}
+
+		if len(resp.GetStderr()) > 0 {
+			if _, err = stderr.Write(resp.GetStderr()); err != nil {
+				break
+			}
+		}
+
+		if len(resp.GetStdout()) > 0 {
+			if _, err = stdout.Write(resp.GetStdout()); err != nil {
+				break
+			}
+		}
+	}
+	if err == io.EOF {
+		err = nil
+	}
+
+	if err != nil {
+		return exitStatus, err
+	}
+
+	select {
+	case errSend := <-errC:
+		if errSend != nil {
+			// This should not happen
+			errSend = fmt.Errorf("stdin send error: %v", errSend)
+		}
+		return exitStatus, errSend
+	default:
+		return exitStatus, nil
+	}
+}
diff --git a/client/upload_pack.go b/client/upload_pack.go
new file mode 100644
index 000000000..eb0fc7d92
--- /dev/null
+++ b/client/upload_pack.go
@@ -0,0 +1,40 @@
+package client
+
+import (
+	"io"
+
+	"google.golang.org/grpc"
+
+	"golang.org/x/net/context"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	pbhelper "gitlab.com/gitlab-org/gitaly-proto/go/helper"
+)
+
+// UploadPack proxies an SSH git-upload-pack (git fetch) session to Gitaly
+func UploadPack(ctx context.Context, conn *grpc.ClientConn, stdin io.Reader, stdout, stderr io.Writer, req *pb.SSHUploadPackRequest) (int32, error) {
+	ctx2, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	ssh := pb.NewSSHClient(conn)
+	stream, err := ssh.SSHUploadPack(ctx2)
+	if err != nil {
+		return 0, err
+	}
+
+	if err = stream.Send(req); err != nil {
+		return 0, err
+	}
+
+	inWriter := pbhelper.NewSendWriter(func(p []byte) error {
+		return stream.Send(&pb.SSHUploadPackRequest{Stdin: p})
+	})
+
+	return streamHandler(func() (stdoutStderrResponse, error) {
+		return stream.Recv()
+	}, func(errC chan error) {
+		_, errRecv := io.Copy(inWriter, stdin)
+		stream.CloseSend()
+		errC <- errRecv
+	}, stdout, stderr)
+}
diff --git a/internal/service/ssh/cmd/gitaly-receive-pack/main.go b/internal/service/ssh/cmd/gitaly-receive-pack/main.go
new file mode 100644
index 000000000..104700cd7
--- /dev/null
+++ b/internal/service/ssh/cmd/gitaly-receive-pack/main.go
@@ -0,0 +1,48 @@
+package main
+
+import (
+	"log"
+	"os"
+	"strings"
+	"time"
+
+	"golang.org/x/net/context"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/client"
+)
+
+func main() {
+	if !(len(os.Args) >= 3 && strings.HasPrefix(os.Args[2], "git-receive-pack")) {
+		log.Fatalf("Not a valid command")
+	}
+
+	addr := os.Getenv("GITALY_SOCKET")
+	if len(addr) == 0 {
+		log.Fatalf("GITALY_SOCKET not set")
+	}
+
+	conn, err := client.Dial(addr, client.DefaultDialOpts)
+	if err != nil {
+		log.Fatalf("Error: %v", err)
+	}
+	defer conn.Close()
+
+	req := &pb.SSHReceivePackRequest{
+		Repository: &pb.Repository{
+			RelativePath: os.Getenv("GL_RELATIVEPATH"),
+			StorageName:  os.Getenv("GL_STORAGENAME"),
+		},
+		GlRepository: os.Getenv("GL_REPOSITORY"),
+		GlId:         os.Getenv("GL_ID"),
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	code, err := client.ReceivePack(ctx, conn, os.Stdin, os.Stdout, os.Stderr, req)
+	if err != nil {
+		log.Fatalf("Error: %v", err)
+	}
+
+	os.Exit(int(code))
+}
diff --git a/internal/service/ssh/cmd/gitaly-upload-pack/main.go b/internal/service/ssh/cmd/gitaly-upload-pack/main.go
new file mode 100644
index 000000000..ccf55d23d
--- /dev/null
+++ b/internal/service/ssh/cmd/gitaly-upload-pack/main.go
@@ -0,0 +1,46 @@
+package main
+
+import (
+	"log"
+	"os"
+	"strings"
+	"time"
+
+	"golang.org/x/net/context"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/client"
+)
+
+func main() {
+	if !(len(os.Args) >= 3 && strings.HasPrefix(os.Args[2], "git-upload-pack")) {
+		log.Fatalf("Not a valid command")
+	}
+
+	addr := os.Getenv("GITALY_SOCKET")
+	if len(addr) == 0 {
+		log.Fatalf("GITALY_SOCKET not set")
+	}
+
+	conn, err := client.Dial(addr, client.DefaultDialOpts)
+	if err != nil {
+		log.Fatalf("Error: %v", err)
+	}
+	defer conn.Close()
+
+	req := &pb.SSHUploadPackRequest{
+		Repository: &pb.Repository{
+			RelativePath: os.Getenv("GL_RELATIVEPATH"),
+			StorageName:  os.Getenv("GL_STORAGENAME"),
+		},
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	code, err := client.UploadPack(ctx, conn, os.Stdin, os.Stdout, os.Stderr, req)
+	if err != nil {
+		log.Fatalf("Error: %v", err)
+	}
+
+	os.Exit(int(code))
+}
diff --git a/internal/service/ssh/receive_pack_test.go b/internal/service/ssh/receive_pack_test.go
index 9a571563f..8069d4e5d 100644
--- a/internal/service/ssh/receive_pack_test.go
+++ b/internal/service/ssh/receive_pack_test.go
@@ -1,7 +1,15 @@
 package ssh
 
 import (
+	"bytes"
+	"fmt"
+	"os"
+	"os/exec"
+	"path"
+	"path/filepath"
+	"strings"
 	"testing"
+	"time"
 
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 
@@ -17,30 +25,148 @@ func TestFailedReceivePackRequestDueToValidationError(t *testing.T) {
 
 	client := newSSHClient(t)
 
-	rpcRequests := []pb.SSHReceivePackRequest{
-		{Repository: &pb.Repository{StorageName: "default", RelativePath: ""}, GlId: "user-123"},                                    // Repository.RelativePath is empty
-		{Repository: nil, GlId: "user-123"},                                                                                         // Repository is nil
-		{Repository: &pb.Repository{StorageName: "default", RelativePath: "path/to/repo"}, GlId: ""},                                // Empty GlId
-		{Repository: &pb.Repository{StorageName: "default", RelativePath: "path/to/repo"}, GlId: "user-123", Stdin: []byte("Fail")}, // Data exists on first request
+	tests := []struct {
+		Req  *pb.SSHReceivePackRequest
+		Code codes.Code
+	}{
+		{ // Repository.RelativePath is empty
+			Req:  &pb.SSHReceivePackRequest{Repository: &pb.Repository{StorageName: "default", RelativePath: ""}, GlId: "user-123"},
+			Code: codes.NotFound,
+		},
+		{ // Repository is nil
+			Req:  &pb.SSHReceivePackRequest{Repository: nil, GlId: "user-123"},
+			Code: codes.InvalidArgument,
+		},
+		{ // Empty GlId
+			Req:  &pb.SSHReceivePackRequest{Repository: &pb.Repository{StorageName: "default", RelativePath: "path/to/repo"}, GlId: ""},
+			Code: codes.InvalidArgument,
+		},
+		{ // Data exists on first request
+			Req:  &pb.SSHReceivePackRequest{Repository: &pb.Repository{StorageName: "default", RelativePath: "path/to/repo"}, GlId: "user-123", Stdin: []byte("Fail")},
+			Code: codes.InvalidArgument,
+		},
 	}
 
-	for _, rpcRequest := range rpcRequests {
-		t.Logf("test case: %v", rpcRequest)
+	for _, test := range tests {
+		t.Logf("test case: %v", test.Req)
 		stream, err := client.SSHReceivePack(context.Background())
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		if err = stream.Send(&rpcRequest); err != nil {
+		if err = stream.Send(test.Req); err != nil {
 			t.Fatal(err)
 		}
 		stream.CloseSend()
 
 		err = drainPostReceivePackResponse(stream)
-		testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+		testhelper.AssertGrpcError(t, err, test.Code, "")
 	}
 }
 
+func TestReceivePackPushSuccess(t *testing.T) {
+	server := runSSHServer(t)
+	defer server.Stop()
+
+	lHead, rHead, err := testCloneAndPush(t, testRepo.GetStorageName(), "1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Compare(lHead, rHead) != 0 {
+		t.Errorf("local and remote head not equal. push failed: %q != %q", lHead, rHead)
+	}
+}
+
+func TestReceivePackPushFailure(t *testing.T) {
+	server := runSSHServer(t)
+	defer server.Stop()
+
+	_, _, err := testCloneAndPush(t, "foobar", "1")
+	if err == nil {
+		t.Errorf("local and remote head equal. push did not fail")
+	}
+	_, _, err = testCloneAndPush(t, testRepo.GetStorageName(), "")
+	if err == nil {
+		t.Errorf("local and remote head equal. push did not fail")
+	}
+}
+
+func testCloneAndPush(t *testing.T, storageName, glID string) (string, string, error) {
+	storagePath := testhelper.GitlabTestStoragePath()
+	tempRepo := "gitlab-test-ssh-receive-pack.git"
+	testRepoPath := path.Join(storagePath, testRepo.GetRelativePath())
+	remoteRepoPath := path.Join(storagePath, tempRepo)
+	localRepoPath := path.Join(storagePath, "gitlab-test-ssh-receive-pack-local")
+	// Make a bare clone of the test repo to act as a remote one and to leave the original repo intact for other tests
+	if err := os.RemoveAll(remoteRepoPath); err != nil && !os.IsNotExist(err) {
+		t.Fatal(err)
+	}
+	testhelper.MustRunCommand(t, nil, "git", "clone", "--bare", testRepoPath, remoteRepoPath)
+	// Make a non-bare clone of the test repo to act as a local one
+	if err := os.RemoveAll(localRepoPath); err != nil && !os.IsNotExist(err) {
+		t.Fatal(err)
+	}
+	testhelper.MustRunCommand(t, nil, "git", "clone", remoteRepoPath, localRepoPath)
+	// We need git thinking we're pushing over SSH...
+	defer os.RemoveAll(remoteRepoPath)
+	defer os.RemoveAll(localRepoPath)
+
+	makeCommit(t, localRepoPath)
+
+	socketPath, err := filepath.Rel(localRepoPath, path.Join(cwd, serverSocketPath))
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Logf("socketPath: %q", socketPath)
+
+	cmd := exec.Command("git", "-C", localRepoPath, "push", "-v", "git@localhost:test/test.git", "master")
+	cmd.Env = []string{
+		fmt.Sprintf("GITALY_SOCKET=unix://%s", socketPath),
+		fmt.Sprintf("GL_STORAGENAME=%s", storageName),
+		fmt.Sprintf("GL_RELATIVEPATH=%s", tempRepo),
+		fmt.Sprintf("GL_REPOSITORY=%s", testRepo.GetRelativePath()),
+		fmt.Sprintf("GOPATH=%s", os.Getenv("GOPATH")),
+		fmt.Sprintf("PATH=%s", ".:"+os.Getenv("PATH")),
+		fmt.Sprintf("GIT_SSH_COMMAND=%s", receivePackPath),
+		fmt.Sprintf("GL_ID=%s", glID),
+	}
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		return "", "", fmt.Errorf("Error pushing: %v: %q", err, out)
+	}
+	if !cmd.ProcessState.Success() {
+		return "", "", fmt.Errorf("Failed to run `git push`: %q", out)
+	}
+
+	localHead := bytes.TrimSpace(testhelper.MustRunCommand(t, nil, "git", "-C", localRepoPath, "rev-parse", "master"))
+	remoteHead := bytes.TrimSpace(testhelper.MustRunCommand(t, nil, "git", "-C", remoteRepoPath, "rev-parse", "master"))
+
+	return string(localHead), string(remoteHead), nil
+}
+
+// makeCommit creates a new commit and returns oldHead, newHead, success
+func makeCommit(t *testing.T, localRepoPath string) ([]byte, []byte, bool) {
+	commitMsg := fmt.Sprintf("Testing ReceivePack RPC around %d", time.Now().Unix())
+	committerName := "Scrooge McDuck"
+	committerEmail := "scrooge@mcduck.com"
+
+	// The latest commit ID on the remote repo
+	oldHead := bytes.TrimSpace(testhelper.MustRunCommand(t, nil, "git", "-C", localRepoPath, "rev-parse", "master"))
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", localRepoPath,
+		"-c", fmt.Sprintf("user.name=%s", committerName),
+		"-c", fmt.Sprintf("user.email=%s", committerEmail),
+		"commit", "--allow-empty", "-m", commitMsg)
+	if t.Failed() {
+		return nil, nil, false
+	}
+
+	// The commit ID we want to push to the remote repo
+	newHead := bytes.TrimSpace(testhelper.MustRunCommand(t, nil, "git", "-C", localRepoPath, "rev-parse", "master"))
+
+	return oldHead, newHead, t.Failed()
+}
+
 func drainPostReceivePackResponse(stream pb.SSH_SSHReceivePackClient) error {
 	var err error
 	for err == nil {
diff --git a/internal/service/ssh/testhelper_test.go b/internal/service/ssh/testhelper_test.go
index 9363d88aa..8c04a5ecc 100644
--- a/internal/service/ssh/testhelper_test.go
+++ b/internal/service/ssh/testhelper_test.go
@@ -12,29 +12,72 @@ import (
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
 
 	"gitlab.com/gitlab-org/gitaly/internal/service/renameadapter"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/reflection"
 )
 
 const (
-	scratchDir = "testdata/scratch"
+	testPath     = "testdata"
+	scratchDir   = testPath + "/scratch"
+	testRepoRoot = testPath + "/data"
 )
 
 var (
 	serverSocketPath = path.Join(scratchDir, "gitaly.sock")
+	workDir          string
+	testRepo         *pb.Repository
+
+	uploadPackPath  string
+	receivePackPath string
+
+	cwd string
 )
 
 func TestMain(m *testing.M) {
+	cwd = mustGetCwd()
+
+	err := os.RemoveAll(testPath)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	testRepo = testhelper.TestRepository()
+
+	if err := os.MkdirAll(testRepoRoot, 0755); err != nil {
+		log.Fatal(err)
+	}
+	defer os.RemoveAll(testRepoRoot)
+
 	if err := os.MkdirAll(scratchDir, 0755); err != nil {
 		log.Fatal(err)
 	}
 	defer os.RemoveAll(scratchDir)
 
+	// Build the test-binary that we need
+	os.Remove("gitaly-upload-pack")
+	testhelper.MustRunCommand(nil, nil, "go", "build", "gitlab.com/gitlab-org/gitaly/internal/service/ssh/cmd/gitaly-upload-pack")
+	defer os.Remove("gitaly-upload-pack")
+	uploadPackPath = path.Join(cwd, "gitaly-upload-pack")
+
+	os.Remove("gitaly-receive-pack")
+	testhelper.MustRunCommand(nil, nil, "go", "build", "gitlab.com/gitlab-org/gitaly/internal/service/ssh/cmd/gitaly-receive-pack")
+	defer os.Remove("gitaly-receive-pack")
+	receivePackPath = path.Join(cwd, "gitaly-receive-pack")
+
 	os.Exit(func() int {
 		return m.Run()
 	}())
 }
 
+func mustGetCwd() string {
+	wd, err := os.Getwd()
+	if err != nil {
+		log.Panic(err)
+	}
+	return wd
+}
+
 func runSSHServer(t *testing.T) *grpc.Server {
 	server := grpc.NewServer()
 	listener, err := net.Listen("unix", serverSocketPath)
diff --git a/internal/service/ssh/uploadpack.go b/internal/service/ssh/upload_pack.go
index 05aec775c..05aec775c 100644
--- a/internal/service/ssh/uploadpack.go
+++ b/internal/service/ssh/upload_pack.go
diff --git a/internal/service/ssh/upload_pack_test.go b/internal/service/ssh/upload_pack_test.go
index 96375943a..d465c5777 100644
--- a/internal/service/ssh/upload_pack_test.go
+++ b/internal/service/ssh/upload_pack_test.go
@@ -1,6 +1,12 @@
 package ssh
 
 import (
+	"bytes"
+	"fmt"
+	"os"
+	"os/exec"
+	"path"
+	"strings"
 	"testing"
 
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -17,29 +23,105 @@ func TestFailedUploadPackRequestDueToValidationError(t *testing.T) {
 
 	client := newSSHClient(t)
 
-	rpcRequests := []pb.SSHUploadPackRequest{
-		{Repository: &pb.Repository{StorageName: "default", RelativePath: ""}}, // Repository.RelativePath is empty
-		{Repository: nil}, // Repository is nil
-		{Repository: &pb.Repository{StorageName: "default", RelativePath: "path/to/repo"}, Stdin: []byte("Fail")}, // Data exists on first request
+	tests := []struct {
+		Req  *pb.SSHUploadPackRequest
+		Code codes.Code
+	}{
+		{ // Repository.RelativePath is empty
+			Req:  &pb.SSHUploadPackRequest{Repository: &pb.Repository{StorageName: "default", RelativePath: ""}},
+			Code: codes.NotFound,
+		},
+		{ // Repository is nil
+			Req:  &pb.SSHUploadPackRequest{Repository: nil},
+			Code: codes.InvalidArgument,
+		},
+		{ // Data exists on first request
+			Req:  &pb.SSHUploadPackRequest{Repository: &pb.Repository{StorageName: "default", RelativePath: "path/to/repo"}, Stdin: []byte("Fail")},
+			Code: codes.InvalidArgument,
+		},
 	}
 
-	for _, rpcRequest := range rpcRequests {
-		t.Logf("test case: %v", rpcRequest)
+	for _, test := range tests {
+		t.Logf("test case: %v", test.Req)
 		stream, err := client.SSHUploadPack(context.Background())
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		if err = stream.Send(&rpcRequest); err != nil {
+		if err = stream.Send(test.Req); err != nil {
 			t.Fatal(err)
 		}
 		stream.CloseSend()
 
 		err = drainPostUploadPackResponse(stream)
-		testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+		testhelper.AssertGrpcError(t, err, test.Code, "")
 	}
 }
 
+func TestUploadPackCloneSuccess(t *testing.T) {
+	server := runSSHServer(t)
+	defer server.Stop()
+
+	localRepoPath := path.Join(testRepoRoot, "gitlab-test-upload-pack-local")
+
+	tests := []*exec.Cmd{
+		exec.Command("git", "clone", "--depth", "1", "git@localhost:test/test.git", localRepoPath),
+		exec.Command("git", "clone", "git@localhost:test/test.git", localRepoPath),
+	}
+
+	for _, cmd := range tests {
+		lHead, rHead, err := testClone(t, testRepo.GetStorageName(), testRepo.GetRelativePath(), localRepoPath, cmd)
+		if err != nil {
+			t.Fatalf("clone failed: %v", err)
+		}
+		if strings.Compare(lHead, rHead) != 0 {
+			t.Fatalf("local and remote head not equal. clone failed: %q != %q", lHead, rHead)
+		}
+	}
+}
+
+func TestUploadPackCloneFailure(t *testing.T) {
+	server := runSSHServer(t)
+	defer server.Stop()
+
+	localRepoPath := path.Join(testRepoRoot, "gitlab-test-upload-pack-local")
+
+	cmd := exec.Command("git", "clone", "git@localhost:test/test.git", localRepoPath)
+
+	_, _, err := testClone(t, "foobar", testRepo.GetRelativePath(), localRepoPath, cmd)
+	if err == nil {
+		t.Fatalf("clone didn't fail")
+	}
+}
+
+func testClone(t *testing.T, storageName, relativePath, localRepoPath string, cmd *exec.Cmd) (string, string, error) {
+	defer os.RemoveAll(localRepoPath)
+	cmd.Env = []string{
+		fmt.Sprintf("GITALY_SOCKET=unix://%s", serverSocketPath),
+		fmt.Sprintf("GL_STORAGENAME=%s", storageName),
+		fmt.Sprintf("GL_RELATIVEPATH=%s", relativePath),
+		fmt.Sprintf("GL_REPOSITORY=%s", testRepo.GetRelativePath()),
+		fmt.Sprintf("GOPATH=%s", os.Getenv("GOPATH")),
+		fmt.Sprintf("PATH=%s", ".:"+os.Getenv("PATH")),
+		"GIT_SSH_COMMAND=gitaly-upload-pack",
+	}
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		return "", "", fmt.Errorf("%v: %q", err, out)
+	}
+	if !cmd.ProcessState.Success() {
+		return "", "", fmt.Errorf("Failed to run `git clone`: %q", out)
+	}
+
+	storagePath := testhelper.GitlabTestStoragePath()
+	testRepoPath := path.Join(storagePath, testRepo.GetRelativePath())
+
+	remoteHead := bytes.TrimSpace(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "master"))
+	localHead := bytes.TrimSpace(testhelper.MustRunCommand(t, nil, "git", "-C", localRepoPath, "rev-parse", "master"))
+
+	return string(localHead), string(remoteHead), nil
+}
+
 func drainPostUploadPackResponse(stream pb.SSH_SSHUploadPackClient) error {
 	var err error
 	for err == nil {
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 45a2a58c7..15107681c 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -97,8 +97,13 @@ func MustRunCommand(t *testing.T, stdin io.Reader, name string, args ...string)
 
 	output, err := cmd.Output()
 	if err != nil {
-		t.Log(name, args)
-		t.Fatal(err)
+		if t == nil {
+			log.Print(name, args)
+			log.Fatal(err)
+		} else {
+			t.Log(name, args)
+			t.Fatal(err)
+		}
 	}
 
 	return output