Commit b7e58108 authored by Vladimir Shushlin's avatar Vladimir Shushlin Committed by Kamil Trzciński

Don't serve pages stored on legacy storage

returnting 500 error would cause pages to retry
this ensures that pages simply would ignore the project
parent 24590717
......@@ -4,6 +4,8 @@ module Pages
class LookupPath
include Gitlab::Utils::StrongMemoize
LegacyStorageDisabledError = Class.new(::StandardError)
def initialize(project, trim_prefix: nil, domain: nil)
@project = project
@domain = domain
......@@ -24,7 +26,7 @@ module Pages
end
def source
zip_source || file_source
zip_source || legacy_source
end
def prefix
......@@ -64,11 +66,17 @@ module Pages
}
end
def file_source
def legacy_source
raise LegacyStorageDisabledError unless Feature.enabled?(:pages_serve_from_legacy_storage, default_enabled: true)
{
type: 'file',
path: File.join(project.full_path, 'public/')
}
rescue LegacyStorageDisabledError => e
Gitlab::ErrorTracking.track_exception(e)
nil
end
end
end
......@@ -17,9 +17,16 @@ module Pages
end
def lookup_paths
projects.map do |project|
paths = projects.map do |project|
project.pages_lookup_path(trim_prefix: trim_prefix, domain: domain)
end.sort_by(&:prefix).reverse
end
# TODO: remove in https://gitlab.com/gitlab-org/gitlab/-/issues/297524
# source can only be nil if pages_serve_from_legacy_storage FF is disabled
# we can remove this filtering once we remove legacy storage
paths = paths.select(&:source)
paths.sort_by(&:prefix).reverse
end
private
......
---
name: pages_serve_from_legacy_storage
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/297228
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/297524
milestone: '13.8'
type: development
group: group::release
default_enabled: true
......@@ -56,6 +56,15 @@ RSpec.describe Pages::LookupPath do
include_examples 'uses disk storage'
it 'return nil when legacy storage is disabled and there is no deployment' do
stub_feature_flags(pages_serve_from_legacy_storage: false)
expect(Gitlab::ErrorTracking).to receive(:track_exception)
.with(described_class::LegacyStorageDisabledError)
.and_call_original
expect(source).to eq(nil)
end
context 'when there is pages deployment' do
let(:deployment) { create(:pages_deployment, project: project) }
......
......@@ -26,31 +26,34 @@ RSpec.describe Pages::VirtualDomain do
describe '#lookup_paths' do
let(:project_a) { instance_double(Project) }
let(:project_z) { instance_double(Project) }
let(:pages_lookup_path_a) { instance_double(Pages::LookupPath, prefix: 'aaa') }
let(:pages_lookup_path_z) { instance_double(Pages::LookupPath, prefix: 'zzz') }
let(:project_b) { instance_double(Project) }
let(:project_c) { instance_double(Project) }
let(:pages_lookup_path_a) { instance_double(Pages::LookupPath, prefix: 'aaa', source: { type: 'zip', path: 'https://example.com' }) }
let(:pages_lookup_path_b) { instance_double(Pages::LookupPath, prefix: 'bbb', source: { type: 'zip', path: 'https://example.com' }) }
let(:pages_lookup_path_without_source) { instance_double(Pages::LookupPath, prefix: 'ccc', source: nil) }
context 'when there is pages domain provided' do
let(:domain) { instance_double(PagesDomain) }
subject(:virtual_domain) { described_class.new([project_a, project_z], domain: domain) }
subject(:virtual_domain) { described_class.new([project_a, project_b, project_c], domain: domain) }
it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do
expect(project_a).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_a)
expect(project_z).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_z)
expect(project_b).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_b)
expect(project_c).to receive(:pages_lookup_path).with(domain: domain, trim_prefix: nil).and_return(pages_lookup_path_without_source)
expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_z, pages_lookup_path_a])
expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_b, pages_lookup_path_a])
end
end
context 'when there is trim_prefix provided' do
subject(:virtual_domain) { described_class.new([project_a, project_z], trim_prefix: 'group/') }
subject(:virtual_domain) { described_class.new([project_a, project_b], trim_prefix: 'group/') }
it 'returns collection of projects pages lookup paths sorted by prefix in reverse' do
expect(project_a).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_a)
expect(project_z).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_z)
expect(project_b).to receive(:pages_lookup_path).with(trim_prefix: 'group/', domain: nil).and_return(pages_lookup_path_b)
expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_z, pages_lookup_path_a])
expect(virtual_domain.lookup_paths).to eq([pages_lookup_path_b, pages_lookup_path_a])
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment