Commit df313634 authored by Grzegorz Bizon's avatar Grzegorz Bizon Committed by Grzegorz Bizon

Do not allow to modify build if it has been erased

parent 21152d7d
...@@ -38,6 +38,8 @@ module Ci ...@@ -38,6 +38,8 @@ module Ci
authenticate_runner! authenticate_runner!
update_runner_last_contact update_runner_last_contact
build = Ci::Build.where(runner_id: current_runner.id).running.find(params[:id]) build = Ci::Build.where(runner_id: current_runner.id).running.find(params[:id])
forbidden!('Build has been erased!') if build.erased?
build.update_attributes(trace: params[:trace]) if params[:trace] build.update_attributes(trace: params[:trace]) if params[:trace]
case params[:state].to_s case params[:state].to_s
...@@ -99,6 +101,7 @@ module Ci ...@@ -99,6 +101,7 @@ module Ci
not_found! unless build not_found! unless build
authenticate_build_token!(build) authenticate_build_token!(build)
forbidden!('Build is not running!') unless build.running? forbidden!('Build is not running!') unless build.running?
forbidden!('Build has been erased!') if build.erased?
artifacts_upload_path = ArtifactUploader.artifacts_upload_path artifacts_upload_path = ArtifactUploader.artifacts_upload_path
artifacts = uploaded_file(:file, artifacts_upload_path) artifacts = uploaded_file(:file, artifacts_upload_path)
......
...@@ -131,39 +131,41 @@ describe Ci::API::API do ...@@ -131,39 +131,41 @@ describe Ci::API::API do
end end
describe "PUT /builds/:id" do describe "PUT /builds/:id" do
let(:commit) { FactoryGirl.create(:ci_commit, project: project)} let(:commit) {create(:ci_commit, project: project)}
let(:build) { FactoryGirl.create(:ci_build, commit: commit, runner_id: runner.id) } let(:build) { create(:ci_build_with_trace, commit: commit, runner_id: runner.id) }
it "should update a running build" do before do
build.run! build.run!
put ci_api("/builds/#{build.id}"), token: runner.token put ci_api("/builds/#{build.id}"), token: runner.token
end
it "should update a running build" do
expect(response.status).to eq(200) expect(response.status).to eq(200)
end end
it 'Should not override trace information when no trace is given' do it 'should not override trace information when no trace is given' do
build.run! expect(build.reload.trace).to eq 'BUILD TRACE'
build.update!(trace: 'hello_world') end
put ci_api("/builds/#{build.id}"), token: runner.token
expect(build.reload.trace).to eq 'hello_world' context 'build has been erased' do
let(:build) { create(:ci_build, runner_id: runner.id, erased_at: Time.now) }
it 'should respond with forbidden' do
expect(response.status).to eq 403
end
end end
end end
describe 'DELETE /builds/:id/content' do describe 'DELETE /builds/:id/content' do
let(:build) { create(:ci_build_with_trace, :artifacts, :success) }
before { delete ci_api("/builds/#{build.id}/content"), token: build.token } before { delete ci_api("/builds/#{build.id}/content"), token: build.token }
let!(:build) { create(:ci_build_with_trace, :artifacts, :success) }
it 'should respond with valid status' do it 'should erase build content' do
expect(response.status).to eq 200 expect(response.status).to eq 200
end expect(build.trace).to be_empty
it 'should remove build artifacts' do
expect(build.artifacts_file.exists?).to be_falsy expect(build.artifacts_file.exists?).to be_falsy
expect(build.artifacts_metadata.exists?).to be_falsy expect(build.artifacts_metadata.exists?).to be_falsy
end end
it 'should remove build trace' do
expect(build.trace).to be_empty
end
end end
context "Artifacts" do context "Artifacts" do
...@@ -209,9 +211,10 @@ describe Ci::API::API do ...@@ -209,9 +211,10 @@ describe Ci::API::API do
end end
end end
context 'token is invalid' do context 'authorization token is invalid' do
it 'should respond with forbidden'do before { post authorize_url, { token: 'invalid', filesize: 100 } }
post authorize_url, { token: 'invalid', filesize: 100 }
it 'should respond with forbidden' do
expect(response.status).to eq(403) expect(response.status).to eq(403)
end end
end end
...@@ -224,6 +227,15 @@ describe Ci::API::API do ...@@ -224,6 +227,15 @@ describe Ci::API::API do
allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return('/') allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return('/')
end end
context 'build has been erased' do
let(:build) { create(:ci_build, erased_at: Time.now) }
before { upload_artifacts(file_upload, headers_with_token) }
it 'should respond with forbidden' do
expect(response.status).to eq 403
end
end
context "should post artifact to running build" do context "should post artifact to running build" do
it "uses regual file post" do it "uses regual file post" do
upload_artifacts(file_upload, headers_with_token, false) upload_artifacts(file_upload, headers_with_token, false)
...@@ -252,7 +264,9 @@ describe Ci::API::API do ...@@ -252,7 +264,9 @@ describe Ci::API::API do
let(:stored_artifacts_file) { build.reload.artifacts_file.file } let(:stored_artifacts_file) { build.reload.artifacts_file.file }
let(:stored_metadata_file) { build.reload.artifacts_metadata.file } let(:stored_metadata_file) { build.reload.artifacts_metadata.file }
before { post(post_url, post_data, headers_with_token) } before do
post(post_url, post_data, headers_with_token)
end
context 'post data accelerated by workhorse is correct' do context 'post data accelerated by workhorse is correct' do
let(:post_data) do let(:post_data) do
......
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