diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-12-20 09:43:10 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-12-20 09:43:10 +0300 |
commit | 39dba873cac5ee3fbf51ebade7e71f3edef00efd (patch) | |
tree | 73246aed3895c1ec40e9a2e45cf02ab9b6c05ac3 | |
parent | 93bf486f9e7c9afdc19de19763a83783d3742c33 (diff) | |
parent | ffb1e8070908bd410613619559f38e0ad39ecc9e (diff) |
Merge branch 'pks-helper-errorf-conversion-pt3' into 'master'
helper: Convert `ErrNotFoundf()` and `ErrInvalidArgumentf()`
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5194
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
167 files changed, 489 insertions, 855 deletions
diff --git a/internal/git/command_options.go b/internal/git/command_options.go index 546ab7a8c..b5f9e0010 100644 --- a/internal/git/command_options.go +++ b/internal/git/command_options.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/command" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/x509" @@ -279,11 +278,11 @@ func withInternalFetch(req repoScopedRequest, withSidechannel bool) func(ctx con storageInfo, ok := serversInfo[req.GetRepository().GetStorageName()] if !ok { - return helper.ErrInvalidArgumentf("no storage info for %q", req.GetRepository().GetStorageName()) + return structerr.NewInvalidArgument("no storage info for %q", req.GetRepository().GetStorageName()) } if storageInfo.Address == "" { - return helper.ErrInvalidArgumentf("empty Gitaly address") + return structerr.NewInvalidArgument("empty Gitaly address") } var flagsWithValue []string diff --git a/internal/git/localrepo/paths.go b/internal/git/localrepo/paths.go index 65d0799f6..daaf48387 100644 --- a/internal/git/localrepo/paths.go +++ b/internal/git/localrepo/paths.go @@ -6,7 +6,7 @@ import ( "strings" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" ) // Path returns the on-disk path of the repository. @@ -24,7 +24,7 @@ func (repo *Repo) ObjectDirectoryPath() (string, error) { objectDirectoryPath := repo.GetGitObjectDirectory() if objectDirectoryPath == "" { - return "", helper.ErrInvalidArgumentf("object directory path is not set") + return "", structerr.NewInvalidArgument("object directory path is not set") } // We need to check whether the relative object directory as given by the repository is @@ -37,20 +37,20 @@ func (repo *Repo) ObjectDirectoryPath() (string, error) { if _, origError := storage.ValidateRelativePath(repoPath, objectDirectoryPath); origError != nil { tempDir, err := repo.locator.TempDir(repo.GetStorageName()) if err != nil { - return "", helper.ErrInvalidArgumentf("getting storage's temporary directory: %s", err) + return "", structerr.NewInvalidArgument("getting storage's temporary directory: %s", err) } expectedQuarantinePrefix := filepath.Join(tempDir, storage.QuarantineDirectoryPrefix(repo)) absoluteObjectDirectoryPath := filepath.Join(repoPath, objectDirectoryPath) if !strings.HasPrefix(absoluteObjectDirectoryPath, expectedQuarantinePrefix) { - return "", helper.ErrInvalidArgumentf("not a valid relative path: %s", origError) + return "", structerr.NewInvalidArgument("not a valid relative path: %s", origError) } } fullPath := filepath.Join(repoPath, objectDirectoryPath) if _, err := os.Stat(fullPath); os.IsNotExist(err) { - return "", helper.ErrNotFoundf("object directory does not exist: %q", fullPath) + return "", structerr.NewNotFound("object directory does not exist: %q", fullPath) } return fullPath, nil diff --git a/internal/git/objectpool/fetch.go b/internal/git/objectpool/fetch.go index 23f3d02d1..35034dacf 100644 --- a/internal/git/objectpool/fetch.go +++ b/internal/git/objectpool/fetch.go @@ -15,7 +15,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" ) @@ -26,7 +26,7 @@ func (o *ObjectPool) FetchFromOrigin(ctx context.Context, origin *localrepo.Repo logger := ctxlogrus.Extract(ctx) if !o.Exists() { - return helper.ErrInvalidArgumentf("object pool does not exist") + return structerr.NewInvalidArgument("object pool does not exist") } originPath, err := origin.Path() diff --git a/internal/git/objectpool/fetch_test.go b/internal/git/objectpool/fetch_test.go index 5e917bcee..d702b5bc2 100644 --- a/internal/git/objectpool/fetch_test.go +++ b/internal/git/objectpool/fetch_test.go @@ -15,8 +15,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" ) @@ -239,7 +239,7 @@ func TestFetchFromOrigin_missingPool(t *testing.T) { // object pool. require.NoError(t, pool.Remove(ctx)) - require.Equal(t, helper.ErrInvalidArgumentf("object pool does not exist"), pool.FetchFromOrigin(ctx, repo)) + require.Equal(t, structerr.NewInvalidArgument("object pool does not exist"), pool.FetchFromOrigin(ctx, repo)) require.False(t, pool.Exists()) } diff --git a/internal/git/objectpool/pool_test.go b/internal/git/objectpool/pool_test.go index e82e15697..0f2c9a734 100644 --- a/internal/git/objectpool/pool_test.go +++ b/internal/git/objectpool/pool_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -48,7 +48,7 @@ func TestFromProto(t *testing.T) { RelativePath: gittest.NewObjectPoolName(t), }, }) - require.Equal(t, helper.ErrInvalidArgumentf("GetStorageByName: no such storage: %q", "mepmep"), err) + require.Equal(t, structerr.NewInvalidArgument("GetStorageByName: no such storage: %q", "mepmep"), err) }) } diff --git a/internal/gitaly/config/locator.go b/internal/gitaly/config/locator.go index 48343167c..89de12990 100644 --- a/internal/gitaly/config/locator.go +++ b/internal/gitaly/config/locator.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/repository" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" ) @@ -49,7 +48,7 @@ func (l *configLocator) GetRepoPath(repo repository.GitRepo) (string, error) { return repoPath, nil } - return "", helper.ErrNotFoundf("GetRepoPath: not a git repository: %q", repoPath) + return "", structerr.NewNotFound("GetRepoPath: not a git repository: %q", repoPath) } // GetPath returns the path of the repo passed as first argument. An error is @@ -63,19 +62,19 @@ func (l *configLocator) GetPath(repo repository.GitRepo) (string, error) { if _, err := os.Stat(storagePath); err != nil { if os.IsNotExist(err) { - return "", helper.ErrNotFoundf("GetPath: does not exist: %v", err) + return "", structerr.NewNotFound("GetPath: does not exist: %v", err) } return "", structerr.NewInternal("GetPath: storage path: %v", err) } relativePath := repo.GetRelativePath() if len(relativePath) == 0 { - err := helper.ErrInvalidArgumentf("GetPath: relative path missing") + err := structerr.NewInvalidArgument("GetPath: relative path missing") return "", err } if _, err := storage.ValidateRelativePath(storagePath, relativePath); err != nil { - return "", helper.ErrInvalidArgumentf("GetRepoPath: %s", err) + return "", structerr.NewInvalidArgument("GetRepoPath: %s", err) } return filepath.Join(storagePath, relativePath), nil @@ -86,7 +85,7 @@ func (l *configLocator) GetPath(repo repository.GitRepo) (string, error) { func (l *configLocator) GetStorageByName(storageName string) (string, error) { storagePath, ok := l.conf.StoragePath(storageName) if !ok { - return "", helper.ErrInvalidArgumentf("GetStorageByName: no such storage: %q", storageName) + return "", structerr.NewInvalidArgument("GetStorageByName: no such storage: %q", storageName) } return storagePath, nil @@ -110,7 +109,7 @@ func (l *configLocator) TempDir(storageName string) (string, error) { func (l *configLocator) getPath(storageName, prefix string) (string, error) { storagePath, ok := l.conf.StoragePath(storageName) if !ok { - return "", helper.ErrInvalidArgumentf("%s dir: no such storage: %q", + return "", structerr.NewInvalidArgument("%s dir: no such storage: %q", filepath.Base(prefix), storageName) } diff --git a/internal/gitaly/config/locator_test.go b/internal/gitaly/config/locator_test.go index 4260cda54..e647fc822 100644 --- a/internal/gitaly/config/locator_test.go +++ b/internal/gitaly/config/locator_test.go @@ -12,7 +12,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/setup" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver" @@ -52,37 +52,37 @@ func TestConfigLocator_GetRepoPath(t *testing.T) { { desc: "storage is empty", repo: &gitalypb.Repository{RelativePath: repo.RelativePath}, - expErr: helper.ErrInvalidArgumentf(`GetStorageByName: no such storage: ""`), + expErr: structerr.NewInvalidArgument(`GetStorageByName: no such storage: ""`), }, { desc: "unknown storage", repo: &gitalypb.Repository{StorageName: "invalid", RelativePath: repo.RelativePath}, - expErr: helper.ErrInvalidArgumentf(`GetStorageByName: no such storage: "invalid"`), + expErr: structerr.NewInvalidArgument(`GetStorageByName: no such storage: "invalid"`), }, { desc: "storage doesn't exist on disk", repo: &gitalypb.Repository{StorageName: cfg.Storages[1].Name, RelativePath: repo.RelativePath}, - expErr: helper.ErrNotFoundf(`GetPath: does not exist: stat %s: no such file or directory`, cfg.Storages[1].Path), + expErr: structerr.NewNotFound(`GetPath: does not exist: stat %s: no such file or directory`, cfg.Storages[1].Path), }, { desc: "relative path is empty", repo: &gitalypb.Repository{StorageName: storageName, RelativePath: ""}, - expErr: helper.ErrInvalidArgumentf("GetPath: relative path missing"), + expErr: structerr.NewInvalidArgument("GetPath: relative path missing"), }, { desc: "unknown relative path", repo: &gitalypb.Repository{StorageName: storageName, RelativePath: "invalid"}, - expErr: helper.ErrNotFoundf(`GetRepoPath: not a git repository: %q`, filepath.Join(cfg.Storages[0].Path, "invalid")), + expErr: structerr.NewNotFound(`GetRepoPath: not a git repository: %q`, filepath.Join(cfg.Storages[0].Path, "invalid")), }, { desc: "path exists but not a git repository", repo: &gitalypb.Repository{StorageName: storageName, RelativePath: notRepositoryFolder}, - expErr: helper.ErrNotFoundf(`GetRepoPath: not a git repository: %q`, filepath.Join(cfg.Storages[0].Path, notRepositoryFolder)), + expErr: structerr.NewNotFound(`GetRepoPath: not a git repository: %q`, filepath.Join(cfg.Storages[0].Path, notRepositoryFolder)), }, { desc: "relative path escapes parent folder", repo: &gitalypb.Repository{StorageName: storageName, RelativePath: "../.."}, - expErr: helper.ErrInvalidArgumentf(`GetRepoPath: relative path escapes root directory`), + expErr: structerr.NewInvalidArgument(`GetRepoPath: relative path escapes root directory`), }, } { t.Run(tc.desc, func(t *testing.T) { @@ -150,27 +150,27 @@ func TestConfigLocator_GetPath(t *testing.T) { { desc: "storage is empty", repo: &gitalypb.Repository{RelativePath: repo.RelativePath}, - expErr: helper.ErrInvalidArgumentf(`GetStorageByName: no such storage: ""`), + expErr: structerr.NewInvalidArgument(`GetStorageByName: no such storage: ""`), }, { desc: "unknown storage", repo: &gitalypb.Repository{StorageName: "invalid", RelativePath: repo.RelativePath}, - expErr: helper.ErrInvalidArgumentf(`GetStorageByName: no such storage: "invalid"`), + expErr: structerr.NewInvalidArgument(`GetStorageByName: no such storage: "invalid"`), }, { desc: "storage doesn't exist on disk", repo: &gitalypb.Repository{StorageName: cfg.Storages[1].Name, RelativePath: repo.RelativePath}, - expErr: helper.ErrNotFoundf(`GetPath: does not exist: stat %s: no such file or directory`, cfg.Storages[1].Path), + expErr: structerr.NewNotFound(`GetPath: does not exist: stat %s: no such file or directory`, cfg.Storages[1].Path), }, { desc: "relative path is empty", repo: &gitalypb.Repository{StorageName: storageName, RelativePath: ""}, - expErr: helper.ErrInvalidArgumentf("GetPath: relative path missing"), + expErr: structerr.NewInvalidArgument("GetPath: relative path missing"), }, { desc: "relative path escapes parent folder", repo: &gitalypb.Repository{StorageName: storageName, RelativePath: "../.."}, - expErr: helper.ErrInvalidArgumentf(`GetRepoPath: relative path escapes root directory`), + expErr: structerr.NewInvalidArgument(`GetRepoPath: relative path escapes root directory`), }, } { t.Run(tc.desc, func(t *testing.T) { @@ -202,7 +202,7 @@ func TestConfigLocator_CacheDir(t *testing.T) { t.Run("unknown storage", func(t *testing.T) { _, err := locator.CacheDir("unknown") - require.Equal(t, helper.ErrInvalidArgumentf(`cache dir: no such storage: "unknown"`), err) + require.Equal(t, structerr.NewInvalidArgument(`cache dir: no such storage: "unknown"`), err) }) } @@ -227,7 +227,7 @@ func TestConfigLocator_StateDir(t *testing.T) { t.Run("unknown storage", func(t *testing.T) { _, err := locator.StateDir("unknown") - require.Equal(t, helper.ErrInvalidArgumentf(`state dir: no such storage: "unknown"`), err) + require.Equal(t, structerr.NewInvalidArgument(`state dir: no such storage: "unknown"`), err) }) } @@ -252,6 +252,6 @@ func TestConfigLocator_TempDir(t *testing.T) { t.Run("unknown storage", func(t *testing.T) { _, err := locator.TempDir("unknown") - require.Equal(t, helper.ErrInvalidArgumentf(`tmp dir: no such storage: "unknown"`), err) + require.Equal(t, structerr.NewInvalidArgument(`tmp dir: no such storage: "unknown"`), err) }) } diff --git a/internal/gitaly/service/blob/blobs.go b/internal/gitaly/service/blob/blobs.go index 73d36be73..2eefea64c 100644 --- a/internal/gitaly/service/blob/blobs.go +++ b/internal/gitaly/service/blob/blobs.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -38,7 +37,7 @@ func verifyListBlobsRequest(req *gitalypb.ListBlobsRequest) error { // revisions. func (s *server) ListBlobs(req *gitalypb.ListBlobsRequest, stream gitalypb.BlobService_ListBlobsServer) error { if err := verifyListBlobsRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } ctx := stream.Context() diff --git a/internal/gitaly/service/blob/get_blob.go b/internal/gitaly/service/blob/get_blob.go index ead9a700d..30b5635d5 100644 --- a/internal/gitaly/service/blob/get_blob.go +++ b/internal/gitaly/service/blob/get_blob.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -17,7 +16,7 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic ctx := stream.Context() if err := validateRequest(in); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(in.GetRepository()) diff --git a/internal/gitaly/service/blob/get_blob_test.go b/internal/gitaly/service/blob/get_blob_test.go index 181cb5b61..dc464100d 100644 --- a/internal/gitaly/service/blob/get_blob_test.go +++ b/internal/gitaly/service/blob/get_blob_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -158,7 +158,7 @@ func TestGetBlob_invalidRequest(t *testing.T) { Repository: nil, Oid: oid, }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -172,7 +172,7 @@ func TestGetBlob_invalidRequest(t *testing.T) { }, Oid: oid, }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( fmt.Sprintf("create object reader: GetStorageByName: no such storage: %q", "fake"), "repo scoped: invalid Repository", )), @@ -186,7 +186,7 @@ func TestGetBlob_invalidRequest(t *testing.T) { }, Oid: oid, }, - expectedErr: helper.ErrNotFoundf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewNotFound(testhelper.GitalyOrPraefect( fmt.Sprintf("create object reader: GetRepoPath: not a git repository: %q", filepath.Join(cfg.Storages[0].Path, "path")), fmt.Sprintf("accessor call: route repository accessor: consistent storages: repository %q/%q not found", cfg.Storages[0].Name, "path"), )), @@ -196,7 +196,7 @@ func TestGetBlob_invalidRequest(t *testing.T) { request: &gitalypb.GetBlobRequest{ Repository: repo, }, - expectedErr: helper.ErrInvalidArgumentf("empty Oid"), + expectedErr: structerr.NewInvalidArgument("empty Oid"), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/blob/get_blobs.go b/internal/gitaly/service/blob/get_blobs.go index cf51d9ae8..167ba899a 100644 --- a/internal/gitaly/service/blob/get_blobs.go +++ b/internal/gitaly/service/blob/get_blobs.go @@ -8,7 +8,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -156,7 +155,7 @@ func sendBlobTreeEntry( func (s *server) GetBlobs(req *gitalypb.GetBlobsRequest, stream gitalypb.BlobService_GetBlobsServer) error { if err := validateGetBlobsRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(req.GetRepository()) diff --git a/internal/gitaly/service/blob/lfs_pointers.go b/internal/gitaly/service/blob/lfs_pointers.go index a44523f45..6bade6fd6 100644 --- a/internal/gitaly/service/blob/lfs_pointers.go +++ b/internal/gitaly/service/blob/lfs_pointers.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -34,11 +33,11 @@ func (s *server) ListLFSPointers(in *gitalypb.ListLFSPointersRequest, stream git return err } if len(in.Revisions) == 0 { - return helper.ErrInvalidArgumentf("missing revisions") + return structerr.NewInvalidArgument("missing revisions") } for _, revision := range in.Revisions { if strings.HasPrefix(revision, "-") && revision != "--all" && revision != "--not" { - return helper.ErrInvalidArgumentf("invalid revision: %q", revision) + return structerr.NewInvalidArgument("invalid revision: %q", revision) } } @@ -83,7 +82,7 @@ func (s *server) ListAllLFSPointers(in *gitalypb.ListAllLFSPointersRequest, stre repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) @@ -127,7 +126,7 @@ func (s *server) GetLFSPointers(req *gitalypb.GetLFSPointersRequest, stream gita ctx := stream.Context() if err := validateGetLFSPointersRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(req.GetRepository()) 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 361503ad3..99236d312 100644 --- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go +++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/cleanup/internalrefs" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/cleanup/notifier" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -33,7 +32,7 @@ func (s *server) ApplyBfgObjectMapStream(server gitalypb.CleanupService_ApplyBfg } if err := validateFirstRequest(firstRequest); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } ctx := server.Context() @@ -56,7 +55,7 @@ func (s *server) ApplyBfgObjectMapStream(server gitalypb.CleanupService_ApplyBfg if err := cleaner.ApplyObjectMap(ctx, reader.streamReader()); err != nil { if invalidErr, ok := err.(internalrefs.ErrInvalidObjectMap); ok { - return helper.ErrInvalidArgumentf("%w", invalidErr) + return structerr.NewInvalidArgument("%w", invalidErr) } return structerr.NewInternal("%w", 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 da808ce3d..f8ab6c229 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 @@ -14,7 +14,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -127,7 +127,7 @@ func TestApplyBfgObjectMapStreamFailsOnInvalidInput(t *testing.T) { ObjectMap: []byte("invalid-data here as you can see"), }) require.Nil(t, response) - testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf("object map invalid at line 0"), err) + testhelper.RequireGrpcError(t, structerr.NewInvalidArgument("object map invalid at line 0"), err) }) t.Run("no repository provided", func(t *testing.T) { @@ -136,7 +136,7 @@ func TestApplyBfgObjectMapStreamFailsOnInvalidInput(t *testing.T) { ObjectMap: []byte("does not matter"), }) require.Nil(t, response) - testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + testhelper.RequireGrpcError(t, structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), err) diff --git a/internal/gitaly/service/commit/check_objects_exist.go b/internal/gitaly/service/commit/check_objects_exist.go index 584b4b31a..949efee10 100644 --- a/internal/gitaly/service/commit/check_objects_exist.go +++ b/internal/gitaly/service/commit/check_objects_exist.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -32,7 +31,7 @@ func (s *server) CheckObjectsExist( repository := request.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } objectInfoReader, cancel, err := s.catfileCache.ObjectInfoReader( diff --git a/internal/gitaly/service/commit/check_objects_exist_test.go b/internal/gitaly/service/commit/check_objects_exist_test.go index 91a142164..b98ef3d45 100644 --- a/internal/gitaly/service/commit/check_objects_exist_test.go +++ b/internal/gitaly/service/commit/check_objects_exist_test.go @@ -10,7 +10,7 @@ import ( "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -69,9 +69,9 @@ func TestCheckObjectsExist(t *testing.T) { }, expectedErr: func() error { if testhelper.IsPraefectEnabled() { - return helper.ErrInvalidArgumentf("repo scoped: empty Repository") + return structerr.NewInvalidArgument("repo scoped: empty Repository") } - return helper.ErrInvalidArgumentf("empty Repository") + return structerr.NewInvalidArgument("empty Repository") }(), }, { @@ -170,7 +170,7 @@ func TestCheckObjectsExist(t *testing.T) { }, }, }, - expectedErr: helper.ErrInvalidArgumentf("invalid revision: revision can't start with '-'"), + expectedErr: structerr.NewInvalidArgument("invalid revision: revision can't start with '-'"), expectedErrorMetadata: map[string]any{ "revision": "-not-a-rev", }, @@ -185,7 +185,7 @@ func TestCheckObjectsExist(t *testing.T) { }, }, }, - expectedErr: helper.ErrInvalidArgumentf("invalid revision: revision can't contain whitespace"), + expectedErr: structerr.NewInvalidArgument("invalid revision: revision can't contain whitespace"), expectedErrorMetadata: map[string]any{ "revision": fmt.Sprintf("%s\n%s", commitID1, commitID2), }, @@ -205,7 +205,7 @@ func TestCheckObjectsExist(t *testing.T) { }, }, }, - expectedErr: helper.ErrInvalidArgumentf("invalid revision: revision can't start with '-'"), + expectedErr: structerr.NewInvalidArgument("invalid revision: revision can't start with '-'"), expectedErrorMetadata: map[string]any{ "revision": "-not-a-rev", }, diff --git a/internal/gitaly/service/commit/commit_messages.go b/internal/gitaly/service/commit/commit_messages.go index be34c3a74..6d3207b59 100644 --- a/internal/gitaly/service/commit/commit_messages.go +++ b/internal/gitaly/service/commit/commit_messages.go @@ -8,7 +8,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -16,7 +15,7 @@ import ( func (s *server) GetCommitMessages(request *gitalypb.GetCommitMessagesRequest, stream gitalypb.CommitService_GetCommitMessagesServer) error { if err := validateGetCommitMessagesRequest(request); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.getAndStreamCommitMessages(request, stream); err != nil { return structerr.NewInternal("%w", err) diff --git a/internal/gitaly/service/commit/commit_signatures.go b/internal/gitaly/service/commit/commit_signatures.go index 585731bc0..caeb4033a 100644 --- a/internal/gitaly/service/commit/commit_signatures.go +++ b/internal/gitaly/service/commit/commit_signatures.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -20,7 +19,7 @@ var gpgSiganturePrefix = []byte("gpgsig") func (s *server) GetCommitSignatures(request *gitalypb.GetCommitSignaturesRequest, stream gitalypb.CommitService_GetCommitSignaturesServer) error { if err := validateGetCommitSignaturesRequest(request); err != nil { - return helper.ErrInvalidArgumentf("GetCommitSignatures: %w", err) + return structerr.NewInvalidArgument("GetCommitSignatures: %w", err) } return s.getCommitSignatures(request, stream) diff --git a/internal/gitaly/service/commit/commits_by_message.go b/internal/gitaly/service/commit/commits_by_message.go index 86a68d7af..52c3643f5 100644 --- a/internal/gitaly/service/commit/commits_by_message.go +++ b/internal/gitaly/service/commit/commits_by_message.go @@ -5,7 +5,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/protobuf/proto" @@ -27,7 +26,7 @@ func (sender *commitsByMessageSender) Send() error { func (s *server) CommitsByMessage(in *gitalypb.CommitsByMessageRequest, stream gitalypb.CommitService_CommitsByMessageServer) error { if err := validateCommitsByMessageRequest(in); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.commitsByMessage(in, stream); err != nil { diff --git a/internal/gitaly/service/commit/count_diverging_commits.go b/internal/gitaly/service/commit/count_diverging_commits.go index 256c6b57c..7c04ba235 100644 --- a/internal/gitaly/service/commit/count_diverging_commits.go +++ b/internal/gitaly/service/commit/count_diverging_commits.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -19,7 +18,7 @@ import ( // accurate because --max-count is applied before it does the rev walk. func (s *server) CountDivergingCommits(ctx context.Context, req *gitalypb.CountDivergingCommitsRequest) (*gitalypb.CountDivergingCommitsResponse, error) { if err := s.validateCountDivergingCommitsRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } from, to := string(req.GetFrom()), string(req.GetTo()) diff --git a/internal/gitaly/service/commit/filter_shas_with_signatures.go b/internal/gitaly/service/commit/filter_shas_with_signatures.go index 13022db2e..0056bb9cc 100644 --- a/internal/gitaly/service/commit/filter_shas_with_signatures.go +++ b/internal/gitaly/service/commit/filter_shas_with_signatures.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -19,7 +18,7 @@ func (s *server) FilterShasWithSignatures(bidi gitalypb.CommitService_FilterShas } if err = validateFirstFilterShasWithSignaturesRequest(firstRequest); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.filterShasWithSignatures(bidi, firstRequest); err != nil { diff --git a/internal/gitaly/service/commit/find_all_commits.go b/internal/gitaly/service/commit/find_all_commits.go index 994500355..efd2d5398 100644 --- a/internal/gitaly/service/commit/find_all_commits.go +++ b/internal/gitaly/service/commit/find_all_commits.go @@ -5,14 +5,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) FindAllCommits(in *gitalypb.FindAllCommitsRequest, stream gitalypb.CommitService_FindAllCommitsServer) error { if err := validateFindAllCommitsRequest(in); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } ctx := stream.Context() @@ -23,7 +22,7 @@ func (s *server) FindAllCommits(in *gitalypb.FindAllCommitsRequest, stream gital if len(in.GetRevision()) == 0 { refs, err := repo.GetReferences(ctx, "refs/heads") if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } for _, ref := range refs { diff --git a/internal/gitaly/service/commit/find_commit.go b/internal/gitaly/service/commit/find_commit.go index 98ae845d8..ac1c0d162 100644 --- a/internal/gitaly/service/commit/find_commit.go +++ b/internal/gitaly/service/commit/find_commit.go @@ -7,7 +7,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -23,7 +23,7 @@ func validateFindCommitRequest(in *gitalypb.FindCommitRequest) error { func (s *server) FindCommit(ctx context.Context, in *gitalypb.FindCommitRequest) (*gitalypb.FindCommitResponse, error) { if err := validateFindCommitRequest(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(in.GetRepository()) diff --git a/internal/gitaly/service/commit/find_commit_test.go b/internal/gitaly/service/commit/find_commit_test.go index d946325d9..497fc84a4 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/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" "google.golang.org/grpc/codes" @@ -264,7 +265,7 @@ func TestFailedFindCommitRequest(t *testing.T) { desc: "Invalid repo", repo: invalidRepo, revision: []byte("master"), - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "GetStorageByName: no such storage: \"fake\"", "repo scoped: invalid Repository", )), @@ -273,19 +274,19 @@ func TestFailedFindCommitRequest(t *testing.T) { desc: "Empty revision", repo: repo, revision: []byte(""), - expectedErr: helper.ErrInvalidArgumentf("empty revision"), + expectedErr: structerr.NewInvalidArgument("empty revision"), }, { desc: "Invalid revision", repo: repo, revision: []byte("-master"), - expectedErr: helper.ErrInvalidArgumentf("revision can't start with '-'"), + expectedErr: structerr.NewInvalidArgument("revision can't start with '-'"), }, { desc: "Invalid revision", repo: repo, revision: []byte("mas:ter"), - expectedErr: helper.ErrInvalidArgumentf("revision can't contain ':'"), + expectedErr: structerr.NewInvalidArgument("revision can't contain ':'"), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/commit/find_commits.go b/internal/gitaly/service/commit/find_commits.go index 0ce452505..5a1cb5b3d 100644 --- a/internal/gitaly/service/commit/find_commits.go +++ b/internal/gitaly/service/commit/find_commits.go @@ -16,7 +16,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/trailerparser" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -38,7 +37,7 @@ func (s *server) FindCommits(req *gitalypb.FindCommitsRequest, stream gitalypb.C ctx := stream.Context() if err := validateFindCommitsRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(req.GetRepository()) @@ -55,7 +54,7 @@ func (s *server) FindCommits(req *gitalypb.FindCommitsRequest, stream gitalypb.C // Clients might send empty paths. That is an error for _, path := range req.Paths { if len(path) == 0 { - return helper.ErrInvalidArgumentf("path is empty string") + return structerr.NewInvalidArgument("path is empty string") } } diff --git a/internal/gitaly/service/commit/isancestor.go b/internal/gitaly/service/commit/isancestor.go index 1f3e268ac..4053cc56e 100644 --- a/internal/gitaly/service/commit/isancestor.go +++ b/internal/gitaly/service/commit/isancestor.go @@ -8,7 +8,6 @@ import ( log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -28,7 +27,7 @@ func validateCommitIsAncestorRequest(in *gitalypb.CommitIsAncestorRequest) error func (s *server) CommitIsAncestor(ctx context.Context, in *gitalypb.CommitIsAncestorRequest) (*gitalypb.CommitIsAncestorResponse, error) { if err := validateCommitIsAncestorRequest(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } ret, err := s.commitIsAncestorName(ctx, in.Repository, in.AncestorId, in.ChildId) diff --git a/internal/gitaly/service/commit/languages.go b/internal/gitaly/service/commit/languages.go index c62baac24..cc9098aa8 100644 --- a/internal/gitaly/service/commit/languages.go +++ b/internal/gitaly/service/commit/languages.go @@ -12,7 +12,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/linguist" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -32,7 +31,7 @@ func (s *server) validateCommitLanguagesRequest(req *gitalypb.CommitLanguagesReq func (s *server) CommitLanguages(ctx context.Context, req *gitalypb.CommitLanguagesRequest) (*gitalypb.CommitLanguagesResponse, error) { if err := s.validateCommitLanguagesRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(req.GetRepository()) diff --git a/internal/gitaly/service/commit/last_commit_for_path.go b/internal/gitaly/service/commit/last_commit_for_path.go index 13632fcce..d7f07b3a5 100644 --- a/internal/gitaly/service/commit/last_commit_for_path.go +++ b/internal/gitaly/service/commit/last_commit_for_path.go @@ -6,14 +6,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/log" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) LastCommitForPath(ctx context.Context, in *gitalypb.LastCommitForPathRequest) (*gitalypb.LastCommitForPathResponse, error) { if err := validateLastCommitForPathRequest(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } resp, err := s.lastCommitForPath(ctx, in) diff --git a/internal/gitaly/service/commit/last_commit_for_path_test.go b/internal/gitaly/service/commit/last_commit_for_path_test.go index 28e19c7d0..65603dc64 100644 --- a/internal/gitaly/service/commit/last_commit_for_path_test.go +++ b/internal/gitaly/service/commit/last_commit_for_path_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" ) @@ -92,7 +92,7 @@ func TestFailedLastCommitForPathRequest(t *testing.T) { Repository: invalidRepo, Revision: []byte("some-branch"), }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "GetStorageByName: no such storage: \"fake\"", "repo scoped: invalid Repository", )), @@ -102,7 +102,7 @@ func TestFailedLastCommitForPathRequest(t *testing.T) { request: &gitalypb.LastCommitForPathRequest{ Revision: []byte("some-branch"), }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -112,7 +112,7 @@ func TestFailedLastCommitForPathRequest(t *testing.T) { request: &gitalypb.LastCommitForPathRequest{ Repository: repo, Path: []byte("foo/bar"), }, - expectedErr: helper.ErrInvalidArgumentf("empty revision"), + expectedErr: structerr.NewInvalidArgument("empty revision"), }, { desc: "Revision is invalid", @@ -121,7 +121,7 @@ func TestFailedLastCommitForPathRequest(t *testing.T) { Path: []byte("foo/bar"), Revision: []byte("--output=/meow"), }, - expectedErr: helper.ErrInvalidArgumentf("revision can't start with '-'"), + expectedErr: structerr.NewInvalidArgument("revision can't start with '-'"), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/commit/list_all_commits.go b/internal/gitaly/service/commit/list_all_commits.go index a4eb6c2eb..03c949ea5 100644 --- a/internal/gitaly/service/commit/list_all_commits.go +++ b/internal/gitaly/service/commit/list_all_commits.go @@ -5,7 +5,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -20,7 +19,7 @@ func (s *server) ListAllCommits( stream gitalypb.CommitService_ListAllCommitsServer, ) error { if err := verifyListAllCommitsRequest(request); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } ctx := stream.Context() diff --git a/internal/gitaly/service/commit/list_commits_by_oid.go b/internal/gitaly/service/commit/list_commits_by_oid.go index 084ab807f..0de257074 100644 --- a/internal/gitaly/service/commit/list_commits_by_oid.go +++ b/internal/gitaly/service/commit/list_commits_by_oid.go @@ -6,8 +6,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/protobuf/proto" ) @@ -27,7 +27,7 @@ func (s *server) ListCommitsByOid(in *gitalypb.ListCommitsByOidRequest, stream g ctx := stream.Context() repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) 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 ab7b87291..3d57cae52 100644 --- a/internal/gitaly/service/commit/list_commits_by_ref_name.go +++ b/internal/gitaly/service/commit/list_commits_by_ref_name.go @@ -4,7 +4,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -15,7 +14,7 @@ func (s *server) ListCommitsByRefName(in *gitalypb.ListCommitsByRefNameRequest, ctx := stream.Context() repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) diff --git a/internal/gitaly/service/commit/list_files.go b/internal/gitaly/service/commit/list_files.go index 99a58a477..39e79710d 100644 --- a/internal/gitaly/service/commit/list_files.go +++ b/internal/gitaly/service/commit/list_files.go @@ -8,7 +8,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/lstree" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -21,7 +20,7 @@ func (s *server) ListFiles(in *gitalypb.ListFilesRequest, stream gitalypb.Commit }).Debug("ListFiles") if err := validateListFilesRequest(in); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(in.GetRepository()) @@ -33,7 +32,7 @@ func (s *server) ListFiles(in *gitalypb.ListFilesRequest, stream gitalypb.Commit if len(revision) == 0 { defaultBranch, err := repo.GetDefaultBranch(stream.Context()) if err != nil { - return helper.ErrNotFoundf("revision not found %q", revision) + return structerr.NewNotFound("revision not found %q", revision) } if len(defaultBranch) == 0 { 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 972b96580..e520104cb 100644 --- a/internal/gitaly/service/commit/list_last_commits_for_tree.go +++ b/internal/gitaly/service/commit/list_last_commits_for_tree.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/log" "gitlab.com/gitlab-org/gitaly/v15/internal/git/lstree" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -19,7 +18,7 @@ var maxNumStatBatchSize = 10 func (s *server) ListLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeRequest, stream gitalypb.CommitService_ListLastCommitsForTreeServer) error { if err := validateListLastCommitsForTreeRequest(in); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.listLastCommitsForTree(in, stream); err != nil { diff --git a/internal/gitaly/service/commit/stats.go b/internal/gitaly/service/commit/stats.go index 96b8fee22..9a8153b73 100644 --- a/internal/gitaly/service/commit/stats.go +++ b/internal/gitaly/service/commit/stats.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -26,7 +25,7 @@ func validateCommitStatsRequest(in *gitalypb.CommitStatsRequest) error { func (s *server) CommitStats(ctx context.Context, in *gitalypb.CommitStatsRequest) (*gitalypb.CommitStatsResponse, error) { if err := validateCommitStatsRequest(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } resp, err := s.commitStats(ctx, in) diff --git a/internal/gitaly/service/commit/stats_test.go b/internal/gitaly/service/commit/stats_test.go index adbe9005f..b39f0a0b5 100644 --- a/internal/gitaly/service/commit/stats_test.go +++ b/internal/gitaly/service/commit/stats_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "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" @@ -109,7 +108,7 @@ func TestCommitStatsFailure(t *testing.T) { }, Revision: []byte("test-do-not-touch"), }, - expectedErr: helper.ErrNotFoundf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewNotFound(testhelper.GitalyOrPraefect( fmt.Sprintf("GetRepoPath: not a git repository: %q", filepath.Join(cfg.Storages[0].Path, "bar.git")), "accessor call: route repository accessor: consistent storages: repository \"default\"/\"bar.git\" not found", )), @@ -123,7 +122,7 @@ func TestCommitStatsFailure(t *testing.T) { }, Revision: []byte("test-do-not-touch"), }, - expectedErr: helper.ErrNotFoundf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewNotFound(testhelper.GitalyOrPraefect( fmt.Sprintf("GetRepoPath: not a git repository: %q", filepath.Join(cfg.Storages[0].Path, "bar.git")), "accessor call: route repository accessor: consistent storages: repository \"default\"/\"bar.git\" not found", )), @@ -137,7 +136,7 @@ func TestCommitStatsFailure(t *testing.T) { }, Revision: []byte("test-do-not-touch"), }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "GetStorageByName: no such storage: \"foo\"", "repo scoped: invalid Repository", )), @@ -156,7 +155,7 @@ func TestCommitStatsFailure(t *testing.T) { Repository: repo, Revision: []byte("--outpu=/meow"), }, - expectedErr: helper.ErrInvalidArgumentf("revision can't start with '-'"), + expectedErr: structerr.NewInvalidArgument("revision can't start with '-'"), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/commit/tree_entries_test.go b/internal/gitaly/service/commit/tree_entries_test.go index a0e768bab..18321a7b5 100644 --- a/internal/gitaly/service/commit/tree_entries_test.go +++ b/internal/gitaly/service/commit/tree_entries_test.go @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" "google.golang.org/grpc/codes" @@ -671,7 +671,7 @@ func TestGetTreeEntries_validation(t *testing.T) { Revision: revision, Path: path, }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "GetStorageByName: no such storage: \"fake\"", "repo scoped: invalid Repository", )), @@ -683,7 +683,7 @@ func TestGetTreeEntries_validation(t *testing.T) { Revision: revision, Path: path, }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -695,7 +695,7 @@ func TestGetTreeEntries_validation(t *testing.T) { Revision: nil, Path: path, }, - expectedErr: helper.ErrInvalidArgumentf("empty revision"), + expectedErr: structerr.NewInvalidArgument("empty revision"), }, { desc: "path is empty", @@ -703,7 +703,7 @@ func TestGetTreeEntries_validation(t *testing.T) { Repository: repo, Revision: revision, }, - expectedErr: helper.ErrInvalidArgumentf("empty Path"), + expectedErr: structerr.NewInvalidArgument("empty Path"), }, { desc: "revision is invalid", @@ -712,7 +712,7 @@ func TestGetTreeEntries_validation(t *testing.T) { Revision: []byte("--output=/meow"), Path: path, }, - expectedErr: helper.ErrInvalidArgumentf("revision can't start with '-'"), + expectedErr: structerr.NewInvalidArgument("revision can't start with '-'"), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/commit/tree_entry.go b/internal/gitaly/service/commit/tree_entry.go index b94afb647..ae6c34f3f 100644 --- a/internal/gitaly/service/commit/tree_entry.go +++ b/internal/gitaly/service/commit/tree_entry.go @@ -8,7 +8,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -29,7 +28,7 @@ func sendTreeEntry( } if treeEntry == nil || len(treeEntry.Oid) == 0 { - return helper.ErrNotFoundf("not found: %s", path) + return structerr.NewNotFound("not found: %s", path) } if treeEntry.Type == gitalypb.TreeEntry_COMMIT { diff --git a/internal/gitaly/service/commit/tree_entry_test.go b/internal/gitaly/service/commit/tree_entry_test.go index 95229df9a..30a74f90b 100644 --- a/internal/gitaly/service/commit/tree_entry_test.go +++ b/internal/gitaly/service/commit/tree_entry_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "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" @@ -169,7 +168,7 @@ func TestFailedTreeEntry(t *testing.T) { { name: "Repository doesn't exist", req: &gitalypb.TreeEntryRequest{Repository: &gitalypb.Repository{StorageName: "fake", RelativePath: "path"}, Revision: revision, Path: path}, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "GetStorageByName: no such storage: \"fake\"", "repo scoped: invalid Repository", )), @@ -177,7 +176,7 @@ func TestFailedTreeEntry(t *testing.T) { { name: "Repository is nil", req: &gitalypb.TreeEntryRequest{Repository: nil, Revision: revision, Path: path}, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -185,27 +184,27 @@ func TestFailedTreeEntry(t *testing.T) { { name: "Revision is empty", req: &gitalypb.TreeEntryRequest{Repository: repo, Revision: nil, Path: path}, - expectedErr: helper.ErrInvalidArgumentf("empty revision"), + expectedErr: structerr.NewInvalidArgument("empty revision"), }, { name: "Path is empty", req: &gitalypb.TreeEntryRequest{Repository: repo, Revision: revision}, - expectedErr: helper.ErrInvalidArgumentf("empty Path"), + expectedErr: structerr.NewInvalidArgument("empty Path"), }, { name: "Revision is invalid", req: &gitalypb.TreeEntryRequest{Repository: repo, Revision: []byte("--output=/meow"), Path: path}, - expectedErr: helper.ErrInvalidArgumentf("revision can't start with '-'"), + expectedErr: structerr.NewInvalidArgument("revision can't start with '-'"), }, { name: "Limit is negative", req: &gitalypb.TreeEntryRequest{Repository: repo, Revision: revision, Path: path, Limit: -1}, - expectedErr: helper.ErrInvalidArgumentf("negative Limit"), + expectedErr: structerr.NewInvalidArgument("negative Limit"), }, { name: "MaximumSize is negative", req: &gitalypb.TreeEntryRequest{Repository: repo, Revision: revision, Path: path, MaxSize: -1}, - expectedErr: helper.ErrInvalidArgumentf("negative MaxSize"), + expectedErr: structerr.NewInvalidArgument("negative MaxSize"), }, { name: "Object bigger than MaxSize", @@ -215,17 +214,17 @@ func TestFailedTreeEntry(t *testing.T) { { name: "Path is outside of repository", req: &gitalypb.TreeEntryRequest{Repository: repo, Revision: []byte("913c66a37b4a45b9769037c55c2d238bd0942d2e"), Path: []byte("../bar/.gitkeep")}, // Git blows up on paths like this - expectedErr: helper.ErrNotFoundf("not found: ../bar/.gitkeep"), + expectedErr: structerr.NewNotFound("not found: ../bar/.gitkeep"), }, { name: "Missing file with space in path", req: &gitalypb.TreeEntryRequest{Repository: repo, Revision: []byte("deadfacedeadfacedeadfacedeadfacedeadface"), Path: []byte("with space/README.md")}, - expectedErr: helper.ErrNotFoundf("not found: with space/README.md"), + expectedErr: structerr.NewNotFound("not found: with space/README.md"), }, { name: "Missing file", req: &gitalypb.TreeEntryRequest{Repository: repo, Revision: []byte("e63f41fe459e62e1228fcef60d7189127aeba95a"), Path: []byte("missing.rb")}, - expectedErr: helper.ErrNotFoundf("not found: missing.rb"), + expectedErr: structerr.NewNotFound("not found: missing.rb"), }, } { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/gitaly/service/conflicts/list_conflict_files.go b/internal/gitaly/service/conflicts/list_conflict_files.go index 2037d1e5b..59c5cc7b9 100644 --- a/internal/gitaly/service/conflicts/list_conflict_files.go +++ b/internal/gitaly/service/conflicts/list_conflict_files.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -20,7 +19,7 @@ func (s *server) ListConflictFiles(request *gitalypb.ListConflictFilesRequest, s ctx := stream.Context() if err := validateListConflictFilesRequest(request); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(request.GetRepository()) @@ -47,7 +46,7 @@ func (s *server) ListConflictFiles(request *gitalypb.ListConflictFilesRequest, s }) if err != nil { if errors.Is(err, git2go.ErrInvalidArgument) { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } return structerr.NewInternal("%w", err) } diff --git a/internal/gitaly/service/conflicts/resolve_conflicts.go b/internal/gitaly/service/conflicts/resolve_conflicts.go index 081354ea4..d1056c38d 100644 --- a/internal/gitaly/service/conflicts/resolve_conflicts.go +++ b/internal/gitaly/service/conflicts/resolve_conflicts.go @@ -20,7 +20,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/repository" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -129,7 +128,7 @@ func (s *server) resolveConflicts(header *gitalypb.ResolveConflictsRequestHeader _, sectionExists := ck["sections"] _, contentExists := ck["content"] if !sectionExists && !contentExists { - return helper.ErrInvalidArgumentf("missing sections or content for a resolution") + return structerr.NewInvalidArgument("missing sections or content for a resolution") } } @@ -187,7 +186,7 @@ func (s *server) resolveConflicts(header *gitalypb.ResolveConflictsRequestHeader }) if err != nil { if errors.Is(err, git2go.ErrInvalidArgument) { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } return err } diff --git a/internal/gitaly/service/diff/find_changed_paths.go b/internal/gitaly/service/diff/find_changed_paths.go index 70705e593..ba6bb4026 100644 --- a/internal/gitaly/service/diff/find_changed_paths.go +++ b/internal/gitaly/service/diff/find_changed_paths.go @@ -13,7 +13,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -177,13 +176,13 @@ func (t *findChangedPathsSender) Send() error { func resolveObjectWithType(ctx context.Context, repo *localrepo.Repo, revision string, expectedType string) (git.ObjectID, error) { if revision == "" { - return "", helper.ErrInvalidArgumentf("revision cannot be empty") + return "", structerr.NewInvalidArgument("revision cannot be empty") } oid, err := repo.ResolveRevision(ctx, git.Revision(fmt.Sprintf("%s^{%s}", revision, expectedType))) if err != nil { if errors.Is(err, git.ErrReferenceNotFound) { - return "", helper.ErrNotFoundf("revision can not be found: %q", revision) + return "", structerr.NewNotFound("revision can not be found: %q", revision) } return "", err } @@ -194,7 +193,7 @@ func resolveObjectWithType(ctx context.Context, repo *localrepo.Repo, revision s func (s *server) validateFindChangedPathsRequestParams(ctx context.Context, in *gitalypb.FindChangedPathsRequest) error { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if _, err := s.locator.GetRepoPath(repository); err != nil { return err @@ -204,7 +203,7 @@ func (s *server) validateFindChangedPathsRequestParams(ctx context.Context, in * if len(in.GetCommits()) > 0 { //nolint:staticcheck if len(in.GetRequests()) > 0 { - return helper.ErrInvalidArgumentf("cannot specify both commits and requests") + return structerr.NewInvalidArgument("cannot specify both commits and requests") } in.Requests = make([]*gitalypb.FindChangedPathsRequest_Request, len(in.GetCommits())) //nolint:staticcheck diff --git a/internal/gitaly/service/diff/find_changed_paths_test.go b/internal/gitaly/service/diff/find_changed_paths_test.go index f69cc5b4c..bbe1a1dfa 100644 --- a/internal/gitaly/service/diff/find_changed_paths_test.go +++ b/internal/gitaly/service/diff/find_changed_paths_test.go @@ -9,7 +9,7 @@ import ( "testing" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" ) @@ -481,7 +481,7 @@ func TestFindChangedPathsRequest_failing(t *testing.T) { desc: "Repository not provided", repo: nil, commits: []string{"e4003da16c1c2c3fc4567700121b17bf8e591c6c", "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab"}, - err: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + err: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -490,7 +490,7 @@ func TestFindChangedPathsRequest_failing(t *testing.T) { desc: "Repo not found", repo: &gitalypb.Repository{StorageName: repo.GetStorageName(), RelativePath: "bar.git"}, commits: []string{"e4003da16c1c2c3fc4567700121b17bf8e591c6c", "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab"}, - err: helper.ErrNotFoundf(testhelper.GitalyOrPraefect( + err: structerr.NewNotFound(testhelper.GitalyOrPraefect( fmt.Sprintf("GetRepoPath: not a git repository: %q", filepath.Join(cfg.Storages[0].Path, "bar.git")), `accessor call: route repository accessor: consistent storages: repository "default"/"bar.git" not found`, )), @@ -499,7 +499,7 @@ func TestFindChangedPathsRequest_failing(t *testing.T) { desc: "Storage not found", repo: &gitalypb.Repository{StorageName: "foo", RelativePath: "bar.git"}, commits: []string{"e4003da16c1c2c3fc4567700121b17bf8e591c6c", "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab"}, - err: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + err: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( `GetStorageByName: no such storage: "foo"`, "repo scoped: invalid Repository", )), @@ -508,7 +508,7 @@ func TestFindChangedPathsRequest_failing(t *testing.T) { desc: "Commits cannot contain an empty commit", repo: repo, commits: []string{""}, - err: helper.ErrInvalidArgumentf("resolving commit: revision cannot be empty"), + err: structerr.NewInvalidArgument("resolving commit: revision cannot be empty"), }, { desc: "Specifying both commits and requests", @@ -523,19 +523,19 @@ func TestFindChangedPathsRequest_failing(t *testing.T) { }, }, }, - err: helper.ErrInvalidArgumentf("cannot specify both commits and requests"), + err: structerr.NewInvalidArgument("cannot specify both commits and requests"), }, { desc: "Invalid commit", repo: repo, commits: []string{"invalidinvalidinvalid", "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab"}, - err: helper.ErrNotFoundf(`resolving commit: revision can not be found: "invalidinvalidinvalid"`), + err: structerr.NewNotFound(`resolving commit: revision can not be found: "invalidinvalidinvalid"`), }, { desc: "Commit not found", repo: repo, commits: []string{"z4003da16c1c2c3fc4567700121b17bf8e591c6c", "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab"}, - err: helper.ErrNotFoundf(`resolving commit: revision can not be found: "z4003da16c1c2c3fc4567700121b17bf8e591c6c"`), + err: structerr.NewNotFound(`resolving commit: revision can not be found: "z4003da16c1c2c3fc4567700121b17bf8e591c6c"`), }, { desc: "Tree object as commit", @@ -549,7 +549,7 @@ func TestFindChangedPathsRequest_failing(t *testing.T) { }, }, }, - err: helper.ErrNotFoundf(`resolving commit: revision can not be found: "07f8147e8e73aab6c935c296e8cdc5194dee729b"`), + err: structerr.NewNotFound(`resolving commit: revision can not be found: "07f8147e8e73aab6c935c296e8cdc5194dee729b"`), }, { desc: "Tree object as parent commit", @@ -566,7 +566,7 @@ func TestFindChangedPathsRequest_failing(t *testing.T) { }, }, }, - err: helper.ErrNotFoundf(`resolving commit parent: revision can not be found: "07f8147e8e73aab6c935c296e8cdc5194dee729b"`), + err: structerr.NewNotFound(`resolving commit parent: revision can not be found: "07f8147e8e73aab6c935c296e8cdc5194dee729b"`), }, { desc: "Blob object as left tree", @@ -581,7 +581,7 @@ func TestFindChangedPathsRequest_failing(t *testing.T) { }, }, }, - err: helper.ErrNotFoundf(`resolving left tree: revision can not be found: "50b27c6518be44c42c4d87966ae2481ce895624c"`), + err: structerr.NewNotFound(`resolving left tree: revision can not be found: "50b27c6518be44c42c4d87966ae2481ce895624c"`), }, { desc: "Blob object as right tree", @@ -596,7 +596,7 @@ func TestFindChangedPathsRequest_failing(t *testing.T) { }, }, }, - err: helper.ErrNotFoundf(`resolving right tree: revision can not be found: "50b27c6518be44c42c4d87966ae2481ce895624c"`), + err: structerr.NewNotFound(`resolving right tree: revision can not be found: "50b27c6518be44c42c4d87966ae2481ce895624c"`), }, } diff --git a/internal/gitaly/service/diff/numstat.go b/internal/gitaly/service/diff/numstat.go index 952dc74ea..eb285f862 100644 --- a/internal/gitaly/service/diff/numstat.go +++ b/internal/gitaly/service/diff/numstat.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/diff" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -80,7 +79,7 @@ func sendStats(batch []*gitalypb.DiffStats, stream gitalypb.DiffService_DiffStat func (s *server) validateDiffStatsRequestParams(in *gitalypb.DiffStatsRequest) error { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if _, err := s.locator.GetRepoPath(repository); err != nil { return err diff --git a/internal/gitaly/service/hook/pack_objects.go b/internal/gitaly/service/hook/pack_objects.go index ef396c390..9ede49427 100644 --- a/internal/gitaly/service/hook/pack_objects.go +++ b/internal/gitaly/service/hook/pack_objects.go @@ -372,18 +372,18 @@ func bufferStdin(r io.Reader, h hash.Hash) (_ io.ReadCloser, err error) { func (s *server) PackObjectsHookWithSidechannel(ctx context.Context, req *gitalypb.PackObjectsHookWithSidechannelRequest) (*gitalypb.PackObjectsHookWithSidechannelResponse, error) { if err := service.ValidateRepository(req.GetRepository()); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } args, err := parsePackObjectsArgs(req.Args) if err != nil { - return nil, helper.ErrInvalidArgumentf("invalid pack-objects command: %v: %w", req.Args, err) + return nil, structerr.NewInvalidArgument("invalid pack-objects command: %v: %w", req.Args, err) } c, err := gitalyhook.GetSidechannel(ctx) if err != nil { if errors.As(err, &gitalyhook.ErrInvalidSidechannelAddress{}) { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return nil, structerr.NewInternal("get side-channel: %w", err) } diff --git a/internal/gitaly/service/hook/post_receive.go b/internal/gitaly/service/hook/post_receive.go index 8524bbcb9..5000279f8 100644 --- a/internal/gitaly/service/hook/post_receive.go +++ b/internal/gitaly/service/hook/post_receive.go @@ -7,7 +7,6 @@ import ( "sync" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -31,7 +30,7 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ } if err := validatePostReceiveHookRequest(firstRequest); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } stdin := streamio.NewReader(func() ([]byte, error) { diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go index 112c9b8c1..a57b51115 100644 --- a/internal/gitaly/service/hook/pre_receive.go +++ b/internal/gitaly/service/hook/pre_receive.go @@ -7,7 +7,6 @@ import ( "sync" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -20,7 +19,7 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer } if err := validatePreReceiveHookRequest(firstRequest); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repository := firstRequest.GetRepository() diff --git a/internal/gitaly/service/hook/reference_transaction.go b/internal/gitaly/service/hook/reference_transaction.go index ecebfd04b..f6a9eacef 100644 --- a/internal/gitaly/service/hook/reference_transaction.go +++ b/internal/gitaly/service/hook/reference_transaction.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -23,7 +22,7 @@ func (s *server) ReferenceTransactionHook(stream gitalypb.HookService_ReferenceT } if err := validateReferenceTransactionHookRequest(request); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } var state hook.ReferenceTransactionState @@ -35,7 +34,7 @@ func (s *server) ReferenceTransactionHook(stream gitalypb.HookService_ReferenceT case gitalypb.ReferenceTransactionHookRequest_ABORTED: state = hook.ReferenceTransactionAborted default: - return helper.ErrInvalidArgumentf("invalid hook state") + return structerr.NewInvalidArgument("invalid hook state") } stdin := streamio.NewReader(func() ([]byte, error) { diff --git a/internal/gitaly/service/hook/update.go b/internal/gitaly/service/hook/update.go index ed7a57156..963c138be 100644 --- a/internal/gitaly/service/hook/update.go +++ b/internal/gitaly/service/hook/update.go @@ -6,7 +6,6 @@ import ( "sync" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -18,7 +17,7 @@ func validateUpdateHookRequest(in *gitalypb.UpdateHookRequest) error { func (s *server) UpdateHook(in *gitalypb.UpdateHookRequest, stream gitalypb.HookService_UpdateHookServer) error { if err := validateUpdateHookRequest(in); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } var m sync.Mutex diff --git a/internal/gitaly/service/internalgitaly/walkrepos.go b/internal/gitaly/service/internalgitaly/walkrepos.go index 8ba2e9a55..b47ae658f 100644 --- a/internal/gitaly/service/internalgitaly/walkrepos.go +++ b/internal/gitaly/service/internalgitaly/walkrepos.go @@ -6,7 +6,6 @@ import ( "path/filepath" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/protobuf/types/known/timestamppb" @@ -31,7 +30,7 @@ func (s *server) storagePath(storageName string) (string, error) { return storage.Path, nil } } - return "", helper.ErrNotFoundf("storage name %q not found", storageName) + return "", structerr.NewNotFound("storage name %q not found", storageName) } func walkStorage(ctx context.Context, storagePath string, stream gitalypb.InternalGitaly_WalkReposServer) error { diff --git a/internal/gitaly/service/namespace/namespace.go b/internal/gitaly/service/namespace/namespace.go index b3330bacb..1c2ab5773 100644 --- a/internal/gitaly/service/namespace/namespace.go +++ b/internal/gitaly/service/namespace/namespace.go @@ -7,7 +7,6 @@ import ( "os" "path/filepath" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -74,7 +73,7 @@ func (s *server) validateRenameNamespaceRequest(ctx context.Context, in *gitalyp func (s *server) RenameNamespace(ctx context.Context, in *gitalypb.RenameNamespaceRequest) (*gitalypb.RenameNamespaceResponse, error) { if err := s.validateRenameNamespaceRequest(ctx, in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } storagePath, err := s.locator.GetStorageByName(in.GetStorageName()) diff --git a/internal/gitaly/service/objectpool/alternates.go b/internal/gitaly/service/objectpool/alternates.go index 3a3c08ede..33f5b6d5f 100644 --- a/internal/gitaly/service/objectpool/alternates.go +++ b/internal/gitaly/service/objectpool/alternates.go @@ -14,7 +14,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" @@ -31,7 +30,7 @@ import ( func (s *server) DisconnectGitAlternates(ctx context.Context, req *gitalypb.DisconnectGitAlternatesRequest) (*gitalypb.DisconnectGitAlternatesResponse, error) { repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) diff --git a/internal/gitaly/service/objectpool/create.go b/internal/gitaly/service/objectpool/create.go index 9d2909159..e4011a785 100644 --- a/internal/gitaly/service/objectpool/create.go +++ b/internal/gitaly/service/objectpool/create.go @@ -5,14 +5,13 @@ import ( "errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git/objectpool" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) // errMissingOriginRepository is returned when the request is missing the // origin repository. -var errMissingOriginRepository = helper.ErrInvalidArgumentf("no origin repository") +var errMissingOriginRepository = structerr.NewInvalidArgument("no origin repository") func (s *server) CreateObjectPool(ctx context.Context, in *gitalypb.CreateObjectPoolRequest) (*gitalypb.CreateObjectPoolResponse, error) { if in.GetOrigin() == nil { diff --git a/internal/gitaly/service/objectpool/create_test.go b/internal/gitaly/service/objectpool/create_test.go index a0a43a106..11ec21ccc 100644 --- a/internal/gitaly/service/objectpool/create_test.go +++ b/internal/gitaly/service/objectpool/create_test.go @@ -14,7 +14,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/objectpool" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -102,7 +102,7 @@ func TestCreate_unsuccessful(t *testing.T) { request: &gitalypb.CreateObjectPoolRequest{ Origin: repo, }, - expectedErr: helper.ErrInvalidArgumentf("GetStorageByName: no such storage: %q", ""), + expectedErr: structerr.NewInvalidArgument("GetStorageByName: no such storage: %q", ""), }, { desc: "outside pools directory", diff --git a/internal/gitaly/service/objectpool/delete_test.go b/internal/gitaly/service/objectpool/delete_test.go index da6cef78a..2ac01f211 100644 --- a/internal/gitaly/service/objectpool/delete_test.go +++ b/internal/gitaly/service/objectpool/delete_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" ) @@ -35,8 +35,8 @@ func TestDelete(t *testing.T) { desc: "no pool in request fails", noPool: true, expectedErr: testhelper.GitalyOrPraefect( - helper.ErrInvalidArgumentf("GetStorageByName: no such storage: %q", ""), - helper.ErrInvalidArgumentf("no object pool repository"), + structerr.NewInvalidArgument("GetStorageByName: no such storage: %q", ""), + structerr.NewInvalidArgument("no object pool repository"), ), }, { diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool.go b/internal/gitaly/service/objectpool/fetch_into_object_pool.go index 9cf0ca71c..9a947c6c5 100644 --- a/internal/gitaly/service/objectpool/fetch_into_object_pool.go +++ b/internal/gitaly/service/objectpool/fetch_into_object_pool.go @@ -6,19 +6,18 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/objectpool" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) FetchIntoObjectPool(ctx context.Context, req *gitalypb.FetchIntoObjectPoolRequest) (*gitalypb.FetchIntoObjectPoolResponse, error) { if err := validateFetchIntoObjectPoolRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } objectPool, err := objectpool.FromProto(s.locator, s.gitCmdFactory, s.catfileCache, s.txManager, s.housekeepingManager, req.GetObjectPool()) if err != nil { - return nil, helper.ErrInvalidArgumentf("object pool invalid: %w", err) + return nil, structerr.NewInvalidArgument("object pool invalid: %w", err) } origin := s.localrepo(req.GetOrigin()) diff --git a/internal/gitaly/service/objectpool/get.go b/internal/gitaly/service/objectpool/get.go index 1cf5e4135..310b586e8 100644 --- a/internal/gitaly/service/objectpool/get.go +++ b/internal/gitaly/service/objectpool/get.go @@ -6,14 +6,14 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/internal/git/objectpool" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) GetObjectPool(ctx context.Context, in *gitalypb.GetObjectPoolRequest) (*gitalypb.GetObjectPoolResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) diff --git a/internal/gitaly/service/objectpool/link_test.go b/internal/gitaly/service/objectpool/link_test.go index ac11a2da9..14f1d5f58 100644 --- a/internal/gitaly/service/objectpool/link_test.go +++ b/internal/gitaly/service/objectpool/link_test.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver" @@ -44,7 +43,7 @@ func TestLink(t *testing.T) { Repository: nil, ObjectPool: poolProto, }, - expectedErr: helper.ErrInvalidArgumentf("empty Repository"), + expectedErr: structerr.NewInvalidArgument("empty Repository"), }, { desc: "unset object pool", @@ -52,7 +51,7 @@ func TestLink(t *testing.T) { Repository: repo, ObjectPool: nil, }, - expectedErr: helper.ErrInvalidArgumentf("GetStorageByName: no such storage: %q", ""), + expectedErr: structerr.NewInvalidArgument("GetStorageByName: no such storage: %q", ""), }, { desc: "successful", @@ -141,8 +140,8 @@ func TestLink_noPool(t *testing.T) { }, }) testhelper.RequireGrpcError(t, testhelper.GitalyOrPraefect( - error(structerr.NewFailedPrecondition("object pool is not a valid git repository")), - helper.ErrNotFoundf( + structerr.NewFailedPrecondition("object pool is not a valid git repository"), + structerr.NewNotFound( "mutator call: route repository mutator: resolve additional replica path: get additional repository id: repository %q/%q not found", cfg.Storages[0].Name, poolRelativePath, diff --git a/internal/gitaly/service/objectpool/reduplicate.go b/internal/gitaly/service/objectpool/reduplicate.go index 438b39d92..3978fdf6b 100644 --- a/internal/gitaly/service/objectpool/reduplicate.go +++ b/internal/gitaly/service/objectpool/reduplicate.go @@ -5,14 +5,14 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) ReduplicateRepository(ctx context.Context, req *gitalypb.ReduplicateRepositoryRequest) (*gitalypb.ReduplicateRepositoryResponse, error) { repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } cmd, err := s.gitCmdFactory.New(ctx, repository, git.Command{ diff --git a/internal/gitaly/service/objectpool/util.go b/internal/gitaly/service/objectpool/util.go index 5b15dbfeb..a70069dcd 100644 --- a/internal/gitaly/service/objectpool/util.go +++ b/internal/gitaly/service/objectpool/util.go @@ -4,16 +4,15 @@ import ( "errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git/objectpool" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) var ( - errInvalidPoolDir = helper.ErrInvalidArgumentf("%w", objectpool.ErrInvalidPoolDir) + errInvalidPoolDir = structerr.NewInvalidArgument("%w", objectpool.ErrInvalidPoolDir) // errMissingPool is returned when the request is missing the object pool. - errMissingPool = helper.ErrInvalidArgumentf("no object pool repository") + errMissingPool = structerr.NewInvalidArgument("no object pool repository") ) // PoolRequest is the interface of a gRPC request that carries an object pool. diff --git a/internal/gitaly/service/operations/apply_patch.go b/internal/gitaly/service/operations/apply_patch.go index 8c7e651ee..ac7fa1421 100644 --- a/internal/gitaly/service/operations/apply_patch.go +++ b/internal/gitaly/service/operations/apply_patch.go @@ -42,11 +42,11 @@ func (s *Server) UserApplyPatch(stream gitalypb.OperationService_UserApplyPatchS header := firstRequest.GetHeader() if header == nil { - return helper.ErrInvalidArgumentf("empty UserApplyPatch_Header") + return structerr.NewInvalidArgument("empty UserApplyPatch_Header") } if err := validateUserApplyPatchHeader(header); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.userApplyPatch(stream.Context(), header, stream); err != nil { @@ -90,7 +90,7 @@ func (s *Server) userApplyPatch(ctx context.Context, header *gitalypb.UserApplyP if header.Timestamp != nil { committerTime, err = dateFromProto(header) if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } } diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go index bc7d79b96..a7acc2485 100644 --- a/internal/gitaly/service/operations/branches.go +++ b/internal/gitaly/service/operations/branches.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -33,7 +32,7 @@ func validateUserCreateBranchRequest(in *gitalypb.UserCreateBranchRequest) error //nolint:revive // This is unintentionally missing documentation. func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateBranchRequest) (*gitalypb.UserCreateBranchResponse, error) { if err := validateUserCreateBranchRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository()) if err != nil { @@ -54,7 +53,7 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB startPointOID, err := git.ObjectHashSHA1.FromHex(startPointCommit.Id) if err != nil { - return nil, helper.ErrInvalidArgumentf("could not parse start point commit ID: %w", err) + return nil, structerr.NewInvalidArgument("could not parse start point commit ID: %w", err) } referenceName := git.NewReferenceNameFromBranchName(string(req.BranchName)) @@ -121,7 +120,7 @@ func validateUserUpdateBranchGo(req *gitalypb.UserUpdateBranchRequest) error { func (s *Server) UserUpdateBranch(ctx context.Context, req *gitalypb.UserUpdateBranchRequest) (*gitalypb.UserUpdateBranchResponse, error) { // Validate the request if err := validateUserUpdateBranchGo(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } newOID, err := git.ObjectHashSHA1.FromHex(string(req.Newrev)) @@ -178,7 +177,7 @@ func validateUserDeleteBranchRequest(in *gitalypb.UserDeleteBranchRequest) error // hooks and contacts Rails to verify that the user is indeed allowed to delete that branch. func (s *Server) UserDeleteBranch(ctx context.Context, req *gitalypb.UserDeleteBranchRequest) (*gitalypb.UserDeleteBranchResponse, error) { if err := validateUserDeleteBranchRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } referenceName := git.NewReferenceNameFromBranchName(string(req.BranchName)) diff --git a/internal/gitaly/service/operations/cherry_pick.go b/internal/gitaly/service/operations/cherry_pick.go index 14b22d45c..f770c5bba 100644 --- a/internal/gitaly/service/operations/cherry_pick.go +++ b/internal/gitaly/service/operations/cherry_pick.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -19,7 +18,7 @@ import ( // branch. See the protobuf documentation for details. func (s *Server) UserCherryPick(ctx context.Context, req *gitalypb.UserCherryPickRequest) (*gitalypb.UserCherryPickResponse, error) { if err := validateCherryPickOrRevertRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository()) @@ -89,9 +88,9 @@ func (s *Server) UserCherryPick(ctx context.Context, req *gitalypb.UserCherryPic }, ) case errors.As(err, &git2go.CommitNotFoundError{}): - return nil, helper.ErrNotFoundf("%w", err) + return nil, structerr.NewNotFound("%w", err) case errors.Is(err, git2go.ErrInvalidArgument): - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) default: return nil, structerr.NewInternal("cherry-pick command: %w", err) } diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go index 234ca56da..aba13af9d 100644 --- a/internal/gitaly/service/operations/commit_files.go +++ b/internal/gitaly/service/operations/commit_files.go @@ -18,7 +18,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -34,11 +33,11 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile header := firstRequest.GetHeader() if header == nil { - return helper.ErrInvalidArgumentf("empty UserCommitFilesRequestHeader") + return structerr.NewInvalidArgument("empty UserCommitFilesRequestHeader") } if err = validateUserCommitFilesHeader(header); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } ctx := stream.Context() @@ -92,7 +91,7 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile }, ) case errors.As(err, new(git2go.InvalidArgumentError)): - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) default: return err } @@ -105,7 +104,7 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile case errors.As(err, &customHookErr): response = gitalypb.UserCommitFilesResponse{PreReceiveError: customHookErr.Error()} case errors.As(err, new(git2go.InvalidArgumentError)): - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) default: return structerr.NewInternal("%w", err) } @@ -192,7 +191,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi } else { parentCommitOID, err = git.ObjectHashSHA1.FromHex(header.StartSha) if err != nil { - return helper.ErrInvalidArgumentf("cannot resolve parent commit: %w", err) + return structerr.NewInvalidArgument("cannot resolve parent commit: %w", err) } } @@ -238,12 +237,12 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi actions := make([]git2go.Action, 0, len(pbActions)) for _, pbAction := range pbActions { if _, ok := gitalypb.UserCommitFilesActionHeader_ActionType_name[int32(pbAction.header.Action)]; !ok { - return helper.ErrInvalidArgumentf("NoMethodError: undefined method `downcase' for %d:Integer", pbAction.header.Action) + return structerr.NewInvalidArgument("NoMethodError: undefined method `downcase' for %d:Integer", pbAction.header.Action) } path, err := validatePath(repoPath, string(pbAction.header.FilePath)) if err != nil { - return helper.ErrInvalidArgumentf("validate path: %w", err) + return structerr.NewInvalidArgument("validate path: %w", err) } content := io.Reader(bytes.NewReader(pbAction.content)) @@ -271,7 +270,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi case gitalypb.UserCommitFilesActionHeader_MOVE: prevPath, err := validatePath(repoPath, string(pbAction.header.PreviousPath)) if err != nil { - return helper.ErrInvalidArgumentf("validate previous path: %w", err) + return structerr.NewInvalidArgument("validate previous path: %w", err) } var oid git.ObjectID @@ -311,7 +310,7 @@ func (s *Server) userCommitFiles(ctx context.Context, header *gitalypb.UserCommi now, err := dateFromProto(header) if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } committer := git2go.NewSignature(string(header.User.Name), string(header.User.Email), now) diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index 496271c98..901a37b32 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -13,7 +13,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -60,7 +59,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc } if err := validateMergeBranchRequest(firstRequest); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, firstRequest.GetRepository()) @@ -100,7 +99,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc authorDate, err := dateFromProto(firstRequest) if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } merge, err := s.git2goExecutor.Merge(ctx, quarantineRepo, git2go.MergeCommand{ @@ -114,7 +113,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc }) if err != nil { if errors.Is(err, git2go.ErrInvalidArgument) { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } var conflictErr git2go.ConflictingFilesError @@ -246,7 +245,7 @@ func validateFFRequest(in *gitalypb.UserFFBranchRequest) error { //nolint:revive // This is unintentionally missing documentation. func (s *Server) UserFFBranch(ctx context.Context, in *gitalypb.UserFFBranchRequest) (*gitalypb.UserFFBranchResponse, error) { if err := validateFFRequest(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } referenceName := git.NewReferenceNameFromBranchName(string(in.Branch)) @@ -261,12 +260,12 @@ func (s *Server) UserFFBranch(ctx context.Context, in *gitalypb.UserFFBranchRequ revision, err := quarantineRepo.ResolveRevision(ctx, referenceName.Revision()) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } commitID, err := git.ObjectHashSHA1.FromHex(in.CommitId) if err != nil { - return nil, helper.ErrInvalidArgumentf("cannot parse commit ID: %w", err) + return nil, structerr.NewInvalidArgument("cannot parse commit ID: %w", err) } ancestor, err := quarantineRepo.IsAncestor(ctx, revision.Revision(), commitID.Revision()) @@ -336,7 +335,7 @@ func validateUserMergeToRefRequest(in *gitalypb.UserMergeToRefRequest) error { // Branch or FirstParentRef and updates TargetRef to the merge commit. func (s *Server) UserMergeToRef(ctx context.Context, request *gitalypb.UserMergeToRefRequest) (*gitalypb.UserMergeToRefResponse, error) { if err := validateUserMergeToRefRequest(request); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repoPath, err := s.locator.GetPath(request.Repository) @@ -353,17 +352,17 @@ func (s *Server) UserMergeToRef(ctx context.Context, request *gitalypb.UserMerge oid, err := repo.ResolveRevision(ctx, revision) if err != nil { - return nil, helper.ErrInvalidArgumentf("Invalid merge source") + return nil, structerr.NewInvalidArgument("Invalid merge source") } sourceOID, err := repo.ResolveRevision(ctx, git.Revision(request.SourceSha)) if err != nil { - return nil, helper.ErrInvalidArgumentf("Invalid merge source") + return nil, structerr.NewInvalidArgument("Invalid merge source") } authorDate, err := dateFromProto(request) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } // Resolve the current state of the target reference. We do not care whether it @@ -408,7 +407,7 @@ func (s *Server) UserMergeToRef(ctx context.Context, request *gitalypb.UserMerge ).Error("unable to create merge commit") if errors.Is(err, git2go.ErrInvalidArgument) { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return nil, structerr.NewFailedPrecondition("Failed to create merge commit for source_sha %s and target_sha %s at %s", sourceOID, oid, string(request.TargetRef)) diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go index d77e93821..a821c1f58 100644 --- a/internal/gitaly/service/operations/merge_test.go +++ b/internal/gitaly/service/operations/merge_test.go @@ -21,7 +21,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v15/internal/gitlab" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" @@ -363,7 +362,7 @@ func TestUserMergeBranch_failure(t *testing.T) { Message: []byte("sample-message"), } }, - expectedErr: helper.ErrInvalidArgumentf("empty user"), + expectedErr: structerr.NewInvalidArgument("empty user"), }, { desc: "empty user name", @@ -381,7 +380,7 @@ func TestUserMergeBranch_failure(t *testing.T) { Message: []byte("sample-message"), } }, - expectedErr: helper.ErrInvalidArgumentf("empty user name"), + expectedErr: structerr.NewInvalidArgument("empty user name"), }, { desc: "empty user email", @@ -399,7 +398,7 @@ func TestUserMergeBranch_failure(t *testing.T) { Message: []byte("sample-message"), } }, - expectedErr: helper.ErrInvalidArgumentf("empty user email"), + expectedErr: structerr.NewInvalidArgument("empty user email"), }, { desc: "empty commit", @@ -411,7 +410,7 @@ func TestUserMergeBranch_failure(t *testing.T) { Message: []byte("sample-message"), } }, - expectedErr: helper.ErrInvalidArgumentf("empty commit ID"), + expectedErr: structerr.NewInvalidArgument("empty commit ID"), }, { desc: "empty branch", @@ -423,7 +422,7 @@ func TestUserMergeBranch_failure(t *testing.T) { Message: []byte("sample-message"), } }, - expectedErr: helper.ErrInvalidArgumentf("empty branch name"), + expectedErr: structerr.NewInvalidArgument("empty branch name"), }, { desc: "empty message", @@ -435,7 +434,7 @@ func TestUserMergeBranch_failure(t *testing.T) { Branch: []byte(branchToMerge), } }, - expectedErr: helper.ErrInvalidArgumentf("empty message"), + expectedErr: structerr.NewInvalidArgument("empty message"), }, } diff --git a/internal/gitaly/service/operations/rebase.go b/internal/gitaly/service/operations/rebase.go index e21a27c1d..2010ed8c0 100644 --- a/internal/gitaly/service/operations/rebase.go +++ b/internal/gitaly/service/operations/rebase.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -23,11 +22,11 @@ func (s *Server) UserRebaseConfirmable(stream gitalypb.OperationService_UserReba header := firstRequest.GetHeader() if header == nil { - return helper.ErrInvalidArgumentf("empty UserRebaseConfirmableRequest.Header") + return structerr.NewInvalidArgument("empty UserRebaseConfirmableRequest.Header") } if err := validateUserRebaseConfirmableHeader(header); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } ctx := stream.Context() @@ -45,7 +44,7 @@ func (s *Server) UserRebaseConfirmable(stream gitalypb.OperationService_UserReba branch := git.NewReferenceNameFromBranchName(string(header.Branch)) oldrev, err := git.ObjectHashSHA1.FromHex(header.BranchSha) if err != nil { - return helper.ErrNotFoundf("%w", err) + return structerr.NewNotFound("%w", err) } remoteFetch := rebaseRemoteFetch{header: header} diff --git a/internal/gitaly/service/operations/revert.go b/internal/gitaly/service/operations/revert.go index 1bc28e4b6..c59f34f92 100644 --- a/internal/gitaly/service/operations/revert.go +++ b/internal/gitaly/service/operations/revert.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/remoterepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -18,7 +17,7 @@ import ( //nolint:revive // This is unintentionally missing documentation. func (s *Server) UserRevert(ctx context.Context, req *gitalypb.UserRevertRequest) (*gitalypb.UserRevertResponse, error) { if err := validateCherryPickOrRevertRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository()) @@ -48,7 +47,7 @@ func (s *Server) UserRevert(ctx context.Context, req *gitalypb.UserRevertRequest authorDate, err := dateFromProto(req) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } newrev, err := s.git2goExecutor.Revert(ctx, quarantineRepo, git2go.RevertCommand{ @@ -73,7 +72,7 @@ func (s *Server) UserRevert(ctx context.Context, req *gitalypb.UserRevertRequest CreateTreeErrorCode: gitalypb.UserRevertResponse_EMPTY, }, nil } else if errors.Is(err, git2go.ErrInvalidArgument) { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } else { return nil, structerr.NewInternal("revert command: %w", err) } @@ -87,7 +86,7 @@ func (s *Server) UserRevert(ctx context.Context, req *gitalypb.UserRevertRequest branchCreated = true oldrev = git.ObjectHashSHA1.ZeroOID } else if err != nil { - return nil, helper.ErrInvalidArgumentf("resolve ref: %w", err) + return nil, structerr.NewInvalidArgument("resolve ref: %w", err) } if req.DryRun { @@ -153,7 +152,7 @@ func (s *Server) fetchStartRevision( startRevision, err := remoteRepo.ResolveRevision(ctx, git.Revision(fmt.Sprintf("%s^{commit}", startBranchName))) if err != nil { - return "", helper.ErrInvalidArgumentf("resolve start ref: %w", err) + return "", structerr.NewInvalidArgument("resolve start ref: %w", err) } if req.GetStartRepository() == nil { @@ -171,7 +170,7 @@ func (s *Server) fetchStartRevision( return "", structerr.NewInternal("fetch start: %w", err) } } else if err != nil { - return "", helper.ErrInvalidArgumentf("resolve start: %w", err) + return "", structerr.NewInvalidArgument("resolve start: %w", err) } return startRevision, nil diff --git a/internal/gitaly/service/operations/squash.go b/internal/gitaly/service/operations/squash.go index d08782bce..2d6721142 100644 --- a/internal/gitaly/service/operations/squash.go +++ b/internal/gitaly/service/operations/squash.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -23,7 +22,7 @@ const ( // commit whose single parent is the start revision. func (s *Server) UserSquash(ctx context.Context, req *gitalypb.UserSquashRequest) (*gitalypb.UserSquashResponse, error) { if err := validateUserSquashRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } sha, err := s.userSquash(ctx, req) @@ -122,7 +121,7 @@ func (s *Server) userSquash(ctx context.Context, req *gitalypb.UserSquashRequest commitDate, err := dateFromProto(req) if err != nil { - return "", helper.ErrInvalidArgumentf("%w", err) + return "", structerr.NewInvalidArgument("%w", err) } message := string(req.GetCommitMessage()) diff --git a/internal/gitaly/service/operations/squash_test.go b/internal/gitaly/service/operations/squash_test.go index daf8aac24..160f6304d 100644 --- a/internal/gitaly/service/operations/squash_test.go +++ b/internal/gitaly/service/operations/squash_test.go @@ -15,7 +15,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" @@ -719,7 +718,7 @@ func TestUserSquash_gitError(t *testing.T) { EndSha: endSha, }, expectedErr: errWithDetails(t, - helper.ErrInvalidArgumentf("resolving start revision: reference not found"), + structerr.NewInvalidArgument("resolving start revision: reference not found"), &gitalypb.UserSquashError{ Error: &gitalypb.UserSquashError_ResolveRevision{ ResolveRevision: &gitalypb.ResolveRevisionError{ @@ -740,7 +739,7 @@ func TestUserSquash_gitError(t *testing.T) { EndSha: "doesntexisting", }, expectedErr: errWithDetails(t, - helper.ErrInvalidArgumentf("resolving end revision: reference not found"), + structerr.NewInvalidArgument("resolving end revision: reference not found"), &gitalypb.UserSquashError{ Error: &gitalypb.UserSquashError_ResolveRevision{ ResolveRevision: &gitalypb.ResolveRevisionError{ @@ -760,7 +759,7 @@ func TestUserSquash_gitError(t *testing.T) { StartSha: startSha, EndSha: endSha, }, - expectedErr: helper.ErrInvalidArgumentf("empty user name"), + expectedErr: structerr.NewInvalidArgument("empty user name"), }, { desc: "author has no name set", @@ -772,7 +771,7 @@ func TestUserSquash_gitError(t *testing.T) { StartSha: startSha, EndSha: endSha, }, - expectedErr: helper.ErrInvalidArgumentf("empty author name"), + expectedErr: structerr.NewInvalidArgument("empty author name"), }, { desc: "author has no email set", @@ -784,7 +783,7 @@ func TestUserSquash_gitError(t *testing.T) { StartSha: startSha, EndSha: endSha, }, - expectedErr: helper.ErrInvalidArgumentf("empty author email"), + expectedErr: structerr.NewInvalidArgument("empty author email"), }, } diff --git a/internal/gitaly/service/operations/submodules.go b/internal/gitaly/service/operations/submodules.go index a91c09004..1fe56c500 100644 --- a/internal/gitaly/service/operations/submodules.go +++ b/internal/gitaly/service/operations/submodules.go @@ -12,7 +12,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -20,7 +19,7 @@ import ( //nolint:revive // This is unintentionally missing documentation. func (s *Server) UserUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpdateSubmoduleRequest) (*gitalypb.UserUpdateSubmoduleResponse, error) { if err := validateUserUpdateSubmoduleRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return s.userUpdateSubmodule(ctx, req) @@ -79,7 +78,7 @@ func (s *Server) userUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpda branchOID, err := quarantineRepo.ResolveRevision(ctx, referenceName.Revision()) if err != nil { if errors.Is(err, git.ErrReferenceNotFound) { - return nil, helper.ErrInvalidArgumentf("Cannot find branch") + return nil, structerr.NewInvalidArgument("Cannot find branch") } return nil, structerr.NewInternal("resolving revision: %w", err) } @@ -91,7 +90,7 @@ func (s *Server) userUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpda authorDate, err := dateFromProto(req) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } result, err := s.git2goExecutor.Submodule(ctx, quarantineRepo, git2go.SubmoduleCommand{ @@ -138,7 +137,7 @@ func (s *Server) userUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpda commitID, err := git.ObjectHashSHA1.FromHex(result.CommitID) if err != nil { - return nil, helper.ErrInvalidArgumentf("cannot parse commit ID: %w", err) + return nil, structerr.NewInvalidArgument("cannot parse commit ID: %w", err) } if err := s.updateReferenceWithHooks( diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go index f7d1164c3..437f33d67 100644 --- a/internal/gitaly/service/operations/tags.go +++ b/internal/gitaly/service/operations/tags.go @@ -14,7 +14,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -35,7 +34,7 @@ func validateUserDeleteTagRequest(in *gitalypb.UserDeleteTagRequest) error { //nolint:revive // This is unintentionally missing documentation. func (s *Server) UserDeleteTag(ctx context.Context, req *gitalypb.UserDeleteTagRequest) (*gitalypb.UserDeleteTagResponse, error) { if err := validateUserDeleteTagRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } referenceName := git.ReferenceName(fmt.Sprintf("refs/tags/%s", req.TagName)) @@ -114,7 +113,7 @@ func validateUserCreateTag(req *gitalypb.UserCreateTagRequest) error { //nolint:revive // This is unintentionally missing documentation. func (s *Server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagRequest) (*gitalypb.UserCreateTagResponse, error) { if err := validateUserCreateTag(req); err != nil { - return nil, helper.ErrInvalidArgumentf("validating request: %w", err) + return nil, structerr.NewInvalidArgument("validating request: %w", err) } targetRevision := git.Revision(req.TargetRevision) @@ -122,7 +121,7 @@ func (s *Server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagR committerTime, err := dateFromProto(req) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository()) @@ -289,7 +288,7 @@ func (s *Server) createTag( var MktagError localrepo.MktagError if errors.As(err, &MktagError) { - return nil, "", helper.ErrNotFoundf("Gitlab::Git::CommitError: %s", err.Error()) + return nil, "", structerr.NewNotFound("Gitlab::Git::CommitError: %s", err.Error()) } return nil, "", structerr.NewInternal("writing tag: %w", err) } diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go index 9c9ec593f..b2aac01dd 100644 --- a/internal/gitaly/service/operations/tags_test.go +++ b/internal/gitaly/service/operations/tags_test.go @@ -14,7 +14,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" @@ -596,7 +595,7 @@ func TestUserCreateTag_message(t *testing.T) { { desc: "error: contains null byte", message: "\000", - expectedErr: helper.ErrInvalidArgumentf("validating request: tag message contains NUL byte"), + expectedErr: structerr.NewInvalidArgument("validating request: tag message contains NUL byte"), }, { desc: "annotated: some control characters", @@ -1267,7 +1266,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { tagName: "shiny-new-tag", targetRevision: "", user: gittest.TestUser, - expectedErr: helper.ErrInvalidArgumentf("validating request: empty target revision"), + expectedErr: structerr.NewInvalidArgument("validating request: empty target revision"), }, { desc: "empty user", @@ -1275,7 +1274,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { tagName: "shiny-new-tag", targetRevision: "main", user: nil, - expectedErr: helper.ErrInvalidArgumentf("validating request: empty user"), + expectedErr: structerr.NewInvalidArgument("validating request: empty user"), }, { desc: "empty starting point", @@ -1283,7 +1282,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { tagName: "new-tag", targetRevision: "", user: gittest.TestUser, - expectedErr: helper.ErrInvalidArgumentf("validating request: empty target revision"), + expectedErr: structerr.NewInvalidArgument("validating request: empty target revision"), }, { desc: "non-existing starting point", @@ -1299,7 +1298,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { tagName: "a tag", targetRevision: "main", user: gittest.TestUser, - expectedErr: helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"), + expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"), }, { desc: "space in annotated tag name", @@ -1308,7 +1307,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { targetRevision: "main", message: "a message", user: gittest.TestUser, - expectedErr: helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"), + expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"), }, { desc: "newline in lightweight tag name", @@ -1316,7 +1315,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { tagName: "a\ntag", targetRevision: "main", user: gittest.TestUser, - expectedErr: helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"), + expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"), }, { desc: "newline in annotated tag name", @@ -1325,7 +1324,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { targetRevision: "main", message: "a message", user: gittest.TestUser, - expectedErr: helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"), + expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"), }, { desc: "injection in lightweight tag name", @@ -1333,7 +1332,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { tagName: injectedTag, targetRevision: "main", user: gittest.TestUser, - expectedErr: helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"), + expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"), }, { desc: "injection in annotated tag name", @@ -1342,7 +1341,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) { targetRevision: "main", message: "a message", user: gittest.TestUser, - expectedErr: helper.ErrInvalidArgumentf("validating request: invalid tag name: revision can't contain whitespace"), + expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/ref/branches.go b/internal/gitaly/service/ref/branches.go index 383bd5535..02d47a3e2 100644 --- a/internal/gitaly/service/ref/branches.go +++ b/internal/gitaly/service/ref/branches.go @@ -6,17 +6,17 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest) (*gitalypb.FindBranchResponse, error) { repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if len(req.GetName()) == 0 { - return nil, helper.ErrInvalidArgumentf("Branch name cannot be empty") + return nil, structerr.NewInvalidArgument("Branch name cannot be empty") } repo := s.localrepo(repository) @@ -36,7 +36,7 @@ func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest branch, ok := branchName.Branch() if !ok { - return nil, helper.ErrInvalidArgumentf("reference is not a branch") + return nil, structerr.NewInvalidArgument("reference is not a branch") } return &gitalypb.FindBranchResponse{ diff --git a/internal/gitaly/service/ref/delete_refs.go b/internal/gitaly/service/ref/delete_refs.go index 290ebd67b..d8e1c867e 100644 --- a/internal/gitaly/service/ref/delete_refs.go +++ b/internal/gitaly/service/ref/delete_refs.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" @@ -20,7 +19,7 @@ import ( func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest) (_ *gitalypb.DeleteRefsResponse, returnedErr error) { if err := validateDeleteRefRequest(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(in.GetRepository()) @@ -33,7 +32,7 @@ func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest) updater, err := updateref.New(ctx, repo, updateref.WithNoDeref()) if err != nil { if errors.Is(err, git.ErrInvalidArg) { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return nil, structerr.NewInternal("%w", err) } diff --git a/internal/gitaly/service/ref/find_all_tags.go b/internal/gitaly/service/ref/find_all_tags.go index 9b78a9d44..35b709c0d 100644 --- a/internal/gitaly/service/ref/find_all_tags.go +++ b/internal/gitaly/service/ref/find_all_tags.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -22,12 +21,12 @@ func (s *server) FindAllTags(in *gitalypb.FindAllTagsRequest, stream gitalypb.Re ctx := stream.Context() if err := s.validateFindAllTagsRequest(in); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } sortField, err := getTagSortField(in.GetSortBy()) if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } opts := buildPaginationOpts(ctx, in.GetPaginationParams()) @@ -152,7 +151,7 @@ func (s *server) findAllTags(ctx context.Context, repo *localrepo.Repo, sortFiel } if !pastPageToken { - return helper.ErrInvalidArgumentf("could not find page token") + return structerr.NewInvalidArgument("could not find page token") } if err := catfileObjectsIter.Err(); err != nil { diff --git a/internal/gitaly/service/ref/find_all_tags_test.go b/internal/gitaly/service/ref/find_all_tags_test.go index a1532e878..a1c118140 100644 --- a/internal/gitaly/service/ref/find_all_tags_test.go +++ b/internal/gitaly/service/ref/find_all_tags_test.go @@ -19,6 +19,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" + "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" "google.golang.org/grpc/codes" @@ -547,14 +548,14 @@ func TestFindAllTags_pagination(t *testing.T) { desc: "with page token only", paginationParams: &gitalypb.PaginationParameter{PageToken: "refs/tags/v1.1.0"}, expected: func(context.Context) ([]string, error) { - return nil, helper.ErrInvalidArgumentf("could not find page token") + return nil, structerr.NewInvalidArgument("could not find page token") }, }, { desc: "with invalid page token", paginationParams: &gitalypb.PaginationParameter{Limit: 3, PageToken: "refs/tags/invalid"}, expected: func(ctx context.Context) ([]string, error) { - return nil, helper.ErrInvalidArgumentf("could not find page token") + return nil, structerr.NewInvalidArgument("could not find page token") }, }, } { diff --git a/internal/gitaly/service/ref/find_refs_by_oid.go b/internal/gitaly/service/ref/find_refs_by_oid.go index e02880c6d..de9ddafd7 100644 --- a/internal/gitaly/service/ref/find_refs_by_oid.go +++ b/internal/gitaly/service/ref/find_refs_by_oid.go @@ -7,14 +7,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) FindRefsByOID(ctx context.Context, in *gitalypb.FindRefsByOIDRequest) (*gitalypb.FindRefsByOIDResponse, error) { if err := validateFindRefsReq(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(in.GetRepository()) @@ -42,7 +41,7 @@ func (s *server) FindRefsByOID(ctx context.Context, in *gitalypb.FindRefsByOIDRe // git uses exit status 129 to indicate errors in command line usage // https://www.git-scm.com/docs/api-error-handling if strings.Contains(err.Error(), "exit status 129") { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return nil, structerr.NewInternal("%w", err) } diff --git a/internal/gitaly/service/ref/find_refs_by_oid_test.go b/internal/gitaly/service/ref/find_refs_by_oid_test.go index a2d103a72..735118c1a 100644 --- a/internal/gitaly/service/ref/find_refs_by_oid_test.go +++ b/internal/gitaly/service/ref/find_refs_by_oid_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" "google.golang.org/grpc/codes" @@ -156,7 +156,7 @@ func TestFindRefsByOID_failure(t *testing.T) { return &gitalypb.FindRefsByOIDRequest{ Repository: repo, Oid: oid.String(), - }, helper.ErrNotFoundf("GetRepoPath: not a git repository: %q", repoPath) + }, structerr.NewNotFound("GetRepoPath: not a git repository: %q", repoPath) }, }, { @@ -171,7 +171,7 @@ func TestFindRefsByOID_failure(t *testing.T) { return &gitalypb.FindRefsByOIDRequest{ Repository: repo, Oid: oid.String(), - }, helper.ErrNotFoundf("GetRepoPath: not a git repository: %q", repoPath) + }, structerr.NewNotFound("GetRepoPath: not a git repository: %q", repoPath) }, }, { @@ -200,7 +200,7 @@ func TestFindRefsByOID_failure(t *testing.T) { return &gitalypb.FindRefsByOIDRequest{ Repository: repo, Oid: oid.String()[:2], - }, helper.ErrInvalidArgumentf("for-each-ref pipeline command: exit status 129") + }, structerr.NewInvalidArgument("for-each-ref pipeline command: exit status 129") }, }, } diff --git a/internal/gitaly/service/ref/find_tag.go b/internal/gitaly/service/ref/find_tag.go index c93828b1f..a859dff19 100644 --- a/internal/gitaly/service/ref/find_tag.go +++ b/internal/gitaly/service/ref/find_tag.go @@ -10,14 +10,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) FindTag(ctx context.Context, in *gitalypb.FindTagRequest) (*gitalypb.FindTagResponse, error) { if err := s.validateFindTagRequest(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(in.GetRepository()) diff --git a/internal/gitaly/service/ref/list_refs.go b/internal/gitaly/service/ref/list_refs.go index 777a3ad14..bffbc8ccd 100644 --- a/internal/gitaly/service/ref/list_refs.go +++ b/internal/gitaly/service/ref/list_refs.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/lines" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -15,7 +14,7 @@ import ( func (s *server) ListRefs(in *gitalypb.ListRefsRequest, stream gitalypb.RefService_ListRefsServer) error { if err := validateListRefsRequest(in); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(in.GetRepository()) diff --git a/internal/gitaly/service/ref/pack_refs.go b/internal/gitaly/service/ref/pack_refs.go index b8558ae1f..ea74ce103 100644 --- a/internal/gitaly/service/ref/pack_refs.go +++ b/internal/gitaly/service/ref/pack_refs.go @@ -6,14 +6,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/repository" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) PackRefs(ctx context.Context, in *gitalypb.PackRefsRequest) (*gitalypb.PackRefsResponse, error) { if err := validatePackRefsRequest(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if err := s.packRefs(ctx, in.GetRepository()); err != nil { diff --git a/internal/gitaly/service/ref/refexists.go b/internal/gitaly/service/ref/refexists.go index 109f8c456..c9b421dca 100644 --- a/internal/gitaly/service/ref/refexists.go +++ b/internal/gitaly/service/ref/refexists.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/command" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -15,13 +14,13 @@ import ( // RefExists returns true if the given reference exists. The ref must start with the string `ref/` func (s *server) RefExists(ctx context.Context, in *gitalypb.RefExistsRequest) (*gitalypb.RefExistsResponse, error) { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } ref := string(in.Ref) if !isValidRefName(ref) { - return nil, helper.ErrInvalidArgumentf("invalid refname") + return nil, structerr.NewInvalidArgument("invalid refname") } exists, err := s.refExists(ctx, in.Repository, ref) diff --git a/internal/gitaly/service/ref/refnames.go b/internal/gitaly/service/ref/refnames.go index fe046f096..0ca444f58 100644 --- a/internal/gitaly/service/ref/refnames.go +++ b/internal/gitaly/service/ref/refnames.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -17,7 +16,7 @@ import ( // FindAllBranchNames creates a stream of ref names for all branches in the given repository func (s *server) FindAllBranchNames(in *gitalypb.FindAllBranchNamesRequest, stream gitalypb.RefService_FindAllBranchNamesServer) error { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } chunker := chunk.New(&findAllBranchNamesSender{stream: stream}) @@ -46,7 +45,7 @@ func (ts *findAllBranchNamesSender) Send() error { // FindAllTagNames creates a stream of ref names for all tags in the given repository func (s *server) FindAllTagNames(in *gitalypb.FindAllTagNamesRequest, stream gitalypb.RefService_FindAllTagNamesServer) error { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } chunker := chunk.New(&findAllTagNamesSender{stream: stream}) diff --git a/internal/gitaly/service/ref/refnames_containing.go b/internal/gitaly/service/ref/refnames_containing.go index d751425e2..97395c124 100644 --- a/internal/gitaly/service/ref/refnames_containing.go +++ b/internal/gitaly/service/ref/refnames_containing.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -18,10 +17,10 @@ import ( // which contain the SHA1 passed as argument func (s *server) ListBranchNamesContainingCommit(in *gitalypb.ListBranchNamesContainingCommitRequest, stream gitalypb.RefService_ListBranchNamesContainingCommitServer) error { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := git.ObjectHashSHA1.ValidateHex(in.GetCommitId()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } chunker := chunk.New(&branchNamesContainingCommitSender{stream: stream}) @@ -64,10 +63,10 @@ func (bs *branchNamesContainingCommitSender) Send() error { // which contain the SHA1 passed as argument func (s *server) ListTagNamesContainingCommit(in *gitalypb.ListTagNamesContainingCommitRequest, stream gitalypb.RefService_ListTagNamesContainingCommitServer) error { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := git.ObjectHashSHA1.ValidateHex(in.GetCommitId()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } chunker := chunk.New(&tagNamesContainingCommitSender{stream: stream}) diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go index de7d5aa88..0a36f5e3b 100644 --- a/internal/gitaly/service/ref/refs.go +++ b/internal/gitaly/service/ref/refs.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/lines" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -73,7 +72,7 @@ func (s *server) findRefs(ctx context.Context, writer lines.Sender, repo git.Rep func (s *server) FindDefaultBranchName(ctx context.Context, in *gitalypb.FindDefaultBranchNameRequest) (*gitalypb.FindDefaultBranchNameResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) @@ -101,7 +100,7 @@ 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 := service.ValidateRepository(in.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.findLocalBranches(in, stream); err != nil { return structerr.NewInternal("%w", err) @@ -137,7 +136,7 @@ func (s *server) findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream func (s *server) FindAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefService_FindAllBranchesServer) error { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.findAllBranches(in, stream); err != nil { return structerr.NewInternal("%w", err) diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go index 7dba0a181..160fd8620 100644 --- a/internal/gitaly/service/ref/refs_test.go +++ b/internal/gitaly/service/ref/refs_test.go @@ -777,7 +777,7 @@ func TestInvalidFindAllBranchesRequest(t *testing.T) { { description: "Empty request", request: &gitalypb.FindAllBranchesRequest{}, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -790,7 +790,7 @@ func TestInvalidFindAllBranchesRequest(t *testing.T) { RelativePath: "repo", }, }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "creating object reader: GetStorageByName: no such storage: \"fake\"", "repo scoped: invalid Repository", )), diff --git a/internal/gitaly/service/ref/remote_branches.go b/internal/gitaly/service/ref/remote_branches.go index 121947d08..86e715e1d 100644 --- a/internal/gitaly/service/ref/remote_branches.go +++ b/internal/gitaly/service/ref/remote_branches.go @@ -7,14 +7,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) FindAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesRequest, stream gitalypb.RefService_FindAllRemoteBranchesServer) error { if err := validateFindAllRemoteBranchesRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.findAllRemoteBranches(req, stream); err != nil { diff --git a/internal/gitaly/service/ref/tag_messages.go b/internal/gitaly/service/ref/tag_messages.go index 922191ccb..47b4dcccd 100644 --- a/internal/gitaly/service/ref/tag_messages.go +++ b/internal/gitaly/service/ref/tag_messages.go @@ -8,7 +8,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -16,7 +15,7 @@ import ( func (s *server) GetTagMessages(request *gitalypb.GetTagMessagesRequest, stream gitalypb.RefService_GetTagMessagesServer) error { if err := validateGetTagMessagesRequest(request); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.getAndStreamTagMessages(request, stream); err != nil { return structerr.NewInternal("%w", err) diff --git a/internal/gitaly/service/ref/tag_signatures.go b/internal/gitaly/service/ref/tag_signatures.go index 0c036b1de..1531a92b9 100644 --- a/internal/gitaly/service/ref/tag_signatures.go +++ b/internal/gitaly/service/ref/tag_signatures.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -35,7 +34,7 @@ func verifyGetTagSignaturesRequest(req *gitalypb.GetTagSignaturesRequest) error func (s *server) GetTagSignatures(req *gitalypb.GetTagSignaturesRequest, stream gitalypb.RefService_GetTagSignaturesServer) error { if err := verifyGetTagSignaturesRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } ctx := stream.Context() diff --git a/internal/gitaly/service/remote/find_remote_repository.go b/internal/gitaly/service/remote/find_remote_repository.go index 3b713bc39..bd6f54e82 100644 --- a/internal/gitaly/service/remote/find_remote_repository.go +++ b/internal/gitaly/service/remote/find_remote_repository.go @@ -6,14 +6,13 @@ import ( "io" "gitlab.com/gitlab-org/gitaly/v15/internal/git" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) FindRemoteRepository(ctx context.Context, req *gitalypb.FindRemoteRepositoryRequest) (*gitalypb.FindRemoteRepositoryResponse, error) { if req.GetRemote() == "" { - return nil, helper.ErrInvalidArgumentf("empty remote can't be checked.") + return nil, structerr.NewInvalidArgument("empty remote can't be checked.") } cmd, err := s.gitCmdFactory.NewWithoutRepo(ctx, diff --git a/internal/gitaly/service/remote/find_remote_root_ref.go b/internal/gitaly/service/remote/find_remote_root_ref.go index 348cd3cd9..990dc63b3 100644 --- a/internal/gitaly/service/remote/find_remote_root_ref.go +++ b/internal/gitaly/service/remote/find_remote_root_ref.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/command" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -23,7 +22,7 @@ func (s *server) findRemoteRootRefCmd(ctx context.Context, request *gitalypb.Fin if resolvedAddress := request.GetResolvedAddress(); resolvedAddress != "" { modifiedURL, resolveConfig, err := git.GetURLAndResolveConfig(remoteURL, resolvedAddress) if err != nil { - return nil, helper.ErrInvalidArgumentf("couldn't get curloptResolve config: %w", err) + return nil, structerr.NewInvalidArgument("couldn't get curloptResolve config: %w", err) } remoteURL = modifiedURL @@ -70,7 +69,7 @@ func (s *server) findRemoteRootRef(ctx context.Context, request *gitalypb.FindRe if strings.HasPrefix(line, headPrefix) { rootRef := strings.TrimPrefix(line, headPrefix) if rootRef == "(unknown)" { - return "", helper.ErrNotFoundf("no remote HEAD found") + return "", structerr.NewNotFound("no remote HEAD found") } return rootRef, nil } @@ -84,16 +83,16 @@ func (s *server) findRemoteRootRef(ctx context.Context, request *gitalypb.FindRe return "", err } - return "", helper.ErrNotFoundf("couldn't query the remote HEAD") + return "", structerr.NewNotFound("couldn't query the remote HEAD") } // FindRemoteRootRef queries the remote to determine its HEAD func (s *server) FindRemoteRootRef(ctx context.Context, in *gitalypb.FindRemoteRootRefRequest) (*gitalypb.FindRemoteRootRefResponse, error) { if in.GetRemoteUrl() == "" { - return nil, helper.ErrInvalidArgumentf("missing remote URL") + return nil, structerr.NewInvalidArgument("missing remote URL") } if err := service.ValidateRepository(in.GetRepository()); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } ref, err := s.findRemoteRootRef(ctx, in) diff --git a/internal/gitaly/service/remote/find_remote_root_ref_test.go b/internal/gitaly/service/remote/find_remote_root_ref_test.go index b021e40ad..61d5f82bb 100644 --- a/internal/gitaly/service/remote/find_remote_root_ref_test.go +++ b/internal/gitaly/service/remote/find_remote_root_ref_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -92,7 +92,7 @@ func TestFindRemoteRootRefFailedDueToValidation(t *testing.T) { Repository: &gitalypb.Repository{StorageName: "fake", RelativePath: "path"}, RemoteUrl: "remote-url", }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( `GetStorageByName: no such storage: "fake"`, "repo scoped: invalid Repository", )), @@ -102,7 +102,7 @@ func TestFindRemoteRootRefFailedDueToValidation(t *testing.T) { request: &gitalypb.FindRemoteRootRefRequest{ RemoteUrl: "remote-url", }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -112,7 +112,7 @@ func TestFindRemoteRootRefFailedDueToValidation(t *testing.T) { request: &gitalypb.FindRemoteRootRefRequest{ Repository: repo, }, - expectedErr: helper.ErrInvalidArgumentf("missing remote URL"), + expectedErr: structerr.NewInvalidArgument("missing remote URL"), }, } @@ -215,7 +215,7 @@ func TestServer_findRemoteRootRefCmd(t *testing.T) { ResolvedAddress: "foo/bar", Repository: repo, }, - expectedErr: helper.ErrInvalidArgumentf("couldn't get curloptResolve config: %w", fmt.Errorf("resolved address has invalid IPv4/IPv6 address")), + expectedErr: structerr.NewInvalidArgument("couldn't get curloptResolve config: %w", fmt.Errorf("resolved address has invalid IPv4/IPv6 address")), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/remote/update_remote_mirror.go b/internal/gitaly/service/remote/update_remote_mirror.go index fd8b975c5..63e87b1e6 100644 --- a/internal/gitaly/service/remote/update_remote_mirror.go +++ b/internal/gitaly/service/remote/update_remote_mirror.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -32,7 +31,7 @@ func (s *server) UpdateRemoteMirror(stream gitalypb.RemoteService_UpdateRemoteMi } if err = validateUpdateRemoteMirrorRequest(stream.Context(), firstRequest); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.updateRemoteMirror(stream, firstRequest); err != nil { diff --git a/internal/gitaly/service/repository/apply_gitattributes.go b/internal/gitaly/service/repository/apply_gitattributes.go index 284fb50d1..973d59581 100644 --- a/internal/gitaly/service/repository/apply_gitattributes.go +++ b/internal/gitaly/service/repository/apply_gitattributes.go @@ -14,8 +14,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/safe" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/txinfo" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -30,7 +30,7 @@ func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, o _, err := repo.ResolveRevision(ctx, git.Revision(revision)+"^{commit}") if err != nil { if errors.Is(err, git.ErrReferenceNotFound) { - return helper.ErrInvalidArgumentf("revision does not exist") + return structerr.NewInvalidArgument("revision does not exist") } return err @@ -131,7 +131,7 @@ func (s *server) vote(ctx context.Context, oid git.ObjectID, phase voting.Phase) func (s *server) ApplyGitattributes(ctx context.Context, in *gitalypb.ApplyGitattributesRequest) (*gitalypb.ApplyGitattributesResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) repoPath, err := s.locator.GetRepoPath(repo) @@ -140,7 +140,7 @@ func (s *server) ApplyGitattributes(ctx context.Context, in *gitalypb.ApplyGitat } if err := git.ValidateRevision(in.GetRevision()); err != nil { - return nil, helper.ErrInvalidArgumentf("revision: %w", err) + return nil, structerr.NewInvalidArgument("revision: %w", err) } objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) diff --git a/internal/gitaly/service/repository/apply_gitattributes_test.go b/internal/gitaly/service/repository/apply_gitattributes_test.go index 7d5885bb2..d1c048021 100644 --- a/internal/gitaly/service/repository/apply_gitattributes_test.go +++ b/internal/gitaly/service/repository/apply_gitattributes_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/backchannel" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" @@ -230,7 +229,7 @@ func TestApplyGitattributes_failure(t *testing.T) { desc: "no repository provided", repo: nil, revision: nil, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -242,7 +241,7 @@ func TestApplyGitattributes_failure(t *testing.T) { StorageName: "foo", }, revision: []byte("master"), - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( `GetStorageByName: no such storage: "foo"`, "repo scoped: invalid Repository", )), @@ -253,7 +252,7 @@ func TestApplyGitattributes_failure(t *testing.T) { RelativePath: repo.GetRelativePath(), }, revision: []byte("master"), - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty StorageName", "repo scoped: invalid Repository", )), @@ -265,7 +264,7 @@ func TestApplyGitattributes_failure(t *testing.T) { RelativePath: "bar", }, revision: []byte("master"), - expectedErr: helper.ErrNotFoundf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewNotFound(testhelper.GitalyOrPraefect( `GetRepoPath: not a git repository: "`+cfg.Storages[0].Path+`/bar"`, `mutator call: route repository mutator: get repository id: repository "default"/"bar" not found`, )), @@ -274,19 +273,19 @@ func TestApplyGitattributes_failure(t *testing.T) { desc: "no revision provided", repo: repo, revision: []byte(""), - expectedErr: helper.ErrInvalidArgumentf("revision: empty revision"), + expectedErr: structerr.NewInvalidArgument("revision: empty revision"), }, { desc: "unknown revision", repo: repo, revision: []byte("not-existing-ref"), - expectedErr: helper.ErrInvalidArgumentf("revision does not exist"), + expectedErr: structerr.NewInvalidArgument("revision does not exist"), }, { desc: "invalid revision", repo: repo, revision: []byte("--output=/meow"), - expectedErr: helper.ErrInvalidArgumentf("revision: revision can't start with '-'"), + expectedErr: structerr.NewInvalidArgument("revision: revision can't start with '-'"), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go index 209506120..7e3ffeae5 100644 --- a/internal/gitaly/service/repository/archive.go +++ b/internal/gitaly/service/repository/archive.go @@ -16,7 +16,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/smudge" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/log" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -38,7 +37,7 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo ctx := stream.Context() repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } compressArgs, format := parseArchiveFormat(in.GetFormat()) repo := s.localrepo(repository) @@ -50,14 +49,14 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo path, err := storage.ValidateRelativePath(repoRoot, string(in.GetPath())) if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } exclude := make([]string, len(in.GetExclude())) for i, ex := range in.GetExclude() { exclude[i], err = storage.ValidateRelativePath(repoRoot, string(ex)) if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } } @@ -74,7 +73,7 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo pathSlash := path + string(os.PathSeparator) for i := range exclude { if !strings.HasPrefix(exclude[i], pathSlash) { - return helper.ErrInvalidArgumentf("invalid exclude: %q is not a subdirectory of %q", exclude[i], path) + return structerr.NewInvalidArgument("invalid exclude: %q is not a subdirectory of %q", exclude[i], path) } exclude[i] = exclude[i][len(pathSlash):] @@ -115,11 +114,11 @@ func parseArchiveFormat(format gitalypb.GetArchiveRequest_Format) ([]string, str func validateGetArchiveRequest(in *gitalypb.GetArchiveRequest, format string) error { if err := git.ValidateRevision([]byte(in.GetCommitId())); err != nil { - return helper.ErrInvalidArgumentf("invalid commitId: %w", err) + return structerr.NewInvalidArgument("invalid commitId: %w", err) } if len(format) == 0 { - return helper.ErrInvalidArgumentf("invalid format") + return structerr.NewInvalidArgument("invalid format") } return nil diff --git a/internal/gitaly/service/repository/backup_custom_hooks.go b/internal/gitaly/service/repository/backup_custom_hooks.go index df89e47a5..c9dc778e7 100644 --- a/internal/gitaly/service/repository/backup_custom_hooks.go +++ b/internal/gitaly/service/repository/backup_custom_hooks.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/command" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -17,7 +16,7 @@ const customHooksDir = "custom_hooks" func (s *server) BackupCustomHooks(in *gitalypb.BackupCustomHooksRequest, stream gitalypb.RepositoryService_BackupCustomHooksServer) error { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repoPath, err := s.locator.GetPath(in.Repository) if err != nil { diff --git a/internal/gitaly/service/repository/calculate_checksum.go b/internal/gitaly/service/repository/calculate_checksum.go index a1f73abba..371fa56f9 100644 --- a/internal/gitaly/service/repository/calculate_checksum.go +++ b/internal/gitaly/service/repository/calculate_checksum.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -17,7 +16,7 @@ import ( func (s *server) CalculateChecksum(ctx context.Context, in *gitalypb.CalculateChecksumRequest) (*gitalypb.CalculateChecksumResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := repository repoPath, err := s.locator.GetRepoPath(repo) diff --git a/internal/gitaly/service/repository/cleanup.go b/internal/gitaly/service/repository/cleanup.go index 58cb56017..1557844fd 100644 --- a/internal/gitaly/service/repository/cleanup.go +++ b/internal/gitaly/service/repository/cleanup.go @@ -5,14 +5,14 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/housekeeping" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) Cleanup(ctx context.Context, in *gitalypb.CleanupRequest) (*gitalypb.CleanupResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(in.GetRepository()) diff --git a/internal/gitaly/service/repository/commit_graph.go b/internal/gitaly/service/repository/commit_graph.go index 1880d0171..699884e71 100644 --- a/internal/gitaly/service/repository/commit_graph.go +++ b/internal/gitaly/service/repository/commit_graph.go @@ -5,7 +5,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/housekeeping" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -17,13 +16,13 @@ func (s *server) WriteCommitGraph( ) (*gitalypb.WriteCommitGraphResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) if in.GetSplitStrategy() != gitalypb.WriteCommitGraphRequest_SizeMultiple { - return nil, helper.ErrInvalidArgumentf("unsupported split strategy: %v", in.GetSplitStrategy()) + return nil, structerr.NewInvalidArgument("unsupported split strategy: %v", in.GetSplitStrategy()) } writeCommitGraphCfg, err := housekeeping.WriteCommitGraphConfigForRepository(ctx, repo) diff --git a/internal/gitaly/service/repository/config.go b/internal/gitaly/service/repository/config.go index 680fd3efe..d577d3e33 100644 --- a/internal/gitaly/service/repository/config.go +++ b/internal/gitaly/service/repository/config.go @@ -6,7 +6,6 @@ import ( "path/filepath" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -19,7 +18,7 @@ func (s *server) GetConfig( ) error { repository := request.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repoPath, err := s.locator.GetPath(repository) if err != nil { @@ -31,7 +30,7 @@ func (s *server) GetConfig( gitconfig, err := os.Open(configPath) if err != nil { if os.IsNotExist(err) { - return helper.ErrNotFoundf("opening gitconfig: %w", err) + return structerr.NewNotFound("opening gitconfig: %w", err) } return structerr.NewInternal("opening gitconfig: %w", err) } diff --git a/internal/gitaly/service/repository/create_bundle.go b/internal/gitaly/service/repository/create_bundle.go index 1acfa22d7..5d70babdf 100644 --- a/internal/gitaly/service/repository/create_bundle.go +++ b/internal/gitaly/service/repository/create_bundle.go @@ -5,7 +5,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -14,7 +13,7 @@ import ( func (s *server) CreateBundle(req *gitalypb.CreateBundleRequest, stream gitalypb.RepositoryService_CreateBundleServer) error { repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("CreateBundle: %w", err) + return structerr.NewInvalidArgument("CreateBundle: %w", err) } ctx := stream.Context() diff --git a/internal/gitaly/service/repository/create_bundle_from_ref_list.go b/internal/gitaly/service/repository/create_bundle_from_ref_list.go index c5921825e..cc4fd935c 100644 --- a/internal/gitaly/service/repository/create_bundle_from_ref_list.go +++ b/internal/gitaly/service/repository/create_bundle_from_ref_list.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/command" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -21,7 +20,7 @@ func (s *server) CreateBundleFromRefList(stream gitalypb.RepositoryService_Creat repository := firstRequest.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } ctx := stream.Context() diff --git a/internal/gitaly/service/repository/create_fork.go b/internal/gitaly/service/repository/create_fork.go index b182e65f4..f4c297db9 100644 --- a/internal/gitaly/service/repository/create_fork.go +++ b/internal/gitaly/service/repository/create_fork.go @@ -8,7 +8,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -18,10 +17,10 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest sourceRepository := req.SourceRepository if sourceRepository == nil { - return nil, helper.ErrInvalidArgumentf("empty SourceRepository") + return nil, structerr.NewInvalidArgument("empty SourceRepository") } if err := service.ValidateRepository(req.GetRepository()); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if err := s.createRepository(ctx, targetRepository, func(repo *gitalypb.Repository) error { diff --git a/internal/gitaly/service/repository/create_repository.go b/internal/gitaly/service/repository/create_repository.go index d78f142c2..1c3722cf3 100644 --- a/internal/gitaly/service/repository/create_repository.go +++ b/internal/gitaly/service/repository/create_repository.go @@ -5,7 +5,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -13,12 +12,12 @@ import ( func (s *server) CreateRepository(ctx context.Context, req *gitalypb.CreateRepositoryRequest) (*gitalypb.CreateRepositoryResponse, error) { repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } hash, err := git.ObjectHashByProto(req.GetObjectFormat()) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if err := s.createRepository( diff --git a/internal/gitaly/service/repository/create_repository_from_bundle.go b/internal/gitaly/service/repository/create_repository_from_bundle.go index 65a16dfd5..714f4f5d7 100644 --- a/internal/gitaly/service/repository/create_repository_from_bundle.go +++ b/internal/gitaly/service/repository/create_repository_from_bundle.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/tempdir" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -25,7 +24,7 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr repository := firstRequest.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("CreateRepositoryFromBundle: %w", err) + return structerr.NewInvalidArgument("CreateRepositoryFromBundle: %w", err) } ctx := stream.Context() diff --git a/internal/gitaly/service/repository/create_repository_from_snapshot.go b/internal/gitaly/service/repository/create_repository_from_snapshot.go index 1d4b6111a..41c40073c 100644 --- a/internal/gitaly/service/repository/create_repository_from_snapshot.go +++ b/internal/gitaly/service/repository/create_repository_from_snapshot.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/command" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/labkit/correlation" @@ -50,7 +49,7 @@ var httpClient = &http.Client{ func newResolvedHTTPClient(httpAddress, resolvedAddress string) (*http.Client, error) { url, err := url.ParseRequestURI(httpAddress) if err != nil { - return nil, helper.ErrInvalidArgumentf("parsing HTTP URL: %w", err) + return nil, structerr.NewInvalidArgument("parsing HTTP URL: %w", err) } port := url.Port() @@ -61,13 +60,13 @@ func newResolvedHTTPClient(httpAddress, resolvedAddress string) (*http.Client, e case "https": port = "443" default: - return nil, helper.ErrInvalidArgumentf("unsupported schema %q", url.Scheme) + return nil, structerr.NewInvalidArgument("unsupported schema %q", url.Scheme) } } // Sanity-check whether the resolved address is a valid IP address. if net.ParseIP(resolvedAddress) == nil { - return nil, helper.ErrInvalidArgumentf("invalid resolved address %q", resolvedAddress) + return nil, structerr.NewInvalidArgument("invalid resolved address %q", resolvedAddress) } transport := httpTransport.Clone() @@ -87,14 +86,14 @@ func newResolvedHTTPClient(httpAddress, resolvedAddress string) (*http.Client, e func untar(ctx context.Context, path string, in *gitalypb.CreateRepositoryFromSnapshotRequest) error { req, err := http.NewRequestWithContext(ctx, "GET", in.HttpUrl, nil) if err != nil { - return helper.ErrInvalidArgumentf("Bad HTTP URL: %w", err) + return structerr.NewInvalidArgument("Bad HTTP URL: %w", err) } client := httpClient if resolvedAddress := in.GetResolvedAddress(); resolvedAddress != "" { client, err = newResolvedHTTPClient(in.HttpUrl, resolvedAddress) if err != nil { - return helper.ErrInvalidArgumentf("creating resolved HTTP client: %w", err) + return structerr.NewInvalidArgument("creating resolved HTTP client: %w", err) } } @@ -127,7 +126,7 @@ func untar(ctx context.Context, path string, in *gitalypb.CreateRepositoryFromSn func (s *server) CreateRepositoryFromSnapshot(ctx context.Context, in *gitalypb.CreateRepositoryFromSnapshotRequest) (*gitalypb.CreateRepositoryFromSnapshotResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if err := s.createRepository(ctx, repository, func(repo *gitalypb.Repository) error { path, err := s.locator.GetPath(repo) diff --git a/internal/gitaly/service/repository/create_repository_from_url.go b/internal/gitaly/service/repository/create_repository_from_url.go index f0c86d805..48165c8f2 100644 --- a/internal/gitaly/service/repository/create_repository_from_url.go +++ b/internal/gitaly/service/repository/create_repository_from_url.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/command" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -60,7 +59,7 @@ func (s *server) cloneFromURLCommand( if resolvedAddress != "" { modifiedURL, resolveConfig, err := git.GetURLAndResolveConfig(u.String(), resolvedAddress) if err != nil { - return nil, helper.ErrInvalidArgumentf("couldn't get curloptResolve config: %w", err) + return nil, structerr.NewInvalidArgument("couldn't get curloptResolve config: %w", err) } urlString = modifiedURL @@ -86,7 +85,7 @@ func (s *server) cloneFromURLCommand( func (s *server) CreateRepositoryFromURL(ctx context.Context, req *gitalypb.CreateRepositoryFromURLRequest) (*gitalypb.CreateRepositoryFromURLResponse, error) { if err := validateCreateRepositoryFromURLRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("CreateRepositoryFromURL: %w", err) + return nil, structerr.NewInvalidArgument("CreateRepositoryFromURL: %w", err) } if err := s.createRepository(ctx, req.GetRepository(), func(repo *gitalypb.Repository) error { diff --git a/internal/gitaly/service/repository/create_repository_test.go b/internal/gitaly/service/repository/create_repository_test.go index e5cbc7a6a..4fcc8a93b 100644 --- a/internal/gitaly/service/repository/create_repository_test.go +++ b/internal/gitaly/service/repository/create_repository_test.go @@ -14,7 +14,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/auth" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/praefectutil" @@ -165,7 +164,7 @@ func TestCreateRepository_withObjectFormat(t *testing.T) { { desc: "invalid object format", objectFormat: 3, - expectedErr: helper.ErrInvalidArgumentf("unknown object format: \"3\""), + expectedErr: structerr.NewInvalidArgument("unknown object format: \"3\""), }, } { t.Run(tc.desc, func(t *testing.T) { @@ -217,7 +216,7 @@ func TestCreateRepository_invalidArguments(t *testing.T) { { desc: "missing repository", repo: nil, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -228,7 +227,7 @@ func TestCreateRepository_invalidArguments(t *testing.T) { StorageName: "does not exist", RelativePath: "foobar.git", }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( `creating repository: locate repository: GetStorageByName: no such storage: "does not exist"`, "repo scoped: invalid Repository", )), diff --git a/internal/gitaly/service/repository/fetch.go b/internal/gitaly/service/repository/fetch.go index 80f8f6dc9..afbfaf3ed 100644 --- a/internal/gitaly/service/repository/fetch.go +++ b/internal/gitaly/service/repository/fetch.go @@ -29,7 +29,7 @@ func validateFetchSourceBranchRequest(in *gitalypb.FetchSourceBranchRequest) err func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourceBranchRequest) (*gitalypb.FetchSourceBranchResponse, error) { if err := validateFetchSourceBranchRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } targetRepo := s.localrepo(req.GetRepository()) diff --git a/internal/gitaly/service/repository/fetch_bundle.go b/internal/gitaly/service/repository/fetch_bundle.go index 5fcd8f92c..9d53c82ea 100644 --- a/internal/gitaly/service/repository/fetch_bundle.go +++ b/internal/gitaly/service/repository/fetch_bundle.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/tempdir" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -28,7 +27,7 @@ func (s *server) FetchBundle(stream gitalypb.RepositoryService_FetchBundleServer } if err := service.ValidateRepository(firstRequest.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } firstRead := true diff --git a/internal/gitaly/service/repository/fetch_remote.go b/internal/gitaly/service/repository/fetch_remote.go index 1d37b1e34..c0ad9293e 100644 --- a/internal/gitaly/service/repository/fetch_remote.go +++ b/internal/gitaly/service/repository/fetch_remote.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/txinfo" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" @@ -156,15 +155,15 @@ func didTagsChange(r io.Reader) bool { func (s *server) validateFetchRemoteRequest(req *gitalypb.FetchRemoteRequest) error { if err := service.ValidateRepository(req.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if req.GetRemoteParams() == nil { - return helper.ErrInvalidArgumentf("missing remote params") + return structerr.NewInvalidArgument("missing remote params") } if req.GetRemoteParams().GetUrl() == "" { - return helper.ErrInvalidArgumentf("blank or empty remote URL") + return structerr.NewInvalidArgument("blank or empty remote URL") } return nil diff --git a/internal/gitaly/service/repository/fsck.go b/internal/gitaly/service/repository/fsck.go index 15a759a7c..9b5f0a9e9 100644 --- a/internal/gitaly/service/repository/fsck.go +++ b/internal/gitaly/service/repository/fsck.go @@ -6,14 +6,14 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) Fsck(ctx context.Context, req *gitalypb.FsckRequest) (*gitalypb.FsckResponse, error) { repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } var stdout, stderr bytes.Buffer diff --git a/internal/gitaly/service/repository/fullpath.go b/internal/gitaly/service/repository/fullpath.go index ce2c12d72..9689f7f2b 100644 --- a/internal/gitaly/service/repository/fullpath.go +++ b/internal/gitaly/service/repository/fullpath.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -21,11 +20,11 @@ func (s *server) SetFullPath( ) (*gitalypb.SetFullPathResponse, error) { repository := request.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if len(request.GetPath()) == 0 { - return nil, helper.ErrInvalidArgumentf("no path provided") + return nil, structerr.NewInvalidArgument("no path provided") } repo := s.localrepo(repository) @@ -42,7 +41,7 @@ func (s *server) SetFullPath( func (s *server) FullPath(ctx context.Context, request *gitalypb.FullPathRequest) (*gitalypb.FullPathResponse, error) { repository := request.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) diff --git a/internal/gitaly/service/repository/fullpath_test.go b/internal/gitaly/service/repository/fullpath_test.go index d25eee5c2..999bfec1a 100644 --- a/internal/gitaly/service/repository/fullpath_test.go +++ b/internal/gitaly/service/repository/fullpath_test.go @@ -9,8 +9,8 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" + "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" "google.golang.org/grpc/codes" @@ -40,7 +40,7 @@ func TestSetFullPath(t *testing.T) { Path: "", }) require.Nil(t, response) - testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf("no path provided"), err) + testhelper.RequireGrpcError(t, structerr.NewInvalidArgument("no path provided"), err) }) t.Run("invalid storage", func(t *testing.T) { diff --git a/internal/gitaly/service/repository/gc.go b/internal/gitaly/service/repository/gc.go index 5dad3ac1a..1dd58091a 100644 --- a/internal/gitaly/service/repository/gc.go +++ b/internal/gitaly/service/repository/gc.go @@ -16,7 +16,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -29,7 +28,7 @@ func (s *server) GarbageCollect(ctx context.Context, in *gitalypb.GarbageCollect repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) @@ -76,7 +75,7 @@ func (s *server) gc(ctx context.Context, in *gitalypb.GarbageCollectRequest) err ) if err != nil { if git.IsInvalidArgErr(err) { - return helper.ErrInvalidArgumentf("gitCommand: %w", err) + return structerr.NewInvalidArgument("gitCommand: %w", err) } return structerr.NewInternal("gitCommand: %w", err) diff --git a/internal/gitaly/service/repository/has_local_branches.go b/internal/gitaly/service/repository/has_local_branches.go index 2b1f9e84d..e6cc1513b 100644 --- a/internal/gitaly/service/repository/has_local_branches.go +++ b/internal/gitaly/service/repository/has_local_branches.go @@ -4,7 +4,6 @@ import ( "context" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -12,7 +11,7 @@ import ( func (s *server) HasLocalBranches(ctx context.Context, in *gitalypb.HasLocalBranchesRequest) (*gitalypb.HasLocalBranchesResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } hasBranches, err := s.localrepo(repository).HasBranches(ctx) if err != nil { diff --git a/internal/gitaly/service/repository/has_local_branches_test.go b/internal/gitaly/service/repository/has_local_branches_test.go index 4c53a77e8..e352b0ebb 100644 --- a/internal/gitaly/service/repository/has_local_branches_test.go +++ b/internal/gitaly/service/repository/has_local_branches_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" ) @@ -67,7 +67,7 @@ func TestHasLocalBranches_failure(t *testing.T) { { desc: "repository nil", repository: nil, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -78,7 +78,7 @@ func TestHasLocalBranches_failure(t *testing.T) { StorageName: "fake", RelativePath: "path", }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( `GetStorageByName: no such storage: "fake"`, "repo scoped: invalid Repository", )), diff --git a/internal/gitaly/service/repository/info_attributes.go b/internal/gitaly/service/repository/info_attributes.go index a39127a59..04b13d151 100644 --- a/internal/gitaly/service/repository/info_attributes.go +++ b/internal/gitaly/service/repository/info_attributes.go @@ -6,7 +6,6 @@ import ( "path/filepath" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -15,7 +14,7 @@ import ( func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream gitalypb.RepositoryService_GetInfoAttributesServer) error { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repoPath, err := s.locator.GetRepoPath(repository) if err != nil { diff --git a/internal/gitaly/service/repository/license.go b/internal/gitaly/service/repository/license.go index 58a34d9c8..2888d690b 100644 --- a/internal/gitaly/service/repository/license.go +++ b/internal/gitaly/service/repository/license.go @@ -18,7 +18,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/lstree" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -44,7 +43,7 @@ var nicknameByLicenseIdentifier = map[string]string{ func (s *server) FindLicense(ctx context.Context, req *gitalypb.FindLicenseRequest) (*gitalypb.FindLicenseResponse, error) { repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if featureflag.GoFindLicense.IsEnabled(ctx) { repo := localrepo.New(s.locator, s.gitCmdFactory, s.catfileCache, repository) diff --git a/internal/gitaly/service/repository/merge_base.go b/internal/gitaly/service/repository/merge_base.go index bbee0f77a..33491423a 100644 --- a/internal/gitaly/service/repository/merge_base.go +++ b/internal/gitaly/service/repository/merge_base.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -15,7 +14,7 @@ import ( func (s *server) FindMergeBase(ctx context.Context, req *gitalypb.FindMergeBaseRequest) (*gitalypb.FindMergeBaseResponse, error) { repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } var revisions []string for _, rev := range req.GetRevisions() { @@ -23,7 +22,7 @@ func (s *server) FindMergeBase(ctx context.Context, req *gitalypb.FindMergeBaseR } if len(revisions) < 2 { - return nil, helper.ErrInvalidArgumentf("at least 2 revisions are required") + return nil, structerr.NewInvalidArgument("at least 2 revisions are required") } cmd, err := s.gitCmdFactory.New(ctx, repository, diff --git a/internal/gitaly/service/repository/midx.go b/internal/gitaly/service/repository/midx.go index dd2733429..5aa9789ab 100644 --- a/internal/gitaly/service/repository/midx.go +++ b/internal/gitaly/service/repository/midx.go @@ -16,7 +16,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/repository" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -29,7 +28,7 @@ const ( func (s *server) MidxRepack(ctx context.Context, in *gitalypb.MidxRepackRequest) (*gitalypb.MidxRepackResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) @@ -41,7 +40,7 @@ func (s *server) MidxRepack(ctx context.Context, in *gitalypb.MidxRepackRequest) for _, cmd := range []midxSubCommand{s.midxWrite, s.midxExpire, s.midxRepack} { if err := s.safeMidxCommand(ctx, repository, cmd); err != nil { if git.IsInvalidArgErr(err) { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return nil, structerr.NewInternal("...%w", err) diff --git a/internal/gitaly/service/repository/optimize.go b/internal/gitaly/service/repository/optimize.go index cd2dfe185..32bffb60a 100644 --- a/internal/gitaly/service/repository/optimize.go +++ b/internal/gitaly/service/repository/optimize.go @@ -5,7 +5,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/housekeeping" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -34,7 +33,7 @@ func (s *server) OptimizeRepository(ctx context.Context, in *gitalypb.OptimizeRe strategyOpt = housekeeping.WithOptimizationStrategy(strategy) default: - return nil, helper.ErrInvalidArgumentf("unsupported optimization strategy %d", in.GetStrategy()) + return nil, structerr.NewInvalidArgument("unsupported optimization strategy %d", in.GetStrategy()) } if err := s.housekeepingManager.OptimizeRepository(ctx, repo, strategyOpt); err != nil { @@ -47,7 +46,7 @@ func (s *server) OptimizeRepository(ctx context.Context, in *gitalypb.OptimizeRe func (s *server) validateOptimizeRepositoryRequest(in *gitalypb.OptimizeRepositoryRequest) error { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } _, err := s.locator.GetRepoPath(repository) diff --git a/internal/gitaly/service/repository/optimize_test.go b/internal/gitaly/service/repository/optimize_test.go index 5c0ed9722..35cb5f9e1 100644 --- a/internal/gitaly/service/repository/optimize_test.go +++ b/internal/gitaly/service/repository/optimize_test.go @@ -18,8 +18,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/housekeeping" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -296,7 +296,7 @@ func TestOptimizeRepository_validation(t *testing.T) { { desc: "empty repository", request: &gitalypb.OptimizeRepositoryRequest{}, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -309,7 +309,7 @@ func TestOptimizeRepository_validation(t *testing.T) { RelativePath: repo.GetRelativePath(), }, }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( `GetStorageByName: no such storage: "non-existent"`, "repo scoped: invalid Repository"), ), @@ -322,7 +322,7 @@ func TestOptimizeRepository_validation(t *testing.T) { RelativePath: "path/not/exist", }, }, - expectedErr: helper.ErrNotFoundf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewNotFound(testhelper.GitalyOrPraefect( fmt.Sprintf(`GetRepoPath: not a git repository: "%s/path/not/exist"`, cfg.Storages[0].Path), `routing repository maintenance: getting repository metadata: repository not found`, )), @@ -333,7 +333,7 @@ func TestOptimizeRepository_validation(t *testing.T) { Repository: repo, Strategy: 12, }, - expectedErr: helper.ErrInvalidArgumentf("unsupported optimization strategy 12"), + expectedErr: structerr.NewInvalidArgument("unsupported optimization strategy 12"), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/repository/prune_unreachable_objects.go b/internal/gitaly/service/repository/prune_unreachable_objects.go index cbf5d48f6..6c0d68e80 100644 --- a/internal/gitaly/service/repository/prune_unreachable_objects.go +++ b/internal/gitaly/service/repository/prune_unreachable_objects.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/housekeeping" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -22,7 +21,7 @@ func (s *server) PruneUnreachableObjects( ) (*gitalypb.PruneUnreachableObjectsResponse, error) { repository := request.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) diff --git a/internal/gitaly/service/repository/prune_unreachable_objects_test.go b/internal/gitaly/service/repository/prune_unreachable_objects_test.go index 0206c5dab..d4badb60d 100644 --- a/internal/gitaly/service/repository/prune_unreachable_objects_test.go +++ b/internal/gitaly/service/repository/prune_unreachable_objects_test.go @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" ) @@ -30,9 +30,9 @@ func TestPruneUnreachableObjects(t *testing.T) { t.Run("missing repository", func(t *testing.T) { _, err := client.PruneUnreachableObjects(ctx, &gitalypb.PruneUnreachableObjectsRequest{}) if testhelper.IsPraefectEnabled() { - testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf("repo scoped: empty Repository"), err) + testhelper.RequireGrpcError(t, structerr.NewInvalidArgument("repo scoped: empty Repository"), err) } else { - testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf("empty Repository"), err) + testhelper.RequireGrpcError(t, structerr.NewInvalidArgument("empty Repository"), err) } }) @@ -43,7 +43,7 @@ func TestPruneUnreachableObjects(t *testing.T) { _, err := client.PruneUnreachableObjects(ctx, &gitalypb.PruneUnreachableObjectsRequest{ Repository: repo, }) - testhelper.RequireGrpcError(t, helper.ErrNotFoundf("GetRepoPath: not a git repository: %q", repoPath), err) + testhelper.RequireGrpcError(t, structerr.NewNotFound("GetRepoPath: not a git repository: %q", repoPath), err) }) t.Run("empty repository", func(t *testing.T) { diff --git a/internal/gitaly/service/repository/raw_changes.go b/internal/gitaly/service/repository/raw_changes.go index 1237921e0..b3fcfc46b 100644 --- a/internal/gitaly/service/repository/raw_changes.go +++ b/internal/gitaly/service/repository/raw_changes.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/rawdiff" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -22,7 +21,7 @@ func (s *server) GetRawChanges(req *gitalypb.GetRawChangesRequest, stream gitaly ctx := stream.Context() repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) @@ -33,7 +32,7 @@ func (s *server) GetRawChanges(req *gitalypb.GetRawChangesRequest, stream gitaly defer cancel() if err := validateRawChangesRequest(ctx, req, objectInfoReader); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.getRawChanges(stream, repo, objectInfoReader, req.GetFromRevision(), req.GetToRevision()); err != nil { diff --git a/internal/gitaly/service/repository/raw_changes_test.go b/internal/gitaly/service/repository/raw_changes_test.go index 5079243c3..eca213dbe 100644 --- a/internal/gitaly/service/repository/raw_changes_test.go +++ b/internal/gitaly/service/repository/raw_changes_test.go @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "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" ) @@ -176,7 +176,7 @@ func TestGetRawChangesFailures(t *testing.T) { FromRevision: "", ToRevision: "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863", }, - expectedErr: helper.ErrInvalidArgumentf("invalid 'from' revision: %q", ""), + expectedErr: structerr.NewInvalidArgument("invalid 'from' revision: %q", ""), }, { desc: "missing repository", @@ -184,7 +184,7 @@ func TestGetRawChangesFailures(t *testing.T) { FromRevision: "cfe32cf61b73a0d5e9f13e774abde7ff789b1660", ToRevision: "913c66a37b4a45b9769037c55c2d238bd0942d2e", }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -197,7 +197,7 @@ func TestGetRawChangesFailures(t *testing.T) { FromRevision: "32800ed8206c0087f65e90a1a396b76d3c33f648", ToRevision: "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863", }, - expectedErr: helper.ErrInvalidArgumentf("invalid 'from' revision: %q", "32800ed8206c0087f65e90a1a396b76d3c33f648"), + expectedErr: structerr.NewInvalidArgument("invalid 'from' revision: %q", "32800ed8206c0087f65e90a1a396b76d3c33f648"), }, } { t.Run(fmt.Sprintf(tc.desc), func(t *testing.T) { diff --git a/internal/gitaly/service/repository/remove.go b/internal/gitaly/service/repository/remove.go index d6d3510e9..08967d7a5 100644 --- a/internal/gitaly/service/repository/remove.go +++ b/internal/gitaly/service/repository/remove.go @@ -10,7 +10,6 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/safe" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/txinfo" @@ -21,7 +20,7 @@ import ( func (s *server) RemoveRepository(ctx context.Context, in *gitalypb.RemoveRepositoryRequest) (*gitalypb.RemoveRepositoryResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } path, err := s.locator.GetPath(repository) if err != nil { @@ -46,7 +45,7 @@ func (s *server) RemoveRepository(ctx context.Context, in *gitalypb.RemoveReposi // care they may still just return `NotFound` errors. if _, err := os.Stat(path); err != nil { if os.IsNotExist(err) { - return nil, helper.ErrNotFoundf("repository does not exist") + return nil, structerr.NewNotFound("repository does not exist") } return nil, structerr.NewInternal("statting repository: %w", err) @@ -76,7 +75,7 @@ func (s *server) RemoveRepository(ctx context.Context, in *gitalypb.RemoveReposi // holding the lock. if _, err := os.Stat(path); err != nil { if os.IsNotExist(err) { - return nil, helper.ErrNotFoundf("repository was concurrently removed") + return nil, structerr.NewNotFound("repository was concurrently removed") } return nil, structerr.NewInternal("re-statting repository: %w", err) } diff --git a/internal/gitaly/service/repository/remove_test.go b/internal/gitaly/service/repository/remove_test.go index ced68cbca..81e981286 100644 --- a/internal/gitaly/service/repository/remove_test.go +++ b/internal/gitaly/service/repository/remove_test.go @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver" @@ -49,7 +48,7 @@ func TestRemoveRepository_doesNotExist(t *testing.T) { _, err := client.RemoveRepository(ctx, &gitalypb.RemoveRepositoryRequest{ Repository: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "/does/not/exist"}, }) - testhelper.RequireGrpcError(t, helper.ErrNotFoundf("repository does not exist"), err) + testhelper.RequireGrpcError(t, structerr.NewNotFound("repository does not exist"), err) } func TestRemoveRepository_validate(t *testing.T) { diff --git a/internal/gitaly/service/repository/rename.go b/internal/gitaly/service/repository/rename.go index c232afc50..5e4079cd8 100644 --- a/internal/gitaly/service/repository/rename.go +++ b/internal/gitaly/service/repository/rename.go @@ -9,7 +9,6 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/safe" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -17,7 +16,7 @@ import ( func (s *server) RenameRepository(ctx context.Context, in *gitalypb.RenameRepositoryRequest) (*gitalypb.RenameRepositoryResponse, error) { if err := validateRenameRepositoryRequest(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } targetRepo := &gitalypb.Repository{ @@ -35,12 +34,12 @@ func (s *server) RenameRepository(ctx context.Context, in *gitalypb.RenameReposi func (s *server) renameRepository(ctx context.Context, sourceRepo, targetRepo *gitalypb.Repository) error { sourcePath, err := s.locator.GetRepoPath(sourceRepo) if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } targetPath, err := s.locator.GetPath(targetRepo) if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } // Check up front whether the target path exists already. If it does, we can avoid going @@ -99,7 +98,7 @@ func (s *server) renameRepository(ctx context.Context, sourceRepo, targetRepo *g func validateRenameRepositoryRequest(in *gitalypb.RenameRepositoryRequest) error { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if in.GetRelativePath() == "" { diff --git a/internal/gitaly/service/repository/repack.go b/internal/gitaly/service/repository/repack.go index a2fffea1a..164902ee7 100644 --- a/internal/gitaly/service/repository/repack.go +++ b/internal/gitaly/service/repository/repack.go @@ -8,7 +8,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/housekeeping" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -28,7 +27,7 @@ func init() { func (s *server) RepackFull(ctx context.Context, in *gitalypb.RepackFullRequest) (*gitalypb.RepackFullResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) @@ -60,7 +59,7 @@ func (s *server) RepackFull(ctx context.Context, in *gitalypb.RepackFullRequest) func (s *server) RepackIncremental(ctx context.Context, in *gitalypb.RepackIncrementalRequest) (*gitalypb.RepackIncrementalResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) diff --git a/internal/gitaly/service/repository/replicate.go b/internal/gitaly/service/repository/replicate.go index 0dc09b210..f3912ea9d 100644 --- a/internal/gitaly/service/repository/replicate.go +++ b/internal/gitaly/service/repository/replicate.go @@ -35,7 +35,7 @@ var ErrInvalidSourceRepository = status.Error(codes.NotFound, "invalid source re func (s *server) ReplicateRepository(ctx context.Context, in *gitalypb.ReplicateRepositoryRequest) (*gitalypb.ReplicateRepositoryResponse, error) { if err := validateReplicateRepository(in); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repoPath, err := s.locator.GetPath(in.GetRepository()) diff --git a/internal/gitaly/service/repository/repository_exists.go b/internal/gitaly/service/repository/repository_exists.go index f4e0d58f2..2e0a2ae07 100644 --- a/internal/gitaly/service/repository/repository_exists.go +++ b/internal/gitaly/service/repository/repository_exists.go @@ -5,13 +5,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) RepositoryExists(ctx context.Context, in *gitalypb.RepositoryExistsRequest) (*gitalypb.RepositoryExistsResponse, error) { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } path, err := s.locator.GetPath(in.Repository) if err != nil { diff --git a/internal/gitaly/service/repository/restore_custom_hooks.go b/internal/gitaly/service/repository/restore_custom_hooks.go index 9bdfcdf22..80edad305 100644 --- a/internal/gitaly/service/repository/restore_custom_hooks.go +++ b/internal/gitaly/service/repository/restore_custom_hooks.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/command" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/safe" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" @@ -33,7 +32,7 @@ func (s *server) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCus repository := firstRequest.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } reader := streamio.NewReader(func() ([]byte, error) { @@ -83,7 +82,7 @@ func (s *server) restoreCustomHooksWithVoting(stream gitalypb.RepositoryService_ repository := firstRequest.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } v := voting.NewVoteHash() diff --git a/internal/gitaly/service/repository/search_files.go b/internal/gitaly/service/repository/search_files.go index a9783b81f..11286f630 100644 --- a/internal/gitaly/service/repository/search_files.go +++ b/internal/gitaly/service/repository/search_files.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/lstree" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -29,7 +28,7 @@ var contentDelimiter = []byte("--\n") func (s *server) SearchFilesByContent(req *gitalypb.SearchFilesByContentRequest, stream gitalypb.RepositoryService_SearchFilesByContentServer) error { if err := validateSearchFilesRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } ctx := stream.Context() @@ -99,18 +98,18 @@ func sendSearchFilesResultChunked(cmd *command.Command, stream gitalypb.Reposito func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, stream gitalypb.RepositoryService_SearchFilesByNameServer) error { if err := validateSearchFilesRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } var filter *regexp.Regexp if req.GetFilter() != "" { if len(req.GetFilter()) > searchFilesFilterMaxLength { - return helper.ErrInvalidArgumentf("filter exceeds maximum length") + return structerr.NewInvalidArgument("filter exceeds maximum length") } var err error filter, err = regexp.Compile(req.GetFilter()) if err != nil { - return helper.ErrInvalidArgumentf("filter did not compile: %w", err) + return structerr.NewInvalidArgument("filter did not compile: %w", err) } } diff --git a/internal/gitaly/service/repository/size.go b/internal/gitaly/service/repository/size.go index 78443b220..41e167392 100644 --- a/internal/gitaly/service/repository/size.go +++ b/internal/gitaly/service/repository/size.go @@ -18,8 +18,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -36,7 +36,7 @@ import ( func (s *server) RepositorySize(ctx context.Context, in *gitalypb.RepositorySizeRequest) (*gitalypb.RepositorySizeResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) @@ -168,7 +168,7 @@ func calculateSizeWithRevlist(ctx context.Context, repo *localrepo.Repo) (int64, func (s *server) GetObjectDirectorySize(ctx context.Context, in *gitalypb.GetObjectDirectorySizeRequest) (*gitalypb.GetObjectDirectorySizeResponse, error) { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } repo := s.localrepo(repository) diff --git a/internal/gitaly/service/repository/snapshot.go b/internal/gitaly/service/repository/snapshot.go index b43654d6a..9b0a33ebb 100644 --- a/internal/gitaly/service/repository/snapshot.go +++ b/internal/gitaly/service/repository/snapshot.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/archive" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -24,7 +23,7 @@ var objectFiles = []*regexp.Regexp{ func (s *server) GetSnapshot(in *gitalypb.GetSnapshotRequest, stream gitalypb.RepositoryService_GetSnapshotServer) error { if err := service.ValidateRepository(in.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } path, err := s.locator.GetRepoPath(in.Repository) diff --git a/internal/gitaly/service/repository/util.go b/internal/gitaly/service/repository/util.go index 2e4664916..c26a7a6e7 100644 --- a/internal/gitaly/service/repository/util.go +++ b/internal/gitaly/service/repository/util.go @@ -12,7 +12,6 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/safe" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/tempdir" @@ -62,7 +61,7 @@ func (s *server) createRepository( ) error { targetPath, err := s.locator.GetPath(repository) if err != nil { - return helper.ErrInvalidArgumentf("locate repository: %w", err) + return structerr.NewInvalidArgument("locate repository: %w", err) } // The repository must not exist on disk already, or otherwise we won't be able to diff --git a/internal/gitaly/service/repository/write_ref.go b/internal/gitaly/service/repository/write_ref.go index b1be72478..4bd8aef52 100644 --- a/internal/gitaly/service/repository/write_ref.go +++ b/internal/gitaly/service/repository/write_ref.go @@ -9,14 +9,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) WriteRef(ctx context.Context, req *gitalypb.WriteRefRequest) (*gitalypb.WriteRefResponse, error) { if err := validateWriteRefRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if err := s.writeRef(ctx, req); err != nil { return nil, structerr.NewInternal("%w", err) diff --git a/internal/gitaly/service/server/clocksynced.go b/internal/gitaly/service/server/clocksynced.go index 2f68a2d12..964e7a4c6 100644 --- a/internal/gitaly/service/server/clocksynced.go +++ b/internal/gitaly/service/server/clocksynced.go @@ -4,13 +4,14 @@ import ( "context" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) // ClockSynced returns whether the system clock has an acceptable time drift when compared to NTP service. func (s *server) ClockSynced(_ context.Context, req *gitalypb.ClockSyncedRequest) (*gitalypb.ClockSyncedResponse, error) { if err := req.DriftThreshold.CheckValid(); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } synced, err := helper.CheckClockSync(req.NtpHost, req.DriftThreshold.AsDuration()) if err != nil { diff --git a/internal/gitaly/service/smarthttp/inforefs.go b/internal/gitaly/service/smarthttp/inforefs.go index 9ce61eec7..6b2de57f3 100644 --- a/internal/gitaly/service/smarthttp/inforefs.go +++ b/internal/gitaly/service/smarthttp/inforefs.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -24,7 +23,7 @@ const ( func (s *server) InfoRefsUploadPack(in *gitalypb.InfoRefsRequest, stream gitalypb.SmartHTTPService_InfoRefsUploadPackServer) error { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repoPath, err := s.locator.GetRepoPath(repository) if err != nil { @@ -43,7 +42,7 @@ func (s *server) InfoRefsUploadPack(in *gitalypb.InfoRefsRequest, stream gitalyp func (s *server) InfoRefsReceivePack(in *gitalypb.InfoRefsRequest, stream gitalypb.SmartHTTPService_InfoRefsReceivePackServer) error { repository := in.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } repoPath, err := s.locator.GetRepoPath(repository) if err != nil { diff --git a/internal/gitaly/service/smarthttp/receive_pack.go b/internal/gitaly/service/smarthttp/receive_pack.go index 9d2ecc235..64668413f 100644 --- a/internal/gitaly/service/smarthttp/receive_pack.go +++ b/internal/gitaly/service/smarthttp/receive_pack.go @@ -8,7 +8,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -97,13 +96,13 @@ func (s *server) PostReceivePack(stream gitalypb.SmartHTTPService_PostReceivePac func validateReceivePackRequest(req *gitalypb.PostReceivePackRequest) error { if req.GlId == "" { - return helper.ErrInvalidArgumentf("empty GlId") + return structerr.NewInvalidArgument("empty GlId") } if req.Data != nil { - return helper.ErrInvalidArgumentf("non-empty Data") + return structerr.NewInvalidArgument("non-empty Data") } if err := service.ValidateRepository(req.GetRepository()); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } return nil diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go index d7b066969..7851ac51f 100644 --- a/internal/gitaly/service/smarthttp/receive_pack_test.go +++ b/internal/gitaly/service/smarthttp/receive_pack_test.go @@ -21,10 +21,10 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" gitalyhook "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v15/internal/gitlab" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver" @@ -402,10 +402,10 @@ func TestPostReceivePack_requestValidation(t *testing.T) { }, expectedErr: func() error { if testhelper.IsPraefectEnabled() { - return helper.ErrNotFoundf("mutator call: route repository mutator: get repository id: repository %q/%q not found", cfg.Storages[0].Name, "path/to/repo") + return structerr.NewNotFound("mutator call: route repository mutator: get repository id: repository %q/%q not found", cfg.Storages[0].Name, "path/to/repo") } - return helper.ErrInvalidArgumentf("empty GlId") + return structerr.NewInvalidArgument("empty GlId") }(), }, { @@ -420,10 +420,10 @@ func TestPostReceivePack_requestValidation(t *testing.T) { }, expectedErr: func() error { if testhelper.IsPraefectEnabled() { - return helper.ErrNotFoundf("mutator call: route repository mutator: get repository id: repository %q/%q not found", cfg.Storages[0].Name, "path/to/repo") + return structerr.NewNotFound("mutator call: route repository mutator: get repository id: repository %q/%q not found", cfg.Storages[0].Name, "path/to/repo") } - return helper.ErrInvalidArgumentf("non-empty Data") + return structerr.NewInvalidArgument("non-empty Data") }(), }, } { diff --git a/internal/gitaly/service/smarthttp/upload_pack.go b/internal/gitaly/service/smarthttp/upload_pack.go index 972ca080b..14369ca89 100644 --- a/internal/gitaly/service/smarthttp/upload_pack.go +++ b/internal/gitaly/service/smarthttp/upload_pack.go @@ -11,7 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/sidechannel" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -33,12 +32,12 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS } if req.Data != nil { - return helper.ErrInvalidArgumentf("non-empty Data") + return structerr.NewInvalidArgument("non-empty Data") } repoPath, gitConfig, err := s.validateUploadPackRequest(ctx, req) if err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } stdin := streamio.NewReader(func() ([]byte, error) { @@ -60,7 +59,7 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS func (s *server) PostUploadPackWithSidechannel(ctx context.Context, req *gitalypb.PostUploadPackWithSidechannelRequest) (*gitalypb.PostUploadPackWithSidechannelResponse, error) { repoPath, gitConfig, err := s.validateUploadPackRequest(ctx, req) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } conn, err := sidechannel.OpenSidechannel(ctx) diff --git a/internal/gitaly/service/smarthttp/upload_pack_test.go b/internal/gitaly/service/smarthttp/upload_pack_test.go index 6b650e57b..1da6b3d14 100644 --- a/internal/gitaly/service/smarthttp/upload_pack_test.go +++ b/internal/gitaly/service/smarthttp/upload_pack_test.go @@ -21,7 +21,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/sidechannel" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" @@ -341,7 +340,7 @@ func testServerPostUploadPackValidation(t *testing.T, ctx context.Context, makeR request: &gitalypb.PostUploadPackRequest{ Repository: &gitalypb.Repository{StorageName: "fake", RelativePath: "path"}, }, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( fmt.Sprintf("GetStorageByName: no such storage: %q", "fake"), "repo scoped: invalid Repository", )), @@ -349,7 +348,7 @@ func testServerPostUploadPackValidation(t *testing.T, ctx context.Context, makeR { desc: "unset repository", request: &gitalypb.PostUploadPackRequest{Repository: nil}, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), @@ -360,7 +359,7 @@ func testServerPostUploadPackValidation(t *testing.T, ctx context.Context, makeR Repository: repo, Data: []byte("Fail"), }, - expectedErr: helper.ErrInvalidArgumentf("non-empty Data"), + expectedErr: structerr.NewInvalidArgument("non-empty Data"), }, } { t.Run(tc.desc, func(t *testing.T) { @@ -388,7 +387,7 @@ func testServerPostUploadPackWithSideChannelValidation(t *testing.T, ctx context { desc: "Repository doesn't exist", req: &gitalypb.PostUploadPackRequest{Repository: &gitalypb.Repository{StorageName: "fake", RelativePath: "path"}}, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( `GetStorageByName: no such storage: "fake"`, "repo scoped: invalid Repository", )), @@ -396,7 +395,7 @@ func testServerPostUploadPackWithSideChannelValidation(t *testing.T, ctx context { desc: "Repository no provided", req: &gitalypb.PostUploadPackRequest{Repository: nil}, - expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefect( + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( "empty Repository", "repo scoped: empty Repository", )), diff --git a/internal/gitaly/service/ssh/receive_pack.go b/internal/gitaly/service/ssh/receive_pack.go index 70e1579c1..ea292e6f3 100644 --- a/internal/gitaly/service/ssh/receive_pack.go +++ b/internal/gitaly/service/ssh/receive_pack.go @@ -13,7 +13,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -35,7 +34,7 @@ func (s *server) SSHReceivePack(stream gitalypb.SSHService_SSHReceivePackServer) }).Debug("SSHReceivePack") if err = validateFirstReceivePackRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err := s.sshReceivePack(stream, req); err != nil { diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go index 7603c58ea..78256747e 100644 --- a/internal/gitaly/service/ssh/receive_pack_test.go +++ b/internal/gitaly/service/ssh/receive_pack_test.go @@ -24,7 +24,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v15/internal/gitlab" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" @@ -65,10 +64,10 @@ func TestReceivePack_validation(t *testing.T) { }, expectedErr: func() error { if testhelper.IsPraefectEnabled() { - return helper.ErrInvalidArgumentf("repo scoped: invalid Repository") + return structerr.NewInvalidArgument("repo scoped: invalid Repository") } - return helper.ErrInvalidArgumentf("empty RelativePath") + return structerr.NewInvalidArgument("empty RelativePath") }(), }, { @@ -79,10 +78,10 @@ func TestReceivePack_validation(t *testing.T) { }, expectedErr: func() error { if testhelper.IsPraefectEnabled() { - return helper.ErrInvalidArgumentf("repo scoped: empty Repository") + return structerr.NewInvalidArgument("repo scoped: empty Repository") } - return helper.ErrInvalidArgumentf("empty Repository") + return structerr.NewInvalidArgument("empty Repository") }(), }, { @@ -94,7 +93,7 @@ func TestReceivePack_validation(t *testing.T) { }, GlId: "", }, - expectedErr: helper.ErrInvalidArgumentf("empty GlId"), + expectedErr: structerr.NewInvalidArgument("empty GlId"), }, { desc: "stdin on first request", @@ -106,7 +105,7 @@ func TestReceivePack_validation(t *testing.T) { GlId: "user-123", Stdin: []byte("Fail"), }, - expectedErr: helper.ErrInvalidArgumentf("non-empty data in first request"), + expectedErr: structerr.NewInvalidArgument("non-empty data in first request"), }, } { t.Run(tc.desc, func(t *testing.T) { @@ -333,16 +332,16 @@ func TestReceivePack_failure(t *testing.T) { require.Error(t, err) if testhelper.IsPraefectEnabled() { - require.Contains(t, err.Error(), helper.ErrInvalidArgumentf("repo scoped: invalid Repository").Error()) + require.Contains(t, err.Error(), structerr.NewInvalidArgument("repo scoped: invalid Repository").Error()) } else { - require.Contains(t, err.Error(), helper.ErrInvalidArgumentf("GetStorageByName: no such storage: \\\"foobar\\\"\\n").Error()) + require.Contains(t, err.Error(), structerr.NewInvalidArgument("GetStorageByName: no such storage: \\\"foobar\\\"\\n").Error()) } }) t.Run("clone with invalid GlId", func(t *testing.T) { _, _, err := testCloneAndPush(t, ctx, cfg, cfg.SocketPath, repo, repoPath, pushParams{storageName: cfg.Storages[0].Name, glID: ""}) require.Error(t, err) - require.Contains(t, err.Error(), helper.ErrInvalidArgumentf("empty GlId").Error()) + require.Contains(t, err.Error(), structerr.NewInvalidArgument("empty GlId").Error()) }) } diff --git a/internal/gitaly/service/ssh/upload_archive.go b/internal/gitaly/service/ssh/upload_archive.go index 717884ca1..8f087bb0b 100644 --- a/internal/gitaly/service/ssh/upload_archive.go +++ b/internal/gitaly/service/ssh/upload_archive.go @@ -22,7 +22,7 @@ func (s *server) SSHUploadArchive(stream gitalypb.SSHService_SSHUploadArchiveSer return structerr.NewInternal("%w", err) } if err = validateFirstUploadArchiveRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } if err = s.sshUploadArchive(stream, req); err != nil { diff --git a/internal/gitaly/service/ssh/upload_pack.go b/internal/gitaly/service/ssh/upload_pack.go index 8bbbf11a7..7542c3a2a 100644 --- a/internal/gitaly/service/ssh/upload_pack.go +++ b/internal/gitaly/service/ssh/upload_pack.go @@ -38,7 +38,7 @@ func (s *server) SSHUploadPack(stream gitalypb.SSHService_SSHUploadPackServer) e }).Debug("SSHUploadPack") if err = validateFirstUploadPackRequest(req); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } stdin := streamio.NewReader(func() ([]byte, error) { diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go index a478ff53f..2dce494db 100644 --- a/internal/gitaly/service/ssh/upload_pack_test.go +++ b/internal/gitaly/service/ssh/upload_pack_test.go @@ -21,7 +21,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/sidechannel" @@ -442,9 +441,9 @@ func TestUploadPack_validation(t *testing.T) { }, expectedErr: func() error { if testhelper.IsPraefectEnabled() { - return helper.ErrInvalidArgumentf("repo scoped: invalid Repository") + return structerr.NewInvalidArgument("repo scoped: invalid Repository") } - return helper.ErrInvalidArgumentf("empty RelativePath") + return structerr.NewInvalidArgument("empty RelativePath") }(), }, { @@ -454,9 +453,9 @@ func TestUploadPack_validation(t *testing.T) { }, expectedErr: func() error { if testhelper.IsPraefectEnabled() { - return helper.ErrInvalidArgumentf("repo scoped: empty Repository") + return structerr.NewInvalidArgument("repo scoped: empty Repository") } - return helper.ErrInvalidArgumentf("empty Repository") + return structerr.NewInvalidArgument("empty Repository") }(), }, { @@ -470,9 +469,9 @@ func TestUploadPack_validation(t *testing.T) { }, expectedErr: func() error { if testhelper.IsPraefectEnabled() { - return helper.ErrNotFoundf("accessor call: route repository accessor: consistent storages: repository %q/%q not found", cfg.Storages[0].Name, "path/to/repo") + return structerr.NewNotFound("accessor call: route repository accessor: consistent storages: repository %q/%q not found", cfg.Storages[0].Name, "path/to/repo") } - return helper.ErrInvalidArgumentf("non-empty stdin in first request") + return structerr.NewInvalidArgument("non-empty stdin in first request") }(), }, } { diff --git a/internal/helper/error.go b/internal/helper/error.go index b9cb5c60a..ede49f5cd 100644 --- a/internal/helper/error.go +++ b/internal/helper/error.go @@ -2,106 +2,11 @@ package helper import ( "errors" - "fmt" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) -type statusWrapper struct { - error - status *status.Status -} - -func (sw statusWrapper) GRPCStatus() *status.Status { - return sw.status -} - -func (sw statusWrapper) Unwrap() error { - return sw.error -} - -// ErrInvalidArgumentf wraps a formatted error with codes.InvalidArgument, unless the formatted -// error is a wrapped gRPC error. -func ErrInvalidArgumentf(format string, a ...interface{}) error { - return formatError(codes.InvalidArgument, format, a...) -} - -// ErrNotFoundf wraps a formatted error with codes.NotFound, unless the -// formatted error is a wrapped gRPC error. -func ErrNotFoundf(format string, a ...interface{}) error { - return formatError(codes.NotFound, format, a...) -} - -// grpcErrorMessageWrapper is used to wrap a gRPC `status.Status`-style error such that it behaves -// like a `status.Status`, except that it generates a readable error message. -type grpcErrorMessageWrapper struct { - *status.Status -} - -func (e grpcErrorMessageWrapper) GRPCStatus() *status.Status { - return e.Status -} - -func (e grpcErrorMessageWrapper) Error() string { - return e.Message() -} - -func (e grpcErrorMessageWrapper) Unwrap() error { - return e.Status.Err() -} - -type grpcStatuser interface { - GRPCStatus() *status.Status -} - -// formatError will create a new error from the given format string. If the error string contains a -// %w verb and its corresponding error has a gRPC error code, then the returned error will keep this -// gRPC error code instead of using the one provided as an argument. -func formatError(code codes.Code, format string, a ...interface{}) error { - args := make([]interface{}, 0, len(a)) - for _, a := range a { - err, ok := a.(error) - if !ok { - args = append(args, a) - continue - } - - status, ok := status.FromError(err) - if !ok { - args = append(args, a) - continue - } - - // Wrap gRPC status errors so that the resulting error message stays readable. - args = append(args, grpcErrorMessageWrapper{status}) - } - - err := fmt.Errorf(format, args...) - - for current := err; current != nil; current = errors.Unwrap(current) { - nestedCode := GrpcCode(current) - if nestedCode != codes.OK && nestedCode != codes.Unknown { - code = nestedCode - } - } - - st := status.New(code, err.Error()) - - var wrappedGRPCStatus grpcStatuser - if errors.As(err, &wrappedGRPCStatus) { - wrappedProto := wrappedGRPCStatus.GRPCStatus().Proto() - - if len(wrappedProto.Details) > 0 { - proto := st.Proto() - proto.Details = wrappedProto.Details - st = status.FromProto(proto) - } - } - - return statusWrapper{err, st} -} - // GrpcCode translates errors into codes.Code values. // It unwraps the nested errors until it finds the most nested one that returns the codes.Code. // If err is nil it returns codes.OK. diff --git a/internal/helper/error_test.go b/internal/helper/error_test.go index 06478ff20..9e83407cf 100644 --- a/internal/helper/error_test.go +++ b/internal/helper/error_test.go @@ -1,169 +1,15 @@ package helper import ( - "errors" "fmt" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "google.golang.org/grpc/test/grpc_testing" - "google.golang.org/protobuf/types/known/anypb" ) -// unusedErrorCode is any error code that we don't have any wrapping functions for yet. This is used -// to verify that we correctly wrap errors that already have a different gRPC error code than the -// one under test. -const unusedErrorCode = codes.OutOfRange - -func TestErrorf(t *testing.T) { - for _, tc := range []struct { - desc string - errorf func(format string, a ...interface{}) error - expectedCode codes.Code - }{ - { - desc: "InvalidArgumentf", - errorf: ErrInvalidArgumentf, - expectedCode: codes.InvalidArgument, - }, - { - desc: "NotFoundf", - errorf: ErrNotFoundf, - expectedCode: codes.NotFound, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - t.Run("with non-gRPC error", func(t *testing.T) { - err := tc.errorf("top-level: %w", errors.New("nested")) - require.EqualError(t, err, "top-level: nested") - require.Equal(t, tc.expectedCode, status.Code(err)) - - s, ok := status.FromError(err) - require.True(t, ok) - require.Equal(t, status.New(tc.expectedCode, "top-level: nested"), s) - }) - - t.Run("with status.Errorf error and %v", func(t *testing.T) { - require.NotEqual(t, tc.expectedCode, unusedErrorCode) - - err := tc.errorf("top-level: %v", status.Errorf(unusedErrorCode, "deeply: %s", "nested")) - require.EqualError(t, err, "top-level: deeply: nested") - - // The error code of the nested error should be discarded. - require.Equal(t, tc.expectedCode, status.Code(err)) - s, ok := status.FromError(err) - require.True(t, ok) - require.Equal(t, status.New(tc.expectedCode, "top-level: deeply: nested"), s) - }) - - t.Run("with status.Errorf error and %w", func(t *testing.T) { - require.NotEqual(t, tc.expectedCode, unusedErrorCode) - - err := tc.errorf("top-level: %w", status.Errorf(unusedErrorCode, "deeply: %s", "nested")) - require.EqualError(t, err, "top-level: deeply: nested") - - // We should be reporting the error code of the nested error. - require.Equal(t, unusedErrorCode, status.Code(err)) - s, ok := status.FromError(err) - require.True(t, ok) - require.Equal(t, status.New(unusedErrorCode, "top-level: deeply: nested"), s) - }) - - t.Run("with status.Error error and %v", func(t *testing.T) { - require.NotEqual(t, tc.expectedCode, unusedErrorCode) - - err := tc.errorf("top-level: %v", status.Error(unusedErrorCode, "nested")) - require.EqualError(t, err, "top-level: nested") - - // The error code of the nested error should be discarded. - require.Equal(t, tc.expectedCode, status.Code(err)) - s, ok := status.FromError(err) - require.True(t, ok) - require.Equal(t, status.New(tc.expectedCode, "top-level: nested"), s) - }) - - t.Run("with status.Error error and %w", func(t *testing.T) { - require.NotEqual(t, tc.expectedCode, unusedErrorCode) - - err := tc.errorf("top-level: %w", status.Error(unusedErrorCode, "nested")) - require.EqualError(t, err, "top-level: nested") - - // We should be reporting the error code of the nested error. - require.Equal(t, unusedErrorCode, status.Code(err)) - s, ok := status.FromError(err) - require.True(t, ok) - require.Equal(t, status.New(unusedErrorCode, "top-level: nested"), s) - }) - - t.Run("multi-nesting gRPC errors", func(t *testing.T) { - require.NotEqual(t, tc.expectedCode, unusedErrorCode) - - err := tc.errorf("first: %w", - ErrNotFoundf("second: %w", - status.Error(unusedErrorCode, "third"), - ), - ) - require.EqualError(t, err, "first: second: third") - - // We should be reporting the error code of the nested error. - require.Equal(t, unusedErrorCode, status.Code(err)) - s, ok := status.FromError(err) - require.True(t, ok) - require.Equal(t, status.New(unusedErrorCode, "first: second: third"), s) - }) - - t.Run("multi-nesting with standard error wrapping", func(t *testing.T) { - require.NotEqual(t, tc.expectedCode, unusedErrorCode) - - err := tc.errorf("first: %w", - fmt.Errorf("second: %w", - formatError(unusedErrorCode, "third"), - ), - ) - require.EqualError(t, err, "first: second: third") - - // We should be reporting the error code of the nested error. - require.Equal(t, unusedErrorCode, status.Code(err)) - s, ok := status.FromError(err) - require.True(t, ok) - require.Equal(t, status.New(unusedErrorCode, "first: second: third"), s) - }) - - t.Run("wrapping formatted gRPC error with details", func(t *testing.T) { - require.NotEqual(t, tc.expectedCode, unusedErrorCode) - - marshaledDetail, err := anypb.New(&grpc_testing.Payload{ - Body: []byte("contents"), - }) - require.NoError(t, err) - - proto := status.New(unusedErrorCode, "details").Proto() - proto.Details = []*anypb.Any{marshaledDetail} - errWithDetails := status.ErrorProto(proto) - - err = tc.errorf("top: %w", fmt.Errorf("detailed: %w", errWithDetails)) - // We can't do anything about the "rpc error:" part in the middle as - // this is put there by `fmt.Errorf()` already. - require.EqualError(t, err, "top: detailed: rpc error: code = OutOfRange desc = details") - // We should be reporting the error code of the wrapped gRPC status. - require.Equal(t, unusedErrorCode, status.Code(err)) - - expectedErr, marshallingErr := status.New(unusedErrorCode, "top: detailed: rpc error: code = OutOfRange desc = details").WithDetails(&grpc_testing.Payload{ - Body: []byte("contents"), - }) - require.NoError(t, marshallingErr) - - s, ok := status.FromError(err) - require.True(t, ok) - require.Equal(t, expectedErr, s) - }) - }) - } -} - func TestGrpcCode(t *testing.T) { t.Parallel() for desc, tc := range map[string]struct { @@ -179,20 +25,16 @@ func TestGrpcCode(t *testing.T) { exp: codes.NotFound, }, "unwrapped status created by helpers": { - in: ErrNotFoundf(""), - exp: codes.NotFound, + in: structerr.NewInternal(""), + exp: codes.Internal, }, "wrapped status created by helpers": { - in: fmt.Errorf("context: %w", ErrNotFoundf("")), - exp: codes.NotFound, + in: fmt.Errorf("context: %w", structerr.NewInternal("")), + exp: codes.Internal, }, "double wrapped status created by helpers": { - in: fmt.Errorf("outer: %w", fmt.Errorf("context: %w", ErrNotFoundf(""))), - exp: codes.NotFound, - }, - "double helper wrapped status": { - in: ErrInvalidArgumentf("outer: %w", fmt.Errorf("context: %w", ErrNotFoundf(""))), - exp: codes.NotFound, + in: fmt.Errorf("outer: %w", fmt.Errorf("context: %w", structerr.NewInternal(""))), + exp: codes.Internal, }, "nil input": { in: nil, diff --git a/internal/middleware/statushandler/statushandler_test.go b/internal/middleware/statushandler/statushandler_test.go index 58ce6c02c..2ca194ca5 100644 --- a/internal/middleware/statushandler/statushandler_test.go +++ b/internal/middleware/statushandler/statushandler_test.go @@ -6,8 +6,8 @@ import ( "testing" "github.com/stretchr/testify/assert" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -56,12 +56,12 @@ func testUnary(t *testing.T, ctx context.Context) { }, "wrapped error": { ctx: ctx, - err: helper.ErrInvalidArgumentf("%w", assert.AnError), + err: structerr.NewInvalidArgument("%w", assert.AnError), expectedErr: status.Error(codes.InvalidArgument, assert.AnError.Error()), }, "formatted wrapped error": { ctx: ctx, - err: fmt.Errorf("cause: %w", helper.ErrInvalidArgumentf("%w", assert.AnError)), + err: fmt.Errorf("cause: %w", structerr.NewInvalidArgument("%w", assert.AnError)), expectedErr: func(ff bool) error { if ff { return status.Error(codes.InvalidArgument, "cause: "+assert.AnError.Error()) @@ -141,12 +141,12 @@ func testStream(t *testing.T, ctx context.Context) { }, "wrapped error": { ctx: ctx, - err: helper.ErrInvalidArgumentf("%w", assert.AnError), + err: structerr.NewInvalidArgument("%w", assert.AnError), expectedErr: status.Error(codes.InvalidArgument, assert.AnError.Error()), }, "formatted wrapped error": { ctx: ctx, - err: fmt.Errorf("cause: %w", helper.ErrInvalidArgumentf("%w", assert.AnError)), + err: fmt.Errorf("cause: %w", structerr.NewInvalidArgument("%w", assert.AnError)), expectedErr: func(ff bool) error { if ff { return status.Error(codes.InvalidArgument, "cause: "+assert.AnError.Error()) diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go index 0c6ecadc6..170c456ee 100644 --- a/internal/praefect/coordinator.go +++ b/internal/praefect/coordinator.go @@ -267,11 +267,11 @@ func (c *Coordinator) directRepositoryScopedMessage(ctx context.Context, call gr if err != nil { if errors.As(err, new(commonerr.RepositoryNotFoundError)) { - return nil, helper.ErrNotFoundf("%w", err) + return nil, structerr.NewNotFound("%w", err) } if errors.Is(err, commonerr.ErrRepositoryNotFound) { - return nil, helper.ErrNotFoundf("%w", err) + return nil, structerr.NewNotFound("%w", err) } return nil, err @@ -377,7 +377,7 @@ func (c *Coordinator) mutatorStreamParameters(ctx context.Context, call grpcCall var additionalRepoRelativePath string if additionalRepo, ok, err := call.methodInfo.AdditionalRepo(call.msg); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } else if ok { additionalRepoRelativePath = additionalRepo.GetRelativePath() } @@ -668,11 +668,11 @@ func (c *Coordinator) StreamDirector(ctx context.Context, fullMethodName string, if mi.Scope == protoregistry.ScopeRepository { targetRepo, err := mi.TargetRepo(m) if err != nil { - return nil, helper.ErrInvalidArgumentf("repo scoped: %w", err) + return nil, structerr.NewInvalidArgument("repo scoped: %w", err) } if err := c.validateTargetRepo(targetRepo); err != nil { - return nil, helper.ErrInvalidArgumentf("repo scoped: %w", err) + return nil, structerr.NewInvalidArgument("repo scoped: %w", err) } sp, err := c.directRepositoryScopedMessage(ctx, grpcCall{ @@ -683,7 +683,7 @@ func (c *Coordinator) StreamDirector(ctx context.Context, fullMethodName string, }) if err != nil { if errors.Is(err, nodes.ErrVirtualStorageNotExist) { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if errors.Is(err, commonerr.ErrRepositoryAlreadyExists) { @@ -705,11 +705,11 @@ func (c *Coordinator) StreamDirector(ctx context.Context, fullMethodName string, func (c *Coordinator) directStorageScopedMessage(ctx context.Context, mi protoregistry.MethodInfo, msg proto.Message) (*proxy.StreamParameters, error) { virtualStorage, err := mi.Storage(msg) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if virtualStorage == "" { - return nil, helper.ErrInvalidArgumentf("storage scoped: target storage is invalid") + return nil, structerr.NewInvalidArgument("storage scoped: target storage is invalid") } var ps *proxy.StreamParameters @@ -728,7 +728,7 @@ func (c *Coordinator) accessorStorageStreamParameters(ctx context.Context, mi pr node, err := c.router.RouteStorageAccessor(ctx, virtualStorage) if err != nil { if errors.Is(err, nodes.ErrVirtualStorageNotExist) { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return nil, structerr.NewInternal("accessor storage scoped: route storage accessor %q: %w", virtualStorage, err) } @@ -737,7 +737,7 @@ func (c *Coordinator) accessorStorageStreamParameters(ctx context.Context, mi pr b, err := rewrittenStorageMessage(mi, msg, node.Storage) if err != nil { - return nil, helper.ErrInvalidArgumentf("accessor storage scoped: %w", err) + return nil, structerr.NewInvalidArgument("accessor storage scoped: %w", err) } // As this is a read operation it could be routed to another storage (not only primary) if it meets constraints @@ -756,7 +756,7 @@ func (c *Coordinator) mutatorStorageStreamParameters(ctx context.Context, mi pro route, err := c.router.RouteStorageMutator(ctx, virtualStorage) if err != nil { if errors.Is(err, nodes.ErrVirtualStorageNotExist) { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return nil, structerr.NewInternal("mutator storage scoped: get shard %q: %w", virtualStorage, err) } @@ -765,7 +765,7 @@ func (c *Coordinator) mutatorStorageStreamParameters(ctx context.Context, mi pro b, err := rewrittenStorageMessage(mi, msg, route.Primary.Storage) if err != nil { - return nil, helper.ErrInvalidArgumentf("mutator storage scoped: %w", err) + return nil, structerr.NewInvalidArgument("mutator storage scoped: %w", err) } primaryDest := proxy.Destination{ @@ -778,7 +778,7 @@ func (c *Coordinator) mutatorStorageStreamParameters(ctx context.Context, mi pro for i, secondary := range route.Secondaries { b, err := rewrittenStorageMessage(mi, msg, secondary.Storage) if err != nil { - return nil, helper.ErrInvalidArgumentf("mutator storage scoped: %w", err) + return nil, structerr.NewInvalidArgument("mutator storage scoped: %w", err) } secondaryDests[i] = proxy.Destination{Ctx: ctx, Conn: secondary.Connection, Msg: b} } @@ -792,7 +792,7 @@ func rewrittenRepositoryMessage(mi protoregistry.MethodInfo, m proto.Message, st m = proto.Clone(m) targetRepo, err := mi.TargetRepo(m) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } // rewrite the target repository @@ -801,7 +801,7 @@ func rewrittenRepositoryMessage(mi protoregistry.MethodInfo, m proto.Message, st additionalRepo, ok, err := mi.AdditionalRepo(m) if err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } if ok { @@ -814,7 +814,7 @@ func rewrittenRepositoryMessage(mi protoregistry.MethodInfo, m proto.Message, st func rewrittenStorageMessage(mi protoregistry.MethodInfo, m proto.Message, storage string) ([]byte, error) { if err := mi.SetStorage(m, storage); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return proxy.NewCodec().Marshal(m) diff --git a/internal/praefect/coordinator_test.go b/internal/praefect/coordinator_test.go index 18dca4943..04cf1aa33 100644 --- a/internal/praefect/coordinator_test.go +++ b/internal/praefect/coordinator_test.go @@ -24,7 +24,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" gconfig "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" gitaly_metadata "gitlab.com/gitlab-org/gitaly/v15/internal/metadata" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/middleware/metadatahandler" @@ -35,6 +34,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/nodes" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/protoregistry" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/transactions" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/promtest" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" @@ -181,7 +181,7 @@ func TestStreamDirectorMutator(t *testing.T) { }, { desc: "repository not found", - error: helper.ErrNotFoundf("%w", fmt.Errorf("mutator call: route repository mutator: %w", fmt.Errorf("get repository id: %w", commonerr.NewRepositoryNotFoundError(targetRepo.StorageName, targetRepo.RelativePath)))), + error: structerr.NewNotFound("%w", fmt.Errorf("mutator call: route repository mutator: %w", fmt.Errorf("get repository id: %w", commonerr.NewRepositoryNotFoundError(targetRepo.StorageName, targetRepo.RelativePath)))), }, } { t.Run(tc.desc, func(t *testing.T) { @@ -676,19 +676,19 @@ func TestStreamDirector_maintenance(t *testing.T) { }, { desc: "primary returns an error", - primaryErr: helper.ErrNotFoundf("primary error"), - expectedErr: helper.ErrNotFoundf("primary error"), + primaryErr: structerr.NewNotFound("primary error"), + expectedErr: structerr.NewNotFound("primary error"), }, { desc: "secondary returns an error", - secondaryErr: helper.ErrNotFoundf("secondary error"), - expectedErr: helper.ErrNotFoundf("secondary error"), + secondaryErr: structerr.NewNotFound("secondary error"), + expectedErr: structerr.NewNotFound("secondary error"), }, { desc: "primary error preferred", - primaryErr: helper.ErrNotFoundf("primary error"), - secondaryErr: helper.ErrNotFoundf("secondary error"), - expectedErr: helper.ErrNotFoundf("primary error"), + primaryErr: structerr.NewNotFound("primary error"), + secondaryErr: structerr.NewNotFound("secondary error"), + expectedErr: structerr.NewNotFound("primary error"), }, } { t.Run(tc.desc, func(t *testing.T) { @@ -1108,7 +1108,7 @@ func TestStreamDirectorAccessor(t *testing.T) { return RepositoryAccessorRoute{}, commonerr.NewRepositoryNotFoundError(virtualStorage, relativePath) }, }, - error: helper.ErrNotFoundf("%w", fmt.Errorf("accessor call: route repository accessor: %w", commonerr.NewRepositoryNotFoundError(targetRepo.StorageName, targetRepo.RelativePath))), + error: structerr.NewNotFound("%w", fmt.Errorf("accessor call: route repository accessor: %w", commonerr.NewRepositoryNotFoundError(targetRepo.StorageName, targetRepo.RelativePath))), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/praefect/delete_object_pool.go b/internal/praefect/delete_object_pool.go index c3c059b6f..bc45dfcf5 100644 --- a/internal/praefect/delete_object_pool.go +++ b/internal/praefect/delete_object_pool.go @@ -7,8 +7,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/housekeeping" "gitlab.com/gitlab-org/gitaly/v15/internal/git/objectpool" objectpoolsvc "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/objectpool" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc" "google.golang.org/protobuf/proto" @@ -30,7 +30,7 @@ func DeleteObjectPoolHandler(rs datastore.RepositoryStore, conns Connections) gr } if !housekeeping.IsRailsPoolRepository(repo) { - return nil, helper.ErrInvalidArgumentf("%w", objectpool.ErrInvalidPoolDir) + return nil, structerr.NewInvalidArgument("%w", objectpool.ErrInvalidPoolDir) } return repo, nil diff --git a/internal/praefect/remove_repository.go b/internal/praefect/remove_repository.go index 5b6ef285d..c1ec91eca 100644 --- a/internal/praefect/remove_repository.go +++ b/internal/praefect/remove_repository.go @@ -9,9 +9,9 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "github.com/sirupsen/logrus" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/commonerr" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc" "google.golang.org/protobuf/proto" @@ -66,7 +66,7 @@ func removeRepositoryHandler(rs datastore.RepositoryStore, conns Connections, pa if errors.As(err, new(commonerr.RepositoryNotFoundError)) { if errorOnNotFound { if errors.As(err, new(commonerr.RepositoryNotFoundError)) { - return helper.ErrNotFoundf("repository does not exist") + return structerr.NewNotFound("repository does not exist") } } diff --git a/internal/praefect/remove_repository_test.go b/internal/praefect/remove_repository_test.go index 4d8917378..7bdce8a76 100644 --- a/internal/praefect/remove_repository_test.go +++ b/internal/praefect/remove_repository_test.go @@ -11,11 +11,11 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/setup" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/grpc-proxy/proxy" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/protoregistry" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testdb" @@ -49,7 +49,7 @@ func TestRemoveRepositoryHandler(t *testing.T) { { desc: "repository not found", repository: &gitalypb.Repository{StorageName: "virtual-storage", RelativePath: "doesn't exist"}, - error: helper.ErrNotFoundf("repository does not exist"), + error: structerr.NewNotFound("repository does not exist"), }, { desc: "repository found", diff --git a/internal/praefect/rename_repository.go b/internal/praefect/rename_repository.go index eb52b08cb..9e48337cc 100644 --- a/internal/praefect/rename_repository.go +++ b/internal/praefect/rename_repository.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/commonerr" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" @@ -19,11 +18,11 @@ func validateRenameRepositoryRequest(req *gitalypb.RenameRepositoryRequest, virt // Gitaly's tested behavior. repository := req.GetRepository() if err := service.ValidateRepository(repository); err != nil { - return helper.ErrInvalidArgumentf("%w", err) + return structerr.NewInvalidArgument("%w", err) } else if req.GetRelativePath() == "" { - return helper.ErrInvalidArgumentf("destination relative path is empty") + return structerr.NewInvalidArgument("destination relative path is empty") } else if _, ok := virtualStorages[repository.GetStorageName()]; !ok { - return helper.ErrInvalidArgumentf("GetStorageByName: no such storage: %q", repository.GetStorageName()) + return structerr.NewInvalidArgument("GetStorageByName: no such storage: %q", repository.GetStorageName()) } else if _, err := storage.ValidateRelativePath("/fake-root", req.GetRelativePath()); err != nil { // Gitaly uses ValidateRelativePath to verify there are no traversals, so we use the same function // here. Praefect is not susceptible to path traversals as it generates its own disk paths but we @@ -31,7 +30,7 @@ func validateRenameRepositoryRequest(req *gitalypb.RenameRepositoryRequest, virt // seeing whether the relative path escapes the root directory. It's not possible to traverse up // from the /, so the traversals in the path wouldn't be caught. To allow for the check to work, // we use the /fake-root directory simply to notice if there were traversals in the path. - return helper.ErrInvalidArgumentf("GetRepoPath: %s", err) + return structerr.NewInvalidArgument("GetRepoPath: %s", err) } return nil @@ -61,7 +60,7 @@ func RenameRepositoryHandler(virtualStoragesNames []string, rs datastore.Reposit req.GetRelativePath(), ); err != nil { if errors.Is(err, commonerr.ErrRepositoryNotFound) { - return helper.ErrNotFoundf( + return structerr.NewNotFound( `GetRepoPath: not a git repository: "%s/%s"`, req.GetRepository().GetStorageName(), req.GetRepository().GetRelativePath(), diff --git a/internal/praefect/server_test.go b/internal/praefect/server_test.go index 91134c591..c71c466fd 100644 --- a/internal/praefect/server_test.go +++ b/internal/praefect/server_test.go @@ -25,7 +25,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/setup" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/listenmux" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore" @@ -643,7 +642,7 @@ func TestRenameRepository(t *testing.T) { }, RelativePath: virtualRepo2.RelativePath, }) - testhelper.RequireGrpcError(t, helper.ErrNotFoundf(`GetRepoPath: not a git repository: "praefect/not-found"`), err) + testhelper.RequireGrpcError(t, structerr.NewNotFound(`GetRepoPath: not a git repository: "praefect/not-found"`), err) _, err = repoServiceClient.RenameRepository(ctx, &gitalypb.RenameRepositoryRequest{ Repository: virtualRepo1, diff --git a/internal/praefect/service/info/metadata.go b/internal/praefect/service/info/metadata.go index 91d49d4aa..97c57ae77 100644 --- a/internal/praefect/service/info/metadata.go +++ b/internal/praefect/service/info/metadata.go @@ -4,7 +4,6 @@ import ( "context" "errors" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/commonerr" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" @@ -31,7 +30,7 @@ func (s *Server) GetRepositoryMetadata(ctx context.Context, req *gitalypb.GetRep metadata, err := getMetadata() if err != nil { if errors.Is(err, commonerr.ErrRepositoryNotFound) { - return nil, helper.ErrNotFoundf("%w", err) + return nil, structerr.NewNotFound("%w", err) } return nil, structerr.NewInternal("get metadata: %w", err) diff --git a/internal/praefect/service/info/replication_factor.go b/internal/praefect/service/info/replication_factor.go index 9f799bd26..82df2968f 100644 --- a/internal/praefect/service/info/replication_factor.go +++ b/internal/praefect/service/info/replication_factor.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -17,7 +16,7 @@ func (s *Server) SetReplicationFactor(ctx context.Context, req *gitalypb.SetRepl if err != nil { var invalidArg datastore.InvalidArgumentError if errors.As(err, &invalidArg) { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } return nil, structerr.NewInternal("%w", err) diff --git a/internal/praefect/service/info/server.go b/internal/praefect/service/info/server.go index 5b5ba0f69..8a6654f50 100644 --- a/internal/praefect/service/info/server.go +++ b/internal/praefect/service/info/server.go @@ -4,7 +4,6 @@ import ( "context" "errors" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/commonerr" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore" @@ -65,7 +64,7 @@ func NewServer( func (s *Server) SetAuthoritativeStorage(ctx context.Context, req *gitalypb.SetAuthoritativeStorageRequest) (*gitalypb.SetAuthoritativeStorageResponse, error) { storages := s.conf.StorageNames()[req.VirtualStorage] if storages == nil { - return nil, helper.ErrInvalidArgumentf("unknown virtual storage: %q", req.VirtualStorage) + return nil, structerr.NewInvalidArgument("unknown virtual storage: %q", req.VirtualStorage) } foundStorage := false @@ -77,12 +76,12 @@ func (s *Server) SetAuthoritativeStorage(ctx context.Context, req *gitalypb.SetA } if !foundStorage { - return nil, helper.ErrInvalidArgumentf("unknown authoritative storage: %q", req.AuthoritativeStorage) + return nil, structerr.NewInvalidArgument("unknown authoritative storage: %q", req.AuthoritativeStorage) } if err := s.rs.SetAuthoritativeReplica(ctx, req.VirtualStorage, req.RelativePath, req.AuthoritativeStorage); err != nil { if errors.As(err, &commonerr.RepositoryNotFoundError{}) { - return nil, helper.ErrInvalidArgumentf("repository %q does not exist on virtual storage %q", req.RelativePath, req.VirtualStorage) + return nil, structerr.NewInvalidArgument("repository %q does not exist on virtual storage %q", req.RelativePath, req.VirtualStorage) } return nil, structerr.NewInternal("%w", err) diff --git a/internal/praefect/service/server/clocksynced.go b/internal/praefect/service/server/clocksynced.go index 80a325a36..373489b05 100644 --- a/internal/praefect/service/server/clocksynced.go +++ b/internal/praefect/service/server/clocksynced.go @@ -4,13 +4,14 @@ import ( "context" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) // ClockSynced returns whether the system clock has an acceptable time drift when compared to NTP service. func (s *Server) ClockSynced(_ context.Context, req *gitalypb.ClockSyncedRequest) (*gitalypb.ClockSyncedResponse, error) { if err := req.DriftThreshold.CheckValid(); err != nil { - return nil, helper.ErrInvalidArgumentf("%w", err) + return nil, structerr.NewInvalidArgument("%w", err) } synced, err := helper.CheckClockSync(req.NtpHost, req.DriftThreshold.AsDuration()) if err != nil { diff --git a/internal/praefect/service/transaction/server.go b/internal/praefect/service/transaction/server.go index a2cdb7848..f4b2baec5 100644 --- a/internal/praefect/service/transaction/server.go +++ b/internal/praefect/service/transaction/server.go @@ -4,7 +4,6 @@ import ( "context" "errors" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/transactions" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" @@ -30,13 +29,13 @@ func NewServer(txMgr *transactions.Manager) gitalypb.RefTransactionServer { func (s *Server) VoteTransaction(ctx context.Context, in *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error) { vote, err := voting.VoteFromHash(in.GetReferenceUpdatesHash()) if err != nil { - return nil, helper.ErrInvalidArgumentf("invalid reference update hash: %v", err) + return nil, structerr.NewInvalidArgument("invalid reference update hash: %v", err) } if err := s.txMgr.VoteTransaction(ctx, in.TransactionId, in.Node, vote); err != nil { switch { case errors.Is(err, transactions.ErrNotFound): - return nil, helper.ErrNotFoundf("%w", err) + return nil, structerr.NewNotFound("%w", err) case errors.Is(err, transactions.ErrTransactionCanceled): return nil, structerr.NewCanceled("%w", err) case errors.Is(err, transactions.ErrTransactionStopped): @@ -65,7 +64,7 @@ func (s *Server) StopTransaction(ctx context.Context, in *gitalypb.StopTransacti if err := s.txMgr.StopTransaction(ctx, in.TransactionId); err != nil { switch { case errors.Is(err, transactions.ErrNotFound): - return nil, helper.ErrNotFoundf("%w", err) + return nil, structerr.NewNotFound("%w", err) case errors.Is(err, transactions.ErrTransactionCanceled): return nil, structerr.NewCanceled("%w", err) case errors.Is(err, transactions.ErrTransactionStopped): |