From 9ac98c6d0f3f9ff32f497d1cee85853ad1b756cd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt <psteinhardt@gitlab.com> Date: Tue, 2 Aug 2022 02:12:13 +0000 Subject: [PATCH] Gemfile: Update version of the Gitaly Gem to v15.3.0-rc3 Update the version of the Gitaly Gem to v15.3.0-rc3 in order to obtain the definitions for the newly introduced UserCreateTagError message. --- Gemfile | 2 +- Gemfile.lock | 4 +- lib/gitlab/gitaly_client/operation_service.rb | 22 +- .../gitaly_client/operation_service_spec.rb | 204 +++++++++++++++--- 4 files changed, 196 insertions(+), 36 deletions(-) diff --git a/Gemfile b/Gemfile index 13e84392ec9e1..9cc49dbeebbb8 100644 --- a/Gemfile +++ b/Gemfile @@ -486,7 +486,7 @@ gem 'ssh_data', '~> 1.3' gem 'spamcheck', '~> 0.1.0' # Gitaly GRPC protocol definitions -gem 'gitaly', '~> 15.2.0-rc1' +gem 'gitaly', '~> 15.3.0-rc3' # KAS GRPC protocol definitions gem 'kas-grpc', '~> 0.0.2' diff --git a/Gemfile.lock b/Gemfile.lock index 02ca1516f83fa..ca5636d12d686 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -501,7 +501,7 @@ GEM rails (>= 3.2.0) git (1.11.0) rchardet (~> 1.8) - gitaly (15.2.0.pre.rc1) + gitaly (15.3.0.pre.rc3) grpc (~> 1.0) github-markup (1.7.0) gitlab (4.16.1) @@ -1561,7 +1561,7 @@ DEPENDENCIES gettext (~> 3.3) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly (~> 15.2.0.pre.rc1) + gitaly (~> 15.3.0.pre.rc3) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) gitlab-dangerfiles (~> 3.5.0) diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index 35d3ddf5d7f62..8f5d0645a5d61 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -44,8 +44,26 @@ def add_tag(tag_name, user, target, message) end Gitlab::Git::Tag.new(@repository, response.tag) - rescue GRPC::FailedPrecondition => e - raise Gitlab::Git::Repository::InvalidRef, e + rescue GRPC::BadStatus => e + detailed_error = GitalyClient.decode_detailed_error(e) + + case detailed_error&.error + when :access_check + access_check_error = detailed_error.access_check + # These messages were returned from internal/allowed API calls + raise Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message) + when :custom_hook + raise Gitlab::Git::PreReceiveError.new(custom_hook_error_message(detailed_error.custom_hook), + fallback_message: e.details) + when :reference_exists + raise Gitlab::Git::Repository::TagExistsError + else + if e.code == GRPC::Core::StatusCodes::FAILED_PRECONDITION + raise Gitlab::Git::Repository::InvalidRef, e + end + + raise + end end def user_create_branch(branch_name, user, start_point) diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index e04895d975f0d..64a6c5d2b15ed 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -84,37 +84,6 @@ subject end - describe '#user_merge_to_ref' do - let(:first_parent_ref) { 'refs/heads/my-branch' } - let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } - let(:ref) { 'refs/merge-requests/x/merge' } - let(:message) { 'validación' } - let(:response) { Gitaly::UserMergeToRefResponse.new(commit_id: 'new-commit-id') } - - let(:payload) do - { source_sha: source_sha, branch: 'branch', target_ref: ref, - message: message, first_parent_ref: first_parent_ref, allow_conflicts: true } - end - - it 'sends a user_merge_to_ref message' do - freeze_time do - expect_any_instance_of(Gitaly::OperationService::Stub).to receive(:user_merge_to_ref) do |_, request, options| - expect(options).to be_kind_of(Hash) - expect(request.to_h).to eq( - payload.merge({ - repository: repository.gitaly_repository.to_h, - message: message.dup.force_encoding(Encoding::ASCII_8BIT), - user: Gitlab::Git::User.from_gitlab(user).to_gitaly.to_h, - timestamp: { nanos: 0, seconds: Time.current.to_i } - }) - ) - end.and_return(response) - - client.user_merge_to_ref(user, **payload) - end - end - end - context "when pre_receive_error is present" do let(:response) do Gitaly::UserUpdateBranchResponse.new(pre_receive_error: "GitLab: something failed") @@ -131,6 +100,37 @@ end end + describe '#user_merge_to_ref' do + let(:first_parent_ref) { 'refs/heads/my-branch' } + let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } + let(:ref) { 'refs/merge-requests/x/merge' } + let(:message) { 'validación' } + let(:response) { Gitaly::UserMergeToRefResponse.new(commit_id: 'new-commit-id') } + + let(:payload) do + { source_sha: source_sha, branch: 'branch', target_ref: ref, + message: message, first_parent_ref: first_parent_ref, allow_conflicts: true } + end + + it 'sends a user_merge_to_ref message' do + freeze_time do + expect_any_instance_of(Gitaly::OperationService::Stub).to receive(:user_merge_to_ref) do |_, request, options| + expect(options).to be_kind_of(Hash) + expect(request.to_h).to eq( + payload.merge({ + repository: repository.gitaly_repository.to_h, + message: message.dup.force_encoding(Encoding::ASCII_8BIT), + user: Gitlab::Git::User.from_gitlab(user).to_gitaly.to_h, + timestamp: { nanos: 0, seconds: Time.current.to_i } + }) + ) + end.and_return(response) + + client.user_merge_to_ref(user, **payload) + end + end + end + describe '#user_delete_branch' do let(:branch_name) { 'my-branch' } let(:request) do @@ -813,4 +813,146 @@ end end end + + describe '#add_tag' do + let(:tag_name) { 'some-tag' } + let(:tag_message) { nil } + let(:target) { 'master' } + + subject(:add_tag) do + client.add_tag(tag_name, user, target, tag_message) + end + + context 'without tag message' do + let(:tag_name) { 'lightweight-tag' } + + it 'creates a lightweight tag' do + tag = add_tag + expect(tag.name).to eq(tag_name) + expect(tag.message).to eq('') + end + end + + context 'with tag message' do + let(:tag_name) { 'annotated-tag' } + let(:tag_message) { "tag message" } + + it 'creates an annotated tag' do + tag = add_tag + expect(tag.name).to eq(tag_name) + expect(tag.message).to eq(tag_message) + end + end + + context 'with preexisting tag' do + let(:tag_name) { 'v1.0.0' } + + it 'raises a TagExistsError' do + expect { add_tag }.to raise_error(Gitlab::Git::Repository::TagExistsError) + end + end + + context 'with invalid target' do + let(:target) { 'refs/heads/does-not-exist' } + + it 'raises an InvalidRef error' do + expect { add_tag }.to raise_error(Gitlab::Git::Repository::InvalidRef) + end + end + + context 'with pre-receive error' do + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_create_tag) + .and_return(Gitaly::UserCreateTagResponse.new(pre_receive_error: "GitLab: something failed")) + end + + it 'raises a PreReceiveError' do + expect { add_tag }.to raise_error(Gitlab::Git::PreReceiveError, "something failed") + end + end + + context 'with internal error' do + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_create_tag) + .and_raise(GRPC::Internal.new('undetailed internal error')) + end + + it 'raises an Internal error' do + expect { add_tag }.to raise_error do |error| + expect(error).to be_a(GRPC::Internal) + expect(error.details).to eq('undetailed internal error') + end + end + end + + context 'with structured errors' do + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_create_tag) + .and_raise(structured_error) + end + + context 'with ReferenceExistsError' do + let(:structured_error) do + new_detailed_error( + GRPC::Core::StatusCodes::ALREADY_EXISTS, + 'tag exists already', + Gitaly::UserCreateTagError.new( + reference_exists: Gitaly::ReferenceExistsError.new( + reference_name: tag_name, + oid: 'something' + ))) + end + + it 'raises a TagExistsError' do + expect { add_tag }.to raise_error(Gitlab::Git::Repository::TagExistsError) + end + end + + context 'with AccessCheckError' do + let(:structured_error) do + new_detailed_error( + GRPC::Core::StatusCodes::PERMISSION_DENIED, + "error creating tag", + Gitaly::UserCreateTagError.new( + access_check: Gitaly::AccessCheckError.new( + error_message: "You are not allowed to create this tag.", + protocol: "web", + user_id: "user-15", + changes: "df15b32277d2c55c6c595845a87109b09c913c556 5d6e0f935ad9240655f64e883cd98fad6f9a17ee refs/tags/v1.0.0\n" + ))) + end + + it 'raises a PreReceiveError' do + expect { add_tag }.to raise_error do |error| + expect(error).to be_a(Gitlab::Git::PreReceiveError) + expect(error.message).to eq("You are not allowed to create this tag.") + end + end + end + + context 'with CustomHookError' do + let(:structured_error) do + new_detailed_error( + GRPC::Core::StatusCodes::PERMISSION_DENIED, + "custom hook error", + Gitaly::UserCreateTagError.new( + custom_hook: Gitaly::CustomHookError.new( + stdout: "some stdout", + stderr: "GitLab: some custom hook error message", + hook_type: Gitaly::CustomHookError::HookType::HOOK_TYPE_PRERECEIVE + ))) + end + + it 'raises a PreReceiveError' do + expect { add_tag }.to raise_error do |error| + expect(error).to be_a(Gitlab::Git::PreReceiveError) + expect(error.message).to eq("some custom hook error message") + end + end + end + end + end end -- GitLab