Merge branch 'feature/gb/variables-expressions-in-only-except' into 'master'

Pipeline variables expression in only/except configuration

Closes #37397

See merge request gitlab-org/gitlab-ce!17316

10 files changed, 226 insertions, 48 deletions

diff --git a/spec/lib/gitlab/ci/build/policy/variables_spec.rb b/spec/lib/gitlab/ci/build/policy/variables_spec.rb
new file mode 100644
index 00000000000..2ce858836e3
--- /dev/null
+++ b/spec/lib/gitlab/ci/build/policy/variables_spec.rb
@@ -0,0 +1,72 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Build::Policy::Variables do
+  set(:project) { create(:project) }
+
+  let(:pipeline) do
+    build(:ci_empty_pipeline, project: project, ref: 'master', source: :push)
+  end
+
+  let(:ci_build) do
+    build(:ci_build, pipeline: pipeline, project: project, ref: 'master')
+  end
+
+  let(:seed) { double('build seed', to_resource: ci_build) }
+
+  before do
+    pipeline.variables.build(key: 'CI_PROJECT_NAME', value: '')
+  end
+
+  describe '#satisfied_by?' do
+    it 'is satisfied by at least one matching statement' do
+      policy = described_class.new(['$CI_PROJECT_ID', '$UNDEFINED'])
+
+      expect(policy).to be_satisfied_by(pipeline, seed)
+    end
+
+    it 'is not satisfied by an overriden empty variable' do
+      policy = described_class.new(['$CI_PROJECT_NAME'])
+
+      expect(policy).not_to be_satisfied_by(pipeline, seed)
+    end
+
+    it 'is satisfied by a truthy pipeline expression' do
+      policy = described_class.new([%($CI_PIPELINE_SOURCE == "push")])
+
+      expect(policy).to be_satisfied_by(pipeline, seed)
+    end
+
+    it 'is not satisfied by a falsy pipeline expression' do
+      policy = described_class.new([%($CI_PIPELINE_SOURCE == "invalid source")])
+
+      expect(policy).not_to be_satisfied_by(pipeline, seed)
+    end
+
+    it 'is satisfied by a truthy expression using undefined variable' do
+      policy = described_class.new(['$UNDEFINED == null'])
+
+      expect(policy).to be_satisfied_by(pipeline, seed)
+    end
+
+    it 'is not satisfied by a falsy expression using undefined variable' do
+      policy = described_class.new(['$UNDEFINED'])
+
+      expect(policy).not_to be_satisfied_by(pipeline, seed)
+    end
+
+    it 'allows to evaluate regular secret variables' do
+      create(:ci_variable, project: project, key: 'SECRET', value: 'my secret')
+
+      policy = described_class.new(["$SECRET == 'my secret'"])
+
+      expect(policy).to be_satisfied_by(pipeline, seed)
+    end
+
+    it 'does not persist neither pipeline nor build' do
+      described_class.new('$VAR').satisfied_by?(pipeline, seed)
+
+      expect(pipeline).not_to be_persisted
+      expect(seed.to_resource).not_to be_persisted
+    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..08718c382b9 100644
--- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb
@@ -83,6 +83,39 @@ 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/
+      end
+    end
+
+    context 'when specifying invalid variables expressions token' do
+      let(:config) { { variables: ['$MY_VAR == 123'] } }
+
+      it 'reports an error about invalid statement' do
+        expect(entry.errors).to include /invalid expression syntax/
+      end
+    end
+
     context 'when specifying unknown policy' do
       let(:config) { { refs: ['master'], invalid: :something } }
 
diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
index 2258ae83f38..8312fa47cfa 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
@@ -6,7 +6,8 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
 
   let(:pipeline) do
     build(:ci_pipeline_with_one_job, project: project,
-                                     ref: 'master')
+                                     ref: 'master',
+                                     user: user)
   end
 
   let(:command) do
