Merge branch 'pks-lfs-quarantine' into 'master'

Use object quarantine directory to enumerate new LFS pointers [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!58634

Gemfile

@@ -475,7 +475,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 13.9.0.pre.rc1'
+gem 'gitaly', '~> 13.11.0.pre.rc1'
 
 gem 'grpc', '~> 1.30.2'
 

Gemfile.lock

@@ -440,7 +440,7 @@ GEM
       rails (>= 3.2.0)
     git (1.7.0)
       rchardet (~> 1.8)
-    gitaly (13.9.0.pre.rc1)
+    gitaly (13.11.0.pre.rc1)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab (4.16.1)
@@ -1427,7 +1427,7 @@ DEPENDENCIES
   gettext (~> 3.3)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 13.9.0.pre.rc1)
+  gitaly (~> 13.11.0.pre.rc1)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
   gitlab-dangerfiles (~> 1.1.1)

changelogs/unreleased/pks-lfs-quarantine.yml

+---
+title: Use object quarantine directory to enumerate new LFS pointers
+merge_request: 58634
+author:
+type: performance

config/feature_flags/development/lfs_integrity_inspect_quarantined_objects.yml

+---
+name: lfs_integrity_inspect_quarantined_objects
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58634
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/327440
+milestone: '13.11'
+type: development
+group: group::gitaly
+default_enabled: false

lib/gitlab/gitaly_client/blob_service.rb

@@ -78,17 +78,7 @@ module Gitlab
       end
 
       def get_new_lfs_pointers(revision, limit, not_in, dynamic_timeout = nil)
-        request = Gitaly::GetNewLFSPointersRequest.new(
-          repository: @gitaly_repo,
-          revision: encode_binary(revision),
-          limit: limit || 0
-        )
-
-        if not_in.nil? || not_in == :all
-          request.not_in_all = true
-        else
-          request.not_in_refs += not_in
-        end
+        request, rpc = create_new_lfs_pointers_request(revision, limit, not_in)
 
         timeout =
           if dynamic_timeout
@@ -100,7 +90,7 @@ module Gitlab
         response = GitalyClient.call(
           @gitaly_repo.storage_name,
           :blob_service,
-          :get_new_lfs_pointers,
+          rpc,
           request,
           timeout: timeout
         )
@@ -118,6 +108,39 @@ module Gitlab
 
       private
 
+      def create_new_lfs_pointers_request(revision, limit, not_in)
+        # If the check happens for a change which is using a quarantine
+        # environment for incoming objects, then we can avoid doing the
+        # necessary graph walk to detect only new LFS pointers and instead scan
+        # through all quarantined objects.
+        git_env = ::Gitlab::Git::HookEnv.all(@gitaly_repo.gl_repository)
+        if Feature.enabled?(:lfs_integrity_inspect_quarantined_objects, @project, default_enabled: :yaml) && git_env['GIT_OBJECT_DIRECTORY_RELATIVE'].present?
+          repository = @gitaly_repo.dup
+          repository.git_alternate_object_directories = Google::Protobuf::RepeatedField.new(:string)
+
+          request = Gitaly::ListAllLFSPointersRequest.new(
+            repository: repository,
+            limit: limit || 0
+          )
+
+          [request, :list_all_lfs_pointers]
+        else
+          request = Gitaly::GetNewLFSPointersRequest.new(
+            repository: @gitaly_repo,
+            revision: encode_binary(revision),
+            limit: limit || 0
+          )
+
+          if not_in.nil? || not_in == :all
+            request.not_in_all = true
+          else
+            request.not_in_refs += not_in
+          end
+
+          [request, :get_new_lfs_pointers]
+        end
+      end
+
       def consume_blob_response(response)
         data = []
         blob = nil

spec/lib/gitlab/gitaly_client/blob_service_spec.rb

@@ -43,6 +43,33 @@ RSpec.describe Gitlab::GitalyClient::BlobService do
         subject
       end
     end
+
+    context 'with hook environment' do
+      let(:git_env) do
+        {
+          'GIT_OBJECT_DIRECTORY_RELATIVE' => '.git/objects',
+          'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['/dir/one', '/dir/two']
+        }
+      end
+
+      let(:expected_params) do
+        expected_repository = repository.gitaly_repository
+        expected_repository.git_alternate_object_directories = Google::Protobuf::RepeatedField.new(:string)
+
+        { limit: limit, repository: expected_repository }
+      end
+
+      it 'sends a list_all_lfs_pointers message' do
+        allow(Gitlab::Git::HookEnv).to receive(:all).with(repository.gl_repository).and_return(git_env)
+
+        expect_any_instance_of(Gitaly::BlobService::Stub)
+          .to receive(:list_all_lfs_pointers)
+          .with(gitaly_request_with_params(expected_params), kind_of(Hash))
+          .and_return([])
+
+        subject
+      end
+    end
   end
 
   describe '#get_all_lfs_pointers' do