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

changelogs/unreleased/fix-gb-fix-deserializing-ci-yaml-variables.yml

+---
+title: Fix accessing imported pipeline builds
+merge_request: 20713
+author:
+type: fixed

lib/gitlab/ci/variables/collection/item.rb

@@ -34,7 +34,7 @@ def to_runner_variable
           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

lib/gitlab/serializer/ci/variables.rb

@@ -13,8 +13,9 @@ def load(string)
           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
 

spec/lib/gitlab/ci/variables/collection/item_spec.rb

@@ -75,6 +75,14 @@
       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)

spec/lib/gitlab/serializer/ci/variables_spec.rb

-require 'spec_helper'
+require 'fast_spec_helper'
 
 describe Gitlab::Serializer::Ci::Variables do
   subject do
@@ -6,11 +6,11 @@
   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 }

spec/models/ci/build_spec.rb

@@ -2269,6 +2269,34 @@ def create_mr(build, pipeline, factory: :merge_request, created_at: Time.now)
     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) }