From a2a0a8abd204f10ec32378af1f5fd2549111dbda Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Mon, 7 Nov 2022 13:16:00 +0100 Subject: [PATCH 1/2] Add raw support for sending trigger variables This is another step of implementing the raw/expand to CI config YAML. All changes are behind the FF ci_raw_variables_in_yaml_config --- app/models/ci/bridge.rb | 36 ++++++++- spec/models/ci/bridge_spec.rb | 75 +++++++++++++++++++ .../create_pipeline_service/variables_spec.rb | 44 +++++++++++ 3 files changed, 152 insertions(+), 3 deletions(-) diff --git a/app/models/ci/bridge.rb b/app/models/ci/bridge.rb index b2bdbda9da95d7..f7defccbfcc39b 100644 --- a/app/models/ci/bridge.rb +++ b/app/models/ci/bridge.rb @@ -288,7 +288,15 @@ def downstream_yaml_variables(expand_variables) return [] unless forward_yaml_variables? yaml_variables.to_a.map do |hash| - { key: hash[:key], value: ::ExpandVariables.expand(hash[:value], expand_variables) } + if ci_raw_variables_in_yaml_config_enabled? + if hash[:raw] + { key: hash[:key], value: hash[:value], raw: true } + else + { key: hash[:key], value: ::ExpandVariables.expand(hash[:value], expand_variables) } + end + else + { key: hash[:key], value: ::ExpandVariables.expand(hash[:value], expand_variables) } + end end end @@ -296,7 +304,15 @@ def downstream_pipeline_variables(expand_variables) return [] unless forward_pipeline_variables? pipeline.variables.to_a.map do |variable| - { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } + if ci_raw_variables_in_yaml_config_enabled? + if variable.raw? + { key: variable.key, value: variable.value, raw: true } + else + { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } + end + else + { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } + end end end @@ -305,7 +321,15 @@ def downstream_pipeline_schedule_variables(expand_variables) return [] unless pipeline.pipeline_schedule pipeline.pipeline_schedule.variables.to_a.map do |variable| - { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } + if ci_raw_variables_in_yaml_config_enabled? + if variable.raw? + { key: variable.key, value: variable.value, raw: true } + else + { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } + end + else + { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } + end end end @@ -324,6 +348,12 @@ def forward_pipeline_variables? result.nil? ? FORWARD_DEFAULTS[:pipeline_variables] : result end end + + def ci_raw_variables_in_yaml_config_enabled? + strong_memoize(:ci_raw_variables_in_yaml_config_enabled) do + ::Feature.enabled?(:ci_raw_variables_in_yaml_config, project) + end + end end end diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 3df278a49dda0e..5370aae3c70c5f 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -206,6 +206,8 @@ end describe '#downstream_variables' do + subject(:downstream_variables) { bridge.downstream_variables } + it 'returns variables that are going to be passed downstream' do expect(bridge.downstream_variables) .to include(key: 'BRIDGE', value: 'cross') @@ -330,6 +332,79 @@ end end end + + context 'when using raw variables' do + let(:options) do + { + trigger: { + project: 'my/project', + branch: 'master', + forward: { yaml_variables: true, + pipeline_variables: true }.compact + } + } + end + + let(:yaml_variables) do + [ + { + key: 'VAR6', + value: 'value6 $VAR1' + }, + { + key: 'VAR7', + value: 'value7 $VAR1', + raw: true + } + ] + end + + let(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) } + let(:pipeline) { create(:ci_pipeline, pipeline_schedule: pipeline_schedule) } + + before do + create(:ci_pipeline_variable, pipeline: pipeline, key: 'VAR1', value: 'value1') + create(:ci_pipeline_variable, pipeline: pipeline, key: 'VAR2', value: 'value2 $VAR1') + create(:ci_pipeline_variable, pipeline: pipeline, key: 'VAR3', value: 'value3 $VAR1', raw: true) + + pipeline_schedule.variables.create!(key: 'VAR4', value: 'value4 $VAR1') + pipeline_schedule.variables.create!(key: 'VAR5', value: 'value5 $VAR1', raw: true) + + bridge.yaml_variables.concat(yaml_variables) + end + + it 'expands variables according to their raw attributes' do + expect(downstream_variables).to contain_exactly( + { key: 'BRIDGE', value: 'cross' }, + { key: 'VAR1', value: 'value1' }, + { key: 'VAR2', value: 'value2 value1' }, + { key: 'VAR3', value: 'value3 $VAR1', raw: true }, + { key: 'VAR4', value: 'value4 value1' }, + { key: 'VAR5', value: 'value5 $VAR1', raw: true }, + { key: 'VAR6', value: 'value6 value1' }, + { key: 'VAR7', value: 'value7 $VAR1', raw: true } + ) + end + + context 'when the FF ci_raw_variables_in_yaml_config is disabled' do + before do + stub_feature_flags(ci_raw_variables_in_yaml_config: false) + end + + it 'ignores the raw attribute' do + expect(downstream_variables).to contain_exactly( + { key: 'BRIDGE', value: 'cross' }, + { key: 'VAR1', value: 'value1' }, + { key: 'VAR2', value: 'value2 value1' }, + { key: 'VAR3', value: 'value3 value1' }, + { key: 'VAR4', value: 'value4 value1' }, + { key: 'VAR5', value: 'value5 value1' }, + { key: 'VAR6', value: 'value6 value1' }, + { key: 'VAR7', value: 'value7 value1' } + ) + end + end + end end describe 'metadata support' do diff --git a/spec/services/ci/create_pipeline_service/variables_spec.rb b/spec/services/ci/create_pipeline_service/variables_spec.rb index c0ed8ad2a9e77a..e9e0cf2c6e048c 100644 --- a/spec/services/ci/create_pipeline_service/variables_spec.rb +++ b/spec/services/ci/create_pipeline_service/variables_spec.rb @@ -82,6 +82,50 @@ end end end + + context 'when trigger variables have expand: true/false' do + let(:config) do + <<-YAML + child: + variables: + VAR1: "PROJECTID-$CI_PROJECT_ID" + VAR2: "PIPELINEID-$CI_PIPELINE_ID and $VAR1" + VAR3: + value: "PIPELINEID-$CI_PIPELINE_ID and $VAR1" + expand: false + trigger: + include: child.yml + YAML + end + + let(:child) { find_job('child') } + + it 'creates the pipeline with a trigger job that has downstream_variables expanded according to "expand"' do + expect(pipeline).to be_created_successfully + + expect(child.downstream_variables).to include( + { key: 'VAR1', value: "PROJECTID-#{project.id}" }, + { key: 'VAR2', value: "PIPELINEID-#{pipeline.id} and PROJECTID-$CI_PROJECT_ID" }, + { key: 'VAR3', value: "PIPELINEID-$CI_PIPELINE_ID and $VAR1", raw: true } + ) + end + + context 'when the FF ci_raw_variables_in_yaml_config is disabled' do + before do + stub_feature_flags(ci_raw_variables_in_yaml_config: false) + end + + it 'creates the pipeline with a job that has all variables expanded' do + expect(pipeline).to be_created_successfully + + expect(child.downstream_variables).to include( + { key: 'VAR1', value: "PROJECTID-#{project.id}" }, + { key: 'VAR2', value: "PIPELINEID-#{pipeline.id} and PROJECTID-$CI_PROJECT_ID" }, + { key: 'VAR3', value: "PIPELINEID-#{pipeline.id} and PROJECTID-$CI_PROJECT_ID" } + ) + end + end + end end private -- GitLab From bbfb20fa14fb716796712de2f2af7d8e9ab79a20 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Wed, 9 Nov 2022 15:30:16 +0100 Subject: [PATCH 2/2] Convert to one-line condition --- app/models/ci/bridge.rb | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/app/models/ci/bridge.rb b/app/models/ci/bridge.rb index f7defccbfcc39b..b1f768825059c3 100644 --- a/app/models/ci/bridge.rb +++ b/app/models/ci/bridge.rb @@ -288,12 +288,8 @@ def downstream_yaml_variables(expand_variables) return [] unless forward_yaml_variables? yaml_variables.to_a.map do |hash| - if ci_raw_variables_in_yaml_config_enabled? - if hash[:raw] - { key: hash[:key], value: hash[:value], raw: true } - else - { key: hash[:key], value: ::ExpandVariables.expand(hash[:value], expand_variables) } - end + if hash[:raw] && ci_raw_variables_in_yaml_config_enabled? + { key: hash[:key], value: hash[:value], raw: true } else { key: hash[:key], value: ::ExpandVariables.expand(hash[:value], expand_variables) } end @@ -304,12 +300,8 @@ def downstream_pipeline_variables(expand_variables) return [] unless forward_pipeline_variables? pipeline.variables.to_a.map do |variable| - if ci_raw_variables_in_yaml_config_enabled? - if variable.raw? - { key: variable.key, value: variable.value, raw: true } - else - { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } - end + if variable.raw? && ci_raw_variables_in_yaml_config_enabled? + { key: variable.key, value: variable.value, raw: true } else { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } end @@ -321,12 +313,8 @@ def downstream_pipeline_schedule_variables(expand_variables) return [] unless pipeline.pipeline_schedule pipeline.pipeline_schedule.variables.to_a.map do |variable| - if ci_raw_variables_in_yaml_config_enabled? - if variable.raw? - { key: variable.key, value: variable.value, raw: true } - else - { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } - end + if variable.raw? && ci_raw_variables_in_yaml_config_enabled? + { key: variable.key, value: variable.value, raw: true } else { key: variable.key, value: ::ExpandVariables.expand(variable.value, expand_variables) } end -- GitLab