Commit 5048baaa authored by Grzegorz Bizon's avatar Grzegorz Bizon

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
parents 8c8c4f8c 4d8a2bc9
...@@ -18,7 +18,7 @@ module Ci ...@@ -18,7 +18,7 @@ module Ci
end end
serialize :options serialize :options
serialize :yaml_variables serialize :yaml_variables, Gitlab::Serialize::Ci::Variables
validates :coverage, numericality: true, allow_blank: true validates :coverage, numericality: true, allow_blank: true
validates_presence_of :ref validates_presence_of :ref
......
---
title: Convert CI YAML variables keys into strings
merge_request: 8088
author:
...@@ -118,7 +118,7 @@ module Ci ...@@ -118,7 +118,7 @@ module Ci
.merge(job_variables(name)) .merge(job_variables(name))
variables.map do |key, value| variables.map do |key, value|
{ key: key, value: value, public: true } { key: key.to_s, value: value, public: true }
end end
end end
......
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
...@@ -22,7 +22,7 @@ FactoryGirl.define do ...@@ -22,7 +22,7 @@ FactoryGirl.define do
yaml_variables do yaml_variables do
[ [
{ key: :DB_NAME, value: 'postgres', public: true } { key: 'DB_NAME', value: 'postgres', public: true }
] ]
end end
......
...@@ -483,7 +483,7 @@ module Ci ...@@ -483,7 +483,7 @@ module Ci
context 'when global variables are defined' do context 'when global variables are defined' do
let(:variables) do let(:variables) do
{ VAR1: 'value1', VAR2: 'value2' } { 'VAR1' => 'value1', 'VAR2' => 'value2' }
end end
let(:config) do let(:config) do
{ {
...@@ -495,18 +495,18 @@ module Ci ...@@ -495,18 +495,18 @@ module Ci
it 'returns global variables' do it 'returns global variables' do
expect(subject).to contain_exactly( expect(subject).to contain_exactly(
{ key: :VAR1, value: 'value1', public: true }, { key: 'VAR1', value: 'value1', public: true },
{ key: :VAR2, value: 'value2', public: true } { key: 'VAR2', value: 'value2', public: true }
) )
end end
end end
context 'when job and global variables are defined' do context 'when job and global variables are defined' do
let(:global_variables) do let(:global_variables) do
{ VAR1: 'global1', VAR3: 'global3' } { 'VAR1' => 'global1', 'VAR3' => 'global3' }
end end
let(:job_variables) do let(:job_variables) do
{ VAR1: 'value1', VAR2: 'value2' } { 'VAR1' => 'value1', 'VAR2' => 'value2' }
end end
let(:config) do let(:config) do
{ {
...@@ -518,9 +518,9 @@ module Ci ...@@ -518,9 +518,9 @@ module Ci
it 'returns all unique variables' do it 'returns all unique variables' do
expect(subject).to contain_exactly( expect(subject).to contain_exactly(
{ key: :VAR3, value: 'global3', public: true }, { key: 'VAR3', value: 'global3', public: true },
{ key: :VAR1, value: 'value1', public: true }, { key: 'VAR1', value: 'value1', public: true },
{ key: :VAR2, value: 'value2', public: true } { key: 'VAR2', value: 'value2', public: true }
) )
end end
end end
...@@ -535,13 +535,13 @@ module Ci ...@@ -535,13 +535,13 @@ module Ci
context 'when syntax is correct' do context 'when syntax is correct' do
let(:variables) do let(:variables) do
{ VAR1: 'value1', VAR2: 'value2' } { 'VAR1' => 'value1', 'VAR2' => 'value2' }
end end
it 'returns job variables' do it 'returns job variables' do
expect(subject).to contain_exactly( expect(subject).to contain_exactly(
{ key: :VAR1, value: 'value1', public: true }, { key: 'VAR1', value: 'value1', public: true },
{ key: :VAR2, value: 'value2', public: true } { key: 'VAR2', value: 'value2', public: true }
) )
end end
end end
...@@ -549,7 +549,7 @@ module Ci ...@@ -549,7 +549,7 @@ module Ci
context 'when syntax is incorrect' do context 'when syntax is incorrect' do
context 'when variables defined but invalid' do context 'when variables defined but invalid' do
let(:variables) do let(:variables) do
[ :VAR1, 'value1', :VAR2, 'value2' ] [ 'VAR1', 'value1', 'VAR2', 'value2' ]
end end
it 'raises error' do it 'raises error' do
......
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
...@@ -458,7 +458,7 @@ describe Ci::Build, models: true do ...@@ -458,7 +458,7 @@ describe Ci::Build, models: true do
}) })
end end
let(:variables) do let(:variables) do
[{ key: :KEY, value: 'value', public: true }] [{ key: 'KEY', value: 'value', public: true }]
end end
it { is_expected.to eq(predefined_variables + variables) } it { is_expected.to eq(predefined_variables + variables) }
...@@ -1306,11 +1306,25 @@ describe Ci::Build, models: true do ...@@ -1306,11 +1306,25 @@ describe Ci::Build, models: true do
describe '#expanded_environment_name' do describe '#expanded_environment_name' do
subject { build.expanded_environment_name } subject { build.expanded_environment_name }
context 'when environment uses variables' do context 'when environment uses $CI_BUILD_REF_NAME' do
let(:build) { create(:ci_build, ref: 'master', environment: 'review/$CI_BUILD_REF_NAME') } let(:build) do
create(:ci_build,
ref: 'master',
environment: 'review/$CI_BUILD_REF_NAME')
end
it { is_expected.to eq('review/master') } it { is_expected.to eq('review/master') }
end 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 end
describe '#detailed_status' do describe '#detailed_status' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment