Merge branch 'zj-hide-object-pool-refs' into 'master'

Hide object pools .have refs

See merge request gitlab-org/gitaly!1323

6 files changed, 113 insertions, 14 deletions

diff --git a/changelogs/unreleased/zj-hide-object-pool-refs.yml b/changelogs/unreleased/zj-hide-object-pool-refs.yml
new file mode 100644
index 000000000..a3a78262b
--- /dev/null
+++ b/changelogs/unreleased/zj-hide-object-pool-refs.yml
@@ -0,0 +1,5 @@
+---
+title: Hide object pools .have refs
+merge_request: 1323
+author:
+type: performance
diff --git a/internal/service/smarthttp/inforefs.go b/internal/service/smarthttp/inforefs.go
index 3947c1cd9..7b0018679 100644
--- a/internal/service/smarthttp/inforefs.go
+++ b/internal/service/smarthttp/inforefs.go
@@ -42,7 +42,13 @@ func handleInfoRefs(ctx context.Context, service string, req *gitalypb.InfoRefsR
 		return err
 	}
 
-	args := []string{}
+	// In case the repository belongs to an object pool, we want to prevent
+	// Git from including the pool's refs in the ref advertisement. We do
+	// this by rigging core.alternateRefsCommand to produce no output.
+	// Because Git itself will append the pool repository directory, the
+	// command ends with a "#". The end result is that Git runs
+	// `/bin/sh -c 'exit 0 # /path/to/pool.git`.
+	args := []string{"-c", "core.alternateRefsCommand=exit 0 #"}
 	for _, params := range req.GitConfigOptions {
 		args = append(args, "-c", params)
 	}
diff --git a/internal/service/smarthttp/inforefs_test.go b/internal/service/smarthttp/inforefs_test.go
index 6683a6336..d1cc8fb89 100644
--- a/internal/service/smarthttp/inforefs_test.go
+++ b/internal/service/smarthttp/inforefs_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/objectpool"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 	"google.golang.org/grpc/codes"
@@ -139,6 +140,43 @@ func TestSuccessfulInfoRefsReceivePack(t *testing.T) {
 	})
 }
 
+func TestObjectPoolRefAdvertisementHiding(t *testing.T) {
+	server, serverSocketPath := runSmartHTTPServer(t)
+	defer server.Stop()
+
+	client, conn := newSmartHTTPClient(t, serverSocketPath)
+	defer conn.Close()
+
+	repo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	pool, err := objectpool.NewObjectPool(repo.GetStorageName(), testhelper.NewTestObjectPoolName(t))
+	require.NoError(t, err)
+
+	require.NoError(t, pool.Create(ctx, repo))
+	defer pool.Remove(ctx)
+
+	commitID := testhelper.CreateCommit(t, pool.FullPath(), t.Name(), nil)
+
+	require.NoError(t, pool.Link(ctx, repo))
+
+	rpcRequest := &gitalypb.InfoRefsRequest{Repository: repo}
+
+	c, err := client.InfoRefsReceivePack(ctx, rpcRequest)
+	require.NoError(t, err)
+
+	response, err := ioutil.ReadAll(streamio.NewReader(func() ([]byte, error) {
+		resp, err := c.Recv()
+		return resp.GetData(), err
+	}))
+
+	require.NoError(t, err)
+	require.NotContains(t, string(response), commitID+" .have")
+}
+
 func TestFailureRepoNotFoundInfoRefsReceivePack(t *testing.T) {
 	server, serverSocketPath := runSmartHTTPServer(t)
 	defer server.Stop()
diff --git a/internal/service/smarthttp/receive_pack.go b/internal/service/smarthttp/receive_pack.go
index 5ff0d584e..fd214f98c 100644
--- a/internal/service/smarthttp/receive_pack.go
+++ b/internal/service/smarthttp/receive_pack.go
@@ -62,6 +62,7 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac
 	env = append(env, command.GitEnv...)
 
 	opts := append([]string{fmt.Sprintf("core.hooksPath=%s", hooks.Path())}, req.GitConfigOptions...)
+
 	gitOptions := git.BuildGitOptions(opts, "receive-pack", "--stateless-rpc", repoPath)
 	cmd, err := git.BareCommand(ctx, stdin, stdout, nil, env, gitOptions...)
 
diff --git a/internal/service/ssh/receive_pack.go b/internal/service/ssh/receive_pack.go
index e952e7777..e8224808c 100644
--- a/internal/service/ssh/receive_pack.go
+++ b/internal/service/ssh/receive_pack.go
@@ -70,6 +70,14 @@ func sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, req *gitaly
 	env = append(env, command.GitEnv...)
 
 	opts := append([]string{fmt.Sprintf("core.hooksPath=%s", hooks.Path())}, req.GitConfigOptions...)
+
+	// In case the repository belongs to an object pool, we want to prevent
+	// Git from including the pool's refs in the ref advertisement. We do
+	// this by rigging core.alternateRefsCommand to produce no output.
+	// Because Git itself will append the pool repository directory, the
+	// command ends with a "#". The end result is that Git runs
+	// `/bin/sh -c 'exit 0 # /path/to/pool.git`.
+	opts = append(opts, "core.alternateRefsCommand=exit 0 #")
 	gitOptions := git.BuildGitOptions(opts, "receive-pack", repoPath)
 	cmd, err := git.BareCommand(ctx, stdin, stdout, stderr, env, gitOptions...)
 
diff --git a/internal/service/ssh/receive_pack_test.go b/internal/service/ssh/receive_pack_test.go
index 44f0ac1c0..a823b56cd 100644
--- a/internal/service/ssh/receive_pack_test.go
+++ b/internal/service/ssh/receive_pack_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -18,7 +19,9 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
+	"gitlab.com/gitlab-org/gitaly/internal/git/objectpool"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/streamio"
 	"google.golang.org/grpc/codes"
 )
 
@@ -60,15 +63,12 @@ func TestFailedReceivePackRequestDueToValidationError(t *testing.T) {
 		t.Run(test.Desc, func(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
+
 			stream, err := client.SSHReceivePack(ctx)
-			if err != nil {
-				t.Fatal(err)
-			}
+			require.NoError(t, err)
 
-			if err = stream.Send(test.Req); err != nil {
-				t.Fatal(err)
-			}
-			stream.CloseSend()
+			require.NoError(t, stream.Send(test.Req))
+			require.NoError(t, stream.CloseSend())
 
 			err = drainPostReceivePackResponse(stream)
 			testhelper.RequireGrpcError(t, err, test.Code)
@@ -130,14 +130,10 @@ func TestReceivePackPushFailure(t *testing.T) {
 	defer server.Stop()
 
 	_, _, err := testCloneAndPush(t, serverSocketPath, pushParams{storageName: "foobar", glID: "1"})
-	if err == nil {
-		t.Errorf("local and remote head equal. push did not fail")
-	}
+	require.Error(t, err, "local and remote head equal. push did not fail")
 
 	_, _, err = testCloneAndPush(t, serverSocketPath, pushParams{storageName: testRepo.GetStorageName(), glID: ""})
-	if err == nil {
-		t.Errorf("local and remote head equal. push did not fail")
-	}
+	require.Error(t, err, "local and remote head equal. push did not fail")
 }
 
 func TestReceivePackPushHookFailure(t *testing.T) {
@@ -161,6 +157,51 @@ func TestReceivePackPushHookFailure(t *testing.T) {
 	require.Contains(t, err.Error(), "(pre-receive hook declined)")
 }
 
+func TestObjectPoolRefAdvertisementHidingSSH(t *testing.T) {
+	server, serverSocketPath := runSSHServer(t)
+	defer server.Stop()
+
+	client, conn := newSSHClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	stream, err := client.SSHReceivePack(ctx)
+	require.NoError(t, err)
+
+	repo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := objectpool.NewObjectPool(repo.GetStorageName(), testhelper.NewTestObjectPoolName(t))
+	require.NoError(t, err)
+
+	require.NoError(t, pool.Create(ctx, repo))
+	defer pool.Remove(ctx)
+
+	require.NoError(t, pool.Link(ctx, repo))
+
+	commitID := testhelper.CreateCommit(t, pool.FullPath(), t.Name(), nil)
+
+	// First request
+	require.NoError(t, stream.Send(&gitalypb.SSHReceivePackRequest{
+		Repository: &gitalypb.Repository{StorageName: "default", RelativePath: repo.GetRelativePath()}, GlId: "user-123",
+	}))
+
+	require.NoError(t, stream.Send(&gitalypb.SSHReceivePackRequest{Stdin: []byte("0000")}))
+	require.NoError(t, stream.CloseSend())
+
+	r := streamio.NewReader(func() ([]byte, error) {
+		msg, err := stream.Recv()
+		return msg.GetStdout(), err
+	})
+
+	var b bytes.Buffer
+	_, err = io.Copy(&b, r)
+	require.NoError(t, err)
+	require.NotContains(t, b.String(), commitID+" .have")
+}
+
 func testCloneAndPush(t *testing.T, serverSocketPath string, params pushParams) (string, string, error) {
 	storagePath := testhelper.GitlabTestStoragePath()
 	tempRepo := "gitlab-test-ssh-receive-pack.git"