diff options
-rw-r--r-- | lib/ci/api/builds.rb | 19 | ||||
-rw-r--r-- | lib/ci/api/helpers.rb | 15 | ||||
-rw-r--r-- | spec/requests/ci/api/builds_spec.rb | 20 |
3 files changed, 37 insertions, 17 deletions
diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index ed87a2603e8..142bce82286 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -41,7 +41,7 @@ module Ci put ":id" do authenticate_runner! build = Ci::Build.where(runner_id: current_runner.id).running.find(params[:id]) - forbidden!('Build has been erased!') if build.erased? + validate_build!(build) update_runner_info @@ -71,9 +71,7 @@ module Ci # PATCH /builds/:id/trace.txt patch ":id/trace.txt" do build = Ci::Build.find_by_id(params[:id]) - not_found! unless build - authenticate_build_token!(build) - forbidden!('Build has been erased!') if build.erased? + authenticate_build!(build) error!('400 Missing header Content-Range', 400) unless request.headers.has_key?('Content-Range') content_range = request.headers['Content-Range'] @@ -104,8 +102,7 @@ module Ci Gitlab::Workhorse.verify_api_request!(headers) not_allowed! unless Gitlab.config.artifacts.enabled build = Ci::Build.find_by_id(params[:id]) - not_found! unless build - authenticate_build_token!(build) + authenticate_build!(build) forbidden!('build is not running') unless build.running? if params[:filesize] @@ -142,10 +139,8 @@ module Ci require_gitlab_workhorse! not_allowed! unless Gitlab.config.artifacts.enabled build = Ci::Build.find_by_id(params[:id]) - not_found! unless build - authenticate_build_token!(build) + authenticate_build!(build) forbidden!('Build is not running!') unless build.running? - forbidden!('Build has been erased!') if build.erased? artifacts_upload_path = ArtifactUploader.artifacts_upload_path artifacts = uploaded_file(:file, artifacts_upload_path) @@ -176,8 +171,7 @@ module Ci # GET /builds/:id/artifacts get ":id/artifacts" do build = Ci::Build.find_by_id(params[:id]) - not_found! unless build - authenticate_build_token!(build) + authenticate_build!(build) artifacts_file = build.artifacts_file unless artifacts_file.file_storage? @@ -202,8 +196,7 @@ module Ci # DELETE /builds/:id/artifacts delete ":id/artifacts" do build = Ci::Build.find_by_id(params[:id]) - not_found! unless build - authenticate_build_token!(build) + authenticate_build!(build) build.erase_artifacts! end diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index e608f5f6cad..31fbd1da108 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -13,8 +13,19 @@ module Ci forbidden! unless current_runner end - def authenticate_build_token!(build) - forbidden! unless build_token_valid?(build) + def authenticate_build!(build) + validate_build!(build) do + forbidden! unless build_token_valid?(build) + end + end + + def validate_build!(build) + not_found! unless build + + yield if block_given? + + forbidden!('Project has been deleted!') unless build.project + forbidden!('Build has been erased!') if build.erased? end def runner_registration_token_valid? diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 80652129928..79f12ace999 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -249,7 +249,11 @@ describe Ci::API::Builds do end describe 'PATCH /builds/:id/trace.txt' do - let(:build) { create(:ci_build, :pending, :trace, runner_id: runner.id) } + let(:build) do + attributes = { runner_id: runner.id, pipeline: pipeline } + create(:ci_build, :running, :trace, attributes) + end + let(:headers) { { Ci::API::Helpers::BUILD_TOKEN_HEADER => build.token, 'Content-Type' => 'text/plain' } } let(:headers_with_range) { headers.merge({ 'Content-Range' => '11-20' }) } let(:update_interval) { 10.seconds.to_i } @@ -276,7 +280,6 @@ describe Ci::API::Builds do end before do - build.run! initial_patch_the_trace end @@ -329,6 +332,19 @@ describe Ci::API::Builds do end end end + + context 'when project for the build has been deleted' do + let(:build) do + attributes = { runner_id: runner.id, pipeline: pipeline } + create(:ci_build, :running, :trace, attributes) do |build| + build.project.update(pending_delete: true) + end + end + + it 'responds with forbidden' do + expect(response.status).to eq(403) + end + end end context 'when Runner makes a force-patch' do |