Commit e34d7c85 authored by Sincheol (David) Kim's avatar Sincheol (David) Kim

Merge branch 'sh-fix-url-blocker' into 'master'

Fix URL blocker when object storage enabled but type is disabled

See merge request gitlab-org/gitlab!84511
parents da9a8aa0 7560d2dc
......@@ -289,9 +289,10 @@ module Gitlab
ObjectStoreSettings::SUPPORTED_TYPES.collect do |type|
section_setting = config.try(type)
next unless section_setting
next unless section_setting && section_setting['enabled']
object_store_setting = section_setting['object_store']
# Use #to_h to avoid Settingslogic bug: https://gitlab.com/gitlab-org/gitlab/-/issues/286873
object_store_setting = section_setting['object_store']&.to_h
next unless object_store_setting && object_store_setting['enabled']
......
......@@ -167,6 +167,7 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil do
context 'for object_storage uri' do
let(:enabled_object_storage_setting) do
{
'enabled' => true,
'object_store' =>
{
'enabled' => true,
......
......@@ -43,6 +43,7 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
let(:import_url) { "#{host}/external-diffs/merge_request_diffs/mr-1/diff-1" }
let(:enabled_object_storage_setting) do
{
'enabled' => true,
'object_store' =>
{
'enabled' => true,
......@@ -81,6 +82,49 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
let(:expected_hostname) { nil }
end
end
context 'when LFS object storage is enabled' do
let(:lfs_config) do
{
'enabled' => lfs_enabled,
# This nesting of Settingslogic is necessary to trigger the bug
'object_store' => Settingslogic.new({ 'enabled' => true })
}
end
let(:config) do
{
'gitlab' => Gitlab.config.gitlab,
'repositories' => { 'storages' => { 'default' => 'test' } },
'lfs' => Settingslogic.new(lfs_config)
}
end
let(:host) { 'http://127.0.0.1:9000' }
let(:settings) { Settingslogic.new(config) }
before do
allow(Gitlab).to receive(:config).and_return(settings)
# Triggers Settingslogic bug: https://gitlab.com/gitlab-org/gitlab/-/issues/286873
settings.repositories.storages.default
end
context 'when LFS is disabled' do
let(:lfs_enabled) { false }
it 'raises an error' do
expect { subject }.to raise_error(described_class::BlockedUrlError)
end
end
context 'when LFS is enabled with no connection endpoint' do
let(:lfs_enabled) { true }
it 'raises an error' do
expect { subject }.to raise_error(described_class::BlockedUrlError)
end
end
end
end
context 'when allow_object_storage is false' do
......
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