summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGrzegorz Bizon <grzegorz@gitlab.com>2016-12-19 19:09:04 +0000
committerGrzegorz Bizon <grzegorz@gitlab.com>2016-12-19 19:09:04 +0000
commit5048baaa371ebaafeb057608d60bcb09a866f07c (patch)
treef5689998378d710decaeb48f6344672396fc8b5a
parent8c8c4f8c5a5f5845b9ebca415aaa29278cb904c7 (diff)
parent4d8a2bc987dbdd417f7ae971c884b084a8982f0a (diff)
downloadgitlab-ce-5048baaa371ebaafeb057608d60bcb09a866f07c.tar.gz
Merge branch 'fix-yaml-variables' into 'master'
Convert CI YAML variables keys into strings So that this would be more consistent with the other variables, which all of them are string based. Closes #25554 See merge request !8088
-rw-r--r--app/models/ci/build.rb2
-rw-r--r--changelogs/unreleased/fix-yaml-variables.yml4
-rw-r--r--lib/ci/gitlab_ci_yaml_processor.rb2
-rw-r--r--lib/gitlab/serialize/ci/variables.rb27
-rw-r--r--spec/factories/ci/builds.rb2
-rw-r--r--spec/lib/ci/gitlab_ci_yaml_processor_spec.rb24
-rw-r--r--spec/lib/gitlab/serialize/ci/variables_spec.rb18
-rw-r--r--spec/models/build_spec.rb20
8 files changed, 81 insertions, 18 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 591aba6bdc9..cb76cdf5981 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -18,7 +18,7 @@ module Ci
end
serialize :options
- serialize :yaml_variables
+ serialize :yaml_variables, Gitlab::Serialize::Ci::Variables
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..7463bd719d5 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, public: true }
end
end
diff --git a/lib/gitlab/serialize/ci/variables.rb b/lib/gitlab/serialize/ci/variables.rb
new file mode 100644
index 00000000000..3a9443bfcd9
--- /dev/null
+++ b/lib/gitlab/serialize/ci/variables.rb
@@ -0,0 +1,27 @@
+module Gitlab
+ module Serialize
+ module Ci
+ # 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 Variables
+ extend self
+
+ def load(string)
+ return unless string
+
+ object = YAML.safe_load(string, [Symbol])
+
+ object.map do |variable|
+ variable[:key] = variable[:key].to_s
+ variable
+ end
+ end
+
+ def dump(object)
+ YAML.dump(object)
+ end
+ 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/ci/variables_spec.rb b/spec/lib/gitlab/serialize/ci/variables_spec.rb
new file mode 100644
index 00000000000..7ea74da5252
--- /dev/null
+++ b/spec/lib/gitlab/serialize/ci/variables_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe Gitlab::Serialize::Ci::Variables do
+ subject do
+ described_class.load(described_class.dump(object))
+ end
+
+ let(:object) do
+ [{ key: :key, value: 'value', public: true },
+ { key: 'wee', value: 1, public: false }]
+ end
+
+ it 'converts keys 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 6f1c2ae0fd8..cd3b6d51545 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -458,7 +458,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) }
@@ -1306,11 +1306,25 @@ describe Ci::Build, models: true do
describe '#expanded_environment_name' do
subject { build.expanded_environment_name }
- context 'when environment uses variables' do
- let(:build) { create(:ci_build, ref: 'master', environment: 'review/$CI_BUILD_REF_NAME') }
+ context 'when environment uses $CI_BUILD_REF_NAME' do
+ let(:build) do
+ create(:ci_build,
+ ref: 'master',
+ environment: 'review/$CI_BUILD_REF_NAME')
+ end
it { is_expected.to eq('review/master') }
end
+
+ context 'when environment uses yaml_variables containing symbol keys' do
+ let(:build) do
+ create(:ci_build,
+ yaml_variables: [{ key: :APP_HOST, value: 'host' }],
+ environment: 'review/$APP_HOST')
+ end
+
+ it { is_expected.to eq('review/host') }
+ end
end
describe '#detailed_status' do