From d4ad92cd06fd0367f93bcb908cf50261d013fe43 Mon Sep 17 00:00:00 2001
From: Michael Kozono <mkozono@gmail.com>
Date: Wed, 6 Jun 2018 12:51:03 -0700
Subject: Support verifying remote store uploads

---
 lib/gitlab/verify/batch_verifier.rb    | 24 +++++++++++++++++++++---
 lib/gitlab/verify/job_artifacts.rb     |  4 ++++
 lib/gitlab/verify/lfs_objects.rb       |  4 ++++
 lib/gitlab/verify/uploads.rb           |  6 +++++-
 spec/lib/gitlab/verify/uploads_spec.rb | 16 ++++++++++------
 5 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/lib/gitlab/verify/batch_verifier.rb b/lib/gitlab/verify/batch_verifier.rb
index 1ef369a4b67..fd01602c256 100644
--- a/lib/gitlab/verify/batch_verifier.rb
+++ b/lib/gitlab/verify/batch_verifier.rb
@@ -34,15 +34,28 @@ module Gitlab
       end
 
       def verify(object)
+        if local?(object)
+          verify_local(object)
+        else
+          verify_remote(object)
+        end
+
+        nil
+      rescue => err
+        [object, err]
+      end
+
+      def verify_local(object)
         expected = expected_checksum(object)
         actual = actual_checksum(object)
 
         raise 'Checksum missing' unless expected.present?
         raise 'Checksum mismatch' unless expected == actual
+      end
 
-        nil
-      rescue => err
-        [object, err]
+      # We don't calculate checksum for remote objects, so just check existence
+      def verify_remote(object)
+        raise 'Remote object does not exist' unless object.build_uploader.exists?
       end
 
       # This should return an ActiveRecord::Relation suitable for calling #in_batches on
@@ -50,6 +63,11 @@ module Gitlab
         raise NotImplementedError.new
       end
 
+      # Should return true if the object is stored locally
+      def local?(_object)
+        raise NotImplementedError.new
+      end
+
       # The checksum we expect the object to have
       def expected_checksum(_object)
         raise NotImplementedError.new
diff --git a/lib/gitlab/verify/job_artifacts.rb b/lib/gitlab/verify/job_artifacts.rb
index 03500a61074..afee3b84268 100644
--- a/lib/gitlab/verify/job_artifacts.rb
+++ b/lib/gitlab/verify/job_artifacts.rb
@@ -15,6 +15,10 @@ module Gitlab
         ::Ci::JobArtifact.all
       end
 
+      def local?(artifact)
+        artifact.local_store?
+      end
+
       def expected_checksum(artifact)
         artifact.file_sha256
       end
diff --git a/lib/gitlab/verify/lfs_objects.rb b/lib/gitlab/verify/lfs_objects.rb
index 970e2a7b718..bfda339f129 100644
--- a/lib/gitlab/verify/lfs_objects.rb
+++ b/lib/gitlab/verify/lfs_objects.rb
@@ -15,6 +15,10 @@ module Gitlab
         LfsObject.with_files_stored_locally
       end
 
+      def local?(lfs_object)
+        lfs_object.local_store?
+      end
+
       def expected_checksum(lfs_object)
         lfs_object.oid
       end
diff --git a/lib/gitlab/verify/uploads.rb b/lib/gitlab/verify/uploads.rb
index 0ffa71a6d72..70916aeed4f 100644
--- a/lib/gitlab/verify/uploads.rb
+++ b/lib/gitlab/verify/uploads.rb
@@ -12,7 +12,11 @@ module Gitlab
       private
 
       def relation
-        Upload.with_files_stored_locally
+        Upload.all
+      end
+
+      def local?(upload)
+        upload.local?
       end
 
       def expected_checksum(upload)
diff --git a/spec/lib/gitlab/verify/uploads_spec.rb b/spec/lib/gitlab/verify/uploads_spec.rb
index 85768308edc..1a4e5c626a4 100644
--- a/spec/lib/gitlab/verify/uploads_spec.rb
+++ b/spec/lib/gitlab/verify/uploads_spec.rb
@@ -44,16 +44,20 @@ describe Gitlab::Verify::Uploads do
     context 'with remote files' do
       before do
         stub_uploads_object_storage(AvatarUploader)
+        upload.update!(store: ObjectStorage::Store::REMOTE)
       end
 
-      it 'skips uploads in object storage' do
-        local_failure = create(:upload)
-        create(:upload, :object_storage)
+      it 'passes uploads in object storage that exist' do
+        expect_any_instance_of(AvatarUploader).to receive(:exists?).and_return(true)
 
-        failures = {}
-        described_class.new(batch_size: 10).run_batches { |_, failed| failures.merge!(failed) }
+        expect(failures).to eq({})
+      end
+
+      it 'fails uploads in object storage that do not exist' do
+        expect_any_instance_of(AvatarUploader).to receive(:exists?).and_return(false)
 
-        expect(failures.keys).to contain_exactly(local_failure)
+        expect(failures.keys).to contain_exactly(upload)
+        expect(failure.to_s).to include('Remote object does not exist')
       end
     end
   end
-- 
cgit v1.2.1