Merge branch 'pages-migration-service' into 'master'

Add Pages migration service

See merge request gitlab-org/gitlab!49473

app/models/project.rb

@@ -1829,6 +1829,15 @@ class Project < ApplicationRecord
     ensure_pages_metadatum.update!(pages_deployment: deployment)
   end
 
+  def set_first_pages_deployment!(deployment)
+    ensure_pages_metadatum
+
+    # where().update_all to perform update in the single transaction with check for null
+    ProjectPagesMetadatum
+      .where(project_id: id, pages_deployment_id: nil)
+      .update_all(pages_deployment_id: deployment.id)
+  end
+
   def write_repository_config(gl_full_path: full_path)
     # We'd need to keep track of project full path otherwise directory tree
     # created with hashed storage enabled cannot be usefully imported using

app/services/pages/migrate_legacy_storage_to_deployment_service.rb

+# frozen_string_literal: true
+
+module Pages
+  class MigrateLegacyStorageToDeploymentService
+    ExclusiveLeaseTaken = Class.new(StandardError)
+
+    include ::Pages::LegacyStorageLease
+
+    attr_reader :project
+
+    def initialize(project)
+      @project = project
+    end
+
+    def execute
+      migrated = try_obtain_lease do
+        execute_unsafe
+
+        true
+      end
+
+      raise ExclusiveLeaseTaken, "Can't migrate pages for project #{project.id}: exclusive lease taken" unless migrated
+    end
+
+    private
+
+    def execute_unsafe
+      archive_path, entries_count = ::Pages::ZipDirectoryService.new(project.pages_path).execute
+
+      deployment = nil
+      File.open(archive_path) do |file|
+        deployment = project.pages_deployments.create!(
+          file: file,
+          file_count: entries_count,
+          file_sha256: Digest::SHA256.file(archive_path).hexdigest
+        )
+      end
+
+      project.set_first_pages_deployment!(deployment)
+    rescue ::Pages::ZipDirectoryService::InvalidArchiveError => e
+      Gitlab::ErrorTracking.track_exception(e, project_id: project.id)
+
+      if !project.pages_metadatum&.reload&.pages_deployment &&
+         Feature.enabled?(:pages_migration_mark_as_not_deployed, project)
+        project.mark_pages_as_not_deployed
+      end
+
+    ensure
+      FileUtils.rm_f(archive_path) if archive_path
+    end
+  end
+end

app/services/pages/zip_directory_service.rb

@@ -2,32 +2,40 @@
 
 module Pages
   class ZipDirectoryService
+    include Gitlab::Utils::StrongMemoize
+
     InvalidArchiveError = Class.new(RuntimeError)
     InvalidEntryError = Class.new(RuntimeError)
 
     PUBLIC_DIR = 'public'
 
     def initialize(input_dir)
-      @input_dir = File.realpath(input_dir)
-      @output_file = File.join(@input_dir, "@migrated.zip") # '@' to avoid any name collision with groups or projects
+      @input_dir = input_dir
     end
 
     def execute
-      FileUtils.rm_f(@output_file)
+      raise InvalidArchiveError unless valid_work_directory?
+
+      output_file = File.join(real_dir, "@migrated.zip") # '@' to avoid any name collision with groups or projects
+
+      FileUtils.rm_f(output_file)
 
       count = 0
-      ::Zip::File.open(@output_file, ::Zip::File::CREATE) do |zipfile|
+      ::Zip::File.open(output_file, ::Zip::File::CREATE) do |zipfile|
         write_entry(zipfile, PUBLIC_DIR)
         count = zipfile.entries.count
       end
 
-      [@output_file, count]
+      [output_file, count]
+    rescue => e
+      FileUtils.rm_f(output_file) if output_file
+      raise e
     end
 
     private
 
     def write_entry(zipfile, zipfile_path)
