Merge branch 'fix/gb/fix-deserializing-ci-yaml-variables' into 'master'

Fix deserializing YAML variables when a build has been imported

Closes #49406

See merge request gitlab-org/gitlab-ce!20713

6 files changed, 48 insertions, 6 deletions

diff --git a/changelogs/unreleased/fix-gb-fix-deserializing-ci-yaml-variables.yml b/changelogs/unreleased/fix-gb-fix-deserializing-ci-yaml-variables.yml
new file mode 100644
index 00000000000..80b069c9251
--- /dev/null
+++ b/changelogs/unreleased/fix-gb-fix-deserializing-ci-yaml-variables.yml
@@ -0,0 +1,5 @@
+---
+title: Fix accessing imported pipeline builds
+merge_request: 20713
+author:
+type: fixed
diff --git a/lib/gitlab/ci/variables/collection/item.rb b/lib/gitlab/ci/variables/collection/item.rb
index 222aa06b800..7da6d09d440 100644
--- a/lib/gitlab/ci/variables/collection/item.rb
+++ b/lib/gitlab/ci/variables/collection/item.rb
@@ -34,7 +34,7 @@ module Gitlab
           def self.fabricate(resource)
             case resource
             when Hash
-              self.new(resource)
+              self.new(resource.symbolize_keys)
             when ::HasVariable
               self.new(resource.to_runner_variable)
             when self
diff --git a/lib/gitlab/serializer/ci/variables.rb b/lib/gitlab/serializer/ci/variables.rb
index c059c454eac..292c8de6229 100644
--- a/lib/gitlab/serializer/ci/variables.rb
+++ b/lib/gitlab/serializer/ci/variables.rb
@@ -13,8 +13,9 @@ module Gitlab
           object = YAML.safe_load(string, [Symbol])
 
           object.map do |variable|
-            variable[:key] = variable[:key].to_s
-            variable
+            variable.symbolize_keys.tap do |variable|
+              variable[:key] = variable[:key].to_s
+            end
           end
         end
 
diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb
index adb3ff4321f..46874662edd 100644
--- a/spec/lib/gitlab/ci/variables/collection/item_spec.rb
+++ b/spec/lib/gitlab/ci/variables/collection/item_spec.rb
@@ -75,6 +75,14 @@ describe Gitlab::Ci::Variables::Collection::Item do
       expect(resource).to eq variable
     end
 
+    it 'supports using a hash with stringified values' do
+      variable = { 'key' => 'VARIABLE', 'value' => 'my value' }
+
+      resource = described_class.fabricate(variable)
+
+      expect(resource).to eq(key: 'VARIABLE', value: 'my value')
+    end
+
     it 'supports using an active record resource' do
       variable = create(:ci_variable, key: 'CI_VAR', value: '123')
       resource = described_class.fabricate(variable)
diff --git a/spec/lib/gitlab/serializer/ci/variables_spec.rb b/spec/lib/gitlab/serializer/ci/variables_spec.rb
index c4b7fda5dbb..1d1fd5b0763 100644
--- a/spec/lib/gitlab/serializer/ci/variables_spec.rb
+++ b/spec/lib/gitlab/serializer/ci/variables_spec.rb
@@ -1,4 +1,4 @@
-require 'spec_helper'
+require 'fast_spec_helper'
 
 describe Gitlab::Serializer::Ci::Variables do
   subject do
@@ -6,11 +6,11 @@ describe Gitlab::Serializer::Ci::Variables do
   end
 
   let(:object) do
-    [{ key: :key, value: 'value', public: true },
+    [{ 'key' => :key, 'value' => 'value', 'public' => true },
      { key: 'wee', value: 1, public: false }]
   end
 
-  it 'converts keys into strings' do
+  it 'converts keys into strings and symbolizes hash' do
     is_expected.to eq([
       { key: 'key', value: 'value', public: true },
       { key: 'wee', value: 1, public: false }
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index ee923374480..67199eb6d26 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -2269,6 +2269,34 @@ describe Ci::Build do
     end
   end
 
+  describe '#yaml_variables' do
+    before do
+      build.update_attribute(:yaml_variables, variables)
+    end
+
+    context 'when serialized valu is a symbolized hash' do
+      let(:variables) do
+        [{ key: :VARIABLE, value: 'my value 1' }]
+      end
+
+      it 'keeps symbolizes keys and stringifies variables names' do
+        expect(build.yaml_variables)
+          .to eq [{ key: 'VARIABLE', value: 'my value 1' }]
+      end
+    end
+
+    context 'when serialized value is a hash with string keys' do
+      let(:variables) do
+        [{ 'key' => :VARIABLE, 'value' => 'my value 2' }]
+      end
+
+      it 'symblizes variables hash' do
+        expect(build.yaml_variables)
+          .to eq [{ key: 'VARIABLE', value: 'my value 2' }]
+      end
+    end
+  end
+
   describe 'state transition: any => [:pending]' do
     let(:build) { create(:ci_build, :created) }