Commit aa2c6366 authored by David Kim's avatar David Kim

Handle newly introduced UserMergeBranchError from gitaly

parent 54fb0a01
---
name: gitaly_user_merge_branch_access_error
introduced_by_url: https://gitlab.com/gitlab-org/gitaly/-/merge_requests/3705
rollout_issue_url: https://gitlab.com/gitlab-org/gitaly/-/issues/3757
milestone: '14.3'
type: development
group: group::gitaly
default_enabled: false
...@@ -162,6 +162,14 @@ module Gitlab ...@@ -162,6 +162,14 @@ module Gitlab
raise Gitlab::Git::CommitError, 'failed to apply merge to branch' unless branch_update.commit_id.present? raise Gitlab::Git::CommitError, 'failed to apply merge to branch' unless branch_update.commit_id.present?
Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update) Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update)
rescue GRPC::BadStatus => e
decoded_error = decode_detailed_error(e)
raise unless decoded_error.present?
raise decoded_error
ensure ensure
request_enum.close request_enum.close
end end
...@@ -471,6 +479,31 @@ module Gitlab ...@@ -471,6 +479,31 @@ module Gitlab
rescue RangeError rescue RangeError
raise ArgumentError, "Unknown action '#{action[:action]}'" raise ArgumentError, "Unknown action '#{action[:action]}'"
end end
def decode_detailed_error(err)
# details could have more than one in theory, but we only have one to worry about for now.
detailed_error = err.to_rpc_status&.details&.first
return unless detailed_error.present?
prefix = %r{type\.googleapis\.com\/gitaly\.(?<error_type>.+)}
error_type = prefix.match(detailed_error.type_url)[:error_type]
detailed_error = Gitaly.const_get(error_type, false).decode(detailed_error.value)
case detailed_error.error
when :access_check
access_check_error = detailed_error.access_check
# These messages were returned from internal/allowed API calls
Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message)
else
# We're handling access_check only for now, but we'll add more detailed error types
nil
end
rescue NameError, NoMethodError
# Error Class might not be known to ruby yet
nil
end
end end
end end
end end
...@@ -169,6 +169,56 @@ RSpec.describe Gitlab::GitalyClient::OperationService do ...@@ -169,6 +169,56 @@ RSpec.describe Gitlab::GitalyClient::OperationService do
end end
end end
describe '#user_merge_branch' do
let(:target_branch) { 'master' }
let(:source_sha) { '5937ac0a7beb003549fc5fd26fc247adbce4a52e' }
let(:message) { 'Merge a branch' }
subject { client.user_merge_branch(user, source_sha, target_branch, message) {} }
it 'sends a user_merge_branch message' do
expect(subject).to be_a(Gitlab::Git::OperationService::BranchUpdate)
expect(subject.newrev).to be_present
expect(subject.repo_created).to be(false)
expect(subject.branch_created).to be(false)
end
context 'with an exception with the UserMergeBranchError' do
let(:permission_error) do
GRPC::PermissionDenied.new(
"GitLab: You are not allowed to push code to this project.",
{ "grpc-status-details-bin" =>
"\b\a\x129GitLab: You are not allowed to push code to this project.\x1A\xDE\x01\n/type.googleapis.com/gitaly.UserMergeBranchError\x12\xAA\x01\n\xA7\x01\n1You are not allowed to push code to this project.\x12\x03web\x1A\auser-15\"df15b32277d2c55c6c595845a87109b09c913c556 5d6e0f935ad9240655f64e883cd98fad6f9a17ee refs/heads/master\n" }
)
end
it 'raises PreRecieveError with the error message' do
expect_any_instance_of(Gitaly::OperationService::Stub)
.to receive(:user_merge_branch).with(kind_of(Enumerator), kind_of(Hash))
.and_raise(permission_error)
expect { subject }.to raise_error do |error|
expect(error).to be_a(Gitlab::Git::PreReceiveError)
expect(error.message).to eq("You are not allowed to push code to this project.")
end
end
end
context 'with an exception without the detailed error' do
let(:permission_error) do
GRPC::PermissionDenied.new
end
it 'raises PermissionDenied' do
expect_any_instance_of(Gitaly::OperationService::Stub)
.to receive(:user_merge_branch).with(kind_of(Enumerator), kind_of(Hash))
.and_raise(permission_error)
expect { subject }.to raise_error(GRPC::PermissionDenied)
end
end
end
describe '#user_ff_branch' do describe '#user_ff_branch' do
let(:target_branch) { 'my-branch' } let(:target_branch) { 'my-branch' }
let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment