Merge branch 'pks-localrepo-fetch-internal-protocol-v2' into 'master'

localrepo: Use protocol v2 for internal fetches

See merge request gitlab-org/gitaly!4303

8 files changed, 16 insertions, 10 deletions

diff --git a/internal/git/gittest/protocol.go b/internal/git/gittest/protocol.go
index 5ba3df11e..301cf34e5 100644
--- a/internal/git/gittest/protocol.go
+++ b/internal/git/gittest/protocol.go
@@ -13,10 +13,10 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 )
 
-// EnableGitProtocolV2Support creates a new intercepting Git command factory that allows the
+// NewProtocolDetectingCommandFactory creates a new intercepting Git command factory that allows the
 // protocol to be tested. It returns this factory and a function to read the GIT_PROTOCOL
 // environment variable created by the wrapper script.
-func EnableGitProtocolV2Support(ctx context.Context, t testing.TB, cfg config.Cfg) (git.CommandFactory, func() string) {
+func NewProtocolDetectingCommandFactory(ctx context.Context, t testing.TB, cfg config.Cfg) (git.CommandFactory, func() string) {
 	envPath := filepath.Join(testhelper.TempDir(t), "git-env")
 
 	gitCmdFactory := NewInterceptingCommandFactory(ctx, t, cfg, func(execEnv git.ExecutionEnvironment) string {
diff --git a/internal/git/localrepo/remote.go b/internal/git/localrepo/remote.go
index 1ef7617a9..69ffb29d8 100644
--- a/internal/git/localrepo/remote.go
+++ b/internal/git/localrepo/remote.go
@@ -129,6 +129,7 @@ func (repo *Repo) FetchInternal(
 		git.WithInternalFetch(&gitalypb.SSHUploadPackRequest{
 			Repository:       remoteRepo,
 			GitConfigOptions: []string{"uploadpack.allowAnySHA1InWant=true"},
+			GitProtocol:      git.ProtocolV2,
 		}),
 		git.WithEnv(opts.Env...),
 		git.WithStderr(opts.Stderr),
diff --git a/internal/git/localrepo/remote_extra_test.go b/internal/git/localrepo/remote_extra_test.go
index 31d07e972..1b473c06e 100644
--- a/internal/git/localrepo/remote_extra_test.go
+++ b/internal/git/localrepo/remote_extra_test.go
@@ -25,6 +25,8 @@ func TestRepo_FetchInternal(t *testing.T) {
 	ctx := testhelper.Context(t)
 
 	cfg, remoteRepoProto, _ := testcfg.BuildWithRepo(t)
+	gitCmdFactory, readGitProtocol := gittest.NewProtocolDetectingCommandFactory(ctx, t, cfg)
+
 	cfg.SocketPath = testserver.RunGitalyServer(t, cfg, nil, func(srv *grpc.Server, deps *service.Dependencies) {
 		gitalypb.RegisterSSHServiceServer(srv, ssh.NewServer(
 			deps.GetLocator(),
@@ -36,7 +38,7 @@ func TestRepo_FetchInternal(t *testing.T) {
 			deps.GetGitCmdFactory(),
 			deps.GetPackObjectsCache(),
 		))
-	})
+	}, testserver.WithGitCommandFactory(gitCmdFactory))
 
 	remoteRepo := localrepo.NewTestRepo(t, cfg, remoteRepoProto)
 	testcfg.BuildGitalySSH(t, cfg)
@@ -78,6 +80,9 @@ func TestRepo_FetchInternal(t *testing.T) {
 		// to do so.
 		require.NoFileExists(t, filepath.Join(repoPath, "objects/info/commit-graph"))
 		require.NoDirExists(t, filepath.Join(repoPath, "objects/info/commit-graphs"))
+
+		// Assert that we're using the expected Git protocol version, which is protocol v2.
+		require.Equal(t, "GIT_PROTOCOL=version=2\n", readGitProtocol())
 	})
 
 	t.Run("refspec without tags", func(t *testing.T) {
@@ -129,7 +134,7 @@ func TestRepo_FetchInternal(t *testing.T) {
 		)
 		require.EqualError(t, err, "exit status 128")
 		require.IsType(t, err, localrepo.ErrFetchFailed{})
-		require.Contains(t, stderr.String(), "fatal: couldn't find remote ref refs/does/not/exist\nfatal: the remote end hung up unexpectedly\n")
+		require.Equal(t, stderr.String(), "fatal: couldn't find remote ref refs/does/not/exist\n")
 	})
 
 	t.Run("with env", func(t *testing.T) {
diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go
index 116c28914..bf32b6460 100644
--- a/internal/gitaly/service/smarthttp/inforefs_test.go
+++ b/internal/gitaly/service/smarthttp/inforefs_test.go
@@ -115,7 +115,7 @@ func TestSuccessfulInfoRefsUploadPackWithGitProtocol(t *testing.T) {
 	cfg, repo, _ := testcfg.BuildWithRepo(t)
 	ctx := testhelper.Context(t)
 
-	gitCmdFactory, readProtocol := gittest.EnableGitProtocolV2Support(ctx, t, cfg)
+	gitCmdFactory, readProtocol := gittest.NewProtocolDetectingCommandFactory(ctx, t, cfg)
 
 	server := startSmartHTTPServerWithOptions(t, cfg, nil, []testserver.GitalyServerOpt{
 		testserver.WithGitCommandFactory(gitCmdFactory),
diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go
index 773600cc0..742058bce 100644
--- a/internal/gitaly/service/smarthttp/receive_pack_test.go
+++ b/internal/gitaly/service/smarthttp/receive_pack_test.go
@@ -167,7 +167,7 @@ func TestSuccessfulReceivePackRequestWithGitProtocol(t *testing.T) {
 	testcfg.BuildGitalyHooks(t, cfg)
 	ctx := testhelper.Context(t)
 
-	gitCmdFactory, readProto := gittest.EnableGitProtocolV2Support(ctx, t, cfg)
+	gitCmdFactory, readProto := gittest.NewProtocolDetectingCommandFactory(ctx, t, cfg)
 
 	server := startSmartHTTPServerWithOptions(t, cfg, nil, []testserver.GitalyServerOpt{
 		testserver.WithGitCommandFactory(gitCmdFactory),
diff --git a/internal/gitaly/service/smarthttp/upload_pack_test.go b/internal/gitaly/service/smarthttp/upload_pack_test.go
index c0fda063b..d7923f571 100644
--- a/internal/gitaly/service/smarthttp/upload_pack_test.go
+++ b/internal/gitaly/service/smarthttp/upload_pack_test.go
@@ -175,7 +175,7 @@ func TestServer_PostUploadPackWithSidechannel_gitProtocol(t *testing.T) {
 
 func testServerPostUploadPackGitProtocol(t *testing.T, ctx context.Context, makeRequest requestMaker, opts ...testcfg.Option) {
 	cfg, repo, _ := testcfg.BuildWithRepo(t, opts...)
-	gitCmdFactory, readProto := gittest.EnableGitProtocolV2Support(ctx, t, cfg)
+	gitCmdFactory, readProto := gittest.NewProtocolDetectingCommandFactory(ctx, t, cfg)
 	server := startSmartHTTPServerWithOptions(t, cfg, nil, []testserver.GitalyServerOpt{
 		testserver.WithGitCommandFactory(gitCmdFactory),
 	})
diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go
index 6c06a86c5..a6ecd2375 100644
--- a/internal/gitaly/service/ssh/receive_pack_test.go
+++ b/internal/gitaly/service/ssh/receive_pack_test.go
@@ -168,7 +168,7 @@ func TestReceivePackPushSuccessWithGitProtocol(t *testing.T) {
 	testcfg.BuildGitalyHooks(t, cfg)
 	ctx := testhelper.Context(t)
 
-	gitCmdFactory, readProto := gittest.EnableGitProtocolV2Support(ctx, t, cfg)
+	gitCmdFactory, readProto := gittest.NewProtocolDetectingCommandFactory(ctx, t, cfg)
 
 	serverSocketPath := runSSHServer(t, cfg, testserver.WithGitCommandFactory(gitCmdFactory))
 
@@ -491,7 +491,7 @@ func TestSSHReceivePackToHooks(t *testing.T) {
 	)
 	ctx := testhelper.Context(t)
 
-	gitCmdFactory, readProto := gittest.EnableGitProtocolV2Support(ctx, t, cfg)
+	gitCmdFactory, readProto := gittest.NewProtocolDetectingCommandFactory(ctx, t, cfg)
 
 	tempGitlabShellDir := testhelper.TempDir(t)
 
diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go
index d96df2fb8..9f89e84fe 100644
--- a/internal/gitaly/service/ssh/upload_pack_test.go
+++ b/internal/gitaly/service/ssh/upload_pack_test.go
@@ -466,7 +466,7 @@ func testUploadPackCloneSuccessWithGitProtocol(t *testing.T, opts ...testcfg.Opt
 	cfg, repo, repoPath := testcfg.BuildWithRepo(t, opts...)
 	ctx := testhelper.Context(t)
 
-	gitCmdFactory, readProto := gittest.EnableGitProtocolV2Support(ctx, t, cfg)
+	gitCmdFactory, readProto := gittest.NewProtocolDetectingCommandFactory(ctx, t, cfg)
 
 	serverSocketPath := runSSHServer(t, cfg, testserver.WithGitCommandFactory(gitCmdFactory))