diff options
| -rw-r--r-- | lib/gitlab/ci/config/entry/policy.rb | 20 | ||||
| -rw-r--r-- | lib/gitlab/ci/pipeline/expression/statement.rb | 14 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config/entry/policy_spec.rb | 25 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb | 40 | ||||
| -rw-r--r-- | spec/models/ci/build_spec.rb | 2 |
5 files changed, 88 insertions, 13 deletions
diff --git a/lib/gitlab/ci/config/entry/policy.rb b/lib/gitlab/ci/config/entry/policy.rb index 0027e9ec8c5..b6d137a7e68 100644 --- a/lib/gitlab/ci/config/entry/policy.rb +++ b/lib/gitlab/ci/config/entry/policy.rb @@ -25,15 +25,31 @@ module Gitlab include Entry::Validatable include Entry::Attributable - attributes :refs, :kubernetes + attributes :refs, :kubernetes, :variables validations do validates :config, presence: true - validates :config, allowed_keys: %i[refs kubernetes] + validates :config, allowed_keys: %i[refs kubernetes variables] + validate :variables_expressions_syntax with_options allow_nil: true do validates :refs, array_of_strings_or_regexps: true validates :kubernetes, allowed_values: %w[active] + validates :variables, array_of_strings: true + end + + def variables_expressions_syntax + return unless variables.is_a?(Array) + + statements = variables.map do |statement| + ::Gitlab::Ci::Pipeline::Expression::Statement.new(statement) + end + + statements.each do |statement| + unless statement.valid? + errors.add(:variables, "Invalid expression #{statement.inspect}") + end + end end end end diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb index 08e662eccf9..616a9fe204c 100644 --- a/lib/gitlab/ci/pipeline/expression/statement.rb +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -14,9 +14,11 @@ module Gitlab %w[variable] ].freeze - def initialize(statement, pipeline) + def initialize(statement, pipeline = nil) @lexer = Expression::Lexer.new(statement) + return if pipeline.nil? + @variables = pipeline.variables.map do |variable| [variable.key.to_sym, variable.value] end @@ -35,6 +37,16 @@ module Gitlab def evaluate parse_tree.evaluate(@variables.to_h) end + + def inspect + "syntax: #{@lexer.lexemes.join(' ')}" + end + + def valid? + parse_tree.is_a?(Lexeme::Base) + rescue StatementError + false + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb index 5e83abf645b..f06d3a13ce0 100644 --- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb @@ -83,6 +83,31 @@ describe Gitlab::Ci::Config::Entry::Policy do end end + context 'when specifying valid variables expressions policy' do + let(:config) { { variables: ['$VAR == null'] } } + + it 'is a correct configuraton' do + expect(entry).to be_valid + expect(entry.value).to eq(config) + end + end + + context 'when specifying variables expressions in invalid format' do + let(:config) { { variables: '$MY_VAR' } } + + it 'reports an error about invalid format' do + expect(entry.errors).to include /should be an array of strings/ + end + end + + context 'when specifying invalid variables expressions statement' do + let(:config) { { variables: ['$MY_VAR =='] } } + + it 'reports an error about invalid statement' do + expect(entry.errors).to include /invalid expression syntax: variable equals/ + end + end + context 'when specifying unknown policy' do let(:config) { { refs: ['master'], invalid: :something } } diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index 472a58599d8..3d97d71d629 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -11,6 +11,16 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do pipeline.variables.build([key: 'VARIABLE', value: 'my variable']) end + describe '.new' do + context 'when pipeline is not provided' do + it 'allows to properly initialize the statement' do + statement = described_class.new('$VARIABLE') + + expect(statement.evaluate).to be_nil + end + end + end + describe '#parse_tree' do context 'when expression is empty' do let(:text) { '' } @@ -23,18 +33,26 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do context 'when expression grammar is incorrect' do table = [ - '$VAR "text"', # missing operator - '== "123"', # invalid right side - "'single quotes'", # single quotes string - '$VAR ==', # invalid right side - '12345', # unknown syntax - '' # empty statement + '$VAR "text"', # missing operator + '== "123"', # invalid left side + '"some string"', # only string provided + '$VAR ==', # invalid right side + '12345', # unknown syntax + '' # empty statement ] table.each do |syntax| - it "raises an error when syntax is `#{syntax}`" do - expect { described_class.new(syntax, pipeline).parse_tree } - .to raise_error described_class::StatementError + context "when expression grammar is #{syntax.inspect}" do + let(:text) { syntax } + + it 'aises a statement error exception' do + expect { subject.parse_tree } + .to raise_error described_class::StatementError + end + + it 'is an invalid statement' do + expect(subject).not_to be_valid + end end end end @@ -47,6 +65,10 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do expect(subject.parse_tree) .to be_a Gitlab::Ci::Pipeline::Expression::Lexeme::Equals end + + it 'is a valid statement' do + expect(subject).to be_valid + end end context 'when using a single token' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d1db61084c3..2328c3ee902 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1896,7 +1896,7 @@ describe Ci::Build do context 'when there are duplicated YAML variables' do before do build.yaml_variables = [{ key: 'MYVAR', value: 'first', public: true }, - { key: 'MYVAR', value: 'second', public: true}] + { key: 'MYVAR', value: 'second', public: true }] end it 'keeps the last occurence of a variable by given key' do |