Merge branch 'feature/migrate-repository-rm-tag-to-gitaly' into 'master'

Migrate Git::Repository#rm_tag to Gitaly

Closes gitaly#562

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

GITALY_SERVER_VERSION

-0.40.0
+0.42.0

Gemfile

@@ -398,7 +398,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.33.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.37.0', require: 'gitaly'
 
 gem 'toml-rb', '~> 0.3.15', require: false
 

Gemfile.lock

@@ -275,7 +275,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly-proto (0.33.0)
+    gitaly-proto (0.37.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -1025,7 +1025,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.2.0)
-  gitaly-proto (~> 0.33.0)
+  gitaly-proto (~> 0.37.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-markup (~> 1.6.2)

lib/gitlab/git/repository.rb

@@ -671,7 +671,13 @@ module Gitlab
       end
 
       def rm_tag(tag_name, user:)
-        OperationService.new(user, self).rm_tag(find_tag(tag_name))
+        gitaly_migrate(:operation_user_delete_tag) do |is_enabled|
+          if is_enabled
+            gitaly_operations_client.rm_tag(tag_name, user)
+          else
+            Gitlab::Git::OperationService.new(user, self).rm_tag(find_tag(tag_name))
+          end
+        end
       end
 
       def find_tag(name)
@@ -1048,6 +1054,10 @@ module Gitlab
         Gitlab::GitalyClient::Util.repository(@storage, @relative_path)
       end
 
+      def gitaly_operations_client
+        @gitaly_operations_client ||= Gitlab::GitalyClient::OperationService.new(self)
+      end
+
       def gitaly_ref_client
         @gitaly_ref_client ||= Gitlab::GitalyClient::RefService.new(self)
       end

lib/gitlab/gitaly_client/operation_service.rb

+module Gitlab
+  module GitalyClient
+    class OperationService
+      def initialize(repository)
+        @gitaly_repo = repository.gitaly_repository
+        @repository = repository
+      end
+
+      def rm_tag(tag_name, user)
+        request = Gitaly::UserDeleteTagRequest.new(
+          repository: @gitaly_repo,
+          tag_name: GitalyClient.encode(tag_name),
+          user: Util.gitaly_user(user)
+        )
+
+        response = GitalyClient.call(@repository.storage, :operation_service, :user_delete_tag, request)
+
+        if pre_receive_error = response.pre_receive_error.presence
+          raise Gitlab::Git::HooksService::PreReceiveError, pre_receive_error
+        end
+      end
+    end
+  end
+end

lib/gitlab/gitaly_client/util.rb

@@ -10,6 +10,16 @@ module Gitlab
             git_alternate_object_directories: Array.wrap(Gitlab::Git::Env['GIT_ALTERNATE_OBJECT_DIRECTORIES'])
           )
         end
+
+        def gitaly_user(gitlab_user)
+          return unless gitlab_user
+
+          Gitaly::User.new(
+            gl_id: Gitlab::GlId.gl_id(gitlab_user),
+            name: GitalyClient.encode(gitlab_user.name),
+            email: GitalyClient.encode(gitlab_user.email)
+          )
+        end
       end
     end
   end

spec/features/tags/master_deletes_tag_spec.rb

@@ -35,6 +35,20 @@ feature 'Master deletes tag' do
   end
 
   context 'when pre-receive hook fails', js: true do
+    context 'when Gitaly operation_user_delete_tag feature is enabled' do
+      before do
+        allow_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:rm_tag)
+          .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'Do not delete tags')
+      end
+
+      scenario 'shows the error message' do
+        delete_first_tag
+
+        expect(page).to have_content('Do not delete tags')
+      end
+    end
+
+    context 'when Gitaly operation_user_delete_tag feature is disabled', skip_gitaly_mock: true do
       before do
         allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute)
           .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'Do not delete tags')
@@ -46,6 +60,7 @@ feature 'Master deletes tag' do
         expect(page).to have_content('Do not delete tags')
       end
     end
+  end
 
   def delete_first_tag
     page.within('.content') do

spec/lib/gitlab/workhorse_spec.rb

@@ -218,7 +218,8 @@ describe Gitlab::Workhorse do
           storage_name: 'default',
           relative_path: project.full_path + '.git',
           git_object_directory: '',
-          git_alternate_object_directories: []
+          git_alternate_object_directories: [],
+          gl_repository: ''
         } }
 
         expect(subject).to include(repo_param)

spec/models/repository_spec.rb

@@ -1682,15 +1682,25 @@ describe Repository do
   end
 
   describe '#rm_tag' do
+    shared_examples 'removing tag' do
       it 'removes a tag' do
         expect(repository).to receive(:before_remove_tag)
 
-      repository.rm_tag(create(:user), 'v1.1.0')
+        repository.rm_tag(build_stubbed(:user), 'v1.1.0')
 
         expect(repository.find_tag('v1.1.0')).to be_nil
       end
     end
 
+    context 'when Gitaly operation_user_delete_tag feature is enabled' do
+      it_behaves_like 'removing tag'
+    end
+
+    context 'when Gitaly operation_user_delete_tag feature is disabled', skip_gitaly_mock: true do
+      it_behaves_like 'removing tag'
+    end
+  end
+
   describe '#avatar' do
     it 'returns nil if repo does not exist' do
       expect(repository).to receive(:file_on_head)