Commit 62067b74 authored by Grzegorz Bizon's avatar Grzegorz Bizon Committed by Rémy Coutable

Merge branch 'ci/remove-builds' into 'master'

Make it possible to erase build content (artifacts, trace)

This feature makes it possible to remove build content - build artifacts and build traces.

- [x] Remove artifacts
- [x] Remove metadata
- [x] Remove build traces
- [x] Wait for https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/1942 this to be merged
- [x] Fix the permissions after the merge

Closes #3421

See merge request !2560
parent 64f1bab6
......@@ -69,6 +69,7 @@ v 8.5.0 (unreleased)
- Allow SAML users to login with no previous account without having to allow
all Omniauth providers to do so.
- Allow existing users to auto link their SAML credentials by logging in via SAML
- Make it possible to erase a build (trace, artifacts) using UI and API
v 8.4.4
- Update omniauth-saml gem to 1.4.2
......
......@@ -56,6 +56,12 @@ class Projects::BuildsController < Projects::ApplicationController
render json: @build.to_json(only: [:status, :id, :sha, :coverage], methods: :sha)
end
def erase
@build.erase(erased_by: current_user)
redirect_to namespace_project_build_path(project.namespace, project, @build),
notice: "Build has been sucessfully erased!"
end
private
def build
......
......@@ -31,15 +31,19 @@
# artifacts_file :text
# gl_project_id :integer
# artifacts_metadata :text
# erased_by_id :integer
# erased_at :datetime
#
module Ci
class Build < CommitStatus
include Gitlab::Application.routes.url_helpers
LAZY_ATTRIBUTES = ['trace']
belongs_to :runner, class_name: 'Ci::Runner'
belongs_to :trigger_request, class_name: 'Ci::TriggerRequest'
belongs_to :erased_by, class_name: 'User'
serialize :options
......@@ -203,6 +207,10 @@ module Ci
end
end
def has_trace?
raw_trace.present?
end
def raw_trace
if File.file?(path_to_trace)
File.read(path_to_trace)
......@@ -359,6 +367,33 @@ module Ci
Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path, **options).to_entry
end
def erase(opts = {})
return false unless erasable?
remove_artifacts_file!
remove_artifacts_metadata!
erase_trace!
update_erased!(opts[:erased_by])
end
def erasable?
complete? && (artifacts? || has_trace?)
end
def erased?
!self.erased_at.nil?
end
private
def erase_trace!
self.trace = nil
end
def update_erased!(user = nil)
self.update(erased_by: user, erased_at: Time.now)
end
private
def yaml_variables
......
......@@ -76,10 +76,16 @@
= link_to '#down-build-trace', class: 'btn' do
%i.fa.fa-angle-down
- if @build.erased?
.erased.alert.alert-warning
- erased_by = "by #{link_to @build.erased_by.name, user_path(@build.erased_by)}" if @build.erased_by
Build has been erased #{erased_by.html_safe} #{time_ago_with_tooltip(@build.erased_at)}
- else
%pre.trace#build-trace
%code.bash
= preserve do
= raw @build.trace_html
%div#down-build-trace
.col-md-3
......@@ -94,20 +100,34 @@
%h4.title Build artifacts
.center
.btn-group{ role: :group }
= link_to "Download", @build.artifacts_download_url, class: 'btn btn-sm btn-primary'
= link_to @build.artifacts_download_url, class: 'btn btn-sm btn-primary' do
= icon('download')
Download
- if @build.artifacts_metadata?
= link_to "Browse", @build.artifacts_browse_url, class: 'btn btn-sm btn-primary'
= link_to @build.artifacts_browse_url, class: 'btn btn-sm btn-primary' do
= icon('folder-open')
Browse
.build-widget
%h4.title
Build ##{@build.id}
- if can?(current_user, :update_build, @project)
.pull-right
.center
.btn-group{ role: :group }
- if @build.cancel_url
= link_to "Cancel", @build.cancel_url, class: 'btn btn-sm btn-danger', method: :post
- elsif @build.retry_url
= link_to "Retry", @build.retry_url, class: 'btn btn-sm btn-primary', method: :post
- if @build.erasable?
= link_to erase_namespace_project_build_path(@project.namespace, @project, @build),
class: 'btn btn-sm btn-warning', method: :post,
data: { confirm: 'Are you sure you want to erase this build?' } do
= icon('eraser')
Erase
.clearfix
- if @build.duration
%p
%span.attr-name Duration:
......@@ -119,6 +139,10 @@
%p
%span.attr-name Finished:
#{time_ago_with_tooltip(@build.finished_at)}
- if @build.erased_at
%p
%span.attr-name Erased:
#{time_ago_with_tooltip(@build.erased_at)}
%p
%span.attr-name Runner:
- if @build.runner && current_user && current_user.admin
......
......@@ -617,6 +617,7 @@ Rails.application.routes.draw do
get :status
post :cancel
post :retry
post :erase
end
resource :artifacts, only: [] do
......
class AddErasableToCiBuild < ActiveRecord::Migration
def change
add_reference :ci_builds, :erased_by, references: :users, index: true
add_column :ci_builds, :erased_at, :datetime
end
end
......@@ -129,6 +129,8 @@ ActiveRecord::Schema.define(version: 20160209130428) do
t.text "artifacts_file"
t.integer "gl_project_id"
t.text "artifacts_metadata"
t.integer "erased_by_id"
t.datetime "erased_at"
end
add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree
......@@ -136,6 +138,7 @@ ActiveRecord::Schema.define(version: 20160209130428) do
add_index "ci_builds", ["commit_id", "type", "name", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_name_and_ref", using: :btree
add_index "ci_builds", ["commit_id", "type", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_ref", using: :btree
add_index "ci_builds", ["commit_id"], name: "index_ci_builds_on_commit_id", using: :btree
add_index "ci_builds", ["erased_by_id"], name: "index_ci_builds_on_erased_by_id", using: :btree
add_index "ci_builds", ["gl_project_id"], name: "index_ci_builds_on_gl_project_id", using: :btree
add_index "ci_builds", ["project_id", "commit_id"], name: "index_ci_builds_on_project_id_and_commit_id", using: :btree
add_index "ci_builds", ["project_id"], name: "index_ci_builds_on_project_id", using: :btree
......
......@@ -338,3 +338,53 @@ Example of response
"user": null
}
```
## Erase a build
Erase a single build of a project (remove build artifacts and a build trace)
```
POST /projects/:id/builds/:build_id/erase
```
Parameters
| Attribute | Type | required | Description |
|-------------|---------|----------|---------------------|
| `id` | integer | yes | The ID of a project |
| `build_id` | integer | yes | The ID of a build |
Example of request
```
curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/erase"
```
Example of response
```json
{
"commit": {
"author_email": "admin@example.com",
"author_name": "Administrator",
"created_at": "2015-12-24T16:51:14.000+01:00",
"id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd",
"message": "Test the CI integration.",
"short_id": "0ff3ae19",
"title": "Test the CI integration."
},
"coverage": null,
"download_url": null,
"id": 69,
"name": "rubocop",
"ref": "master",
"runner": null,
"stage": "test",
"created_at": "2016-01-11T10:13:33.506Z",
"started_at": "2016-01-11T10:13:33.506Z",
"finished_at": "2016-01-11T10:15:10.506Z",
"status": "failed",
"tag": false,
"user": null
}
```
......@@ -13,3 +13,12 @@ Feature: Project Builds Summary
Scenario: I browse project builds page
When I visit project builds page
Then I see button to CI Lint
Scenario: I erase a build
Given recent build is successful
And recent build has a build trace
When I visit recent build details page
And I click erase build button
Then recent build has been erased
And recent build summary does not have artifacts widget
And recent build summary contains information saying that build has been erased
......@@ -10,4 +10,24 @@ class Spinach::Features::ProjectBuildsSummary < Spinach::FeatureSteps
expect(ci_lint_tool_link[:href]).to eq ci_lint_path
end
end
step 'I click erase build button' do
click_link 'Erase'
end
step 'recent build has been erased' do
expect(@build.artifacts_file.exists?).to be_falsy
expect(@build.artifacts_metadata.exists?).to be_falsy
expect(@build.trace).to be_empty
end
step 'recent build summary does not have artifacts widget' do
expect(page).to have_no_css('.artifacts')
end
step 'recent build summary contains information saying that build has been erased' do
page.within('.erased') do
expect(page).to have_content 'Build has been erased'
end
end
end
......@@ -42,6 +42,10 @@ module SharedBuilds
@build.update_attributes(artifacts_metadata: gzip)
end
step 'recent build has a build trace' do
@build.trace = 'build trace'
end
step 'download of build artifacts archive starts' do
expect(page.response_headers['Content-Type']).to eq 'application/zip'
expect(page.response_headers['Content-Transfer-Encoding']).to eq 'binary'
......
......@@ -115,13 +115,33 @@ module API
authorize_update_builds!
build = get_build(params[:build_id])
return forbidden!('Build is not retryable') unless build && build.retryable?
return not_found!(build) unless build
return forbidden!('Build is not retryable') unless build.retryable?
build = Ci::Build.retry(build)
present build, with: Entities::Build,
user_can_download_artifacts: can?(current_user, :read_build, user_project)
end
# Erase build (remove artifacts and build trace)
#
# Parameters:
# id (required) - the id of a project
# build_id (required) - the id of a build
# example Request:
# post /projects/:id/build/:build_id/erase
post ':id/builds/:build_id/erase' do
authorize_update_builds!
build = get_build(params[:build_id])
return not_found!(build) unless build
return forbidden!('Build is not erasable!') unless build.erasable?
build.erase(erased_by: current_user)
present build, with: Entities::Build,
user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
end
end
helpers do
......
......@@ -38,6 +38,8 @@ module Ci
authenticate_runner!
update_runner_last_contact
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]
case params[:state].to_s
......@@ -99,6 +101,7 @@ module Ci
not_found! unless build
authenticate_build_token!(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)
......@@ -143,7 +146,7 @@ module Ci
present_file!(artifacts_file.path, artifacts_file.filename)
end
# Remove the artifacts file from build
# Remove the artifacts file from build - Runners only
#
# Parameters:
# id (required) - The ID of a build
......@@ -156,6 +159,7 @@ module Ci
build = Ci::Build.find_by_id(params[:id])
not_found! unless build
authenticate_build_token!(build)
build.remove_artifacts_file!
build.remove_artifacts_metadata!
end
......
......@@ -62,14 +62,13 @@ FactoryGirl.define do
trait :artifacts do
after(:create) do |build, _|
build.artifacts_file =
fixture_file_upload(Rails.root +
'spec/fixtures/ci_build_artifacts.zip',
fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'),
'application/zip')
build.artifacts_metadata =
fixture_file_upload(Rails.root +
'spec/fixtures/ci_build_artifacts_metadata.gz',
fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'),
'application/x-gzip')
build.save!
end
end
......
......@@ -346,15 +346,14 @@ describe Ci::Build, models: true do
describe :artifacts_download_url do
subject { build.artifacts_download_url }
it "should be nil if artifact doesn't exist" do
build.update_attributes(artifacts_file: nil)
is_expected.to be_nil
context 'artifacts file does not exist' do
before { build.update_attributes(artifacts_file: nil) }
it { is_expected.to be_nil }
end
it 'should not be nil if artifact exist' do
gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
build.update_attributes(artifacts_file: gif)
is_expected.to_not be_nil
context 'artifacts file exists' do
let(:build) { create(:ci_build, :artifacts) }
it { is_expected.to_not be_nil }
end
end
......@@ -381,11 +380,7 @@ describe Ci::Build, models: true do
end
context 'artifacts archive exists' do
before do
gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
build.update_attributes(artifacts_file: gif)
end
let(:build) { create(:ci_build, :artifacts) }
it { is_expected.to be_truthy }
end
end
......@@ -398,16 +393,7 @@ describe Ci::Build, models: true do
end
context 'artifacts archive is a zip file and metadata exists' do
before do
fixture_dir = Rails.root + 'spec/fixtures/'
archive = fixture_file_upload(fixture_dir + 'ci_build_artifacts.zip',
'application/zip')
metadata = fixture_file_upload(fixture_dir + 'ci_build_artifacts_metadata.gz',
'application/x-gzip')
build.update_attributes(artifacts_file: archive)
build.update_attributes(artifacts_metadata: metadata)
end
let(:build) { create(:ci_build, :artifacts) }
it { is_expected.to be_truthy }
end
end
......@@ -511,6 +497,103 @@ describe Ci::Build, models: true do
expect(@build2.merge_request.id).to eq(@merge_request.id)
end
end
end
describe 'build erasable' do
shared_examples 'erasable' do
it 'should remove artifact file' do
expect(build.artifacts_file.exists?).to be_falsy
end
it 'should remove artifact metadata file' do
expect(build.artifacts_metadata.exists?).to be_falsy
end
it 'should erase build trace in trace file' do
expect(build.trace).to be_empty
end
it 'should set erased to true' do
expect(build.erased?).to be true
end
it 'should set erase date' do
expect(build.erased_at).to_not be_falsy
end
end
context 'build is not erasable' do
let!(:build) { create(:ci_build) }
describe '#erase' do
subject { build.erase }
it { is_expected.to be false }
end
describe '#erasable?' do
subject { build.erasable? }
it { is_expected.to eq false }
end
end
context 'build is erasable' do
let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
describe '#erase' do
before { build.erase(erased_by: user) }
context 'erased by user' do
let!(:user) { create(:user, username: 'eraser') }
include_examples 'erasable'
it 'should record user who erased a build' do
expect(build.erased_by).to eq user
end
end
context 'erased by system' do
let(:user) { nil }
include_examples 'erasable'
it 'should not set user who erased a build' do
expect(build.erased_by).to be_nil
end
end
end
describe '#erasable?' do
subject { build.erasable? }
it { is_expected.to eq true }
end
describe '#erased?' do
let!(:build) { create(:ci_build_with_trace, :success, :artifacts) }
subject { build.erased? }
context 'build has not been erased' do
it { is_expected.to be false }
end
context 'build has been erased' do
before { build.erase }
it { is_expected.to be true }
end
end
context 'metadata and build trace are not available' do
let!(:build) { create(:ci_build, :success, :artifacts) }
before { build.remove_artifacts_metadata! }
describe '#erase' do
it 'should not raise error' do
expect { build.erase }.to_not raise_error
end
end
end
end
end
end
......@@ -169,4 +169,34 @@ describe API::API, api: true do
end
end
end
describe 'POST /projects/:id/builds/:build_id/erase' do
before do
post api("/projects/#{project.id}/builds/#{build.id}/erase", user)
end
context 'build is erasable' do
let(:build) { create(:ci_build_with_trace, :artifacts, :success, project: project, commit: commit) }
it 'should erase build content' do
expect(response.status).to eq 201
expect(build.trace).to be_empty
expect(build.artifacts_file.exists?).to be_falsy
expect(build.artifacts_metadata.exists?).to be_falsy
end
it 'should update build' do
expect(build.reload.erased_at).to be_truthy
expect(build.reload.erased_by).to eq user
end
end
context 'build is not erasable' do
let(:build) { create(:ci_build_with_trace, project: project, commit: commit) }
it 'should respond with forbidden' do
expect(response.status).to eq 403
end
end
end
end
......@@ -131,20 +131,28 @@ describe Ci::API::API do
end
describe "PUT /builds/:id" do
let(:commit) { FactoryGirl.create(:ci_commit, project: project)}
let(:build) { FactoryGirl.create(:ci_build, commit: commit, runner_id: runner.id) }
let(:commit) {create(:ci_commit, project: project)}
let(:build) { create(:ci_build_with_trace, commit: commit, runner_id: runner.id) }
it "should update a running build" do
before do
build.run!
put ci_api("/builds/#{build.id}"), token: runner.token
end
it "should update a running build" do
expect(response.status).to eq(200)
end
it 'Should not override trace information when no trace is given' do
build.run!
build.update!(trace: 'hello_world')
put ci_api("/builds/#{build.id}"), token: runner.token
expect(build.reload.trace).to eq 'hello_world'
it 'should not override trace information when no trace is given' do
expect(build.reload.trace).to eq 'BUILD TRACE'
end
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
......@@ -191,9 +199,10 @@ describe Ci::API::API do
end
end
context 'token is invalid' do
it 'should respond with forbidden'do
post authorize_url, { token: 'invalid', filesize: 100 }
context 'authorization token is invalid' do
before { post authorize_url, { token: 'invalid', filesize: 100 } }
it 'should respond with forbidden' do
expect(response.status).to eq(403)
end
end
......@@ -206,6 +215,15 @@ describe Ci::API::API do
allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return('/')
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
it "uses regual file post" do
upload_artifacts(file_upload, headers_with_token, false)
......@@ -234,7 +252,9 @@ describe Ci::API::API do
let(:stored_artifacts_file) { build.reload.artifacts_file.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
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