From ef065f1e55b31be704077e519999eb7e01a27832 Mon Sep 17 00:00:00 2001 From: Pavlo Strokov Date: Tue, 15 Dec 2020 18:49:27 +0200 Subject: [PATCH 1/3] Break dependency of the catfile on global config.Config The catfile package directly or indirectly uses config.Config value. And as we are about to remove config.Config from the code we need to break this dependency and substitute with other abstractions. For now those are storage.Locator and alternates.Env instead of the old alternates.PathAndEnv that depends on the global var. It was decided to extend constructor function of the catfile to accept storage.Locator as a dependency as it is already injected in most of the "server"s which handle requests. The other option could be creation of the "factory", but it will require a new dependency to be injected into all existing services which is a lot more changes and adds currently unnecessary abstraction that needs to be managed. Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699 --- cmd/gitaly-git2go/submodule_test.go | 2 +- internal/git/catfile/batch.go | 7 +++- internal/git/catfile/batchcheck.go | 7 +++- internal/git/catfile/catfile.go | 13 ++++--- internal/git/catfile/catfile_test.go | 15 ++++---- internal/git/log/commit.go | 5 ++- internal/git/log/commit_test.go | 3 +- internal/git/log/log.go | 5 ++- internal/git/log/tag_test.go | 3 +- internal/git/objectpool/link.go | 22 ++++++----- internal/git/updateref/updateref_test.go | 31 ++++++++++------ internal/gitaly/service/blob/get_blob.go | 2 +- internal/gitaly/service/blob/get_blobs.go | 4 +- .../cleanup/apply_bfg_object_map_stream.go | 2 +- .../apply_bfg_object_map_stream_test.go | 4 +- .../service/cleanup/notifier/notifier.go | 5 ++- internal/gitaly/service/cleanup/server.go | 6 ++- .../gitaly/service/cleanup/testhelper_test.go | 5 ++- internal/gitaly/service/commit/between.go | 2 +- .../gitaly/service/commit/commit_messages.go | 6 +-- .../service/commit/commit_signatures.go | 6 +-- .../service/commit/commits_by_message.go | 7 ++-- .../gitaly/service/commit/commits_helper.go | 5 ++- .../commit/filter_shas_with_signatures.go | 2 +- .../gitaly/service/commit/find_all_commits.go | 7 ++-- internal/gitaly/service/commit/find_commit.go | 2 +- .../gitaly/service/commit/find_commit_test.go | 5 ++- .../gitaly/service/commit/find_commits.go | 6 +-- .../service/commit/last_commit_for_path.go | 6 +-- .../service/commit/list_commits_by_oid.go | 2 +- .../commit/list_commits_by_ref_name.go | 2 +- .../commit/list_last_commits_for_tree.go | 6 +-- internal/gitaly/service/commit/stats.go | 6 +-- .../gitaly/service/commit/tree_entries.go | 2 +- internal/gitaly/service/commit/tree_entry.go | 2 +- .../conflicts/resolve_conflicts_test.go | 4 +- .../gitaly/service/objectpool/link_test.go | 2 +- .../service/operations/apply_patch_test.go | 4 +- .../gitaly/service/operations/branches.go | 2 +- .../service/operations/branches_test.go | 6 ++- .../service/operations/cherry_pick_test.go | 29 +++++++++++---- .../service/operations/commit_files_test.go | 29 ++++++++++----- .../gitaly/service/operations/merge_test.go | 20 +++++++--- .../gitaly/service/operations/rebase_test.go | 23 +++++++++--- .../gitaly/service/operations/revert_test.go | 37 +++++++++++++------ .../gitaly/service/operations/squash_test.go | 13 +++++-- .../service/operations/submodules_test.go | 5 ++- .../gitaly/service/operations/tags_test.go | 12 ++++-- .../operations/update_branches_test.go | 5 ++- internal/gitaly/service/ref/branches.go | 2 +- internal/gitaly/service/ref/branches_test.go | 4 +- internal/gitaly/service/ref/list_new_blobs.go | 6 +-- .../gitaly/service/ref/list_new_commits.go | 6 +-- internal/gitaly/service/ref/refs.go | 24 ++++++------ internal/gitaly/service/ref/refs_test.go | 15 +++++--- .../gitaly/service/ref/remote_branches.go | 6 +-- .../service/ref/remote_branches_test.go | 6 ++- internal/gitaly/service/ref/tag_messages.go | 2 +- internal/gitaly/service/register.go | 2 +- .../service/repository/apply_gitattributes.go | 2 +- internal/gitaly/service/repository/archive.go | 6 +-- .../repository/create_from_bundle_test.go | 2 +- .../gitaly/service/repository/fetch_test.go | 4 +- internal/gitaly/service/repository/gc.go | 2 +- .../gitaly/service/repository/raw_changes.go | 2 +- .../service/repository/snapshot_test.go | 8 ++-- .../gitaly/service/wiki/delete_page_test.go | 2 +- .../gitaly/service/wiki/testhelper_test.go | 2 +- .../gitaly/service/wiki/update_page_test.go | 2 +- .../gitaly/service/wiki/write_page_test.go | 2 +- 70 files changed, 321 insertions(+), 192 deletions(-) diff --git a/cmd/gitaly-git2go/submodule_test.go b/cmd/gitaly-git2go/submodule_test.go index cf9ab14b114..51bb84d9f57 100644 --- a/cmd/gitaly-git2go/submodule_test.go +++ b/cmd/gitaly-git2go/submodule_test.go @@ -101,7 +101,7 @@ func TestSubmodule(t *testing.T) { } require.NoError(t, err) - commit, err := log.GetCommit(ctx, testRepo, response.CommitID) + commit, err := log.GetCommit(ctx, config.NewLocator(config.Config), testRepo, response.CommitID) require.NoError(t, err) require.Equal(t, commit.Author.Email, testhelper.TestUser.Email) require.Equal(t, commit.Committer.Email, testhelper.TestUser.Email) diff --git a/internal/git/catfile/batch.go b/internal/git/catfile/batch.go index 58bf98c306c..f4a8d125e0f 100644 --- a/internal/git/catfile/batch.go +++ b/internal/git/catfile/batch.go @@ -12,6 +12,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/alternates" "gitlab.com/gitlab-org/gitaly/internal/git/repository" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/labkit/correlation" ) @@ -34,12 +35,14 @@ type batchProcess struct { sync.Mutex } -func newBatchProcess(ctx context.Context, repo repository.GitRepo) (*batchProcess, error) { - repoPath, env, err := alternates.PathAndEnv(repo) +func newBatchProcess(ctx context.Context, locator storage.Locator, repo repository.GitRepo) (*batchProcess, error) { + repoPath, err := locator.GetRepoPath(repo) if err != nil { return nil, err } + env := alternates.Env(repoPath, repo.GetGitObjectDirectory(), repo.GetGitAlternateObjectDirectories()) + totalCatfileProcesses.Inc() b := &batchProcess{} diff --git a/internal/git/catfile/batchcheck.go b/internal/git/catfile/batchcheck.go index 28afdd9b717..f11075de7f0 100644 --- a/internal/git/catfile/batchcheck.go +++ b/internal/git/catfile/batchcheck.go @@ -11,6 +11,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/alternates" "gitlab.com/gitlab-org/gitaly/internal/git/repository" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/labkit/correlation" ) @@ -21,12 +22,14 @@ type batchCheck struct { sync.Mutex } -func newBatchCheck(ctx context.Context, repo repository.GitRepo) (*batchCheck, error) { - repoPath, env, err := alternates.PathAndEnv(repo) +func newBatchCheck(ctx context.Context, locator storage.Locator, repo repository.GitRepo) (*batchCheck, error) { + repoPath, err := locator.GetRepoPath(repo) if err != nil { return nil, err } + env := alternates.Env(repoPath, repo.GetGitObjectDirectory(), repo.GetGitAlternateObjectDirectories()) + bc := &batchCheck{} var stdinReader io.Reader diff --git a/internal/git/catfile/catfile.go b/internal/git/catfile/catfile.go index 57769d7965e..f07026c61e5 100644 --- a/internal/git/catfile/catfile.go +++ b/internal/git/catfile/catfile.go @@ -8,6 +8,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "gitlab.com/gitlab-org/gitaly/internal/git/repository" "gitlab.com/gitlab-org/gitaly/internal/metadata" + "gitlab.com/gitlab-org/gitaly/internal/storage" ) var catfileCacheCounter = prometheus.NewCounterVec( @@ -141,14 +142,14 @@ func (c *batch) isClosed() bool { // New returns a new Batch instance. It is important that ctx gets canceled // somewhere, because if it doesn't the cat-file processes spawned by // New() never terminate. -func New(ctx context.Context, repo repository.GitRepo) (Batch, error) { +func New(ctx context.Context, locator storage.Locator, repo repository.GitRepo) (Batch, error) { if ctx.Done() == nil { panic("empty ctx.Done() in catfile.Batch.New()") } sessionID := metadata.GetValue(ctx, SessionIDField) if sessionID == "" { - c, err := newBatch(ctx, repo) + c, err := newBatch(ctx, locator, repo) if err != nil { return nil, err } @@ -166,7 +167,7 @@ func New(ctx context.Context, repo repository.GitRepo) (Batch, error) { // if we are using caching, create a fresh context for the new batch // and initialize the new batch with a cache key and cancel function cacheCtx, cacheCancel := context.WithCancel(context.Background()) - c, err := newBatch(cacheCtx, repo) + c, err := newBatch(cacheCtx, locator, repo) if err != nil { cacheCancel() return nil, err @@ -200,7 +201,7 @@ type simulatedBatchSpawnError struct{} func (simulatedBatchSpawnError) Error() string { return "simulated spawn error" } -func newBatch(ctx context.Context, repo repository.GitRepo) (_ *batch, err error) { +func newBatch(ctx context.Context, locator storage.Locator, repo repository.GitRepo) (_ *batch, err error) { ctx, cancel := context.WithCancel(ctx) defer func() { if err != nil { @@ -208,12 +209,12 @@ func newBatch(ctx context.Context, repo repository.GitRepo) (_ *batch, err error } }() - b, err := newBatchProcess(ctx, repo) + b, err := newBatchProcess(ctx, locator, repo) if err != nil { return nil, err } - batchCheck, err := newBatchCheck(ctx, repo) + batchCheck, err := newBatchCheck(ctx, locator, repo) if err != nil { return nil, err } diff --git a/internal/git/catfile/catfile_test.go b/internal/git/catfile/catfile_test.go index a370e686de9..9f0a6827484 100644 --- a/internal/git/catfile/catfile_test.go +++ b/internal/git/catfile/catfile_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/command" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -26,7 +27,7 @@ func TestInfo(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, testRepository) + c, err := New(ctx, config.NewLocator(config.Config), testRepository) require.NoError(t, err) testCases := []struct { @@ -62,7 +63,7 @@ func TestBlob(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, testRepository) + c, err := New(ctx, config.NewLocator(config.Config), testRepository) require.NoError(t, err) gitignoreBytes, err := ioutil.ReadFile("testdata/blob-dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82") @@ -129,7 +130,7 @@ func TestCommit(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, testRepository) + c, err := New(ctx, config.NewLocator(config.Config), testRepository) require.NoError(t, err) commitBytes, err := ioutil.ReadFile("testdata/commit-e63f41fe459e62e1228fcef60d7189127aeba95a") @@ -167,7 +168,7 @@ func TestTag(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, testRepository) + c, err := New(ctx, config.NewLocator(config.Config), testRepository) require.NoError(t, err) tagBytes, err := ioutil.ReadFile("testdata/tag-a509fa67c27202a2bc9dd5e014b4af7e6063ac76") @@ -234,7 +235,7 @@ func TestTree(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, testRepository) + c, err := New(ctx, config.NewLocator(config.Config), testRepository) require.NoError(t, err) treeBytes, err := ioutil.ReadFile("testdata/tree-7e2f26d033ee47cd0745649d1a28277c56197921") @@ -301,7 +302,7 @@ func TestRepeatedCalls(t *testing.T) { testRepository, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - c, err := New(ctx, testRepository) + c, err := New(ctx, config.NewLocator(config.Config), testRepository) require.NoError(t, err) treeOid := "7e2f26d033ee47cd0745649d1a28277c56197921" @@ -416,7 +417,7 @@ func catfileWithFreshSessionID(ctx context.Context, repo *gitalypb.Repository) ( SessionIDField: id, }) - return New(metadata.NewIncomingContext(ctx, md), repo) + return New(metadata.NewIncomingContext(ctx, md), config.NewLocator(config.Config), repo) } func waitTrue(callback func() bool) bool { diff --git a/internal/git/log/commit.go b/internal/git/log/commit.go index e6bf695c860..cf25df46565 100644 --- a/internal/git/log/commit.go +++ b/internal/git/log/commit.go @@ -13,13 +13,14 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) // GetCommit tries to resolve revision to a Git commit. Returns nil if // no object is found at revision. -func GetCommit(ctx context.Context, repo *gitalypb.Repository, revision string) (*gitalypb.GitCommit, error) { - c, err := catfile.New(ctx, repo) +func GetCommit(ctx context.Context, locator storage.Locator, repo *gitalypb.Repository, revision string) (*gitalypb.GitCommit, error) { + c, err := catfile.New(ctx, locator, repo) if err != nil { return nil, err } diff --git a/internal/git/log/commit_test.go b/internal/git/log/commit_test.go index 470ddfb5a12..4fcf0777b20 100644 --- a/internal/git/log/commit_test.go +++ b/internal/git/log/commit_test.go @@ -8,6 +8,7 @@ import ( "github.com/golang/protobuf/ptypes/timestamp" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/metadata" @@ -150,7 +151,7 @@ func TestGetCommitCatfile(t *testing.T) { }, } - c, err := catfile.New(ctx, testRepo) + c, err := catfile.New(ctx, config.NewLocator(config.Config), testRepo) require.NoError(t, err) for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/git/log/log.go b/internal/git/log/log.go index 462a9ed557c..6081a43d7c0 100644 --- a/internal/git/log/log.go +++ b/internal/git/log/log.go @@ -7,6 +7,7 @@ import ( "io" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -22,8 +23,8 @@ type Parser struct { } // NewLogParser returns a new Parser -func NewLogParser(ctx context.Context, repo *gitalypb.Repository, src io.Reader) (*Parser, error) { - c, err := catfile.New(ctx, repo) +func NewLogParser(ctx context.Context, locator storage.Locator, repo *gitalypb.Repository, src io.Reader) (*Parser, error) { + c, err := catfile.New(ctx, locator, repo) if err != nil { return nil, err } diff --git a/internal/git/log/tag_test.go b/internal/git/log/tag_test.go index ab975ee25f1..31a639890f6 100644 --- a/internal/git/log/tag_test.go +++ b/internal/git/log/tag_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/testhelper" ) @@ -46,7 +47,7 @@ func TestGetTag(t *testing.T) { }, } - c, err := catfile.New(ctx, testRepo) + c, err := catfile.New(ctx, config.NewLocator(config.Config), testRepo) require.NoError(t, err) for _, testCase := range testCases { t.Run(testCase.tagName, func(t *testing.T) { diff --git a/internal/git/objectpool/link.go b/internal/git/objectpool/link.go index ea48e2787a7..22a5c75e5dc 100644 --- a/internal/git/objectpool/link.go +++ b/internal/git/objectpool/link.go @@ -13,7 +13,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/remote" "gitlab.com/gitlab-org/gitaly/internal/git/repository" - "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -72,7 +71,12 @@ func (o *ObjectPool) Link(ctx context.Context, repo *gitalypb.Repository) error // change "eventually" to "immediately", so that users won't see the // warning. https://gitlab.com/gitlab-org/gitaly/issues/1728 func (o *ObjectPool) removeMemberBitmaps(repo repository.GitRepo) error { - poolBitmaps, err := getBitmaps(o) + poolPath, err := o.locator.GetPath(o) + if err != nil { + return err + } + + poolBitmaps, err := getBitmaps(poolPath) if err != nil { return err } @@ -80,7 +84,12 @@ func (o *ObjectPool) removeMemberBitmaps(repo repository.GitRepo) error { return nil } - memberBitmaps, err := getBitmaps(repo) + repoPath, err := o.locator.GetPath(repo) + if err != nil { + return err + } + + memberBitmaps, err := getBitmaps(repoPath) if err != nil { return err } @@ -97,12 +106,7 @@ func (o *ObjectPool) removeMemberBitmaps(repo repository.GitRepo) error { return nil } -func getBitmaps(repo repository.GitRepo) ([]string, error) { - repoPath, err := helper.GetPath(repo) - if err != nil { - return nil, err - } - +func getBitmaps(repoPath string) ([]string, error) { packDir := filepath.Join(repoPath, "objects/pack") entries, err := ioutil.ReadDir(packDir) if err != nil { diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go index 270c04de340..b801393b11b 100644 --- a/internal/git/updateref/updateref_test.go +++ b/internal/git/updateref/updateref_test.go @@ -10,6 +10,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/hooks" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -41,7 +42,8 @@ func TestCreate(t *testing.T) { ctx, testRepo, _, teardown := setup(t) defer teardown() - headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") + locator := config.NewLocator(config.Config) + headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") require.NoError(t, err) updater, err := New(ctx, testRepo) @@ -54,7 +56,7 @@ func TestCreate(t *testing.T) { require.NoError(t, updater.Wait()) // check the ref was created - commit, logErr := log.GetCommit(ctx, testRepo, ref) + commit, logErr := log.GetCommit(ctx, locator, testRepo, ref) require.NoError(t, logErr) require.Equal(t, commit.Id, sha, "reference was created with the wrong SHA") } @@ -63,7 +65,8 @@ func TestUpdate(t *testing.T) { ctx, testRepo, _, teardown := setup(t) defer teardown() - headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") + locator := config.NewLocator(config.Config) + headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") require.NoError(t, err) updater, err := New(ctx, testRepo) @@ -73,7 +76,7 @@ func TestUpdate(t *testing.T) { sha := headCommit.Id // Sanity check: ensure the ref exists before we start - commit, logErr := log.GetCommit(ctx, testRepo, ref) + commit, logErr := log.GetCommit(ctx, locator, testRepo, ref) require.NoError(t, logErr) require.NotEqual(t, commit.Id, sha, "%s points to HEAD: %s in the test repository", ref, sha) @@ -81,17 +84,17 @@ func TestUpdate(t *testing.T) { require.NoError(t, updater.Wait()) // check the ref was updated - commit, logErr = log.GetCommit(ctx, testRepo, ref) + commit, logErr = log.GetCommit(ctx, locator, testRepo, ref) require.NoError(t, logErr) require.Equal(t, commit.Id, sha, "reference was not updated") // since ref has been updated to HEAD, we know that it does not point to HEAD^. So, HEAD^ is an invalid "old value" for updating ref - parentCommit, err := log.GetCommit(ctx, testRepo, "HEAD^") + parentCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD^") require.NoError(t, err) require.Error(t, updater.Update(ref, parentCommit.Id, parentCommit.Id)) // check the ref was not updated - commit, logErr = log.GetCommit(ctx, testRepo, ref) + commit, logErr = log.GetCommit(ctx, locator, testRepo, ref) require.NoError(t, logErr) require.NotEqual(t, commit.Id, parentCommit.Id, "reference was updated when it shouldn't have been") } @@ -108,8 +111,10 @@ func TestDelete(t *testing.T) { require.NoError(t, updater.Delete(ref)) require.NoError(t, updater.Wait()) + locator := config.NewLocator(config.Config) + // check the ref was removed - _, err = log.GetCommit(ctx, testRepo, ref) + _, err = log.GetCommit(ctx, locator, testRepo, ref) require.True(t, log.IsNotFound(err), "expected 'not found' error got %v", err) } @@ -117,7 +122,9 @@ func TestBulkOperation(t *testing.T) { ctx, testRepo, _, teardown := setup(t) defer teardown() - headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") + locator := config.NewLocator(config.Config) + + headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") require.NoError(t, err) updater, err := New(ctx, testRepo) @@ -139,7 +146,9 @@ func TestContextCancelAbortsRefChanges(t *testing.T) { ctx, testRepo, _, teardown := setup(t) defer teardown() - headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") + locator := config.NewLocator(config.Config) + + headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") require.NoError(t, err) childCtx, childCancel := context.WithCancel(ctx) @@ -155,6 +164,6 @@ func TestContextCancelAbortsRefChanges(t *testing.T) { require.Error(t, updater.Wait()) // check the ref doesn't exist - _, err = log.GetCommit(ctx, testRepo, ref) + _, err = log.GetCommit(ctx, locator, testRepo, ref) require.True(t, log.IsNotFound(err), "expected 'not found' error got %v", err) } diff --git a/internal/gitaly/service/blob/get_blob.go b/internal/gitaly/service/blob/get_blob.go index b94c4e03087..c3f36d4d2f8 100644 --- a/internal/gitaly/service/blob/get_blob.go +++ b/internal/gitaly/service/blob/get_blob.go @@ -19,7 +19,7 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic return status.Errorf(codes.InvalidArgument, "GetBlob: %v", err) } - c, err := catfile.New(stream.Context(), in.Repository) + c, err := catfile.New(stream.Context(), s.locator, in.Repository) if err != nil { return status.Errorf(codes.Internal, "GetBlob: %v", err) } diff --git a/internal/gitaly/service/blob/get_blobs.go b/internal/gitaly/service/blob/get_blobs.go index ba3fec0b491..f8eb5ad867d 100644 --- a/internal/gitaly/service/blob/get_blobs.go +++ b/internal/gitaly/service/blob/get_blobs.go @@ -139,12 +139,12 @@ func sendBlobTreeEntry(response *gitalypb.GetBlobsResponse, stream gitalypb.Blob return nil } -func (*server) GetBlobs(req *gitalypb.GetBlobsRequest, stream gitalypb.BlobService_GetBlobsServer) error { +func (s *server) GetBlobs(req *gitalypb.GetBlobsRequest, stream gitalypb.BlobService_GetBlobsServer) error { if err := validateGetBlobsRequest(req); err != nil { return err } - c, err := catfile.New(stream.Context(), req.Repository) + c, err := catfile.New(stream.Context(), s.locator, req.Repository) if err != nil { return err } diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go index f20ee57fcaa..67b80995ae3 100644 --- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go +++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go @@ -40,7 +40,7 @@ func (s *server) ApplyBfgObjectMapStream(server gitalypb.CleanupService_ApplyBfg reader := &bfgStreamReader{firstRequest: firstRequest, server: server} chunker := chunk.New(&bfgStreamWriter{server: server}) - notifier, err := notifier.New(ctx, repo, chunker) + notifier, err := notifier.New(ctx, s.locator, repo, chunker) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go index ec436f60c0d..d910b20b57c 100644 --- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go +++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream_test.go @@ -18,6 +18,8 @@ import ( ) func TestApplyBfgObjectMapStreamSuccess(t *testing.T) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runCleanupServiceServer(t, config.Config) defer stop() @@ -30,7 +32,7 @@ func TestApplyBfgObjectMapStreamSuccess(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - headCommit, err := log.GetCommit(ctx, testRepo, "HEAD") + headCommit, err := log.GetCommit(ctx, locator, testRepo, "HEAD") require.NoError(t, err) // A known blob: the CHANGELOG in the test repository diff --git a/internal/gitaly/service/cleanup/notifier/notifier.go b/internal/gitaly/service/cleanup/notifier/notifier.go index ab2163f9a67..7817c4d6010 100644 --- a/internal/gitaly/service/cleanup/notifier/notifier.go +++ b/internal/gitaly/service/cleanup/notifier/notifier.go @@ -5,6 +5,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/internal/helper/chunk" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -16,8 +17,8 @@ type Notifier struct { } // New instantiates a new Notifier -func New(ctx context.Context, repo *gitalypb.Repository, chunker *chunk.Chunker) (*Notifier, error) { - catfile, err := catfile.New(ctx, repo) +func New(ctx context.Context, locator storage.Locator, repo *gitalypb.Repository, chunker *chunk.Chunker) (*Notifier, error) { + catfile, err := catfile.New(ctx, locator, repo) if err != nil { return nil, err } diff --git a/internal/gitaly/service/cleanup/server.go b/internal/gitaly/service/cleanup/server.go index e482b6069e9..4e8137dbbf6 100644 --- a/internal/gitaly/service/cleanup/server.go +++ b/internal/gitaly/service/cleanup/server.go @@ -1,13 +1,15 @@ package cleanup import ( + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) type server struct { + locator storage.Locator } // NewServer creates a new instance of a grpc CleanupServer -func NewServer() gitalypb.CleanupServiceServer { - return &server{} +func NewServer(locator storage.Locator) gitalypb.CleanupServiceServer { + return &server{locator: locator} } diff --git a/internal/gitaly/service/cleanup/testhelper_test.go b/internal/gitaly/service/cleanup/testhelper_test.go index a0807ca3d86..45125dddd3a 100644 --- a/internal/gitaly/service/cleanup/testhelper_test.go +++ b/internal/gitaly/service/cleanup/testhelper_test.go @@ -29,8 +29,9 @@ func testMain(m *testing.M) int { func runCleanupServiceServer(t *testing.T, cfg config.Cfg) (string, func()) { srv := testhelper.NewServer(t, nil, nil, testhelper.WithInternalSocket(cfg)) - gitalypb.RegisterCleanupServiceServer(srv.GrpcServer(), NewServer()) - gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hookservice.NewServer(cfg, hook.NewManager(config.NewLocator(cfg), hook.GitlabAPIStub, cfg))) + locator := config.NewLocator(cfg) + gitalypb.RegisterCleanupServiceServer(srv.GrpcServer(), NewServer(locator)) + gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hookservice.NewServer(cfg, hook.NewManager(locator, hook.GitlabAPIStub, cfg))) reflection.Register(srv.GrpcServer()) require.NoError(t, srv.Start()) diff --git a/internal/gitaly/service/commit/between.go b/internal/gitaly/service/commit/between.go index 2a424ab4851..05e8df7fa3f 100644 --- a/internal/gitaly/service/commit/between.go +++ b/internal/gitaly/service/commit/between.go @@ -31,7 +31,7 @@ func (s *server) CommitsBetween(in *gitalypb.CommitsBetweenRequest, stream gital sender := &commitsBetweenSender{stream: stream} revisionRange := fmt.Sprintf("%s..%s", in.GetFrom(), in.GetTo()) - if err := sendCommits(stream.Context(), sender, in.GetRepository(), []string{revisionRange}, nil, nil, git.Flag{Name: "--reverse"}); err != nil { + if err := sendCommits(stream.Context(), sender, s.locator, in.GetRepository(), []string{revisionRange}, nil, nil, git.Flag{Name: "--reverse"}); err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/commit/commit_messages.go b/internal/gitaly/service/commit/commit_messages.go index f66ea7b5ed6..86ec95292c6 100644 --- a/internal/gitaly/service/commit/commit_messages.go +++ b/internal/gitaly/service/commit/commit_messages.go @@ -16,16 +16,16 @@ func (s *server) GetCommitMessages(request *gitalypb.GetCommitMessagesRequest, s if err := validateGetCommitMessagesRequest(request); err != nil { return helper.ErrInvalidArgument(err) } - if err := getAndStreamCommitMessages(request, stream); err != nil { + if err := s.getAndStreamCommitMessages(request, stream); err != nil { return helper.ErrInternal(err) } return nil } -func getAndStreamCommitMessages(request *gitalypb.GetCommitMessagesRequest, stream gitalypb.CommitService_GetCommitMessagesServer) error { +func (s *server) getAndStreamCommitMessages(request *gitalypb.GetCommitMessagesRequest, stream gitalypb.CommitService_GetCommitMessagesServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, request.GetRepository()) + c, err := catfile.New(ctx, s.locator, request.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/commit/commit_signatures.go b/internal/gitaly/service/commit/commit_signatures.go index 0601cdeb95c..f78f03f53fc 100644 --- a/internal/gitaly/service/commit/commit_signatures.go +++ b/internal/gitaly/service/commit/commit_signatures.go @@ -23,13 +23,13 @@ func (s *server) GetCommitSignatures(request *gitalypb.GetCommitSignaturesReques return status.Errorf(codes.InvalidArgument, "GetCommitSignatures: %v", err) } - return getCommitSignatures(s, request, stream) + return s.getCommitSignatures(request, stream) } -func getCommitSignatures(s *server, request *gitalypb.GetCommitSignaturesRequest, stream gitalypb.CommitService_GetCommitSignaturesServer) error { +func (s *server) getCommitSignatures(request *gitalypb.GetCommitSignaturesRequest, stream gitalypb.CommitService_GetCommitSignaturesServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, request.GetRepository()) + c, err := catfile.New(ctx, s.locator, request.GetRepository()) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/commit/commits_by_message.go b/internal/gitaly/service/commit/commits_by_message.go index 9a2388202f7..5ce5b1e02ef 100644 --- a/internal/gitaly/service/commit/commits_by_message.go +++ b/internal/gitaly/service/commit/commits_by_message.go @@ -6,6 +6,7 @@ import ( "github.com/golang/protobuf/proto" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -28,14 +29,14 @@ func (s *server) CommitsByMessage(in *gitalypb.CommitsByMessageRequest, stream g return helper.ErrInvalidArgument(err) } - if err := commitsByMessage(in, stream); err != nil { + if err := commitsByMessage(s.locator, in, stream); err != nil { return helper.ErrInternal(err) } return nil } -func commitsByMessage(in *gitalypb.CommitsByMessageRequest, stream gitalypb.CommitService_CommitsByMessageServer) error { +func commitsByMessage(locator storage.Locator, in *gitalypb.CommitsByMessageRequest, stream gitalypb.CommitService_CommitsByMessageServer) error { ctx := stream.Context() sender := &commitsByMessageSender{stream: stream} @@ -65,7 +66,7 @@ func commitsByMessage(in *gitalypb.CommitsByMessageRequest, stream gitalypb.Comm paths = append(paths, string(path)) } - return sendCommits(stream.Context(), sender, in.GetRepository(), []string{string(revision)}, paths, in.GetGlobalOptions(), gitLogExtraOptions...) + return sendCommits(stream.Context(), sender, locator, in.GetRepository(), []string{string(revision)}, paths, in.GetGlobalOptions(), gitLogExtraOptions...) } func validateCommitsByMessageRequest(in *gitalypb.CommitsByMessageRequest) error { diff --git a/internal/gitaly/service/commit/commits_helper.go b/internal/gitaly/service/commit/commits_helper.go index ba486b880ae..1d92a9e1133 100644 --- a/internal/gitaly/service/commit/commits_helper.go +++ b/internal/gitaly/service/commit/commits_helper.go @@ -7,16 +7,17 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/helper/chunk" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) -func sendCommits(ctx context.Context, sender chunk.Sender, repo *gitalypb.Repository, revisionRange []string, paths []string, options *gitalypb.GlobalOptions, extraArgs ...git.Option) error { +func sendCommits(ctx context.Context, sender chunk.Sender, locator storage.Locator, repo *gitalypb.Repository, revisionRange []string, paths []string, options *gitalypb.GlobalOptions, extraArgs ...git.Option) error { cmd, err := log.GitLogCommand(ctx, repo, revisionRange, paths, options, extraArgs...) if err != nil { return err } - logParser, err := log.NewLogParser(ctx, repo, cmd) + logParser, err := log.NewLogParser(ctx, locator, repo, cmd) if err != nil { return err } diff --git a/internal/gitaly/service/commit/filter_shas_with_signatures.go b/internal/gitaly/service/commit/filter_shas_with_signatures.go index 92f2aae5d6e..3152e52a045 100644 --- a/internal/gitaly/service/commit/filter_shas_with_signatures.go +++ b/internal/gitaly/service/commit/filter_shas_with_signatures.go @@ -36,7 +36,7 @@ func validateFirstFilterShasWithSignaturesRequest(in *gitalypb.FilterShasWithSig func (s *server) filterShasWithSignatures(bidi gitalypb.CommitService_FilterShasWithSignaturesServer, firstRequest *gitalypb.FilterShasWithSignaturesRequest) error { ctx := bidi.Context() - c, err := catfile.New(ctx, firstRequest.GetRepository()) + c, err := catfile.New(ctx, s.locator, firstRequest.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/commit/find_all_commits.go b/internal/gitaly/service/commit/find_all_commits.go index 5e02cad7733..40878218e00 100644 --- a/internal/gitaly/service/commit/find_all_commits.go +++ b/internal/gitaly/service/commit/find_all_commits.go @@ -7,6 +7,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/ref" "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -46,7 +47,7 @@ func (s *server) FindAllCommits(in *gitalypb.FindAllCommitsRequest, stream gital revisions = []string{string(in.GetRevision())} } - if err := findAllCommits(in, stream, revisions); err != nil { + if err := findAllCommits(s.locator, in, stream, revisions); err != nil { return helper.ErrInternal(err) } @@ -61,7 +62,7 @@ func validateFindAllCommitsRequest(in *gitalypb.FindAllCommitsRequest) error { return nil } -func findAllCommits(in *gitalypb.FindAllCommitsRequest, stream gitalypb.CommitService_FindAllCommitsServer, revisions []string) error { +func findAllCommits(locator storage.Locator, in *gitalypb.FindAllCommitsRequest, stream gitalypb.CommitService_FindAllCommitsServer, revisions []string) error { sender := &findAllCommitsSender{stream: stream} var gitLogExtraOptions []git.Option @@ -80,5 +81,5 @@ func findAllCommits(in *gitalypb.FindAllCommitsRequest, stream gitalypb.CommitSe gitLogExtraOptions = append(gitLogExtraOptions, git.Flag{Name: "--topo-order"}) } - return sendCommits(stream.Context(), sender, in.GetRepository(), revisions, nil, nil, gitLogExtraOptions...) + return sendCommits(stream.Context(), sender, locator, in.GetRepository(), revisions, nil, nil, gitLogExtraOptions...) } diff --git a/internal/gitaly/service/commit/find_commit.go b/internal/gitaly/service/commit/find_commit.go index ca2cf7ca137..9e149d3c609 100644 --- a/internal/gitaly/service/commit/find_commit.go +++ b/internal/gitaly/service/commit/find_commit.go @@ -17,7 +17,7 @@ func (s *server) FindCommit(ctx context.Context, in *gitalypb.FindCommitRequest) repo := in.GetRepository() - commit, err := log.GetCommit(ctx, repo, string(revision)) + commit, err := log.GetCommit(ctx, s.locator, repo, string(revision)) if log.IsNotFound(err) { return &gitalypb.FindCommitResponse{}, nil } diff --git a/internal/gitaly/service/commit/find_commit_test.go b/internal/gitaly/service/commit/find_commit_test.go index 82f23e042dd..166a378d2a1 100644 --- a/internal/gitaly/service/commit/find_commit_test.go +++ b/internal/gitaly/service/commit/find_commit_test.go @@ -11,6 +11,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "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/proto/go/gitalypb" @@ -35,12 +36,14 @@ func TestSuccessfulFindCommitRequest(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + bigMessage := "An empty commit with REALLY BIG message\n\n" + strings.Repeat("MOAR!\n", 20*1024) bigCommitID := testhelper.CreateCommit(t, testRepoPath, "local-big-commits", &testhelper.CreateCommitOpts{ Message: bigMessage, ParentID: "60ecb67744cb56576c30214ff52294f8ce2def98", }) - bigCommit, err := log.GetCommit(ctx, testRepo, bigCommitID) + bigCommit, err := log.GetCommit(ctx, locator, testRepo, bigCommitID) require.NoError(t, err) testCases := []struct { diff --git a/internal/gitaly/service/commit/find_commits.go b/internal/gitaly/service/commit/find_commits.go index 9f074adea9f..ca95a1429d4 100644 --- a/internal/gitaly/service/commit/find_commits.go +++ b/internal/gitaly/service/commit/find_commits.go @@ -40,21 +40,21 @@ func (s *server) FindCommits(req *gitalypb.FindCommitsRequest, stream gitalypb.C } } - if err := findCommits(ctx, req, stream); err != nil { + if err := s.findCommits(ctx, req, stream); err != nil { return helper.ErrInternal(err) } return nil } -func findCommits(ctx context.Context, req *gitalypb.FindCommitsRequest, stream gitalypb.CommitService_FindCommitsServer) error { +func (s *server) findCommits(ctx context.Context, req *gitalypb.FindCommitsRequest, stream gitalypb.CommitService_FindCommitsServer) error { globals := git.ConvertGlobalOptions(req.GetGlobalOptions()) logCmd, err := git.SafeCmd(ctx, req.GetRepository(), globals, getLogCommandSubCmd(req)) if err != nil { return fmt.Errorf("error when creating git log command: %v", err) } - batch, err := catfile.New(ctx, req.GetRepository()) + batch, err := catfile.New(ctx, s.locator, req.GetRepository()) if err != nil { return fmt.Errorf("creating catfile: %v", err) } diff --git a/internal/gitaly/service/commit/last_commit_for_path.go b/internal/gitaly/service/commit/last_commit_for_path.go index ff54582d73a..b13c9fed6c8 100644 --- a/internal/gitaly/service/commit/last_commit_for_path.go +++ b/internal/gitaly/service/commit/last_commit_for_path.go @@ -15,7 +15,7 @@ func (s *server) LastCommitForPath(ctx context.Context, in *gitalypb.LastCommitF return nil, helper.ErrInvalidArgument(err) } - resp, err := lastCommitForPath(ctx, in) + resp, err := s.lastCommitForPath(ctx, in) if err != nil { return nil, helper.ErrInternal(err) } @@ -23,14 +23,14 @@ func (s *server) LastCommitForPath(ctx context.Context, in *gitalypb.LastCommitF return resp, nil } -func lastCommitForPath(ctx context.Context, in *gitalypb.LastCommitForPathRequest) (*gitalypb.LastCommitForPathResponse, error) { +func (s *server) lastCommitForPath(ctx context.Context, in *gitalypb.LastCommitForPathRequest) (*gitalypb.LastCommitForPathResponse, error) { path := string(in.GetPath()) if len(path) == 0 || path == "/" { path = "." } repo := in.GetRepository() - c, err := catfile.New(ctx, repo) + c, err := catfile.New(ctx, s.locator, repo) if err != nil { return nil, err } diff --git a/internal/gitaly/service/commit/list_commits_by_oid.go b/internal/gitaly/service/commit/list_commits_by_oid.go index 24875761a96..27c1fb84b44 100644 --- a/internal/gitaly/service/commit/list_commits_by_oid.go +++ b/internal/gitaly/service/commit/list_commits_by_oid.go @@ -29,7 +29,7 @@ func init() { func (s *server) ListCommitsByOid(in *gitalypb.ListCommitsByOidRequest, stream gitalypb.CommitService_ListCommitsByOidServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, in.Repository) + c, err := catfile.New(ctx, s.locator, in.Repository) if err != nil { return err } diff --git a/internal/gitaly/service/commit/list_commits_by_ref_name.go b/internal/gitaly/service/commit/list_commits_by_ref_name.go index f0573dcf637..62bbd2ea491 100644 --- a/internal/gitaly/service/commit/list_commits_by_ref_name.go +++ b/internal/gitaly/service/commit/list_commits_by_ref_name.go @@ -12,7 +12,7 @@ import ( func (s *server) ListCommitsByRefName(in *gitalypb.ListCommitsByRefNameRequest, stream gitalypb.CommitService_ListCommitsByRefNameServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, in.Repository) + c, err := catfile.New(ctx, s.locator, in.Repository) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/commit/list_last_commits_for_tree.go b/internal/gitaly/service/commit/list_last_commits_for_tree.go index 8ca02c3d3a5..c7cac99f21f 100644 --- a/internal/gitaly/service/commit/list_last_commits_for_tree.go +++ b/internal/gitaly/service/commit/list_last_commits_for_tree.go @@ -30,14 +30,14 @@ func (s *server) ListLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeReque return helper.ErrInvalidArgument(err) } - if err := listLastCommitsForTree(in, stream); err != nil { + if err := s.listLastCommitsForTree(in, stream); err != nil { return helper.ErrInternal(err) } return nil } -func listLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeRequest, stream gitalypb.CommitService_ListLastCommitsForTreeServer) error { +func (s *server) listLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeRequest, stream gitalypb.CommitService_ListLastCommitsForTreeServer) error { cmd, parser, err := newLSTreeParser(in, stream) if err != nil { return err @@ -45,7 +45,7 @@ func listLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeRequest, stream g ctx := stream.Context() repo := in.GetRepository() - c, err := catfile.New(ctx, repo) + c, err := catfile.New(ctx, s.locator, repo) if err != nil { return err } diff --git a/internal/gitaly/service/commit/stats.go b/internal/gitaly/service/commit/stats.go index 1eb7efd0a13..2102b757d4a 100644 --- a/internal/gitaly/service/commit/stats.go +++ b/internal/gitaly/service/commit/stats.go @@ -18,7 +18,7 @@ func (s *server) CommitStats(ctx context.Context, in *gitalypb.CommitStatsReques return nil, helper.ErrInvalidArgument(err) } - resp, err := commitStats(ctx, in) + resp, err := s.commitStats(ctx, in) if err != nil { return nil, helper.ErrInternal(err) } @@ -26,8 +26,8 @@ func (s *server) CommitStats(ctx context.Context, in *gitalypb.CommitStatsReques return resp, nil } -func commitStats(ctx context.Context, in *gitalypb.CommitStatsRequest) (*gitalypb.CommitStatsResponse, error) { - commit, err := log.GetCommit(ctx, in.Repository, string(in.Revision)) +func (s *server) commitStats(ctx context.Context, in *gitalypb.CommitStatsRequest) (*gitalypb.CommitStatsResponse, error) { + commit, err := log.GetCommit(ctx, s.locator, in.Repository, string(in.Revision)) if err != nil { return nil, err } diff --git a/internal/gitaly/service/commit/tree_entries.go b/internal/gitaly/service/commit/tree_entries.go index 4e873627ac0..7d5c3c2f91f 100644 --- a/internal/gitaly/service/commit/tree_entries.go +++ b/internal/gitaly/service/commit/tree_entries.go @@ -99,7 +99,7 @@ func (s *server) GetTreeEntries(in *gitalypb.GetTreeEntriesRequest, stream gital return status.Errorf(codes.InvalidArgument, "TreeEntry: %v", err) } - c, err := catfile.New(stream.Context(), in.Repository) + c, err := catfile.New(stream.Context(), s.locator, in.Repository) if err != nil { return err } diff --git a/internal/gitaly/service/commit/tree_entry.go b/internal/gitaly/service/commit/tree_entry.go index b11fdd658f4..00663bfa37c 100644 --- a/internal/gitaly/service/commit/tree_entry.go +++ b/internal/gitaly/service/commit/tree_entry.go @@ -125,7 +125,7 @@ func (s *server) TreeEntry(in *gitalypb.TreeEntryRequest, stream gitalypb.Commit requestPath = strings.TrimRight(requestPath, "/") } - c, err := catfile.New(stream.Context(), in.Repository) + c, err := catfile.New(stream.Context(), s.locator, in.Repository) if err != nil { return err diff --git a/internal/gitaly/service/conflicts/resolve_conflicts_test.go b/internal/gitaly/service/conflicts/resolve_conflicts_test.go index 1e334be30dc..508e667bb45 100644 --- a/internal/gitaly/service/conflicts/resolve_conflicts_test.go +++ b/internal/gitaly/service/conflicts/resolve_conflicts_test.go @@ -37,6 +37,8 @@ func TestSuccessfulResolveConflictsRequest(t *testing.T) { } func testSuccessfulResolveConflictsRequest(t *testing.T, ctx context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, clean := runFullServer(t) defer clean() @@ -110,7 +112,7 @@ func testSuccessfulResolveConflictsRequest(t *testing.T, ctx context.Context) { require.NoError(t, err) require.Empty(t, r.GetResolutionError()) - headCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch) + headCommit, err := log.GetCommit(ctxOuter, locator, testRepo, sourceBranch) require.NoError(t, err) require.Contains(t, headCommit.ParentIds, "1450cd639e0bc6721eb02800169e464f212cde06") require.Contains(t, headCommit.ParentIds, "824be604a34828eb682305f0d963056cfac87b2d") diff --git a/internal/gitaly/service/objectpool/link_test.go b/internal/gitaly/service/objectpool/link_test.go index 0bd5b9e3d78..b941fe21280 100644 --- a/internal/gitaly/service/objectpool/link_test.go +++ b/internal/gitaly/service/objectpool/link_test.go @@ -82,7 +82,7 @@ func TestLink(t *testing.T) { require.NoError(t, err, "error from LinkRepositoryToObjectPool") - commit, err := log.GetCommit(ctx, testRepo, poolCommitID) + commit, err := log.GetCommit(ctx, locator, testRepo, poolCommitID) require.NoError(t, err) require.NotNil(t, commit) require.Equal(t, poolCommitID, commit.Id) diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go index 1b24b239151..f7225ee59e5 100644 --- a/internal/gitaly/service/operations/apply_patch_test.go +++ b/internal/gitaly/service/operations/apply_patch_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/streamio" @@ -124,7 +125,8 @@ func testSuccessfulUserApplyPatch(t *testing.T, ctx context.Context) { } for index, sha := range shas { - commit, err := log.GetCommit(ctx, testRepo, sha) + locator := config.NewLocator(config.Config) + commit, err := log.GetCommit(ctx, locator, testRepo, sha) require.NoError(t, err) require.NotNil(t, commit) diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go index 7da311fc5ae..61a43740eca 100644 --- a/internal/gitaly/service/operations/branches.go +++ b/internal/gitaly/service/operations/branches.go @@ -40,7 +40,7 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB // // startPointReference, err := git.NewRepository(req.Repository).GetReference(ctx, "refs/heads/"+string(req.StartPoint)) // startPointCommit, err := log.GetCommit(ctx, req.Repository, startPointReference.Target) - startPointCommit, err := log.GetCommit(ctx, req.Repository, string(req.StartPoint)) + startPointCommit, err := log.GetCommit(ctx, s.locator, req.Repository, string(req.StartPoint)) // END TODO if err != nil { return nil, status.Errorf(codes.FailedPrecondition, "revspec '%s' not found", req.StartPoint) diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go index f7e279d5ef7..c29f2a4c5e6 100644 --- a/internal/gitaly/service/operations/branches_test.go +++ b/internal/gitaly/service/operations/branches_test.go @@ -48,8 +48,9 @@ func testSuccessfulCreateBranchRequest(t *testing.T, ctx context.Context) { client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() + locator := config.NewLocator(config.Config) startPoint := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd" - startPointCommit, err := log.GetCommit(ctx, testRepo, startPoint) + startPointCommit, err := log.GetCommit(ctx, locator, testRepo, startPoint) require.NoError(t, err) testCases := []struct { @@ -320,7 +321,8 @@ func testSuccessfulCreateBranchRequestWithStartPointRefPrefix(t *testing.T, ctx // //targetCommitOK, err := log.GetCommit(ctx, testRepo, testCase.startPointCommit) // END TODO - targetCommitOK, err := log.GetCommit(ctx, testRepo, "1e292f8fedd741b75372e19097c76d327140c312") + locator := config.NewLocator(config.Config) + targetCommitOK, err := log.GetCommit(ctx, locator, testRepo, "1e292f8fedd741b75372e19097c76d327140c312") require.NoError(t, err) response, err := client.UserCreateBranch(ctx, request) diff --git a/internal/gitaly/service/operations/cherry_pick_test.go b/internal/gitaly/service/operations/cherry_pick_test.go index 1af9c597480..81b322e74d2 100644 --- a/internal/gitaly/service/operations/cherry_pick_test.go +++ b/internal/gitaly/service/operations/cherry_pick_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -16,6 +17,8 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -28,10 +31,10 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { destinationBranch := "cherry-picking-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master") + masterHeadCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "master") require.NoError(t, err) - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") + cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) testRepoCopy, testRepoCopyPath, cleanup := testhelper.NewTestRepo(t) // read-only repo @@ -154,7 +157,7 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { response, err := client.UserCherryPick(ctx, testCase.request) require.NoError(t, err) - headCommit, err := log.GetCommit(ctx, testCase.request.Repository, string(testCase.request.BranchName)) + headCommit, err := log.GetCommit(ctx, locator, testCase.request.Repository, string(testCase.request.BranchName)) require.NoError(t, err) expectedBranchUpdate := testCase.branchUpdate @@ -183,6 +186,8 @@ func TestSuccessfulGitHooksForUserCherryPickRequest(t *testing.T) { } func testSuccessfulGitHooksForUserCherryPickRequest(t *testing.T, ctxOuter context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -195,7 +200,7 @@ func testSuccessfulGitHooksForUserCherryPickRequest(t *testing.T, ctxOuter conte destinationBranch := "cherry-picking-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") + cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ @@ -230,6 +235,8 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -239,7 +246,7 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") + cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) destinationBranch := "cherry-picking-dst" @@ -310,6 +317,8 @@ func TestFailedUserCherryPickRequestDueToPreReceiveError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -322,7 +331,7 @@ func TestFailedUserCherryPickRequestDueToPreReceiveError(t *testing.T) { destinationBranch := "cherry-picking-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") + cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ @@ -355,6 +364,8 @@ func TestFailedUserCherryPickRequestDueToCreateTreeError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -368,7 +379,7 @@ func TestFailedUserCherryPickRequestDueToCreateTreeError(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") // This commit already exists in master - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "4a24d82dbca5c11c61556f3b35ca472b7463187e") + cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "4a24d82dbca5c11c61556f3b35ca472b7463187e") require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ @@ -392,6 +403,8 @@ func TestFailedUserCherryPickRequestDueToCommitError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -406,7 +419,7 @@ func TestFailedUserCherryPickRequestDueToCommitError(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", sourceBranch, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch) + cherryPickedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, sourceBranch) require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go index 32433e0bb4c..de5d815ffd4 100644 --- a/internal/gitaly/service/operations/commit_files_test.go +++ b/internal/gitaly/service/operations/commit_files_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -841,6 +842,8 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) { testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -920,7 +923,7 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) { require.Equal(t, tc.repoCreated, resp.GetBranchUpdate().GetRepoCreated()) require.Equal(t, tc.branchCreated, resp.GetBranchUpdate().GetBranchCreated()) - headCommit, err := log.GetCommit(ctx, tc.repo, tc.branchName) + headCommit, err := log.GetCommit(ctx, locator, tc.repo, tc.branchName) require.NoError(t, err) require.Equal(t, authorName, headCommit.Author.Name) require.Equal(t, testhelper.TestUser.Name, headCommit.Committer.Name) @@ -1008,6 +1011,8 @@ func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) { } func testSuccessfulUserCommitFilesRequestForceCommit(t *testing.T, ctx context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -1022,10 +1027,10 @@ func testSuccessfulUserCommitFilesRequestForceCommit(t *testing.T, ctx context.C targetBranchName := "feature" startBranchName := []byte("master") - startBranchCommit, err := log.GetCommit(ctx, testRepo, string(startBranchName)) + startBranchCommit, err := log.GetCommit(ctx, locator, testRepo, string(startBranchName)) require.NoError(t, err) - targetBranchCommit, err := log.GetCommit(ctx, testRepo, targetBranchName) + targetBranchCommit, err := log.GetCommit(ctx, locator, testRepo, targetBranchName) require.NoError(t, err) mergeBaseOut := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "merge-base", targetBranchCommit.Id, startBranchCommit.Id) @@ -1047,7 +1052,7 @@ func testSuccessfulUserCommitFilesRequestForceCommit(t *testing.T, ctx context.C require.NoError(t, err) update := resp.GetBranchUpdate() - newTargetBranchCommit, err := log.GetCommit(ctx, testRepo, targetBranchName) + newTargetBranchCommit, err := log.GetCommit(ctx, locator, testRepo, targetBranchName) require.NoError(t, err) require.Equal(t, newTargetBranchCommit.Id, update.CommitId) @@ -1059,6 +1064,8 @@ func TestSuccessfulUserCommitFilesRequestStartSha(t *testing.T) { } func testSuccessfulUserCommitFilesRequestStartSha(t *testing.T, ctx context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -1070,7 +1077,7 @@ func testSuccessfulUserCommitFilesRequestStartSha(t *testing.T, ctx context.Cont targetBranchName := "new" - startCommit, err := log.GetCommit(ctx, testRepo, "master") + startCommit, err := log.GetCommit(ctx, locator, testRepo, "master") require.NoError(t, err) headerRequest := headerRequest(testRepo, testhelper.TestUser, targetBranchName, commitFilesMessage) @@ -1086,7 +1093,7 @@ func testSuccessfulUserCommitFilesRequestStartSha(t *testing.T, ctx context.Cont require.NoError(t, err) update := resp.GetBranchUpdate() - newTargetBranchCommit, err := log.GetCommit(ctx, testRepo, targetBranchName) + newTargetBranchCommit, err := log.GetCommit(ctx, locator, testRepo, targetBranchName) require.NoError(t, err) require.Equal(t, newTargetBranchCommit.Id, update.CommitId) @@ -1109,6 +1116,8 @@ func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header // Regular table driven test did not work here as there is some state shared in the helpers between the subtests. // Running them in different top level tests works, so we use a parameterized function instead to share the code. return func(t *testing.T, ctx context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -1129,7 +1138,7 @@ func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header targetBranchName := "new" - startCommit, err := log.GetCommit(ctx, testRepo, "master") + startCommit, err := log.GetCommit(ctx, locator, testRepo, "master") require.NoError(t, err) headerRequest := headerRequest(newRepo, testhelper.TestUser, targetBranchName, commitFilesMessage) @@ -1146,7 +1155,7 @@ func testSuccessfulUserCommitFilesRemoteRepositoryRequest(setHeader func(header require.NoError(t, err) update := resp.GetBranchUpdate() - newTargetBranchCommit, err := log.GetCommit(ctx, newRepo, targetBranchName) + newTargetBranchCommit, err := log.GetCommit(ctx, locator, newRepo, targetBranchName) require.NoError(t, err) require.Equal(t, newTargetBranchCommit.Id, update.CommitId) @@ -1159,6 +1168,8 @@ func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes } func testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *testing.T, ctx context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -1199,7 +1210,7 @@ func testSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes _, err = stream.CloseAndRecv() require.NoError(t, err) - newCommit, err := log.GetCommit(ctx, testRepo, targetBranchName) + newCommit, err := log.GetCommit(ctx, locator, testRepo, targetBranchName) require.NoError(t, err) require.Equal(t, tc.author.Name, newCommit.Author.Name, "author name") diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go index 1f3ec02406c..e04a0a6ab01 100644 --- a/internal/gitaly/service/operations/merge_test.go +++ b/internal/gitaly/service/operations/merge_test.go @@ -44,6 +44,8 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -85,7 +87,7 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) { firstResponse, err := mergeBidi.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.CommitId) + _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.CommitId) require.NoError(t, err, "look up git commit before merge is applied") require.NoError(t, mergeBidi.Send(&gitalypb.UserMergeBranchRequest{Apply: true}), "apply merge") @@ -99,7 +101,7 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) { }) require.NoError(t, err, "consume EOF") - commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName) + commit, err := gitlog.GetCommit(ctx, locator, testRepo, mergeBranchName) require.NoError(t, err, "look up git commit after call has finished") require.Equal(t, gitalypb.OperationBranchUpdate{CommitId: commit.Id}, *(secondResponse.BranchUpdate)) @@ -132,6 +134,8 @@ func TestAbortedMerge(t *testing.T) { } func testAbortedMerge(t *testing.T, ctx context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -188,7 +192,7 @@ func testAbortedMerge(t *testing.T, ctx context.Context) { require.Equal(t, "", secondResponse.GetBranchUpdate().GetCommitId(), "merge should not have been applied") require.Error(t, err) - commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName) + commit, err := gitlog.GetCommit(ctx, locator, testRepo, mergeBranchName) require.NoError(t, err, "look up git commit after call has finished") require.Equal(t, mergeBranchHeadBefore, commit.Id, "branch should not change when the merge is aborted") @@ -204,6 +208,8 @@ func testFailedMergeConcurrentUpdate(t *testing.T, ctx context.Context) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -239,7 +245,7 @@ func testFailedMergeConcurrentUpdate(t *testing.T, ctx context.Context) { require.NoError(t, err, "receive second response") testhelper.ProtoEqual(t, secondResponse, &gitalypb.UserMergeBranchResponse{}) - commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName) + commit, err := gitlog.GetCommit(ctx, locator, testRepo, mergeBranchName) require.NoError(t, err, "get commit after RPC finished") require.Equal(t, commit.Id, concurrentCommitID, "RPC should not have trampled concurrent update") } @@ -497,6 +503,8 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { ctx, cleanup := testhelper.Context() defer cleanup() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -568,7 +576,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { FirstParentRef: testCase.firstParentRef, } - commitBeforeRefMerge, fetchRefBeforeMergeErr := gitlog.GetCommit(ctx, testRepo, string(testCase.targetRef)) + commitBeforeRefMerge, fetchRefBeforeMergeErr := gitlog.GetCommit(ctx, locator, testRepo, string(testCase.targetRef)) if testCase.emptyRef { require.Error(t, fetchRefBeforeMergeErr, "error when fetching empty ref commit") } else { @@ -578,7 +586,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { resp, err := client.UserMergeToRef(ctx, request) require.NoError(t, err) - commit, err := gitlog.GetCommit(ctx, testRepo, string(testCase.targetRef)) + commit, err := gitlog.GetCommit(ctx, locator, testRepo, string(testCase.targetRef)) require.NoError(t, err, "look up git commit after call has finished") // Asserts commit parent SHAs diff --git a/internal/gitaly/service/operations/rebase_test.go b/internal/gitaly/service/operations/rebase_test.go index 1741968e60f..8aeaf51b55a 100644 --- a/internal/gitaly/service/operations/rebase_test.go +++ b/internal/gitaly/service/operations/rebase_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/catfile" gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -32,6 +33,8 @@ func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) { require.NoError(t, ruby.Start()) defer ruby.Stop() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServerWithRubyServer(t, &ruby) defer stop() @@ -73,7 +76,7 @@ func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.GetRebaseSha()) + _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.GetRebaseSha()) require.NoError(t, err, "look up git commit before rebase is applied") applyRequest := buildApplyRequest(true) @@ -245,6 +248,8 @@ func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -271,7 +276,7 @@ func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.GetRebaseSha()) + _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.GetRebaseSha()) require.NoError(t, err, "look up git commit before rebase is applied") applyRequest := buildApplyRequest(false) @@ -291,6 +296,8 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -325,7 +332,7 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.GetRebaseSha()) + _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.GetRebaseSha()) require.NoError(t, err, "look up git commit before rebase is applied") applyRequest := buildApplyRequest(true) @@ -400,6 +407,8 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -434,7 +443,7 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, testRepo, firstResponse.GetRebaseSha()) + _, err = gitlog.GetCommit(ctx, locator, testRepo, firstResponse.GetRebaseSha()) require.NoError(t, err, "look up git commit before rebase is applied") applyRequest := buildApplyRequest(true) @@ -458,6 +467,8 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { } func TestRebaseOntoRemoteBranch(t *testing.T) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -490,7 +501,7 @@ func TestRebaseOntoRemoteBranch(t *testing.T) { rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - _, err = gitlog.GetCommit(ctx, localRepo, remoteBranchHash) + _, err = gitlog.GetCommit(ctx, locator, localRepo, remoteBranchHash) require.True(t, catfile.IsNotFound(err), "remote commit does not yet exist in local repository") headerRequest := buildHeaderRequest(localRepo, testhelper.TestUser, "1", localBranch, localBranchHash, remoteRepo, remoteBranch) @@ -499,7 +510,7 @@ func TestRebaseOntoRemoteBranch(t *testing.T) { firstResponse, err := rebaseStream.Recv() require.NoError(t, err, "receive first response") - _, err = gitlog.GetCommit(ctx, localRepo, remoteBranchHash) + _, err = gitlog.GetCommit(ctx, locator, localRepo, remoteBranchHash) require.NoError(t, err, "remote commit does now exist in local repository") applyRequest := buildApplyRequest(true) diff --git a/internal/gitaly/service/operations/revert_test.go b/internal/gitaly/service/operations/revert_test.go index 4786eaf9973..c674c28a5d9 100644 --- a/internal/gitaly/service/operations/revert_test.go +++ b/internal/gitaly/service/operations/revert_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -17,6 +18,8 @@ func TestServer_UserRevert_successful(t *testing.T) { } func testServerUserRevertSuccessful(t *testing.T, ctxOuter context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -29,10 +32,10 @@ func testServerUserRevertSuccessful(t *testing.T, ctxOuter context.Context) { destinationBranch := "revert-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master") + masterHeadCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "master") require.NoError(t, err) - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) testRepoCopy, testRepoCopyPath, cleanup := testhelper.NewTestRepo(t) // read-only repo @@ -155,7 +158,7 @@ func testServerUserRevertSuccessful(t *testing.T, ctxOuter context.Context) { response, err := client.UserRevert(ctx, testCase.request) require.NoError(t, err) - headCommit, err := log.GetCommit(ctx, testCase.request.Repository, string(testCase.request.BranchName)) + headCommit, err := log.GetCommit(ctx, locator, testCase.request.Repository, string(testCase.request.BranchName)) require.NoError(t, err) expectedBranchUpdate := testCase.branchUpdate @@ -181,6 +184,8 @@ func TestServer_UserRevert_successful_into_empty_repo(t *testing.T) { } func testServerUserRevertSuccessfulIntoNewRepo(t *testing.T, ctxOuter context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -190,10 +195,10 @@ func testServerUserRevertSuccessfulIntoNewRepo(t *testing.T, ctxOuter context.Co startRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - revertedCommit, err := log.GetCommit(ctxOuter, startRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, locator, startRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) - masterHeadCommit, err := log.GetCommit(ctxOuter, startRepo, "master") + masterHeadCommit, err := log.GetCommit(ctxOuter, locator, startRepo, "master") require.NoError(t, err) testRepo, _, cleanup := testhelper.InitBareRepo(t) @@ -215,7 +220,7 @@ func testServerUserRevertSuccessfulIntoNewRepo(t *testing.T, ctxOuter context.Co response, err := client.UserRevert(ctx, request) require.NoError(t, err) - headCommit, err := log.GetCommit(ctx, testRepo, string(request.BranchName)) + headCommit, err := log.GetCommit(ctx, locator, testRepo, string(request.BranchName)) require.NoError(t, err) expectedBranchUpdate := &gitalypb.OperationBranchUpdate{ @@ -236,6 +241,8 @@ func TestServer_UserRevert_successful_git_hooks(t *testing.T) { } func testServerUserRevertSuccessfulGitHooks(t *testing.T, ctxOuter context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -248,7 +255,7 @@ func testServerUserRevertSuccessfulGitHooks(t *testing.T, ctxOuter context.Conte destinationBranch := "revert-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) request := &gitalypb.UserRevertRequest{ @@ -284,6 +291,8 @@ func TestServer_UserRevert_failued_due_to_validations(t *testing.T) { } func testServerUserRevertFailuedDueToValidations(t *testing.T, ctxOuter context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -293,7 +302,7 @@ func testServerUserRevertFailuedDueToValidations(t *testing.T, ctxOuter context. testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) destinationBranch := "revert-dst" @@ -365,6 +374,8 @@ func TestServer_UserRevert_failed_due_to_pre_receive_error(t *testing.T) { } func testServerUserRevertFailedDueToPreReceiveError(t *testing.T, ctxOuter context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -377,7 +388,7 @@ func testServerUserRevertFailedDueToPreReceiveError(t *testing.T, ctxOuter conte destinationBranch := "revert-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") + revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) request := &gitalypb.UserRevertRequest{ @@ -411,6 +422,8 @@ func TestServer_UserRevert_failed_due_to_create_tree_error(t *testing.T) { } func testServerUserRevertFailedDueToCreateTreeError(t *testing.T, ctxOuter context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -424,7 +437,7 @@ func testServerUserRevertFailedDueToCreateTreeError(t *testing.T, ctxOuter conte testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") // This revert patch of the following commit cannot be applied to the destinationBranch above - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "372ab6950519549b14d220271ee2322caa44d4eb") + revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, "372ab6950519549b14d220271ee2322caa44d4eb") require.NoError(t, err) request := &gitalypb.UserRevertRequest{ @@ -449,6 +462,8 @@ func TestServer_UserRevert_failed_due_to_commit_error(t *testing.T) { } func testServerUserRevertFailedDueToCommitError(t *testing.T, ctxOuter context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -463,7 +478,7 @@ func testServerUserRevertFailedDueToCommitError(t *testing.T, ctxOuter context.C testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", sourceBranch, "a5391128b0ef5d21df5dd23d98557f4ef12fae20") - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch) + revertedCommit, err := log.GetCommit(ctxOuter, locator, testRepo, sourceBranch) require.NoError(t, err) request := &gitalypb.UserRevertRequest{ diff --git a/internal/gitaly/service/operations/squash_test.go b/internal/gitaly/service/operations/squash_test.go index 58d3a11ad43..f890a45100a 100644 --- a/internal/gitaly/service/operations/squash_test.go +++ b/internal/gitaly/service/operations/squash_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -46,6 +47,8 @@ func TestSuccessfulUserSquashRequest(t *testing.T) { } func testSuccessfulUserSquashRequest(t *testing.T, ctx context.Context, start, end string) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -69,7 +72,7 @@ func testSuccessfulUserSquashRequest(t *testing.T, ctx context.Context, start, e require.NoError(t, err) require.Empty(t, response.GetGitError()) - commit, err := log.GetCommit(ctx, testRepo, response.SquashSha) + commit, err := log.GetCommit(ctx, locator, testRepo, response.SquashSha) require.NoError(t, err) require.Equal(t, []string{start}, commit.ParentIds) require.Equal(t, author.Name, commit.Author.Name) @@ -100,6 +103,8 @@ func TestSuccessfulUserSquashRequestWith3wayMerge(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -124,7 +129,7 @@ func TestSuccessfulUserSquashRequestWith3wayMerge(t *testing.T) { require.NoError(t, err) require.Empty(t, response.GetGitError()) - commit, err := log.GetCommit(ctx, testRepo, response.SquashSha) + commit, err := log.GetCommit(ctx, locator, testRepo, response.SquashSha) require.NoError(t, err) require.Equal(t, []string{"6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"}, commit.ParentIds) require.Equal(t, author.Name, commit.Author.Name) @@ -183,6 +188,8 @@ func TestSquashRequestWithRenamedFiles(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -231,7 +238,7 @@ func TestSquashRequestWithRenamedFiles(t *testing.T) { require.NoError(t, err) require.Empty(t, response.GetGitError()) - commit, err := log.GetCommit(ctx, testRepo, response.SquashSha) + commit, err := log.GetCommit(ctx, locator, testRepo, response.SquashSha) require.NoError(t, err) require.Equal(t, []string{startCommitID}, commit.ParentIds) require.Equal(t, author.Name, commit.Author.Name) diff --git a/internal/gitaly/service/operations/submodules_test.go b/internal/gitaly/service/operations/submodules_test.go index 779d9ca4f87..51d060d4ca5 100644 --- a/internal/gitaly/service/operations/submodules_test.go +++ b/internal/gitaly/service/operations/submodules_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/git/lstree" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -22,6 +23,8 @@ func TestSuccessfulUserUpdateSubmoduleRequest(t *testing.T) { } func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -69,7 +72,7 @@ func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context) require.Empty(t, response.GetCommitError()) require.Empty(t, response.GetPreReceiveError()) - commit, err := log.GetCommit(ctx, testRepo, response.BranchUpdate.CommitId) + commit, err := log.GetCommit(ctx, locator, testRepo, response.BranchUpdate.CommitId) require.NoError(t, err) require.Equal(t, commit.Author.Email, testhelper.TestUser.Email) require.Equal(t, commit.Committer.Email, testhelper.TestUser.Email) diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go index 93596dfca85..ccd03e157a8 100644 --- a/internal/gitaly/service/operations/tags_test.go +++ b/internal/gitaly/service/operations/tags_test.go @@ -158,6 +158,8 @@ func TestSuccessfulUserCreateTagRequest(t *testing.T) { } func testSuccessfulUserCreateTagRequest(t *testing.T, ctx context.Context) { + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -168,7 +170,7 @@ func testSuccessfulUserCreateTagRequest(t *testing.T, ctx context.Context) { defer cleanupFn() targetRevision := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd" - targetRevisionCommit, err := log.GetCommit(ctx, testRepo, targetRevision) + targetRevisionCommit, err := log.GetCommit(ctx, locator, testRepo, targetRevision) require.NoError(t, err) inputTagName := "to-be-créated-soon" @@ -381,6 +383,8 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -463,7 +467,7 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) { // Fake it up for all levels, except for ^{} == "commit" responseOk.Tag.TargetCommit = response.Tag.TargetCommit if testCase.targetObjectType == "commit" { - responseOk.Tag.TargetCommit, err = log.GetCommit(ctx, testRepo, testCase.targetObject) + responseOk.Tag.TargetCommit, err = log.GetCommit(ctx, locator, testRepo, testCase.targetObject) require.NoError(t, err) } require.Equal(t, responseOk, response) @@ -541,6 +545,8 @@ func TestUserCreateTagsuccessfulCreationOfPrefixedTag(t *testing.T) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -579,7 +585,7 @@ func TestUserCreateTagsuccessfulCreationOfPrefixedTag(t *testing.T) { response, err := client.UserCreateTag(ctx, request) require.Equal(t, testCase.err, err) - commitOk, err := log.GetCommit(ctx, testRepo, testCase.tagTargetRevisionInput) + commitOk, err := log.GetCommit(ctx, locator, testRepo, testCase.tagTargetRevisionInput) require.NoError(t, err) responseOk := &gitalypb.UserCreateTagResponse{ diff --git a/internal/gitaly/service/operations/update_branches_test.go b/internal/gitaly/service/operations/update_branches_test.go index 0af47e90e13..cd55343c634 100644 --- a/internal/gitaly/service/operations/update_branches_test.go +++ b/internal/gitaly/service/operations/update_branches_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -30,6 +31,8 @@ func testSuccessfulUserUpdateBranchRequest(t *testing.T, ctx context.Context) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() + locator := config.NewLocator(config.Config) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -49,7 +52,7 @@ func testSuccessfulUserUpdateBranchRequest(t *testing.T, ctx context.Context) { require.NoError(t, err) require.Empty(t, response.PreReceiveError) - branchCommit, err := log.GetCommit(ctx, testRepo, updateBranchName) + branchCommit, err := log.GetCommit(ctx, locator, testRepo, updateBranchName) require.NoError(t, err) require.Equal(t, string(newrev), branchCommit.Id) diff --git a/internal/gitaly/service/ref/branches.go b/internal/gitaly/service/ref/branches.go index 67dcaa6a59f..d92489f5d84 100644 --- a/internal/gitaly/service/ref/branches.go +++ b/internal/gitaly/service/ref/branches.go @@ -34,7 +34,7 @@ func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest return nil, err } - commit, err := log.GetCommit(ctx, repo, branch.Target) + commit, err := log.GetCommit(ctx, s.locator, repo, branch.Target) if err != nil { return nil, err } diff --git a/internal/gitaly/service/ref/branches_test.go b/internal/gitaly/service/ref/branches_test.go index 0dbde77a894..2c1557dac75 100644 --- a/internal/gitaly/service/ref/branches_test.go +++ b/internal/gitaly/service/ref/branches_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -14,6 +15,7 @@ func TestSuccessfulFindBranchRequest(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -24,7 +26,7 @@ func TestSuccessfulFindBranchRequest(t *testing.T) { defer cleanupFn() branchNameInput := "master" - branchTarget, err := log.GetCommit(ctx, testRepo, branchNameInput) + branchTarget, err := log.GetCommit(ctx, locator, testRepo, branchNameInput) require.NoError(t, err) branch := &gitalypb.Branch{ diff --git a/internal/gitaly/service/ref/list_new_blobs.go b/internal/gitaly/service/ref/list_new_blobs.go index f942eee529a..d23019c84a8 100644 --- a/internal/gitaly/service/ref/list_new_blobs.go +++ b/internal/gitaly/service/ref/list_new_blobs.go @@ -17,14 +17,14 @@ func (s *server) ListNewBlobs(in *gitalypb.ListNewBlobsRequest, stream gitalypb. return helper.ErrInvalidArgument(err) } - if err := listNewBlobs(in, stream, oid); err != nil { + if err := s.listNewBlobs(in, stream, oid); err != nil { return helper.ErrInternal(err) } return nil } -func listNewBlobs(in *gitalypb.ListNewBlobsRequest, stream gitalypb.RefService_ListNewBlobsServer, oid string) error { +func (s *server) listNewBlobs(in *gitalypb.ListNewBlobsRequest, stream gitalypb.RefService_ListNewBlobsServer, oid string) error { ctx := stream.Context() cmdFlags := []git.Option{git.Flag{Name: "--objects"}, git.Flag{Name: "--not"}, git.Flag{Name: "--all"}} @@ -38,7 +38,7 @@ func listNewBlobs(in *gitalypb.ListNewBlobsRequest, stream gitalypb.RefService_L return err } - batch, err := catfile.New(ctx, in.GetRepository()) + batch, err := catfile.New(ctx, s.locator, in.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/ref/list_new_commits.go b/internal/gitaly/service/ref/list_new_commits.go index e5533e42aa3..1f12f07e62e 100644 --- a/internal/gitaly/service/ref/list_new_commits.go +++ b/internal/gitaly/service/ref/list_new_commits.go @@ -16,14 +16,14 @@ func (s *server) ListNewCommits(in *gitalypb.ListNewCommitsRequest, stream gital return helper.ErrInvalidArgument(err) } - if err := listNewCommits(in, stream, oid); err != nil { + if err := s.listNewCommits(in, stream, oid); err != nil { return helper.ErrInternal(err) } return nil } -func listNewCommits(in *gitalypb.ListNewCommitsRequest, stream gitalypb.RefService_ListNewCommitsServer, oid string) error { +func (s *server) listNewCommits(in *gitalypb.ListNewCommitsRequest, stream gitalypb.RefService_ListNewCommitsServer, oid string) error { ctx := stream.Context() revList, err := git.SafeCmd(ctx, in.GetRepository(), nil, git.SubCmd{ @@ -35,7 +35,7 @@ func listNewCommits(in *gitalypb.ListNewCommitsRequest, stream gitalypb.RefServi return err } - batch, err := catfile.New(ctx, in.GetRepository()) + batch, err := catfile.New(ctx, s.locator, in.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go index 82b7c996496..f122bc8d791 100644 --- a/internal/gitaly/service/ref/refs.go +++ b/internal/gitaly/service/ref/refs.go @@ -87,7 +87,7 @@ func (t *tagSender) Send() error { }) } -func parseAndReturnTags(ctx context.Context, repo *gitalypb.Repository, stream gitalypb.RefService_FindAllTagsServer) error { +func (s *server) parseAndReturnTags(ctx context.Context, repo *gitalypb.Repository, stream gitalypb.RefService_FindAllTagsServer) error { tagsCmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{ Name: "for-each-ref", Flags: []git.Option{ @@ -99,7 +99,7 @@ func parseAndReturnTags(ctx context.Context, repo *gitalypb.Repository, stream g return fmt.Errorf("for-each-ref error: %v", err) } - c, err := catfile.New(ctx, repo) + c, err := catfile.New(ctx, s.locator, repo) if err != nil { return fmt.Errorf("error creating catfile: %v", err) } @@ -136,7 +136,7 @@ func (s *server) FindAllTags(in *gitalypb.FindAllTagsRequest, stream gitalypb.Re return helper.ErrInvalidArgument(err) } - if err := parseAndReturnTags(ctx, in.GetRepository(), stream); err != nil { + if err := s.parseAndReturnTags(ctx, in.GetRepository(), stream); err != nil { return helper.ErrInternal(err) } return nil @@ -291,16 +291,16 @@ func parseSortKey(sortKey gitalypb.FindLocalBranchesRequest_SortBy) string { // FindLocalBranches creates a stream of branches for all local branches in the given repository func (s *server) FindLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream gitalypb.RefService_FindLocalBranchesServer) error { - if err := findLocalBranches(in, stream); err != nil { + if err := s.findLocalBranches(in, stream); err != nil { return helper.ErrInternal(err) } return nil } -func findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream gitalypb.RefService_FindLocalBranchesServer) error { +func (s *server) findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream gitalypb.RefService_FindLocalBranchesServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, in.Repository) + c, err := catfile.New(ctx, s.locator, in.Repository) if err != nil { return err } @@ -317,14 +317,14 @@ func findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream gitalypb.Re } func (s *server) FindAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefService_FindAllBranchesServer) error { - if err := findAllBranches(in, stream); err != nil { + if err := s.findAllBranches(in, stream); err != nil { return helper.ErrInternal(err) } return nil } -func findAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefService_FindAllBranchesServer) error { +func (s *server) findAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefService_FindAllBranchesServer) error { args := []git.Option{ // %00 inserts the null character into the output (see for-each-ref docs) git.Flag{Name: "--format=" + strings.Join(localBranchFormatFields, "%00")}, @@ -350,7 +350,7 @@ func findAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefSer } ctx := stream.Context() - c, err := catfile.New(ctx, in.Repository) + c, err := catfile.New(ctx, s.locator, in.Repository) if err != nil { return err } @@ -371,7 +371,7 @@ func (s *server) FindTag(ctx context.Context, in *gitalypb.FindTagRequest) (*git var tag *gitalypb.Tag - if tag, err = findTag(ctx, in.GetRepository(), in.GetTagName()); err != nil { + if tag, err = s.findTag(ctx, in.GetRepository(), in.GetTagName()); err != nil { return nil, helper.ErrInternal(err) } @@ -412,7 +412,7 @@ func parseTagLine(ctx context.Context, c catfile.Batch, tagLine string) (*gitaly } } -func findTag(ctx context.Context, repository *gitalypb.Repository, tagName []byte) (*gitalypb.Tag, error) { +func (s *server) findTag(ctx context.Context, repository *gitalypb.Repository, tagName []byte) (*gitalypb.Tag, error) { tagCmd, err := git.SafeCmd(ctx, repository, nil, git.SubCmd{ Name: "tag", @@ -427,7 +427,7 @@ func findTag(ctx context.Context, repository *gitalypb.Repository, tagName []byt return nil, fmt.Errorf("for-each-ref error: %v", err) } - c, err := catfile.New(ctx, repository) + c, err := catfile.New(ctx, s.locator, repository) if err != nil { return nil, err } diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go index 0a655e2f9bd..fbee01fbf50 100644 --- a/internal/gitaly/service/ref/refs_test.go +++ b/internal/gitaly/service/ref/refs_test.go @@ -471,6 +471,7 @@ func TestInvalidRepoFindDefaultBranchNameRequest(t *testing.T) { } func TestSuccessfulFindAllTagsRequest(t *testing.T) { + locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -498,7 +499,7 @@ func TestSuccessfulFindAllTagsRequest(t *testing.T) { Message: "An empty commit with REALLY BIG message\n\n" + strings.Repeat("a", helper.MaxCommitOrTagMessageSize+1), ParentID: "60ecb67744cb56576c30214ff52294f8ce2def98", }) - bigCommit, err := log.GetCommit(ctx, testRepoCopy, bigCommitID) + bigCommit, err := log.GetCommit(ctx, locator, testRepoCopy, bigCommitID) require.NoError(t, err) annotatedTagID := testhelper.CreateTag(t, testRepoCopyPath, "v1.2.0", blobID, &testhelper.CreateTagOpts{Message: "Blob tag"}) @@ -672,6 +673,7 @@ func TestSuccessfulFindAllTagsRequest(t *testing.T) { } func TestFindAllTagNestedTags(t *testing.T) { + locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -716,7 +718,7 @@ func TestFindAllTagNestedTags(t *testing.T) { tags := bytes.NewReader(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoCopyPath, "tag")) testhelper.MustRunCommand(t, tags, "xargs", config.Config.Git.BinPath, "-C", testRepoCopyPath, "tag", "-d") - batch, err := catfile.New(ctx, testRepoCopy) + batch, err := catfile.New(ctx, locator, testRepoCopy) require.NoError(t, err) info, err := batch.Info(ctx, tc.originalOid) @@ -1108,6 +1110,7 @@ func TestSuccessfulFindAllBranchesRequest(t *testing.T) { } func TestSuccessfulFindAllBranchesRequestWithMergedBranches(t *testing.T) { + locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -1143,7 +1146,7 @@ func TestSuccessfulFindAllBranchesRequestWithMergedBranches(t *testing.T) { expectedBranches = append(expectedBranches, branch) } - masterCommit, err := log.GetCommit(ctx, testRepo, "master") + masterCommit, err := log.GetCommit(ctx, locator, testRepo, "master") require.NoError(t, err) expectedBranches = append(expectedBranches, &gitalypb.FindAllBranchesResponse_Branch{ Name: []byte("refs/heads/master"), @@ -1430,6 +1433,7 @@ func TestListBranchNamesContainingCommit(t *testing.T) { } func TestSuccessfulFindTagRequest(t *testing.T) { + locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -1448,7 +1452,7 @@ func TestSuccessfulFindTagRequest(t *testing.T) { Message: "An empty commit with REALLY BIG message\n\n" + strings.Repeat("a", helper.MaxCommitOrTagMessageSize+1), ParentID: "60ecb67744cb56576c30214ff52294f8ce2def98", }) - bigCommit, err := log.GetCommit(ctx, testRepoCopy, bigCommitID) + bigCommit, err := log.GetCommit(ctx, locator, testRepoCopy, bigCommitID) require.NoError(t, err) annotatedTagID := testhelper.CreateTag(t, testRepoCopyPath, "v1.2.0", blobID, &testhelper.CreateTagOpts{Message: "Blob tag"}) @@ -1600,6 +1604,7 @@ func TestSuccessfulFindTagRequest(t *testing.T) { } func TestFindTagNestedTag(t *testing.T) { + locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -1647,7 +1652,7 @@ func TestFindTagNestedTag(t *testing.T) { tags := bytes.NewReader(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoCopyPath, "tag")) testhelper.MustRunCommand(t, tags, "xargs", config.Config.Git.BinPath, "-C", testRepoCopyPath, "tag", "-d") - batch, err := catfile.New(ctx, testRepoCopy) + batch, err := catfile.New(ctx, locator, testRepoCopy) require.NoError(t, err) info, err := batch.Info(ctx, tc.originalOid) diff --git a/internal/gitaly/service/ref/remote_branches.go b/internal/gitaly/service/ref/remote_branches.go index f72b4d2969b..0aeafd478c3 100644 --- a/internal/gitaly/service/ref/remote_branches.go +++ b/internal/gitaly/service/ref/remote_branches.go @@ -15,14 +15,14 @@ func (s *server) FindAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesReques return helper.ErrInvalidArgument(err) } - if err := findAllRemoteBranches(req, stream); err != nil { + if err := s.findAllRemoteBranches(req, stream); err != nil { return helper.ErrInternal(err) } return nil } -func findAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesRequest, stream gitalypb.RefService_FindAllRemoteBranchesServer) error { +func (s *server) findAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesRequest, stream gitalypb.RefService_FindAllRemoteBranchesServer) error { args := []git.Option{ git.Flag{Name: "--format=" + strings.Join(localBranchFormatFields, "%00")}, } @@ -30,7 +30,7 @@ func findAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesRequest, stream gi patterns := []string{"refs/remotes/" + req.GetRemoteName()} ctx := stream.Context() - c, err := catfile.New(ctx, req.GetRepository()) + c, err := catfile.New(ctx, s.locator, req.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/ref/remote_branches_test.go b/internal/gitaly/service/ref/remote_branches_test.go index a88ca9f6130..23650367df6 100644 --- a/internal/gitaly/service/ref/remote_branches_test.go +++ b/internal/gitaly/service/ref/remote_branches_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -15,6 +16,7 @@ func TestSuccessfulFindAllRemoteBranchesRequest(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() + locator := config.NewLocator(config.Config) stop, serverSocketPath := runRefServiceServer(t) defer stop() @@ -53,7 +55,7 @@ func TestSuccessfulFindAllRemoteBranchesRequest(t *testing.T) { require.Len(t, branches, len(expectedBranches)) for branchName, commitID := range expectedBranches { - targetCommit, err := log.GetCommit(ctx, testRepo, commitID) + targetCommit, err := log.GetCommit(ctx, locator, testRepo, commitID) require.NoError(t, err) expectedBranch := &gitalypb.Branch{ @@ -65,7 +67,7 @@ func TestSuccessfulFindAllRemoteBranchesRequest(t *testing.T) { } for branchName, commitID := range excludedBranches { - targetCommit, err := log.GetCommit(ctx, testRepo, commitID) + targetCommit, err := log.GetCommit(ctx, locator, testRepo, commitID) require.NoError(t, err) excludedBranch := &gitalypb.Branch{ diff --git a/internal/gitaly/service/ref/tag_messages.go b/internal/gitaly/service/ref/tag_messages.go index d4ec3e07ecd..1539b19c398 100644 --- a/internal/gitaly/service/ref/tag_messages.go +++ b/internal/gitaly/service/ref/tag_messages.go @@ -36,7 +36,7 @@ func validateGetTagMessagesRequest(request *gitalypb.GetTagMessagesRequest) erro func (s *server) getAndStreamTagMessages(request *gitalypb.GetTagMessagesRequest, stream gitalypb.RefService_GetTagMessagesServer) error { ctx := stream.Context() - c, err := catfile.New(ctx, request.GetRepository()) + c, err := catfile.New(ctx, s.locator, request.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go index 65f898fc74b..4d55026eead 100644 --- a/internal/gitaly/service/register.go +++ b/internal/gitaly/service/register.go @@ -70,7 +70,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver }) gitalypb.RegisterBlobServiceServer(grpcServer, blob.NewServer(rubyServer, locator)) - gitalypb.RegisterCleanupServiceServer(grpcServer, cleanup.NewServer()) + gitalypb.RegisterCleanupServiceServer(grpcServer, cleanup.NewServer(locator)) gitalypb.RegisterCommitServiceServer(grpcServer, commit.NewServer(cfg, locator)) gitalypb.RegisterDiffServiceServer(grpcServer, diff.NewServer(locator)) gitalypb.RegisterNamespaceServiceServer(grpcServer, namespace.NewServer(locator)) diff --git a/internal/gitaly/service/repository/apply_gitattributes.go b/internal/gitaly/service/repository/apply_gitattributes.go index a49b5556c23..eef1e868a84 100644 --- a/internal/gitaly/service/repository/apply_gitattributes.go +++ b/internal/gitaly/service/repository/apply_gitattributes.go @@ -89,7 +89,7 @@ func (s *server) ApplyGitattributes(ctx context.Context, in *gitalypb.ApplyGitat return nil, status.Errorf(codes.InvalidArgument, "ApplyGitAttributes: revision: %v", err) } - c, err := catfile.New(ctx, repo) + c, err := catfile.New(ctx, s.locator, repo) if err != nil { return nil, err } diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go index 4cbc96273b0..7c620e5a24c 100644 --- a/internal/gitaly/service/repository/archive.go +++ b/internal/gitaly/service/repository/archive.go @@ -62,7 +62,7 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo return err } - if err := validateGetArchivePrecondition(ctx, in, path, exclude); err != nil { + if err := s.validateGetArchivePrecondition(ctx, in, path, exclude); err != nil { return err } @@ -134,8 +134,8 @@ func validateGetArchiveRequest(in *gitalypb.GetArchiveRequest, format string, pa return nil } -func validateGetArchivePrecondition(ctx context.Context, in *gitalypb.GetArchiveRequest, path string, exclude []string) error { - c, err := catfile.New(ctx, in.GetRepository()) +func (s *server) validateGetArchivePrecondition(ctx context.Context, in *gitalypb.GetArchiveRequest, path string, exclude []string) error { + c, err := catfile.New(ctx, s.locator, in.GetRepository()) if err != nil { return err } diff --git a/internal/gitaly/service/repository/create_from_bundle_test.go b/internal/gitaly/service/repository/create_from_bundle_test.go index 4ad385ac454..68263b6a041 100644 --- a/internal/gitaly/service/repository/create_from_bundle_test.go +++ b/internal/gitaly/service/repository/create_from_bundle_test.go @@ -81,7 +81,7 @@ func TestServer_CreateRepositoryFromBundle_successful(t *testing.T) { require.NoError(t, err) require.NotEqual(t, 0, info.Mode()&os.ModeSymlink) - commit, err := log.GetCommit(ctx, importedRepo, "refs/custom-refs/ref1") + commit, err := log.GetCommit(ctx, locator, importedRepo, "refs/custom-refs/ref1") require.NoError(t, err) require.NotNil(t, commit) } diff --git a/internal/gitaly/service/repository/fetch_test.go b/internal/gitaly/service/repository/fetch_test.go index 296172074f3..e7176aa0305 100644 --- a/internal/gitaly/service/repository/fetch_test.go +++ b/internal/gitaly/service/repository/fetch_test.go @@ -75,7 +75,7 @@ func TestFetchSourceBranchSourceRepositorySuccess(t *testing.T) { require.NoError(t, err) require.True(t, resp.Result, "response.Result should be true") - fetchedCommit, err := gitLog.GetCommit(ctx, targetRepo, targetRef) + fetchedCommit, err := gitLog.GetCommit(ctx, locator, targetRepo, targetRef) require.NoError(t, err) require.Equal(t, newCommitID, fetchedCommit.GetId()) }) @@ -132,7 +132,7 @@ func TestFetchSourceBranchSameRepositorySuccess(t *testing.T) { require.NoError(t, err) require.True(t, resp.Result, "response.Result should be true") - fetchedCommit, err := gitLog.GetCommit(ctx, repo, targetRef) + fetchedCommit, err := gitLog.GetCommit(ctx, locator, repo, targetRef) require.NoError(t, err) require.Equal(t, newCommitID, fetchedCommit.GetId()) }) diff --git a/internal/gitaly/service/repository/gc.go b/internal/gitaly/service/repository/gc.go index 28528e9b11c..e7e6f7a3b8b 100644 --- a/internal/gitaly/service/repository/gc.go +++ b/internal/gitaly/service/repository/gc.go @@ -120,7 +120,7 @@ func (s *server) cleanupKeepArounds(ctx context.Context, repo *gitalypb.Reposito return nil } - batch, err := catfile.New(ctx, repo) + batch, err := catfile.New(ctx, s.locator, repo) if err != nil { return nil } diff --git a/internal/gitaly/service/repository/raw_changes.go b/internal/gitaly/service/repository/raw_changes.go index f4564564f51..967ede2a4f6 100644 --- a/internal/gitaly/service/repository/raw_changes.go +++ b/internal/gitaly/service/repository/raw_changes.go @@ -21,7 +21,7 @@ func (s *server) GetRawChanges(req *gitalypb.GetRawChangesRequest, stream gitaly ctx := stream.Context() repo := req.Repository - batch, err := catfile.New(stream.Context(), repo) + batch, err := catfile.New(stream.Context(), s.locator, repo) if err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/repository/snapshot_test.go b/internal/gitaly/service/repository/snapshot_test.go index 3b9bdce4038..592ebe9d902 100644 --- a/internal/gitaly/service/repository/snapshot_test.go +++ b/internal/gitaly/service/repository/snapshot_test.go @@ -134,14 +134,14 @@ func TestGetSnapshotWithDedupe(t *testing.T) { commitSha := testhelper.CreateCommitInAlternateObjectDirectory(t, repoPath, alternateObjDir, cmd) originalAlternatesCommit := string(commitSha) + locator := config.NewLocator(config.Config) + // ensure commit cannot be found in current repository - c, err := catfile.New(ctx, testRepo) + c, err := catfile.New(ctx, locator, testRepo) require.NoError(t, err) _, err = c.Info(ctx, originalAlternatesCommit) require.True(t, catfile.IsNotFound(err)) - locator := config.NewLocator(config.Config) - // write alternates file to point to alt objects folder alternatesPath, err := locator.InfoAlternatesPath(testRepo) require.NoError(t, err) @@ -154,7 +154,7 @@ func TestGetSnapshotWithDedupe(t *testing.T) { "commit", "--allow-empty", "-m", "Another empty commit") commitSha = testhelper.CreateCommitInAlternateObjectDirectory(t, repoPath, alternateObjDir, cmd) - c, err = catfile.New(ctx, testRepo) + c, err = catfile.New(ctx, locator, testRepo) require.NoError(t, err) _, err = c.Info(ctx, string(commitSha)) require.NoError(t, err) diff --git a/internal/gitaly/service/wiki/delete_page_test.go b/internal/gitaly/service/wiki/delete_page_test.go index 084812daf24..07610988fcc 100644 --- a/internal/gitaly/service/wiki/delete_page_test.go +++ b/internal/gitaly/service/wiki/delete_page_test.go @@ -73,7 +73,7 @@ func TestSuccessfulWikiDeletePageRequest(t *testing.T) { require.NoError(t, err) headID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") - commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID)) + commit, err := gitlog.GetCommit(ctx, locator, wikiRepo, string(headID)) require.NoError(t, err, "look up git commit after deleting a wiki page") require.Equal(t, authorName, commit.Author.Name, "author name mismatched") diff --git a/internal/gitaly/service/wiki/testhelper_test.go b/internal/gitaly/service/wiki/testhelper_test.go index f79dfa578d3..d469e2674a6 100644 --- a/internal/gitaly/service/wiki/testhelper_test.go +++ b/internal/gitaly/service/wiki/testhelper_test.go @@ -197,7 +197,7 @@ func createTestWikiPage(t *testing.T, locator storage.Locator, client gitalypb.W require.NoError(t, err) writeWikiPage(t, client, wikiRepo, opts) head1ID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") - pageCommit, err := gitlog.GetCommit(ctx, wikiRepo, string(head1ID)) + pageCommit, err := gitlog.GetCommit(ctx, locator, wikiRepo, string(head1ID)) require.NoError(t, err, "look up git commit after writing a wiki page") return pageCommit diff --git a/internal/gitaly/service/wiki/update_page_test.go b/internal/gitaly/service/wiki/update_page_test.go index 88d2ade88dc..3565b02fd87 100644 --- a/internal/gitaly/service/wiki/update_page_test.go +++ b/internal/gitaly/service/wiki/update_page_test.go @@ -99,7 +99,7 @@ func TestSuccessfulWikiUpdatePageRequest(t *testing.T) { require.NoError(t, err) headID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") - commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID)) + commit, err := gitlog.GetCommit(ctx, locator, wikiRepo, string(headID)) require.NoError(t, err, "look up git commit before merge is applied") require.Equal(t, authorName, commit.Author.Name, "author name mismatched") diff --git a/internal/gitaly/service/wiki/write_page_test.go b/internal/gitaly/service/wiki/write_page_test.go index be0e3579eef..6a6f4c6fd95 100644 --- a/internal/gitaly/service/wiki/write_page_test.go +++ b/internal/gitaly/service/wiki/write_page_test.go @@ -102,7 +102,7 @@ func TestSuccessfulWikiWritePageRequest(t *testing.T) { require.Empty(t, resp.DuplicateError, "DuplicateError must be empty") headID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD") - commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID)) + commit, err := gitlog.GetCommit(ctx, locator, wikiRepo, string(headID)) require.NoError(t, err, "look up git commit after writing a wiki page") require.Equal(t, authorName, commit.Author.Name, "author name mismatched") -- GitLab From 1f73d6fd3796378e16c25dddd6a12536d7f4cf14 Mon Sep 17 00:00:00 2001 From: Pavlo Strokov Date: Tue, 15 Dec 2020 19:24:04 +0200 Subject: [PATCH 2/3] Removal of helper.GetRepoPath helper.GetRepoPath is not used anymore in the production code so we remove it and all outdated dependencies of it. Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699 --- internal/git/alternates/alternates.go | 19 ---- internal/helper/repo.go | 22 ---- internal/helper/repo_test.go | 156 -------------------------- 3 files changed, 197 deletions(-) diff --git a/internal/git/alternates/alternates.go b/internal/git/alternates/alternates.go index 60636da83e8..f74180a6de2 100644 --- a/internal/git/alternates/alternates.go +++ b/internal/git/alternates/alternates.go @@ -4,27 +4,8 @@ import ( "fmt" "path/filepath" "strings" - - "gitlab.com/gitlab-org/gitaly/internal/git/repository" - "gitlab.com/gitlab-org/gitaly/internal/helper" ) -// PathAndEnv finds the disk path to a repository, and returns the -// alternate object directory environment variables encoded in the -// gitalypb.Repository instance. -// Deprecated: please use storage.Locator to define the project path and alternates.Env -// to get alternate object directory environment variables. -func PathAndEnv(repo repository.GitRepo) (string, []string, error) { - repoPath, err := helper.GetRepoPath(repo) - if err != nil { - return "", nil, err - } - - env := Env(repoPath, repo.GetGitObjectDirectory(), repo.GetGitAlternateObjectDirectories()) - - return repoPath, env, nil -} - // Env returns the alternate object directory environment variables. func Env(repoPath, objectDirectory string, alternateObjectDirectories []string) []string { var env []string diff --git a/internal/helper/repo.go b/internal/helper/repo.go index 2d61a73eca4..f65f5c996a7 100644 --- a/internal/helper/repo.go +++ b/internal/helper/repo.go @@ -12,28 +12,6 @@ import ( "google.golang.org/grpc/status" ) -// GetRepoPath returns the full path of the repository referenced by an -// RPC Repository message. The errors returned are gRPC errors with -// relevant error codes and should be passed back to gRPC without further -// decoration. -// Deprecated: please use storage.Locator to define the project path. -func GetRepoPath(repo repository.GitRepo) (string, error) { - repoPath, err := GetPath(repo) - if err != nil { - return "", err - } - - if repoPath == "" { - return "", status.Errorf(codes.InvalidArgument, "GetRepoPath: empty repo") - } - - if storage.IsGitDirectory(repoPath) { - return repoPath, nil - } - - return "", status.Errorf(codes.NotFound, "GetRepoPath: not a git repository '%s'", repoPath) -} - // RepoPathEqual compares if two repositories are in the same location func RepoPathEqual(a, b repository.GitRepo) bool { return a.GetStorageName() == b.GetStorageName() && diff --git a/internal/helper/repo_test.go b/internal/helper/repo_test.go index ba8648ee3a7..8f4b3d8b694 100644 --- a/internal/helper/repo_test.go +++ b/internal/helper/repo_test.go @@ -2,14 +2,11 @@ package helper import ( "os" - "path/filepath" "testing" "github.com/stretchr/testify/assert" - "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" - "google.golang.org/grpc/codes" ) func TestMain(m *testing.M) { @@ -73,156 +70,3 @@ func TestRepoPathEqual(t *testing.T) { }) } } - -func TestGetRepoPath(t *testing.T) { - defer func(oldStorages []config.Storage) { - config.Config.Storages = oldStorages - }(config.Config.Storages) - - testRepo, _, cleanup := testhelper.NewTestRepo(t) - defer cleanup() - - repoPath, err := GetRepoPath(testRepo) - if err != nil { - t.Fatal(err) - } - - exampleStorages := []config.Storage{ - {Name: "default", Path: testhelper.GitlabTestStoragePath()}, - {Name: "other", Path: "/home/git/repositories2"}, - {Name: "third", Path: "/home/git/repositories3"}, - } - - testCases := []struct { - desc string - storages []config.Storage - repo *gitalypb.Repository - path string - err codes.Code - }{ - { - desc: "storages configured", - storages: exampleStorages, - repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: testRepo.GetRelativePath()}, - path: repoPath, - }, - { - desc: "no storage config, storage name provided", - repo: &gitalypb.Repository{StorageName: "does not exist", RelativePath: testRepo.GetRelativePath()}, - err: codes.InvalidArgument, - }, - { - desc: "no storage config, nil repo", - err: codes.InvalidArgument, - }, - { - desc: "storage config provided, empty repo", - storages: exampleStorages, - repo: &gitalypb.Repository{}, - err: codes.InvalidArgument, - }, - { - desc: "no storage config, empty repo", - repo: &gitalypb.Repository{}, - err: codes.InvalidArgument, - }, - { - desc: "non existing repo", - storages: exampleStorages, - repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "made/up/path.git"}, - err: codes.NotFound, - }, - { - desc: "non existing storage", - storages: exampleStorages, - repo: &gitalypb.Repository{StorageName: "does not exists", RelativePath: testRepo.GetRelativePath()}, - err: codes.InvalidArgument, - }, - { - desc: "storage defined but storage dir does not exist", - storages: []config.Storage{{Name: testRepo.GetStorageName(), Path: "/does/not/exist"}}, - repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "foobar.git"}, - err: codes.Internal, - }, - { - desc: "relative path with directory traversal", - storages: exampleStorages, - repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "../bazqux.git"}, - err: codes.InvalidArgument, - }, - { - desc: "valid path with ..", - storages: exampleStorages, - repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "foo../bazqux.git"}, - err: codes.NotFound, // Because the directory doesn't exist - }, - { - desc: "relative path with sneaky directory traversal", - storages: exampleStorages, - repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "/../bazqux.git"}, - err: codes.InvalidArgument, - }, - { - desc: "relative path with traversal outside storage", - storages: exampleStorages, - repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: testRepo.GetRelativePath() + "/../../../../.."}, - err: codes.InvalidArgument, - }, - { - desc: "relative path with traversal outside storage with trailing slash", - storages: exampleStorages, - repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: testRepo.GetRelativePath() + "/../../../../../"}, - err: codes.InvalidArgument, - }, - { - desc: "relative path with deep traversal at the end", - storages: exampleStorages, - repo: &gitalypb.Repository{StorageName: testRepo.GetStorageName(), RelativePath: "bazqux.git/../.."}, - err: codes.InvalidArgument, - }, - } - - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - config.Config.Storages = tc.storages - path, err := GetRepoPath(tc.repo) - - if tc.err != codes.OK { - testhelper.RequireGrpcError(t, err, tc.err) - return - } - - if err != nil { - assert.NoError(t, err) - return - } - - assert.Equal(t, tc.path, path) - }) - } -} - -func assertInvalidRepoWithoutFile(t *testing.T, repo *gitalypb.Repository, repoPath, file string) { - oldRoute := filepath.Join(repoPath, file) - renamedRoute := filepath.Join(repoPath, file+"moved") - os.Rename(oldRoute, renamedRoute) - defer func() { - os.Rename(renamedRoute, oldRoute) - }() - - _, err := GetRepoPath(repo) - - testhelper.RequireGrpcError(t, err, codes.NotFound) -} - -func TestGetRepoPathWithCorruptedRepo(t *testing.T) { - testRepo, _, cleanup := testhelper.NewTestRepo(t) - defer cleanup() - - testRepoStoragePath := testhelper.GitlabTestStoragePath() - testRepoPath := filepath.Join(testRepoStoragePath, testRepo.RelativePath) - - for _, file := range []string{"objects", "refs", "HEAD"} { - assertInvalidRepoWithoutFile(t, testRepo, testRepoPath, file) - } -} -- GitLab From 3e7864d5fa4f44053b07f6612ae3c4c5712d80c7 Mon Sep 17 00:00:00 2001 From: Pavlo Strokov Date: Tue, 15 Dec 2020 20:18:50 +0200 Subject: [PATCH 3/3] Removal of helper.GetPath helper.GetPath is not used anymore in the production code so we remove it and refactor all usages of it. Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699 --- .../remote/fetch_internal_remote_test.go | 11 +++---- internal/helper/repo.go | 30 ------------------- internal/praefect/info_service_test.go | 8 +++-- internal/praefect/server_test.go | 19 +++++++----- internal/tempdir/tempdir_test.go | 6 ++-- 5 files changed, 26 insertions(+), 48 deletions(-) diff --git a/internal/gitaly/service/remote/fetch_internal_remote_test.go b/internal/gitaly/service/remote/fetch_internal_remote_test.go index bd18b512ff1..274047d2746 100644 --- a/internal/gitaly/service/remote/fetch_internal_remote_test.go +++ b/internal/gitaly/service/remote/fetch_internal_remote_test.go @@ -13,6 +13,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/remote" "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/ssh" "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -59,12 +60,12 @@ func TestSuccessfulFetchInternalRemote(t *testing.T) { repo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - gitaly0Repo, gitaly0RepoPath, cleanup := cloneRepoAtStorage(t, repo, "gitaly-0") + gitaly0Repo, gitaly0RepoPath, cleanup := cloneRepoAtStorage(t, locator, repo, "gitaly-0") defer cleanup() testhelper.CreateCommit(t, gitaly0RepoPath, "master", nil) - gitaly1Repo, gitaly1RepoPath, cleanup := cloneRepoAtStorage(t, repo, "gitaly-1") + gitaly1Repo, gitaly1RepoPath, cleanup := cloneRepoAtStorage(t, locator, repo, "gitaly-1") defer cleanup() testhelper.MustRunCommand(t, nil, "git", "-C", gitaly1RepoPath, "symbolic-ref", "HEAD", "refs/heads/feature") @@ -187,14 +188,14 @@ func runFullServer(t *testing.T) (string, func()) { } } -func cloneRepoAtStorage(t testing.TB, src *gitalypb.Repository, storageName string) (*gitalypb.Repository, string, func()) { +func cloneRepoAtStorage(t testing.TB, locator storage.Locator, src *gitalypb.Repository, storageName string) (*gitalypb.Repository, string, func()) { dst := *src dst.StorageName = storageName - dstP, err := helper.GetPath(&dst) + dstP, err := locator.GetPath(&dst) require.NoError(t, err) - srcP, err := helper.GetPath(src) + srcP, err := locator.GetPath(src) require.NoError(t, err) require.NoError(t, os.MkdirAll(dstP, 0755)) diff --git a/internal/helper/repo.go b/internal/helper/repo.go index f65f5c996a7..603c793198f 100644 --- a/internal/helper/repo.go +++ b/internal/helper/repo.go @@ -1,12 +1,8 @@ package helper import ( - "os" - "path/filepath" - "gitlab.com/gitlab-org/gitaly/internal/git/repository" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -18,32 +14,6 @@ func RepoPathEqual(a, b repository.GitRepo) bool { a.GetRelativePath() == b.GetRelativePath() } -// GetPath returns the path of the repo passed as first argument. An error is -// returned when either the storage can't be found or the path includes -// constructs trying to perform directory traversal. -func GetPath(repo repository.GitRepo) (string, error) { - storagePath, err := GetStorageByName(repo.GetStorageName()) - if err != nil { - return "", err - } - - if _, err := os.Stat(storagePath); err != nil { - return "", status.Errorf(codes.Internal, "GetPath: storage path: %v", err) - } - - relativePath := repo.GetRelativePath() - if len(relativePath) == 0 { - err := status.Errorf(codes.InvalidArgument, "GetPath: relative path missing from %+v", repo) - return "", err - } - - if _, err := storage.ValidateRelativePath(storagePath, relativePath); err != nil { - return "", status.Errorf(codes.InvalidArgument, "GetRepoPath: %s", err) - } - - return filepath.Join(storagePath, relativePath), nil -} - // GetStorageByName will return the path for the storage, which is fetched by // its key. An error is return if it cannot be found. // Deprecated: please use storage.Locator to define the storage path. diff --git a/internal/praefect/info_service_test.go b/internal/praefect/info_service_test.go index 625e6428c3f..f2daeba58cb 100644 --- a/internal/praefect/info_service_test.go +++ b/internal/praefect/info_service_test.go @@ -50,15 +50,17 @@ func TestInfoService_RepositoryReplicas(t *testing.T) { testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() + locator := gconfig.NewLocator(gconfig.Config) + cc, _, cleanup := runPraefectServerWithGitaly(t, gconfig.Config, conf) defer cleanup() - testRepoPrimary, _, cleanup := cloneRepoAtStorage(t, testRepo, conf.VirtualStorages[0].Nodes[0].Storage) + testRepoPrimary, _, cleanup := cloneRepoAtStorage(t, locator, testRepo, conf.VirtualStorages[0].Nodes[0].Storage) defer cleanup() - _, _, cleanup = cloneRepoAtStorage(t, testRepo, conf.VirtualStorages[0].Nodes[1].Storage) + _, _, cleanup = cloneRepoAtStorage(t, locator, testRepo, conf.VirtualStorages[0].Nodes[1].Storage) defer cleanup() - _, testRepoSecondary2Path, cleanup := cloneRepoAtStorage(t, testRepo, conf.VirtualStorages[0].Nodes[2].Storage) + _, testRepoSecondary2Path, cleanup := cloneRepoAtStorage(t, locator, testRepo, conf.VirtualStorages[0].Nodes[2].Storage) defer cleanup() // create a commit in the second replica so we can check that its checksum is different than the primary diff --git a/internal/praefect/server_test.go b/internal/praefect/server_test.go index c092b2d8394..4680a39e770 100644 --- a/internal/praefect/server_test.go +++ b/internal/praefect/server_test.go @@ -34,6 +34,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/praefect/nodes/tracker" "gitlab.com/gitlab-org/gitaly/internal/praefect/protoregistry" "gitlab.com/gitlab-org/gitaly/internal/praefect/transactions" + "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/promtest" "gitlab.com/gitlab-org/gitaly/internal/version" @@ -389,12 +390,14 @@ func TestRepoRemoval(t *testing.T) { } }() + locator := gconfig.NewLocator(gconfig.Config) + tRepo, _, tCleanup := testhelper.NewTestRepo(t) defer tCleanup() - _, path1, cleanup1 := cloneRepoAtStorage(t, tRepo, conf.VirtualStorages[0].Nodes[1].Storage) + _, path1, cleanup1 := cloneRepoAtStorage(t, locator, tRepo, conf.VirtualStorages[0].Nodes[1].Storage) defer cleanup1() - _, path2, cleanup2 := cloneRepoAtStorage(t, tRepo, conf.VirtualStorages[0].Nodes[2].Storage) + _, path2, cleanup2 := cloneRepoAtStorage(t, locator, tRepo, conf.VirtualStorages[0].Nodes[2].Storage) defer cleanup2() // prerequisite: repos should exist at expected paths @@ -511,14 +514,16 @@ func TestRepoRename(t *testing.T) { require.Len(t, gconfig.Config.Storages, 3, "1 default storage and 2 replicas of it") + locator := gconfig.NewLocator(gconfig.Config) + // repo0 is a template that is used to create replica set by cloning it into other storage (directories) repo0, path0, cleanup0 := testhelper.NewTestRepo(t) defer cleanup0() - _, path1, cleanup1 := cloneRepoAtStorage(t, repo0, virtualStorage.Nodes[1].Storage) + _, path1, cleanup1 := cloneRepoAtStorage(t, locator, repo0, virtualStorage.Nodes[1].Storage) defer cleanup1() - _, path2, cleanup2 := cloneRepoAtStorage(t, repo0, virtualStorage.Nodes[2].Storage) + _, path2, cleanup2 := cloneRepoAtStorage(t, locator, repo0, virtualStorage.Nodes[2].Storage) defer cleanup2() var canCheckRepo sync.WaitGroup @@ -1004,14 +1009,14 @@ func tempStoragePath(t testing.TB) string { return p } -func cloneRepoAtStorage(t testing.TB, src *gitalypb.Repository, storageName string) (*gitalypb.Repository, string, func()) { +func cloneRepoAtStorage(t testing.TB, locator storage.Locator, src *gitalypb.Repository, storageName string) (*gitalypb.Repository, string, func()) { dst := *src dst.StorageName = storageName - dstP, err := helper.GetPath(&dst) + dstP, err := locator.GetPath(&dst) require.NoError(t, err) - srcP, err := helper.GetPath(src) + srcP, err := locator.GetPath(src) require.NoError(t, err) require.NoError(t, os.MkdirAll(dstP, 0755)) diff --git a/internal/tempdir/tempdir_test.go b/internal/tempdir/tempdir_test.go index 9ee5f4bd4e1..c53958fded3 100644 --- a/internal/tempdir/tempdir_test.go +++ b/internal/tempdir/tempdir_test.go @@ -10,7 +10,6 @@ import ( "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" "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/proto/go/gitalypb" ) @@ -22,13 +21,14 @@ func TestNewAsRepositorySuccess(t *testing.T) { repo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - tempRepo, tempDir, err := NewAsRepository(ctx, repo, config.NewLocator(config.Config)) + locator := config.NewLocator(config.Config) + tempRepo, tempDir, err := NewAsRepository(ctx, repo, locator) require.NoError(t, err) require.NotEqual(t, repo, tempRepo) require.Equal(t, repo.StorageName, tempRepo.StorageName) require.NotEqual(t, repo.RelativePath, tempRepo.RelativePath) - calculatedPath, err := helper.GetPath(tempRepo) + calculatedPath, err := locator.GetPath(tempRepo) require.NoError(t, err) require.Equal(t, tempDir, calculatedPath) -- GitLab