diff options
| author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-12-15 13:10:53 +0300 |
|---|---|---|
| committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-12-20 09:46:26 +0300 |
| commit | 73493c529f1c6e45e865c71a107d2ce136819ee8 (patch) | |
| tree | 3a23fc86133da835b9df8ec2eb8d2010367e2ffb | |
| parent | c797df4901140c967ade2341ea7347a17b158a74 (diff) | |
git/objectpool: Resolve object pool paths when creating the structure
While we already try to resolve the storage path right now and thus fail
with an error in case the storage is not known, we don't do this for the
object pool path yet. While we check whether the object pool path is a
valid path at a later point anyway, we will require the path before
doing that check in a subsequent commit. As a consequence, the error
returned would fail in one of our tests.
Refactor the code to ask for the repository path early on so that we
don't need to change tests when introducing new logic. The new behaviour
could be claimed to be better than before as we now return a proper
error in case the path is misformed. But the previous behaviour was just
fine, too.
| -rw-r--r-- | internal/git/objectpool/pool.go | 6 | ||||
| -rw-r--r-- | internal/gitaly/service/objectpool/delete_test.go | 6 |
2 files changed, 7 insertions, 5 deletions
diff --git a/internal/git/objectpool/pool.go b/internal/git/objectpool/pool.go index f03d4139c..e558c1882 100644 --- a/internal/git/objectpool/pool.go +++ b/internal/git/objectpool/pool.go @@ -51,10 +51,8 @@ func FromProto( housekeepingManager housekeeping.Manager, proto *gitalypb.ObjectPool, ) (*ObjectPool, error) { - // TODO: this is retained for backwards compatibility for now. We should eventually amend - // `FromProto()` to always return an error if the Protobuf representation is invalid, the - // pool directory doesn't exist, or if the directory does not contain a valid repository. - if _, err := locator.GetStorageByName(proto.GetRepository().GetStorageName()); err != nil { + _, err := locator.GetPath(proto.GetRepository()) + if err != nil { return nil, err } diff --git a/internal/gitaly/service/objectpool/delete_test.go b/internal/gitaly/service/objectpool/delete_test.go index 2ac01f211..de6283bd5 100644 --- a/internal/gitaly/service/objectpool/delete_test.go +++ b/internal/gitaly/service/objectpool/delete_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" + "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -72,7 +73,10 @@ func TestDelete(t *testing.T) { { desc: "path traversing fails", relativePath: validPoolPath + "/../../../../..", - expectedErr: errInvalidPoolDir, + expectedErr: testhelper.GitalyOrPraefect( + structerr.NewInvalidArgument("GetRepoPath: %w", storage.ErrRelativePathEscapesRoot), + errInvalidPoolDir, + ), }, { desc: "deleting pool succeeds", |