-      disk_file_path = File.join(@input_dir, zipfile_path)
+      disk_file_path = File.join(real_dir, zipfile_path)
 
       unless valid_path?(disk_file_path)
         # archive without public directory is completelly unusable
@@ -71,13 +79,27 @@ module Pages
     def valid_path?(disk_file_path)
       realpath = File.realpath(disk_file_path)
 
-      realpath == File.join(@input_dir, PUBLIC_DIR) ||
-        realpath.start_with?(File.join(@input_dir, PUBLIC_DIR + "/"))
+      realpath == File.join(real_dir, PUBLIC_DIR) ||
+        realpath.start_with?(File.join(real_dir, PUBLIC_DIR + "/"))
     # happens if target of symlink isn't there
     rescue => e
-      Gitlab::ErrorTracking.track_exception(e, input_dir: @input_dir, disk_file_path: disk_file_path)
+      Gitlab::ErrorTracking.track_exception(e, input_dir: real_dir, disk_file_path: disk_file_path)
 
       false
     end
+
+    def valid_work_directory?
+      Dir.exist?(real_dir)
+    rescue => e
+      Gitlab::ErrorTracking.track_exception(e, input_dir: @input_dir)
+
+      false
+    end
+
+    def real_dir
+      strong_memoize(:real_dir) do
+        File.realpath(@input_dir) rescue nil
+      end
+    end
   end
 end

config/feature_flags/development/pages_migration_mark_as_not_deployed.yml

+---
+name: pages_migration_mark_as_not_deployed
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49473
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/295187
+milestone: '13.8'
+type: development
+group: group::release
+default_enabled: false

spec/models/project_spec.rb

@@ -6002,6 +6002,43 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  describe '#set_first_pages_deployment!' do
+    let(:project) { create(:project) }
+    let(:deployment) { create(:pages_deployment, project: project) }
+
+    it "creates new metadata record if none exists yet and sets deployment" do
+      project.pages_metadatum.destroy!
+      project.reload
+
+      project.set_first_pages_deployment!(deployment)
+
+      expect(project.pages_metadatum.reload.pages_deployment).to eq(deployment)
+    end
+
+    it "updates the existing metadara record with deployment" do
+      expect do
+        project.set_first_pages_deployment!(deployment)
+      end.to change { project.pages_metadatum.reload.pages_deployment }.from(nil).to(deployment)
+    end
+
+    it 'only updates metadata for this project' do
+      other_project = create(:project)
+
+      expect do
+        project.set_first_pages_deployment!(deployment)
+      end.not_to change { other_project.pages_metadatum.reload.pages_deployment }.from(nil)
+    end
+
+    it 'does nothing if metadata already references some deployment' do
+      existing_deployment = create(:pages_deployment, project: project)
+      project.set_first_pages_deployment!(existing_deployment)
+
+      expect do
+        project.set_first_pages_deployment!(deployment)
+      end.not_to change { project.pages_metadatum.reload.pages_deployment }.from(existing_deployment)
+    end
+  end
+
   describe '#has_pool_repsitory?' do
     it 'returns false when it does not have a pool repository' do
       subject = create(:project, :repository)

