Validate only and except regexpvalidate-only-except-regexp

Currently the RegexpError can be raised when processing next stage which leads to 500 in different places of code base.
This adds early check that regexps used in only and except are valid.

4 files changed, 80 insertions, 5 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f3ac35be81f..891a2276648 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -59,6 +59,7 @@ v 8.9.0 (unreleased)
   - Bamboo Service: Fix missing credentials & URL handling when base URL contains a path (Benjamin Schmid)
   - TeamCity Service: Fix URL handling when base URL contains a path
   - Todos will display target state if issuable target is 'Closed' or 'Merged'
+  - Validate only and except regexp
   - Fix bug when sorting issues by milestone due date and filtering by two or more labels
   - Add support for using Yubikeys (U2F) for two-factor authentication
   - Link to blank group icon doesn't throw a 404 anymore
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index a66602f9194..325ab795def 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -204,12 +204,12 @@ module Ci
         raise ValidationError, "#{name} job: tags parameter should be an array of strings"
       end
 
-      if job[:only] && !validate_array_of_strings(job[:only])
-        raise ValidationError, "#{name} job: only parameter should be an array of strings"
+      if job[:only] && !validate_array_of_strings_or_regexps(job[:only])
+        raise ValidationError, "#{name} job: only parameter should be an array of strings or regexps"
       end
 
-      if job[:except] && !validate_array_of_strings(job[:except])
-        raise ValidationError, "#{name} job: except parameter should be an array of strings"
+      if job[:except] && !validate_array_of_strings_or_regexps(job[:except])
+        raise ValidationError, "#{name} job: except parameter should be an array of strings or regexps"
       end
 
       if job[:allow_failure] && !validate_boolean(job[:allow_failure])
diff --git a/lib/gitlab/ci/config/node/validation_helpers.rb b/lib/gitlab/ci/config/node/validation_helpers.rb
index 3900fc89391..72f648975dc 100644
--- a/lib/gitlab/ci/config/node/validation_helpers.rb
+++ b/lib/gitlab/ci/config/node/validation_helpers.rb
@@ -15,6 +15,10 @@ module Gitlab
             values.is_a?(Array) && values.all? { |value| validate_string(value) }
           end
 
+          def validate_array_of_strings_or_regexps(values)
+            values.is_a?(Array) && values.all? { |value| validate_string_or_regexp(value) }
+          end
+
           def validate_variables(variables)
             variables.is_a?(Hash) &&
               variables.all? { |key, value| validate_string(key) && validate_string(value) }
@@ -24,6 +28,19 @@ module Gitlab
             value.is_a?(String) || value.is_a?(Symbol)
           end
 
+          def validate_string_or_regexp(value)
+            return true if value.is_a?(Symbol)
+            return false unless value.is_a?(String)
+
+            if value.first == '/' && value.last == '/'
+              Regexp.new(value[1...-1])
+            else
+              true
+            end
+          rescue RegexpError
+            false
+          end
+
           def validate_environment(value)
             value.is_a?(String) && value =~ Gitlab::Regex.environment_name_regex
           end
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 143e2e6d238..b0b03146d0c 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -157,6 +157,35 @@ module Ci
           expect(config_processor.builds_for_stage_and_ref("test", "deploy").size).to eq(1)
           expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(1)
         end
+
+        context 'for invalid value' do
+          let(:config) { { rspec: { script: "rspec", type: "test", only: only } } }
+          let(:processor) { GitlabCiYamlProcessor.new(YAML.dump(config)) }
+
+          shared_examples 'raises an error' do
+            it do
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'rspec job: only parameter should be an array of strings or regexps')
+            end
+          end
+
+          context 'when it is integer' do
+            let(:only) { 1 }
+
+            it_behaves_like 'raises an error'
+          end
+
+          context 'when it is an array of integers' do
+            let(:only) { [1, 1] }
+
+            it_behaves_like 'raises an error'
+          end
+
+          context 'when it is invalid regex' do
+            let(:only) { ["/*invalid/"] }
+
+            it_behaves_like 'raises an error'
+          end
+        end
       end
 
       describe :except do
@@ -284,8 +313,36 @@ module Ci
           expect(config_processor.builds_for_stage_and_ref("test", "test").size).to eq(0)
           expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(0)
         end
-      end
 
+        context 'for invalid value' do
+          let(:config) { { rspec: { script: "rspec", except: except } } }
+          let(:processor) { GitlabCiYamlProcessor.new(YAML.dump(config)) }
+
+          shared_examples 'raises an error' do
+            it do
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'rspec job: except parameter should be an array of strings or regexps')
+            end
+          end
+
+          context 'when it is integer' do
+            let(:except) { 1 }
+
+            it_behaves_like 'raises an error'
+          end
+
+          context 'when it is an array of integers' do
+            let(:except) { [1, 1] }
+
+            it_behaves_like 'raises an error'
+          end
+
+          context 'when it is invalid regex' do
+            let(:except) { ["/*invalid/"] }
+
+            it_behaves_like 'raises an error'
+          end
+        end
+      end
     end
     
     describe "Scripts handling" do