repository: Demonstrate excessive voting for CreateRepositoryFromBundle

The `CreateRepositoryFromBundle()` RPC performs a git-clone(1) followed
by a git-fetch(1) to obtain references not obtained via the default
fetchspec. While git-clone(1) behaves just fine with regards to the
reference transaction hook, git-fetch(1) will vote twice per reference
that's being updated. For repos which have lots of references which are
neither branches nor tags, this will thus lead to excessive voting.

Extend the existing testcase for transactional voting to create such
references in order to demonstrate this behaviour.

1 files changed, 51 insertions, 19 deletions

diff --git a/internal/gitaly/service/repository/create_from_bundle_test.go b/internal/gitaly/service/repository/create_from_bundle_test.go
index 2b530ff29..98b544e6a 100644
--- a/internal/gitaly/service/repository/create_from_bundle_test.go
+++ b/internal/gitaly/service/repository/create_from_bundle_test.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -104,6 +105,16 @@ func TestServer_CreateRepositoryFromBundle_transactional(t *testing.T) {
 	cfg, repoProto, repoPath, client := setupRepositoryService(t,
 		testserver.WithTransactionManager(txManager))
 
+	masterOID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "refs/heads/master"))
+	featureOID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "refs/heads/feature"))
+
+	// keep-around refs are not cloned in the initial step, but are added via the second call to
+	// git-fetch(1). We thus create some of them to exercise their behaviour with regards to
+	// transactional voting.
+	for _, keepAroundRef := range []string{"refs/keep-around/1", "refs/keep-around/2"} {
+		gittest.Exec(t, cfg, "-C", repoPath, "update-ref", keepAroundRef, masterOID)
+	}
+
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 	ctx, err := txinfo.InjectTransaction(ctx, 1, "primary", true)
@@ -120,7 +131,8 @@ func TestServer_CreateRepositoryFromBundle_transactional(t *testing.T) {
 		},
 	}))
 
-	bundle := gittest.Exec(t, cfg, "-C", repoPath, "bundle", "create", "-", "master", "feature")
+	bundle := gittest.Exec(t, cfg, "-C", repoPath, "bundle", "create", "-",
+		"refs/heads/master", "refs/heads/feature", "refs/keep-around/1", "refs/keep-around/2")
 	require.Greater(t, len(bundle), 100*1024)
 
 	_, err = io.Copy(streamio.NewWriter(func(p []byte) error {
@@ -134,25 +146,45 @@ func TestServer_CreateRepositoryFromBundle_transactional(t *testing.T) {
 	_, err = stream.CloseAndRecv()
 	require.NoError(t, err)
 
-	masterOID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "master"))
-	featureOID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "feature"))
-
-	// This accounts for the first two votes which first do a git-clone(1) followed by a fetch.
-	// Given that voting is done via git's reference-transaction hook, the format is `<oldrev>
-	// <newrev> <reference>`.
-	fetchInput := fmt.Sprintf("%s %s refs/heads/feature\n%s %s refs/heads/master\n",
-		git.ZeroOID, featureOID, git.ZeroOID, masterOID)
-
-	// And this accounts for the final vote in `Create()`, which does vote on all references in
-	// the target repo as listed by git-for-each-ref(1).
-	refsInput := fmt.Sprintf("%s commit\trefs/heads/feature\n%s commit\trefs/heads/master\n",
-		featureOID, masterOID)
+	var votingInput []string
+
+	// This accounts for the first two votes via git-clone(1). Given that git-clone(1) creates
+	// all references in a single reference transaction, the hook is executed twice for all
+	// fetched references (once for "prepare", once for "commit").
+	votingInput = append(votingInput,
+		fmt.Sprintf("%s %s refs/heads/feature\n%s %s refs/heads/master\n", git.ZeroOID, featureOID, git.ZeroOID, masterOID),
+		fmt.Sprintf("%s %s refs/heads/feature\n%s %s refs/heads/master\n", git.ZeroOID, featureOID, git.ZeroOID, masterOID),
+	)
+
+	// Keep-around references are not fetched via git-clone(1) because non-mirror clones only
+	// fetch branches and tags. These additional references are thus obtained via git-fetch(1),
+	// which by default creates one reference transaction per reference. We thus see two commits
+	// (again, for "prepare" and "commit" phases) per reference.
+	votingInput = append(votingInput,
+		fmt.Sprintf("%s %s refs/keep-around/2\n", git.ZeroOID, masterOID),
+		fmt.Sprintf("%s %s refs/keep-around/2\n", git.ZeroOID, masterOID),
+		fmt.Sprintf("%s %s refs/keep-around/1\n", git.ZeroOID, masterOID),
+		fmt.Sprintf("%s %s refs/keep-around/1\n", git.ZeroOID, masterOID),
+	)
+
+	// And this is the final vote in Create(), which does a git-for-each-ref(1) in the target
+	// repository and then manually invokes the hook. The format is thus different from above
+	// votes.
+	votingInput = append(votingInput,
+		strings.Join([]string{
+			featureOID + " commit\trefs/heads/feature",
+			masterOID + " commit\trefs/heads/master",
+			masterOID + " commit\trefs/keep-around/1",
+			masterOID + " commit\trefs/keep-around/2",
+		}, "\n")+"\n",
+	)
+
+	var expectedVotes []voting.Vote
+	for _, expectedVote := range votingInput {
+		expectedVotes = append(expectedVotes, voting.VoteFromData([]byte(expectedVote)))
+	}
 
-	require.Equal(t, []voting.Vote{
-		voting.VoteFromData([]byte(fetchInput)),
-		voting.VoteFromData([]byte(fetchInput)),
-		voting.VoteFromData([]byte(refsInput)),
-	}, votes)
+	require.Equal(t, votes, expectedVotes)
 }
 
 func TestServer_CreateRepositoryFromBundle_failed_invalid_bundle(t *testing.T) {