spec/services/pages/migrate_legacy_storage_to_deployment_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Pages::MigrateLegacyStorageToDeploymentService do
+  let(:project) { create(:project, :repository) }
+  let(:service) { described_class.new(project) }
+
+  it 'marks pages as not deployed if public directory is absent' do
+    project.mark_pages_as_deployed
+
+    expect do
+      service.execute
+    end.to change { project.pages_metadatum.reload.deployed }.from(true).to(false)
+  end
+
+  it 'does not mark pages as not deployed if public directory is absent but pages_deployment exists' do
+    deployment = create(:pages_deployment, project: project)
+    project.update_pages_deployment!(deployment)
+    project.mark_pages_as_deployed
+
+    expect do
+      service.execute
+    end.not_to change { project.pages_metadatum.reload.deployed }.from(true)
+  end
+
+  it 'does not mark pages as not deployed if public directory is absent but feature is disabled' do
+    stub_feature_flags(pages_migration_mark_as_not_deployed: false)
+
+    project.mark_pages_as_deployed
+
+    expect do
+      service.execute
+    end.not_to change { project.pages_metadatum.reload.deployed }.from(true)
+  end
+
+  it 'removes pages archive when can not save deployment' do
+    archive = fixture_file_upload("spec/fixtures/pages.zip")
+    expect_next_instance_of(::Pages::ZipDirectoryService) do |zip_service|
+      expect(zip_service).to receive(:execute).and_return([archive.path, 3])
+    end
+
+    expect_next_instance_of(PagesDeployment) do |deployment|
+      expect(deployment).to receive(:save!).and_raise("Something")
+    end
+
+    expect do
+      service.execute
+    end.to raise_error("Something")
+
+    expect(File.exist?(archive.path)).to eq(false)
+  end
+
+  context 'when pages site is deployed to legacy storage' do
+    before do
+      FileUtils.mkdir_p File.join(project.pages_path, "public")
+      File.open(File.join(project.pages_path, "public/index.html"), "w") do |f|
+        f.write("Hello!")
+      end
+    end
+
+    it 'creates pages deployment' do
+      expect do
+        described_class.new(project).execute
+      end.to change { project.reload.pages_deployments.count }.by(1)
+
+      deployment = project.pages_metadatum.pages_deployment
+
+      Zip::File.open(deployment.file.path) do |zip_file|
+        expect(zip_file.glob("public").first.ftype).to eq(:directory)
+        expect(zip_file.glob("public/index.html").first.get_input_stream.read).to eq("Hello!")
+      end
+
+      expect(deployment.file_count).to eq(2)
+      expect(deployment.file_sha256).to eq(Digest::SHA256.file(deployment.file.path).hexdigest)
+    end
+
+    it 'removes tmp pages archive' do
+      described_class.new(project).execute
+
+      expect(File.exist?(File.join(project.pages_path, '@migrated.zip'))).to eq(false)
+    end
+
+    it 'does not change pages deployment if it is set' do
+      old_deployment = create(:pages_deployment, project: project)
+      project.update_pages_deployment!(old_deployment)
+
+      expect do
+        described_class.new(project).execute
+      end.not_to change { project.pages_metadatum.reload.pages_deployment_id }.from(old_deployment.id)
+    end
+
+    it 'raises exception if exclusive lease is taken' do
+      described_class.new(project).try_obtain_lease do
+        expect do
+          described_class.new(project).execute
+        end.to raise_error(described_class::ExclusiveLeaseTaken)
+      end
+    end
+  end
+end

spec/services/pages/zip_directory_service_spec.rb

@@ -3,207 +3,217 @@
 require 'spec_helper'
 
 RSpec.describe Pages::ZipDirectoryService do
-  around do |example|
-    Dir.mktmpdir do |dir|
-      @work_dir = dir
-      example.run
-    end
-  end
-
-  let(:result) do
-    described_class.new(@work_dir).execute
+  it 'raises error if project pages dir does not exist' do
+    expect do
+      described_class.new("/tmp/not/existing/dir").execute
+    end.to raise_error(described_class::InvalidArchiveError)
   end
 
-  let(:archive) { result.first }
-  let(:entries_count) { result.second }
+  context 'when work dir exists' do
+    around do |example|
+      Dir.mktmpdir do |dir|
+        @work_dir = dir
+        example.run
+      end
+    end
 
-  it 'raises error if there is no public directory' do
-    expect { archive }.to raise_error(described_class::InvalidArchiveError)
-  end
+    let(:result) do
+      described_class.new(@work_dir).execute
+    end
 
-  it 'raises error if public directory is a symlink' do
-    create_dir('target')
-    create_file('./target/index.html', 'hello')
-    create_link("public", "./target")
+    let(:archive) { result.first }
+    let(:entries_count) { result.second }
 
