Commit 2e01bd49 authored by Philip Cunningham's avatar Philip Cunningham Committed by Robert Speicher

Include validation errors when DAST service fails

Include validation errors if there's a model validation error and return
them via a method called full_messages on a custom exception.
parent 405fd65a
...@@ -36,7 +36,7 @@ module Mutations ...@@ -36,7 +36,7 @@ module Mutations
service = Ci::RunDastScanService.new(project: project, user: current_user) service = Ci::RunDastScanService.new(project: project, user: current_user)
pipeline = service.execute(branch: branch, target_url: target_url) pipeline = service.execute(branch: branch, target_url: target_url)
success_response(project: project, pipeline: pipeline) success_response(project: project, pipeline: pipeline)
rescue *Ci::RunDastScanService::EXCEPTIONS => err rescue Ci::RunDastScanService::RunError => err
error_response(err) error_response(err)
end end
...@@ -58,7 +58,7 @@ module Mutations ...@@ -58,7 +58,7 @@ module Mutations
end end
def error_response(err) def error_response(err)
{ errors: [err.message] } { errors: err.full_messages }
end end
end end
end end
......
...@@ -4,13 +4,14 @@ module Ci ...@@ -4,13 +4,14 @@ module Ci
class RunDastScanService class RunDastScanService
DEFAULT_SHA_FOR_PROJECTS_WITHOUT_COMMITS = :placeholder DEFAULT_SHA_FOR_PROJECTS_WITHOUT_COMMITS = :placeholder
EXCEPTIONS = [ class RunError < StandardError
NotAllowed = Class.new(StandardError), attr_reader :full_messages
CreatePipelineError = Class.new(StandardError),
CreateStageError = Class.new(StandardError), def initialize(msg, full_messages = [])
CreateBuildError = Class.new(StandardError), @full_messages = full_messages.unshift(msg)
EnqueueError = Class.new(StandardError) super(msg)
].freeze end
end
def initialize(project:, user:) def initialize(project:, user:)
@project = project @project = project
...@@ -18,7 +19,10 @@ module Ci ...@@ -18,7 +19,10 @@ module Ci
end end
def execute(branch:, target_url:) def execute(branch:, target_url:)
raise NotAllowed unless allowed? unless allowed?
msg = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR
raise RunError.new(msg)
end
ActiveRecord::Base.transaction do ActiveRecord::Base.transaction do
pipeline = create_pipeline!(branch) pipeline = create_pipeline!(branch)
...@@ -38,7 +42,7 @@ module Ci ...@@ -38,7 +42,7 @@ module Ci
end end
def create_pipeline!(branch) def create_pipeline!(branch)
reraise!(with: CreatePipelineError.new('Could not create pipeline')) do reraise!(msg: 'Pipeline could not be created') do
Ci::Pipeline.create!( Ci::Pipeline.create!(
project: project, project: project,
ref: branch, ref: branch,
...@@ -50,7 +54,7 @@ module Ci ...@@ -50,7 +54,7 @@ module Ci
end end
def create_stage!(pipeline) def create_stage!(pipeline)
reraise!(with: CreateStageError.new('Could not create stage')) do reraise!(msg: 'Stage could not be created') do
Ci::Stage.create!( Ci::Stage.create!(
name: 'dast', name: 'dast',
pipeline: pipeline, pipeline: pipeline,
...@@ -60,7 +64,7 @@ module Ci ...@@ -60,7 +64,7 @@ module Ci
end end
def create_build!(pipeline, stage, branch, target_url) def create_build!(pipeline, stage, branch, target_url)
reraise!(with: CreateBuildError.new('Could not create build')) do reraise!(msg: 'Build could not be created') do
Ci::Build.create!( Ci::Build.create!(
name: 'DAST Scan', name: 'DAST Scan',
pipeline: pipeline, pipeline: pipeline,
...@@ -75,16 +79,19 @@ module Ci ...@@ -75,16 +79,19 @@ module Ci
end end
def enqueue!(build) def enqueue!(build)
reraise!(with: EnqueueError.new('Could not enqueue build')) do reraise!(msg: 'Build could not be enqueued') do
build.enqueue! build.enqueue!
end end
end end
def reraise!(with:) def reraise!(msg:)
yield yield
rescue ActiveRecord::RecordInvalid => err
Gitlab::ErrorTracking.track_exception(err)
raise RunError.new(msg, err.record.errors.full_messages)
rescue => err rescue => err
Gitlab::ErrorTracking.track_exception(err) Gitlab::ErrorTracking.track_exception(err)
raise with raise RunError.new(msg)
end end
def options def options
......
...@@ -63,7 +63,7 @@ describe 'Running a DAST Scan' do ...@@ -63,7 +63,7 @@ describe 'Running a DAST Scan' do
allow(Ci::Pipeline).to receive(:create!).and_raise(StandardError) allow(Ci::Pipeline).to receive(:create!).and_raise(StandardError)
end end
it_behaves_like 'a mutation that returns errors in the response', errors: ['Could not create pipeline'] it_behaves_like 'a mutation that returns errors in the response', errors: ['Pipeline could not be created']
end end
context 'when the stage could not be created' do context 'when the stage could not be created' do
...@@ -71,7 +71,7 @@ describe 'Running a DAST Scan' do ...@@ -71,7 +71,7 @@ describe 'Running a DAST Scan' do
allow(Ci::Stage).to receive(:create!).and_raise(StandardError) allow(Ci::Stage).to receive(:create!).and_raise(StandardError)
end end
it_behaves_like 'a mutation that returns errors in the response', errors: ['Could not create stage'] it_behaves_like 'a mutation that returns errors in the response', errors: ['Stage could not be created']
end end
context 'when the build could not be created' do context 'when the build could not be created' do
...@@ -79,7 +79,7 @@ describe 'Running a DAST Scan' do ...@@ -79,7 +79,7 @@ describe 'Running a DAST Scan' do
allow(Ci::Build).to receive(:create!).and_raise(StandardError) allow(Ci::Build).to receive(:create!).and_raise(StandardError)
end end
it_behaves_like 'a mutation that returns errors in the response', errors: ['Could not create build'] it_behaves_like 'a mutation that returns errors in the response', errors: ['Build could not be created']
end end
context 'when the build could not be enqueued' do context 'when the build could not be enqueued' do
...@@ -87,7 +87,7 @@ describe 'Running a DAST Scan' do ...@@ -87,7 +87,7 @@ describe 'Running a DAST Scan' do
allow_any_instance_of(Ci::Build).to receive(:enqueue!).and_raise(StandardError) allow_any_instance_of(Ci::Build).to receive(:enqueue!).and_raise(StandardError)
end end
it_behaves_like 'a mutation that returns errors in the response', errors: ['Could not enqueue build'] it_behaves_like 'a mutation that returns errors in the response', errors: ['Build could not be enqueued']
end end
end end
end end
......
...@@ -12,8 +12,10 @@ describe Ci::RunDastScanService do ...@@ -12,8 +12,10 @@ describe Ci::RunDastScanService do
subject { described_class.new(project: project, user: user).execute(branch: branch, target_url: target_url) } subject { described_class.new(project: project, user: user).execute(branch: branch, target_url: target_url) }
context 'when the user does not have permission to run a dast scan' do context 'when the user does not have permission to run a dast scan' do
it 'raises an exception' do it 'raises an exeception with #full_messages populated' do
expect { subject }.to raise_error(described_class::NotAllowed) expect { subject }.to raise_error(Ci::RunDastScanService::RunError) do |error|
expect(error.full_messages[0]).to include('you don\'t have permission to perform this action')
end
end end
end end
...@@ -110,8 +112,10 @@ describe Ci::RunDastScanService do ...@@ -110,8 +112,10 @@ describe Ci::RunDastScanService do
allow(Ci::Pipeline).to receive(:create!).and_raise(StandardError) allow(Ci::Pipeline).to receive(:create!).and_raise(StandardError)
end end
it 'raises an exception' do it 'raises an exeception with #full_messages populated' do
expect { subject }.to raise_error(Ci::RunDastScanService::CreatePipelineError) expect { subject }.to raise_error(Ci::RunDastScanService::RunError) do |error|
expect(error.full_messages).to include('Pipeline could not be created')
end
end end
end end
...@@ -120,8 +124,10 @@ describe Ci::RunDastScanService do ...@@ -120,8 +124,10 @@ describe Ci::RunDastScanService do
allow(Ci::Stage).to receive(:create!).and_raise(StandardError) allow(Ci::Stage).to receive(:create!).and_raise(StandardError)
end end
it 'raises an exception' do it 'raises an exeception with #full_messages populated' do
expect { subject }.to raise_error(Ci::RunDastScanService::CreateStageError) expect { subject }.to raise_error(Ci::RunDastScanService::RunError) do |error|
expect(error.full_messages).to include('Stage could not be created')
end
end end
it 'does not create a pipeline' do it 'does not create a pipeline' do
...@@ -134,8 +140,10 @@ describe Ci::RunDastScanService do ...@@ -134,8 +140,10 @@ describe Ci::RunDastScanService do
allow(Ci::Build).to receive(:create!).and_raise(StandardError) allow(Ci::Build).to receive(:create!).and_raise(StandardError)
end end
it 'raises an exception' do it 'raises an exeception with #full_messages populated' do
expect { subject }.to raise_error(Ci::RunDastScanService::CreateBuildError) expect { subject }.to raise_error(Ci::RunDastScanService::RunError) do |error|
expect(error.full_messages).to include('Build could not be created')
end
end end
it 'does not create a stage' do it 'does not create a stage' do
...@@ -148,14 +156,33 @@ describe Ci::RunDastScanService do ...@@ -148,14 +156,33 @@ describe Ci::RunDastScanService do
allow_any_instance_of(Ci::Build).to receive(:enqueue!).and_raise(StandardError) allow_any_instance_of(Ci::Build).to receive(:enqueue!).and_raise(StandardError)
end end
it 'raises an exception' do it 'raises an exeception with #full_messages populated' do
expect { subject }.to raise_error(Ci::RunDastScanService::EnqueueError) expect { subject }.to raise_error(Ci::RunDastScanService::RunError) do |error|
expect(error.full_messages).to include('Build could not be enqueued')
end
end end
it 'does not create a build' do it 'does not create a build' do
expect { subject rescue nil }.not_to change(Ci::Pipeline, :count) expect { subject rescue nil }.not_to change(Ci::Pipeline, :count)
end end
end end
context 'when a validation error is raised' do
before do
klass = Ci::Pipeline
allow(klass).to receive(:create!).and_raise(
ActiveRecord::RecordInvalid, klass.new.tap do |pl|
pl.errors.add(:sha, 'can\'t be blank')
end
)
end
it 'raises an exeception with #full_messages populated' do
expect { subject }.to raise_error(Ci::RunDastScanService::RunError) do |error|
expect(error.full_messages).to include('Sha can\'t be blank')
end
end
end
end end
end end
end end
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