diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2020-10-14 11:53:22 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2020-10-14 11:53:22 +0300 |
commit | 9d0ee3ce409b0abd364135d7334fb0873f7bfc8b (patch) | |
tree | 6758a9480e104dc4079886891ff20cac146ae0fa | |
parent | 510c473c9f4dfbd999a9167c9144decb617c923b (diff) |
Revert "Merge branch 'po-remote-repo-resolve-refish' into 'master'"
This reverts commit 510c473c9f4dfbd999a9167c9144decb617c923b, reversing
changes made to c0ba6b3b67598563a14ea7be5f538787354d39be.
-rw-r--r-- | changelogs/unreleased/po-remote-repo-resolve-refish.yml | 5 | ||||
-rw-r--r-- | internal/git/remote.go | 57 | ||||
-rw-r--r-- | internal/git/repository.go | 61 | ||||
-rw-r--r-- | internal/git/repository_test.go | 132 | ||||
-rw-r--r-- | internal/gitaly/service/operations/merge.go | 4 | ||||
-rw-r--r-- | internal/helper/storage.go | 10 | ||||
-rw-r--r-- | internal/helper/storage_test.go | 15 | ||||
-rw-r--r-- | internal/praefect/helper_test.go | 67 | ||||
-rw-r--r-- | internal/testhelper/testserver/gitaly.go | 89 |
9 files changed, 123 insertions, 317 deletions
diff --git a/changelogs/unreleased/po-remote-repo-resolve-refish.yml b/changelogs/unreleased/po-remote-repo-resolve-refish.yml deleted file mode 100644 index 003ec9ebd..000000000 --- a/changelogs/unreleased/po-remote-repo-resolve-refish.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Remote repository abstraction for resolving refish -merge_request: 2629 -author: -type: other diff --git a/internal/git/remote.go b/internal/git/remote.go deleted file mode 100644 index c97760c25..000000000 --- a/internal/git/remote.go +++ /dev/null @@ -1,57 +0,0 @@ -package git - -import ( - "context" - "fmt" - - "gitlab.com/gitlab-org/gitaly/client" - "gitlab.com/gitlab-org/gitaly/internal/helper" - "gitlab.com/gitlab-org/gitaly/internal/storage" - "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" -) - -// remoteRepository represents a Git repository on a different Gitaly storage -type remoteRepository struct { - UnimplementedRepo - repo *gitalypb.Repository - server storage.ServerInfo - pool *client.Pool -} - -// NewRepository creates a new remote Repository from its protobuf representation. -func NewRemoteRepository(ctx context.Context, repo *gitalypb.Repository, pool *client.Pool) (Repository, error) { - server, err := helper.ExtractGitalyServer(ctx, repo.GetStorageName()) - if err != nil { - return nil, fmt.Errorf("remote repository: %w", err) - } - - return remoteRepository{ - repo: repo, - server: server, - pool: pool, - }, nil -} - -// ResolveRefish will dial to the remote repository and attempt to resolve the -// refish string via the gRPC interface -func (rr remoteRepository) ResolveRefish(ctx context.Context, ref string, verify bool) (string, error) { - if verify { - return "", ErrUnimplemented - } - - cc, err := rr.pool.Dial(ctx, rr.server.Address, rr.server.Token) - if err != nil { - return "", err - } - - cli := gitalypb.NewCommitServiceClient(cc) - resp, err := cli.FindCommit(ctx, &gitalypb.FindCommitRequest{ - Repository: rr.repo, - Revision: []byte(ref), - }) - if err != nil { - return "", err - } - - return resp.GetCommit().GetId(), nil -} diff --git a/internal/git/repository.go b/internal/git/repository.go index 51bdf20a7..ab0dce686 100644 --- a/internal/git/repository.go +++ b/internal/git/repository.go @@ -35,9 +35,7 @@ type Repository interface { // gitrevisions(1) for accepted syntax. This will not verify whether the // object ID exists. To do so, you can peel the reference to a given // object type, e.g. by passing `refs/heads/master^{commit}`. - // Verify indicates if you want to return an error when no valid SHA-1 - // can be returned for a given refish. - ResolveRefish(ctx context.Context, ref string, verify bool) (string, error) + ResolveRefish(ctx context.Context, ref string) (string, error) // ContainsRef checks if a ref in the repository exists. This will not // verify whether the target object exists. To do so, you can peel the @@ -76,52 +74,6 @@ type Repository interface { CatFile(ctx context.Context, oid string) ([]byte, error) } -// ErrUnimplemented indicates the repository abstraction does not yet implement -// a specific behavior -var ErrUnimplemented = errors.New("behavior not implemented yet") - -// UnimplementedRepo satisfies the Repository interface to reduce friction in -// writing new Repository implementations -type UnimplementedRepo struct{} - -func (UnimplementedRepo) ResolveRefish(ctx context.Context, ref string, verify bool) (string, error) { - return "", ErrUnimplemented -} - -func (UnimplementedRepo) ContainsRef(ctx context.Context, ref string) (bool, error) { - return false, ErrUnimplemented -} - -func (UnimplementedRepo) GetReference(ctx context.Context, ref string) (Reference, error) { - return Reference{}, ErrUnimplemented -} - -func (UnimplementedRepo) GetReferences(ctx context.Context, pattern string) ([]Reference, error) { - return nil, ErrUnimplemented -} - -func (UnimplementedRepo) GetBranch(ctx context.Context, branch string) (Reference, error) { - return Reference{}, ErrUnimplemented -} - -func (UnimplementedRepo) GetBranches(ctx context.Context) ([]Reference, error) { - return nil, ErrUnimplemented -} - -func (UnimplementedRepo) UpdateRef(ctx context.Context, reference, newrev, oldrev string) error { - return ErrUnimplemented -} - -func (UnimplementedRepo) WriteBlob(context.Context, string, io.Reader) (string, error) { - return "", ErrUnimplemented -} - -func (UnimplementedRepo) CatFile(context.Context, string) ([]byte, error) { - return nil, ErrUnimplemented -} - -var _ Repository = UnimplementedRepo{} // compile time assertion - // localRepository represents a local Git repository. type localRepository struct { repo repository.GitRepo @@ -198,19 +150,14 @@ func (repo *localRepository) CatFile(ctx context.Context, oid string) ([]byte, e return stdout.Bytes(), nil } -func (repo *localRepository) ResolveRefish(ctx context.Context, refish string, verify bool) (string, error) { +func (repo *localRepository) ResolveRefish(ctx context.Context, refish string) (string, error) { if refish == "" { return "", errors.New("repository cannot contain empty reference name") } - var flags []Option - if verify { - flags = append(flags, Flag{Name: "--verify"}) - } - cmd, err := repo.command(ctx, nil, SubCmd{ Name: "rev-parse", - Flags: flags, + Flags: []Option{Flag{Name: "--verify"}}, Args: []string{refish}, }) if err != nil { @@ -236,7 +183,7 @@ func (repo *localRepository) ResolveRefish(ctx context.Context, refish string, v } func (repo *localRepository) ContainsRef(ctx context.Context, ref string) (bool, error) { - if _, err := repo.ResolveRefish(ctx, ref, true); err != nil { + if _, err := repo.ResolveRefish(ctx, ref); err != nil { if errors.Is(err, ErrReferenceNotFound) { return false, nil } diff --git a/internal/git/repository_test.go b/internal/git/repository_test.go index b30cc7f08..3b6a2e3e0 100644 --- a/internal/git/repository_test.go +++ b/internal/git/repository_test.go @@ -1,8 +1,7 @@ -package git_test +package git import ( "errors" - "fmt" "io" "io/ioutil" "os" @@ -12,12 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/client" - "gitlab.com/gitlab-org/gitaly/internal/git" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/testhelper" - "gitlab.com/gitlab-org/gitaly/internal/testhelper/testserver" ) const ( @@ -25,10 +19,12 @@ const ( NonexistentID = "ba4f184e126b751d1bffad5897f263108befc780" ) -func TestRepository_ResolveRefish(t *testing.T) { +func TestLocalRepository_ResolveRefish(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() + repo := NewRepository(testhelper.TestRepository()) + testcases := []struct { desc string refish string @@ -64,42 +60,18 @@ func TestRepository_ResolveRefish(t *testing.T) { }, } - _, serverSocketPath, cleanup := testserver.RunInternalGitalyServer(t, config.Config.Storages, config.Config.Auth.Token) - defer cleanup() + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + oid, err := repo.ResolveRefish(ctx, tc.refish) - for _, repo := range []git.Repository{ - git.NewRepository(testhelper.TestRepository()), - func() git.Repository { - ctx, err := helper.InjectGitalyServers(ctx, "default", serverSocketPath, config.Config.Auth.Token) - require.NoError(t, err) + if tc.expected == "" { + require.Error(t, err) + require.Equal(t, err, ErrReferenceNotFound) + return + } - r, err := git.NewRemoteRepository( - helper.OutgoingToIncoming(ctx), - testhelper.TestRepository(), - client.NewPool(), - ) require.NoError(t, err) - return r - }(), - } { - t.Run(fmt.Sprintf("%T", repo), func(t *testing.T) { - for _, tc := range testcases { - t.Run(tc.desc, func(t *testing.T) { - oid, err := repo.ResolveRefish(ctx, tc.refish, true) - if errors.Is(err, git.ErrUnimplemented) { - t.Skip() - } - - if tc.expected == "" { - require.Error(t, err) - require.Equal(t, err, git.ErrReferenceNotFound) - return - } - - require.NoError(t, err) - require.Equal(t, tc.expected, oid) - }) - } + require.Equal(t, tc.expected, oid) }) } } @@ -108,7 +80,7 @@ func TestLocalRepository_ContainsRef(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - repo := git.NewRepository(testhelper.TestRepository()) + repo := NewRepository(testhelper.TestRepository()) testcases := []struct { desc string @@ -145,32 +117,32 @@ func TestLocalRepository_GetReference(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - repo := git.NewRepository(testhelper.TestRepository()) + repo := NewRepository(testhelper.TestRepository()) testcases := []struct { desc string ref string - expected git.Reference + expected Reference }{ { desc: "fully qualified master branch", ref: "refs/heads/master", - expected: git.NewReference("refs/heads/master", MasterID), + expected: NewReference("refs/heads/master", MasterID), }, { desc: "unqualified master branch fails", ref: "master", - expected: git.Reference{}, + expected: Reference{}, }, { desc: "nonexistent branch", ref: "refs/heads/nonexistent", - expected: git.Reference{}, + expected: Reference{}, }, { desc: "nonexistent branch", ref: "nonexistent", - expected: git.Reference{}, + expected: Reference{}, }, } @@ -178,7 +150,7 @@ func TestLocalRepository_GetReference(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { ref, err := repo.GetReference(ctx, tc.ref) if tc.expected.Name == "" { - require.True(t, errors.Is(err, git.ErrReferenceNotFound)) + require.True(t, errors.Is(err, ErrReferenceNotFound)) } else { require.NoError(t, err) require.Equal(t, tc.expected, ref) @@ -191,32 +163,32 @@ func TestLocalRepository_GetBranch(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - repo := git.NewRepository(testhelper.TestRepository()) + repo := NewRepository(testhelper.TestRepository()) testcases := []struct { desc string ref string - expected git.Reference + expected Reference }{ { desc: "fully qualified master branch", ref: "refs/heads/master", - expected: git.NewReference("refs/heads/master", MasterID), + expected: NewReference("refs/heads/master", MasterID), }, { desc: "half-qualified master branch", ref: "heads/master", - expected: git.NewReference("refs/heads/master", MasterID), + expected: NewReference("refs/heads/master", MasterID), }, { desc: "fully qualified master branch", ref: "master", - expected: git.NewReference("refs/heads/master", MasterID), + expected: NewReference("refs/heads/master", MasterID), }, { desc: "nonexistent branch", ref: "nonexistent", - expected: git.Reference{}, + expected: Reference{}, }, } @@ -224,7 +196,7 @@ func TestLocalRepository_GetBranch(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { ref, err := repo.GetBranch(ctx, tc.ref) if tc.expected.Name == "" { - require.True(t, errors.Is(err, git.ErrReferenceNotFound)) + require.True(t, errors.Is(err, ErrReferenceNotFound)) } else { require.NoError(t, err) require.Equal(t, tc.expected, ref) @@ -237,40 +209,40 @@ func TestLocalRepository_GetReferences(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - repo := git.NewRepository(testhelper.TestRepository()) + repo := NewRepository(testhelper.TestRepository()) testcases := []struct { desc string pattern string - match func(t *testing.T, refs []git.Reference) + match func(t *testing.T, refs []Reference) }{ { desc: "master branch", pattern: "refs/heads/master", - match: func(t *testing.T, refs []git.Reference) { - require.Equal(t, []git.Reference{ - git.NewReference("refs/heads/master", MasterID), + match: func(t *testing.T, refs []Reference) { + require.Equal(t, []Reference{ + NewReference("refs/heads/master", MasterID), }, refs) }, }, { desc: "all references", pattern: "", - match: func(t *testing.T, refs []git.Reference) { + match: func(t *testing.T, refs []Reference) { require.Len(t, refs, 94) }, }, { desc: "branches", pattern: "refs/heads/", - match: func(t *testing.T, refs []git.Reference) { + match: func(t *testing.T, refs []Reference) { require.Len(t, refs, 91) }, }, { desc: "branches", pattern: "refs/heads/nonexistent", - match: func(t *testing.T, refs []git.Reference) { + match: func(t *testing.T, refs []Reference) { require.Empty(t, refs) }, }, @@ -303,7 +275,7 @@ crlf binary lf text `), os.ModePerm)) - repo := git.NewRepository(pbRepo) + repo := NewRepository(pbRepo) for _, tc := range []struct { desc string @@ -316,7 +288,7 @@ lf text { desc: "error reading", input: ReaderFunc(func([]byte) (int, error) { return 0, assert.AnError }), - error: fmt.Errorf("%w, stderr: %q", assert.AnError, []byte{}), + error: errorWithStderr(assert.AnError, nil), }, { desc: "successful empty blob", @@ -364,7 +336,7 @@ func TestLocalRepository_CatFile(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - repo := git.NewRepository(testhelper.TestRepository()) + repo := NewRepository(testhelper.TestRepository()) for _, tc := range []struct { desc string @@ -374,8 +346,8 @@ func TestLocalRepository_CatFile(t *testing.T) { }{ { desc: "invalid object", - oid: git.NullSHA, - error: git.InvalidObjectError(git.NullSHA), + oid: NullSHA, + error: InvalidObjectError(NullSHA), }, { desc: "valid object", @@ -396,7 +368,7 @@ func TestLocalRepository_GetBranches(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - repo := git.NewRepository(testhelper.TestRepository()) + repo := NewRepository(testhelper.TestRepository()) refs, err := repo.GetBranches(ctx) require.NoError(t, err) @@ -410,7 +382,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - otherRef, err := git.NewRepository(testRepo).GetReference(ctx, "refs/heads/gitaly-test-ref") + otherRef, err := NewRepository(testRepo).GetReference(ctx, "refs/heads/gitaly-test-ref") require.NoError(t, err) testcases := []struct { @@ -418,14 +390,14 @@ func TestLocalRepository_UpdateRef(t *testing.T) { ref string newrev string oldrev string - verify func(t *testing.T, repo git.Repository, err error) + verify func(t *testing.T, repo Repository, err error) }{ { desc: "successfully update master", ref: "refs/heads/master", newrev: otherRef.Target, oldrev: MasterID, - verify: func(t *testing.T, repo git.Repository, err error) { + verify: func(t *testing.T, repo Repository, err error) { require.NoError(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -437,7 +409,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { ref: "refs/heads/master", newrev: otherRef.Target, oldrev: NonexistentID, - verify: func(t *testing.T, repo git.Repository, err error) { + verify: func(t *testing.T, repo Repository, err error) { require.Error(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -449,7 +421,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { ref: "refs/heads/master", newrev: NonexistentID, oldrev: MasterID, - verify: func(t *testing.T, repo git.Repository, err error) { + verify: func(t *testing.T, repo Repository, err error) { require.Error(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -461,7 +433,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { ref: "refs/heads/master", newrev: otherRef.Target, oldrev: "", - verify: func(t *testing.T, repo git.Repository, err error) { + verify: func(t *testing.T, repo Repository, err error) { require.NoError(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -473,7 +445,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { ref: "master", newrev: otherRef.Target, oldrev: MasterID, - verify: func(t *testing.T, repo git.Repository, err error) { + verify: func(t *testing.T, repo Repository, err error) { require.Error(t, err) ref, err := repo.GetReference(ctx, "refs/heads/master") require.NoError(t, err) @@ -485,7 +457,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { ref: "refs/heads/master", newrev: strings.Repeat("0", 40), oldrev: MasterID, - verify: func(t *testing.T, repo git.Repository, err error) { + verify: func(t *testing.T, repo Repository, err error) { require.NoError(t, err) _, err = repo.GetReference(ctx, "refs/heads/master") require.Error(t, err) @@ -496,7 +468,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { ref: "refs/heads/new", newrev: MasterID, oldrev: strings.Repeat("0", 40), - verify: func(t *testing.T, repo git.Repository, err error) { + verify: func(t *testing.T, repo Repository, err error) { require.NoError(t, err) ref, err := repo.GetReference(ctx, "refs/heads/new") require.NoError(t, err) @@ -511,7 +483,7 @@ func TestLocalRepository_UpdateRef(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - repo := git.NewRepository(testRepo) + repo := NewRepository(testRepo) err := repo.UpdateRef(ctx, tc.ref, tc.newrev, tc.oldrev) tc.verify(t, repo, err) diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index e666f94bb..466cbf5e0 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -393,13 +393,13 @@ func (s *server) userMergeToRef(ctx context.Context, request *gitalypb.UserMerge refName = string(request.FirstParentRef) } - ref, err := repo.ResolveRefish(ctx, refName, true) + ref, err := repo.ResolveRefish(ctx, refName) if err != nil { //nolint:stylecheck return nil, helper.ErrInvalidArgument(errors.New("Invalid merge source")) } - sourceRef, err := repo.ResolveRefish(ctx, request.SourceSha, true) + sourceRef, err := repo.ResolveRefish(ctx, request.SourceSha) if err != nil { //nolint:stylecheck return nil, helper.ErrInvalidArgument(errors.New("Invalid merge source")) diff --git a/internal/helper/storage.go b/internal/helper/storage.go index 28d4d19c7..254e51fa6 100644 --- a/internal/helper/storage.go +++ b/internal/helper/storage.go @@ -60,16 +60,6 @@ func IncomingToOutgoing(ctx context.Context) context.Context { return metadata.NewOutgoingContext(ctx, md) } -// OutgoingToIncoming creates an incoming context out of an outgoing context with the same storage metadata -func OutgoingToIncoming(ctx context.Context) context.Context { - md, ok := metadata.FromOutgoingContext(ctx) - if !ok { - return ctx - } - - return metadata.NewIncomingContext(ctx, md) -} - // InjectGitalyServers injects gitaly-servers metadata into an outgoing context func InjectGitalyServers(ctx context.Context, name, address, token string) (context.Context, error) { gitalyServers := storage.GitalyServers{ diff --git a/internal/helper/storage_test.go b/internal/helper/storage_test.go index 6c4a70ba2..44e49b218 100644 --- a/internal/helper/storage_test.go +++ b/internal/helper/storage_test.go @@ -97,18 +97,3 @@ func TestInjectGitalyServers(t *testing.T) { require.Equal(t, []string{"bar"}, md["foo"]) }) } - -func TestOutgoingToIncoming(t *testing.T) { - ctx := context.Background() - ctx, err := InjectGitalyServers(ctx, "a", "b", "c") - require.NoError(t, err) - - _, err = ExtractGitalyServer(ctx, "a") - require.Error(t, err, "server should not be found in the incoming context") - - ctx = OutgoingToIncoming(ctx) - - info, err := ExtractGitalyServer(ctx, "a") - require.NoError(t, err) - require.Equal(t, storage.ServerInfo{Address: "b", Token: "c"}, info) -} diff --git a/internal/praefect/helper_test.go b/internal/praefect/helper_test.go index 960185cf4..499df18d8 100644 --- a/internal/praefect/helper_test.go +++ b/internal/praefect/helper_test.go @@ -14,6 +14,9 @@ import ( gconfig "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" internalauth "gitlab.com/gitlab-org/gitaly/internal/gitaly/config/auth" "gitlab.com/gitlab-org/gitaly/internal/gitaly/server/auth" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/internalgitaly" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/repository" + gitalyserver "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/server" "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/gitaly/internal/praefect/config" "gitlab.com/gitlab-org/gitaly/internal/praefect/datastore" @@ -24,7 +27,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/praefect/transactions" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/promtest" - "gitlab.com/gitlab-org/gitaly/internal/testhelper/testserver" + "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" correlation "gitlab.com/gitlab-org/labkit/correlation/grpc" "google.golang.org/grpc" "google.golang.org/grpc/health" @@ -141,7 +144,7 @@ func flattenVirtualStoragesToStoragePath(virtualStorages []*config.VirtualStorag func withRealGitalyShared(t testing.TB) func([]*config.VirtualStorage) []testhelper.Cleanup { return func(virtualStorages []*config.VirtualStorage) []testhelper.Cleanup { gStorages := flattenVirtualStoragesToStoragePath(virtualStorages, testhelper.GitlabTestStoragePath()) - _, backendAddr, cleanupGitaly := testserver.RunInternalGitalyServer(t, gStorages, virtualStorages[0].Nodes[0].Token) + _, backendAddr, cleanupGitaly := runInternalGitalyServer(t, gStorages, virtualStorages[0].Nodes[0].Token) for _, vs := range virtualStorages { for i, node := range vs.Nodes { @@ -260,6 +263,66 @@ func runPraefectServer(t testing.TB, conf config.Config, opt buildOptions) (*grp return cc, prf, cleanup } +// partialGitaly is a subset of Gitaly's behavior needed to properly test +// Praefect +type partialGitaly interface { + gitalypb.ServerServiceServer + gitalypb.RepositoryServiceServer + gitalypb.InternalGitalyServer + healthpb.HealthServer +} + +func registerGitalyServices(server *grpc.Server, pg partialGitaly) { + gitalypb.RegisterServerServiceServer(server, pg) + gitalypb.RegisterRepositoryServiceServer(server, pg) + gitalypb.RegisterInternalGitalyServer(server, pg) + healthpb.RegisterHealthServer(server, pg) +} + +func realGitaly(storages []gconfig.Storage, authToken, internalSocketPath string) partialGitaly { + return struct { + gitalypb.ServerServiceServer + gitalypb.RepositoryServiceServer + gitalypb.InternalGitalyServer + healthpb.HealthServer + }{ + gitalyserver.NewServer(storages), + repository.NewServer(RubyServer, gconfig.NewLocator(gconfig.Config), internalSocketPath), + internalgitaly.NewServer(gconfig.Config.Storages), + health.NewServer(), + } +} + +func runInternalGitalyServer(t testing.TB, storages []gconfig.Storage, token string) (*grpc.Server, string, func()) { + streamInt := []grpc.StreamServerInterceptor{auth.StreamServerInterceptor(internalauth.Config{Token: token})} + unaryInt := []grpc.UnaryServerInterceptor{auth.UnaryServerInterceptor(internalauth.Config{Token: token})} + + server := testhelper.NewTestGrpcServer(t, streamInt, unaryInt) + serverSocketPath := testhelper.GetTemporaryGitalySocketFileName() + + listener, err := net.Listen("unix", serverSocketPath) + require.NoError(t, err) + + internalSocketPath := gconfig.GitalyInternalSocketPath() + internalListener, err := net.Listen("unix", internalSocketPath) + require.NoError(t, err) + + registerGitalyServices(server, realGitaly(storages, token, internalSocketPath)) + + errQ := make(chan error) + + go func() { errQ <- server.Serve(listener) }() + go func() { errQ <- server.Serve(internalListener) }() + + cleanup := func() { + server.Stop() + require.NoError(t, <-errQ) + require.NoError(t, <-errQ) + } + + return server, "unix://" + serverSocketPath, cleanup +} + func mustLoadProtoReg(t testing.TB) *descriptor.FileDescriptorProto { gz, _ := (*mock.SimpleRequest)(nil).Descriptor() fd, err := protoregistry.ExtractFileDescriptor(gz) diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go deleted file mode 100644 index be38eba67..000000000 --- a/internal/testhelper/testserver/gitaly.go +++ /dev/null @@ -1,89 +0,0 @@ -package testserver - -import ( - "net" - "testing" - - "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - internalauth "gitlab.com/gitlab-org/gitaly/internal/gitaly/config/auth" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/server/auth" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/commit" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/internalgitaly" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/repository" - gitalyserver "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/server" - "gitlab.com/gitlab-org/gitaly/internal/testhelper" - "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" - "google.golang.org/grpc" - "google.golang.org/grpc/health" - healthpb "google.golang.org/grpc/health/grpc_health_v1" -) - -var RubyServer = &rubyserver.Server{} - -// PartialGitaly is a subset of Gitaly's behavior -type PartialGitaly interface { - gitalypb.ServerServiceServer - gitalypb.RepositoryServiceServer - gitalypb.InternalGitalyServer - gitalypb.CommitServiceServer - healthpb.HealthServer -} - -func registerGitalyServices(server *grpc.Server, pg PartialGitaly) { - gitalypb.RegisterServerServiceServer(server, pg) - gitalypb.RegisterRepositoryServiceServer(server, pg) - gitalypb.RegisterInternalGitalyServer(server, pg) - gitalypb.RegisterCommitServiceServer(server, pg) - healthpb.RegisterHealthServer(server, pg) -} - -// RealGitaly instantiates an instance of PartialGitaly that uses the real world -// services. This is intended to be used in integration tests to validate -// Gitaly services. -func RealGitaly(storages []config.Storage, authToken, internalSocketPath string) PartialGitaly { - return struct { - gitalypb.ServerServiceServer - gitalypb.RepositoryServiceServer - gitalypb.InternalGitalyServer - gitalypb.CommitServiceServer - healthpb.HealthServer - }{ - gitalyserver.NewServer(storages), - repository.NewServer(RubyServer, config.NewLocator(config.Config), internalSocketPath), - internalgitaly.NewServer(config.Config.Storages), - commit.NewServer(config.NewLocator(config.Config)), - health.NewServer(), - } -} - -func RunInternalGitalyServer(t testing.TB, storages []config.Storage, token string) (*grpc.Server, string, func()) { - streamInt := []grpc.StreamServerInterceptor{auth.StreamServerInterceptor(internalauth.Config{Token: token})} - unaryInt := []grpc.UnaryServerInterceptor{auth.UnaryServerInterceptor(internalauth.Config{Token: token})} - - server := testhelper.NewTestGrpcServer(t, streamInt, unaryInt) - serverSocketPath := testhelper.GetTemporaryGitalySocketFileName() - - listener, err := net.Listen("unix", serverSocketPath) - require.NoError(t, err) - - internalSocketPath := config.GitalyInternalSocketPath() - internalListener, err := net.Listen("unix", internalSocketPath) - require.NoError(t, err) - - registerGitalyServices(server, RealGitaly(storages, token, internalSocketPath)) - - errQ := make(chan error) - - go func() { errQ <- server.Serve(listener) }() - go func() { errQ <- server.Serve(internalListener) }() - - cleanup := func() { - server.Stop() - require.NoError(t, <-errQ) - require.NoError(t, <-errQ) - } - - return server, "unix://" + serverSocketPath, cleanup -} |