-    expect { archive }.to raise_error(described_class::InvalidArchiveError)
-  end
+    it 'raises error if there is no public directory and does not leave archive' do
+      expect { archive }.to raise_error(described_class::InvalidArchiveError)
 
-  context 'when there is a public directory' do
-    before do
-      create_dir('public')
+      expect(File.exist?(File.join(@work_dir, '@migrated.zip'))).to eq(false)
     end
 
-    it 'creates the file next the public directory' do
-      expect(archive).to eq(File.join(@work_dir, "@migrated.zip"))
+    it 'raises error if public directory is a symlink' do
+      create_dir('target')
+      create_file('./target/index.html', 'hello')
+      create_link("public", "./target")
+
+      expect { archive }.to raise_error(described_class::InvalidArchiveError)
     end
 
-    it 'includes public directory' do
-      with_zip_file do |zip_file|
-        entry = zip_file.get_entry("public/")
-        expect(entry.ftype).to eq(:directory)
+    context 'when there is a public directory' do
+      before do
+        create_dir('public')
       end
-    end
 
-    it 'returns number of entries' do
-      create_file("public/index.html", "hello")
-      create_link("public/link.html", "./index.html")
-      expect(entries_count).to eq(3) # + 'public' directory
-    end
+      it 'creates the file next the public directory' do
+        expect(archive).to eq(File.join(@work_dir, "@migrated.zip"))
+      end
 
-    it 'removes the old file if it exists' do
-      # simulate the old run
-      described_class.new(@work_dir).execute
+      it 'includes public directory' do
+        with_zip_file do |zip_file|
+          entry = zip_file.get_entry("public/")
+          expect(entry.ftype).to eq(:directory)
+        end
+      end
 
-      with_zip_file do |zip_file|
-        expect(zip_file.entries.count).to eq(1)
+      it 'returns number of entries' do
+        create_file("public/index.html", "hello")
+        create_link("public/link.html", "./index.html")
+        expect(entries_count).to eq(3) # + 'public' directory
       end
-    end
 
-    it 'ignores other top level files and directories' do
-      create_file("top_level.html", "hello")
-      create_dir("public2")
+      it 'removes the old file if it exists' do
+        # simulate the old run
+        described_class.new(@work_dir).execute
 
-      with_zip_file do |zip_file|
-        expect { zip_file.get_entry("top_level.html") }.to raise_error(Errno::ENOENT)
-        expect { zip_file.get_entry("public2/") }.to raise_error(Errno::ENOENT)
+        with_zip_file do |zip_file|
+          expect(zip_file.entries.count).to eq(1)
+        end
       end
-    end
 
-    it 'includes index.html file' do
-      create_file("public/index.html", "Hello!")
+      it 'ignores other top level files and directories' do
+        create_file("top_level.html", "hello")
+        create_dir("public2")
 
-      with_zip_file do |zip_file|
-        entry = zip_file.get_entry("public/index.html")
-        expect(zip_file.read(entry)).to eq("Hello!")
+        with_zip_file do |zip_file|
+          expect { zip_file.get_entry("top_level.html") }.to raise_error(Errno::ENOENT)
+          expect { zip_file.get_entry("public2/") }.to raise_error(Errno::ENOENT)
+        end
       end
-    end
 
-    it 'includes hidden file' do
-      create_file("public/.hidden.html", "Hello!")
+      it 'includes index.html file' do
+        create_file("public/index.html", "Hello!")
 
-      with_zip_file do |zip_file|
-        entry = zip_file.get_entry("public/.hidden.html")
-        expect(zip_file.read(entry)).to eq("Hello!")
+        with_zip_file do |zip_file|
+          entry = zip_file.get_entry("public/index.html")
+          expect(zip_file.read(entry)).to eq("Hello!")
+        end
       end
-    end
 
