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 /internal/gitaly/hook/prereceive_test.go | |
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 'internal/gitaly/hook/prereceive_test.go')
-rw-r--r-- | internal/gitaly/hook/prereceive_test.go | 7 |
1 files changed, 6 insertions, 1 deletions
diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go index 479aa06b9..eafeb5657 100644 --- a/internal/gitaly/hook/prereceive_test.go +++ b/internal/gitaly/hook/prereceive_test.go @@ -306,7 +306,12 @@ func TestPrereceive_gitlab(t *testing.T) { return false, "you shall not pass", nil }, expectHookCall: false, - expectedErr: NotAllowedError{Message: "you shall not pass"}, + expectedErr: NotAllowedError{ + Message: "you shall not pass", + Protocol: "web", + UserID: "1234", + Changes: []byte("changes\n"), + }, }, { desc: "allowed returns error", |