Merge branch 'pks-create-fork-test-refactorings' into 'master'

repository: Test refactorings and SHA256 object format support

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

Merged-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>

2 files changed, 37 insertions, 74 deletions

diff --git a/internal/git/gittest/repo.go b/internal/git/gittest/repo.go
index 208d189f8..3c7a91468 100644
--- a/internal/git/gittest/repo.go
+++ b/internal/git/gittest/repo.go
@@ -20,6 +20,7 @@ import (
 	"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/testhelper"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
@@ -98,12 +99,26 @@ type CreateRepositoryConfig struct {
 }
 
 func dialService(tb testing.TB, ctx context.Context, cfg config.Cfg) *grpc.ClientConn {
+	tb.Helper()
+
 	dialOptions := []grpc.DialOption{internalclient.UnaryInterceptor(), internalclient.StreamInterceptor()}
 	if cfg.Auth.Token != "" {
 		dialOptions = append(dialOptions, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(cfg.Auth.Token)))
 	}
 
-	conn, err := client.DialContext(ctx, cfg.SocketPath, dialOptions)
+	var addr string
+	switch {
+	case cfg.SocketPath != "" && cfg.SocketPath != testcfg.UnconfiguredSocketPath:
+		addr = cfg.SocketPath
+	case cfg.ListenAddr != "":
+		addr = cfg.ListenAddr
+	case cfg.TLSListenAddr != "":
+		addr = cfg.TLSListenAddr
+	default:
+		require.FailNow(tb, "cannot dial service without configured address")
+	}
+
+	conn, err := client.DialContext(ctx, addr, dialOptions)
 	require.NoError(tb, err)
 	return conn
 }
diff --git a/internal/gitaly/service/repository/create_fork_test.go b/internal/gitaly/service/repository/create_fork_test.go
index cb936e263..b21ff5321 100644
--- a/internal/gitaly/service/repository/create_fork_test.go
+++ b/internal/gitaly/service/repository/create_fork_test.go
@@ -1,11 +1,7 @@
-//go:build !gitaly_test_sha256
-
 package repository
 
 import (
 	"bytes"
-	"crypto/tls"
-	"crypto/x509"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -13,8 +9,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	gitalyauth "gitlab.com/gitlab-org/gitaly/v15/auth"
-	"gitlab.com/gitlab-org/gitaly/v15/client"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm"
@@ -25,9 +19,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	gitalyx509 "gitlab.com/gitlab-org/gitaly/v15/internal/x509"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
-	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/status"
 )
 
