diff --git a/lib/gitlab/ci/build/rules.rb b/lib/gitlab/ci/build/rules.rb index a39afee194c50d25a03e02bd8084a4ad6d730780..2d4f9cf635b53c62569f3070b9e363d4bc7f7c7d 100644 --- a/lib/gitlab/ci/build/rules.rb +++ b/lib/gitlab/ci/build/rules.rb @@ -7,30 +7,17 @@ class Rules include ::Gitlab::Utils::StrongMemoize Result = Struct.new(:when, :start_in, :allow_failure, :variables) do - def build_attributes(seed_attributes = {}) + def build_attributes { when: self.when, options: { start_in: start_in }.compact, - allow_failure: allow_failure, - yaml_variables: yaml_variables(seed_attributes[:yaml_variables]) + allow_failure: allow_failure }.compact end def pass? self.when != 'never' end - - private - - def yaml_variables(seed_variables) - return unless variables && seed_variables - - indexed_seed_variables = seed_variables.deep_dup.index_by { |var| var[:key] } - - variables.each_with_object(indexed_seed_variables) do |var, hash| - hash[var[0].to_s] = { key: var[0].to_s, value: var[1], public: true } - end.values - end end def initialize(rule_hashes, default_when:) diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index 94680d052d9cc7b1a803fb2780bdde033f34606c..48411af6f38e31d9017f2284f4aa88e673e9509c 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -159,7 +159,11 @@ def rules_attributes next {} unless @using_rules if ::Gitlab::Ci::Features.rules_variables_enabled?(@pipeline.project) - rules_result.build_attributes(@seed_attributes) + rules_variables_result = ::Gitlab::Ci::Variables::Helpers.merge_variables( + @seed_attributes[:yaml_variables], rules_result.variables + ) + + rules_result.build_attributes.merge(yaml_variables: rules_variables_result) else rules_result.build_attributes end diff --git a/lib/gitlab/ci/variables/helpers.rb b/lib/gitlab/ci/variables/helpers.rb new file mode 100644 index 0000000000000000000000000000000000000000..e2a54f90ecb97e48e7f42f2d792a8c26728ce583 --- /dev/null +++ b/lib/gitlab/ci/variables/helpers.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Variables + module Helpers + class << self + def merge_variables(current_vars, new_vars) + current_vars = transform_from_yaml_variables(current_vars) + new_vars = transform_from_yaml_variables(new_vars) + + transform_to_yaml_variables( + current_vars.merge(new_vars) + ) + end + + def transform_to_yaml_variables(vars) + vars.to_h.map do |key, value| + { key: key.to_s, value: value, public: true } + end + end + + def transform_from_yaml_variables(vars) + return vars.stringify_keys if vars.is_a?(Hash) + + vars.to_a.map { |var| [var[:key].to_s, var[:value]] }.to_h + end + end + end + end + end +end diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index 86749cda9c7d8e56ed45be1e7aa7404526487444..3459b69bebca66f8fd01cebefec8ac5596b50407 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -123,9 +123,7 @@ def release(job) end def transform_to_yaml_variables(variables) - variables.to_h.map do |key, value| - { key: key.to_s, value: value, public: true } - end + ::Gitlab::Ci::Variables::Helpers.transform_to_yaml_variables(variables) end end end diff --git a/spec/lib/gitlab/ci/build/rules_spec.rb b/spec/lib/gitlab/ci/build/rules_spec.rb index a1af5b75f87d206fc3f204c6a3318983a8141400..0b50def05d460d8a3d2816bafa5dda48f3fbcfe1 100644 --- a/spec/lib/gitlab/ci/build/rules_spec.rb +++ b/spec/lib/gitlab/ci/build/rules_spec.rb @@ -201,40 +201,13 @@ end describe '#build_attributes' do - let(:seed_attributes) { {} } - subject(:build_attributes) do - result.build_attributes(seed_attributes) + result.build_attributes end it 'compacts nil values' do is_expected.to eq(options: {}, when: 'on_success') end - - context 'when there are variables in rules' do - let(:variables) { { VAR1: 'new var 1', VAR3: 'var 3' } } - - 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 'returns yaml_variables with override' do - is_expected.to include( - yaml_variables: [{ key: 'VAR1', value: 'new var 1', public: true }, - { key: 'VAR2', value: 'var 2', public: true }, - { key: 'VAR3', value: 'var 3', public: true }] - ) - 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 '#pass?' do diff --git a/spec/lib/gitlab/ci/variables/helpers_spec.rb b/spec/lib/gitlab/ci/variables/helpers_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b45abf8c0e157c46019bc7baf0223749be5ea88e --- /dev/null +++ b/spec/lib/gitlab/ci/variables/helpers_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::Ci::Variables::Helpers do + describe '.merge_variables' do + let(:current_variables) do + [{ key: 'key1', value: 'value1' }, + { key: 'key2', value: 'value2' }] + end + + let(:new_variables) do + [{ key: 'key2', value: 'value22' }, + { key: 'key3', value: 'value3' }] + end + + let(:result) do + [{ key: 'key1', value: 'value1', public: true }, + { key: 'key2', value: 'value22', public: true }, + { key: 'key3', value: 'value3', public: true }] + end + + subject { described_class.merge_variables(current_variables, new_variables) } + + it { is_expected.to eq(result) } + + context 'when new variables is a hash' do + let(:new_variables) do + { 'key2' => 'value22', 'key3' => 'value3' } + end + + it { is_expected.to eq(result) } + end + + context 'when new variables is a hash with symbol keys' do + let(:new_variables) do + { key2: 'value22', key3: 'value3' } + end + + it { is_expected.to eq(result) } + end + + context 'when new variables is nil' do + let(:new_variables) {} + let(:result) do + [{ key: 'key1', value: 'value1', public: true }, + { key: 'key2', value: 'value2', public: true }] + end + + it { is_expected.to eq(result) } + end + end + + describe '.transform_to_yaml_variables' do + let(:variables) do + { 'key1' => 'value1', 'key2' => 'value2' } + end + + let(:result) do + [{ key: 'key1', value: 'value1', public: true }, + { key: 'key2', value: 'value2', public: true }] + end + + subject { described_class.transform_to_yaml_variables(variables) } + + it { is_expected.to eq(result) } + + context 'when variables is nil' do + let(:variables) {} + + it { is_expected.to eq([]) } + end + end + + describe '.transform_from_yaml_variables' do + let(:variables) do + [{ key: 'key1', value: 'value1', public: true }, + { key: 'key2', value: 'value2', public: true }] + end + + let(:result) do + { 'key1' => 'value1', 'key2' => 'value2' } + end + + subject { described_class.transform_from_yaml_variables(variables) } + + it { is_expected.to eq(result) } + + context 'when variables is nil' do + let(:variables) {} + + it { is_expected.to eq({}) } + end + + context 'when variables is a hash' do + let(:variables) do + { key1: 'value1', 'key2' => 'value2' } + end + + it { is_expected.to eq(result) } + end + end +end