diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-08-30 13:06:19 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-08-30 13:13:01 +0300 |
commit | e542a7d87461c7a8c83c17640b37b53387040bb0 (patch) | |
tree | af1e6c4959145e4e5ed349a6e941a405371a59a0 /ruby | |
parent | 2afa4ccd943a2e5267a05fd0f4fc373a4e50bce9 (diff) |
operations: Implement rich errors for UserMergeBranch access checks
When `UserMergeBranch` fails during hook execution, then we will not
cause the RPC to fail but instead embed the error in the response's
`PreReceiveError` field. This has led to quite some problems: Praefect
cannot determine failing RPC calls, we need to do weird games where we
need to make sure to pass through Rails' error messages verbosely, and
most importantly the upstream caller cannot easily deduce what really
has failed without comparing returned error messages. As a result, this
pattern which we use across many of our user-facing RPCs is quite
lacking.
To fix this, we can use gRPC's rich error model. This error model allows
us to embed arbitrary proto messages into the returned error, which
grants us the ability to return structured errors to the caller. Like
this, we can embed various information into errors in a structured way
which can then be extracted by upstream callers simply by unmarshalling
those error details.
Convert UserMergeBranch to use this rich error model as a proof concept.
For now, rich errors only include details in case the access checks help
to keep this change as focussed as possible. If this new model proves
viable, we may extend this to also include additional information in
different error cases, like for example the set of conflicting files in
case a merge fails.
Changelog: changed
Diffstat (limited to 'ruby')
-rw-r--r-- | ruby/proto/gitaly/errors_pb.rb | 19 | ||||
-rw-r--r-- | ruby/proto/gitaly/operations_pb.rb | 7 |
2 files changed, 26 insertions, 0 deletions
diff --git a/ruby/proto/gitaly/errors_pb.rb b/ruby/proto/gitaly/errors_pb.rb new file mode 100644 index 000000000..e2c7a0ae8 --- /dev/null +++ b/ruby/proto/gitaly/errors_pb.rb @@ -0,0 +1,19 @@ +# Generated by the protocol buffer compiler. DO NOT EDIT! +# source: errors.proto + +require 'google/protobuf' + +Google::Protobuf::DescriptorPool.generated_pool.build do + add_file("errors.proto", :syntax => :proto3) do + add_message "gitaly.AccessCheckError" do + optional :error_message, :string, 1 + optional :protocol, :string, 2 + optional :user_id, :string, 3 + optional :changes, :bytes, 4 + end + end +end + +module Gitaly + AccessCheckError = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("gitaly.AccessCheckError").msgclass +end diff --git a/ruby/proto/gitaly/operations_pb.rb b/ruby/proto/gitaly/operations_pb.rb index 9ee34cea9..24e6f9197 100644 --- a/ruby/proto/gitaly/operations_pb.rb +++ b/ruby/proto/gitaly/operations_pb.rb @@ -5,6 +5,7 @@ require 'google/protobuf' require 'lint_pb' require 'shared_pb' +require 'errors_pb' require 'google/protobuf/timestamp_pb' Google::Protobuf::DescriptorPool.generated_pool.build do add_file("operations.proto", :syntax => :proto3) do @@ -71,6 +72,11 @@ Google::Protobuf::DescriptorPool.generated_pool.build do optional :branch_update, :message, 3, "gitaly.OperationBranchUpdate" optional :pre_receive_error, :string, 4 end + add_message "gitaly.UserMergeBranchError" do + oneof :error do + optional :access_check, :message, 1, "gitaly.AccessCheckError" + end + end add_message "gitaly.UserMergeToRefRequest" do optional :repository, :message, 1, "gitaly.Repository" optional :user, :message, 2, "gitaly.User" @@ -277,6 +283,7 @@ module Gitaly UserCreateTagResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("gitaly.UserCreateTagResponse").msgclass UserMergeBranchRequest = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("gitaly.UserMergeBranchRequest").msgclass UserMergeBranchResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("gitaly.UserMergeBranchResponse").msgclass + UserMergeBranchError = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("gitaly.UserMergeBranchError").msgclass UserMergeToRefRequest = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("gitaly.UserMergeToRefRequest").msgclass UserMergeToRefResponse = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("gitaly.UserMergeToRefResponse").msgclass OperationBranchUpdate = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("gitaly.OperationBranchUpdate").msgclass |