Merge branch 'yaml-processor-validation-errors' into 'master'

Ci::YamlProcessor.new_with_validation_errors to prevent double processing

See merge request gitlab-org/gitlab!21589

app/controllers/projects/ci/lints_controller.rb

@@ -8,11 +8,13 @@ class Projects::Ci::LintsController < Projects::ApplicationController
 
   def create
     @content = params[:content]
-    @error = Gitlab::Ci::YamlProcessor.validation_message(@content, yaml_processor_options)
-    @status = @error.blank?
+    result   = Gitlab::Ci::YamlProcessor.new_with_validation_errors(@content, yaml_processor_options)
 
-    if @error.blank?
-      @config_processor = Gitlab::Ci::YamlProcessor.new(@content, yaml_processor_options)
+    @error   = result.errors.join(', ')
+    @status  = result.valid?
+
+    if result.valid?
+      @config_processor = result.content
       @stages = @config_processor.stages
       @builds = @config_processor.builds
       @jobs = @config_processor.jobs

changelogs/unreleased/yaml-processor-validation-errors.yml

+---
+title: Return multiple errors from CI linter
+merge_request: 21589
+author:
+type: added

lib/gitlab/ci/yaml_processor.rb

@@ -9,6 +9,12 @@ module Gitlab
 
       attr_reader :stages, :jobs
 
+      ResultWithErrors = Struct.new(:content, :errors) do
+        def valid?
+          errors.empty?
+        end
+      end
+
       def initialize(config, opts = {})
         @ci_config = Gitlab::Ci::Config.new(config, **opts)
         @config = @ci_config.to_hash
@@ -22,6 +28,18 @@ module Gitlab
         raise ValidationError, e.message
       end
 
+      def self.new_with_validation_errors(content, opts = {})
+        return ResultWithErrors.new('', ['Please provide content of .gitlab-ci.yml']) if content.blank?
+
+        config = Gitlab::Ci::Config.new(content, **opts)
+        return ResultWithErrors.new("", config.errors) unless config.valid?
+
+        config = Gitlab::Ci::YamlProcessor.new(content, opts)
+        ResultWithErrors.new(config, [])
+      rescue ValidationError, Gitlab::Ci::Config::ConfigError => e
+        ResultWithErrors.new('', [e.message])
+      end
+
       def builds
         @jobs.map do |name, _|
           build_attributes(name)

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -2235,6 +2235,70 @@ module Gitlab
           it { is_expected.to be_nil }
         end
       end
+
+      describe '.new_with_validation_errors' do
+        subject { Gitlab::Ci::YamlProcessor.new_with_validation_errors(content) }
+
+        context 'when the YAML could not be parsed' do
+          let(:content) { YAML.dump('invalid: yaml: test') }
+
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['Invalid configuration format'])
+            expect(subject.content).to be_blank
+          end
+        end
+
+        context 'when the tags parameter is invalid' do
+          let(:content) { YAML.dump({ rspec: { script: 'test', tags: 'mysql' } }) }
+
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['jobs:rspec:tags config should be an array of strings'])
+            expect(subject.content).to be_blank
+          end
+        end
+
+        context 'when the configuration contains multiple keyword-syntax errors' do
+          let(:content) { YAML.dump({ rspec: { script: 'test', bad_tags: 'mysql', rules: { wrong: 'format' } } }) }
+
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['jobs:rspec config contains unknown keys: bad_tags', 'jobs:rspec rules should be an array of hashes'])
+            expect(subject.content).to be_blank
+          end
+        end
+
+        context 'when YAML content is empty' do
+          let(:content) { '' }
+
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['Please provide content of .gitlab-ci.yml'])
+            expect(subject.content).to be_blank
+          end
+        end
+
+        context 'when the YAML contains an unknown alias' do
+          let(:content) { 'steps: *bad_alias' }
+
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(false)
+            expect(subject.errors).to eq(['Unknown alias: bad_alias'])
+            expect(subject.content).to be_blank
+          end
+        end
+
+        context 'when the YAML is valid' do
+          let(:content) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) }
+
+          it 'returns errors and empty configuration' do
+            expect(subject.valid?).to eq(true)
+            expect(subject.errors).to be_empty
+            expect(subject.content).to be_present
+          end
+        end
+      end
     end
   end
 end