diff --git a/app/controllers/concerns/send_file_upload.rb b/app/controllers/concerns/send_file_upload.rb index 0bb7b7efed023ebb922619e34508c9188a1c04de..515a9eede8e8672f0fe2d447cae3b7f6300e8b4a 100644 --- a/app/controllers/concerns/send_file_upload.rb +++ b/app/controllers/concerns/send_file_upload.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module SendFileUpload - def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, disposition: 'attachment') + def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, proxy: false, disposition: 'attachment') if attachment # Response-Content-Type will not override an existing Content-Type in # Google Cloud Storage, so the metadata needs to be cleared on GCS for @@ -17,7 +17,7 @@ module SendFileUpload if file_upload.file_storage? send_file file_upload.path, send_params - elsif file_upload.class.proxy_download_enabled? + elsif file_upload.class.proxy_download_enabled? || proxy headers.store(*Gitlab::Workhorse.send_url(file_upload.url(**redirect_params))) head :ok else diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 312e256ea6c8d89251b50a250da2189f774d3bc5..ae9c17802b97d564cd4de6da188db33a0d4a7022 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -16,7 +16,7 @@ class Projects::ArtifactsController < Projects::ApplicationController def download return render_404 unless artifacts_file - send_upload(artifacts_file, attachment: artifacts_file.filename) + send_upload(artifacts_file, attachment: artifacts_file.filename, proxy: params[:proxy]) end def browse diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 767fba7fd586da184eadea7bdeb395b69849b536..4f1f6bb31f32263c0fc31337bb41860249555cc6 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -28,8 +28,9 @@ describe SendFileUpload do describe '#send_upload' do let(:controller) { controller_class.new } let(:temp_file) { Tempfile.new('test') } + let(:params) { {} } - subject { controller.send_upload(uploader) } + subject { controller.send_upload(uploader, **params) } before do FileUtils.touch(temp_file) @@ -52,7 +53,7 @@ describe SendFileUpload do end context 'with attachment' do - let(:send_attachment) { controller.send_upload(uploader, attachment: 'test.js') } + let(:params) { { attachment: 'test.js' } } it 'sends a file with content-type of text/plain' do expected_params = { @@ -62,7 +63,7 @@ describe SendFileUpload do } expect(controller).to receive(:send_file).with(uploader.path, expected_params) - send_attachment + subject end context 'with a proxied file in object storage' do @@ -83,7 +84,7 @@ describe SendFileUpload do expect(controller).to receive(:headers) { headers } expect(controller).to receive(:head).with(:ok) - send_attachment + subject end end end @@ -95,11 +96,7 @@ describe SendFileUpload do uploader.store!(temp_file) end - context 'and proxying is enabled' do - before do - allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { true } - end - + shared_examples 'proxied file' do it 'sends a file' do headers = double expect(Gitlab::Workhorse).not_to receive(:send_url).with(/response-content-disposition/) @@ -115,6 +112,14 @@ describe SendFileUpload do end end + context 'and proxying is enabled' do + before do + allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { true } + end + + it_behaves_like 'proxied file' + end + context 'and proxying is disabled' do before do allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { false } @@ -125,6 +130,12 @@ describe SendFileUpload do subject end + + context 'with proxy requested' do + let(:params) { { proxy: true } } + + it_behaves_like 'proxied file' + end end end end diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index 6091185e2523ef9164437935339048518f243b49..b3c8d6a954ec14ff4bb2fdf3af5fba824533ece6 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -47,14 +47,37 @@ describe Projects::ArtifactsController do context 'when codequality file type is supplied' do let(:file_type) { 'codequality' } - before do - create(:ci_job_artifact, :codequality, job: job) + context 'when file is stored locally' do + before do + create(:ci_job_artifact, :codequality, job: job) + end + + it 'sends the codequality report' do + expect(controller).to receive(:send_file).with(job.job_artifacts_codequality.file.path, hash_including(disposition: 'attachment')).and_call_original + + download_artifact(file_type: file_type) + end end - it 'sends the codequality report' do - expect(controller).to receive(:send_file).with(job.job_artifacts_codequality.file.path, hash_including(disposition: 'attachment')).and_call_original + context 'when file is stored remotely' do + before do + stub_artifacts_object_storage + create(:ci_job_artifact, :remote_store, :codequality, job: job) + end + + it 'sends the codequality report' do + expect(controller).to receive(:redirect_to).and_call_original - download_artifact(file_type: file_type) + download_artifact(file_type: file_type) + end + + context 'when proxied' do + it 'sends the codequality report' do + expect(Gitlab::Workhorse).to receive(:send_url).and_call_original + + download_artifact(file_type: file_type, proxy: true) + end + end end end end