Merge branch 'jc-prevent-storage-update-deletion' into 'master'

Prevent ProjectUpdateRepositoryStorageWorker from deleting repo

Closes gitaly#1318

See merge request gitlab-org/gitlab!28469

app/workers/project_update_repository_storage_worker.rb

@@ -3,11 +3,21 @@
 class ProjectUpdateRepositoryStorageWorker # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
 
+  SameFilesystemError = Class.new(StandardError)
+
   feature_category :gitaly
 
   def perform(project_id, new_repository_storage_key)
     project = Project.find(project_id)
 
+    raise SameFilesystemError if same_filesystem?(project.repository.storage, new_repository_storage_key)
+
     ::Projects::UpdateRepositoryStorageService.new(project).execute(new_repository_storage_key)
   end
+
+  private
+
+  def same_filesystem?(old_storage, new_storage)
+    Gitlab::GitalyClient.filesystem_id(old_storage) == Gitlab::GitalyClient.filesystem_id(new_storage)
+  end
 end

changelogs/unreleased/jc-prevent-storage-update-deletion.yml

+---
+title: Prevent ProjectUpdateRepositoryStorageWorker from moving to same filesystem
+merge_request: 28469
+author:
+type: fixed

spec/workers/project_update_repository_storage_worker_spec.rb

 # frozen_string_literal: true
 
 require 'spec_helper'
+require 'securerandom'
 
 describe ProjectUpdateRepositoryStorageWorker do
   let(:project) { create(:project, :repository) }
@@ -8,12 +9,33 @@ describe ProjectUpdateRepositoryStorageWorker do
   subject { described_class.new }
 
   describe "#perform" do
-    it "calls the update repository storage service" do
-      expect_next_instance_of(Projects::UpdateRepositoryStorageService) do |instance|
-        expect(instance).to receive(:execute).with('new_storage')
+    context 'when source and target repositories are on different filesystems' do
+      before do
+        allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+        allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('new_storage').and_return(SecureRandom.uuid)
       end
 
-      subject.perform(project.id, 'new_storage')
+      it "calls the update repository storage service" do
+        expect_next_instance_of(Projects::UpdateRepositoryStorageService) do |instance|
+          expect(instance).to receive(:execute).with('new_storage')
+        end
+
+        subject.perform(project.id, 'new_storage')
+      end
+    end
+
+    context 'when source and target repositories are on the same filesystems' do
+      let(:filesystem_id) { SecureRandom.uuid }
+
+      before do
+        allow(Gitlab::GitalyClient).to receive(:filesystem_id).and_return(filesystem_id)
+      end
+
+      it 'raises an error' do
+        expect_any_instance_of(::Projects::UpdateRepositoryStorageService).not_to receive(:new)
+
+        expect { subject.perform(project.id, 'new_storage') }.to raise_error(ProjectUpdateRepositoryStorageWorker::SameFilesystemError)
+      end
     end
   end
 end