diff --git a/app/services/ci/create_web_ide_terminal_service.rb b/app/services/ci/create_web_ide_terminal_service.rb index 785d82094b9d518f89c3362baa0712ecf11257b9..3b89a599180edc8f57a3b2f8ad1096748593215b 100644 --- a/app/services/ci/create_web_ide_terminal_service.rb +++ b/app/services/ci/create_web_ide_terminal_service.rb @@ -58,7 +58,8 @@ def terminal_stage_seed(pipeline) builds: [terminal_build_seed] } - Gitlab::Ci::Pipeline::Seed::Stage.new(pipeline, attributes, []) + seed_context = Gitlab::Ci::Pipeline::Seed::Context.new(pipeline) + Gitlab::Ci::Pipeline::Seed::Stage.new(seed_context, attributes, []) end def terminal_build_seed diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index 815fe6bac6de956addcd8ee00d60c1f5331045b4..46ecb10ea2b01b2bb5680e20e590a8895f0dd0e7 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -12,7 +12,7 @@ module Chain :seeds_block, :variables_attributes, :push_options, :chat_data, :allow_mirror_update, :bridge, :content, :dry_run, # These attributes are set by Chains during processing: - :config_content, :yaml_processor_result, :pipeline_seed + :config_content, :yaml_processor_result, :workflow_rules_result, :pipeline_seed ) do include Gitlab::Utils::StrongMemoize diff --git a/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb b/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb index 3c910963a2ada5e549d5a70f81e8af88244c4ba9..cceaa52de16d432a35244eeeeaff4bd3d22666f8 100644 --- a/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb +++ b/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb @@ -9,6 +9,8 @@ class EvaluateWorkflowRules < Chain::Base include Chain::Helpers def perform! + @command.workflow_rules_result = workflow_rules_result + error('Pipeline filtered out by workflow rules.') unless workflow_passed? end @@ -19,27 +21,33 @@ def break? private def workflow_passed? - strong_memoize(:workflow_passed) do - workflow_rules.evaluate(@pipeline, global_context).pass? + workflow_rules_result.pass? + end + + def workflow_rules_result + strong_memoize(:workflow_rules_result) do + workflow_rules.evaluate(@pipeline, global_context) end end def workflow_rules Gitlab::Ci::Build::Rules.new( - workflow_config[:rules], default_when: 'always') + workflow_rules_config, default_when: 'always') end def global_context Gitlab::Ci::Build::Context::Global.new( - @pipeline, yaml_variables: workflow_config[:yaml_variables]) + @pipeline, yaml_variables: @command.yaml_processor_result.root_variables) end def has_workflow_rules? - workflow_config[:rules].present? + workflow_rules_config.present? end - def workflow_config - @command.yaml_processor_result.workflow_attributes || {} + def workflow_rules_config + strong_memoize(:workflow_rules_config) do + @command.yaml_processor_result.workflow_rules + end end end end diff --git a/lib/gitlab/ci/pipeline/chain/seed.rb b/lib/gitlab/ci/pipeline/chain/seed.rb index 7b537125b9bd9479e4633ba3fd7d57c74756c6ba..46524e8999f64837543c167322efef97505420f1 100644 --- a/lib/gitlab/ci/pipeline/chain/seed.rb +++ b/lib/gitlab/ci/pipeline/chain/seed.rb @@ -38,9 +38,17 @@ def break? def pipeline_seed strong_memoize(:pipeline_seed) do stages_attributes = @command.yaml_processor_result.stages_attributes - Gitlab::Ci::Pipeline::Seed::Pipeline.new(pipeline, stages_attributes) + Gitlab::Ci::Pipeline::Seed::Pipeline.new(context, stages_attributes) end end + + def context + Gitlab::Ci::Pipeline::Seed::Context.new(pipeline, root_variables: root_variables) + end + + def root_variables + @command.yaml_processor_result.root_variables + end end end end diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index 11b01822e4b6859b14a43eb3bf58b56d77491ab0..b95a23d226330fc14bf30a73ddb05da48cc9fb47 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -11,8 +11,9 @@ class Build < Seed::Base delegate :dig, to: :@seed_attributes - def initialize(pipeline, attributes, previous_stages) - @pipeline = pipeline + def initialize(context, attributes, previous_stages) + @context = context + @pipeline = context.pipeline @seed_attributes = attributes @previous_stages = previous_stages @needs_attributes = dig(:needs_attributes) @@ -29,7 +30,7 @@ def initialize(pipeline, attributes, previous_stages) @rules = Gitlab::Ci::Build::Rules .new(attributes.delete(:rules), default_when: 'on_success') @cache = Gitlab::Ci::Build::Cache - .new(attributes.delete(:cache), pipeline) + .new(attributes.delete(:cache), @pipeline) end def name diff --git a/lib/gitlab/ci/pipeline/seed/context.rb b/lib/gitlab/ci/pipeline/seed/context.rb new file mode 100644 index 0000000000000000000000000000000000000000..6194a78f682dde6113746a48e9ece6a866376a0c --- /dev/null +++ b/lib/gitlab/ci/pipeline/seed/context.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Pipeline + module Seed + class Context + attr_reader :pipeline, :root_variables + + def initialize(pipeline, root_variables: []) + @pipeline = pipeline + @root_variables = root_variables + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/seed/pipeline.rb b/lib/gitlab/ci/pipeline/seed/pipeline.rb index da9d853cf68c01ad824e59272178d565c193cab6..e1a15fb8d5bc1aa906dbc2f320acc24c3596abae 100644 --- a/lib/gitlab/ci/pipeline/seed/pipeline.rb +++ b/lib/gitlab/ci/pipeline/seed/pipeline.rb @@ -7,8 +7,8 @@ module Seed class Pipeline include Gitlab::Utils::StrongMemoize - def initialize(pipeline, stages_attributes) - @pipeline = pipeline + def initialize(context, stages_attributes) + @context = context @stages_attributes = stages_attributes end @@ -37,7 +37,7 @@ def deployments_count def stage_seeds strong_memoize(:stage_seeds) do seeds = @stages_attributes.inject([]) do |previous_stages, attributes| - seed = Gitlab::Ci::Pipeline::Seed::Stage.new(@pipeline, attributes, previous_stages) + seed = Gitlab::Ci::Pipeline::Seed::Stage.new(@context, attributes, previous_stages) previous_stages + [seed] end diff --git a/lib/gitlab/ci/pipeline/seed/stage.rb b/lib/gitlab/ci/pipeline/seed/stage.rb index b600df2f656834afd15ae71d151148b0a97eb488..c988ea10e416f282c5dd72a93a543a46220e1e69 100644 --- a/lib/gitlab/ci/pipeline/seed/stage.rb +++ b/lib/gitlab/ci/pipeline/seed/stage.rb @@ -10,13 +10,14 @@ class Stage < Seed::Base delegate :size, to: :seeds delegate :dig, to: :seeds - def initialize(pipeline, attributes, previous_stages) - @pipeline = pipeline + def initialize(context, attributes, previous_stages) + @context = context + @pipeline = context.pipeline @attributes = attributes @previous_stages = previous_stages @builds = attributes.fetch(:builds).map do |attributes| - Seed::Build.new(@pipeline, attributes, previous_stages) + Seed::Build.new(context, attributes, previous_stages) end end diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index f2c752270021444b3d1cea631ca972e1161417cb..c266dfd75d37858287e2e4fe32c85632d2cf0f88 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -38,11 +38,12 @@ def stage_builds_attributes(stage) .map { |job| build_attributes(job[:name]) } end - def workflow_attributes - { - rules: hash_config.dig(:workflow, :rules), - yaml_variables: transform_to_yaml_variables(variables) - } + def workflow_rules + @workflow_rules ||= hash_config.dig(:workflow, :rules) + end + + def root_variables + @root_variables ||= transform_to_yaml_variables(variables) end def jobs diff --git a/spec/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb index 4ae51ac8bf9c121f8bc6f4ea540d82cfa78f2eab..e30a78546afea5449c3495fb440cf499bc1689bf 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb @@ -16,8 +16,10 @@ describe '#perform!' do context 'when pipeline has been skipped by workflow configuration' do before do - allow(step).to receive(:workflow_passed?) - .and_return(false) + allow(step).to receive(:workflow_rules_result) + .and_return( + double(pass?: false, variables: {}) + ) step.perform! end @@ -33,12 +35,18 @@ it 'attaches an error to the pipeline' do expect(pipeline.errors[:base]).to include('Pipeline filtered out by workflow rules.') end + + it 'saves workflow_rules_result' do + expect(command.workflow_rules_result.variables).to eq({}) + end end context 'when pipeline has not been skipped by workflow configuration' do before do - allow(step).to receive(:workflow_passed?) - .and_return(true) + allow(step).to receive(:workflow_rules_result) + .and_return( + double(pass?: true, variables: { 'VAR1' => 'val2' }) + ) step.perform! end @@ -55,6 +63,10 @@ it 'attaches no errors' do expect(pipeline.errors).to be_empty end + + it 'saves workflow_rules_result' do + expect(command.workflow_rules_result.variables).to eq({ 'VAR1' => 'val2' }) + end end end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index 5506b079d0f78002cecebaf977a12b227e93d2ef..53d8ca740b506a6b1814468549d89b5ac3898bc6 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -22,6 +22,7 @@ [ Gitlab::Ci::Pipeline::Chain::Config::Content.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::Config::Process.new(pipeline, command), + Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::SeedBlock.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::Seed.new(pipeline, command) ] diff --git a/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb index 80013cab6ee3c4246aa1572e76410621536fd2b5..1326b3b0fb5359f7a452f4ae268bb6da3760e7e5 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb @@ -20,7 +20,6 @@ describe '#perform!' do before do stub_ci_pipeline_yaml_file(YAML.dump(config)) - run_chain end let(:config) do @@ -30,21 +29,28 @@ subject(:run_chain) do [ Gitlab::Ci::Pipeline::Chain::Config::Content.new(pipeline, command), - Gitlab::Ci::Pipeline::Chain::Config::Process.new(pipeline, command) + Gitlab::Ci::Pipeline::Chain::Config::Process.new(pipeline, command), + Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules.new(pipeline, command) ].map(&:perform!) described_class.new(pipeline, command).perform! end it 'allocates next IID' do + run_chain + expect(pipeline.iid).to be_present end it 'ensures ci_ref' do + run_chain + expect(pipeline.ci_ref).to be_present end it 'sets the seeds in the command object' do + run_chain + expect(command.pipeline_seed).to be_a(Gitlab::Ci::Pipeline::Seed::Pipeline) expect(command.pipeline_seed.size).to eq 1 end @@ -59,6 +65,8 @@ end it 'correctly fabricates stages and builds' do + run_chain + seed = command.pipeline_seed expect(seed.stages.size).to eq 2 @@ -84,6 +92,8 @@ end it 'returns pipeline seed with jobs only assigned to master' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 1 @@ -103,6 +113,8 @@ end it 'returns pipeline seed with jobs only assigned to schedules' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 1 @@ -130,6 +142,8 @@ let(:pipeline) { build(:ci_pipeline, project: project) } it 'returns seeds for kubernetes dependent job' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 2 @@ -141,6 +155,8 @@ context 'when kubernetes is not active' do it 'does not return seeds for kubernetes dependent job' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 1 @@ -158,6 +174,8 @@ end it 'returns stage seeds only when variables expression is truthy' do + run_chain + seed = command.pipeline_seed expect(seed.size).to eq 1 @@ -171,6 +189,8 @@ end it 'does not execute the block' do + run_chain + expect(pipeline.variables.size).to eq(0) 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 7ec6949f852e4f2100760dbdcc65259ee75d9678..10f2f42e268a8a21cb8eac7053e572d10993f316 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -6,10 +6,12 @@ let_it_be(:project) { create(:project, :repository) } let_it_be(:head_sha) { project.repository.head_commit.id } let(:pipeline) { build(:ci_empty_pipeline, project: project, sha: head_sha) } + let(:root_variables) { [] } + let(:seed_context) { double(pipeline: pipeline, root_variables: root_variables) } let(:attributes) { { name: 'rspec', ref: 'master', scheduling_type: :stage } } let(:previous_stages) { [] } - let(:seed_build) { described_class.new(pipeline, attributes, previous_stages) } + let(:seed_build) { described_class.new(seed_context, attributes, previous_stages) } describe '#attributes' do subject { seed_build.attributes } @@ -301,6 +303,26 @@ it { is_expected.to match a_hash_including(options: { allow_failure_criteria: nil }) } end end + + context 'when the job rule depends on variables' do + let(:attributes) do + { name: 'rspec', + ref: 'master', + yaml_variables: [{ key: 'VAR1', value: 'var 1', public: true }], + rules: rules } + end + + context 'when the rules use job variables' do + let(:rules) do + [{ if: '$VAR1 == "var 1"', variables: { VAR1: 'overridden var 1', VAR2: 'new var 2' } }] + end + + it 'recalculates the variables' do + expect(subject[:yaml_variables]).to contain_exactly({ key: 'VAR1', value: 'overridden var 1', public: true }, + { key: 'VAR2', value: 'new var 2', public: true }) + end + end + end end describe '#bridge?' do @@ -377,7 +399,7 @@ it 'does not have environment' do expect(subject).not_to be_has_environment expect(subject.environment).to be_nil - expect(subject.metadata).to be_nil + expect(subject.metadata&.expanded_environment_name).to be_nil expect(Environment.exists?(name: expected_environment_name)).to eq(false) end end @@ -1080,7 +1102,7 @@ end let(:stage_seed) do - Gitlab::Ci::Pipeline::Seed::Stage.new(pipeline, stage_attributes, []) + Gitlab::Ci::Pipeline::Seed::Stage.new(seed_context, stage_attributes, []) end let(:previous_stages) { [stage_seed] } diff --git a/spec/lib/gitlab/ci/pipeline/seed/pipeline_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/pipeline_spec.rb index 860b07647bdbdd077d11fad247d6e8605110055f..21be8660def00a10f146b866dfa2ac8af878d794 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/pipeline_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/pipeline_spec.rb @@ -6,6 +6,8 @@ let_it_be(:project) { create(:project, :repository) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let(:seed_context) { double(pipeline: pipeline, root_variables: []) } + let(:stages_attributes) do [ { @@ -29,7 +31,7 @@ end subject(:seed) do - described_class.new(pipeline, stages_attributes) + described_class.new(seed_context, stages_attributes) end describe '#stages' do diff --git a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb index 4b9db9fa6c6793050983b8ac15f840ec93400010..5b04d2abd88d4a966ca81abf000167ae2e687048 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb @@ -6,6 +6,7 @@ let(:project) { create(:project, :repository) } let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:previous_stages) { [] } + let(:seed_context) { double(pipeline: pipeline, root_variables: []) } let(:attributes) do { name: 'test', @@ -16,7 +17,7 @@ end subject do - described_class.new(pipeline, attributes, previous_stages) + described_class.new(seed_context, attributes, previous_stages) end describe '#size' do diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 5462a587d16908af4255febe5739b6d0e3acbce7..4f45eb2c9859054a2ed3cb2da5a0e463e5b81688 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -372,7 +372,7 @@ module Ci end end - describe '#workflow_attributes' do + describe 'workflow attributes' do context 'with disallowed workflow:variables' do let(:config) do <<-EOYML @@ -403,11 +403,11 @@ module Ci end it 'parses the workflow:rules configuration' do - expect(subject.workflow_attributes[:rules]).to contain_exactly({ if: '$VAR == "value"' }) + expect(subject.workflow_rules).to contain_exactly({ if: '$VAR == "value"' }) end - it 'parses the root:variables as yaml_variables:' do - expect(subject.workflow_attributes[:yaml_variables]) + it 'parses the root:variables as #root_variables' do + expect(subject.root_variables) .to contain_exactly({ key: 'SUPPORTED', value: 'parsed', public: true }) end end @@ -425,11 +425,11 @@ module Ci end it 'parses the workflow:rules configuration' do - expect(subject.workflow_attributes[:rules]).to contain_exactly({ if: '$VAR == "value"' }) + expect(subject.workflow_rules).to contain_exactly({ if: '$VAR == "value"' }) end - it 'parses the root:variables as yaml_variables:' do - expect(subject.workflow_attributes[:yaml_variables]).to eq([]) + it 'parses the root:variables as #root_variables' do + expect(subject.root_variables).to eq([]) end end @@ -445,11 +445,11 @@ module Ci end it 'parses the workflow:rules configuration' do - expect(subject.workflow_attributes[:rules]).to be_nil + expect(subject.workflow_rules).to be_nil end - it 'parses the root:variables as yaml_variables:' do - expect(subject.workflow_attributes[:yaml_variables]) + it 'parses the root:variables as #root_variables' do + expect(subject.root_variables) .to contain_exactly({ key: 'SUPPORTED', value: 'parsed', public: true }) end end @@ -463,11 +463,11 @@ module Ci end it 'parses the workflow:rules configuration' do - expect(subject.workflow_attributes[:rules]).to be_nil + expect(subject.workflow_rules).to be_nil end - it 'parses the root:variables as yaml_variables:' do - expect(subject.workflow_attributes[:yaml_variables]).to eq([]) + it 'parses the root:variables as #root_variables' do + expect(subject.root_variables).to eq([]) end end end diff --git a/spec/services/ci/create_web_ide_terminal_service_spec.rb b/spec/services/ci/create_web_ide_terminal_service_spec.rb index c1c94e30018361429903ac0f5a5bf036bef97442..c1acf8fd60cef104f6ba4c28b31a7d7f5c70d67a 100644 --- a/spec/services/ci/create_web_ide_terminal_service_spec.rb +++ b/spec/services/ci/create_web_ide_terminal_service_spec.rb @@ -66,6 +66,25 @@ it_behaves_like 'be successful' end + + context 'for configuration with variables' do + let(:config_content) do + <<-EOS + terminal: + script: rspec + variables: + KEY1: VAL1 + EOS + end + + it_behaves_like 'be successful' + + it 'saves the variables' do + expect(subject[:pipeline].builds[0].variables).to include( + key: 'KEY1', value: 'VAL1', public: true, masked: false + ) + end + end end end