Merge branch '267495-add-sha256-of-pages-contents-to-pages_deployments' into 'master'

Add sha256 and file count to pages_deployments

See merge request gitlab-org/gitlab!45522

app/models/pages_deployment.rb

@@ -10,6 +10,8 @@ class PagesDeployment < ApplicationRecord
   validates :file, presence: true
   validates :file_store, presence: true, inclusion: { in: ObjectStorage::SUPPORTED_STORES }
   validates :size, presence: true, numericality: { greater_than: 0, only_integer: true }
+  validates :file_count, presence: true, numericality: { greater_than_or_equal_to: 0, only_integer: true }
+  validates :file_sha256, presence: true
 
   before_validation :set_size, if: :file_changed?
 

app/services/projects/update_pages_service.rb

@@ -97,7 +97,7 @@ module Projects
       build.artifacts_file.use_file do |artifacts_path|
         SafeZip::Extract.new(artifacts_path)
           .extract(directories: [PUBLIC_DIR], to: temp_path)
-        create_pages_deployment(artifacts_path)
+        create_pages_deployment(artifacts_path, build)
       end
     rescue SafeZip::Extract::Error => e
       raise FailedToExtractError, e.message
@@ -119,11 +119,19 @@ module Projects
       FileUtils.rm_r(previous_public_path, force: true)
     end
 
-    def create_pages_deployment(artifacts_path)
+    def create_pages_deployment(artifacts_path, build)
       return unless Feature.enabled?(:zip_pages_deployments, project)
 
+      # we're using the full archive and pages daemon needs to read it
+      # so we want the total count from entries, not only "public/" directory
+      # because it better approximates work we need to do before we can serve the site
+      entries_count = build.artifacts_metadata_entry("", recursive: true).entries.count
+      sha256 = build.job_artifacts_archive.file_sha256
+
       File.open(artifacts_path) do |file|
-        deployment = project.pages_deployments.create!(file: file)
+        deployment = project.pages_deployments.create!(file: file,
+                                                       file_count: entries_count,
+                                                       file_sha256: sha256)
         project.pages_metadatum.update!(pages_deployment: deployment)
       end
 

changelogs/unreleased/267495-add-sha256-of-pages-contents-to-pages_deployments.yml

+---
+title: Add sha256 and file count to pages_deployments
+merge_request: 45522
+author:
+type: added

db/migrate/20201019111952_add_sha256_and_filecount_to_pages_deployments.rb

+# frozen_string_literal: true
+
+class AddSha256AndFilecountToPagesDeployments < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    # pages_deployments were never enabled on any production/staging
+    # environments, so we safely delete them for people who enabled
+    # them locally
+    execute "DELETE FROM pages_deployments"
+
+    # rubocop:disable Rails/NotNullColumn
+    add_column :pages_deployments, :file_count, :integer, null: false
+    add_column :pages_deployments, :file_sha256, :binary, null: false
+    # rubocop:enable Rails/NotNullColumn
+  end
+
+  def down
+    remove_column :pages_deployments, :file_sha256
+    remove_column :pages_deployments, :file_count
+  end
+end

db/schema_migrations/20201019111952

+82d6ed3e066c15352abdb58d58d195f48aa7b1d17bbb28f58f42c19ae67fab54
\ No newline at end of file

db/structure.sql

@@ -14400,6 +14400,8 @@ CREATE TABLE pages_deployments (
     file_store smallint NOT NULL,
     size integer NOT NULL,
     file text NOT NULL,
+    file_count integer NOT NULL,
+    file_sha256 bytea NOT NULL,
     CONSTRAINT check_f0fe8032dd CHECK ((char_length(file) <= 255))
 );
 

ee/spec/services/projects/update_pages_service_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe Projects::UpdatePagesService do
       file = fixture_file_upload('spec/fixtures/pages.zip')
       metafile = fixture_file_upload('spec/fixtures/pages.zip.meta')
 
-      create(:ci_job_artifact, :archive, file: file, job: build)
+      create(:ci_job_artifact, :archive, :correct_checksum, file: file, job: build)
       create(:ci_job_artifact, :metadata, file: metafile, job: build)
 
       allow(build).to receive(:artifacts_metadata_entry)
@@ -31,6 +31,7 @@ RSpec.describe Projects::UpdatePagesService do
 
     it 'uses closest setting for max_pages_size' do
       allow(metadata).to receive(:total_size).and_return(1.megabyte)
+      allow(metadata).to receive(:entries).and_return([])
 
       expect(project).to receive(:closest_setting).with(:max_pages_size).and_call_original
 

spec/factories/pages_deployments.rb

@@ -5,9 +5,13 @@ FactoryBot.define do
     project
 
     after(:build) do |deployment, _evaluator|
