Merge branch 'pks-smarthttp-sha256-testing' into 'master'

smarthttp: Convert InfoRefs and UploadPack to test with SHA256 object hash

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5307

Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>

3 files changed, 51 insertions, 82 deletions

diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go
index 8d6301f4c..3a29d5158 100644
--- a/internal/gitaly/service/smarthttp/inforefs_test.go
+++ b/internal/gitaly/service/smarthttp/inforefs_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package smarthttp
 
 import (
@@ -170,19 +168,15 @@ func TestInfoRefsUploadPack_partialClone(t *testing.T) {
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
-
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
 	ctx := testhelper.Context(t)
 
-	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
+	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
 
-	request := &gitalypb.InfoRefsRequest{
+	partialResponse, err := makeInfoRefsUploadPackRequest(t, ctx, cfg.SocketPath, cfg.Auth.Token, &gitalypb.InfoRefsRequest{
 		Repository: repo,
-	}
-
-	partialResponse, err := makeInfoRefsUploadPackRequest(t, ctx, cfg.SocketPath, cfg.Auth.Token, request)
+	})
 	require.NoError(t, err)
 	partialRefs := stats.ReferenceDiscovery{}
 	err = partialRefs.Parse(bytes.NewReader(partialResponse))
@@ -230,19 +224,15 @@ func TestInfoRefsUploadPack_gitProtocol(t *testing.T) {
 	})
 	cfg.SocketPath = server.Address()
 
-	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
-
-	rpcRequest := &gitalypb.InfoRefsRequest{
-		Repository:  repo,
-		GitProtocol: git.ProtocolV2,
-	}
+	repo, _ := gittest.CreateRepository(t, ctx, cfg)
 
 	client, conn := newSmartHTTPClient(t, server.Address(), cfg.Auth.Token)
 	defer testhelper.MustClose(t, conn)
 
-	c, err := client.InfoRefsUploadPack(ctx, rpcRequest)
+	c, err := client.InfoRefsUploadPack(ctx, &gitalypb.InfoRefsRequest{
+		Repository:  repo,
+		GitProtocol: git.ProtocolV2,
+	})
 	require.NoError(t, err)
 
 	for {
@@ -310,10 +300,10 @@ func TestInfoRefsReceivePack_hiddenRefs(t *testing.T) {
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
 	ctx := testhelper.Context(t)
 
-	repoProto, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
+	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
+	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
+
 	txManager := transaction.NewManager(cfg, backchannel.NewRegistry())
 
 	pool, err := objectpool.Create(
diff --git a/internal/gitaly/service/smarthttp/testhelper_test.go b/internal/gitaly/service/smarthttp/testhelper_test.go
index e5b709a6c..f3a731af9 100644
--- a/internal/gitaly/service/smarthttp/testhelper_test.go
+++ b/internal/gitaly/service/smarthttp/testhelper_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package smarthttp
 
 import (
diff --git a/internal/gitaly/service/smarthttp/upload_pack_test.go b/internal/gitaly/service/smarthttp/upload_pack_test.go
index 1da6b3d14..73e0f67d5 100644
--- a/internal/gitaly/service/smarthttp/upload_pack_test.go
+++ b/internal/gitaly/service/smarthttp/upload_pack_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package smarthttp
 
 import (
@@ -15,7 +13,6 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus"
 	promtest "github.com/prometheus/client_golang/prometheus/testutil"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	gitalyauth "gitlab.com/gitlab-org/gitaly/v15/auth"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
@@ -71,21 +68,15 @@ func TestServer_PostUploadWithChannel(t *testing.T) {
 
 func testServerPostUpload(t *testing.T, ctx context.Context, makeRequest requestMaker, opts ...testcfg.Option) {
 	cfg := testcfg.Build(t, opts...)
+	testcfg.BuildGitalyHooks(t, cfg)
 
 	negotiationMetrics := prometheus.NewCounterVec(prometheus.CounterOpts{}, []string{"feature"})
 	cfg.SocketPath = runSmartHTTPServer(t, cfg, WithPackfileNegotiationMetrics(negotiationMetrics))
 
-	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
-	_, localRepoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
-
-	testcfg.BuildGitalyHooks(t, cfg)
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
+	_, localRepoPath := gittest.CreateRepository(t, ctx, cfg)
 
-	oldCommit, err := git.ObjectHashSHA1.FromHex("1e292f8fedd741b75372e19097c76d327140c312") // refs/heads/master
-	require.NoError(t, err)
+	oldCommit := gittest.WriteCommit(t, cfg, repoPath)
 	newCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master"), gittest.WithParents(oldCommit))
 
 	// UploadPack request is a "want" packet line followed by a packet flush, then many "have" packets followed by a packet flush.
@@ -176,8 +167,10 @@ func testServerPostUploadPackGitConfigOptions(t *testing.T, ctx context.Context,
 
 		// The failure message proves that upload-pack failed because of
 		// GitConfigOptions, and that proves that passing GitConfigOptions works.
-		expected := fmt.Sprintf("0049ERR upload-pack: not our ref %v", hiddenID)
-		require.Equal(t, expected, response.String(), "Ref is hidden, expected error message did not appear")
+		require.Equal(t,
+			gittest.Pktlinef(t, "ERR upload-pack: not our ref %v", hiddenID),
+			response.String(),
+		)
 	})
 }
 
@@ -201,13 +194,12 @@ func testServerPostUploadPackGitProtocol(t *testing.T, ctx context.Context, make
 	})
 	cfg.SocketPath = server.Address()
 
-	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
+	repo, _ := gittest.CreateRepository(t, ctx, cfg)
 
 	// command=ls-refs does not exist in protocol v0, so if this succeeds, we're talking v2
 	requestBody := &bytes.Buffer{}
 	gittest.WritePktlineString(t, requestBody, "command=ls-refs\n")
+	gittest.WritePktlineString(t, requestBody, fmt.Sprintf("object-format=%s\n", gittest.DefaultObjectHash.Format))
 	gittest.WritePktlineDelim(t, requestBody)
 	gittest.WritePktlineString(t, requestBody, "peel\n")
 	gittest.WritePktlineString(t, requestBody, "symrefs\n")
@@ -244,21 +236,19 @@ func testServerPostUploadPackSuppressDeepenExitError(t *testing.T, ctx context.C
 	cfg := testcfg.Build(t, opts...)
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
 
-	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
-
-	requestBody := &bytes.Buffer{}
-	gittest.WritePktlineString(t, requestBody, fmt.Sprintf("want e63f41fe459e62e1228fcef60d7189127aeba95a %s\n", clientCapabilities))
-	gittest.WritePktlineString(t, requestBody, "deepen 1")
-	gittest.WritePktlineFlush(t, requestBody)
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
+	commitID := gittest.WriteCommit(t, cfg, repoPath)
 
-	rpcRequest := &gitalypb.PostUploadPackRequest{Repository: repo}
-	response, err := makeRequest(t, ctx, cfg.SocketPath, cfg.Auth.Token, rpcRequest, requestBody)
+	var requestBody bytes.Buffer
+	gittest.WritePktlineString(t, &requestBody, fmt.Sprintf("want %s %s\n", commitID, clientCapabilities))
+	gittest.WritePktlineString(t, &requestBody, "deepen 1")
+	gittest.WritePktlineFlush(t, &requestBody)
 
-	// This assertion is the main reason this test exists.
-	assert.NoError(t, err)
-	assert.Equal(t, `0034shallow e63f41fe459e62e1228fcef60d7189127aeba95a0000`, response.String())
+	response, err := makeRequest(t, ctx, cfg.SocketPath, cfg.Auth.Token, &gitalypb.PostUploadPackRequest{
+		Repository: repo,
+	}, &requestBody)
+	require.NoError(t, err)
+	require.Equal(t, gittest.Pktlinef(t, "shallow %s", commitID)+"0000", response.String())
 }
 
 func TestServer_PostUploadPack_usesPackObjectsHook(t *testing.T) {
@@ -293,22 +283,19 @@ func testServerPostUploadPackUsesPackObjectsHook(t *testing.T, ctx context.Conte
 
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
 
-	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
-
-	oldHead := bytes.TrimSpace(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "master~"))
-	newHead := bytes.TrimSpace(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "master"))
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
+	oldHead := gittest.WriteCommit(t, cfg, repoPath)
+	newHead := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(oldHead), gittest.WithBranch("master"))
 
-	requestBuffer := &bytes.Buffer{}
-	gittest.WritePktlineString(t, requestBuffer, fmt.Sprintf("want %s %s\n", newHead, clientCapabilities))
-	gittest.WritePktlineFlush(t, requestBuffer)
-	gittest.WritePktlineString(t, requestBuffer, fmt.Sprintf("have %s\n", oldHead))
-	gittest.WritePktlineFlush(t, requestBuffer)
+	var requestBuffer bytes.Buffer
+	gittest.WritePktlineString(t, &requestBuffer, fmt.Sprintf("want %s %s\n", newHead, clientCapabilities))
+	gittest.WritePktlineFlush(t, &requestBuffer)
+	gittest.WritePktlineString(t, &requestBuffer, fmt.Sprintf("have %s\n", oldHead))
+	gittest.WritePktlineFlush(t, &requestBuffer)
 
 	_, err := makeRequest(t, ctx, cfg.SocketPath, cfg.Auth.Token, &gitalypb.PostUploadPackRequest{
 		Repository: repo,
-	}, requestBuffer)
+	}, &requestBuffer)
 	require.NoError(t, err)
 
 	contents := testhelper.MustReadFile(t, outputPath)
@@ -461,19 +448,21 @@ func TestServer_PostUploadPackWithSidechannel_partialClone(t *testing.T) {
 
 func testServerPostUploadPackPartialClone(t *testing.T, ctx context.Context, makeRequest requestMaker, opts ...testcfg.Option) {
 	cfg := testcfg.Build(t, opts...)
+	testcfg.BuildGitalyHooks(t, cfg)
 
 	negotiationMetrics := prometheus.NewCounterVec(prometheus.CounterOpts{}, []string{"feature"})
 	cfg.SocketPath = runSmartHTTPServer(t, cfg, WithPackfileNegotiationMetrics(negotiationMetrics))
 
-	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	_, localRepoPath := gittest.CreateRepository(t, ctx, cfg)
 
-	testcfg.BuildGitalyHooks(t, cfg)
+	blobLessThanLimit := gittest.WriteBlob(t, cfg, repoPath, bytes.Repeat([]byte{1}, 100))
+	blobGreaterThanLimit := gittest.WriteBlob(t, cfg, repoPath, bytes.Repeat([]byte{1}, 1000))
 
-	oldCommit, err := git.ObjectHashSHA1.FromHex("1e292f8fedd741b75372e19097c76d327140c312") // refs/heads/master
-	require.NoError(t, err)
+	oldCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithTreeEntries(
+		gittest.TreeEntry{Path: "small", Mode: "100644", OID: blobLessThanLimit},
+		gittest.TreeEntry{Path: "large", Mode: "100644", OID: blobGreaterThanLimit},
+	))
 	newCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master"), gittest.WithParents(oldCommit))
 
 	var requestBuffer bytes.Buffer
@@ -494,12 +483,6 @@ func testServerPostUploadPackPartialClone(t *testing.T, ctx context.Context, mak
 		"-C", localRepoPath, "unpack-objects", fmt.Sprintf("--pack_header=%d,%d", version, entries),
 	)
 
-	// a4a132b1b0d6720ca9254440a7ba8a6b9bbd69ec is README.md, which is a small file
-	blobLessThanLimit := git.ObjectID("a4a132b1b0d6720ca9254440a7ba8a6b9bbd69ec")
-
-	// c1788657b95998a2f177a4f86d68a60f2a80117f is CONTRIBUTING.md, which is > 200 bytes
-	blobGreaterThanLimit := git.ObjectID("c1788657b95998a2f177a4f86d68a60f2a80117f")
-
 	gittest.RequireObjectExists(t, cfg, localRepoPath, blobLessThanLimit)
 	gittest.RequireObjectExists(t, cfg, repoPath, blobGreaterThanLimit)
 	gittest.RequireObjectNotExists(t, cfg, localRepoPath, blobGreaterThanLimit)
@@ -527,9 +510,7 @@ func testServerPostUploadPackAllowAnySHA1InWant(t *testing.T, ctx context.Contex
 	cfg := testcfg.Build(t, opts...)
 	cfg.SocketPath = runSmartHTTPServer(t, cfg)
 
-	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		Seed: gittest.SeedGitLabTest,
-	})
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	_, localRepoPath := gittest.CreateRepository(t, ctx, cfg)
 
 	testcfg.BuildGitalyHooks(t, cfg)