Push default branch in the first batch of refspecs in UpdateRemoteMirror

UpdateRemoteMirror pushes to a remote mirror repository. The pushes are
done in batches of 10 refspecs. The Ruby implementation of the RPC ensures
the default branch of the repository is always pushed in the first batch
to ensure anything that relies on the default branch works. The Go port
of the RPC is currently not doing so. This commit fixes the situation by
ensuring the default branch is always part of the first push.

2 files changed, 79 insertions, 1 deletions

diff --git a/internal/gitaly/service/remote/update_remote_mirror.go b/internal/gitaly/service/remote/update_remote_mirror.go
index 21f77c32d..337c65130 100644
--- a/internal/gitaly/service/remote/update_remote_mirror.go
+++ b/internal/gitaly/service/remote/update_remote_mirror.go
@@ -10,6 +10,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/service/ref"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -134,6 +135,11 @@ func (s *server) goUpdateRemoteMirror(stream gitalypb.RemoteService_UpdateRemote
 		return errors.New("close stream to gitaly-ruby: rpc error: code = Unknown desc = NoMethodError: undefined method `id' for nil:NilClass")
 	}
 
+	defaultBranch, err := ref.DefaultBranchName(ctx, repo)
+	if err != nil {
+		return fmt.Errorf("get default branch: %w", err)
+	}
+
 	remoteRefs := make(map[git.ReferenceName]string, len(remoteRefsSlice))
 	for _, ref := range remoteRefsSlice {
 		remoteRefs[ref.Name] = ref.Target
@@ -202,6 +208,13 @@ func (s *server) goUpdateRemoteMirror(stream gitalypb.RemoteService_UpdateRemote
 			}
 
 			refspecs = append(refspecs, prefix+reference.String())
+			if reference == git.ReferenceName(defaultBranch) {
+				// The default branch needs to be pushed in the first batch of refspecs as some features
+				// depend on it existing in the repository. The default branch may not exist in the repo
+				// yet if this is the first mirroring push.
+				last := len(refspecs) - 1
+				refspecs[0], refspecs[last] = refspecs[last], refspecs[0]
+			}
 
 			// https://gitlab.com/gitlab-org/gitaly/-/issues/3504
 			name := strings.TrimPrefix(reference.String(), "refs/heads/")
diff --git a/internal/gitaly/service/remote/update_remote_mirror_test.go b/internal/gitaly/service/remote/update_remote_mirror_test.go
index 34edbf84e..c3c7f526f 100644
--- a/internal/gitaly/service/remote/update_remote_mirror_test.go
+++ b/internal/gitaly/service/remote/update_remote_mirror_test.go
@@ -8,9 +8,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
@@ -32,8 +35,16 @@ func testUpdateRemoteMirror(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Se
 	})
 }
 
+type commandFactoryWrapper struct {
+	git.CommandFactory
+	newFunc func(context.Context, repository.GitRepo, git.Cmd, ...git.CmdOpt) (*command.Command, error)
+}
+
+func (w commandFactoryWrapper) New(ctx context.Context, repo repository.GitRepo, sc git.Cmd, opts ...git.CmdOpt) (*command.Command, error) {
+	return w.newFunc(ctx, repo, sc, opts...)
+}
+
 func testUpdateRemoteMirrorFeatured(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	cfg, _, _, client := setupRemoteServiceWithRuby(t, cfg, rubySrv)
 	testhelper.ConfigureGitalyGit2GoBin(t, cfg)
 
 	type refs map[string][]string
@@ -46,6 +57,7 @@ func testUpdateRemoteMirrorFeatured(t *testing.T, ctx context.Context, cfg confi
 		mirrorSymRefs        map[string]string
 		keepDivergentRefs    bool
 		onlyBranchesMatching []string
+		wrapCommandFactory   func(testing.TB, git.CommandFactory) git.CommandFactory
 		requests             []*gitalypb.UpdateRemoteMirrorRequest
 		errorContains        string
 		response             *gitalypb.UpdateRemoteMirrorResponse
@@ -367,6 +379,40 @@ func testUpdateRemoteMirrorFeatured(t *testing.T, ctx context.Context, cfg confi
 			}(),
 		},
 		{
+			desc: "pushes default branch in the first batch",
+			wrapCommandFactory: func(t testing.TB, original git.CommandFactory) git.CommandFactory {
+				firstPush := true
+				return commandFactoryWrapper{
+					CommandFactory: original,
+					newFunc: func(ctx context.Context, repo repository.GitRepo, sc git.Cmd, opts ...git.CmdOpt) (*command.Command, error) {
+						if sc.Subcommand() == "push" && firstPush {
+							firstPush = false
+							args, err := sc.CommandArgs()
+							assert.NoError(t, err)
+							assert.Contains(t, args, "refs/heads/master", "first push should contain the default branch")
+						}
+
+						return original.New(ctx, repo, sc, opts...)
+					},
+				}
+			},
+			sourceRefs: func() refs {
+				out := refs{"refs/heads/master": []string{"commit 1"}}
+				for i := 0; i < 2*pushBatchSize; i++ {
+					out[fmt.Sprintf("refs/heads/branch-%d", i)] = []string{"commit 1"}
+				}
+				return out
+			}(),
+			response: &gitalypb.UpdateRemoteMirrorResponse{},
+			expectedMirrorRefs: func() map[string]string {
+				out := map[string]string{"refs/heads/master": "commit 1"}
+				for i := 0; i < 2*pushBatchSize; i++ {
+					out[fmt.Sprintf("refs/heads/branch-%d", i)] = "commit 1"
+				}
+				return out
+			}(),
+		},
+		{
 			desc: "limits the number of divergent refs returned",
 			sourceRefs: func() refs {
 				out := refs{}
@@ -447,6 +493,25 @@ func testUpdateRemoteMirrorFeatured(t *testing.T, ctx context.Context, cfg confi
 				}
 			}
 
+			addr := testserver.RunGitalyServer(t, cfg, rubySrv, func(srv *grpc.Server, deps *service.Dependencies) {
+				cmdFactory := deps.GetGitCmdFactory()
+				if tc.wrapCommandFactory != nil {
+					cmdFactory = tc.wrapCommandFactory(t, deps.GetGitCmdFactory())
+				}
+
+				gitalypb.RegisterRemoteServiceServer(srv, NewServer(
+					deps.GetCfg(),
+					deps.GetRubyServer(),
+					deps.GetLocator(),
+					cmdFactory,
+					deps.GetCatfileCache(),
+					deps.GetTxManager(),
+				))
+			})
+
+			client, conn := newRemoteClient(t, addr)
+			defer conn.Close()
+
 			stream, err := client.UpdateRemoteMirror(ctx)
 			require.NoError(t, err)