Merge branch 'enforce-variable-value-to-be-a-string' into 'master'

Enforce setting string as value of the CI/CD variable

Closes #48210

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

app/models/ci/pipeline.rb

@@ -561,9 +561,9 @@ module Ci
         .append(key: 'CI_PIPELINE_IID', value: iid.to_s)
         .append(key: 'CI_CONFIG_PATH', value: ci_yaml_file_path)
         .append(key: 'CI_PIPELINE_SOURCE', value: source.to_s)
-        .append(key: 'CI_COMMIT_MESSAGE', value: git_commit_message)
-        .append(key: 'CI_COMMIT_TITLE', value: git_commit_full_title)
-        .append(key: 'CI_COMMIT_DESCRIPTION', value: git_commit_description)
+        .append(key: 'CI_COMMIT_MESSAGE', value: git_commit_message.to_s)
+        .append(key: 'CI_COMMIT_TITLE', value: git_commit_full_title.to_s)
+        .append(key: 'CI_COMMIT_DESCRIPTION', value: git_commit_description.to_s)
     end
 
     def queued_duration

app/models/project_auto_devops.rb

@@ -29,8 +29,8 @@ class ProjectAutoDevops < ActiveRecord::Base
       end
 
       if manual?
-        variables.append(key: 'STAGING_ENABLED', value: 1)
-        variables.append(key: 'INCREMENTAL_ROLLOUT_ENABLED', value: 1)
+        variables.append(key: 'STAGING_ENABLED', value: '1')
+        variables.append(key: 'INCREMENTAL_ROLLOUT_ENABLED', value: '1')
       end
     end
   end

changelogs/unreleased/enforce-variable-value-to-be-a-string.yml

@@ -0,0 +1,5 @@
+---
+title: Fix incremental rollouts for Auto DevOps
+merge_request: 20061
+author:
+type: fixed

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

@@ -4,6 +4,9 @@ module Gitlab
       class Collection
         class Item
           def initialize(key:, value:, public: true, file: false)
+            raise ArgumentError, "`value` must be of type String, while it was: #{value.class}" unless
+              value.is_a?(String) || value.nil?
+
             @variable = {
               key: key, value: value, public: public, file: file
             }

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

@@ -1,19 +1,69 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Variables::Collection::Item do
+  let(:variable_key) { 'VAR' }
+  let(:variable_value) { 'something' }
+  let(:expected_value) { variable_value }
+
   let(:variable) do
-    { key: 'VAR', value: 'something', public: true }
+    { key: variable_key, value: variable_value, public: true }
   end
 
   describe '.new' do
-    it 'raises error if unknown key i specified' do
-      expect { described_class.new(key: 'VAR', value: 'abc', files: true) }
-        .to raise_error ArgumentError, 'unknown keyword: files'
+    context 'when unknown keyword is specified' do
+      it 'raises error' do
+        expect { described_class.new(key: variable_key, value: 'abc', files: true) }
+          .to raise_error ArgumentError, 'unknown keyword: files'
+      end
     end
 
-    it 'raises error when required keywords are not specified' do
-      expect { described_class.new(key: 'VAR') }
-        .to raise_error ArgumentError, 'missing keyword: value'
+    context 'when required keywords are not specified' do
+      it 'raises error' do
+        expect { described_class.new(key: variable_key) }
+          .to raise_error ArgumentError, 'missing keyword: value'
+      end
+    end
+
+    shared_examples 'creates variable' do
+      subject { described_class.new(key: variable_key, value: variable_value) }
+
+      it 'saves given value' do
+        expect(subject[:key]).to eq variable_key
+        expect(subject[:value]).to eq expected_value
+      end
+    end
+
+    shared_examples 'raises error for invalid type' do
+      it do
+        expect { described_class.new(key: variable_key, value: variable_value) }
+          .to raise_error ArgumentError, /`value` must be of type String, while it was:/
+      end
+    end
+
+    it_behaves_like 'creates variable'
+
+    context "when it's nil" do
+      let(:variable_value) { nil }
+      let(:expected_value) { nil }
+
+      it_behaves_like 'creates variable'
+    end
+
+    context "when it's an empty string" do
+      let(:variable_value) { '' }
+      let(:expected_value) { '' }
+
+      it_behaves_like 'creates variable'
+    end
+
+    context 'when provided value is not a string' do
+      [1, false, [], {}, Object.new].each do |val|
+        context "when it's #{val}" do
+          let(:variable_value) { val }
+
+          it_behaves_like 'raises error for invalid type'
+        end
+      end
     end
   end
 

spec/lib/gitlab/ci/variables/collection_spec.rb

@@ -29,7 +29,7 @@ describe Gitlab::Ci::Variables::Collection do
     end
 
     it 'appends an internal resource' do
-      collection = described_class.new([{ key: 'TEST', value: 1 }])
+      collection = described_class.new([{ key: 'TEST', value: '1' }])
 
       subject.append(collection.first)
 
@@ -74,15 +74,15 @@ describe Gitlab::Ci::Variables::Collection do
 
   describe '#+' do
     it 'makes it possible to combine with an array' do
-      collection = described_class.new([{ key: 'TEST', value: 1 }])
+      collection = described_class.new([{ key: 'TEST', value: '1' }])
       variables = [{ key: 'TEST', value: 'something' }]
 
       expect((collection + variables).count).to eq 2
     end
 
     it 'makes it possible to combine with another collection' do
-      collection = described_class.new([{ key: 'TEST', value: 1 }])
-      other = described_class.new([{ key: 'TEST', value: 2 }])
+      collection = described_class.new([{ key: 'TEST', value: '1' }])
+      other = described_class.new([{ key: 'TEST', value: '2' }])
 
       expect((collection + other).count).to eq 2
     end
@@ -90,10 +90,10 @@ describe Gitlab::Ci::Variables::Collection do
 
   describe '#to_runner_variables' do
     it 'creates an array of hashes in a runner-compatible format' do
-      collection = described_class.new([{ key: 'TEST', value: 1 }])
+      collection = described_class.new([{ key: 'TEST', value: '1' }])
 
       expect(collection.to_runner_variables)
-        .to eq [{ key: 'TEST', value: 1, public: true }]
+        .to eq [{ key: 'TEST', value: '1', public: true }]
     end
   end
 

spec/models/ci/build_spec.rb

@@ -1871,7 +1871,11 @@ describe Ci::Build do
     end
 
     context 'when yaml_variables are undefined' do
-      let(:pipeline) { create(:ci_pipeline, project: project) }
+      let(:pipeline) do
+        create(:ci_pipeline, project: project,
+                             sha: project.commit.id,
+                             ref: project.default_branch)
+      end
 
       before do
         build.yaml_variables = nil