Check push size based on quarantine size

Fallback to using the old approach when env var is
not present.

ee/changelogs/unreleased/11126-fix-repository-size-check.yml

+---
+title: Use quarantine size to check push size against repository size limit
+merge_request: 13460
+author:
+type: fixed

ee/lib/ee/gitlab/git_access.rb

@@ -84,20 +84,40 @@ module EE
       def check_push_size!
         return unless check_size_limit?
 
+        git_env = ::Gitlab::Git::HookEnv.all(repository.gl_repository)
+
+        # Use #quarantine_size to get correct push size whenever a lof of changes
+        # gets pushed at the same time containing the same blobs. This is only
+        # doable if GIT_OBJECT_DIRECTORY_RELATIVE env var is set and happens
+        # when git push comes from CLI (not via UI and API).
+        #
+        # Fallback to determining push size using the changes_list so we can still
+        # determine the push size if env var isn't set (e.g. changes are made
+        # via UI and API).
+        push_size = git_env['GIT_OBJECT_DIRECTORY_RELATIVE'].present? ? quarantine_size : changes_size
+
+        if project.changes_will_exceed_size_limit?(push_size)
+          raise ::Gitlab::GitAccess::UnauthorizedError, ::Gitlab::RepositorySizeError.new(project).new_changes_error
+        end
+      end
+
+      def quarantine_size
+        project.repository.object_directory_size
+      end
+
+      def changes_size
         # If there are worktrees with a HEAD pointing to a non-existent object,
         # calls to `git rev-list --all` will fail in git 2.15+. This should also
         # clear stale lock files.
         project.repository.clean_stale_repository_files
 
-        push_size_in_bytes = 0
+        size_in_bytes = 0
 
         changes_list.each do |change|
-          push_size_in_bytes += repository.new_blobs(change[:newrev]).sum(&:size) # rubocop: disable CodeReuse/ActiveRecord
-
-          if project.changes_will_exceed_size_limit?(push_size_in_bytes)
-            raise ::Gitlab::GitAccess::UnauthorizedError, ::Gitlab::RepositorySizeError.new(project).new_changes_error
-          end
+          size_in_bytes += repository.new_blobs(change[:newrev]).sum(&:size) # rubocop: disable CodeReuse/ActiveRecord
         end
+
+        size_in_bytes
       end
 
       def check_size_limit?

ee/spec/lib/gitlab/git_access_spec.rb

@@ -181,18 +181,13 @@ describe Gitlab::GitAccess do
       # Delete branch so Repository#new_blobs can return results
       repository.delete_branch('2-mb-file')
       repository.delete_branch('wip')
-    end
-
-    context 'when repository size is over limit' do
-      before do
-        allow(project).to receive(:repository_and_lfs_size).and_return(2.megabytes)
 
-        project.update_attribute(:repository_size_limit, 1.megabytes)
-      end
+      allow(project).to receive(:repository_and_lfs_size).and_return(repository_size)
+      project.update_attribute(:repository_size_limit, repository_size_limit)
+    end
 
+    shared_examples_for 'a push to repository over the limit' do
       it 'rejects the push' do
-        expect(repository.new_blobs(sha_with_smallest_changes)).to be_present
-
         expect do
           push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_smallest_changes} refs/heads/master")
         end.to raise_error(described_class::UnauthorizedError, /Your push has been rejected/)
@@ -207,13 +202,7 @@ describe Gitlab::GitAccess do
       end
     end
 
-    context 'when repository size is below the limit' do
-      before do
-        allow(project).to receive(:repository_and_lfs_size).and_return(1.megabyte)
-
-        project.update_attribute(:repository_size_limit, 2.megabytes)
-      end
-
+    shared_examples_for 'a push to repository below the limit' do
       context 'when trying to authenticate the user' do
         it 'does not raise an error' do
           expect { push_changes }.not_to raise_error
@@ -229,24 +218,88 @@ describe Gitlab::GitAccess do
           end.not_to raise_error
         end
       end
+    end
 
-      context 'when new change exceeds the limit' do
-        it 'rejects the push' do
-          expect(repository.new_blobs(sha_with_2_mb_file)).to be_present
+    context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is set' do
+      let(:object_directory_size) { 1.megabyte }
 
-          expect do
-            push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_2_mb_file} refs/heads/my_branch_2")
-          end.to raise_error(described_class::UnauthorizedError, /Your push to this repository would cause it to exceed the size limit/)
+      before do
+        allow(Gitlab::Git::HookEnv)
+          .to receive(:all)
+          .with(repository.gl_repository)
+          .and_return({ 'GIT_OBJECT_DIRECTORY_RELATIVE' => 'objects' })
+
+        # Stub the object directory size to "simulate" quarantine size
+        allow(repository)
+          .to receive(:object_directory_size)
+          .and_return(object_directory_size)
+      end
+
+      context 'when repository size is over limit' do
+        let(:repository_size) { 2.megabytes }
+        let(:repository_size_limit) { 1.megabyte }
+
+        it_behaves_like 'a push to repository over the limit'
+      end
+
+      context 'when repository size is below the limit' do
+        let(:repository_size) { 1.megabyte }
+        let(:repository_size_limit) { 2.megabytes }
+
+        it_behaves_like 'a push to repository below the limit'
+
+        context 'when object directory (quarantine) size exceeds the limit' do
+          let(:object_directory_size) { 2.megabytes }
+
+          it 'rejects the push' do
+            expect do
+              push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_2_mb_file} refs/heads/my_branch_2")
+            end.to raise_error(described_class::UnauthorizedError, /Your push to this repository would cause it to exceed the size limit/)
+          end
+        end
+
+        context 'when object directory (quarantine) size does not exceed the limit' do
+          it 'accepts the push' do
+            expect do
+              push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_smallest_changes} refs/heads/my_branch_3")
+            end.not_to raise_error
+          end
         end
       end
+    end
 
-      context 'when new change does not exceeds the limit' do
-        it 'accepts the push' do
-          expect(repository.new_blobs(sha_with_smallest_changes)).to be_present
+    context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is not set' do
+      context 'when repository size is over limit' do
+        let(:repository_size) { 2.megabytes }
+        let(:repository_size_limit) { 1.megabyte }
 
-          expect do
-            push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_smallest_changes} refs/heads/my_branch_3")
-          end.not_to raise_error
+        it_behaves_like 'a push to repository over the limit'
+      end
+
+      context 'when repository size is below the limit' do
+        let(:repository_size) { 1.megabyte }
+        let(:repository_size_limit) { 2.megabytes }
+
+        it_behaves_like 'a push to repository below the limit'
+
+        context 'when new change exceeds the limit' do
+          it 'rejects the push' do
+            expect(repository.new_blobs(sha_with_2_mb_file)).to be_present
+
+            expect do
+              push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_2_mb_file} refs/heads/my_branch_2")
+            end.to raise_error(described_class::UnauthorizedError, /Your push to this repository would cause it to exceed the size limit/)
+          end
+        end
+
+        context 'when new change does not exceed the limit' do
+          it 'accepts the push' do
+            expect(repository.new_blobs(sha_with_smallest_changes)).to be_present
+
+            expect do
+              push_changes("#{Gitlab::Git::BLANK_SHA} #{sha_with_smallest_changes} refs/heads/my_branch_3")
+            end.not_to raise_error
+          end
         end
       end
     end