Commit 66d41d2c authored by Dylan Griffith's avatar Dylan Griffith

Handle all YAML parser exceptions in .gitlab-ci.yml (fixes #41209)

- Move the exception handling as close to the source as possible to
avoid leaking Psych ahstraction
- Also remove unnecessary rescue all statement from LintsController.
This should not be necessary anymore since any YAML errors should all be
caught by the #validation_message method.
parent cf887a8b
...@@ -16,10 +16,7 @@ module Ci ...@@ -16,10 +16,7 @@ module Ci
@builds = @config_processor.builds @builds = @config_processor.builds
@jobs = @config_processor.jobs @jobs = @config_processor.jobs
end end
rescue
@error = 'Undefined error'
@status = false
ensure
render :show render :show
end end
end end
......
...@@ -394,7 +394,7 @@ module Ci ...@@ -394,7 +394,7 @@ module Ci
@config_processor ||= begin @config_processor ||= begin
Gitlab::Ci::YamlProcessor.new(ci_yaml_file) Gitlab::Ci::YamlProcessor.new(ci_yaml_file)
rescue Gitlab::Ci::YamlProcessor::ValidationError, Psych::SyntaxError => e rescue Gitlab::Ci::YamlProcessor::ValidationError => e
self.yaml_errors = e.message self.yaml_errors = e.message
nil nil
rescue rescue
......
---
title: 'Handle all Psych YAML parser exceptions (fixes #41209)'
merge_request:
author:
type: fixed
...@@ -6,6 +6,8 @@ module Gitlab ...@@ -6,6 +6,8 @@ module Gitlab
def initialize(config) def initialize(config)
@config = YAML.safe_load(config, [Symbol], [], true) @config = YAML.safe_load(config, [Symbol], [], true)
rescue Psych::Exception => e
raise FormatError, e.message
end end
def valid? def valid?
......
...@@ -85,7 +85,7 @@ module Gitlab ...@@ -85,7 +85,7 @@ module Gitlab
begin begin
Gitlab::Ci::YamlProcessor.new(content) Gitlab::Ci::YamlProcessor.new(content)
nil nil
rescue ValidationError, Psych::SyntaxError => e rescue ValidationError => e
e.message e.message
end end
end end
......
...@@ -38,6 +38,16 @@ describe Gitlab::Ci::Config::Loader do ...@@ -38,6 +38,16 @@ describe Gitlab::Ci::Config::Loader do
end end
end end
context 'when there is an unknown alias' do
let(:yml) { 'steps: *bad_alias' }
describe '#initialize' do
it 'raises FormatError' do
expect { loader }.to raise_error(Gitlab::Ci::Config::Loader::FormatError, 'Unknown alias: bad_alias')
end
end
end
context 'when yaml config is empty' do context 'when yaml config is empty' do
let(:yml) { '' } let(:yml) { '' }
......
...@@ -1394,7 +1394,7 @@ EOT ...@@ -1394,7 +1394,7 @@ EOT
describe "Error handling" do describe "Error handling" do
it "fails to parse YAML" do it "fails to parse YAML" do
expect {Gitlab::Ci::YamlProcessor.new("invalid: yaml: test")}.to raise_error(Psych::SyntaxError) expect {Gitlab::Ci::YamlProcessor.new("invalid: yaml: test")}.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError)
end end
it "indicates that object is invalid" do it "indicates that object is invalid" do
...@@ -1688,37 +1688,35 @@ EOT ...@@ -1688,37 +1688,35 @@ EOT
end end
describe "#validation_message" do describe "#validation_message" do
subject { Gitlab::Ci::YamlProcessor.validation_message(content) }
context "when the YAML could not be parsed" do context "when the YAML could not be parsed" do
it "returns an error about invalid configutaion" do let(:content) { YAML.dump("invalid: yaml: test") }
content = YAML.dump("invalid: yaml: test")
expect(Gitlab::Ci::YamlProcessor.validation_message(content)) it { is_expected.to eq "Invalid configuration format" }
.to eq "Invalid configuration format"
end
end end
context "when the tags parameter is invalid" do context "when the tags parameter is invalid" do
it "returns an error about invalid tags" do let(:content) { YAML.dump({ rspec: { script: "test", tags: "mysql" } }) }
content = YAML.dump({ rspec: { script: "test", tags: "mysql" } })
expect(Gitlab::Ci::YamlProcessor.validation_message(content)) it { is_expected.to eq "jobs:rspec tags should be an array of strings" }
.to eq "jobs:rspec tags should be an array of strings"
end
end end
context "when YAML content is empty" do context "when YAML content is empty" do
it "returns an error about missing content" do let(:content) { '' }
expect(Gitlab::Ci::YamlProcessor.validation_message('')) it { is_expected.to eq "Please provide content of .gitlab-ci.yml" }
.to eq "Please provide content of .gitlab-ci.yml"
end end
context 'when the YAML contains an unknown alias' do
let(:content) { 'steps: *bad_alias' }
it { is_expected.to eq "Unknown alias: bad_alias" }
end end
context "when the YAML is valid" do context "when the YAML is valid" do
it "does not return any errors" do let(:content) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) }
content = File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
expect(Gitlab::Ci::YamlProcessor.validation_message(content)).to be_nil it { is_expected.to be_nil }
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