summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2018-11-07 16:54:37 +0100
committerKamil Trzciński <ayufan@ayufan.eu>2018-11-07 21:49:45 +0100
commit5da2f42dc9609c07fd60d9d1c7a2302353a820c0 (patch)
tree79d13b23dfdca03f6bcccc4a1ba5c7574df1b145
parent829e452588575c634b2a3dd778e702a6b21465e1 (diff)
downloadgitlab-ce-5da2f42dc9609c07fd60d9d1c7a2302353a820c0.tar.gz
backport: Always proxy reports downloadsalways-proxy-reports
This makes to always proxy reports
-rw-r--r--app/controllers/concerns/send_file_upload.rb4
-rw-r--r--app/controllers/projects/artifacts_controller.rb2
-rw-r--r--spec/controllers/concerns/send_file_upload_spec.rb29
-rw-r--r--spec/controllers/projects/artifacts_controller_spec.rb33
4 files changed, 51 insertions, 17 deletions
diff --git a/app/controllers/concerns/send_file_upload.rb b/app/controllers/concerns/send_file_upload.rb
index 0bb7b7efed0..515a9eede8e 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 312e256ea6c..ae9c17802b9 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 767fba7fd58..4f1f6bb31f3 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 6091185e252..b3c8d6a954e 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