Expose evaluable variables method from build object

4 files changed, 49 insertions, 13 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index fa21b560736..cc277fb5266 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -244,13 +244,24 @@ module Ci
       Gitlab::Utils.slugify(ref.to_s)
     end
 
+    ##
     # Variables whose value does not depend on environment
+    #
     def simple_variables
       variables(environment: nil)
     end
 
-    # All variables, including those dependent on environment, which could
-    # contain unexpanded variables.
+    ##
+    # Variables that are available for evaluation using variables policy.
+    #
+    def evaluable_variables
+      Gitlab::Ci::Variables::Collection.new
+        .concat(simple_variables)
+    end
+
+    ## All variables, including those dependent on environment, which could
+    #  contain unexpanded variables.
+    #
     def variables(environment: persisted_environment)
       collection = Gitlab::Ci::Variables::Collection.new.tap do |variables|
         variables.concat(predefined_variables)
diff --git a/lib/gitlab/ci/build/policy/variables.rb b/lib/gitlab/ci/build/policy/variables.rb
index 2c7d8aba43b..a22e9a1a80e 100644
--- a/lib/gitlab/ci/build/policy/variables.rb
+++ b/lib/gitlab/ci/build/policy/variables.rb
@@ -8,9 +8,8 @@ module Gitlab
           end
 
           def satisfied_by?(pipeline, seed)
-            variables = Gitlab::Ci::Variables::Collection
-              .new(seed.to_resource.simple_variables)
-              .to_hash
+            variables = seed.to_resource
+              .evaluable_variables.to_hash
 
             statements = @expressions.map do |statement|
               ::Gitlab::Ci::Pipeline::Expression::Statement
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/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index c3061940fd7..6081ef04fb7 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1989,13 +1989,33 @@ describe Ci::Build do
         )
       end
 
+      it 'returns static predefined variables' do
+        expect(build.variables.size).to be >= 28
+        expect(build.variables)
+          .to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true)
+        expect(build).not_to be_persisted
+      end
+    end
+  end
+
+  describe '#evaluable_variables' do
+    context 'when build has not been persisted yet' do
+      let(:build) do
+        described_class.new(
+          name: 'rspec',
+          stage: 'test',
+          ref: 'feature',
+          project: project,
+          pipeline: pipeline
+        )
+      end
+
       it 'does not persist the build' do
         expect(build).to be_valid
         expect(build).not_to be_persisted
 
-        variables = build.variables
+        variables = build.evaluable_variables
 
-        expect(variables.size).to be >= 28
         expect(build).not_to be_persisted
       end
 
@@ -2006,13 +2026,14 @@ describe Ci::Build do
                   CI_COMMIT_REF_SLUG
                   CI_JOB_STAGE]
 
-        build.variables.map { |var| var.fetch(:key) }.tap do |names|
+        variables = build.evaluable_variables
+
+        variables.map { |env| env[:key] }.tap do |names|
           expect(names).to include(*keys)
         end
 
-        expect(build.variables).to include(key: 'CI_COMMIT_REF_NAME',
-                                           value: 'feature',
-                                           public: true)
+        expect(variables)
+          .to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true)
       end
 
       it 'does not return prohibited variables' do
@@ -2025,7 +2046,7 @@ describe Ci::Build do
                   CI_REPOSITORY_URL
                   CI_ENVIRONMENT_URL]
 
-        build.variables.map { |var| var.fetch(:key) }.tap do |names|
+        build.evaluable_variables.map { |env| env[:key] }.tap do |names|
           expect(names).not_to include(*keys)
         end
       end