diff options
Diffstat (limited to 'spec/uploaders')
6 files changed, 336 insertions, 165 deletions
diff --git a/spec/uploaders/object_storage/cdn/google_cdn_spec.rb b/spec/uploaders/object_storage/cdn/google_cdn_spec.rb new file mode 100644 index 00000000000..b72f6d66d69 --- /dev/null +++ b/spec/uploaders/object_storage/cdn/google_cdn_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ObjectStorage::CDN::GoogleCDN, + :use_clean_rails_memory_store_caching, :use_clean_rails_redis_caching, :sidekiq_inline do + include StubRequests + + let(:key) { SecureRandom.hex } + let(:key_name) { 'test-key' } + let(:options) { { url: 'https://cdn.gitlab.example.com', key_name: key_name, key: Base64.urlsafe_encode64(key) } } + let(:google_cloud_ips) { File.read(Rails.root.join('spec/fixtures/cdn/google_cloud.json')) } + let(:headers) { { 'Content-Type' => 'application/json' } } + let(:public_ip) { '18.245.0.42' } + + subject { described_class.new(options) } + + before do + WebMock.stub_request(:get, GoogleCloud::FetchGoogleIpListService::GOOGLE_IP_RANGES_URL) + .to_return(status: 200, body: google_cloud_ips, headers: headers) + end + + describe '#use_cdn?' do + using RSpec::Parameterized::TableSyntax + + where(:ip_address, :expected) do + '34.80.0.1' | false + '18.245.0.42' | true + '2500:1900:4180:0000:0000:0000:0000:0000' | true + '2600:1900:4180:0000:0000:0000:0000:0000' | false + '10.10.1.5' | false + 'fc00:0000:0000:0000:0000:0000:0000:0000' | false + end + + with_them do + it { expect(subject.use_cdn?(ip_address)).to eq(expected) } + end + + context 'when the key name is missing' do + let(:options) { { url: 'https://cdn.gitlab.example.com', key: Base64.urlsafe_encode64(SecureRandom.hex) } } + + it 'returns false' do + expect(subject.use_cdn?(public_ip)).to be false + end + end + + context 'when the key is missing' do + let(:options) { { url: 'https://invalid.example.com' } } + + it 'returns false' do + expect(subject.use_cdn?(public_ip)).to be false + end + end + + context 'when the key is invalid' do + let(:options) { { key_name: key_name, key: '\0x1' } } + + it 'returns false' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original + expect(subject.use_cdn?(public_ip)).to be false + end + end + + context 'when the URL is missing' do + let(:options) { { key: Base64.urlsafe_encode64(SecureRandom.hex) } } + + it 'returns false' do + expect(subject.use_cdn?(public_ip)).to be false + end + end + end + + describe '#signed_url' do + let(:path) { '/path/to/file.txt' } + + it 'returns a valid signed URL' do + url = subject.signed_url(path) + + expect(url).to start_with("#{options[:url]}#{path}") + + uri = Addressable::URI.parse(url) + parsed_query = Rack::Utils.parse_nested_query(uri.query) + signature = parsed_query.delete('Signature') + + signed_url = "#{options[:url]}#{path}?Expires=#{parsed_query['Expires']}&KeyName=#{key_name}" + computed_signature = OpenSSL::HMAC.digest('SHA1', key, signed_url) + + aggregate_failures do + expect(parsed_query['Expires'].to_i).to be > 0 + expect(parsed_query['KeyName']).to eq(key_name) + expect(signature).to eq(Base64.urlsafe_encode64(computed_signature)) + end + end + end +end diff --git a/spec/uploaders/object_storage/cdn/google_ip_cache_spec.rb b/spec/uploaders/object_storage/cdn/google_ip_cache_spec.rb new file mode 100644 index 00000000000..d6568636bc0 --- /dev/null +++ b/spec/uploaders/object_storage/cdn/google_ip_cache_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ObjectStorage::CDN::GoogleIpCache, + :use_clean_rails_memory_store_caching, :use_clean_rails_redis_caching do + include StubRequests + + let(:subnets) { [IPAddr.new("34.80.0.0/15"), IPAddr.new("2600:1900:4180::/44")] } + let(:public_ip) { '18.245.0.42' } + + describe '.update!' do + it 'caches to both L1 and L2 caches' do + expect(Gitlab::ProcessMemoryCache.cache_backend.exist?(described_class::GOOGLE_CDN_LIST_KEY)).to be false + expect(Rails.cache.exist?(described_class::GOOGLE_CDN_LIST_KEY)).to be false + + described_class.update!(subnets) + + expect(Gitlab::ProcessMemoryCache.cache_backend.fetch(described_class::GOOGLE_CDN_LIST_KEY)).to eq(subnets) + expect(Rails.cache.fetch(described_class::GOOGLE_CDN_LIST_KEY)).to eq(subnets) + end + end + + describe '.ready?' do + it 'returns false' do + expect(described_class.ready?).to be false + end + + it 'returns true' do + described_class.update!(subnets) + + expect(described_class.ready?).to be true + end + end + + describe '.google_ip?' do + using RSpec::Parameterized::TableSyntax + + where(:ip_address, :expected) do + '34.80.0.1' | true + '18.245.0.42' | false + '2500:1900:4180:0000:0000:0000:0000:0000' | false + '2600:1900:4180:0000:0000:0000:0000:0000' | true + '10.10.1.5' | false + 'fc00:0000:0000:0000:0000:0000:0000:0000' | false + end + + before do + described_class.update!(subnets) + end + + with_them do + it { expect(described_class.google_ip?(ip_address)).to eq(expected) } + end + + it 'uses the L2 cache and updates the L1 cache when L1 is missing' do + Gitlab::ProcessMemoryCache.cache_backend.delete(described_class::GOOGLE_CDN_LIST_KEY) + expect(Rails.cache.fetch(described_class::GOOGLE_CDN_LIST_KEY)).to eq(subnets) + + expect(described_class.google_ip?(public_ip)).to be false + + expect(Gitlab::ProcessMemoryCache.cache_backend.fetch(described_class::GOOGLE_CDN_LIST_KEY)).to eq(subnets) + expect(Rails.cache.fetch(described_class::GOOGLE_CDN_LIST_KEY)).to eq(subnets) + end + + it 'avoids populating L1 cache if L2 is missing' do + Gitlab::ProcessMemoryCache.cache_backend.delete(described_class::GOOGLE_CDN_LIST_KEY) + Rails.cache.delete(described_class::GOOGLE_CDN_LIST_KEY) + + expect(described_class.google_ip?(public_ip)).to be false + + expect(Gitlab::ProcessMemoryCache.cache_backend.exist?(described_class::GOOGLE_CDN_LIST_KEY)).to be false + expect(Rails.cache.exist?(described_class::GOOGLE_CDN_LIST_KEY)).to be false + end + end + + describe '.async_refresh' do + it 'schedules the worker' do + expect(::GoogleCloud::FetchGoogleIpListWorker).to receive(:perform_async) + + described_class.async_refresh + end + end +end diff --git a/spec/uploaders/object_storage/cdn_spec.rb b/spec/uploaders/object_storage/cdn_spec.rb new file mode 100644 index 00000000000..246cb1bf349 --- /dev/null +++ b/spec/uploaders/object_storage/cdn_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ObjectStorage::CDN do + let(:cdn_options) do + { + 'object_store' => { + 'cdn' => { + 'provider' => 'google', + 'url' => 'https://gitlab.example.com', + 'key_name' => 'test-key', + 'key' => '12345' + } + } + }.freeze + end + + let(:uploader_class) do + Class.new(GitlabUploader) do + include ObjectStorage::Concern + include ObjectStorage::CDN::Concern + + private + + # user/:id + def dynamic_segment + File.join(model.class.underscore, model.id.to_s) + end + end + end + + let(:object) { build_stubbed(:user) } + + subject { uploader_class.new(object, :file) } + + context 'with CDN config' do + before do + uploader_class.options = Settingslogic.new(Gitlab.config.uploads.deep_merge(cdn_options)) + end + + describe '#use_cdn?' do + it 'returns true' do + expect_next_instance_of(ObjectStorage::CDN::GoogleCDN) do |cdn| + expect(cdn).to receive(:use_cdn?).and_return(true) + end + + expect(subject.use_cdn?('18.245.0.1')).to be true + end + end + + describe '#cdn_signed_url' do + it 'returns a URL' do + expect_next_instance_of(ObjectStorage::CDN::GoogleCDN) do |cdn| + expect(cdn).to receive(:signed_url).and_return("https://cdn.example.com/path") + end + + expect(subject.cdn_signed_url).to eq("https://cdn.example.com/path") + end + end + end + + context 'without CDN config' do + before do + uploader_class.options = Gitlab.config.uploads + end + + describe '#use_cdn?' do + it 'returns false' do + expect(subject.use_cdn?('18.245.0.1')).to be false + end + end + end + + context 'with an unknown CDN provider' do + before do + cdn_options['object_store']['cdn']['provider'] = 'amazon' + uploader_class.options = Settingslogic.new(Gitlab.config.uploads.deep_merge(cdn_options)) + end + + it 'raises an error' do + expect { subject.use_cdn?('18.245.0.1') }.to raise_error("Unknown CDN provider: amazon") + end + end +end diff --git a/spec/uploaders/packages/debian/distribution_release_file_uploader_spec.rb b/spec/uploaders/packages/debian/distribution_release_file_uploader_spec.rb index 203a453bcdd..dbbf69e3c8d 100644 --- a/spec/uploaders/packages/debian/distribution_release_file_uploader_spec.rb +++ b/spec/uploaders/packages/debian/distribution_release_file_uploader_spec.rb @@ -49,12 +49,12 @@ RSpec.describe Packages::Debian::DistributionReleaseFileUploader do end describe '#filename' do - it { expect(subject.filename).to eq('Release')} + it { expect(subject.filename).to eq('Release') } context 'with signed_file' do let(:uploader) { described_class.new(distribution, :signed_file) } - it { expect(subject.filename).to eq('InRelease')} + it { expect(subject.filename).to eq('InRelease') } end end end diff --git a/spec/uploaders/packages/package_file_uploader_spec.rb b/spec/uploaders/packages/package_file_uploader_spec.rb index e8f4cae7b04..0c7bf6432cb 100644 --- a/spec/uploaders/packages/package_file_uploader_spec.rb +++ b/spec/uploaders/packages/package_file_uploader_spec.rb @@ -2,50 +2,43 @@ require 'spec_helper' RSpec.describe Packages::PackageFileUploader do - { - package_file: %r[^\h{2}/\h{2}/\h{64}/packages/\d+/files/\d+$], - debian_package_file: %r[^\h{2}/\h{2}/\h{64}/packages/debian/files/\d+$] - }.each do |factory, store_dir_regex| - context factory.to_s do - let(:package_file) { create(factory) } # rubocop:disable Rails/SaveBang - let(:uploader) { described_class.new(package_file, :file) } - let(:path) { Gitlab.config.packages.storage_path } - - subject { uploader } - - it_behaves_like "builds correct paths", - store_dir: store_dir_regex, - cache_dir: %r[/packages/tmp/cache], - work_dir: %r[/packages/tmp/work] - - context 'object store is remote' do - before do - stub_package_file_object_storage - end - - include_context 'with storage', described_class::Store::REMOTE - - it_behaves_like "builds correct paths", - store_dir: store_dir_regex - end + let(:package_file) { create(:package_file) } + let(:uploader) { described_class.new(package_file, :file) } + let(:path) { Gitlab.config.packages.storage_path } + + subject { uploader } + + it_behaves_like "builds correct paths", + store_dir: %r[^\h{2}/\h{2}/\h{64}/packages/\d+/files/\d+$], + cache_dir: %r[/packages/tmp/cache], + work_dir: %r[/packages/tmp/work] + + context 'object store is remote' do + before do + stub_package_file_object_storage + end - describe 'remote file' do - let(:package_file) { create(factory, :object_storage) } + include_context 'with storage', described_class::Store::REMOTE - context 'with object storage enabled' do - before do - stub_package_file_object_storage - end + it_behaves_like "builds correct paths", + store_dir: %r[^\h{2}/\h{2}/\h{64}/packages/\d+/files/\d+$] + end + + describe 'remote file' do + let(:package_file) { create(:package_file, :object_storage) } + + context 'with object storage enabled' do + before do + stub_package_file_object_storage + end - it 'can store file remotely' do - allow(ObjectStorage::BackgroundMoveWorker).to receive(:perform_async) + it 'can store file remotely' do + allow(ObjectStorage::BackgroundMoveWorker).to receive(:perform_async) - package_file + package_file - expect(package_file.file_store).to eq(described_class::Store::REMOTE) - expect(package_file.file.path).not_to be_blank - end - end + expect(package_file.file_store).to eq(described_class::Store::REMOTE) + expect(package_file.file.path).not_to be_blank end end end diff --git a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb index fd01a18e810..1746f480c9b 100644 --- a/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb @@ -3,120 +3,62 @@ require 'spec_helper' RSpec.describe ObjectStorage::MigrateUploadsWorker do - let(:model_class) { Project } + let(:project) { create(:project, :with_avatar) } let(:uploads) { Upload.all } - let(:to_store) { ObjectStorage::Store::REMOTE } - def perform(uploads, store = nil) - described_class.new.perform(uploads.ids, model_class.to_s, mounted_as, store || to_store) + def perform(uploads, store = ObjectStorage::Store::REMOTE) + described_class.new.perform(uploads.ids, store) rescue ObjectStorage::MigrateUploadsWorker::Report::MigrationFailures # swallow end - # Expects the calling spec to define: - # - model_class - # - mounted_as - # - to_store - RSpec.shared_examples 'uploads migration worker' do - describe '.enqueue!' do - def enqueue! - described_class.enqueue!(uploads, model_class, mounted_as, to_store) - end - - it 'is guarded by .sanity_check!' do - expect(described_class).to receive(:perform_async) - expect(described_class).to receive(:sanity_check!) + before do + stub_uploads_object_storage(AvatarUploader) + stub_uploads_object_storage(FileUploader) - enqueue! - end + FileUploader.new(project).store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) + end - context 'sanity_check! fails' do - before do - expect(described_class).to receive(:sanity_check!).and_raise(described_class::SanityCheckError) - end + describe '#perform' do + it 'migrates files to remote storage' do + expect(Gitlab::AppLogger).to receive(:info).with(%r{Migrated 2/2 files}) - it 'does not enqueue a job' do - expect(described_class).not_to receive(:perform_async) + perform(uploads) - expect { enqueue! }.to raise_error(described_class::SanityCheckError) - end - end + expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(0) + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(2) end - describe '.sanity_check!' do - shared_examples 'raises a SanityCheckError' do |expected_message| - let(:mount_point) { nil } - - it do - expect { described_class.sanity_check!(uploads, model_class, mount_point) } - .to raise_error(described_class::SanityCheckError).with_message(expected_message) - end + context 'reversed' do + before do + perform(uploads) end - context 'uploader types mismatch' do - let!(:outlier) { create(:upload, uploader: 'GitlabUploader') } + it 'migrates files to local storage' do + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(2) - include_examples 'raises a SanityCheckError', /Multiple uploaders found/ - end + perform(uploads, ObjectStorage::Store::LOCAL) - context 'mount point not found' do - include_examples 'raises a SanityCheckError', /Mount point [a-z:]+ not found in/ do - let(:mount_point) { :potato } - end + expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(2) + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(0) end end - describe '#perform' do - it 'migrates files to remote storage' do - expect(Gitlab::AppLogger).to receive(:info).with(%r{Migrated 1/1 files}) - - perform(uploads) - - expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(0) - end - - context 'reversed' do - let(:to_store) { ObjectStorage::Store::LOCAL } - - before do - perform(uploads, ObjectStorage::Store::REMOTE) - end - - it 'migrates files to local storage' do - expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(1) - - perform(uploads) - - expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(1) - end + context 'migration is unsuccessful' do + before do + allow_any_instance_of(ObjectStorage::Concern) + .to receive(:migrate!).and_raise(CarrierWave::UploadError, 'I am a teapot.') end - context 'migration is unsuccessful' do - before do - allow_any_instance_of(ObjectStorage::Concern) - .to receive(:migrate!).and_raise(CarrierWave::UploadError, 'I am a teapot.') - end - - it 'does not migrate files to remote storage' do - expect(Gitlab::AppLogger).to receive(:warn).with(/Error .* I am a teapot/) + it 'does not migrate files to remote storage' do + expect(Gitlab::AppLogger).to receive(:warn).with(/Error .* I am a teapot/) - perform(uploads) + perform(uploads) - expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(1) - end + expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(2) + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(0) end end - end - - context "for AvatarUploader" do - let!(:project_with_avatar) { create(:project, :with_avatar) } - let(:mounted_as) { :avatar } - - before do - stub_uploads_object_storage(AvatarUploader) - end - - it_behaves_like "uploads migration worker" describe "limits N+1 queries" do it "to N*5" do @@ -127,46 +69,18 @@ RSpec.describe ObjectStorage::MigrateUploadsWorker do expect { perform(Upload.all) }.not_to exceed_query_limit(query_count).with_threshold(5) end end - end - - context "for FileUploader" do - let!(:project_with_file) { create(:project) } - let(:secret) { SecureRandom.hex } - let(:mounted_as) { nil } - - def upload_file(project) - uploader = FileUploader.new(project) - uploader.store!(fixture_file_upload('spec/fixtures/doc_sample.txt')) - end - - before do - stub_uploads_object_storage(FileUploader) - - upload_file(project_with_file) - end - - it_behaves_like "uploads migration worker" - - describe "limits N+1 queries" do - it "to N*5" do - query_count = ActiveRecord::QueryRecorder.new { perform(uploads) } - upload_file(create(:project)) + it 'handles legacy argument format' do + described_class.new.perform(uploads.ids, 'Project', :avatar, ObjectStorage::Store::REMOTE) - expect { perform(Upload.all) }.not_to exceed_query_limit(query_count).with_threshold(5) - end + expect(Upload.where(store: ObjectStorage::Store::LOCAL).count).to eq(0) + expect(Upload.where(store: ObjectStorage::Store::REMOTE).count).to eq(2) end - end - context 'for DesignManagement::DesignV432x230Uploader' do - let(:model_class) { DesignManagement::Action } - let!(:design_action) { create(:design_action, :with_image_v432x230) } - let(:mounted_as) { :image_v432x230 } + it 'logs an error when number of arguments is incorrect' do + expect(Gitlab::AppLogger).to receive(:warn).with(/Job has wrong arguments format/) - before do - stub_uploads_object_storage(DesignManagement::DesignV432x230Uploader) + described_class.new.perform(uploads.ids, 'Project', ObjectStorage::Store::REMOTE) end - - it_behaves_like 'uploads migration worker' end end |