From 156339569d52bef66fe41c85d29ab3c9865f7362 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 21 Jun 2018 13:17:35 +0200 Subject: Enforce setting string as value of the CI/CD variable --- app/models/ci/pipeline.rb | 6 +- app/models/project_auto_devops.rb | 4 +- lib/gitlab/ci/variables/collection/item.rb | 3 + .../gitlab/ci/variables/collection/item_spec.rb | 64 +++++++++++++++++++--- spec/lib/gitlab/ci/variables/collection_spec.rb | 12 ++-- spec/models/ci/build_spec.rb | 6 +- 6 files changed, 76 insertions(+), 19 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index f430f18ca9a..e5caa3ffa41 100644 --- a/app/models/ci/pipeline.rb +++ b/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 diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb index d7d6aaceb27..faa831b1949 100644 --- a/app/models/project_auto_devops.rb +++ b/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 diff --git a/lib/gitlab/ci/variables/collection/item.rb b/lib/gitlab/ci/variables/collection/item.rb index d00e5b07f95..222aa06b800 100644 --- a/lib/gitlab/ci/variables/collection/item.rb +++ b/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 } diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb index e79f0a7f257..adb3ff4321f 100644 --- a/spec/lib/gitlab/ci/variables/collection/item_spec.rb +++ b/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 + + 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 - it 'raises error when required keywords are not specified' do - expect { described_class.new(key: 'VAR') } - .to raise_error ArgumentError, 'missing keyword: value' + 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 diff --git a/spec/lib/gitlab/ci/variables/collection_spec.rb b/spec/lib/gitlab/ci/variables/collection_spec.rb index cb2f7718c9c..5c91816a586 100644 --- a/spec/lib/gitlab/ci/variables/collection_spec.rb +++ b/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 diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 51b9b518117..6758adc59eb 100644 --- a/spec/models/ci/build_spec.rb +++ b/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 -- cgit v1.2.1 From ed99467c03531015bec5126b453f5ea4f1cbd4b8 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 21 Jun 2018 13:30:03 +0200 Subject: Add CHANGELOG entry --- changelogs/unreleased/enforce-variable-value-to-be-a-string.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/enforce-variable-value-to-be-a-string.yml diff --git a/changelogs/unreleased/enforce-variable-value-to-be-a-string.yml b/changelogs/unreleased/enforce-variable-value-to-be-a-string.yml new file mode 100644 index 00000000000..e2a932ee5bb --- /dev/null +++ b/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 -- cgit v1.2.1