diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index bea438e9ade7708f8a0fc26bdacda06231f4a434..40c341bdcdbe83bbbda981fa85368c0e1a63d0c7 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -3.3.1 +3.6.0 diff --git a/changelogs/unreleased-ee/use-send-url-for-incompatible-runners.yml b/changelogs/unreleased-ee/use-send-url-for-incompatible-runners.yml new file mode 100644 index 0000000000000000000000000000000000000000..6e924a9ee0b138eb7fe872fa8f9616fb38c682c5 --- /dev/null +++ b/changelogs/unreleased-ee/use-send-url-for-incompatible-runners.yml @@ -0,0 +1,6 @@ +--- +title: Support SendURL for performing indirect download of artifacts if clients does + not specify that it supports that +merge_request: +author: +type: fixed diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index cc81e4d3595a1cd92b8f506e82cf7dc20287fee1..d4ca945873c6e46c481fd6f3b0f9c70adff77092 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -418,13 +418,17 @@ module API end end - def present_artifacts!(artifacts_file) + def present_artifacts!(artifacts_file, direct_download: true) return not_found! unless artifacts_file.exists? if artifacts_file.file_storage? present_file!(artifacts_file.path, artifacts_file.filename) - else + elsif direct_download redirect(artifacts_file.url) + else + header(*Gitlab::Workhorse.send_url(artifacts_file.url)) + status :ok + body end end diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 1f80646a2ea5c71889c87e434b386f2e7d9d9164..e6e85d41806cdc1cc816091c597f4b49f3c343db 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -244,11 +244,12 @@ module API params do requires :id, type: Integer, desc: %q(Job's ID) optional :token, type: String, desc: %q(Job's authentication token) + optional :direct_download, default: false, type: Boolean, desc: %q(Perform direct download from remote storage instead of proxying artifacts) end get '/:id/artifacts' do job = authenticate_job! - present_artifacts!(job.artifacts_file) + present_artifacts!(job.artifacts_file, direct_download: params[:direct_download]) end end end diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index dfe8acd4833c6226c0012cc9437071b07cd0950b..990a6b1d80d2bda8446f840330764fd22f6d5e7a 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -151,6 +151,18 @@ module Gitlab ] end + def send_url(url, allow_redirects: false) + params = { + 'URL' => url, + 'AllowRedirects' => allow_redirects + } + + [ + SEND_DATA_HEADER, + "send-url:#{encode(params)}" + ] + end + def terminal_websocket(terminal) details = { 'Terminal' => { diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 249c77dc636d8a474ecb0e200002e991c5ed33b9..0b34d71bfb221dbc739acdb2487a7381896c0b64 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -451,4 +451,21 @@ describe Gitlab::Workhorse do end end end + + describe '.send_url' do + let(:url) { 'http://example.com' } + + subject { described_class.send_url(url) } + + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) + + expect(key).to eq("Gitlab-Workhorse-Send-Data") + expect(command).to eq("send-url") + expect(params).to eq({ + 'URL' => url, + 'AllowRedirects' => false + }.deep_stringify_keys) + end + end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 8086b91a48872fb3bad379eee60f31d9a5f8f5ac..c6366ffec62e1f28ec5c99b67d345adf3b53c2c7 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1157,8 +1157,6 @@ describe API::Runner do before do create(:ci_job_artifact, :archive, file_store: store, job: job) - - download_artifact end context 'when using job token' do @@ -1168,6 +1166,10 @@ describe API::Runner do 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } end + before do + download_artifact + end + it 'download artifacts' do expect(response).to have_http_status(200) expect(response.headers).to include download_headers @@ -1178,8 +1180,27 @@ describe API::Runner do let(:store) { JobArtifactUploader::Store::REMOTE } let!(:job) { create(:ci_build) } - it 'download artifacts' do - expect(response).to have_http_status(302) + context 'when proxy download is being used' do + before do + download_artifact(direct_download: false) + end + + it 'uses workhorse send-url' do + expect(response).to have_gitlab_http_status(200) + expect(response.headers).to include( + 'Gitlab-Workhorse-Send-Data' => /send-url:/) + end + end + + context 'when direct download is being used' do + before do + download_artifact(direct_download: true) + end + + it 'receive redirect for downloading artifacts' do + expect(response).to have_gitlab_http_status(302) + expect(response.headers).to include('Location') + end end end end @@ -1187,6 +1208,10 @@ describe API::Runner do context 'when using runnners token' do let(:token) { job.project.runners_token } + before do + download_artifact + end + it 'responds with forbidden' do expect(response).to have_gitlab_http_status(403) end