diff --git a/config/feature_flags/development/ci_fix_rules_if_comparison_with_regexp_variable.yml b/config/feature_flags/development/ci_fix_rules_if_comparison_with_regexp_variable.yml new file mode 100644 index 0000000000000000000000000000000000000000..be21707d3768ce2d5223eb35028fd329e6b9305c --- /dev/null +++ b/config/feature_flags/development/ci_fix_rules_if_comparison_with_regexp_variable.yml @@ -0,0 +1,8 @@ +--- +name: ci_fix_rules_if_comparison_with_regexp_variable +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85310 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/359740 +milestone: '15.0' +type: development +group: group::pipeline authoring +default_enabled: false diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index 1806a677c2341a6f99c5bd392730e182cd7e1743..ac21fdb3714099f3fddda082447089b0c3c1b880 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -3090,6 +3090,8 @@ job: - Unlike variables in [`script`](../variables/index.md#use-cicd-variables-in-job-scripts) sections, variables in rules expressions are always formatted as `$VARIABLE`. - You can use `rules:if` with `include` to [conditionally include other configuration files](includes.md#use-rules-with-include). +- In [GitLab 15.0 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/35438), + variables on the right side of `=~` and `!~` expressions are evaluated as regular expressions. **Related topics**: diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb index 4d65b914d8d4442dab2d46eace7cf8c6b09740db..6efb3a4f16a18593a02d7df6e4b4729b86b72848 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb @@ -11,8 +11,15 @@ class Matches < Lexeme::LogicalOperator def evaluate(variables = {}) text = @left.evaluate(variables) regexp = @right.evaluate(variables) + return false unless regexp + if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable) + # All variables are evaluated as strings, even if they are regexp strings. + # So, we need to convert them to regexp objects. + regexp = Lexeme::Pattern.build_and_evaluate(regexp, variables) + end + regexp.scan(text.to_s).present? end diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb index 29c5aa5d753d812d420cb0acbaa7f21faf6db871..a72e5dbc822f5659a3b0e53dc8fff884d595e2d0 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb @@ -11,8 +11,15 @@ class NotMatches < Lexeme::LogicalOperator def evaluate(variables = {}) text = @left.evaluate(variables) regexp = @right.evaluate(variables) + return true unless regexp + if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable) + # All variables are evaluated as strings, even if they are regexp strings. + # So, we need to convert them to regexp objects. + regexp = Lexeme::Pattern.build_and_evaluate(regexp, variables) + end + regexp.scan(text.to_s).empty? end diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb index c7106f3ec390ea18dfaa5aa61359d83691090c32..cd4106b16bb16431aa14d07926c1f5a237894bb4 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -35,6 +35,18 @@ def self.pattern def self.build(string) new(string) end + + def self.build_and_evaluate(data, variables = {}) + return data if data.is_a?(Gitlab::UntrustedRegexp) + + begin + new_pattern = build(data) + rescue Lexer::SyntaxError + return data + end + + new_pattern.evaluate(variables) + end end end end diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/value.rb b/lib/gitlab/ci/pipeline/expression/lexeme/value.rb index 6d872fee39d095eb644565d4c43dc3a50d105e6c..fa82bbe3275d9f12bb3207d80cac01726d7f076b 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/value.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/value.rb @@ -10,6 +10,8 @@ def self.type :value end + attr_reader :value + def initialize(value) @value = value end diff --git a/spec/lib/gitlab/ci/build/rules/rule/clause/if_spec.rb b/spec/lib/gitlab/ci/build/rules/rule/clause/if_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..81bce989833189384e43569c0d0c07a7dccaf967 --- /dev/null +++ b/spec/lib/gitlab/ci/build/rules/rule/clause/if_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'support/helpers/stubbed_feature' +require 'support/helpers/stub_feature_flags' + +RSpec.describe Gitlab::Ci::Build::Rules::Rule::Clause::If do + include StubFeatureFlags + + subject(:if_clause) { described_class.new(expression) } + + describe '#satisfied_by?' do + let(:context_class) { Gitlab::Ci::Build::Context::Base } + let(:rules_context) { instance_double(context_class, variables_hash: {}) } + + subject(:satisfied_by?) { if_clause.satisfied_by?(nil, rules_context) } + + context 'when expression is a basic string comparison' do + context 'when comparison is true' do + let(:expression) { '"value" == "value"' } + + it { is_expected.to eq(true) } + end + + context 'when comparison is false' do + let(:expression) { '"value" == "other"' } + + it { is_expected.to eq(false) } + end + end + + context 'when expression is a regexp' do + context 'when comparison is true' do + let(:expression) { '"abcde" =~ /^ab.*/' } + + it { is_expected.to eq(true) } + end + + context 'when comparison is false' do + let(:expression) { '"abcde" =~ /^af.*/' } + + it { is_expected.to eq(false) } + end + + context 'when both side of the expression are variables' do + let(:expression) { '$teststring =~ $pattern' } + + context 'when comparison is true' do + let(:rules_context) do + instance_double(context_class, variables_hash: { 'teststring' => 'abcde', 'pattern' => '/^ab.*/' }) + end + + it { is_expected.to eq(true) } + + context 'when the FF ci_fix_rules_if_comparison_with_regexp_variable is disabled' do + before do + stub_feature_flags(ci_fix_rules_if_comparison_with_regexp_variable: false) + end + + it { is_expected.to eq(false) } + end + end + + context 'when comparison is false' do + let(:rules_context) do + instance_double(context_class, variables_hash: { 'teststring' => 'abcde', 'pattern' => '/^af.*/' }) + end + + it { is_expected.to eq(false) } + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/build/rules_spec.rb b/spec/lib/gitlab/ci/build/rules_spec.rb index 37bfdca4d1d75b39c194ebbd3d48e8ec6e34f82d..e82dcd0254d24e65af1e7d91d4575c6665274d41 100644 --- a/spec/lib/gitlab/ci/build/rules_spec.rb +++ b/spec/lib/gitlab/ci/build/rules_spec.rb @@ -188,6 +188,19 @@ it { is_expected.to eq(described_class::Result.new('on_success', nil, nil, { MY_VAR: 'my var' })) } end end + + context 'with a regexp variable matching rule' do + let(:rule_list) { [{ if: '"abcde" =~ $pattern' }] } + + before do + allow(ci_build).to receive(:scoped_variables).and_return( + Gitlab::Ci::Variables::Collection.new + .append(key: 'pattern', value: '/^ab.*/', public: true) + ) + end + + it { is_expected.to eq(described_class::Result.new('on_success')) } + end end describe 'Gitlab::Ci::Build::Rules::Result' do diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb index 0da04d8dcf7be20c50713af119009f234411134b..83742699d3d6858b2b2a44f94b578f7af5ca9497 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb @@ -1,9 +1,13 @@ # frozen_string_literal: true require 'fast_spec_helper' +require 'support/helpers/stubbed_feature' +require 'support/helpers/stub_feature_flags' require_dependency 're2' RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do + include StubFeatureFlags + let(:left) { double('left') } let(:right) { double('right') } @@ -148,5 +152,29 @@ it { is_expected.to eq(false) } end + + context 'when right value is a regexp string' do + let(:right_value) { '/^ab.*/' } + + context 'when matching' do + let(:left_value) { 'abcde' } + + it { is_expected.to eq(true) } + + context 'when the FF ci_fix_rules_if_comparison_with_regexp_variable is disabled' do + before do + stub_feature_flags(ci_fix_rules_if_comparison_with_regexp_variable: false) + end + + it { is_expected.to eq(false) } + end + end + + context 'when not matching' do + let(:left_value) { 'dfg' } + + it { is_expected.to eq(false) } + end + end end end diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb index 9bff2355d58462c788d0506328d5c4ae578aec5a..aad33106647b6d8819d4b4d44bd9b5ea2e752196 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb @@ -1,9 +1,13 @@ # frozen_string_literal: true require 'fast_spec_helper' +require 'support/helpers/stubbed_feature' +require 'support/helpers/stub_feature_flags' require_dependency 're2' RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::NotMatches do + include StubFeatureFlags + let(:left) { double('left') } let(:right) { double('right') } @@ -148,5 +152,29 @@ it { is_expected.to eq(true) } end + + context 'when right value is a regexp string' do + let(:right_value) { '/^ab.*/' } + + context 'when matching' do + let(:left_value) { 'abcde' } + + it { is_expected.to eq(false) } + + context 'when the FF ci_fix_rules_if_comparison_with_regexp_variable is disabled' do + before do + stub_feature_flags(ci_fix_rules_if_comparison_with_regexp_variable: false) + end + + it { is_expected.to eq(true) } + end + end + + context 'when not matching' do + let(:left_value) { 'dfg' } + + it { is_expected.to eq(true) } + end + end end end diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb index fa4f8a20984b5c3ca8fdb1020bc9b516da8020b1..be205395b69400882237780d4ff5e2e6d47a7393 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb @@ -1,8 +1,32 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do + describe '#initialize' do + context 'when the value is a valid regular expression' do + it 'initializes the pattern' do + pattern = described_class.new('/foo/') + + expect(pattern.value).to eq('/foo/') + end + end + + context 'when the value is a valid regular expression with escaped slashes' do + it 'initializes the pattern' do + pattern = described_class.new('/foo\\/bar/') + + expect(pattern.value).to eq('/foo/bar/') + end + end + + context 'when the value is not a valid regular expression' do + it 'raises an error' do + expect { described_class.new('foo') }.to raise_error(Gitlab::Ci::Pipeline::Expression::Lexer::SyntaxError) + end + end + end + describe '.build' do it 'creates a new instance of the token' do expect(described_class.build('/.*/')) @@ -15,6 +39,29 @@ end end + describe '.build_and_evaluate' do + context 'when the value is a valid regular expression' do + it 'returns the value as a Gitlab::UntrustedRegexp' do + expect(described_class.build_and_evaluate('/foo/')) + .to eq(Gitlab::UntrustedRegexp.new('foo')) + end + end + + context 'when the value is a Gitlab::UntrustedRegexp' do + it 'returns the value itself' do + expect(described_class.build_and_evaluate(Gitlab::UntrustedRegexp.new('foo'))) + .to eq(Gitlab::UntrustedRegexp.new('foo')) + end + end + + context 'when the value is not a valid regular expression' do + it 'returns the value itself' do + expect(described_class.build_and_evaluate('foo')) + .to eq('foo') + end + end + end + describe '.type' do it 'is a value lexeme' do expect(described_class.type).to eq :value diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index 84713e2a798ffd5ac729290e7d0bfc1b590c25d0..bbd11a00149a0151d49e7b1718ca03a19f900d6f 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -12,7 +12,7 @@ .to_hash end - subject do + subject(:statement) do described_class.new(text, variables) end @@ -29,6 +29,8 @@ describe '#evaluate' do using RSpec::Parameterized::TableSyntax + subject(:evaluate) { statement.evaluate } + where(:expression, :value) do '$PRESENT_VARIABLE == "my variable"' | true '"my variable" == $PRESENT_VARIABLE' | true @@ -125,7 +127,7 @@ let(:text) { expression } it "evaluates to `#{params[:value].inspect}`" do - expect(subject.evaluate).to eq(value) + expect(evaluate).to eq(value) end end end @@ -133,6 +135,8 @@ describe '#truthful?' do using RSpec::Parameterized::TableSyntax + subject(:truthful?) { statement.truthful? } + where(:expression, :value) do '$PRESENT_VARIABLE == "my variable"' | true "$PRESENT_VARIABLE == 'no match'" | false @@ -151,7 +155,7 @@ let(:text) { expression } it "returns `#{params[:value].inspect}`" do - expect(subject.truthful?).to eq value + expect(truthful?).to eq value end end @@ -159,10 +163,41 @@ let(:text) { '$PRESENT_VARIABLE' } it 'returns false' do - allow(subject).to receive(:evaluate) + allow(statement).to receive(:evaluate) .and_raise(described_class::StatementError) - expect(subject.truthful?).to be_falsey + expect(truthful?).to be_falsey + end + end + + context 'when variables have patterns' do + let(:variables) do + Gitlab::Ci::Variables::Collection.new + .append(key: 'teststring', value: 'abcde') + .append(key: 'pattern1', value: '/^ab.*/') + .append(key: 'pattern2', value: '/^at.*/') + .to_hash + end + + where(:expression, :ff, :result) do + '$teststring =~ "abcde"' | true | true + '$teststring =~ "abcde"' | false | true + '$teststring =~ $teststring' | true | true + '$teststring =~ $teststring' | false | true + '$teststring =~ $pattern1' | true | true + '$teststring =~ $pattern1' | false | false + '$teststring =~ $pattern2' | true | false + '$teststring =~ $pattern2' | false | false + end + + with_them do + let(:text) { expression } + + before do + stub_feature_flags(ci_fix_rules_if_comparison_with_regexp_variable: ff) + end + + it { is_expected.to eq(result) } end end end