Merge branch 'fj-avoid-repository-size-checks-if-migration-bot' into 'master'

Avoid repository size checkings in snippet migrations for migration bot

See merge request gitlab-org/gitlab!31473

changelogs/unreleased/fj-avoid-repository-size-checks-if-migration-bot.yml

+---
+title: Avoid repository size checkings in snippet migrations for migration bot
+merge_request: 31473
+author:
+type: fixed

lib/gitlab/background_migration/backfill_snippet_repositories.rb

@@ -100,6 +100,8 @@ module Gitlab
       # migrate their snippets as well.
       # In this scenario the migration bot user will be the one that will commit the files.
       def commit_author(snippet)
+        return migration_bot_user if snippet_content_size_over_limit?(snippet)
+
         if Gitlab::UserAccessSnippet.new(snippet.author, snippet: snippet).can_do_action?(:update_snippet)
           snippet.author
         else
@@ -119,6 +121,10 @@ module Gitlab
       def set_file_path_error(error)
         @invalid_path_error = error.is_a?(SnippetRepository::InvalidPathError)
       end
+
+      def snippet_content_size_over_limit?(snippet)
+        snippet.content.size > Gitlab::CurrentSettings.snippet_size_limit
+      end
     end
   end
 end

lib/gitlab/git_access_snippet.rb

@@ -128,5 +128,12 @@ module Gitlab
     def check_custom_action(cmd)
       nil
     end
+
+    override :check_size_limit?
+    def check_size_limit?
+      return false if user&.migration_bot?
+
+      super
+    end
   end
 end

spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb

@@ -70,6 +70,17 @@ describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, s
       end
     end
 
+    shared_examples 'migration_bot user commits files' do
+      it do
+        subject
+
+        last_commit = raw_repository(snippet).commit
+
+        expect(last_commit.author_name).to eq migration_bot.name
+        expect(last_commit.author_email).to eq migration_bot.email
+      end
+    end
+
     shared_examples 'commits the file to the repository' do
       context 'when author can update snippet and use git' do
         it 'creates the repository and commit the file' do
@@ -88,17 +99,6 @@ describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, s
       end
 
       context 'when author cannot update snippet or use git' do
-        shared_examples 'migration_bot user commits files' do
-          it do
-            subject
-
-            last_commit = raw_repository(snippet).commit
-
-            expect(last_commit.author_name).to eq migration_bot.name
-            expect(last_commit.author_email).to eq migration_bot.email
-          end
-        end
-
         context 'when user is blocked' do
           let(:user_state) { 'blocked' }
 
@@ -219,6 +219,19 @@ describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, s
         end
       end
     end
+
+    context 'when snippet content size is higher than the existing limit' do
+      let(:limit) { 15 }
+      let(:content) { 'a' * (limit + 1) }
+      let(:snippet) { snippet_without_repo }
+      let(:ids) { [snippet.id, snippet.id] }
+
+      before do
+        allow(Gitlab::CurrentSettings).to receive(:snippet_size_limit).and_return(limit)
+      end
+
+      it_behaves_like 'migration_bot user commits files'
+    end
   end
 
   def blob_at(snippet, path)

spec/lib/gitlab/git_access_snippet_spec.rb

@@ -298,6 +298,16 @@ describe Gitlab::GitAccessSnippet do
     let(:ref) { "refs/heads/snippet/edit-file" }
     let(:changes) { "#{oldrev} #{newrev} #{ref}" }
 
+    shared_examples 'migration bot does not err' do
+      let(:actor) { migration_bot }
+
+      it 'does not err' do
+        expect(snippet.repository_size_checker).not_to receive(:above_size_limit?)
+
+        expect { push_access_check }.not_to raise_error
+      end
+    end
+
     shared_examples_for 'a push to repository already over the limit' do
       it 'errs' do
         expect(snippet.repository_size_checker).to receive(:above_size_limit?).and_return(true)
@@ -306,6 +316,8 @@ describe Gitlab::GitAccessSnippet do
           push_access_check
         end.to raise_error(described_class::ForbiddenError, /Your push has been rejected/)
       end
+
+      it_behaves_like 'migration bot does not err'
     end
 
     shared_examples_for 'a push to repository below the limit' do
@@ -318,6 +330,8 @@ describe Gitlab::GitAccessSnippet do
 
         expect { push_access_check }.not_to raise_error
       end
+
+      it_behaves_like 'migration bot does not err'
     end
 
     shared_examples_for 'a push to repository to make it over the limit' do
@@ -332,6 +346,8 @@ describe Gitlab::GitAccessSnippet do
           push_access_check
         end.to raise_error(described_class::ForbiddenError, /Your push to this repository would cause it to exceed the size limit/)
       end
+
+      it_behaves_like 'migration bot does not err'
     end
 
     context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is set' do
@@ -350,14 +366,6 @@ describe Gitlab::GitAccessSnippet do
       it_behaves_like 'a push to repository already over the limit'
       it_behaves_like 'a push to repository below the limit'
       it_behaves_like 'a push to repository to make it over the limit'
-
-      context 'when user is migration bot' do
-        let(:actor) { migration_bot }
-
-        it_behaves_like 'a push to repository already over the limit'
-        it_behaves_like 'a push to repository below the limit'
-        it_behaves_like 'a push to repository to make it over the limit'
-      end
     end
 
     context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is not set' do
@@ -372,14 +380,6 @@ describe Gitlab::GitAccessSnippet do
       it_behaves_like 'a push to repository already over the limit'
       it_behaves_like 'a push to repository below the limit'
       it_behaves_like 'a push to repository to make it over the limit'
-
-      context 'when user is migration bot' do
-        let(:actor) { migration_bot }
-
-        it_behaves_like 'a push to repository already over the limit'
-        it_behaves_like 'a push to repository below the limit'
-        it_behaves_like 'a push to repository to make it over the limit'
-      end
     end
   end