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 @@ -1978,6 +1978,27 @@ describe Ci::Build do end end + 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 '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( @@ -1993,9 +2014,8 @@ describe Ci::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