Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <psteinhardt@gitlab.com>2023-06-27 12:40:50 +0300
committerPatrick Steinhardt <psteinhardt@gitlab.com>2023-06-28 08:18:46 +0300
commitb1919c1ec2ef38c62ee6563e4eb6c0a93e8eddb7 (patch)
tree274f9ad32cd51807af03c272d4f617c3530455da
parent13c9342533169e26f40af638e76d9d5b1782bcd8 (diff)
gitaly/service: Drop error prefixes when validating requests
Some of our RPCs provide error prefixes when validating the incoming request. This is not best practice, and most of our RPCs will return validation errors directly. Remove the error prefixes. This is a preparatory step to unify Gitaly and Praefect errors.
-rw-r--r--internal/gitaly/service/commit/commit_signatures.go2
-rw-r--r--internal/gitaly/service/commit/commit_signatures_test.go6
-rw-r--r--internal/gitaly/service/operations/tags.go2
-rw-r--r--internal/gitaly/service/operations/tags_test.go22
-rw-r--r--internal/gitaly/service/repository/backup_repository.go2
-rw-r--r--internal/gitaly/service/repository/backup_repository_test.go4
-rw-r--r--internal/gitaly/service/repository/create_bundle.go2
-rw-r--r--internal/gitaly/service/repository/create_bundle_test.go2
-rw-r--r--internal/gitaly/service/repository/create_repository_from_bundle.go2
-rw-r--r--internal/gitaly/service/repository/create_repository_from_bundle_test.go2
-rw-r--r--internal/gitaly/service/repository/create_repository_from_url.go2
-rw-r--r--internal/gitaly/service/repository/create_repository_from_url_test.go5
-rw-r--r--internal/gitaly/service/repository/get_custom_hooks.go4
-rw-r--r--internal/gitaly/service/repository/get_custom_hooks_test.go4
-rw-r--r--internal/gitaly/service/repository/set_custom_hooks.go4
-rw-r--r--internal/gitaly/service/repository/set_custom_hooks_test.go2
16 files changed, 33 insertions, 34 deletions
diff --git a/internal/gitaly/service/commit/commit_signatures.go b/internal/gitaly/service/commit/commit_signatures.go
index f054497e3..a4a83b560 100644
--- a/internal/gitaly/service/commit/commit_signatures.go
+++ b/internal/gitaly/service/commit/commit_signatures.go
@@ -19,7 +19,7 @@ var gpgSiganturePrefix = []byte("gpgsig")
func (s *server) GetCommitSignatures(request *gitalypb.GetCommitSignaturesRequest, stream gitalypb.CommitService_GetCommitSignaturesServer) error {
if err := validateGetCommitSignaturesRequest(s.locator, request); err != nil {
- return structerr.NewInvalidArgument("GetCommitSignatures: %w", err)
+ return structerr.NewInvalidArgument("%w", err)
}
return s.getCommitSignatures(request, stream)
diff --git a/internal/gitaly/service/commit/commit_signatures_test.go b/internal/gitaly/service/commit/commit_signatures_test.go
index 1d858b502..3ddafa13e 100644
--- a/internal/gitaly/service/commit/commit_signatures_test.go
+++ b/internal/gitaly/service/commit/commit_signatures_test.go
@@ -107,7 +107,7 @@ func TestFailedGetCommitSignaturesRequest(t *testing.T) {
CommitIds: []string{"5937ac0a7beb003549fc5fd26fc247adbce4a52e"},
},
expectedErr: testhelper.GitalyOrPraefect(
- structerr.NewInvalidArgument("GetCommitSignatures: %w", storage.ErrRepositoryNotSet),
+ structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
structerr.NewInvalidArgument("repo scoped: %w", storage.ErrRepositoryNotSet),
),
},
@@ -117,7 +117,7 @@ func TestFailedGetCommitSignaturesRequest(t *testing.T) {
Repository: repo,
CommitIds: []string{},
},
- expectedErr: status.Error(codes.InvalidArgument, "GetCommitSignatures: empty CommitIds"),
+ expectedErr: status.Error(codes.InvalidArgument, "empty CommitIds"),
},
{
desc: "commitIDS with shorthand sha",
@@ -125,7 +125,7 @@ func TestFailedGetCommitSignaturesRequest(t *testing.T) {
Repository: repo,
CommitIds: []string{"5937ac0a7beb003549fc5fd26fc247adbce4a52e", "a17a9f6"},
},
- expectedErr: status.Error(codes.InvalidArgument, fmt.Sprintf(`GetCommitSignatures: invalid object ID: "a17a9f6", expected length %v, got 7`, gittest.DefaultObjectHash.EncodedLen())),
+ expectedErr: status.Error(codes.InvalidArgument, fmt.Sprintf(`invalid object ID: "a17a9f6", expected length %v, got 7`, gittest.DefaultObjectHash.EncodedLen())),
},
}
diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go
index e4108f4cb..6e6cc1e0e 100644
--- a/internal/gitaly/service/operations/tags.go
+++ b/internal/gitaly/service/operations/tags.go
@@ -119,7 +119,7 @@ func validateUserCreateTag(locator storage.Locator, req *gitalypb.UserCreateTagR
//nolint:revive // This is unintentionally missing documentation.
func (s *Server) UserCreateTag(ctx context.Context, req *gitalypb.UserCreateTagRequest) (*gitalypb.UserCreateTagResponse, error) {
if err := validateUserCreateTag(s.locator, req); err != nil {
- return nil, structerr.NewInvalidArgument("validating request: %w", err)
+ return nil, structerr.NewInvalidArgument("%w", err)
}
targetRevision := git.Revision(req.TargetRevision)
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index 4eb891c4b..85da22421 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -656,7 +656,7 @@ func TestUserCreateTag_message(t *testing.T) {
{
desc: "error: contains null byte",
message: "\000",
- expectedErr: structerr.NewInvalidArgument("validating request: tag message contains NUL byte"),
+ expectedErr: structerr.NewInvalidArgument("tag message contains NUL byte"),
},
{
desc: "annotated: some control characters",
@@ -1321,7 +1321,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
targetRevision: "main",
user: gittest.TestUser,
expectedErr: testhelper.GitalyOrPraefect(
- structerr.NewInvalidArgument("validating request: %w", storage.ErrRepositoryNotSet),
+ structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
structerr.NewInvalidArgument("repo scoped: %w", storage.ErrRepositoryNotSet),
),
},
@@ -1331,7 +1331,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
tagName: "shiny-new-tag",
targetRevision: "",
user: gittest.TestUser,
- expectedErr: structerr.NewInvalidArgument("validating request: empty target revision"),
+ expectedErr: structerr.NewInvalidArgument("empty target revision"),
},
{
desc: "empty user",
@@ -1339,7 +1339,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
tagName: "shiny-new-tag",
targetRevision: "main",
user: nil,
- expectedErr: structerr.NewInvalidArgument("validating request: empty user"),
+ expectedErr: structerr.NewInvalidArgument("empty user"),
},
{
desc: "empty starting point",
@@ -1347,7 +1347,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
tagName: "new-tag",
targetRevision: "",
user: gittest.TestUser,
- expectedErr: structerr.NewInvalidArgument("validating request: empty target revision"),
+ expectedErr: structerr.NewInvalidArgument("empty target revision"),
},
{
desc: "non-existing starting point",
@@ -1363,7 +1363,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
tagName: "a tag",
targetRevision: "main",
user: gittest.TestUser,
- expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"),
+ expectedErr: structerr.NewInvalidArgument("invalid tag name: revision can't contain whitespace"),
},
{
desc: "space in annotated tag name",
@@ -1372,7 +1372,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
targetRevision: "main",
message: "a message",
user: gittest.TestUser,
- expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"),
+ expectedErr: structerr.NewInvalidArgument("invalid tag name: revision can't contain whitespace"),
},
{
desc: "newline in lightweight tag name",
@@ -1380,7 +1380,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
tagName: "a\ntag",
targetRevision: "main",
user: gittest.TestUser,
- expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"),
+ expectedErr: structerr.NewInvalidArgument("invalid tag name: revision can't contain whitespace"),
},
{
desc: "newline in annotated tag name",
@@ -1389,7 +1389,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
targetRevision: "main",
message: "a message",
user: gittest.TestUser,
- expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"),
+ expectedErr: structerr.NewInvalidArgument("invalid tag name: revision can't contain whitespace"),
},
{
desc: "injection in lightweight tag name",
@@ -1397,7 +1397,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
tagName: injectedTag,
targetRevision: "main",
user: gittest.TestUser,
- expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"),
+ expectedErr: structerr.NewInvalidArgument("invalid tag name: revision can't contain whitespace"),
},
{
desc: "injection in annotated tag name",
@@ -1406,7 +1406,7 @@ func TestUserCreateTag_invalidArgument(t *testing.T) {
targetRevision: "main",
message: "a message",
user: gittest.TestUser,
- expectedErr: structerr.NewInvalidArgument("validating request: invalid tag name: revision can't contain whitespace"),
+ expectedErr: structerr.NewInvalidArgument("invalid tag name: revision can't contain whitespace"),
},
} {
t.Run(tc.desc, func(t *testing.T) {
diff --git a/internal/gitaly/service/repository/backup_repository.go b/internal/gitaly/service/repository/backup_repository.go
index 8e9eea55f..c3b927125 100644
--- a/internal/gitaly/service/repository/backup_repository.go
+++ b/internal/gitaly/service/repository/backup_repository.go
@@ -16,7 +16,7 @@ func (s *server) BackupRepository(ctx context.Context, in *gitalypb.BackupReposi
return nil, structerr.NewFailedPrecondition("backup repository: server-side backups are not configured")
}
if err := s.validateBackupRepositoryRequest(in); err != nil {
- return nil, structerr.NewInvalidArgument("backup repository: %w", err)
+ return nil, structerr.NewInvalidArgument("%w", err)
}
manager := backup.NewManagerLocal(
diff --git a/internal/gitaly/service/repository/backup_repository_test.go b/internal/gitaly/service/repository/backup_repository_test.go
index 82d76ce91..441df4a34 100644
--- a/internal/gitaly/service/repository/backup_repository_test.go
+++ b/internal/gitaly/service/repository/backup_repository_test.go
@@ -70,7 +70,7 @@ func TestServerBackupRepository(t *testing.T) {
backupID: "",
}
},
- expectedErr: structerr.NewInvalidArgument("backup repository: empty BackupId"),
+ expectedErr: structerr.NewInvalidArgument("empty BackupId"),
},
{
desc: "missing repository",
@@ -88,7 +88,7 @@ func TestServerBackupRepository(t *testing.T) {
}
},
expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect(
- "backup repository: repository not set",
+ "repository not set",
"repo scoped: repository not set",
)),
},
diff --git a/internal/gitaly/service/repository/create_bundle.go b/internal/gitaly/service/repository/create_bundle.go
index 9faa2d7f7..1e2733504 100644
--- a/internal/gitaly/service/repository/create_bundle.go
+++ b/internal/gitaly/service/repository/create_bundle.go
@@ -12,7 +12,7 @@ func (s *server) CreateBundle(req *gitalypb.CreateBundleRequest, stream gitalypb
repository := req.GetRepository()
if err := s.locator.ValidateRepository(repository); err != nil {
- return structerr.NewInvalidArgument("CreateBundle: %w", err)
+ return structerr.NewInvalidArgument("%w", err)
}
repo := s.localrepo(repository)
diff --git a/internal/gitaly/service/repository/create_bundle_test.go b/internal/gitaly/service/repository/create_bundle_test.go
index c34dc8e4a..d798694e7 100644
--- a/internal/gitaly/service/repository/create_bundle_test.go
+++ b/internal/gitaly/service/repository/create_bundle_test.go
@@ -77,7 +77,7 @@ func TestFailedCreateBundleRequestDueToValidations(t *testing.T) {
desc: "empty repository",
request: &gitalypb.CreateBundleRequest{},
expectedErr: testhelper.GitalyOrPraefect(
- structerr.NewInvalidArgument("CreateBundle: %w", storage.ErrRepositoryNotSet),
+ structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
structerr.NewInvalidArgument("repo scoped: %w", storage.ErrRepositoryNotSet),
),
},
diff --git a/internal/gitaly/service/repository/create_repository_from_bundle.go b/internal/gitaly/service/repository/create_repository_from_bundle.go
index c079a3f71..4e97ccb92 100644
--- a/internal/gitaly/service/repository/create_repository_from_bundle.go
+++ b/internal/gitaly/service/repository/create_repository_from_bundle.go
@@ -20,7 +20,7 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
repo := firstRequest.GetRepository()
if err := s.locator.ValidateRepository(repo, storage.WithSkipRepositoryExistenceCheck()); err != nil {
- return structerr.NewInvalidArgument("CreateRepositoryFromBundle: %w", err)
+ return structerr.NewInvalidArgument("%w", err)
}
ctx := stream.Context()
diff --git a/internal/gitaly/service/repository/create_repository_from_bundle_test.go b/internal/gitaly/service/repository/create_repository_from_bundle_test.go
index 69edaedbd..8403df23d 100644
--- a/internal/gitaly/service/repository/create_repository_from_bundle_test.go
+++ b/internal/gitaly/service/repository/create_repository_from_bundle_test.go
@@ -255,7 +255,7 @@ func TestCreateRepositoryFromBundle_invalidArgument(t *testing.T) {
_, err = stream.CloseAndRecv()
testhelper.RequireGrpcError(t, testhelper.GitalyOrPraefect(
- structerr.NewInvalidArgument("CreateRepositoryFromBundle: %w", storage.ErrRepositoryNotSet),
+ structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
structerr.NewInvalidArgument("repo scoped: %w", storage.ErrRepositoryNotSet),
), err)
}
diff --git a/internal/gitaly/service/repository/create_repository_from_url.go b/internal/gitaly/service/repository/create_repository_from_url.go
index 9479b42fa..4ed9daed5 100644
--- a/internal/gitaly/service/repository/create_repository_from_url.go
+++ b/internal/gitaly/service/repository/create_repository_from_url.go
@@ -78,7 +78,7 @@ func (s *server) cloneFromURLCommand(
func (s *server) CreateRepositoryFromURL(ctx context.Context, req *gitalypb.CreateRepositoryFromURLRequest) (*gitalypb.CreateRepositoryFromURLResponse, error) {
if err := validateCreateRepositoryFromURLRequest(s.locator, req); err != nil {
- return nil, structerr.NewInvalidArgument("CreateRepositoryFromURL: %w", err)
+ return nil, structerr.NewInvalidArgument("%w", err)
}
if err := repoutil.Create(ctx, s.locator, s.gitCmdFactory, s.txManager, req.GetRepository(), func(repo *gitalypb.Repository) error {
diff --git a/internal/gitaly/service/repository/create_repository_from_url_test.go b/internal/gitaly/service/repository/create_repository_from_url_test.go
index 4074f4214..4a732c4a0 100644
--- a/internal/gitaly/service/repository/create_repository_from_url_test.go
+++ b/internal/gitaly/service/repository/create_repository_from_url_test.go
@@ -21,7 +21,6 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
"google.golang.org/grpc/codes"
- "google.golang.org/grpc/status"
)
func TestCreateRepositoryFromURL_successful(t *testing.T) {
@@ -339,14 +338,14 @@ func TestServer_CloneFromURLCommand_validate(t *testing.T) {
desc: "no repository provided",
req: &gitalypb.CreateRepositoryFromURLRequest{Repository: nil},
expectedErr: testhelper.GitalyOrPraefect(
- structerr.NewInvalidArgument("CreateRepositoryFromURL: %w", storage.ErrRepositoryNotSet),
+ structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
structerr.NewInvalidArgument("repo scoped: %w", storage.ErrRepositoryNotSet),
),
},
{
desc: "no URL provided",
req: &gitalypb.CreateRepositoryFromURLRequest{Repository: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "new"}, Url: ""},
- expectedErr: status.Error(codes.InvalidArgument, "CreateRepositoryFromURL: empty Url"),
+ expectedErr: structerr.NewInvalidArgument("empty Url"),
},
}
diff --git a/internal/gitaly/service/repository/get_custom_hooks.go b/internal/gitaly/service/repository/get_custom_hooks.go
index a3ab82195..0e67f353c 100644
--- a/internal/gitaly/service/repository/get_custom_hooks.go
+++ b/internal/gitaly/service/repository/get_custom_hooks.go
@@ -14,7 +14,7 @@ func (s *server) GetCustomHooks(in *gitalypb.GetCustomHooksRequest, stream gital
ctx := stream.Context()
if err := s.locator.ValidateRepository(in.GetRepository()); err != nil {
- return structerr.NewInvalidArgument("validating repository: %w", err)
+ return structerr.NewInvalidArgument("%w", err)
}
writer := streamio.NewWriter(func(p []byte) error {
@@ -35,7 +35,7 @@ func (s *server) BackupCustomHooks(in *gitalypb.BackupCustomHooksRequest, stream
ctx := stream.Context()
if err := s.locator.ValidateRepository(in.GetRepository()); err != nil {
- return structerr.NewInvalidArgument("validating repository: %w", err)
+ return structerr.NewInvalidArgument("%w", err)
}
writer := streamio.NewWriter(func(p []byte) error {
diff --git a/internal/gitaly/service/repository/get_custom_hooks_test.go b/internal/gitaly/service/repository/get_custom_hooks_test.go
index 7d60a6fc0..645ad9115 100644
--- a/internal/gitaly/service/repository/get_custom_hooks_test.go
+++ b/internal/gitaly/service/repository/get_custom_hooks_test.go
@@ -211,7 +211,7 @@ func TestGetCustomHooks_validate(t *testing.T) {
desc: "repository not provided",
req: &gitalypb.GetCustomHooksRequest{Repository: nil},
expectedErr: testhelper.GitalyOrPraefect(
- structerr.NewInvalidArgument("validating repository: %w", storage.ErrRepositoryNotSet),
+ structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
structerr.NewInvalidArgument("repo scoped: %w", storage.ErrRepositoryNotSet),
),
},
@@ -240,7 +240,7 @@ func TestBackupCustomHooks_validate(t *testing.T) {
desc: "repository not provided",
req: &gitalypb.BackupCustomHooksRequest{Repository: nil},
expectedErr: testhelper.GitalyOrPraefect(
- structerr.NewInvalidArgument("validating repository: %w", storage.ErrRepositoryNotSet),
+ structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
structerr.NewInvalidArgument("repo scoped: %w", storage.ErrRepositoryNotSet),
),
},
diff --git a/internal/gitaly/service/repository/set_custom_hooks.go b/internal/gitaly/service/repository/set_custom_hooks.go
index 1d2d1eab9..517f73732 100644
--- a/internal/gitaly/service/repository/set_custom_hooks.go
+++ b/internal/gitaly/service/repository/set_custom_hooks.go
@@ -20,7 +20,7 @@ func (s *server) SetCustomHooks(stream gitalypb.RepositoryService_SetCustomHooks
repo := firstRequest.GetRepository()
if err := s.locator.ValidateRepository(repo); err != nil {
- return structerr.NewInvalidArgument("validating repo: %w", err)
+ return structerr.NewInvalidArgument("%w", err)
}
reader := streamio.NewReader(func() ([]byte, error) {
@@ -54,7 +54,7 @@ func (s *server) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCus
repo := firstRequest.GetRepository()
if err := s.locator.ValidateRepository(repo); err != nil {
- return structerr.NewInvalidArgument("validating repo: %w", err)
+ return structerr.NewInvalidArgument("%w", err)
}
reader := streamio.NewReader(func() ([]byte, error) {
diff --git a/internal/gitaly/service/repository/set_custom_hooks_test.go b/internal/gitaly/service/repository/set_custom_hooks_test.go
index bc48b9df3..f48ec9ca7 100644
--- a/internal/gitaly/service/repository/set_custom_hooks_test.go
+++ b/internal/gitaly/service/repository/set_custom_hooks_test.go
@@ -170,7 +170,7 @@ func TestSetCustomHooks_failedValidation(t *testing.T) {
err := tc.streamSender(t, ctx, client)
testhelper.RequireGrpcError(t, testhelper.GitalyOrPraefect(
- structerr.NewInvalidArgument("validating repo: %w", storage.ErrRepositoryNotSet),
+ structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
structerr.NewInvalidArgument("repo scoped: %w", storage.ErrRepositoryNotSet),
), err)
})