Commit 281b12f5 authored by Matija Čupić's avatar Matija Čupić

Allow combining hashes and nested hash arrays

Allows combining hashes and nested hash arrays in rules sections of
jobs.
parent ae3ce141
...@@ -16,6 +16,7 @@ module Gitlab ...@@ -16,6 +16,7 @@ module Gitlab
PROCESSABLE_ALLOWED_KEYS = %i[extends stage only except rules variables PROCESSABLE_ALLOWED_KEYS = %i[extends stage only except rules variables
inherit allow_failure when needs resource_group].freeze inherit allow_failure when needs resource_group].freeze
MAX_NESTING_LEVEL = 10
included do included do
validations do validations do
...@@ -31,7 +32,7 @@ module Gitlab ...@@ -31,7 +32,7 @@ module Gitlab
with_options allow_nil: true do with_options allow_nil: true do
validates :extends, array_of_strings_or_string: true validates :extends, array_of_strings_or_string: true
validates :rules, nested_array_of_hashes: true validates :rules, nested_array_of_hashes_or_arrays: { max_level: MAX_NESTING_LEVEL }
validates :resource_group, type: String validates :resource_group, type: String
end end
end end
......
...@@ -90,14 +90,22 @@ module Gitlab ...@@ -90,14 +90,22 @@ module Gitlab
end end
end end
class NestedArrayOfHashesValidator < ArrayOfHashesValidator class NestedArrayOfHashesOrArraysValidator < ArrayOfHashesValidator
include NestedArrayHelpers include NestedArrayHelpers
def validate_each(record, attribute, value) def validate_each(record, attribute, value)
unless validate_nested_array(value, 1, &method(:validate_array_of_hashes)) max_level = options.fetch(:max_level, 1)
unless validate_nested_array(value, max_level, &method(:validate_hash))
record.errors.add(attribute, 'should be an array containing hashes and arrays of hashes') record.errors.add(attribute, 'should be an array containing hashes and arrays of hashes')
end end
end end
private
def validate_hash(value)
value.is_a?(Hash)
end
end end
class ArrayOrStringValidator < ActiveModel::EachValidator class ArrayOrStringValidator < ActiveModel::EachValidator
......
...@@ -169,6 +169,22 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do ...@@ -169,6 +169,22 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do
it { expect(entry).to be_valid } it { expect(entry).to be_valid }
end end
end end
context 'when rules are used' do
let(:config) { { script: 'ls', cache: { key: 'test' }, rules: rules } }
let(:rules) do
[
{ if: '$CI_PIPELINE_SOURCE == "schedule"', when: 'never' },
[
{ if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH' },
{ if: '$CI_PIPELINE_SOURCE == "merge_request_event"' }
]
]
end
it { expect(entry).to be_valid }
end
end end
context 'when entry value is not correct' do context 'when entry value is not correct' do
...@@ -485,6 +501,70 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do ...@@ -485,6 +501,70 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do
end end
end end
end end
context 'when invalid rules are used' do
let(:config) { { script: 'ls', cache: { key: 'test' }, rules: rules } }
context 'with rules nested more than max allowed levels' do
let(:sample_rule) { { if: '$THIS == "other"', when: 'always' } }
let(:rules) do
[
{ if: '$THIS == "that"', when: 'always' },
[
{ if: '$SKIP', when: 'never' },
[
sample_rule,
[
sample_rule,
[
sample_rule,
[
sample_rule,
[
sample_rule,
[
sample_rule,
[
sample_rule,
[
sample_rule,
[
sample_rule,
[
sample_rule,
[sample_rule]
]
]
]
]
]
]
]
]
]
]
]
]
end
it { expect(entry).not_to be_valid }
end
context 'with rules with invalid keys' do
let(:rules) do
[
{ invalid_key: 'invalid' },
[
{ if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH' },
{ if: '$CI_PIPELINE_SOURCE == "merge_request_event"' }
]
]
end
it { expect(entry).not_to be_valid }
end
end
end end
end end
......
...@@ -53,7 +53,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do ...@@ -53,7 +53,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do
let(:config) do let(:config) do
[ [
{ if: '$THIS == "that"', when: 'always' }, { if: '$THIS == "that"', when: 'always' },
[{ if: '$SKIP', when: 'never' }] [{ if: '$SKIP', when: 'never' }, { if: '$THIS == "other"', when: 'always' }]
] ]
end end
...@@ -64,11 +64,11 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do ...@@ -64,11 +64,11 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do
let(:config) do let(:config) do
[ [
{ if: '$THIS == "that"', when: 'always' }, { if: '$THIS == "that"', when: 'always' },
[{ if: '$SKIP', when: 'never' }, [{ if: '$THIS == "other"', when: 'aways' }]] [{ if: '$SKIP', when: 'never' }, [{ if: '$THIS == "other"', when: 'always' }]]
] ]
end end
it { is_expected.not_to be_valid } it { is_expected.to be_valid }
end end
end end
...@@ -119,7 +119,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do ...@@ -119,7 +119,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Rules do
context 'with rules nested more than one level' do context 'with rules nested more than one level' do
let(:first_rule) { { if: '$THIS == "that"', when: 'always' } } let(:first_rule) { { if: '$THIS == "that"', when: 'always' } }
let(:second_rule) { { if: '$SKIP', when: 'never' } } let(:second_rule) { { if: '$SKIP', when: 'never' } }
let(:third_rule) { { if: '$THIS == "other"', when: 'aways' } } let(:third_rule) { { if: '$THIS == "other"', when: 'always' } }
let(:config) do let(:config) 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