diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go index ae4ebcb61259d3c66da829ab6bf0ab0e1db6eaa3..bca5bfdc96a1ebc3b9c028a638f686dc31e1a1ef 100644 --- a/internal/praefect/coordinator.go +++ b/internal/praefect/coordinator.go @@ -6,6 +6,7 @@ import ( "github.com/golang/protobuf/proto" "github.com/sirupsen/logrus" + "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/praefect/config" "gitlab.com/gitlab-org/gitaly/internal/praefect/datastore" "gitlab.com/gitlab-org/gitaly/internal/praefect/grpc-proxy/proxy" @@ -58,13 +59,20 @@ func (c *Coordinator) directRepositoryScopedMessage(ctx context.Context, mi prot targetRepo, err := mi.TargetRepo(m) if err != nil { if err == protoregistry.ErrTargetRepoMissing { - return nil, status.Errorf(codes.InvalidArgument, err.Error()) + return nil, helper.ErrInvalidArgument(err) } return nil, err } + if targetRepo.StorageName == "" || targetRepo.RelativePath == "" { + return nil, helper.ErrInvalidArgumentf("target repo is invalid") + } + shard, err := c.nodeMgr.GetShard(targetRepo.GetStorageName()) if err != nil { + if err == nodes.ErrVirtualStorageNotExist { + return nil, helper.ErrInvalidArgument(err) + } return nil, err } @@ -75,7 +83,7 @@ func (c *Coordinator) directRepositoryScopedMessage(ctx context.Context, mi prot if err = c.rewriteStorageForRepositoryMessage(mi, m, peeker, primary.GetStorage()); err != nil { if err == protoregistry.ErrTargetRepoMissing { - return nil, status.Errorf(codes.InvalidArgument, err.Error()) + return nil, helper.ErrInvalidArgument(err) } return nil, err @@ -127,6 +135,9 @@ func (c *Coordinator) StreamDirector(ctx context.Context, fullMethodName string, // any RPC that gets proxied through praefect must be repository scoped. shard, err := c.nodeMgr.GetShard(c.conf.VirtualStorages[0].Name) if err != nil { + if err == nodes.ErrVirtualStorageNotExist { + return nil, status.Errorf(codes.InvalidArgument, err.Error()) + } return nil, err } diff --git a/internal/praefect/nodes/manager.go b/internal/praefect/nodes/manager.go index e9f2de58afb74d276cd2d9803ed23fb8e85fef12..9a8600eb103a72559eb8345d262f9682769dedd3 100644 --- a/internal/praefect/nodes/manager.go +++ b/internal/praefect/nodes/manager.go @@ -156,11 +156,14 @@ func (n *Mgr) Start(bootstrapInterval, monitorInterval time.Duration) { } } +// ErrVirtualStorageNotExist indicates the node manager is not aware of the virtual storage for which a shard is being requested +var ErrVirtualStorageNotExist = errors.New("virtual storage does not exist") + // GetShard retrieves a shard for a virtual storage name func (n *Mgr) GetShard(virtualStorageName string) (Shard, error) { shard, ok := n.shards[virtualStorageName] if !ok { - return nil, errors.New("virtual storage does not exist") + return nil, ErrVirtualStorageNotExist } if n.failoverEnabled { diff --git a/internal/praefect/protoregistry/find_oid.go b/internal/praefect/protoregistry/find_oid.go index 8b73d494315655d9f76cc126c7602a067f5e1c98..7d8691fe3c1e712cd0e631e88023145b3832cf07 100644 --- a/internal/praefect/protoregistry/find_oid.go +++ b/internal/praefect/protoregistry/find_oid.go @@ -3,6 +3,7 @@ package protoregistry import ( "errors" "fmt" + "math" "reflect" "regexp" "strconv" @@ -22,6 +23,9 @@ var ErrTargetRepoMissing = errors.New("empty Repository") func reflectFindRepoTarget(pbMsg proto.Message, targetOID []int) (*gitalypb.Repository, error) { msgV, e := reflectFindOID(pbMsg, targetOID) if e != nil { + if e == ErrProtoFieldEmpty { + return nil, ErrTargetRepoMissing + } return nil, e } @@ -47,6 +51,9 @@ func reflectFindStorage(pbMsg proto.Message, targetOID []int) (string, error) { return targetRepo, nil } +// ErrProtoFieldEmpty indicates the protobuf field is empty +var ErrProtoFieldEmpty = errors.New("proto field is empty") + // reflectFindOID finds the target repository by using the OID to // navigate the struct tags // Warning: this reflection filled function is full of forbidden dark elf magic @@ -74,7 +81,51 @@ const ( protobufTagRegexFieldGroup = 2 ) +// TODO: This code is copied from the go standard library's reflect package in go 1.13. Once we deprecate support +// for go 1.12, we need to remove this code and call value.isZero directly +func isZero(v reflect.Value) bool { + switch v.Kind() { + case reflect.Bool: + return !v.Bool() + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return v.Int() == 0 + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: + return v.Uint() == 0 + case reflect.Float32, reflect.Float64: + return math.Float64bits(v.Float()) == 0 + case reflect.Complex64, reflect.Complex128: + c := v.Complex() + return math.Float64bits(real(c)) == 0 && math.Float64bits(imag(c)) == 0 + case reflect.Array: + for i := 0; i < v.Len(); i++ { + if !isZero(v.Index(i)) { + return false + } + } + return true + case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Ptr, reflect.Slice, reflect.UnsafePointer: + return v.IsNil() + case reflect.String: + return v.Len() == 0 + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + if !isZero(v.Field(i)) { + return false + } + } + return true + default: + // This should never happens, but will act as a safeguard for + // later, as a default value doesn't makes sense here. + panic(&reflect.ValueError{"reflect.Value.IsZero", v.Kind()}) + } +} + func findProtoField(msgV reflect.Value, protoField int) (reflect.Value, error) { + if isZero(msgV) { + return reflect.Value{}, ErrProtoFieldEmpty + } + msgV = reflect.Indirect(msgV) for i := 0; i < msgV.NumField(); i++ { field := msgV.Type().Field(i) @@ -115,6 +166,10 @@ func tryNumberedField(field reflect.StructField, protoField int) (bool, error) { } func tryOneOfField(msgV reflect.Value, field reflect.StructField, protoField int) (reflect.Value, bool) { + if isZero(msgV) { + return reflect.Value{}, false + } + oneOfTag := field.Tag.Get(protobufOneOfTag) if oneOfTag == "" { return reflect.Value{}, false // empty tag means this is not a oneOf field diff --git a/internal/service/repository/apply_gitattributes_test.go b/internal/service/repository/apply_gitattributes_test.go index 0dd242b92f999ab1b422028893b32d6a035a1020..b07fa4e4c3dc56bbb039418f41f3d81338599d6c 100644 --- a/internal/service/repository/apply_gitattributes_test.go +++ b/internal/service/repository/apply_gitattributes_test.go @@ -14,8 +14,8 @@ import ( ) func TestApplyGitattributesSuccess(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -66,8 +66,8 @@ func TestApplyGitattributesSuccess(t *testing.T) { } func TestApplyGitattributesFailure(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/archive_test.go b/internal/service/repository/archive_test.go index 5e129b8f9088ab3bb3cc1a7f67d8ae57890a02b7..93102a87f729c9321b5912b22b2d4b2e8a187129 100644 --- a/internal/service/repository/archive_test.go +++ b/internal/service/repository/archive_test.go @@ -17,8 +17,8 @@ import ( ) func TestGetArchiveSuccess(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -121,8 +121,8 @@ func TestGetArchiveSuccess(t *testing.T) { } func TestGetArchiveFailure(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -236,8 +236,8 @@ func TestGetArchiveFailure(t *testing.T) { } func TestGetArchivePathInjection(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/backup_custom_hooks_test.go b/internal/service/repository/backup_custom_hooks_test.go index abd3df248dab814c4a6c4a60f0c308bc1851a968..6494f7d81057fb4e5f328d62aecd9ff1531448fe 100644 --- a/internal/service/repository/backup_custom_hooks_test.go +++ b/internal/service/repository/backup_custom_hooks_test.go @@ -18,8 +18,8 @@ import ( ) func TestSuccessfullBackupCustomHooksRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -67,8 +67,8 @@ func TestSuccessfullBackupCustomHooksRequest(t *testing.T) { } func TestSuccessfullBackupCustomHooksSymlink(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -106,8 +106,8 @@ func TestSuccessfullBackupCustomHooksSymlink(t *testing.T) { } func TestSuccessfullBackupCustomHooksRequestWithNoHooks(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/calculate_checksum_test.go b/internal/service/repository/calculate_checksum_test.go index b29d245dc7c4095f970457b72a1758bd7e819717..450127165afa0dff7ba8080e26458cdbd9271892 100644 --- a/internal/service/repository/calculate_checksum_test.go +++ b/internal/service/repository/calculate_checksum_test.go @@ -13,8 +13,8 @@ import ( ) func TestSuccessfulCalculateChecksum(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -70,8 +70,8 @@ func TestRefWhitelist(t *testing.T) { } func TestEmptyRepositoryCalculateChecksum(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -89,8 +89,8 @@ func TestEmptyRepositoryCalculateChecksum(t *testing.T) { } func TestBrokenRepositoryCalculateChecksum(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -110,8 +110,8 @@ func TestBrokenRepositoryCalculateChecksum(t *testing.T) { } func TestFailedCalculateChecksum(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -145,8 +145,8 @@ func TestFailedCalculateChecksum(t *testing.T) { } func TestInvalidRefsCalculateChecksum(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/cleanup_test.go b/internal/service/repository/cleanup_test.go index bb0f2e4e106b82be723d4ecea82def49bceeee8c..6b36eae92cf923407e7e007220fc740f757ff742 100644 --- a/internal/service/repository/cleanup_test.go +++ b/internal/service/repository/cleanup_test.go @@ -15,8 +15,8 @@ import ( ) func TestCleanupDeletesRefsLocks(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -58,8 +58,8 @@ func TestCleanupDeletesRefsLocks(t *testing.T) { } func TestCleanupDeletesPackedRefsLock(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -127,8 +127,8 @@ func TestCleanupDeletesPackedRefsLock(t *testing.T) { // TODO: replace emulated rebase RPC with actual // https://gitlab.com/gitlab-org/gitaly/issues/1750 func TestCleanupDeletesStaleWorktrees(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -199,8 +199,8 @@ func TestCleanupDisconnectedWorktrees(t *testing.T) { worktreeAdminDir = "worktrees" ) - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -258,8 +258,8 @@ func TestCleanupDisconnectedWorktrees(t *testing.T) { } func TestCleanupFileLocks(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/config_test.go b/internal/service/repository/config_test.go index de7f099e9e6e9c0f9d9ef8635b65f494ff6874cf..9cc0fcea142d0ea7194c10b09e851b52a3d503d1 100644 --- a/internal/service/repository/config_test.go +++ b/internal/service/repository/config_test.go @@ -14,8 +14,8 @@ import ( ) func TestDeleteConfig(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -74,8 +74,8 @@ func TestDeleteConfig(t *testing.T) { } func TestSetConfig(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/create_bundle_test.go b/internal/service/repository/create_bundle_test.go index c70373121fbdab49d10761f8e373a7c88031d9a2..966eead17bb9edf3636b2b7efc1759021a739200 100644 --- a/internal/service/repository/create_bundle_test.go +++ b/internal/service/repository/create_bundle_test.go @@ -15,8 +15,8 @@ import ( ) func TestSuccessfulCreateBundleRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -53,8 +53,8 @@ func TestSuccessfulCreateBundleRequest(t *testing.T) { } func TestFailedCreateBundleRequestDueToValidations(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/create_from_bundle_test.go b/internal/service/repository/create_from_bundle_test.go index 161fc1c9b02b075d87ce91925e30e8290871fa61..e6145757f0d04e42c0458c2935f1c748972d5732 100644 --- a/internal/service/repository/create_from_bundle_test.go +++ b/internal/service/repository/create_from_bundle_test.go @@ -18,8 +18,8 @@ import ( ) func TestSuccessfulCreateRepositoryFromBundleRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -85,8 +85,8 @@ func TestSuccessfulCreateRepositoryFromBundleRequest(t *testing.T) { } func TestFailedCreateRepositoryFromBundleRequestDueToValidations(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/create_from_snapshot_test.go b/internal/service/repository/create_from_snapshot_test.go index 2abc1aad501bd2c72c7afef40a932d9c09d7a976..912432759a9b824a4d97941f8861c6812d8d8cc0 100644 --- a/internal/service/repository/create_from_snapshot_test.go +++ b/internal/service/repository/create_from_snapshot_test.go @@ -55,8 +55,8 @@ func generateTarFile(t *testing.T, path string) ([]byte, []string) { } func createFromSnapshot(t *testing.T, req *gitalypb.CreateRepositoryFromSnapshotRequest) (*gitalypb.CreateRepositoryFromSnapshotResponse, error) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/create_from_url_test.go b/internal/service/repository/create_from_url_test.go index d3f45f672ab599e477b11dc5a3ab903417e5137b..26c84a4c5f6e68969e6539679c276683c65b1678 100644 --- a/internal/service/repository/create_from_url_test.go +++ b/internal/service/repository/create_from_url_test.go @@ -18,8 +18,8 @@ import ( ) func TestSuccessfulCreateRepositoryFromURLRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -86,8 +86,8 @@ func TestCloneRepositoryFromUrlCommand(t *testing.T) { } func TestFailedCreateRepositoryFromURLRequestDueToExistingTarget(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -141,8 +141,8 @@ func TestFailedCreateRepositoryFromURLRequestDueToExistingTarget(t *testing.T) { } func TestPreventingRedirect(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/create_test.go b/internal/service/repository/create_test.go index dcfc09de2e953b52084c1a641c8948f3014518a6..889784043fa811f084b64d2d69317b42b26d0ff5 100644 --- a/internal/service/repository/create_test.go +++ b/internal/service/repository/create_test.go @@ -16,8 +16,8 @@ import ( ) func TestCreateRepositorySuccess(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -48,8 +48,8 @@ func TestCreateRepositorySuccess(t *testing.T) { } func TestCreateRepositoryFailure(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -73,8 +73,8 @@ func TestCreateRepositoryFailure(t *testing.T) { } func TestCreateRepositoryFailureInvalidArgs(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -103,8 +103,8 @@ func TestCreateRepositoryFailureInvalidArgs(t *testing.T) { } func TestCreateRepositoryIdempotent(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/fetch_remote_test.go b/internal/service/repository/fetch_remote_test.go index 5f6cce28de72d7c8dc38af6aec62ee8e3bff46cd..943fc58164b6fe471885c4f41e7fcb998a3ee53d 100644 --- a/internal/service/repository/fetch_remote_test.go +++ b/internal/service/repository/fetch_remote_test.go @@ -44,8 +44,8 @@ func TestFetchRemoteSuccess(t *testing.T) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, _ := newRepositoryClient(t, serverSocketPath) @@ -130,8 +130,8 @@ func getRefnames(t *testing.T, repoPath string) []string { } func TestFetchRemoteOverHTTP(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -186,8 +186,8 @@ func TestFetchRemoteOverHTTP(t *testing.T) { } func TestFetchRemoteOverHTTPWithRedirect(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -217,8 +217,8 @@ func TestFetchRemoteOverHTTPWithRedirect(t *testing.T) { } func TestFetchRemoteOverHTTPError(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/fsck_test.go b/internal/service/repository/fsck_test.go index 84057ec9b2a9a5398baf3370dfa0ac07b63f8518..05727d9eaff7e576ebe1d9e1ecc60e0c59f2f1d5 100644 --- a/internal/service/repository/fsck_test.go +++ b/internal/service/repository/fsck_test.go @@ -16,8 +16,8 @@ func TestFsckSuccess(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -35,8 +35,8 @@ func TestFsckFailureSeverelyBrokenRepo(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -61,8 +61,8 @@ func TestFsckFailureSlightlyBrokenRepo(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/gc_test.go b/internal/service/repository/gc_test.go index 9c50e8e84ac18977ddc1c9e88d7c46c826d8e09f..d8703e3b19b003f424d525ae1f7e626ef0004cc0 100644 --- a/internal/service/repository/gc_test.go +++ b/internal/service/repository/gc_test.go @@ -30,8 +30,8 @@ var ( ) func TestGarbageCollectCommitGraph(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -66,8 +66,8 @@ func TestGarbageCollectCommitGraph(t *testing.T) { } func TestGarbageCollectSuccess(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -137,8 +137,8 @@ func TestGarbageCollectLogStatistics(t *testing.T) { defer cancel() ctx = ctxlogrus.ToContext(ctx, log.WithField("test", "logging")) - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -153,8 +153,8 @@ func TestGarbageCollectLogStatistics(t *testing.T) { } func TestGarbageCollectDeletesRefsLocks(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -201,8 +201,8 @@ func TestGarbageCollectDeletesRefsLocks(t *testing.T) { } func TestGarbageCollectFailure(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -231,8 +231,8 @@ func TestGarbageCollectFailure(t *testing.T) { } func TestCleanupInvalidKeepAroundRefs(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -328,8 +328,8 @@ func createFileWithTimes(path string, mTime time.Time) { } func TestGarbageCollectDeltaIslands(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/info_attributes_test.go b/internal/service/repository/info_attributes_test.go index cc867ed6881d77a409c257102e2c4184f606d750..86096dc033f1c57a281d5ec807abd08fdd7f7b3b 100644 --- a/internal/service/repository/info_attributes_test.go +++ b/internal/service/repository/info_attributes_test.go @@ -14,8 +14,8 @@ import ( ) func TestGetInfoAttributesExisting(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -49,8 +49,8 @@ func TestGetInfoAttributesExisting(t *testing.T) { } func TestGetInfoAttributesNonExisting(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/license_test.go b/internal/service/repository/license_test.go index 93d1c6d6a5d50b369e44c26ee49c04da33958b35..63886c5f6c19ac1f4264d2e65c4c114dba4dfd08 100644 --- a/internal/service/repository/license_test.go +++ b/internal/service/repository/license_test.go @@ -11,8 +11,8 @@ import ( ) func TestSuccessfulFindLicenseRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -32,8 +32,8 @@ func TestSuccessfulFindLicenseRequest(t *testing.T) { } func TestFindLicenseRequestEmptyRepo(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/merge_base_test.go b/internal/service/repository/merge_base_test.go index 765479520c8bbbb879a7a812bb5393c18ca9c834..35b394f8cdaaa0608e7abd7f47a1624508aa1e97 100644 --- a/internal/service/repository/merge_base_test.go +++ b/internal/service/repository/merge_base_test.go @@ -10,8 +10,8 @@ import ( ) func TestSuccessfulFindFindMergeBaseRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -86,8 +86,8 @@ func TestSuccessfulFindFindMergeBaseRequest(t *testing.T) { } func TestFailedFindMergeBaseRequestDueToValidations(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/raw_changes_test.go b/internal/service/repository/raw_changes_test.go index 2aafe4f4d4c24cb18d58c080672b8439412537f4..6b3ad06b9bd9c3e54ce63b9dcb5228a516f16bb3 100644 --- a/internal/service/repository/raw_changes_test.go +++ b/internal/service/repository/raw_changes_test.go @@ -13,8 +13,8 @@ import ( ) func TestGetRawChanges(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -130,8 +130,8 @@ func TestGetRawChangesSpecialCharacters(t *testing.T) { // This test looks for a specific path known to contain special // characters. - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -174,8 +174,8 @@ func collectChanges(t *testing.T, stream gitalypb.RepositoryService_GetRawChange } func TestGetRawChangesFailures(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -236,8 +236,8 @@ func TestGetRawChangesFailures(t *testing.T) { } func TestGetRawChangesManyFiles(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -264,8 +264,8 @@ func TestGetRawChangesManyFiles(t *testing.T) { } func TestGetRawChangesMappingOperations(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -315,8 +315,8 @@ func TestGetRawChangesMappingOperations(t *testing.T) { } func TestGetRawChangesInvalidUTF8Paths(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/rebase_in_progress_test.go b/internal/service/repository/rebase_in_progress_test.go index cddc063d4ee78454fc121d0374a11a1d2f6da33b..9f99b678c0e5e1a8cb7ac6439ac35ed3d9c8572b 100644 --- a/internal/service/repository/rebase_in_progress_test.go +++ b/internal/service/repository/rebase_in_progress_test.go @@ -14,8 +14,8 @@ import ( ) func TestSuccessfulIsRebaseInProgressRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -94,8 +94,8 @@ func TestSuccessfulIsRebaseInProgressRequest(t *testing.T) { } func TestFailedIsRebaseInProgressRequestDueToValidations(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/remove_test.go b/internal/service/repository/remove_test.go index 652bccbe51d2eae94cfe620f812c69c2f2152389..24b31f1cd6c25e5038f0f1ac92d19a60d4ec601c 100644 --- a/internal/service/repository/remove_test.go +++ b/internal/service/repository/remove_test.go @@ -9,8 +9,8 @@ import ( ) func TestRemoveRepository(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -28,8 +28,8 @@ func TestRemoveRepository(t *testing.T) { } func TestRemoveRepositoryDoesNotExist(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/rename_test.go b/internal/service/repository/rename_test.go index 70c8c8ea791b76fc39a00935f50ab3967a10bcd3..4edf51f92b956896cd3050bc26ddc479c7a20ca6 100644 --- a/internal/service/repository/rename_test.go +++ b/internal/service/repository/rename_test.go @@ -12,8 +12,8 @@ import ( ) func TestRenameRepositorySuccess(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -45,8 +45,8 @@ func TestRenameRepositorySuccess(t *testing.T) { } func TestRenameRepositoryDestinationExists(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -72,8 +72,8 @@ func TestRenameRepositoryDestinationExists(t *testing.T) { } func TestRenameRepositoryInvalidRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/repack_test.go b/internal/service/repository/repack_test.go index 166b963a4da3085baaff47020a0823de075c31cf..8865fc88ee90453edb45566efcb4c7682dbff312 100644 --- a/internal/service/repository/repack_test.go +++ b/internal/service/repository/repack_test.go @@ -23,8 +23,8 @@ import ( ) func TestRepackIncrementalSuccess(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -63,8 +63,8 @@ func TestRepackIncrementalCollectLogStatistics(t *testing.T) { defer cancel() ctx = ctxlogrus.ToContext(ctx, log.WithField("test", "logging")) - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -79,8 +79,8 @@ func TestRepackIncrementalCollectLogStatistics(t *testing.T) { } func TestRepackLocal(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -118,8 +118,8 @@ func TestRepackLocal(t *testing.T) { } func TestRepackIncrementalFailure(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -146,8 +146,8 @@ func TestRepackIncrementalFailure(t *testing.T) { } func TestRepackFullSuccess(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -212,8 +212,8 @@ func TestRepackFullCollectLogStatistics(t *testing.T) { defer cancel() ctx = ctxlogrus.ToContext(ctx, log.WithField("test", "logging")) - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -254,8 +254,8 @@ func doBitmapsContainHashCache(t *testing.T, bitmapPaths []string) { } func TestRepackFullFailure(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -282,8 +282,8 @@ func TestRepackFullFailure(t *testing.T) { } func TestRepackFullDeltaIslands(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/replicate_test.go b/internal/service/repository/replicate_test.go index 3e8b5a6f779443de1436e6591e0a2764d182ba3c..2dd9a8fbf57c5467d2e7c593b20c1798cb11013f 100644 --- a/internal/service/repository/replicate_test.go +++ b/internal/service/repository/replicate_test.go @@ -179,8 +179,8 @@ func TestReplicateRepositoryInvalidArguments(t *testing.T) { }, } - server, serverSocketPath := repository.RunRepoServer(t) - defer server.Stop() + serverSocketPath, stop := repository.RunRepoServer(t) + defer stop() client, conn := repository.NewRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/repository_test.go b/internal/service/repository/repository_test.go index ea6e91763dcb2b84c3434f1950b17aad1ff03324..0a50c0883b2ef49d5d44458f3abcb39cc7974036 100644 --- a/internal/service/repository/repository_test.go +++ b/internal/service/repository/repository_test.go @@ -16,8 +16,8 @@ import ( ) func TestRepositoryExists(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t, testhelper.WithStorages([]string{"default", "other", "broken"})) + defer stop() storageOtherDir, err := ioutil.TempDir("", "gitaly-repository-exists-test") require.NoError(t, err, "tempdir") @@ -122,8 +122,8 @@ func TestRepositoryExists(t *testing.T) { } func TestSuccessfulHasLocalBranches(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -177,8 +177,8 @@ func TestSuccessfulHasLocalBranches(t *testing.T) { } func TestFailedHasLocalBranches(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/restore_custom_hooks_test.go b/internal/service/repository/restore_custom_hooks_test.go index ebfc4c1644ffe901d4085ed451d14e16c9c57ed7..f8e6ea8068c738a442399ab41456d58924e79343 100644 --- a/internal/service/repository/restore_custom_hooks_test.go +++ b/internal/service/repository/restore_custom_hooks_test.go @@ -15,8 +15,8 @@ import ( ) func TestSuccessfullRestoreCustomHooksRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -59,8 +59,8 @@ func TestSuccessfullRestoreCustomHooksRequest(t *testing.T) { } func TestFailedRestoreCustomHooksDueToValidations(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -78,8 +78,8 @@ func TestFailedRestoreCustomHooksDueToValidations(t *testing.T) { } func TestFailedRestoreCustomHooksDueToBadTar(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/search_files_test.go b/internal/service/repository/search_files_test.go index 3aa5204d2e8c5dbb9942b4ea94025a8811d40a90..26504fb20399a56d1d52a721503bd80d97b15d5c 100644 --- a/internal/service/repository/search_files_test.go +++ b/internal/service/repository/search_files_test.go @@ -80,8 +80,8 @@ func TestSearchFilesByContentSuccessful(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -152,8 +152,8 @@ func TestSearchFilesByContentLargeFile(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -268,8 +268,8 @@ func TestSearchFilesByNameSuccessful(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -315,14 +315,7 @@ func TestSearchFilesByNameSuccessful(t *testing.T) { } func TestSearchFilesByNameFailure(t *testing.T) { - ctx, cancel := testhelper.Context() - defer cancel() - - server, serverSocketPath := runRepoServer(t) - defer server.Stop() - - client, conn := newRepositoryClient(t, serverSocketPath) - defer conn.Close() + server := NewServer(RubyServer, config.GitalyInternalSocketPath()) testCases := []struct { desc string @@ -354,14 +347,12 @@ func TestSearchFilesByNameFailure(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - stream, err := client.SearchFilesByName(ctx, &gitalypb.SearchFilesByNameRequest{ + err := server.SearchFilesByName(&gitalypb.SearchFilesByNameRequest{ Repository: tc.repo, Query: tc.query, Ref: []byte(tc.ref), - }) - require.NoError(t, err) + }, nil) - _, err = consumeFilenameByName(stream) testhelper.RequireGrpcError(t, err, tc.code) require.Contains(t, err.Error(), tc.msg) }) diff --git a/internal/service/repository/size_test.go b/internal/service/repository/size_test.go index 9643a43c8522c670391bc7313f7d02bfe02b8ba9..c227f82c73907979db87eebd8e5ba5e4c96fde1e 100644 --- a/internal/service/repository/size_test.go +++ b/internal/service/repository/size_test.go @@ -15,8 +15,8 @@ import ( const testRepoMinSizeKB = 10000 func TestSuccessfulRepositorySizeRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -37,8 +37,8 @@ func TestSuccessfulRepositorySizeRequest(t *testing.T) { } func TestFailedRepositorySizeRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -67,8 +67,8 @@ func TestFailedRepositorySizeRequest(t *testing.T) { } func TestSuccessfulGetObjectDirectorySizeRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/snapshot_test.go b/internal/service/repository/snapshot_test.go index 446efcc7e1a6fd5746cd82eb83171c98a6d9e92d..6c0d61d1cc025a47f3bdbdf46efe9ae39f281674 100644 --- a/internal/service/repository/snapshot_test.go +++ b/internal/service/repository/snapshot_test.go @@ -25,8 +25,8 @@ import ( ) func getSnapshot(t *testing.T, req *gitalypb.GetSnapshotRequest) ([]byte, error) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/squash_in_progress_test.go b/internal/service/repository/squash_in_progress_test.go index 9dffb44be34705d1d6d0153df1607eeaf641bffc..dda700870b0a4012de7f396259a78a0fd09d0eb2 100644 --- a/internal/service/repository/squash_in_progress_test.go +++ b/internal/service/repository/squash_in_progress_test.go @@ -11,8 +11,8 @@ import ( ) func TestSuccessfulIsSquashInProgressRequest(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -62,8 +62,8 @@ func TestSuccessfulIsSquashInProgressRequest(t *testing.T) { } func TestFailedIsSquashInProgressRequestDueToValidations(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/service/repository/testhelper_test.go b/internal/service/repository/testhelper_test.go index d377ceac299fb48a726b4cbdab39ee36a917659b..cc05745c283fe9f0dcac8df6002f0322cdcacfe4 100644 --- a/internal/service/repository/testhelper_test.go +++ b/internal/service/repository/testhelper_test.go @@ -3,13 +3,13 @@ package repository import ( "crypto/x509" "log" - "net" "os" "path/filepath" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" gitalyauth "gitlab.com/gitlab-org/gitaly/auth" "gitlab.com/gitlab-org/gitaly/client" dcache "gitlab.com/gitlab-org/gitaly/internal/cache" @@ -17,7 +17,6 @@ import ( mcache "gitlab.com/gitlab-org/gitaly/internal/middleware/cache" "gitlab.com/gitlab-org/gitaly/internal/praefect/protoregistry" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" - "gitlab.com/gitlab-org/gitaly/internal/server/auth" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc" @@ -37,7 +36,7 @@ var ( func newRepositoryClient(t *testing.T, serverSocketPath string) (gitalypb.RepositoryServiceClient, *grpc.ClientConn) { connOpts := []grpc.DialOption{ grpc.WithInsecure(), - grpc.WithPerRPCCredentials(gitalyauth.RPCCredentials(testhelper.RepositoryAuthToken)), + grpc.WithPerRPCCredentials(gitalyauth.RPCCredentials(config.Config.Auth.Token)), } conn, err := grpc.Dial(serverSocketPath, connOpts...) if err != nil { @@ -53,7 +52,7 @@ var RunRepoServer = runRepoServer func newSecureRepoClient(t *testing.T, serverSocketPath string, pool *x509.CertPool) (gitalypb.RepositoryServiceClient, *grpc.ClientConn) { connOpts := []grpc.DialOption{ grpc.WithTransportCredentials(credentials.NewClientTLSFromCert(pool, "")), - grpc.WithPerRPCCredentials(gitalyauth.RPCCredentials(testhelper.RepositoryAuthToken)), + grpc.WithPerRPCCredentials(gitalyauth.RPCCredentials(config.Config.Auth.Token)), } conn, err := client.Dial(serverSocketPath, connOpts) @@ -66,41 +65,33 @@ func newSecureRepoClient(t *testing.T, serverSocketPath string, pool *x509.CertP var NewSecureRepoClient = newSecureRepoClient -func runRepoServer(t *testing.T) (*grpc.Server, string) { +func runRepoServer(t *testing.T, opts ...testhelper.TestServerOpt) (string, func()) { streamInt := []grpc.StreamServerInterceptor{ - auth.StreamServerInterceptor(config.Config.Auth), mcache.StreamInvalidator(dcache.LeaseKeyer{}, protoregistry.GitalyProtoPreregistered), } unaryInt := []grpc.UnaryServerInterceptor{ - auth.UnaryServerInterceptor(config.Config.Auth), mcache.UnaryInvalidator(dcache.LeaseKeyer{}, protoregistry.GitalyProtoPreregistered), } - server := testhelper.NewTestGrpcServer(t, streamInt, unaryInt) - serverSocketPath := testhelper.GetTemporaryGitalySocketFileName() + srv := testhelper.NewServerWithAuth(t, streamInt, unaryInt, config.Config.Auth.Token, opts...) - listener, err := net.Listen("unix", serverSocketPath) - if err != nil { - t.Fatal(err) - } - - gitalypb.RegisterRepositoryServiceServer(server, NewServer(RubyServer, config.GitalyInternalSocketPath())) - reflection.Register(server) + gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), NewServer(RubyServer, config.GitalyInternalSocketPath())) + reflection.Register(srv.GrpcServer()) - go server.Serve(listener) + require.NoError(t, srv.Start()) - return server, "unix://" + serverSocketPath + return "unix://" + srv.Socket(), srv.Stop } func TestRepoNoAuth(t *testing.T) { - srv, path := runRepoServer(t) - defer srv.Stop() + socket, stop := runRepoServer(t) + defer stop() connOpts := []grpc.DialOption{ grpc.WithInsecure(), } - conn, err := grpc.Dial(path, connOpts...) + conn, err := grpc.Dial(socket, connOpts...) if err != nil { t.Fatal(err) } diff --git a/internal/service/repository/write_ref_test.go b/internal/service/repository/write_ref_test.go index 905515a7ecddd548b23f9893fa85f86064e8aea2..9d3890a812bfd5adb2334af09b5dc30e75f08162 100644 --- a/internal/service/repository/write_ref_test.go +++ b/internal/service/repository/write_ref_test.go @@ -12,8 +12,8 @@ import ( ) func TestWriteRefSuccessful(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() @@ -78,8 +78,8 @@ func TestWriteRefSuccessful(t *testing.T) { } func TestWriteRefValidationError(t *testing.T) { - server, serverSocketPath := runRepoServer(t) - defer server.Stop() + serverSocketPath, stop := runRepoServer(t) + defer stop() client, conn := newRepositoryClient(t, serverSocketPath) defer conn.Close() diff --git a/internal/testhelper/testserver.go b/internal/testhelper/testserver.go index 6fa2f865fc808d06944b4fbd2ef15ee7bd45f89f..103ea109ca8ab7b8c0b833c13a822bb9852d8b22 100644 --- a/internal/testhelper/testserver.go +++ b/internal/testhelper/testserver.go @@ -22,11 +22,14 @@ import ( grpc_ctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" + gitalyauth "gitlab.com/gitlab-org/gitaly/auth" "gitlab.com/gitlab-org/gitaly/internal/config" + "gitlab.com/gitlab-org/gitaly/internal/config/auth" "gitlab.com/gitlab-org/gitaly/internal/git/hooks" "gitlab.com/gitlab-org/gitaly/internal/helper/fieldextractors" praefectconfig "gitlab.com/gitlab-org/gitaly/internal/praefect/config" "gitlab.com/gitlab-org/gitaly/internal/praefect/models" + serverauth "gitlab.com/gitlab-org/gitaly/internal/server/auth" "google.golang.org/grpc" "google.golang.org/grpc/health" "google.golang.org/grpc/health/grpc_health_v1" @@ -34,11 +37,59 @@ import ( "gopkg.in/yaml.v2" ) +// PraefectEnabled returns whether or not tests should use a praefect proxy +func PraefectEnabled() bool { + _, ok := os.LookupEnv("GITALY_TEST_PRAEFECT_BIN") + return ok +} + +// TestServerOpt is an option for TestServer +type TestServerOpt func(t *TestServer) + +// WithToken is a TestServerOpt that provides a security token +func WithToken(token string) TestServerOpt { + return func(t *TestServer) { + t.token = token + } +} + +// WithStorages is a TestServerOpt that sets the storages for a TestServer +func WithStorages(storages []string) TestServerOpt { + return func(t *TestServer) { + t.storages = storages + } +} + // NewTestServer instantiates a new TestServer -func NewTestServer(srv *grpc.Server) *TestServer { - return &TestServer{ +func NewTestServer(srv *grpc.Server, opts ...TestServerOpt) *TestServer { + ts := &TestServer{ grpcServer: srv, + storages: []string{"default"}, } + + for _, opt := range opts { + opt(ts) + } + + return ts +} + +// NewServerWithAuth creates a new test server with authentication +func NewServerWithAuth(tb testing.TB, streamInterceptors []grpc.StreamServerInterceptor, unaryInterceptors []grpc.UnaryServerInterceptor, token string, opts ...TestServerOpt) *TestServer { + if token != "" { + if PraefectEnabled() { + opts = append(opts, WithToken(token)) + } + streamInterceptors = append(streamInterceptors, serverauth.StreamServerInterceptor(auth.Config{Token: token})) + unaryInterceptors = append(unaryInterceptors, serverauth.UnaryServerInterceptor(auth.Config{Token: token})) + } + + return NewServer( + tb, + streamInterceptors, + unaryInterceptors, + opts..., + ) } // TestServer wraps a grpc Server and handles automatically putting a praefect in front of a gitaly instance @@ -47,6 +98,8 @@ type TestServer struct { grpcServer *grpc.Server socket string process *os.Process + token string + storages []string } // GrpcServer returns the underlying grpc.Server @@ -101,18 +154,23 @@ func (p *TestServer) Start() error { c := praefectconfig.Config{ SocketPath: praefectServerSocketPath, - VirtualStorages: []*praefectconfig.VirtualStorage{ - { - Name: "default", - Nodes: []*models.Node{ - { - Storage: "default", - Address: "unix:/" + gitalyServerSocketPath, - DefaultPrimary: true, - }, + Auth: auth.Config{ + Token: p.token, + }, + } + + for _, storage := range p.storages { + c.VirtualStorages = append(c.VirtualStorages, &praefectconfig.VirtualStorage{ + Name: storage, + Nodes: []*models.Node{ + { + Storage: storage, + Address: "unix:/" + gitalyServerSocketPath, + DefaultPrimary: true, + Token: p.token, }, }, - }, + }) } if err := toml.NewEncoder(configFile).Encode(&c); err != nil { @@ -134,7 +192,12 @@ func (p *TestServer) Start() error { } go cmd.Wait() - conn, err := grpc.Dial("unix://"+praefectServerSocketPath, grpc.WithInsecure()) + opts := []grpc.DialOption{grpc.WithInsecure()} + if p.token != "" { + opts = append(opts, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentials(p.token))) + } + + conn, err := grpc.Dial("unix://"+praefectServerSocketPath, opts...) if err != nil { return fmt.Errorf("dial: %v", err) @@ -169,7 +232,7 @@ func waitForPraefectStartup(conn *grpc.ClientConn) error { } // NewServer creates a Server for testing purposes -func NewServer(tb testing.TB, streamInterceptors []grpc.StreamServerInterceptor, unaryInterceptors []grpc.UnaryServerInterceptor) *TestServer { +func NewServer(tb testing.TB, streamInterceptors []grpc.StreamServerInterceptor, unaryInterceptors []grpc.UnaryServerInterceptor, opts ...TestServerOpt) *TestServer { logger := NewTestLogger(tb) logrusEntry := log.NewEntry(logger).WithField("test", tb.Name()) @@ -184,7 +247,9 @@ func NewServer(tb testing.TB, streamInterceptors []grpc.StreamServerInterceptor, grpc.NewServer( grpc.StreamInterceptor(grpc_middleware.ChainStreamServer(streamInterceptors...)), grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer(unaryInterceptors...)), - )) + ), + opts..., + ) } var changeLineRegex = regexp.MustCompile("^[a-f0-9]{40} [a-f0-9]{40} refs/[^ ]+$")