-    it 'includes nested directories and files' do
-      create_dir("public/nested")
-      create_dir("public/nested/nested2")
-      create_file("public/nested/nested2/nested.html", "Hello nested")
+      it 'includes hidden file' do
+        create_file("public/.hidden.html", "Hello!")
 
-      with_zip_file do |zip_file|
-        entry = zip_file.get_entry("public/nested")
-        expect(entry.ftype).to eq(:directory)
+        with_zip_file do |zip_file|
+          entry = zip_file.get_entry("public/.hidden.html")
+          expect(zip_file.read(entry)).to eq("Hello!")
+        end
+      end
 
-        entry = zip_file.get_entry("public/nested/nested2")
-        expect(entry.ftype).to eq(:directory)
+      it 'includes nested directories and files' do
+        create_dir("public/nested")
+        create_dir("public/nested/nested2")
+        create_file("public/nested/nested2/nested.html", "Hello nested")
 
-        entry = zip_file.get_entry("public/nested/nested2/nested.html")
-        expect(zip_file.read(entry)).to eq("Hello nested")
+        with_zip_file do |zip_file|
+          entry = zip_file.get_entry("public/nested")
+          expect(entry.ftype).to eq(:directory)
+
+          entry = zip_file.get_entry("public/nested/nested2")
+          expect(entry.ftype).to eq(:directory)
+
+          entry = zip_file.get_entry("public/nested/nested2/nested.html")
+          expect(zip_file.read(entry)).to eq("Hello nested")
+        end
       end
-    end
 
-    it 'adds a valid symlink' do
-      create_file("public/target.html", "hello")
-      create_link("public/link.html", "./target.html")
+      it 'adds a valid symlink' do
+        create_file("public/target.html", "hello")
+        create_link("public/link.html", "./target.html")
 
-      with_zip_file do |zip_file|
-        entry = zip_file.get_entry("public/link.html")
-        expect(entry.ftype).to eq(:symlink)
-        expect(zip_file.read(entry)).to eq("./target.html")
+        with_zip_file do |zip_file|
+          entry = zip_file.get_entry("public/link.html")
+          expect(entry.ftype).to eq(:symlink)
+          expect(zip_file.read(entry)).to eq("./target.html")
+        end
       end
-    end
 
-    it 'ignores the symlink pointing outside of public directory' do
-      create_file("target.html", "hello")
-      create_link("public/link.html", "../target.html")
+      it 'ignores the symlink pointing outside of public directory' do
+        create_file("target.html", "hello")
+        create_link("public/link.html", "../target.html")
 
-      with_zip_file do |zip_file|
-        expect { zip_file.get_entry("public/link.html") }.to raise_error(Errno::ENOENT)
+        with_zip_file do |zip_file|
+          expect { zip_file.get_entry("public/link.html") }.to raise_error(Errno::ENOENT)
+        end
       end
-    end
 
-    it 'ignores the symlink if target is absent' do
-      create_link("public/link.html", "./target.html")
+      it 'ignores the symlink if target is absent' do
+        create_link("public/link.html", "./target.html")
 
-      with_zip_file do |zip_file|
-        expect { zip_file.get_entry("public/link.html") }.to raise_error(Errno::ENOENT)
+        with_zip_file do |zip_file|
+          expect { zip_file.get_entry("public/link.html") }.to raise_error(Errno::ENOENT)
+        end
       end
-    end
 
-    it 'ignores symlink if is absolute and points to outside of directory' do
-      target = File.join(@work_dir, "target")
-      FileUtils.touch(target)
+      it 'ignores symlink if is absolute and points to outside of directory' do
+        target = File.join(@work_dir, "target")
+        FileUtils.touch(target)
 
-      create_link("public/link.html", target)
+        create_link("public/link.html", target)
 
-      with_zip_file do |zip_file|
-        expect { zip_file.get_entry("public/link.html") }.to raise_error(Errno::ENOENT)
+        with_zip_file do |zip_file|
+          expect { zip_file.get_entry("public/link.html") }.to raise_error(Errno::ENOENT)
+        end
       end