@@ -36,8 +28,8 @@ func TestCreateFork_successful(t *testing.T) {
 	// certificates once. Changing injected certs during our tests is thus not going to fly well
 	// and would cause failure. We should eventually address this and provide better testing
 	// utilities around this, but now's not the time.
-	certPool, tlsConfig := injectCustomCATestCerts(t)
-	ctx := testhelper.Context(t)
+	certFile, keyFile := testhelper.GenerateCerts(t)
+	t.Setenv(gitalyx509.SSLCertFile, certFile)
 
 	for _, tt := range []struct {
 		name   string
@@ -57,33 +49,24 @@ func TestCreateFork_successful(t *testing.T) {
 			testcfg.BuildGitalyHooks(t, cfg)
 			testcfg.BuildGitalySSH(t, cfg)
 
-			createRepoConfig := gittest.CreateRepositoryConfig{
-				Seed: gittest.SeedGitLabTest,
-			}
-			getReplicaPathConfig := gittest.GetReplicaPathConfig{}
-
 			var client gitalypb.RepositoryServiceClient
 			if tt.secure {
-				cfg.TLS = tlsConfig
+				cfg.TLS = config.TLS{
+					CertPath: certFile,
+					KeyPath:  keyFile,
+				}
 				cfg.TLSListenAddr = "localhost:0"
 
-				_, addr := runRepositoryService(t, cfg, nil)
-				cfg.TLSListenAddr = addr
-
-				var conn *grpc.ClientConn
-				client, conn = newSecureRepoClient(t, cfg.TLSListenAddr, cfg.Auth.Token, certPool)
-				t.Cleanup(func() { conn.Close() })
-
-				createRepoConfig.ClientConn = conn
-				getReplicaPathConfig.ClientConn = conn
+				client, cfg.TLSListenAddr = runRepositoryService(t, cfg, nil)
 			} else {
 				client, cfg.SocketPath = runRepositoryService(t, cfg, nil)
 			}
 
-			repo, _ := gittest.CreateRepository(t, ctx, cfg, createRepoConfig)
-
+			ctx := testhelper.Context(t)
 			ctx = testhelper.MergeOutgoingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg))
 
+			repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
 			forkedRepo := &gitalypb.Repository{
 				RelativePath: gittest.NewRepositoryName(t),
 				StorageName:  repo.GetStorageName(),
@@ -95,7 +78,7 @@ func TestCreateFork_successful(t *testing.T) {
 			})
 			require.NoError(t, err)
 
-			replicaPath := gittest.GetReplicaPath(t, ctx, cfg, forkedRepo, getReplicaPathConfig)
+			replicaPath := gittest.GetReplicaPath(t, ctx, cfg, forkedRepo)
 			forkedRepoPath := filepath.Join(cfg.Storages[0].Path, replicaPath)
 
 			gittest.Exec(t, cfg, "-C", forkedRepoPath, "fsck")
@@ -111,12 +94,7 @@ func TestCreateFork_refs(t *testing.T) {
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
-	cfg := testcfg.Build(t)
-	testcfg.BuildGitalyHooks(t, cfg)
-	testcfg.BuildGitalySSH(t, cfg)
-
-	client, socketPath := runRepositoryService(t, cfg, nil)
-	cfg.SocketPath = socketPath
+	cfg, client := setupRepositoryServiceWithoutRepo(t)
 
 	sourceRepo, sourceRepoPath := gittest.CreateRepository(t, ctx, cfg)
 
@@ -168,13 +146,7 @@ func TestCreateFork_refs(t *testing.T) {
 func TestCreateFork_fsck(t *testing.T) {
 	t.Parallel()
 
-	cfg := testcfg.Build(t)
-
-	testcfg.BuildGitalyHooks(t, cfg)
-	testcfg.BuildGitalySSH(t, cfg)
-
-	client, socketPath := runRepositoryService(t, cfg, nil)
-	cfg.SocketPath = socketPath
+	cfg, client := setupRepositoryServiceWithoutRepo(t)
 
 	ctx := testhelper.Context(t)
 	ctx = testhelper.MergeOutgoingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg))
@@ -223,7 +195,6 @@ func TestCreateFork_fsck(t *testing.T) {
 
 func TestCreateFork_targetExists(t *testing.T) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
 	for _, tc := range []struct {
 		desc                          string
@@ -259,10 +230,12 @@ func TestCreateFork_targetExists(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			cfg, repo, _, client := setupRepositoryService(t, ctx)
-
+			ctx := testhelper.Context(t)
+			cfg, client := setupRepositoryServiceWithoutRepo(t)
 			ctx = testhelper.MergeOutgoingMetadata(ctx, testcfg.GitalyServersMetadataFromCfg(t, cfg))
 
+			repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
 			forkedRepo := &gitalypb.Repository{
 				// As this test can run with Praefect in front of it, we'll use the next replica path Praefect will
 				// assign in order to ensure this repository creation conflicts even with Praefect in front of it.
@@ -285,8 +258,11 @@ func TestCreateFork_targetExists(t *testing.T) {
 
 func TestCreateFork_validate(t *testing.T) {
 	t.Parallel()
+
 	ctx := testhelper.Context(t)
-	_, repo, _, cli := setupRepositoryService(t, ctx)
+	cfg, cli := setupRepositoryServiceWithoutRepo(t)
+	repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
 	for _, tc := range []struct {
 		desc        string
 		req         *gitalypb.CreateForkRequest
@@ -317,31 +293,3 @@ func TestCreateFork_validate(t *testing.T) {
 		})
 	}
 }
-
-func injectCustomCATestCerts(t *testing.T) (*x509.CertPool, config.TLS) {
-	certFile, keyFile := testhelper.GenerateCerts(t)
-	t.Setenv(gitalyx509.SSLCertFile, certFile)
-
-	caPEMBytes := testhelper.MustReadFile(t, certFile)
-	pool := x509.NewCertPool()
-	require.True(t, pool.AppendCertsFromPEM(caPEMBytes))
-
-	return pool, config.TLS{CertPath: certFile, KeyPath: keyFile}
-}
-
-func newSecureRepoClient(tb testing.TB, addr, token string, pool *x509.CertPool) (gitalypb.RepositoryServiceClient, *grpc.ClientConn) {
-	tb.Helper()
-
-	connOpts := []grpc.DialOption{
-		grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{
-			RootCAs:    pool,
-			MinVersion: tls.VersionTLS12,
-		})),
-		grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(token)),
-	}
-
-	conn, err := client.Dial(addr, connOpts)
-	require.NoError(tb, err)
-
-	return gitalypb.NewRepositoryServiceClient(conn), conn
-}