Fix repository cleanup with object storage on

When the BFG object map file is in object storage (i.e., uploads in
general are placed into object storage), we get an instance of the
Gitlab::HttpIO class. This doesn't behave as expected when you try to
read past EOF, so we need to explicitly check for this condition to
avoid ending up in a tight loop around io.read

lib/gitlab/gitaly_client/cleanup_service.rb

@@ -20,6 +20,7 @@ module Gitlab
 
           while data = io.read(RepositoryService::MAX_MSG_SIZE)
             y.yield Gitaly::ApplyBfgObjectMapRequest.new(object_map: data)
+            break if io&.eof?
           end
         end
 

spec/lib/gitlab/git/repository_cleaner_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Git::RepositoryCleaner do
+  include HttpIOHelpers
+
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
   let(:head_sha) { repository.head_commit.id }
-
-  let(:object_map) { StringIO.new("#{head_sha} #{'0' * 40}") }
+  let(:object_map_data) { "#{head_sha} #{'0' * 40}" }
 
   subject(:cleaner) { described_class.new(repository.raw) }
 
   describe '#apply_bfg_object_map' do
-    it 'removes internal references pointing at SHAs in the object map' do
-      # Create some refs we expect to be removed
-      repository.keep_around(head_sha)
-      repository.create_ref(head_sha, 'refs/environments/1')
-      repository.create_ref(head_sha, 'refs/merge-requests/1')
-      repository.create_ref(head_sha, 'refs/heads/_keep')
-      repository.create_ref(head_sha, 'refs/tags/_keep')
+    let(:clean_refs) { %W[refs/environments/1 refs/merge-requests/1 refs/keep-around/#{head_sha}] }
+    let(:keep_refs) { %w[refs/heads/_keep refs/tags/_keep] }
+
+    before do
+      (clean_refs + keep_refs).each { |ref| repository.create_ref(head_sha, ref) }
+    end
+
+    context 'from StringIO' do
+      let(:object_map) { StringIO.new(object_map_data) }
 
+      it 'removes internal references' do
         cleaner.apply_bfg_object_map(object_map)
 
         aggregate_failures do
-        expect(repository.kept_around?(head_sha)).to be_falsy
-        expect(repository.ref_exists?('refs/environments/1')).to be_falsy
-        expect(repository.ref_exists?('refs/merge-requests/1')).to be_falsy
-        expect(repository.ref_exists?('refs/heads/_keep')).to be_truthy
-        expect(repository.ref_exists?('refs/tags/_keep')).to be_truthy
+          clean_refs.each { |ref| expect(repository.ref_exists?(ref)).to be_falsy }
+          keep_refs.each { |ref| expect(repository.ref_exists?(ref)).to be_truthy }
+        end
+      end
+    end
+
+    context 'from Gitlab::HttpIO' do
+      let(:url) { 'http://example.com/bfg_object_map.txt' }
+      let(:tempfile) { Tempfile.new }
+      let(:object_map) { Gitlab::HttpIO.new(url, object_map_data.size) }
+
+      around do |example|
+        begin
+          tempfile.write(object_map_data)
+          tempfile.close
+
+          example.run
+        ensure
+          tempfile.unlink
+        end
+      end
+
+      it 'removes internal references' do
+        stub_remote_url_200(url, tempfile.path)
+
+        cleaner.apply_bfg_object_map(object_map)
+
+        aggregate_failures do
+          clean_refs.each { |ref| expect(repository.ref_exists?(ref)).to be_falsy }
+          keep_refs.each { |ref| expect(repository.ref_exists?(ref)).to be_truthy }
+        end
       end
     end
   end