From 77a6ec22ba9057154925a6484c05ae204423aacd Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 25 Jul 2017 12:45:17 +0200 Subject: [PATCH 1/4] Handle maximum pages artifacts size correctly --- app/services/projects/update_pages_service.rb | 4 +- .../projects/update_pages_service_spec.rb | 72 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index e60b854f916..a819b799ff8 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -130,7 +130,9 @@ module Projects end def max_size - current_application_settings.max_pages_size.megabytes || MAX_SIZE + current_application_settings.max_pages_size.megabytes.tap do |maximum| + return MAX_SIZE if maximum.zero? || maximum > MAX_SIZE + end end def tmp_path diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index fc0a17296f3..8210f8a9608 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -96,6 +96,78 @@ describe Projects::UpdatePagesService do expect(execute).not_to eq(:success) end + describe 'maximum pages artifacts size' do + let(:metadata) { spy('metadata') } + + before do + file = fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip') + metafile = fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip.meta') + + build.update_attributes(artifacts_file: file) + build.update_attributes(artifacts_metadata: metafile) + + allow(build).to receive(:artifacts_metadata_entry) + .and_return(metadata) + end + + shared_examples 'pages size limit exceeded' do + it 'limits the maximum size of gitlab pages' do + subject.execute + + expect(deploy_status.description) + .to match(/artifacts for pages are too large/) + end + end + + context 'when maximum pages size is set to zero' do + before do + stub_application_setting(max_pages_size: 0) + end + + context 'when page size does not exceed internal maximum' do + before do + allow(metadata).to receive(:total_size).and_return(200.megabytes) + end + + it 'updates pages correctly' do + subject.execute + + expect(deploy_status.description).not_to be_present + end + end + + context 'when pages size does exceed internal maximum' do + before do + allow(metadata).to receive(:total_size).and_return(2.terabytes) + end + + it_behaves_like 'pages size limit exceeded' + end + end + + context 'when pages size is greater than max size setting' do + before do + stub_application_setting(max_pages_size: 200) + allow(metadata).to receive(:total_size).and_return(201.megabytes) + end + + it_behaves_like 'pages size limit exceeded' + end + + context 'when max size setting is greater than internal max size' do + before do + stub_application_setting(max_pages_size: 3.terabytes / 1.megabyte) + allow(metadata).to receive(:total_size).and_return(2.terabytes) + end + + it_behaves_like 'pages size limit exceeded' + end + end + + def deploy_status + GenericCommitStatus.find_by(name: 'pages:deploy'); + end + def execute subject.execute[:status] end From 8a50e5fb0c02e37c3eef80c88edfcb902571769c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 25 Jul 2017 13:04:22 +0200 Subject: [PATCH 2/4] Add changelog for max pages artifacts size fix --- .../fix-gb-handle-max-pages-artifacts-size-correctly.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/fix-gb-handle-max-pages-artifacts-size-correctly.yml diff --git a/changelogs/unreleased/fix-gb-handle-max-pages-artifacts-size-correctly.yml b/changelogs/unreleased/fix-gb-handle-max-pages-artifacts-size-correctly.yml new file mode 100644 index 00000000000..3d9592bbf2a --- /dev/null +++ b/changelogs/unreleased/fix-gb-handle-max-pages-artifacts-size-correctly.yml @@ -0,0 +1,4 @@ +--- +title: Handle maximum pages artifacts size correctly +merge_request: 13072 +author: From 7151fb754b82888e022bfced02f2fdfd9000a1ff Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 25 Jul 2017 13:47:03 +0200 Subject: [PATCH 3/4] Fix rubocop offense in update pages service specs --- spec/services/projects/update_pages_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index 8210f8a9608..aa6ad6340f5 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -165,7 +165,7 @@ describe Projects::UpdatePagesService do end def deploy_status - GenericCommitStatus.find_by(name: 'pages:deploy'); + GenericCommitStatus.find_by(name: 'pages:deploy') end def execute From cb405aa45dd5acf766797a7375043b6608d394f8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 26 Jul 2017 11:19:57 +0200 Subject: [PATCH 4/4] Refactor max_size method in update pages service As per review feedback https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13072#note_35853177 --- app/services/projects/update_pages_service.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index a819b799ff8..749a1cc56d8 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -130,9 +130,11 @@ module Projects end def max_size - current_application_settings.max_pages_size.megabytes.tap do |maximum| - return MAX_SIZE if maximum.zero? || maximum > MAX_SIZE - end + max_pages_size = current_application_settings.max_pages_size.megabytes + + return MAX_SIZE if max_pages_size.zero? + + [max_pages_size, MAX_SIZE].min end def tmp_path