repository: Allow CreateRepository to take default_branchjc-create-repo-default-branch

In GitLab rails, there is code to ensure that the default branch on a
new repository is set. This is currently done with an extra WriteRef
call. This is done in a racy way, which causes problems with transaction
voting. To fix this, we can simplify the logic altogether  and avoid
adding another roundtrip, add a parameter in CreateRepositoryRequest
to pass a default_branch.

Changelog: changed

3 files changed, 82 insertions, 7 deletions

diff --git a/internal/gitaly/service/repository/create_repository.go b/internal/gitaly/service/repository/create_repository.go
index 31bd9a0af..6b9dc23e1 100644
--- a/internal/gitaly/service/repository/create_repository.go
+++ b/internal/gitaly/service/repository/create_repository.go
@@ -8,11 +8,15 @@ import (
 )
 
 func (s *server) CreateRepository(ctx context.Context, req *gitalypb.CreateRepositoryRequest) (*gitalypb.CreateRepositoryResponse, error) {
-	if err := s.createRepository(ctx, req.GetRepository(), func(repo *gitalypb.Repository) error {
-		// We do not want to seed the repository with any contents, so we just
-		// return directly.
-		return nil
-	}); err != nil {
+	if err := s.createRepository(
+		ctx,
+		req.GetRepository(),
+		func(repo *gitalypb.Repository) error {
+			// We do not want to seed the repository with any contents, so we just
+			// return directly.
+			return nil
+		},
+		withBranchName(string(req.GetDefaultBranch()))); err != nil {
 		return nil, helper.ErrInternalf("creating repository: %w", err)
 	}
 
diff --git a/internal/gitaly/service/repository/create_repository_test.go b/internal/gitaly/service/repository/create_repository_test.go
index da35e3098..9aafec9ee 100644
--- a/internal/gitaly/service/repository/create_repository_test.go
+++ b/internal/gitaly/service/repository/create_repository_test.go
@@ -13,6 +13,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config/auth"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/transaction"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/praefect/praefectutil"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
@@ -72,6 +73,57 @@ func TestCreateRepository_successful(t *testing.T) {
 	require.Equal(t, symRef, []byte(fmt.Sprintf("ref: %s\n", git.DefaultRef)))
 }
 
+func TestCreateRepository_withDefaultBranch(t *testing.T) {
+	cfg, client := setupRepositoryServiceWithoutRepo(t)
+
+	ctx := testhelper.Context(t)
+
+	testCases := []struct {
+		name              string
+		defaultBranch     string
+		expected          string
+		expectedErrString string
+	}{
+		{
+			name:          "valid default branch",
+			defaultBranch: "develop",
+			expected:      "refs/heads/develop",
+		},
+		{
+			name:          "empty branch name",
+			defaultBranch: "",
+			expected:      "refs/heads/main",
+		},
+		{
+			name:              "invalid branch name",
+			defaultBranch:     "./.lock",
+			expected:          "refs/heads/main",
+			expectedErrString: `creating repository: exit status 128, stderr: "fatal: invalid initial branch name: './.lock'\n"`,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			repo := &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: gittest.NewRepositoryName(t, true)}
+
+			req := &gitalypb.CreateRepositoryRequest{Repository: repo, DefaultBranch: []byte(tc.defaultBranch)}
+			_, err := client.CreateRepository(ctx, req)
+			if tc.expectedErrString != "" {
+				require.Contains(t, err.Error(), tc.expectedErrString)
+			} else {
+				require.NoError(t, err)
+				repoPath := filepath.Join(cfg.Storages[0].Path, gittest.GetReplicaPath(ctx, t, cfg, repo))
+				symRef := text.ChompBytes(gittest.Exec(
+					t,
+					cfg,
+					"-C", repoPath,
+					"symbolic-ref", "HEAD"))
+				require.Equal(t, tc.expected, symRef)
+			}
+		})
+	}
+}
+
 func TestCreateRepository_failure(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
diff --git a/internal/gitaly/service/repository/util.go b/internal/gitaly/service/repository/util.go
index 94cccfb96..97a96de28 100644
--- a/internal/gitaly/service/repository/util.go
+++ b/internal/gitaly/service/repository/util.go
@@ -31,6 +31,18 @@ func (s *server) removeOriginInRepo(ctx context.Context, repository *gitalypb.Re
 	return nil
 }
 
+type createRepositoryOption func(options *[]git.Option)
+
+func withBranchName(branch string) createRepositoryOption {
+	return func(options *[]git.Option) {
+		if branch == "" {
+			return
+		}
+
+		*options = append(*options, git.ValueFlag{Name: "--initial-branch", Value: branch})
+	}
+}
+
 // createRepository will create a new repository in a race-free way with proper transactional
 // semantics. The repository will only be created if it doesn't yet exist and if nodes which take
 // part in the transaction reach quorum. Otherwise, the target path of the new repository will not
@@ -39,6 +51,7 @@ func (s *server) createRepository(
 	ctx context.Context,
 	repository *gitalypb.Repository,
 	seedRepository func(repository *gitalypb.Repository) error,
+	options ...createRepositoryOption,
 ) error {
 	targetPath, err := s.locator.GetPath(repository)
 	if err != nil {
@@ -70,13 +83,19 @@ func (s *server) createRepository(
 	// Note that we do not create the repository directly in its target location, but
 	// instead create it in a temporary directory, first. This is done such that we can
 	// guarantee atomicity and roll back the change easily in case an error happens.
+
+	gitOptions := make([]git.Option, 0, len(options))
+	for _, optionFn := range options {
+		optionFn(&gitOptions)
+	}
+
 	stderr := &bytes.Buffer{}
 	cmd, err := s.gitCmdFactory.NewWithoutRepo(ctx, git.SubCmd{
 		Name: "init",
-		Flags: []git.Option{
+		Flags: append([]git.Option{
 			git.Flag{Name: "--bare"},
 			git.Flag{Name: "--quiet"},
-		},
+		}, gitOptions...),
 		Args: []string{newRepoDir.Path()},
 	}, git.WithStderr(stderr))
 	if err != nil {