-      deployment.file = fixture_file_upload(
-        Rails.root.join("spec/fixtures/pages.zip")
-      )
+      filepath =  Rails.root.join("spec/fixtures/pages.zip")
+
+      deployment.file = fixture_file_upload(filepath)
+      deployment.file_sha256 = Digest::SHA256.file(filepath).hexdigest
+      ::Zip::File.open(filepath) do |zip_archive|
+        deployment.file_count = zip_archive.count
+      end
     end
   end
 end

spec/features/projects/pages_spec.rb

@@ -365,7 +365,7 @@ RSpec.shared_examples 'pages settings editing' do
       end
 
       let!(:artifact) do
-        create(:ci_job_artifact, :archive,
+        create(:ci_job_artifact, :archive, :correct_checksum,
                file: fixture_file_upload(File.join('spec/fixtures/pages.zip')), job: ci_build)
       end
 

spec/models/pages_deployment_spec.rb

@@ -10,8 +10,15 @@ RSpec.describe PagesDeployment do
 
   describe 'validations' do
     it { is_expected.to validate_presence_of(:file) }
+
     it { is_expected.to validate_presence_of(:size) }
     it { is_expected.to validate_numericality_of(:size).only_integer.is_greater_than(0) }
+
+    it { is_expected.to validate_presence_of(:file_count) }
+    it { is_expected.to validate_numericality_of(:file_count).only_integer.is_greater_than_or_equal_to(0) }
+
+    it { is_expected.to validate_presence_of(:file_sha256) }
+
     it { is_expected.to validate_inclusion_of(:file_store).in_array(ObjectStorage::SUPPORTED_STORES) }
 
     it 'is valid when created from the factory' do

spec/services/projects/update_pages_service_spec.rb

@@ -27,7 +27,7 @@ RSpec.describe Projects::UpdatePagesService do
 
   context 'for new artifacts' do
     context "for a valid job" do
-      let!(:artifacts_archive) { create(:ci_job_artifact, file: file, job: build) }
+      let!(:artifacts_archive) { create(:ci_job_artifact, :correct_checksum, file: file, job: build) }
 
       before do
         create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build)
@@ -66,6 +66,8 @@ RSpec.describe Projects::UpdatePagesService do
 
         expect(deployment.size).to eq(file.size)
         expect(deployment.file).to be
+        expect(deployment.file_count).to eq(3)
+        expect(deployment.file_sha256).to eq(artifacts_archive.file_sha256)
         expect(project.pages_metadatum.reload.pages_deployment_id).to eq(deployment.id)
       end
 
@@ -188,6 +190,25 @@ RSpec.describe Projects::UpdatePagesService do
     end
   end
 
+  # this situation should never happen in real life because all new archives have sha256
+  # and we only use new archives
+  # this test is here just to clarify that this behavior is intentional
+  context 'when artifacts archive does not have sha256' do
+    let!(:artifacts_archive) { create(:ci_job_artifact, file: file, job: build) }
+
+    before do
+      create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build)
+
+      build.reload
+    end
+
+    it 'fails with exception raised' do
+      expect do
+        execute
+      end.to raise_error("Validation failed: File sha256 can't be blank")
+    end
+  end
+
   it 'fails to remove project pages when no pages is deployed' do
     expect(PagesWorker).not_to receive(:perform_in)
     expect(project.pages_deployed?).to be_falsey
@@ -210,7 +231,7 @@ RSpec.describe Projects::UpdatePagesService do
       file = fixture_file_upload('spec/fixtures/pages.zip')
       metafile = fixture_file_upload('spec/fixtures/pages.zip.meta')
 
-      create(:ci_job_artifact, :archive, file: file, job: build)
+      create(:ci_job_artifact, :archive, :correct_checksum, file: file, job: build)
       create(:ci_job_artifact, :metadata, file: metafile, job: build)
 
       allow(build).to receive(:artifacts_metadata_entry)

spec/support/shared_examples/services/pages_size_limit_shared_examples.rb

@@ -4,6 +4,7 @@ RSpec.shared_examples 'pages size limit is' do |size_limit|
   context "when size is below the limit" do
     before do
       allow(metadata).to receive(:total_size).and_return(size_limit - 1.megabyte)
+      allow(metadata).to receive(:entries).and_return([])
     end
 
     it 'updates pages correctly' do
@@ -17,6 +18,7 @@ RSpec.shared_examples 'pages size limit is' do |size_limit|
   context "when size is above the limit" do
     before do
       allow(metadata).to receive(:total_size).and_return(size_limit + 1.megabyte)
+      allow(metadata).to receive(:entries).and_return([])
     end
 
     it 'limits the maximum size of gitlab pages' do