Ensure checksums match when updating repository storage

Specs required a major reshuffle in order to stub repository checksums.

app/services/projects/update_repository_storage_service.rb

@@ -48,6 +48,7 @@ module Projects
       repository = type.repository_for(project)
       full_path = repository.full_path
       raw_repository = repository.raw
+      checksum = repository.checksum
 
       # Initialize a git repository on the target path
       gitlab_shell.create_repository(new_storage_key, raw_repository.relative_path, full_path)
@@ -56,7 +57,9 @@ module Projects
                                                    raw_repository.gl_repository,
                                                    full_path)
 
-      new_repository.fetch_repository_as_mirror(raw_repository)
+      result = new_repository.fetch_repository_as_mirror(raw_repository)
+
+      result && checksum == new_repository.checksum
     end
 
     def mark_old_paths_for_archive

changelogs/unreleased/update_repo_storage_checksum.yml

+---
+title: Ensure checksums match when updating repository storage
+merge_request: 26334
+author:
+type: changed

spec/services/projects/fork_service_spec.rb

@@ -315,6 +315,7 @@ describe Projects::ForkService do
       # Stub everything required to move a project to a Gitaly shard that does not exist
       stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/second_storage' })
       allow_any_instance_of(Gitlab::Git::Repository).to receive(:fetch_repository_as_mirror).and_return(true)
+      allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum).and_return(::Gitlab::Git::BLANK_SHA)
 
       Projects::UpdateRepositoryStorageService.new(project).execute('test_second_storage')
       fork_after_move = fork_project(project)

spec/services/projects/update_repository_storage_service_spec.rb

@@ -16,6 +16,15 @@ describe Projects::UpdateRepositoryStorageService do
 
     context 'without wiki and design repository' do
       let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: false) }
+      let!(:checksum) { project.repository.checksum }
+      let(:project_repository_double) { double(:repository) }
+
+      before do
+        allow(Gitlab::Git::Repository).to receive(:new).and_call_original
+        allow(Gitlab::Git::Repository).to receive(:new)
+          .with('test_second_storage', project.repository.raw.relative_path, project.repository.gl_repository, project.repository.full_path)
+          .and_return(project_repository_double)
+      end
 
       context 'when the move succeeds' do
         it 'moves the repository to the new storage and unmarks the repository as read only' do
@@ -23,8 +32,10 @@ describe Projects::UpdateRepositoryStorageService do
             project.repository.path_to_repo
           end
 
-          expect_any_instance_of(Gitlab::Git::Repository).to receive(:fetch_repository_as_mirror)
+          expect(project_repository_double).to receive(:fetch_repository_as_mirror)
             .with(project.repository.raw).and_return(true)
+          expect(project_repository_double).to receive(:checksum)
+            .and_return(checksum)
 
           subject.execute('test_second_storage')
           expect(project).not_to be_repository_read_only
@@ -44,7 +55,7 @@ describe Projects::UpdateRepositoryStorageService do
 
       context 'when the move fails' do
         it 'unmarks the repository as read-only without updating the repository storage' do
-          expect_any_instance_of(Gitlab::Git::Repository).to receive(:fetch_repository_as_mirror)
+          expect(project_repository_double).to receive(:fetch_repository_as_mirror)
             .with(project.repository.raw).and_return(false)
           expect(GitlabShellWorker).not_to receive(:perform_async)
 
@@ -54,6 +65,37 @@ describe Projects::UpdateRepositoryStorageService do
           expect(project.repository_storage).to eq('default')
         end
       end