-    end
 
-    it "includes raw symlink if it's target is a valid directory" do
-      create_dir("public/target")
-      create_file("public/target/index.html", "hello")
-      create_link("public/link", "./target")
+      it "includes raw symlink if it's target is a valid directory" do
+        create_dir("public/target")
+        create_file("public/target/index.html", "hello")
+        create_link("public/link", "./target")
 
-      with_zip_file do |zip_file|
-        expect(zip_file.entries.count).to eq(4) # /public and 3 created above
+        with_zip_file do |zip_file|
+          expect(zip_file.entries.count).to eq(4) # /public and 3 created above
 
-        entry = zip_file.get_entry("public/link")
-        expect(entry.ftype).to eq(:symlink)
-        expect(zip_file.read(entry)).to eq("./target")
+          entry = zip_file.get_entry("public/link")
+          expect(entry.ftype).to eq(:symlink)
+          expect(zip_file.read(entry)).to eq("./target")
+        end
       end
     end
-  end
 
-  context "validating fixtures pages archives" do
-    using RSpec::Parameterized::TableSyntax
+    context "validating fixtures pages archives" do
+      using RSpec::Parameterized::TableSyntax
 
-    where(:fixture_path) do
-      ["spec/fixtures/pages.zip", "spec/fixtures/pages_non_writeable.zip"]
-    end
+      where(:fixture_path) do
+        ["spec/fixtures/pages.zip", "spec/fixtures/pages_non_writeable.zip"]
+      end
 
-    with_them do
-      let(:full_fixture_path) { Rails.root.join(fixture_path) }
+      with_them do
+        let(:full_fixture_path) { Rails.root.join(fixture_path) }
 
-      it 'a created archives contains exactly the same entries' do
-        SafeZip::Extract.new(full_fixture_path).extract(directories: ['public'], to: @work_dir)
+        it 'a created archives contains exactly the same entries' do
+          SafeZip::Extract.new(full_fixture_path).extract(directories: ['public'], to: @work_dir)
 
-        with_zip_file do |created_archive|
-          Zip::File.open(full_fixture_path) do |original_archive|
-            original_archive.entries do |original_entry|
-              created_entry = created_archive.get_entry(original_entry.name)
+          with_zip_file do |created_archive|
+            Zip::File.open(full_fixture_path) do |original_archive|
+              original_archive.entries do |original_entry|
+                created_entry = created_archive.get_entry(original_entry.name)
 
-              expect(created_entry.name).to eq(original_entry.name)
-              expect(created_entry.ftype).to eq(original_entry.ftype)
-              expect(created_archive.read(created_entry)).to eq(original_archive.read(original_entry))
+                expect(created_entry.name).to eq(original_entry.name)
+                expect(created_entry.ftype).to eq(original_entry.ftype)
+                expect(created_archive.read(created_entry)).to eq(original_archive.read(original_entry))
+              end
             end
           end
         end
       end
     end
-  end
 
-  def create_file(name, content)
-    File.open(File.join(@work_dir, name), "w") do |f|
-      f.write(content)
+    def create_file(name, content)
+      File.open(File.join(@work_dir, name), "w") do |f|
+        f.write(content)
+      end
     end
-  end
 
-  def create_dir(dir)
-    Dir.mkdir(File.join(@work_dir, dir))
-  end
+    def create_dir(dir)
+      Dir.mkdir(File.join(@work_dir, dir))
+    end
 
-  def create_link(new_name, target)
-    File.symlink(target, File.join(@work_dir, new_name))
-  end
+    def create_link(new_name, target)
+      File.symlink(target, File.join(@work_dir, new_name))
+    end
 
-  def with_zip_file
-    Zip::File.open(archive) do |zip_file|
-      yield zip_file
+    def with_zip_file
+      Zip::File.open(archive) do |zip_file|
+        yield zip_file
+      end
     end
   end
 end