diff options
46 files changed, 455 insertions, 247 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 5197b2564..7af90e647 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Gitaly changelog +## 12.8.7 + +- No changes. + ## 12.8.6 - No changes. diff --git a/_support/Makefile.template b/_support/Makefile.template index 4135e5bb2..60f334e0e 100644 --- a/_support/Makefile.template +++ b/_support/Makefile.template @@ -8,7 +8,7 @@ # These variables may be overridden at runtime by top-level make PREFIX ?= /usr/local INSTALL_DEST_DIR := $(DESTDIR)$(PREFIX)/bin/ -BUNDLE_FLAGS ?= --deployment +BUNDLE_FLAGS ?= {{ .BundleFlags }} ASSEMBLY_ROOT ?= {{ .BuildDir }}/assembly BUILD_TAGS := tracer_static tracer_static_jaeger diff --git a/_support/makegen.go b/_support/makegen.go index a8e9351ac..81a5ae611 100644 --- a/_support/makegen.go +++ b/_support/makegen.go @@ -334,6 +334,19 @@ func (gm *gitalyMake) GolangCILint() string { return fmt.Sprintf("golangci-lint-%s-%s-%s", gm.GolangCILintVersion(), runtime.GOOS, runtime.GOARCH) } +func (gm *gitalyMake) BundleFlags() string { + if gm.IsGDK() { + return "--no-deployment" + } + + return "--deployment" +} + +func (gm *gitalyMake) IsGDK() bool { + _, err := os.Stat(filepath.Join(gm.SourceDir(), "../.gdk-install-root")) + return err == nil +} + var templateText = func() string { contents, err := ioutil.ReadFile("../_support/Makefile.template") if err != nil { diff --git a/changelogs/unreleased/jc-add-storage-to-healthcheck-error.yml b/changelogs/unreleased/jc-add-storage-to-healthcheck-error.yml new file mode 100644 index 000000000..dca916054 --- /dev/null +++ b/changelogs/unreleased/jc-add-storage-to-healthcheck-error.yml @@ -0,0 +1,5 @@ +--- +title: Add storage name to healthcheck error log +merge_request: 1934 +author: +type: added diff --git a/changelogs/unreleased/ps-verify-storage-locations.yml b/changelogs/unreleased/ps-verify-storage-locations.yml new file mode 100644 index 000000000..3d3abec55 --- /dev/null +++ b/changelogs/unreleased/ps-verify-storage-locations.yml @@ -0,0 +1,5 @@ +--- +title: Praefect ping-node must verify storage locations are served +merge_request: 1881 +author: +type: added diff --git a/cmd/gitaly-ssh/README.md b/cmd/gitaly-ssh/README.md index 8bfe7ab0f..d660ab2eb 100644 --- a/cmd/gitaly-ssh/README.md +++ b/cmd/gitaly-ssh/README.md @@ -5,6 +5,15 @@ Gitaly-ssh is a helper executable that enables Git data traffic installation. It acts as a plugin to `git fetch` using the `GIT_SSH_COMMAND` environment variable. +We created gitaly-ssh because we needed a way to pull Git data from one +Gitaly server to another, without going through one of the "front +doors" of GitLab: gitlab-shell (Git SSH) or gitlab-workhorse (Git +HTTP). To avoid building a special RPC for this, we re-used the +SSHUploadPack RPC that Gitaly already had. By connecting directly to +the Gitaly server we avoided the need to create some kind of service +account in GitLab itself: to go through the front door we would need a +service account. + The implementation shares code with how gitlab-shell handles Git SSH traffic from real users, but it cuts out SSH itself. diff --git a/cmd/praefect/subcmd_pingnodes.go b/cmd/praefect/subcmd_pingnodes.go index 0423c88b0..8c0b7a7ab 100644 --- a/cmd/praefect/subcmd_pingnodes.go +++ b/cmd/praefect/subcmd_pingnodes.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "fmt" "log" "sync" @@ -10,6 +11,7 @@ import ( gitalyauth "gitlab.com/gitlab-org/gitaly/auth" "gitlab.com/gitlab-org/gitaly/client" "gitlab.com/gitlab-org/gitaly/internal/praefect/config" + "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc" "google.golang.org/grpc/health/grpc_health_v1" ) @@ -99,6 +101,61 @@ func (npr *nodePing) healthCheck(cc *grpc.ClientConn) (grpc_health_v1.HealthChec return resp.GetStatus(), nil } +func (npr *nodePing) isConsistent(cc *grpc.ClientConn) bool { + praefect := gitalypb.NewServerServiceClient(cc) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + if len(npr.storages) == 0 { + npr.log("ERROR: current configuration has no storages") + return false + } + + resp, err := praefect.ServerInfo(ctx, &gitalypb.ServerInfoRequest{}) + if err != nil { + npr.log("ERROR: failed to receive state from the remote: %v", err) + return false + } + + if len(resp.StorageStatuses) == 0 { + npr.log("ERROR: remote has no configured storages") + return false + } + + storagesSet := make(map[string]bool, len(resp.StorageStatuses)) + + knownStoragesSet := make(map[string]bool, len(npr.storages)) + for k := range npr.storages { + knownStoragesSet[k] = true + } + + consistent := true + for _, status := range resp.StorageStatuses { + if storagesSet[status.StorageName] { + npr.log("ERROR: remote has duplicated storage: %q", status.StorageName) + consistent = false + continue + } + storagesSet[status.StorageName] = true + + if status.Readable && status.Writeable { + npr.log("SUCCESS: %q is served by Gitaly", status.StorageName) + delete(knownStoragesSet, status.StorageName) // storage found + } else { + npr.log("ERROR: storage %q is not readable or writable", status.StorageName) + consistent = false + } + } + + for storage := range knownStoragesSet { + npr.log("ERROR: configured storage was not reported by remote: %q", storage) + consistent = false + } + + return consistent +} + func (npr *nodePing) log(msg string, args ...interface{}) { log.Printf("[%s]: %s", npr.address, fmt.Sprintf(msg, args...)) } @@ -129,5 +186,14 @@ func (npr *nodePing) checkNode() { npr.log("ERROR: %v", npr.err) return } + npr.log("SUCCESS: node is healthy!") + + npr.log("checking consistency...") + if !npr.isConsistent(cc) { + npr.err = errors.New("consistency check failed") + npr.log("ERROR: %v", npr.err) + return + } + npr.log("SUCCESS: node is consistent!") } diff --git a/doc/README.md b/doc/README.md index 66b327c09..4cc571dca 100644 --- a/doc/README.md +++ b/doc/README.md @@ -32,7 +32,9 @@ For configuration please read [praefects configuration documentation](doc/config - [Delta Islands](delta_islands.md) - [Disk-based Cache](design_diskcache.md) - [Tips for reading Git source code](reading_git_source.md) +- [gitaly-ssh](../cmd/gitaly-ssh/README.md) #### Proposals - [Snapshot storage](proposals/snapshot-storage.md) +- [Praefect Queue storage](proposals/praefect-queue-storage.md) diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go index ae4ebcb61..bca5bfdc9 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 c04767409..9a677327e 100644 --- a/internal/praefect/nodes/manager.go +++ b/internal/praefect/nodes/manager.go @@ -164,11 +164,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 { @@ -280,7 +283,7 @@ func (n *nodeStatus) check() { resp, err := client.Check(ctx, &healthpb.HealthCheckRequest{Service: ""}) if err != nil { - n.log.WithError(err).WithField("address", n.Address).Warn("error when pinging healthcheck") + n.log.WithError(err).WithField("storage", n.Storage).WithField("address", n.Address).Warn("error when pinging healthcheck") resp = &healthpb.HealthCheckResponse{ Status: healthpb.HealthCheckResponse_UNKNOWN, } diff --git a/internal/praefect/protoregistry/find_oid.go b/internal/praefect/protoregistry/find_oid.go index 8b73d4943..7d8691fe3 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 0dd242b92..b07fa4e4c 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 5e129b8f9..93102a87f 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 abd3df248..6494f7d81 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 b29d245dc..450127165 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 bb0f2e4e1..6b36eae92 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 de7f099e9..9cc0fcea1 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 c70373121..966eead17 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 161fc1c9b..e6145757f 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 2abc1aad5..912432759 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 d3f45f672..26c84a4c5 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 dcfc09de2..889784043 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 5f6cce28d..943fc5816 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 84057ec9b..05727d9ea 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 9c50e8e84..d8703e3b1 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 cc867ed68..86096dc03 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 93d1c6d6a..63886c5f6 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 765479520..35b394f8c 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 2aafe4f4d..6b3ad06b9 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 cddc063d4..9f99b678c 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 652bccbe5..24b31f1cd 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 70c8c8ea7..4edf51f92 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 166b963a4..8865fc88e 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 3e8b5a6f7..2dd9a8fbf 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 ea6e91763..0a50c0883 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 ebfc4c164..f8e6ea806 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 3aa5204d2..26504fb20 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 9643a43c8..c227f82c7 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 446efcc7e..6c0d61d1c 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 9dffb44be..dda700870 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 d377ceac2..cc05745c2 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 905515a7e..9d3890a81 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 9d14e2342..38227789f 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/[^ ]+$") diff --git a/ruby/gitlab-shell/lib/object_dirs_helper.rb b/ruby/gitlab-shell/lib/object_dirs_helper.rb index e175a0392..2c1642ba9 100644 --- a/ruby/gitlab-shell/lib/object_dirs_helper.rb +++ b/ruby/gitlab-shell/lib/object_dirs_helper.rb @@ -4,9 +4,7 @@ class ObjectDirsHelper class << self def all_attributes { - "GIT_ALTERNATE_OBJECT_DIRECTORIES" => absolute_alt_object_dirs, "GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE" => relative_alt_object_dirs, - "GIT_OBJECT_DIRECTORY" => absolute_object_dir, "GIT_OBJECT_DIRECTORY_RELATIVE" => relative_object_dir } end diff --git a/ruby/gitlab-shell/spec/object_dirs_helper_spec.rb b/ruby/gitlab-shell/spec/object_dirs_helper_spec.rb index c2d0db7f7..d642608ce 100644 --- a/ruby/gitlab-shell/spec/object_dirs_helper_spec.rb +++ b/ruby/gitlab-shell/spec/object_dirs_helper_spec.rb @@ -9,9 +9,7 @@ describe ObjectDirsHelper do describe '.all_attributes' do it do expect(described_class.all_attributes.keys).to include(*%w[ - GIT_OBJECT_DIRECTORY GIT_OBJECT_DIRECTORY_RELATIVE - GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE ]) end diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb index af9feda30..0ca8daca2 100644 --- a/ruby/lib/gitlab/git/repository.rb +++ b/ruby/lib/gitlab/git/repository.rb @@ -9,14 +9,6 @@ module Gitlab include Gitlab::EncodingHelper include Gitlab::Utils::StrongMemoize - ALLOWED_OBJECT_DIRECTORIES_VARIABLES = %w[ - GIT_OBJECT_DIRECTORY - GIT_ALTERNATE_OBJECT_DIRECTORIES - ].freeze - ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES = %w[ - GIT_OBJECT_DIRECTORY_RELATIVE - GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE - ].freeze # In https://gitlab.com/gitlab-org/gitaly/merge_requests/698 # We copied these two prefixes into gitaly-go, so don't change these # or things will break! (REBASE_WORKTREE_PREFIX and SQUASH_WORKTREE_PREFIX) |