Commit 4a6eb007 authored by Rémy Coutable's avatar Rémy Coutable Committed by Robert Speicher

Merge branch 'validate-only-except-regexp' into 'master'

Validate only and except regexp

## What does this MR do?
Adds a better validation for only and except which can contain regexps.

## Why was this MR needed?
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.

cc @grzesiek 

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] Tests
  - [x] Added for this feature/bug
  - [ ] All builds are passing
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !4736
parent 2f1470d4
...@@ -58,6 +58,7 @@ v 8.9.0 (unreleased) ...@@ -58,6 +58,7 @@ v 8.9.0 (unreleased)
- Bamboo Service: Fix missing credentials & URL handling when base URL contains a path (Benjamin Schmid) - 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 - TeamCity Service: Fix URL handling when base URL contains a path
- Todos will display target state if issuable target is 'Closed' or 'Merged' - 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 - 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 - Add support for using Yubikeys (U2F) for two-factor authentication
- Link to blank group icon doesn't throw a 404 anymore - Link to blank group icon doesn't throw a 404 anymore
......
...@@ -204,12 +204,12 @@ module Ci ...@@ -204,12 +204,12 @@ module Ci
raise ValidationError, "#{name} job: tags parameter should be an array of strings" raise ValidationError, "#{name} job: tags parameter should be an array of strings"
end end
if job[:only] && !validate_array_of_strings(job[:only]) if job[:only] && !validate_array_of_strings_or_regexps(job[:only])
raise ValidationError, "#{name} job: only parameter should be an array of strings" raise ValidationError, "#{name} job: only parameter should be an array of strings or regexps"
end end
if job[:except] && !validate_array_of_strings(job[:except]) if job[:except] && !validate_array_of_strings_or_regexps(job[:except])
raise ValidationError, "#{name} job: except parameter should be an array of strings" raise ValidationError, "#{name} job: except parameter should be an array of strings or regexps"
end end
if job[:allow_failure] && !validate_boolean(job[:allow_failure]) if job[:allow_failure] && !validate_boolean(job[:allow_failure])
......
...@@ -15,6 +15,10 @@ module Gitlab ...@@ -15,6 +15,10 @@ module Gitlab
values.is_a?(Array) && values.all? { |value| validate_string(value) } values.is_a?(Array) && values.all? { |value| validate_string(value) }
end 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) def validate_variables(variables)
variables.is_a?(Hash) && variables.is_a?(Hash) &&
variables.all? { |key, value| validate_string(key) && validate_string(value) } variables.all? { |key, value| validate_string(key) && validate_string(value) }
...@@ -24,6 +28,19 @@ module Gitlab ...@@ -24,6 +28,19 @@ module Gitlab
value.is_a?(String) || value.is_a?(Symbol) value.is_a?(String) || value.is_a?(Symbol)
end 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) def validate_environment(value)
value.is_a?(String) && value =~ Gitlab::Regex.environment_name_regex value.is_a?(String) && value =~ Gitlab::Regex.environment_name_regex
end end
......
...@@ -157,6 +157,35 @@ module Ci ...@@ -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("test", "deploy").size).to eq(1)
expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(1)
end 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 end
describe :except do describe :except do
...@@ -284,8 +313,36 @@ module Ci ...@@ -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("test", "test").size).to eq(0)
expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(0) expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(0)
end 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 end
describe "Scripts handling" do describe "Scripts handling" 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