+
+      context 'when the checksum does not match' do
+        it 'unmarks the repository as read-only without updating the repository storage' do
+          expect(project_repository_double).to receive(:fetch_repository_as_mirror)
+            .with(project.repository.raw).and_return(true)
+          expect(project_repository_double).to receive(:checksum)
+            .and_return('not matching checksum')
+          expect(GitlabShellWorker).not_to receive(:perform_async)
+
+          subject.execute('test_second_storage')
+
+          expect(project).not_to be_repository_read_only
+          expect(project.repository_storage).to eq('default')
+        end
+      end
+
+      context 'when a object pool was joined' do
+        let!(:pool) { create(:pool_repository, :ready, source_project: project) }
+
+        it 'leaves the pool' do
+          expect(project_repository_double).to receive(:fetch_repository_as_mirror)
+            .with(project.repository.raw).and_return(true)
+          expect(project_repository_double).to receive(:checksum)
+            .and_return(checksum)
+
+          subject.execute('test_second_storage')
+
+          expect(project.repository_storage).to eq('test_second_storage')
+          expect(project.reload_pool_repository).to be_nil
+        end
+      end
     end
 
     context 'with wiki repository' do
@@ -66,18 +108,5 @@ describe Projects::UpdateRepositoryStorageService do
         end
       end
     end
-
-    context 'when a object pool was joined' do
-      let(:project) { create(:project, :repository, wiki_enabled: false, repository_read_only: true) }
-      let(:pool) { create(:pool_repository, :ready, source_project: project) }
-
-      it 'leaves the pool' do
-        allow_any_instance_of(Gitlab::Git::Repository).to receive(:fetch_repository_as_mirror).and_return(true)
-
-        subject.execute('test_second_storage')
-
-        expect(project.reload_pool_repository).to be_nil
-      end
-    end
   end
 end

spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb

@@ -2,7 +2,10 @@
 
 RSpec.shared_examples 'moves repository to another storage' do |repository_type|
   let(:project_repository_double) { double(:repository) }
+  let!(:project_repository_checksum) { project.repository.checksum }
+
   let(:repository_double) { double(:repository) }
+  let(:repository_checksum) { repository.checksum }
 
   before do
     # Default stub for non-specified params
@@ -19,15 +22,16 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
 
   context 'when the move succeeds', :clean_gitlab_redis_shared_state do
     before do
-      allow(project_repository_double)
-        .to receive(:fetch_repository_as_mirror)
+      allow(project_repository_double).to receive(:fetch_repository_as_mirror)
         .with(project.repository.raw)
         .and_return(true)
+      allow(project_repository_double).to receive(:checksum)
+        .and_return(project_repository_checksum)
 
-      allow(repository_double)
-        .to receive(:fetch_repository_as_mirror)
-        .with(repository.raw)
-        .and_return(true)
+      allow(repository_double).to receive(:fetch_repository_as_mirror)
+        .with(repository.raw).and_return(true)
+      allow(repository_double).to receive(:checksum)
+        .and_return(repository_checksum)
     end
 
     it "moves the project and its #{repository_type} repository to the new storage and unmarks the repository as read only" do
@@ -87,6 +91,8 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
     it 'unmarks the repository as read-only without updating the repository storage' do
       allow(project_repository_double).to receive(:fetch_repository_as_mirror)
         .with(project.repository.raw).and_return(true)
+      allow(project_repository_double).to receive(:checksum)
+        .and_return(project_repository_checksum)
       allow(repository_double).to receive(:fetch_repository_as_mirror)
         .with(repository.raw).and_return(false)
 
@@ -98,4 +104,25 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
       expect(project.repository_storage).to eq('default')
     end
   end
+
+  context "when the checksum of the #{repository_type} repository does not match" do
+    it 'unmarks the repository as read-only without updating the repository storage' do
+      allow(project_repository_double).to receive(:fetch_repository_as_mirror)
+        .with(project.repository.raw).and_return(true)
+      allow(project_repository_double).to receive(:checksum)
+        .and_return(project_repository_checksum)
+
+      allow(repository_double).to receive(:fetch_repository_as_mirror)
+        .with(repository.raw).and_return(true)
+      allow(repository_double).to receive(:checksum)
+        .and_return('not matching checksum')
+
+      expect(GitlabShellWorker).not_to receive(:perform_async)
+
+      subject.execute('test_second_storage')
+
+      expect(project).not_to be_repository_read_only
+      expect(project.repository_storage).to eq('default')
+    end
+  end
 end