diff options
author | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-06-25 16:14:35 +0300 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-06-25 16:14:35 +0300 |
commit | 7a99a8c2ce38823ca7336ac4e3cb328274d7338b (patch) | |
tree | 2368643987191e0870fdbc137a6bbd2d073e33c6 | |
parent | 7668c9660fe30338044afd7886a0fac56e6e82b9 (diff) | |
parent | ed99467c03531015bec5126b453f5ea4f1cbd4b8 (diff) |
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
-rw-r--r-- | app/models/ci/pipeline.rb | 6 | ||||
-rw-r--r-- | app/models/project_auto_devops.rb | 4 | ||||
-rw-r--r-- | changelogs/unreleased/enforce-variable-value-to-be-a-string.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/ci/variables/collection/item.rb | 3 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/variables/collection/item_spec.rb | 64 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/variables/collection_spec.rb | 12 | ||||
-rw-r--r-- | spec/models/ci/build_spec.rb | 6 |
7 files changed, 81 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/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 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 |