diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2018-02-07 16:09:56 +0300 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2018-02-07 16:09:56 +0300 |
commit | ead97c55eac773444dee547a934112aa282c2e2e (patch) | |
tree | b172b648743d3b22af5d2177bf373bd11ddcb098 | |
parent | 2122d7225012b48bad8121e617bc1dbb06f67cf3 (diff) | |
parent | 1c36766f93180620e4ba4b50bea6908817d01631 (diff) |
Merge branch '41209-ci-linter-fails-on-gitlab-ci-blob-viewer' into 'master'
Resolve "CI linter fails to work"
Closes #41209
See merge request gitlab-org/gitlab-ce!16922
-rw-r--r-- | app/controllers/ci/lints_controller.rb | 5 | ||||
-rw-r--r-- | app/models/ci/pipeline.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/41209-ci-linter-fails-on-gitlab-ci-blob-viewer.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/ci/config/loader.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/ci/yaml_processor.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/config/loader_spec.rb | 10 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/yaml_processor_spec.rb | 43 |
7 files changed, 43 insertions, 26 deletions
diff --git a/app/controllers/ci/lints_controller.rb b/app/controllers/ci/lints_controller.rb index be667687c18..e9bd1689a1e 100644 --- a/app/controllers/ci/lints_controller.rb +++ b/app/controllers/ci/lints_controller.rb @@ -16,10 +16,7 @@ module Ci @builds = @config_processor.builds @jobs = @config_processor.jobs end - rescue - @error = 'Undefined error' - @status = false - ensure + render :show end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index f84bf132854..2abe90dd181 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -394,7 +394,7 @@ module Ci @config_processor ||= begin 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 nil rescue diff --git a/changelogs/unreleased/41209-ci-linter-fails-on-gitlab-ci-blob-viewer.yml b/changelogs/unreleased/41209-ci-linter-fails-on-gitlab-ci-blob-viewer.yml new file mode 100644 index 00000000000..61d6bf8fd36 --- /dev/null +++ b/changelogs/unreleased/41209-ci-linter-fails-on-gitlab-ci-blob-viewer.yml @@ -0,0 +1,5 @@ +--- +title: 'Handle all Psych YAML parser exceptions (fixes #41209)' +merge_request: +author: +type: fixed diff --git a/lib/gitlab/ci/config/loader.rb b/lib/gitlab/ci/config/loader.rb index e7d9f6a7761..141d2714cb6 100644 --- a/lib/gitlab/ci/config/loader.rb +++ b/lib/gitlab/ci/config/loader.rb @@ -6,6 +6,8 @@ module Gitlab def initialize(config) @config = YAML.safe_load(config, [Symbol], [], true) + rescue Psych::Exception => e + raise FormatError, e.message end def valid? diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 0bd78b03448..a7285ac8f9d 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -85,7 +85,7 @@ module Gitlab begin Gitlab::Ci::YamlProcessor.new(content) nil - rescue ValidationError, Psych::SyntaxError => e + rescue ValidationError => e e.message end end diff --git a/spec/lib/gitlab/ci/config/loader_spec.rb b/spec/lib/gitlab/ci/config/loader_spec.rb index 2d44b1f60f1..590fc8904c1 100644 --- a/spec/lib/gitlab/ci/config/loader_spec.rb +++ b/spec/lib/gitlab/ci/config/loader_spec.rb @@ -38,6 +38,16 @@ describe Gitlab::Ci::Config::Loader do 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 let(:yml) { '' } diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 98880fe9f28..f83f932e61e 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -1394,11 +1394,15 @@ EOT describe "Error handling" do it "fails to parse YAML" do - expect {Gitlab::Ci::YamlProcessor.new("invalid: yaml: test")}.to raise_error(Psych::SyntaxError) + expect do + Gitlab::Ci::YamlProcessor.new("invalid: yaml: test") + end.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError) end it "indicates that object is invalid" do - expect {Gitlab::Ci::YamlProcessor.new("invalid_yaml")}.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError) + expect do + Gitlab::Ci::YamlProcessor.new("invalid_yaml") + end.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError) end it "returns errors if tags parameter is invalid" do @@ -1688,37 +1692,36 @@ EOT end describe "#validation_message" do + subject { Gitlab::Ci::YamlProcessor.validation_message(content) } + context "when the YAML could not be parsed" do - it "returns an error about invalid configutaion" do - content = YAML.dump("invalid: yaml: test") + let(:content) { YAML.dump("invalid: yaml: test") } - expect(Gitlab::Ci::YamlProcessor.validation_message(content)) - .to eq "Invalid configuration format" - end + it { is_expected.to eq "Invalid configuration format" } end context "when the tags parameter is invalid" do - it "returns an error about invalid tags" do - content = YAML.dump({ rspec: { script: "test", tags: "mysql" } }) + let(:content) { YAML.dump({ rspec: { script: "test", tags: "mysql" } }) } - expect(Gitlab::Ci::YamlProcessor.validation_message(content)) - .to eq "jobs:rspec tags should be an array of strings" - end + it { is_expected.to eq "jobs:rspec tags should be an array of strings" } end context "when YAML content is empty" do - it "returns an error about missing content" do - expect(Gitlab::Ci::YamlProcessor.validation_message('')) - .to eq "Please provide content of .gitlab-ci.yml" - end + let(:content) { '' } + + it { is_expected.to eq "Please provide content of .gitlab-ci.yml" } + 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 context "when the YAML is valid" do - it "does not return any errors" do - content = File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) + let(:content) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) } - expect(Gitlab::Ci::YamlProcessor.validation_message(content)).to be_nil - end + it { is_expected.to be_nil } end end |