diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 1c082945299..221826121da 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -144,6 +144,7 @@ class ProjectPolicy < BasePolicy enable :destroy_merge_request enable :destroy_issue enable :remove_pages + enable :destroy_pipeline enable :set_issue_iid enable :set_issue_created_at diff --git a/app/services/ci/destroy_pipeline_service.rb b/app/services/ci/destroy_pipeline_service.rb new file mode 100644 index 00000000000..059e871f20e --- /dev/null +++ b/app/services/ci/destroy_pipeline_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Ci + class DestroyPipelineService < BaseService + def execute(pipeline) + return false unless can?(current_user, :destroy_pipeline, project) + + AuditEventService.new(current_user, pipeline).security_event + + pipeline.destroy + end + end +end diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index d4bd4e71343..39d693bb9e9 100644 --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -89,11 +89,11 @@ module API requires :pipeline_id, type: Integer, desc: 'The pipeline ID' end delete ':id/pipelines/:pipeline_id' do - authorize! :admin_pipeline, user_project + authorize! :destroy_pipeline, user_project - AuditEventService.new(current_user, user_project).security_event - - destroy_conditionally!(pipeline) + destroy_conditionally!(pipeline) do + ::Ci::DestroyPipelineService.new(user_project, current_user).execute(pipeline) + end end desc 'Retry builds in the pipeline' do diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index 68de3068568..e786b7531a9 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -440,34 +440,31 @@ describe API::Pipelines do describe 'DELETE /projects/:id/pipelines/:pipeline_id' do context 'authorized user' do - it 'deletes the pipeline' do - delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", user) + let(:owner) { project.owner } + + it 'destroys the pipeline' do + delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) expect(response).to have_gitlab_http_status(204) expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) end it 'returns 404 when it does not exist' do - delete api("/projects/#{project.id}/pipelines/123456", user) + delete api("/projects/#{project.id}/pipelines/123456", owner) expect(response).to have_gitlab_http_status(404) expect(json_response['message']).to eq '404 Not found' end it 'logs an audit event' do - expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", user) }.to change { SecurityEvent.count }.by(1) + expect { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) }.to change { SecurityEvent.count }.by(1) end context 'when the pipeline has jobs' do - let!(:pipeline) do - create(:ci_pipeline, project: project, sha: project.commit.id, - ref: project.default_branch, user: user) - end - let!(:build) { create(:ci_build, project: project, pipeline: pipeline) } - it 'deletes associated jobs' do - delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", user) + it 'destroys associated jobs' do + delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) expect(response).to have_gitlab_http_status(204) expect { build.reload }.to raise_error(ActiveRecord::RecordNotFound) @@ -476,7 +473,7 @@ describe API::Pipelines do end context 'unauthorized user' do - it 'should not return a project pipeline' do + it 'should return a 404' do get api("/projects/#{project.id}/pipelines/#{pipeline.id}", non_member) expect(response).to have_gitlab_http_status(404) diff --git a/spec/services/ci/destroy_pipeline_service_spec.rb b/spec/services/ci/destroy_pipeline_service_spec.rb new file mode 100644 index 00000000000..9f449dd73e8 --- /dev/null +++ b/spec/services/ci/destroy_pipeline_service_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ::Ci::DestroyPipelineService do + let(:project) { create(:project) } + let!(:pipeline) { create(:ci_pipeline, project: project) } + + subject { described_class.new(project, user).execute(pipeline) } + + context 'user is owner' do + let(:user) { project.owner } + + it 'destroys the pipeline' do + subject + + expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'logs an audit event' do + expect { subject }.to change { SecurityEvent.count }.by(1) + end + + context 'when the pipeline has jobs' do + let!(:build) { create(:ci_build, project: project, pipeline: pipeline) } + + it 'destroys associated jobs' do + subject + + expect { build.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'destroys associated stages' do + stages = pipeline.stages + + subject + + expect(stages).to all(raise_error(ActiveRecord::RecordNotFound)) + end + + context 'when job has artifacts' do + let!(:artifact) { create(:ci_job_artifact, :archive, job: build) } + + it 'destroys associated artifacts' do + subject + + expect { artifact.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end + end + + context 'user is not owner' do + let(:user) { create(:user) } + + it 'returns false' do + is_expected.to eq(false) + end + + it 'does not destroy the pipeline' do + subject + + expect { pipeline.reload }.not_to raise_error + end + end +end