From 92434d00bf1b0d8aa231d2d8f233fe45899cc5a4 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 30 Mar 2018 18:16:09 +0900 Subject: [PATCH 01/10] Fix database trace to read raw --- app/controllers/projects/jobs_controller.rb | 30 ++++++++------------- lib/gitlab/ci/trace/stream.rb | 2 +- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 85e972d9731..5d6abcf906d 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -119,17 +119,17 @@ class Projects::JobsController < Projects::ApplicationController end def raw - if trace_artifact_file - send_upload(trace_artifact_file, - send_params: raw_send_params, - redirect_params: raw_redirect_params) - else - build.trace.read do |stream| - if stream.file? - send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' - else - render_404 - end + build.trace.read do |stream| + if trace_artifact_file + send_upload(trace_artifact_file, + send_params: { type: 'text/plain; charset=utf-8', disposition: 'inline' }, + redirect_params: { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } } ) + elsif stream.file? + send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' + elsif build.old_trace + send_data stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' + else + render_404 end end end @@ -144,14 +144,6 @@ class Projects::JobsController < Projects::ApplicationController return access_denied! unless can?(current_user, :erase_build, build) end - def raw_send_params - { type: 'text/plain; charset=utf-8', disposition: 'inline' } - end - - def raw_redirect_params - { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } } - end - def trace_artifact_file @trace_artifact_file ||= build.job_artifacts_trace&.file end diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb index b3fe3ef1c4d..ffb9ec61715 100644 --- a/lib/gitlab/ci/trace/stream.rb +++ b/lib/gitlab/ci/trace/stream.rb @@ -22,7 +22,7 @@ module Gitlab end def file? - self.path.present? + self.path.present? if self.respond_to(:path) end def limit(last_bytes = LIMIT_SIZE) From c0997eca4b78c3daae0c158520d30164a146e26a Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 3 Apr 2018 21:30:14 +0900 Subject: [PATCH 02/10] Fix --- app/controllers/projects/jobs_controller.rb | 32 ++++++++++++++------- lib/gitlab/ci/trace/stream.rb | 2 +- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 5d6abcf906d..a604f623e49 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -119,17 +119,19 @@ class Projects::JobsController < Projects::ApplicationController end def raw - build.trace.read do |stream| - if trace_artifact_file - send_upload(trace_artifact_file, - send_params: { type: 'text/plain; charset=utf-8', disposition: 'inline' }, - redirect_params: { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } } ) - elsif stream.file? - send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' - elsif build.old_trace - send_data stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' - else - render_404 + if trace_artifact_file + send_upload(trace_artifact_file, + send_params: raw_send_params, + redirect_params: raw_redirect_params) + else + build.trace.read do |stream| + if stream.file? + send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' + elsif build.old_trace + send_data stream.raw, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' + else + render_404 + end end end end @@ -144,6 +146,14 @@ class Projects::JobsController < Projects::ApplicationController return access_denied! unless can?(current_user, :erase_build, build) end + def raw_send_params + { type: 'text/plain; charset=utf-8', disposition: 'inline' } + end + + def raw_redirect_params + { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } } + end + def trace_artifact_file @trace_artifact_file ||= build.job_artifacts_trace&.file end diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb index ffb9ec61715..ca9f62ad80c 100644 --- a/lib/gitlab/ci/trace/stream.rb +++ b/lib/gitlab/ci/trace/stream.rb @@ -22,7 +22,7 @@ module Gitlab end def file? - self.path.present? if self.respond_to(:path) + self.path.present? if respond_to?(:path) end def limit(last_bytes = LIMIT_SIZE) From 5cdbec20e92c2ff92b2270d0e879573e9bc80629 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 3 Apr 2018 21:52:23 +0900 Subject: [PATCH 03/10] Add test --- lib/gitlab/ci/trace/stream.rb | 8 ++++++-- .../controllers/projects/jobs_controller_spec.rb | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb index ca9f62ad80c..54894a46077 100644 --- a/lib/gitlab/ci/trace/stream.rb +++ b/lib/gitlab/ci/trace/stream.rb @@ -8,7 +8,7 @@ module Gitlab attr_reader :stream - delegate :close, :tell, :seek, :size, :path, :url, :truncate, to: :stream, allow_nil: true + delegate :close, :tell, :seek, :size, :url, :truncate, to: :stream, allow_nil: true delegate :valid?, to: :stream, as: :present?, allow_nil: true @@ -22,7 +22,11 @@ module Gitlab end def file? - self.path.present? if respond_to?(:path) + self.path.present? + end + + def path + self.stream.path if self.stream.respond_to?(:path) end def limit(last_bytes = LIMIT_SIZE) diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 31046c202e6..20a826cc198 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -513,6 +513,22 @@ describe Projects::JobsController do end end + context 'when job has a trace in database' do + let(:job) { create(:ci_build, pipeline: pipeline) } + + before do + job.update_column(:trace, 'Sample trace') + end + + it 'send a trace file' do + response = subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.content_type).to eq 'text/plain; charset=utf-8' + expect(response.body).to eq 'Sample trace' + end + end + context 'when job does not have a trace file' do let(:job) { create(:ci_build, pipeline: pipeline) } From 16b6c46ab1b6f2f001a48cb33aaf3a5cddd1d6eb Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 3 Apr 2018 15:57:36 +0000 Subject: [PATCH 04/10] Update jobs_controller.rb --- app/controllers/projects/jobs_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index a604f623e49..38b97ac1fd3 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -129,8 +129,6 @@ class Projects::JobsController < Projects::ApplicationController send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' elsif build.old_trace send_data stream.raw, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' - else - render_404 end end end From 326a8c64538ade3a6d09893fb5d5d78c2a5c3f77 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 3 Apr 2018 15:59:24 +0000 Subject: [PATCH 05/10] Update jobs_controller.rb --- app/controllers/projects/jobs_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 38b97ac1fd3..ac9eb2047a0 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -127,7 +127,7 @@ class Projects::JobsController < Projects::ApplicationController build.trace.read do |stream| if stream.file? send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' - elsif build.old_trace + else send_data stream.raw, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' end end From 9b7a0533ede133ad66d9ed0fba15d1b086e416dd Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 30 Mar 2018 18:16:09 +0900 Subject: [PATCH 06/10] Fix database trace to read raw --- app/controllers/projects/jobs_controller.rb | 30 ++++++++------------- lib/gitlab/ci/trace/stream.rb | 2 +- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 85e972d9731..5d6abcf906d 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -119,17 +119,17 @@ class Projects::JobsController < Projects::ApplicationController end def raw - if trace_artifact_file - send_upload(trace_artifact_file, - send_params: raw_send_params, - redirect_params: raw_redirect_params) - else - build.trace.read do |stream| - if stream.file? - send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' - else - render_404 - end + build.trace.read do |stream| + if trace_artifact_file + send_upload(trace_artifact_file, + send_params: { type: 'text/plain; charset=utf-8', disposition: 'inline' }, + redirect_params: { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } } ) + elsif stream.file? + send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' + elsif build.old_trace + send_data stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' + else + render_404 end end end @@ -144,14 +144,6 @@ class Projects::JobsController < Projects::ApplicationController return access_denied! unless can?(current_user, :erase_build, build) end - def raw_send_params - { type: 'text/plain; charset=utf-8', disposition: 'inline' } - end - - def raw_redirect_params - { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } } - end - def trace_artifact_file @trace_artifact_file ||= build.job_artifacts_trace&.file end diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb index b3fe3ef1c4d..ffb9ec61715 100644 --- a/lib/gitlab/ci/trace/stream.rb +++ b/lib/gitlab/ci/trace/stream.rb @@ -22,7 +22,7 @@ module Gitlab end def file? - self.path.present? + self.path.present? if self.respond_to(:path) end def limit(last_bytes = LIMIT_SIZE) From d6b18d3946926cff5c6bd47b609a0d2e0c899317 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 3 Apr 2018 21:30:14 +0900 Subject: [PATCH 07/10] Fix --- app/controllers/projects/jobs_controller.rb | 32 ++++++++++++++------- lib/gitlab/ci/trace/stream.rb | 2 +- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 5d6abcf906d..a604f623e49 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -119,17 +119,19 @@ class Projects::JobsController < Projects::ApplicationController end def raw - build.trace.read do |stream| - if trace_artifact_file - send_upload(trace_artifact_file, - send_params: { type: 'text/plain; charset=utf-8', disposition: 'inline' }, - redirect_params: { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } } ) - elsif stream.file? - send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' - elsif build.old_trace - send_data stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' - else - render_404 + if trace_artifact_file + send_upload(trace_artifact_file, + send_params: raw_send_params, + redirect_params: raw_redirect_params) + else + build.trace.read do |stream| + if stream.file? + send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' + elsif build.old_trace + send_data stream.raw, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' + else + render_404 + end end end end @@ -144,6 +146,14 @@ class Projects::JobsController < Projects::ApplicationController return access_denied! unless can?(current_user, :erase_build, build) end + def raw_send_params + { type: 'text/plain; charset=utf-8', disposition: 'inline' } + end + + def raw_redirect_params + { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } } + end + def trace_artifact_file @trace_artifact_file ||= build.job_artifacts_trace&.file end diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb index ffb9ec61715..ca9f62ad80c 100644 --- a/lib/gitlab/ci/trace/stream.rb +++ b/lib/gitlab/ci/trace/stream.rb @@ -22,7 +22,7 @@ module Gitlab end def file? - self.path.present? if self.respond_to(:path) + self.path.present? if respond_to?(:path) end def limit(last_bytes = LIMIT_SIZE) From 9c990bbe7a4f213778b60fcd63f21719c6433d93 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 3 Apr 2018 21:52:23 +0900 Subject: [PATCH 08/10] Add test --- lib/gitlab/ci/trace/stream.rb | 8 ++++++-- .../controllers/projects/jobs_controller_spec.rb | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb index ca9f62ad80c..54894a46077 100644 --- a/lib/gitlab/ci/trace/stream.rb +++ b/lib/gitlab/ci/trace/stream.rb @@ -8,7 +8,7 @@ module Gitlab attr_reader :stream - delegate :close, :tell, :seek, :size, :path, :url, :truncate, to: :stream, allow_nil: true + delegate :close, :tell, :seek, :size, :url, :truncate, to: :stream, allow_nil: true delegate :valid?, to: :stream, as: :present?, allow_nil: true @@ -22,7 +22,11 @@ module Gitlab end def file? - self.path.present? if respond_to?(:path) + self.path.present? + end + + def path + self.stream.path if self.stream.respond_to?(:path) end def limit(last_bytes = LIMIT_SIZE) diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 31046c202e6..20a826cc198 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -513,6 +513,22 @@ describe Projects::JobsController do end end + context 'when job has a trace in database' do + let(:job) { create(:ci_build, pipeline: pipeline) } + + before do + job.update_column(:trace, 'Sample trace') + end + + it 'send a trace file' do + response = subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.content_type).to eq 'text/plain; charset=utf-8' + expect(response.body).to eq 'Sample trace' + end + end + context 'when job does not have a trace file' do let(:job) { create(:ci_build, pipeline: pipeline) } From 2dcbaf9f0eb4c63d7cbe3252b20220e798ee0079 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 5 Apr 2018 20:56:07 +0900 Subject: [PATCH 09/10] Fix failed spec --- spec/controllers/projects/jobs_controller_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 20a826cc198..b9a979044fe 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -535,7 +535,8 @@ describe Projects::JobsController do it 'returns not_found' do response = subject - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq '' end end From 373f6a9bb20497eebe4cdf41be18eb2399424a59 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 6 Apr 2018 01:44:24 +0900 Subject: [PATCH 10/10] Add change log --- app/controllers/projects/jobs_controller.rb | 2 -- .../unreleased/44665-fix-db-trace-stream-by-raw-access.yml | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/44665-fix-db-trace-stream-by-raw-access.yml diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 539bb9a14a7..ac9eb2047a0 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -127,8 +127,6 @@ class Projects::JobsController < Projects::ApplicationController build.trace.read do |stream| if stream.file? send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline' - elsif build.old_trace - send_data stream.raw, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' else send_data stream.raw, type: 'text/plain; charset=utf-8', disposition: 'inline', filename: 'job.log' end diff --git a/changelogs/unreleased/44665-fix-db-trace-stream-by-raw-access.yml b/changelogs/unreleased/44665-fix-db-trace-stream-by-raw-access.yml new file mode 100644 index 00000000000..4166d4fe320 --- /dev/null +++ b/changelogs/unreleased/44665-fix-db-trace-stream-by-raw-access.yml @@ -0,0 +1,5 @@ +--- +title: Fix `JobsController#raw` endpoint can not read traces in database +merge_request: 18101 +author: +type: fixed