diff options
| -rw-r--r-- | app/models/ci/build.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/fix-yaml-variables.yml | 4 | ||||
| -rw-r--r-- | lib/ci/gitlab_ci_yaml_processor.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/serialize/yaml_variables.rb | 30 | ||||
| -rw-r--r-- | spec/factories/ci/builds.rb | 2 | ||||
| -rw-r--r-- | spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 24 | ||||
| -rw-r--r-- | spec/lib/gitlab/serialize/yaml_variables_spec.rb | 19 | ||||
| -rw-r--r-- | spec/models/build_spec.rb | 2 |
8 files changed, 69 insertions, 16 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index e7cf606a7ae..99f3f4711aa 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -10,7 +10,7 @@ module Ci has_many :deployments, as: :deployable serialize :options - serialize :yaml_variables + serialize :yaml_variables, Gitlab::Serialize::YamlVariables validates :coverage, numericality: true, allow_blank: true validates_presence_of :ref diff --git a/changelogs/unreleased/fix-yaml-variables.yml b/changelogs/unreleased/fix-yaml-variables.yml new file mode 100644 index 00000000000..3abff1e3b08 --- /dev/null +++ b/changelogs/unreleased/fix-yaml-variables.yml @@ -0,0 +1,4 @@ +--- +title: Convert CI YAML variables keys into strings +merge_request: 8088 +author: diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index fef652cb975..8806a506ffa 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -118,7 +118,7 @@ module Ci .merge(job_variables(name)) variables.map do |key, value| - { key: key, value: value, public: true } + { key: key.to_s, value: value.to_s, public: true } end end diff --git a/lib/gitlab/serialize/yaml_variables.rb b/lib/gitlab/serialize/yaml_variables.rb new file mode 100644 index 00000000000..ca44acbd906 --- /dev/null +++ b/lib/gitlab/serialize/yaml_variables.rb @@ -0,0 +1,30 @@ + +module Gitlab + module Serialize + # This serializer could make sure our YAML variables' keys and values + # are always strings. This is more for legacy build data because + # from now on we convert them into strings before saving to database. + module YamlVariables + extend self + + def load(string) + return unless string + + YAML.load(string). + map(&YamlVariables.method(:convert_key_value_to_string)) + end + + def dump(object) + YAML.dump(object) + end + + private + + def convert_key_value_to_string(variable) + variable[:key] = variable[:key].to_s + variable[:value] = variable[:value].to_s + variable + end + end + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 62466c06194..0397d5d4001 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -22,7 +22,7 @@ FactoryGirl.define do yaml_variables do [ - { key: :DB_NAME, value: 'postgres', public: true } + { key: 'DB_NAME', value: 'postgres', public: true } ] end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index ff5dcc06ab3..62d68721574 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -483,7 +483,7 @@ module Ci context 'when global variables are defined' do let(:variables) do - { VAR1: 'value1', VAR2: 'value2' } + { 'VAR1' => 'value1', 'VAR2' => 'value2' } end let(:config) do { @@ -495,18 +495,18 @@ module Ci it 'returns global variables' do expect(subject).to contain_exactly( - { key: :VAR1, value: 'value1', public: true }, - { key: :VAR2, value: 'value2', public: true } + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } ) end end context 'when job and global variables are defined' do let(:global_variables) do - { VAR1: 'global1', VAR3: 'global3' } + { 'VAR1' => 'global1', 'VAR3' => 'global3' } end let(:job_variables) do - { VAR1: 'value1', VAR2: 'value2' } + { 'VAR1' => 'value1', 'VAR2' => 'value2' } end let(:config) do { @@ -518,9 +518,9 @@ module Ci it 'returns all unique variables' do expect(subject).to contain_exactly( - { key: :VAR3, value: 'global3', public: true }, - { key: :VAR1, value: 'value1', public: true }, - { key: :VAR2, value: 'value2', public: true } + { key: 'VAR3', value: 'global3', public: true }, + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } ) end end @@ -535,13 +535,13 @@ module Ci context 'when syntax is correct' do let(:variables) do - { VAR1: 'value1', VAR2: 'value2' } + { 'VAR1' => 'value1', 'VAR2' => 'value2' } end it 'returns job variables' do expect(subject).to contain_exactly( - { key: :VAR1, value: 'value1', public: true }, - { key: :VAR2, value: 'value2', public: true } + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } ) end end @@ -549,7 +549,7 @@ module Ci context 'when syntax is incorrect' do context 'when variables defined but invalid' do let(:variables) do - [ :VAR1, 'value1', :VAR2, 'value2' ] + [ 'VAR1', 'value1', 'VAR2', 'value2' ] end it 'raises error' do diff --git a/spec/lib/gitlab/serialize/yaml_variables_spec.rb b/spec/lib/gitlab/serialize/yaml_variables_spec.rb new file mode 100644 index 00000000000..6d74f8c44d6 --- /dev/null +++ b/spec/lib/gitlab/serialize/yaml_variables_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Gitlab::Serialize::YamlVariables do + subject do + Gitlab::Serialize::YamlVariables.load( + Gitlab::Serialize::YamlVariables.dump(object)) + end + + let(:object) do + [{ key: :key, value: 'value', public: true }, + { key: 'wee', value: 1, public: false }] + end + + it 'converts key and values into strings' do + is_expected.to eq([ + { key: 'key', value: 'value', public: true }, + { key: 'wee', value: '1', public: false }]) + end +end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 7f39aff7639..13928695ccb 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -403,7 +403,7 @@ describe Ci::Build, models: true do }) end let(:variables) do - [{ key: :KEY, value: 'value', public: true }] + [{ key: 'KEY', value: 'value', public: true }] end it { is_expected.to eq(predefined_variables + variables) } |