Merge branch 'gitaly-mandatory-20180627-jv-2' into 'master'

Make Gitaly signature RPC's mandatory

Closes gitaly#745, gitaly#1044, and gitaly#876

See merge request gitlab-org/gitlab-ce!20212

3 files changed, 10 insertions, 73 deletions

diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb
index a6c3f446656..01274505557 100644
--- a/lib/gitlab/git/commit.rb
+++ b/lib/gitlab/git/commit.rb
@@ -220,19 +220,7 @@ module Gitlab
         end
 
         def shas_with_signatures(repository, shas)
-          GitalyClient.migrate(:filter_shas_with_signatures) do |is_enabled|
-            if is_enabled
-              Gitlab::GitalyClient::CommitService.new(repository).filter_shas_with_signatures(shas)
-            else
-              shas.select do |sha|
-                begin
-                  Rugged::Commit.extract_signature(repository.rugged, sha)
-                rescue Rugged::OdbError
-                  false
-                end
-              end
-            end
-          end
+          Gitlab::GitalyClient::CommitService.new(repository).filter_shas_with_signatures(shas)
         end
 
         # Only to be used when the object ids will not necessarily have a
@@ -250,13 +238,7 @@ module Gitlab
         end
 
         def extract_signature(repository, commit_id)
-          repository.gitaly_migrate(:extract_commit_signature) do |is_enabled|
-            if is_enabled
-              repository.gitaly_commit_client.extract_signature(commit_id)
-            else
-              rugged_extract_signature(repository, commit_id)
-            end
-          end
+          repository.gitaly_commit_client.extract_signature(commit_id)
         end
 
         def extract_signature_lazily(repository, commit_id)
@@ -276,36 +258,9 @@ module Gitlab
         end
 
         def batch_signature_extraction(repository, commit_ids)
-          repository.gitaly_migrate(:extract_commit_signature_in_batch) do |is_enabled|
-            if is_enabled
-              gitaly_batch_signature_extraction(repository, commit_ids)
-            else
-              rugged_batch_signature_extraction(repository, commit_ids)
-            end
-          end
-        end
-
-        def gitaly_batch_signature_extraction(repository, commit_ids)
           repository.gitaly_commit_client.get_commit_signatures(commit_ids)
         end
 
-        def rugged_batch_signature_extraction(repository, commit_ids)
-          commit_ids.each_with_object({}) do |commit_id, signatures|
-            signature_data = rugged_extract_signature(repository, commit_id)
-            next unless signature_data
-
-            signatures[commit_id] = signature_data
-          end
-        end
-
-        def rugged_extract_signature(repository, commit_id)
-          begin
-            Rugged::Commit.extract_signature(repository.rugged, commit_id)
-          rescue Rugged::OdbError
-            nil
-          end
-        end
-
         def get_message(repository, commit_id)
           BatchLoader.for({ repository: repository, commit_id: commit_id }).batch do |items, loader|
             items_by_repo = items.group_by { |i| i[:repository] }
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index 077297b9d6d..c9c414e5d33 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -324,6 +324,8 @@ module Gitlab
         return if signature.blank? && signed_text.blank?
 
         [signature, signed_text]
+      rescue GRPC::InvalidArgument => ex
+        raise ArgumentError, ex
       end
 
       def get_commit_signatures(commit_ids)
@@ -341,6 +343,8 @@ module Gitlab
         end
 
         signatures
+      rescue GRPC::InvalidArgument => ex
+        raise ArgumentError, ex
       end
 
       def get_commit_messages(commit_ids)
diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb
index ae69a362dda..0d7c930127c 100644
--- a/spec/lib/gitlab/git/commit_spec.rb
+++ b/spec/lib/gitlab/git/commit_spec.rb
@@ -309,7 +309,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
       it { is_expected.not_to include(SeedRepo::FirstCommit::ID) }
     end
 
-    shared_examples '.shas_with_signatures' do
+    describe '.shas_with_signatures' do
       let(:signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e 570e7b2abdd848b95f2f578043fc23bd6f6fd24d] }
       let(:unsigned_shas) { %w[19e2e9b4ef76b422ce1154af39a91323ccc57434 c642fe9b8b9f28f9225d7ea953fe14e74748d53b] }
       let(:first_signed_shas) { %w[5937ac0a7beb003549fc5fd26fc247adbce4a52e c642fe9b8b9f28f9225d7ea953fe14e74748d53b] }
@@ -330,14 +330,6 @@ describe Gitlab::Git::Commit, seed_helper: true do
       end
     end
 
-    describe '.shas_with_signatures with gitaly on' do
-      it_should_behave_like '.shas_with_signatures'
-    end
-
-    describe '.shas_with_signatures with gitaly disabled', :disable_gitaly do
-      it_should_behave_like '.shas_with_signatures'
-    end
-
     describe '.find_all' do
       shared_examples 'finding all commits' do
         it 'should return a return a collection of commits' do
@@ -498,7 +490,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
     end
 
     describe '.extract_signature_lazily' do
-      shared_examples 'loading signatures in batch once' do
+      describe 'loading signatures in batch once' do
         it 'fetches signatures in batch once' do
           commit_ids = %w[0b4bc9a49b562e85de7cc9e834518ea6828729b9 4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6]
           signatures = commit_ids.map do |commit_id|
@@ -516,27 +508,13 @@ describe Gitlab::Git::Commit, seed_helper: true do
 
       subject { described_class.extract_signature_lazily(repository, commit_id).itself }
 
-      context 'with Gitaly extract_commit_signature_in_batch feature enabled' do
-        it_behaves_like 'extracting commit signature'
-        it_behaves_like 'loading signatures in batch once'
-      end
-
-      context 'with Gitaly extract_commit_signature_in_batch feature disabled', :disable_gitaly do
-        it_behaves_like 'extracting commit signature'
-        it_behaves_like 'loading signatures in batch once'
-      end
+      it_behaves_like 'extracting commit signature'
     end
 
     describe '.extract_signature' do
       subject { described_class.extract_signature(repository, commit_id) }
 
-      context 'with gitaly' do
-        it_behaves_like 'extracting commit signature'
-      end
-
-      context 'without gitaly', :disable_gitaly do
-        it_behaves_like 'extracting commit signature'
-      end
+      it_behaves_like 'extracting commit signature'
     end
   end