@@ -42,6 +43,10 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
       expect(pipeline.stages.first.builds).to be_one
       expect(pipeline.stages.first.builds.first).not_to be_persisted
     end
+
+    it 'correctly assigns user' do
+      expect(pipeline.builds).to all(have_attributes(user: user))
+    end
   end
 
   context 'when pipeline is empty' do
diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb
index 86234dfb9e5..1ccb792d1da 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb
@@ -73,6 +73,22 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::String do
         expect(token).not_to be_nil
         expect(token.build.evaluate).to eq 'some " string'
       end
+
+      it 'allows to use an empty string inside single quotes' do
+        scanner = StringScanner.new(%(''))
+
+        token = described_class.scan(scanner)
+
+        expect(token.build.evaluate).to eq ''
+      end
+
+      it 'allow to use an empty string inside double quotes' do
+        scanner = StringScanner.new(%(""))
+
+        token = described_class.scan(scanner)
+
+        expect(token.build.evaluate).to eq ''
+      end
     end
   end
 
diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb
index 472a58599d8..6685bf5385b 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb
@@ -1,14 +1,23 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Pipeline::Expression::Statement do
-  let(:pipeline) { build(:ci_pipeline) }
-
   subject do
-    described_class.new(text, pipeline)
+    described_class.new(text, variables)
+  end
+
+  let(:variables) do
+    { 'PRESENT_VARIABLE' => 'my variable',
+      EMPTY_VARIABLE: '' }
   end
 
-  before do
-    pipeline.variables.build([key: 'VARIABLE', value: 'my variable'])
+  describe '.new' do
+    context 'when variables are not provided' do
+      it 'allows to properly initializes the statement' do
+        statement = described_class.new('$PRESENT_VARIABLE')
+
+        expect(statement.evaluate).to be_nil
+      end
+    end
   end
 
   describe '#parse_tree' do
@@ -23,18 +32,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,10 +64,14 @@ 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
-        let(:text) { '$VARIABLE' }
+        let(:text) { '$PRESENT_VARIABLE' }
 
         it 'returns a single token instance' do
           expect(subject.parse_tree)
@@ -62,14 +83,17 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do
 
   describe '#evaluate' do
     statements = [
-      ['$VARIABLE == "my variable"', true],
-      ["$VARIABLE == 'my variable'", true],
-      ['"my variable" == $VARIABLE', true],
-      ['$VARIABLE == null', false],
-      ['$VAR == null', true],
-      ['null == $VAR', true],
-      ['$VARIABLE', 'my variable'],
-      ['$VAR', nil]
+      ['$PRESENT_VARIABLE == "my variable"', true],
+      ["$PRESENT_VARIABLE == 'my variable'", true],
+      ['"my variable" == $PRESENT_VARIABLE', true],
+      ['$PRESENT_VARIABLE == null', false],
+      ['$EMPTY_VARIABLE == null', false],
+      ['"" == $EMPTY_VARIABLE', true],
+      ['$EMPTY_VARIABLE', ''],
+      ['$UNDEFINED_VARIABLE == null', true],
+      ['null == $UNDEFINED_VARIABLE', true],
+      ['$PRESENT_VARIABLE', 'my variable'],
+      ['$UNDEFINED_VARIABLE', nil]
     ]
 
     statements.each do |expression, value|
@@ -82,4 +106,25 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do
       end
     end
   end
+
+  describe '#truthful?' do
+    statements = [
+      ['$PRESENT_VARIABLE == "my variable"', true],
+      ["$PRESENT_VARIABLE == 'no match'", false],
+      ['$UNDEFINED_VARIABLE == null', true],
+      ['$PRESENT_VARIABLE', true],
+      ['$UNDEFINED_VARIABLE', false],
+      ['$EMPTY_VARIABLE', false]
+    ]
+
+    statements.each do |expression, value|
+      context "when using expression `#{expression}`" do
+        let(:text) { expression }
+
+        it "returns `#{value.inspect}`" do
+          expect(subject.truthful?).to eq value
+        end
+      end
+    end
+  end
 end
diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
index 116573379e0..fffa727c2ed 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
@@ -21,16 +21,6 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
     end
   end
 
-  describe '#user=' do
-    let(:user) { build(:user) }
-
-    it 'assignes user to a build' do
-      subject.user = user
-
-      expect(subject.attributes).to include(user: user)
-    end
-  end
-
   describe '#to_resource' do
     it 'returns a valid build resource' do
       expect(subject.to_resource).to be_a(::Ci::Build)
diff --git a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb
index 8f0bf40d624..eb1b285c7bd 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb
@@ -95,16 +95,6 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do
     end
   end
 
-  describe '#user=' do
-    let(:user) { build(:user) }
-
-    it 'assignes relevant pipeline attributes' do
-      subject.user = user
-
-      expect(subject.seeds.map(&:attributes)).to all(include(user: user))
-    end
-  end
-
   describe '#to_resource' do
     it 'builds a valid stage object with all builds' do
       subject.to_resource.save!
diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb
index cc1257484d2..bf9208f1ff4 100644
--- a/spec/lib/gitlab/ci/variables/collection/item_spec.rb
+++ b/spec/lib/gitlab/ci/variables/collection/item_spec.rb
@@ -46,9 +46,13 @@ describe Gitlab::Ci::Variables::Collection::Item do
     end
   end
 
-  describe '#to_hash' do
-    it 'returns a hash representation of a collection item' do
-      expect(described_class.new(**variable).to_hash).to eq variable
+  describe '#to_runner_variable' do
+    it 'returns a runner-compatible hash representation' do
+      runner_variable = described_class
+        .new(**variable)
+        .to_runner_variable
+
+      expect(runner_variable).to eq variable
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/variables/collection_spec.rb b/spec/lib/gitlab/ci/variables/collection_spec.rb
index 90b6e178242..cb2f7718c9c 100644
--- a/spec/lib/gitlab/ci/variables/collection_spec.rb
+++ b/spec/lib/gitlab/ci/variables/collection_spec.rb
@@ -7,7 +7,7 @@ describe Gitlab::Ci::Variables::Collection do
 
       collection = described_class.new([variable])
 
-      expect(collection.first.to_hash).to eq variable
+      expect(collection.first.to_runner_variable).to eq variable
     end
 
     it 'can be initialized without an argument' do
@@ -96,4 +96,19 @@ describe Gitlab::Ci::Variables::Collection do
         .to eq [{ key: 'TEST', value: 1, public: true }]
     end
   end
+
+  describe '#to_hash' do
+    it 'returns regular hash in valid order without duplicates' do
+      collection = described_class.new
+        .append(key: 'TEST1', value: 'test-1')
+        .append(key: 'TEST2', value: 'test-2')
+        .append(key: 'TEST1', value: 'test-3')
+
+      expect(collection.to_hash).to eq('TEST1' => 'test-3',
+                                       'TEST2' => 'test-2')
+
+      expect(collection.to_hash).to include(TEST1: 'test-3')
+      expect(collection.to_hash).not_to include(TEST1: 'test-1')
+    end
+  end
 end
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index fbc2af29b98..ecb16daec96 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -1311,6 +1311,14 @@ module Gitlab
             Gitlab::Ci::YamlProcessor.new(config)
           end.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, "jobs:rspec dependencies should be an array of strings")
         end
+
+        it 'returns errors if pipeline variables expression is invalid' do
+          config = YAML.dump({ rspec: { script: 'test', only: { variables: ['== null'] } } })
+
+          expect { Gitlab::Ci::YamlProcessor.new(config) }
+            .to raise_error(Gitlab::Ci::YamlProcessor::ValidationError,
+                            'jobs:rspec:only variables invalid expression syntax')
+        end
       end
 
       describe "Validate configuration templates" do