Add feature head_as_default_branchff_head_as_default_branch

Enabling this feature ensures that the default branch returned by gitaly
is always what is specified in HEAD without using any heuristics.

7 files changed, 86 insertions, 25 deletions

diff --git a/internal/git/gittest/repository_suite.go b/internal/git/gittest/repository_suite.go
index 885af8b40..2ac6f18b7 100644
--- a/internal/git/gittest/repository_suite.go
+++ b/internal/git/gittest/repository_suite.go
@@ -8,6 +8,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 )
 
@@ -19,7 +20,7 @@ type GetRepositoryFunc func(t testing.TB, ctx context.Context) (git.Repository,
 func TestRepository(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFunc) {
 	for _, tc := range []struct {
 		desc string
-		test func(*testing.T, config.Cfg, GetRepositoryFunc)
+		test func(*testing.T, context.Context, config.Cfg, GetRepositoryFunc)
 	}{
 		{
 			desc: "ResolveRevision",
@@ -35,14 +36,14 @@ func TestRepository(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFun
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			tc.test(t, cfg, getRepository)
+			testhelper.NewFeatureSets(featureflag.HeadAsDefaultBranch).Run(t, func(t *testing.T, ctx context.Context) {
+				tc.test(t, ctx, cfg, getRepository)
+			})
 		})
 	}
 }
 
-func testRepositoryResolveRevision(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFunc) {
-	ctx := testhelper.Context(t)
-
+func testRepositoryResolveRevision(t *testing.T, ctx context.Context, cfg config.Cfg, getRepository GetRepositoryFunc) {
 	repo, repoPath := getRepository(t, ctx)
 
 	firstParentCommitID := WriteCommit(t, cfg, repoPath, WithMessage("first parent"))
@@ -96,9 +97,7 @@ func testRepositoryResolveRevision(t *testing.T, cfg config.Cfg, getRepository G
 	}
 }
 
-func testRepositoryHasBranches(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFunc) {
-	ctx := testhelper.Context(t)
-
+func testRepositoryHasBranches(t *testing.T, ctx context.Context, cfg config.Cfg, getRepository GetRepositoryFunc) {
 	repo, repoPath := getRepository(t, ctx)
 
 	emptyCommit := text.ChompBytes(Exec(t, cfg, "-C", repoPath, "commit-tree", DefaultObjectHash.EmptyTreeOID.String()))
@@ -116,9 +115,7 @@ func testRepositoryHasBranches(t *testing.T, cfg config.Cfg, getRepository GetRe
 	require.True(t, hasBranches)
 }
 
-func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository GetRepositoryFunc) {
-	ctx := testhelper.Context(t)
-
+func testRepositoryGetDefaultBranch(t *testing.T, ctx context.Context, cfg config.Cfg, getRepository GetRepositoryFunc) {
 	for _, tc := range []struct {
 		desc         string
 		repo         func(t *testing.T) git.Repository
@@ -142,7 +139,10 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository
 				WriteCommit(t, cfg, repoPath, WithParents(oid), WithBranch("master"))
 				return repo
 			},
-			expectedName: git.LegacyDefaultRef,
+			expectedName: testhelper.EnabledOrDisabledFlag(ctx, featureflag.HeadAsDefaultBranch,
+				git.DefaultRef,
+				git.LegacyDefaultRef,
+			),
 		},
 		{
 			desc: "no branches",
@@ -150,6 +150,10 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository
 				repo, _ := getRepository(t, ctx)
 				return repo
 			},
+			expectedName: testhelper.EnabledOrDisabledFlag(ctx, featureflag.HeadAsDefaultBranch,
+				git.DefaultRef,
+				git.ReferenceName(""),
+			),
 		},
 		{
 			desc: "one branch",
@@ -158,7 +162,10 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository
 				WriteCommit(t, cfg, repoPath, WithBranch("apple"))
 				return repo
 			},
-			expectedName: git.NewReferenceNameFromBranchName("apple"),
+			expectedName: testhelper.EnabledOrDisabledFlag(ctx, featureflag.HeadAsDefaultBranch,
+				git.DefaultRef,
+				git.NewReferenceNameFromBranchName("apple"),
+			),
 		},
 		{
 			desc: "no default branches",
@@ -168,7 +175,10 @@ func testRepositoryGetDefaultBranch(t *testing.T, cfg config.Cfg, getRepository
 				WriteCommit(t, cfg, repoPath, WithParents(oid), WithBranch("banana"))
 				return repo
 			},
-			expectedName: git.NewReferenceNameFromBranchName("apple"),
+			expectedName: testhelper.EnabledOrDisabledFlag(ctx, featureflag.HeadAsDefaultBranch,
+				git.DefaultRef,
+				git.NewReferenceNameFromBranchName("apple"),
+			),
 		},
 		{
 			desc: "test repo HEAD set",
diff --git a/internal/git/localrepo/refs.go b/internal/git/localrepo/refs.go
index 398b2950f..6325a49a0 100644
--- a/internal/git/localrepo/refs.go
+++ b/internal/git/localrepo/refs.go
@@ -14,6 +14,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/safe"
 )
 
@@ -326,6 +327,10 @@ func (repo *Repo) GetRemoteReferences(ctx context.Context, remote string, opts .
 
 // GetDefaultBranch determines the default branch name
 func (repo *Repo) GetDefaultBranch(ctx context.Context) (git.ReferenceName, error) {
+	if featureflag.HeadAsDefaultBranch.IsEnabled(ctx) {
+		return repo.headReference(ctx)
+	}
+
 	branches, err := repo.GetBranches(ctx)
 	if err != nil {
 		return "", err
diff --git a/internal/git/localrepo/refs_test.go b/internal/git/localrepo/refs_test.go
index c46f19d62..b1aec6ffe 100644
--- a/internal/git/localrepo/refs_test.go
+++ b/internal/git/localrepo/refs_test.go
@@ -21,6 +21,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/safe"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
@@ -488,7 +489,10 @@ func TestRepo_UpdateRef(t *testing.T) {
 }
 
 func TestRepo_SetDefaultBranch(t *testing.T) {
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.HeadAsDefaultBranch).Run(t, testRepoSetDefaultBranch)
+}
+
+func testRepoSetDefaultBranch(t *testing.T, ctx context.Context) {
 	cfg, repo, repoPath := setupRepo(t)
 
 	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master"))
@@ -507,9 +511,12 @@ func TestRepo_SetDefaultBranch(t *testing.T) {
 			expectedRef: "refs/heads/feature",
 		},
 		{
-			desc:        "unknown ref",
-			ref:         "refs/heads/non_existent_ref",
-			expectedRef: git.LegacyDefaultRef,
+			desc: "unknown ref",
+			ref:  "refs/heads/non_existent_ref",
+			expectedRef: testhelper.EnabledOrDisabledFlag(ctx, featureflag.HeadAsDefaultBranch,
+				git.ReferenceName("refs/heads/non_existent_ref"),
+				git.LegacyDefaultRef,
+			),
 		},
 	}
 	for _, tc := range testCases {
@@ -565,8 +572,10 @@ func (b *blockingManager) Stop(_ context.Context, _ txinfo.Transaction) error {
 }
 
 func TestRepo_SetDefaultBranch_errors(t *testing.T) {
-	ctx := testhelper.Context(t)
+	testhelper.NewFeatureSets(featureflag.HeadAsDefaultBranch).Run(t, testRepoSetDefaultBranchErrors)
+}
 
+func testRepoSetDefaultBranchErrors(t *testing.T, ctx context.Context) {
 	t.Run("malformed refname", func(t *testing.T) {
 		_, repo, _ := setupRepo(t)
 
diff --git a/internal/gitaly/testhelper_test.go b/internal/gitaly/testhelper_test.go
index e56b93600..f92be0234 100644
--- a/internal/gitaly/testhelper_test.go
+++ b/internal/gitaly/testhelper_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"google.golang.org/protobuf/proto"
 )
@@ -76,5 +77,16 @@ func RequireDefaultBranch(tb testing.TB, ctx context.Context, repo *localrepo.Re
 
 	actualDefaultBranch, err := repo.GetDefaultBranch(ctx)
 	require.NoError(tb, err)
+
+	// Often it is convenient to leave test-case assertions at their zero value
+	// when it isn't relevant to the test at hand. Before HeadAsDefaultBranch
+	// empty repositories would return an empty string as the default branch,
+	// with HeadAsDefaultBranch there is always a default branch. So here we
+	// assume when the default branch is not relevant to the test that it will
+	// be git.DefaultRef.
+	if featureflag.HeadAsDefaultBranch.IsEnabled(ctx) && expectedDefaultBranch == "" {
+		expectedDefaultBranch = git.DefaultRef
+	}
+
 	require.Equal(tb, expectedDefaultBranch, actualDefaultBranch)
 }
diff --git a/internal/gitaly/transaction_manager_test.go b/internal/gitaly/transaction_manager_test.go
index 616a013b8..da30653dc 100644
--- a/internal/gitaly/transaction_manager_test.go
+++ b/internal/gitaly/transaction_manager_test.go
@@ -21,6 +21,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
@@ -64,12 +65,14 @@ func validCustomHooks(tb testing.TB) []byte {
 func noopTransactionFinalizer() {}
 
 func TestTransactionManager(t *testing.T) {
+	testhelper.NewFeatureSets(featureflag.HeadAsDefaultBranch).Run(t, testTransactionManager)
+}
+
+func testTransactionManager(t *testing.T, ctx context.Context) {
 	umask := perm.GetUmask()
 
 	t.Parallel()
 
-	ctx := testhelper.Context(t)
-
 	type testCommit struct {
 		OID git.ObjectID
 	}
@@ -343,8 +346,11 @@ func TestTransactionManager(t *testing.T) {
 				},
 			},
 			expectedState: StateAssertion{
-				DefaultBranch: "refs/heads/parent",
-				References:    []git.Reference{{Name: "refs/heads/parent", Target: setup.Commits.First.OID.String()}},
+				DefaultBranch: testhelper.EnabledOrDisabledFlag(ctx, featureflag.HeadAsDefaultBranch,
+					git.DefaultRef,
+					git.ReferenceName("refs/heads/parent"),
+				),
+				References: []git.Reference{{Name: "refs/heads/parent", Target: setup.Commits.First.OID.String()}},
 				Database: DatabaseState{
 					string(keyAppliedLogIndex(getRepositoryID(setup.Repository))): LogIndex(1).toProto(),
 				},
@@ -417,8 +423,11 @@ func TestTransactionManager(t *testing.T) {
 				},
 			},
 			expectedState: StateAssertion{
-				DefaultBranch: "refs/heads/parent/child",
-				References:    []git.Reference{{Name: "refs/heads/parent/child", Target: setup.Commits.First.OID.String()}},
+				DefaultBranch: testhelper.EnabledOrDisabledFlag(ctx, featureflag.HeadAsDefaultBranch,
+					git.DefaultRef,
+					git.ReferenceName("refs/heads/parent/child"),
+				),
+				References: []git.Reference{{Name: "refs/heads/parent/child", Target: setup.Commits.First.OID.String()}},
 				Database: DatabaseState{
 					string(keyAppliedLogIndex(getRepositoryID(setup.Repository))): LogIndex(1).toProto(),
 				},
diff --git a/internal/metadata/featureflag/ff_head_as_default_branch.go b/internal/metadata/featureflag/ff_head_as_default_branch.go
new file mode 100644
index 000000000..f65777e9d
--- /dev/null
+++ b/internal/metadata/featureflag/ff_head_as_default_branch.go
@@ -0,0 +1,10 @@
+package featureflag
+
+// HeadAsDefaultBranch enables using HEAD as the only source of truth for the
+// default branch.
+var HeadAsDefaultBranch = NewFeatureFlag(
+	"head_as_default_branch",
+	"v15.10.0",
+	"",
+	false,
+)
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 0639e433a..c58eaac23 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -202,6 +202,12 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context {
 	// Randomly enable the use of the catfile cache in localrepo.ReadObject.
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.LocalrepoReadObjectCached, rnd.Int()%2 == 0)
 
+	// Default branch is often called but not explicitly tested very often. So
+	// to save having to change too many tests we randomly enable it when it
+	// shouldn't affect anything, and explicitly test enabled/disabled as
+	// required.
+	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.HeadAsDefaultBranch, rnd.Int()%2 == 0)
+
 	for _, opt := range opts {
 		ctx = opt(ctx)
 	}