From 6c55a9d8f29a4d934dab2c3294c319bb8592fe2b Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 29 Mar 2018 19:37:00 +0200 Subject: [PATCH 1/2] Remove support for legacy tar.gz pages artifacts --- app/services/projects/update_pages_service.rb | 14 +- .../projects/update_pages_service_spec.rb | 120 +++++++++--------- 2 files changed, 59 insertions(+), 75 deletions(-) diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 7e228d1833d..de77f6bf585 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -74,25 +74,13 @@ module Projects end def extract_archive!(temp_path) - if artifacts.ends_with?('.tar.gz') || artifacts.ends_with?('.tgz') - extract_tar_archive!(temp_path) - elsif artifacts.ends_with?('.zip') + if artifacts.ends_with?('.zip') extract_zip_archive!(temp_path) else raise InvaildStateError, 'unsupported artifacts format' end end - def extract_tar_archive!(temp_path) - build.artifacts_file.use_file do |artifacts_path| - results = Open3.pipeline(%W(gunzip -c #{artifacts_path}), - %W(dd bs=#{BLOCK_SIZE} count=#{blocks}), - %W(tar -x -C #{temp_path} #{SITE_PATH}), - err: '/dev/null') - raise FailedToExtractError, 'pages failed to extract' unless results.compact.all?(&:success?) - end - end - def extract_zip_archive!(temp_path) raise InvaildStateError, 'missing artifacts metadata' unless build.artifacts_metadata? diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index dd31a677dfe..1b6caeab15d 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -21,76 +21,72 @@ describe Projects::UpdatePagesService do end context 'legacy artifacts' do - %w(tar.gz zip).each do |format| - let(:extension) { format } + let(:extension) { 'zip' } - context "for valid #{format}" do + before do + build.update_attributes(legacy_artifacts_file: file) + build.update_attributes(legacy_artifacts_metadata: metadata) + end + + describe 'pages artifacts' do + context 'with expiry date' do before do - build.update_attributes(legacy_artifacts_file: file) - build.update_attributes(legacy_artifacts_metadata: metadata) + build.artifacts_expire_in = "2 days" + build.save! end - describe 'pages artifacts' do - context 'with expiry date' do - before do - build.artifacts_expire_in = "2 days" - build.save! - end - - it "doesn't delete artifacts" do - expect(execute).to eq(:success) - - expect(build.reload.artifacts?).to eq(true) - end - end - - context 'without expiry date' do - it "does delete artifacts" do - expect(execute).to eq(:success) - - expect(build.reload.artifacts?).to eq(false) - end - end - end - - it 'succeeds' do - expect(project.pages_deployed?).to be_falsey + it "doesn't delete artifacts" do expect(execute).to eq(:success) - expect(project.pages_deployed?).to be_truthy - # Check that all expected files are extracted - %w[index.html zero .hidden/file].each do |filename| - expect(File.exist?(File.join(project.public_pages_path, filename))).to be_truthy - end - end - - it 'limits pages size' do - stub_application_setting(max_pages_size: 1) - expect(execute).not_to eq(:success) - end - - it 'removes pages after destroy' do - expect(PagesWorker).to receive(:perform_in) - expect(project.pages_deployed?).to be_falsey - expect(execute).to eq(:success) - expect(project.pages_deployed?).to be_truthy - project.destroy - expect(project.pages_deployed?).to be_falsey - end - - it 'fails if sha on branch is not latest' do - build.update_attributes(ref: 'feature') - - expect(execute).not_to eq(:success) - end - - it 'fails for empty file fails' do - build.update_attributes(legacy_artifacts_file: empty_file) - - expect { execute } - .to raise_error(Projects::UpdatePagesService::FailedToExtractError) + expect(build.reload.artifacts?).to eq(true) end end + + context 'without expiry date' do + it "does delete artifacts" do + expect(execute).to eq(:success) + + expect(build.reload.artifacts?).to eq(false) + end + end + end + + it 'succeeds' do + expect(project.pages_deployed?).to be_falsey + expect(execute).to eq(:success) + expect(project.pages_deployed?).to be_truthy + + # Check that all expected files are extracted + %w[index.html zero .hidden/file].each do |filename| + expect(File.exist?(File.join(project.public_pages_path, filename))).to be_truthy + end + end + + it 'limits pages size' do + stub_application_setting(max_pages_size: 1) + expect(execute).not_to eq(:success) + end + + it 'removes pages after destroy' do + expect(PagesWorker).to receive(:perform_in) + expect(project.pages_deployed?).to be_falsey + expect(execute).to eq(:success) + expect(project.pages_deployed?).to be_truthy + project.destroy + expect(project.pages_deployed?).to be_falsey + end + + it 'fails if sha on branch is not latest' do + build.update_attributes(ref: 'feature') + + expect(execute).not_to eq(:success) + end + + it 'fails for empty file fails' do + build.update_attributes(legacy_artifacts_file: empty_file) + + expect { execute } + .to raise_error(Projects::UpdatePagesService::FailedToExtractError) end end From 86758aa134b08f561a89c5a3c067b952bdb61b28 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Fri, 6 Apr 2018 13:24:03 +0200 Subject: [PATCH 2/2] Changelog --- changelogs/unreleased/remove-pages-tar-support.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/remove-pages-tar-support.yml diff --git a/changelogs/unreleased/remove-pages-tar-support.yml b/changelogs/unreleased/remove-pages-tar-support.yml new file mode 100644 index 00000000000..73448687912 --- /dev/null +++ b/changelogs/unreleased/remove-pages-tar-support.yml @@ -0,0 +1,5 @@ +--- +title: Remove support for legacy tar.gz pages artifacts +merge_request: 18090 +author: +type: deprecated