From 590e183fe2c35ec45861f5b480aba1f5885b8d46 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Sat, 16 Jan 2021 20:33:27 +0300 Subject: [PATCH 1/3] Implement variables for pipeline workflow rules With this, variables can be defined according to the conditions. TODO: feature flag --- app/models/ci/pipeline.rb | 6 ++ doc/ci/yaml/README.md | 49 +++++++++++++++ lib/gitlab/ci/build/rules.rb | 10 ++- lib/gitlab/ci/config/entry/processable.rb | 17 +++++- .../ci/pipeline/chain/config/process.rb | 1 + .../pipeline/chain/evaluate_workflow_rules.rb | 22 ++++++- lib/gitlab/ci/pipeline/seed/build.rb | 29 ++++++++- lib/gitlab/ci/variables/collection/item.rb | 2 +- lib/gitlab/ci/yaml_processor/result.rb | 2 +- spec/lib/gitlab/ci/build/rules_spec.rb | 61 +++++++++++++++---- .../ci/config/entry/processable_spec.rb | 49 ++++----------- .../chain/evaluate_workflow_rules_spec.rb | 45 +++++++++++++- .../lib/gitlab/ci/pipeline/seed/build_spec.rb | 36 +++++++++++ .../ci/create_pipeline_service/rules_spec.rb | 38 +++++++++--- 14 files changed, 299 insertions(+), 68 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 4a579892e3fc30..8fa61b041ac5ce 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -37,6 +37,12 @@ class Pipeline < ApplicationRecord # https://gitlab.com/gitlab-org/gitlab/-/issues/259010 attr_accessor :merged_yaml + # This attribute is firstly initialized in Gitlab::Ci::Pipeline::Chain::Config::Process + # with the value of variables defined in CI config file. + # Then it is overridden in Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules according to + # the workflow-rules result. + attr_accessor :yaml_variables + belongs_to :project, inverse_of: :all_pipelines belongs_to :user belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline' diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 19b8f0f1c89e48..a273d13cea77a8 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -248,6 +248,7 @@ a preconfigured `workflow: rules` entry. - [`when`](#when): Specify what to do when the `if` rule evaluates to true. - To run a pipeline, set to `always`. - To prevent pipelines from running, set to `never`. +- [`variables`](#workflowrulesvariables): If not defined, uses the [variables defined elsewhere](#variables). When no rules evaluate to true, the pipeline does not run. @@ -335,6 +336,54 @@ include: - template: 'Workflows/MergeRequest-Pipelines.gitlab-ci.yml' ``` +#### `workflow:rules:variables` + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/294232) in GitLab 13.8. +> - It was [deployed behind a feature flag](../../user/feature_flags.md), disabled by default. +> - It's disabled on GitLab.com. +> - It's not recommended for production use. +> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-workflowrulesvariables). **(CORE ONLY)** + +WARNING: +This feature might not be available to you. Check the **version history** note above for details. + +You can use [`variables`](#variables) in `workflow:rules:` to define variables for specific conditions. + +For example: + +```yaml +variables: + DEPLOY_VARIABLE: "default-deploy" + +workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + DEPLOY_VARIABLE: "deploy-production" # Override DEPLOY_VARIABLE defined + - if: $CI_COMMIT_REF_NAME =~ /feature/ + variables: + IS_A_FEATURE: "true" # Define a new variable. +``` + +##### Enable or disable workflow:rules:variables **(CORE ONLY)** + +rules:variables is under development and not ready for production use. +It is deployed behind a feature flag that is **disabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) +can enable it. + +To enable it: + +```ruby +Feature.enable(:TODO) +``` + +To disable it: + +```ruby +Feature.disable(:TODO) +``` + ### `include` > - Introduced in [GitLab Premium](https://about.gitlab.com/pricing/) 10.5. diff --git a/lib/gitlab/ci/build/rules.rb b/lib/gitlab/ci/build/rules.rb index a39afee194c50d..6538b06f7b44dc 100644 --- a/lib/gitlab/ci/build/rules.rb +++ b/lib/gitlab/ci/build/rules.rb @@ -12,7 +12,13 @@ def build_attributes(seed_attributes = {}) when: self.when, options: { start_in: start_in }.compact, allow_failure: allow_failure, - yaml_variables: yaml_variables(seed_attributes[:yaml_variables]) + yaml_variables: calculate_variables(seed_attributes[:yaml_variables]) + }.compact + end + + def pipeline_attributes(seed_attributes = {}) + { + yaml_variables: calculate_variables(seed_attributes[:yaml_variables]) }.compact end @@ -22,7 +28,7 @@ def pass? private - def yaml_variables(seed_variables) + def calculate_variables(seed_variables) return unless variables && seed_variables indexed_seed_variables = seed_variables.deep_dup.index_by { |var| var[:key] } diff --git a/lib/gitlab/ci/config/entry/processable.rb b/lib/gitlab/ci/config/entry/processable.rb index 5ef8cfbddb7667..6c44e4c307ed0c 100644 --- a/lib/gitlab/ci/config/entry/processable.rb +++ b/lib/gitlab/ci/config/entry/processable.rb @@ -123,7 +123,8 @@ def value stage: stage_value, extends: extends, rules: rules_value, - variables: root_and_job_variables_value, + variables: root_and_job_variables_value.to_h, + variables_with_detail: variables_with_detail, only: only_value, except: except_value }.compact end @@ -137,6 +138,20 @@ def root_and_job_variables_value root_variables.merge(variables_value.to_h) end + def variables_with_detail + root_variables = @root_variables_value.to_h.map do |key, value| + next unless inherit_entry&.variables_entry&.inherit?(key) + + [key, { key: key, value: value, public: true, source: 'workflow' }] + end.to_h + + job_variables = variables_value.to_h.map do |key, value| + [key, { key: key, value: value, public: true, source: 'job' }] + end.to_h + + root_variables.merge(job_variables).values + end + def manual_action? self.when == 'manual' end diff --git a/lib/gitlab/ci/pipeline/chain/config/process.rb b/lib/gitlab/ci/pipeline/chain/config/process.rb index c3fbd0c9e2435e..9b8ba5a27a4e2b 100644 --- a/lib/gitlab/ci/pipeline/chain/config/process.rb +++ b/lib/gitlab/ci/pipeline/chain/config/process.rb @@ -29,6 +29,7 @@ def perform! end @pipeline.merged_yaml = result.merged_yaml + @pipeline.yaml_variables = result.workflow_attributes[:yaml_variables] rescue => ex Gitlab::ErrorTracking.track_exception(ex, diff --git a/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb b/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb index 3c910963a2ada5..707c77136765e6 100644 --- a/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb +++ b/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb @@ -9,7 +9,11 @@ class EvaluateWorkflowRules < Chain::Base include Chain::Helpers def perform! - error('Pipeline filtered out by workflow rules.') unless workflow_passed? + if workflow_passed? + apply_rules + else + error('Pipeline filtered out by workflow rules.') + end end def break? @@ -18,9 +22,21 @@ def break? private + def apply_rules + @pipeline.assign_attributes(rules_attributes) + end + + def rules_attributes + workflow_rules_result.pipeline_attributes(yaml_variables: @pipeline.yaml_variables) + end + 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 diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index fe3c2bca5511bb..d3e211a51f4e80 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -13,7 +13,7 @@ class Build < Seed::Base def initialize(pipeline, attributes, previous_stages) @pipeline = pipeline - @seed_attributes = attributes + @seed_attributes = build_seed_attributes(attributes) @previous_stages = previous_stages @needs_attributes = dig(:needs_attributes) @resource_group_key = attributes.delete(:resource_group_key) @@ -194,6 +194,33 @@ def allow_failure_criteria_attributes { options: { allow_failure_criteria: nil } } end + + def build_seed_attributes(attributes) + assign_pipeline_yaml_variables(attributes) + attributes + end + + def assign_pipeline_yaml_variables(attributes) + @pipeline.yaml_variables ||= [] + attributes[:yaml_variables] ||= [] + + indexed_job_vars = attributes[:yaml_variables] + .select { |var| var[:source] == 'job' || var[:source].nil? } + .index_by { |var| var[:key] } + indexed_workflow_vars = attributes[:yaml_variables] + .select { |var| var[:source] == 'workflow' } + .index_by { |var| var[:key] } + indexed_ruled_workflow_vars = @pipeline.yaml_variables + .index_by { |var| var[:key] } + + attributes[:yaml_variables] = indexed_workflow_vars + .merge(indexed_ruled_workflow_vars) + .merge(indexed_job_vars) + .values + .map do |var| + { key: var[:key], value: var[:value], public: var[:public] } + end + end end end end diff --git a/lib/gitlab/ci/variables/collection/item.rb b/lib/gitlab/ci/variables/collection/item.rb index 84a9280e5071b8..8bf58a9f6c3c08 100644 --- a/lib/gitlab/ci/variables/collection/item.rb +++ b/lib/gitlab/ci/variables/collection/item.rb @@ -5,7 +5,7 @@ module Ci module Variables class Collection class Item - def initialize(key:, value:, public: true, file: false, masked: false) + def initialize(key:, value:, public: true, file: false, masked: false, source: nil) raise ArgumentError, "`#{key}` must be of type String or nil value, while it was: #{value.class}" unless value.is_a?(String) || value.nil? diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index 86749cda9c7d8e..7ab59573fd5f98 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -68,7 +68,7 @@ def build_attributes(name) when: job[:when] || 'on_success', environment: job[:environment_name], coverage_regex: job[:coverage], - yaml_variables: transform_to_yaml_variables(job[:variables]), + yaml_variables: job[:variables_with_detail], needs_attributes: job.dig(:needs, :job), interruptible: job[:interruptible], only: job[:only], diff --git a/spec/lib/gitlab/ci/build/rules_spec.rb b/spec/lib/gitlab/ci/build/rules_spec.rb index a1af5b75f87d20..9b3ff5fdc79ba0 100644 --- a/spec/lib/gitlab/ci/build/rules_spec.rb +++ b/spec/lib/gitlab/ci/build/rules_spec.rb @@ -200,17 +200,7 @@ Gitlab::Ci::Build::Rules::Result.new(when_value, start_in, allow_failure, variables) end - describe '#build_attributes' do - let(:seed_attributes) { {} } - - subject(:build_attributes) do - result.build_attributes(seed_attributes) - end - - it 'compacts nil values' do - is_expected.to eq(options: {}, when: 'on_success') - end - + shared_examples 'yaml variables calculation' do context 'when there are variables in rules' do let(:variables) { { VAR1: 'new var 1', VAR3: 'var 3' } } @@ -235,6 +225,55 @@ end end end + + context 'when there no variable in rules' do + let(:variables) { } + + context 'when there are seed variables' do + let(:seed_attributes) do + { yaml_variables: [{ key: 'VAR1', value: 'var 1', public: true }, + { key: 'VAR2', value: 'var 2', public: true }] } + end + + it 'does not return yaml_variables' do + is_expected.not_to have_key(:yaml_variables) + end + end + + context 'when there is not seed variables' do + it 'does not return yaml_variables' do + is_expected.not_to have_key(:yaml_variables) + end + end + end + end + + describe '#build_attributes' do + let(:seed_attributes) { {} } + + subject(:build_attributes) do + result.build_attributes(seed_attributes) + end + + it 'compacts nil values' do + is_expected.to eq(options: {}, when: 'on_success') + end + + it_behaves_like 'yaml variables calculation' + end + + describe '#pipeline_attributes' do + let(:seed_attributes) { {} } + + subject(:pipeline_attributes) do + result.pipeline_attributes(seed_attributes) + end + + it 'compacts nil values' do + is_expected.to eq({}) + end + + it_behaves_like 'yaml variables calculation' end describe '#pass?' do diff --git a/spec/lib/gitlab/ci/config/entry/processable_spec.rb b/spec/lib/gitlab/ci/config/entry/processable_spec.rb index aadf94365c6c68..5cc093be52d001 100644 --- a/spec/lib/gitlab/ci/config/entry/processable_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/processable_spec.rb @@ -358,47 +358,21 @@ def self.name end end - context 'when root yaml variables are used' do - let(:variables) do - Gitlab::Ci::Config::Entry::Variables.new( - { A: 'root', C: 'root', D: 'root' } - ).value - end + context 'when deps has variables' do + let(:variables) { { 'A' => 'root', 'C' => 'root' } } - it 'does return all variables and overwrite them' do + it 'returns total variables' do expect(entry.value).to include( - variables: { 'A' => 'job', 'B' => 'job', 'C' => 'root', 'D' => 'root' } + variables: { 'A' => 'job', 'B' => 'job', 'C' => 'root' } ) end - context 'when inherit of defaults is disabled' do - let(:config) do - { - variables: { A: 'job', B: 'job' }, - inherit: { variables: false } - } - end - - it 'does return only job variables' do - expect(entry.value).to include( - variables: { 'A' => 'job', 'B' => 'job' } - ) - end - end - - context 'when inherit of only specific variable is enabled' do - let(:config) do - { - variables: { A: 'job', B: 'job' }, - inherit: { variables: ['D'] } - } - end - - it 'does return only job variables' do - expect(entry.value).to include( - variables: { 'A' => 'job', 'B' => 'job', 'D' => 'root' } - ) - end + it 'returns total variables with detail' do + expect(entry.value[:variables_with_detail]).to match_array( + [{ key: 'A', value: 'job', public: true, source: 'job' }, + { key: 'B', value: 'job', public: true, source: 'job' }, + { key: 'C', value: 'root', public: true, source: 'workflow' }] + ) end end end @@ -464,7 +438,8 @@ def self.name name: :rspec, stage: 'test', only: { refs: %w[branches tags] }, - variables: {} + variables: {}, + variables_with_detail: [] ) end end 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 4ae51ac8bf9c12..c4decc19325aac 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 @@ -37,8 +37,11 @@ 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, + pipeline_attributes: {}) + ) step.perform! end @@ -56,5 +59,43 @@ expect(pipeline.errors).to be_empty end end + + describe '#apply_rules' do + before do + pipeline.yaml_variables = [{ key: 'VAR1', value: 'val1', public: true }] + end + + context "when rules don't return yaml variables" do + before do + allow(step).to receive(:workflow_rules_result) + .and_return( + double(pass?: true, + pipeline_attributes: {}) + ) + end + + it 'does not change pipeline yaml variables' do + step.perform! + + expect(pipeline.yaml_variables).to eq([{ key: 'VAR1', value: 'val1', public: true }]) + end + end + + context "when rules returns yaml variables" do + before do + allow(step).to receive(:workflow_rules_result) + .and_return( + double(pass?: true, + pipeline_attributes: { yaml_variables: [{ key: 'VAR1', value: 'val2', public: true }] }) + ) + end + + it 'changes pipeline yaml variables' do + step.perform! + + expect(pipeline.yaml_variables).to eq([{ key: 'VAR1', value: 'val2', public: true }]) + 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 cf020fc343cef4..753c6cba98ad9f 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -231,6 +231,42 @@ it { is_expected.to match a_hash_including(options: { allow_failure_criteria: nil }) } end end + + context '[:yaml_variables]' do + let(:attributes) do + { name: 'rspec', + ref: 'master', + yaml_variables: [{ key: 'VAR1', value: 'var pipeline 1', public: true, source: 'workflow' }, + { key: 'VAR2', value: 'var 2', public: true, source: 'job' }, + { key: 'VAR3', value: 'var 3', public: true, source: 'job' }] } + end + + context 'when the pipeline has variables' do + before do + pipeline.yaml_variables = [{ key: 'VAR1', value: 'var overridden pipeline 1', public: true }, + { key: 'VAR2', value: 'var pipeline 2', public: true }, + { key: 'VAR4', value: 'var pipeline 4', public: true }] + end + + it 'returns calculated yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR1', value: 'var overridden pipeline 1', public: true }, + { key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }, + { key: 'VAR4', value: 'var pipeline 4', public: true }] + ) + end + end + + context 'when the pipeline has not a variable' do + it 'returns seed yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR1', value: 'var pipeline 1', public: true }, + { key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }]) + end + end + end end describe '#bridge?' do diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index ac6c4c188e4007..7eb935cb46d9d3 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -165,11 +165,27 @@ def find_job(name) context 'variables:' do let(:config) do <<-EOY + variables: + VAR4: workflow var 4 + VAR5: workflow var 5 + + workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR4: overridden workflow var 4 + - if: $CI_COMMIT_REF_NAME =~ /feature/ + variables: + VAR5: overridden workflow var 5 + VAR6: new workflow var 6 + - when: always + job: script: "echo job1" variables: - VAR1: my var 1 - VAR2: my var 2 + VAR1: job var 1 + VAR2: job var 2 + VAR5: job var 5 rules: - if: $CI_COMMIT_REF_NAME =~ /master/ variables: @@ -187,12 +203,14 @@ def find_job(name) context 'when matching to the first rule' do let(:ref) { 'refs/heads/master' } - it 'overrides VAR1' do + it 'overrides VAR1 by job and VAR4 workflow' do variables = job.scoped_variables_hash expect(variables['VAR1']).to eq('overridden var 1') - expect(variables['VAR2']).to eq('my var 2') + expect(variables['VAR2']).to eq('job var 2') expect(variables['VAR3']).to be_nil + expect(variables['VAR4']).to eq('overridden workflow var 4') + expect(variables['VAR5']).to eq('job var 5') end context 'when FF ci_rules_variables is disabled' do @@ -203,8 +221,8 @@ def find_job(name) it 'does not affect variables' do variables = job.scoped_variables_hash - expect(variables['VAR1']).to eq('my var 1') - expect(variables['VAR2']).to eq('my var 2') + expect(variables['VAR1']).to eq('job var 1') + expect(variables['VAR2']).to eq('job var 2') expect(variables['VAR3']).to be_nil end end @@ -216,7 +234,7 @@ def find_job(name) it 'overrides VAR2 and adds VAR3' do variables = job.scoped_variables_hash - expect(variables['VAR1']).to eq('my var 1') + expect(variables['VAR1']).to eq('job var 1') expect(variables['VAR2']).to eq('overridden var 2') expect(variables['VAR3']).to eq('new var 3') end @@ -228,9 +246,11 @@ def find_job(name) it 'does not affect vars' do variables = job.scoped_variables_hash - expect(variables['VAR1']).to eq('my var 1') - expect(variables['VAR2']).to eq('my var 2') + expect(variables['VAR1']).to eq('job var 1') + expect(variables['VAR2']).to eq('job var 2') expect(variables['VAR3']).to be_nil + expect(variables['VAR4']).to eq('workflow var 4') + expect(variables['VAR5']).to eq('job var 5') end end end -- GitLab From f2c11fde68aca9a4921381130b8a298f20192de4 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Tue, 19 Jan 2021 16:43:44 +0300 Subject: [PATCH 2/3] try to extract yaml variables calculation --- lib/gitlab/ci/build/rules.rb | 10 ++-------- .../ci/pipeline/chain/evaluate_workflow_rules.rb | 6 +----- lib/gitlab/ci/pipeline/chain/helpers.rb | 10 ++++++++++ .../ci/pipeline/chain/evaluate_workflow_rules_spec.rb | 11 ++++------- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/lib/gitlab/ci/build/rules.rb b/lib/gitlab/ci/build/rules.rb index 6538b06f7b44dc..a39afee194c50d 100644 --- a/lib/gitlab/ci/build/rules.rb +++ b/lib/gitlab/ci/build/rules.rb @@ -12,13 +12,7 @@ def build_attributes(seed_attributes = {}) when: self.when, options: { start_in: start_in }.compact, allow_failure: allow_failure, - yaml_variables: calculate_variables(seed_attributes[:yaml_variables]) - }.compact - end - - def pipeline_attributes(seed_attributes = {}) - { - yaml_variables: calculate_variables(seed_attributes[:yaml_variables]) + yaml_variables: yaml_variables(seed_attributes[:yaml_variables]) }.compact end @@ -28,7 +22,7 @@ def pass? private - def calculate_variables(seed_variables) + def yaml_variables(seed_variables) return unless variables && seed_variables indexed_seed_variables = seed_variables.deep_dup.index_by { |var| var[:key] } diff --git a/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb b/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb index 707c77136765e6..2ce39236b3d935 100644 --- a/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb +++ b/lib/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb @@ -23,11 +23,7 @@ def break? private def apply_rules - @pipeline.assign_attributes(rules_attributes) - end - - def rules_attributes - workflow_rules_result.pipeline_attributes(yaml_variables: @pipeline.yaml_variables) + @pipeline.yaml_variables = merge_variables(@pipeline.yaml_variables, workflow_rules_result.variables) end def workflow_passed? diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb index d7271df1694df1..56e9b0f6324779 100644 --- a/lib/gitlab/ci/pipeline/chain/helpers.rb +++ b/lib/gitlab/ci/pipeline/chain/helpers.rb @@ -27,6 +27,16 @@ def warning(message) def persist_pipeline? command.save_incompleted && !pipeline.readonly? end + + def merge_variables(current_variables, new_variables) + return unless current_variables && new_variables + + indexed_current_variables = current_variables.deep_dup.index_by { |var| var[:key] } + + result = new_variables.each_with_object(indexed_current_variables) do |var, hash| + hash[var[0].to_s] = { key: var[0].to_s, value: var[1], public: true } + end.values + end end end end 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 c4decc19325aac..98b34de55bbb4d 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 @@ -39,8 +39,7 @@ before do allow(step).to receive(:workflow_rules_result) .and_return( - double(pass?: true, - pipeline_attributes: {}) + double(pass?: true, variables: {}) ) step.perform! @@ -65,12 +64,11 @@ pipeline.yaml_variables = [{ key: 'VAR1', value: 'val1', public: true }] end - context "when rules don't return yaml variables" do + context "when rules doesn't return yaml variables" do before do allow(step).to receive(:workflow_rules_result) .and_return( - double(pass?: true, - pipeline_attributes: {}) + double(pass?: true, variables: {}) ) end @@ -85,8 +83,7 @@ before do allow(step).to receive(:workflow_rules_result) .and_return( - double(pass?: true, - pipeline_attributes: { yaml_variables: [{ key: 'VAR1', value: 'val2', public: true }] }) + double(pass?: true, variables: { 'VAR1' => 'val2' }) ) end -- GitLab From ab09863f0269d666632b91d1cd53c152e4ed4e9a Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Wed, 20 Jan 2021 13:02:58 +0300 Subject: [PATCH 3/3] Add a new test case --- spec/services/ci/create_pipeline_service/rules_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index 7eb935cb46d9d3..c681f6ca619cae 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -168,6 +168,7 @@ def find_job(name) variables: VAR4: workflow var 4 VAR5: workflow var 5 + VAR7: workflow var 7 workflow: rules: @@ -178,6 +179,7 @@ def find_job(name) variables: VAR5: overridden workflow var 5 VAR6: new workflow var 6 + VAR7: overridden workflow var 7 - when: always job: @@ -194,6 +196,7 @@ def find_job(name) variables: VAR2: overridden var 2 VAR3: new var 3 + VAR7: overridden var 7 - when: on_success EOY end @@ -231,12 +234,13 @@ def find_job(name) context 'when matching to the second rule' do let(:ref) { 'refs/heads/feature' } - it 'overrides VAR2 and adds VAR3' do + it 'overrides VAR2 and VAR7, then adds VAR3' do variables = job.scoped_variables_hash expect(variables['VAR1']).to eq('job var 1') expect(variables['VAR2']).to eq('overridden var 2') expect(variables['VAR3']).to eq('new var 3') + expect(variables['VAR7']).to eq('overridden var 7') end end -- GitLab