Commit 53bac5ca authored by Stan Hu's avatar Stan Hu

Gracefully handle CI lint errors in artifacts section

If an `artifacts` section is included in the CI lint as an array instead
of a Hash, users would see 500 errors when linting the file. Even though
we separate validation that checks the type, this validation doesn't
halt other validations from running. Since `expose_as_present?` is
called inside a validation, we need to perform a redundant check to
ensure the right type.

Closes https://gitlab.com/gitlab-org/gitlab/issues/194069
parent 62052283
---
title: Gracefully error handle CI lint errors in artifacts section
merge_request: 22388
author:
type: fixed
...@@ -54,6 +54,11 @@ module Gitlab ...@@ -54,6 +54,11 @@ module Gitlab
def expose_as_present? def expose_as_present?
return false unless Feature.enabled?(:ci_expose_arbitrary_artifacts_in_mr, default_enabled: true) return false unless Feature.enabled?(:ci_expose_arbitrary_artifacts_in_mr, default_enabled: true)
# This duplicates the `validates :config, type: Hash` above,
# but Validatable currently doesn't halt the validation
# chain if it encounters a validation error.
return false unless @config.is_a?(Hash)
!@config[:expose_as].nil? !@config[:expose_as].nil?
end end
end end
......
...@@ -1270,6 +1270,19 @@ module Gitlab ...@@ -1270,6 +1270,19 @@ module Gitlab
expect(builds.first[:options][:artifacts][:when]).to eq(when_state) expect(builds.first[:options][:artifacts][:when]).to eq(when_state)
end end
end end
it "gracefully handles errors in artifacts type" do
config = <<~YAML
test:
script:
- echo "Hello world"
artifacts:
- paths:
- test/
YAML
expect { described_class.new(config) }.to raise_error(described_class::ValidationError)
end
end end
describe '#environment' do describe '#environment' 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