Commit af3597c7 authored by Kamil Trzciński's avatar Kamil Trzciński

Always proxy reports downloads

This makes to always proxy reports
parent 146d7b28
# 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
......
......@@ -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
......
......@@ -19,7 +19,8 @@ module EE
return download_project_job_artifacts_path(
job_artifact.project,
job_artifact.job,
file_type: file_type)
file_type: file_type,
proxy: true)
end
if (build_artifact = legacy_report_artifact_for_file_type(file_type)) &&
......
---
title: Always proxy reports downloads
merge_request:
author:
type: fixed
......@@ -108,6 +108,7 @@ describe MergeRequestWidgetEntity do
expect(subject.as_json[:license_management]).to include(:managed_licenses_path)
expect(subject.as_json[:license_management]).to include(:can_manage_licenses)
expect(subject.as_json[:license_management]).to include(:license_management_full_report_path)
expect(subject.as_json[:license_management][:head_path]).to include('proxy=true')
end
context 'when feature is not licensed' do
......
......@@ -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
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment