diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index c0f2a258836ca1faa64cb732da75d8715ae80168..436b0127c3d25d963cbad39cf90172c11b4a5982 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -67,9 +67,9 @@ module Ci @image = @ci_config.image @after_script = @ci_config.after_script @services = @ci_config.services + @variables = @ci_config.variables @stages = @config[:stages] || @config[:types] - @variables = @config[:variables] || {} @cache = @config[:cache] @jobs = {} @@ -126,10 +126,6 @@ module Ci raise ValidationError, "stages should be an array of strings" end - unless @variables.nil? || validate_variables(@variables) - raise ValidationError, "variables should be a map of key-value strings" - end - validate_global_cache! if @cache end diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 8475e47d2b032b716a663b200a1d9b26dd3e0449..1bea9c21f6a2787f51aa253df1e714603ee59a71 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -7,7 +7,8 @@ module Gitlab ## # Temporary delegations that should be removed after refactoring # - delegate :before_script, :image, :services, :after_script, to: :@global + delegate :before_script, :image, :services, :after_script, :variables, + to: :@global def initialize(config) @config = Loader.new(config).load! diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb index 7b8d6d63a0831db6c5d976126f3335d0a9e97e69..3bb6f98ed0f852cb843d01f3ecc25f33f4ee1fa0 100644 --- a/lib/gitlab/ci/config/node/global.rb +++ b/lib/gitlab/ci/config/node/global.rb @@ -20,6 +20,9 @@ module Gitlab allow_node :after_script, Script, description: 'Script that will be executed after each job.' + + allow_node :variables, Variables, + description: 'Environment variables that will be used.' end end end diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb index a76f041c95377a92f1f0f537ea3701fc42af7b29..56f7661daf0b2c7434345215e107168328d2e88a 100644 --- a/lib/gitlab/ci/config/node/validators.rb +++ b/lib/gitlab/ci/config/node/validators.rb @@ -23,6 +23,16 @@ module Gitlab end end end + + class VariablesValidator < ActiveModel::EachValidator + include LegacyValidationHelpers + + def validate_each(record, attribute, value) + unless validate_variables(value) + record.errors.add(attribute, 'should be a hash of key value pairs') + end + end + end end end end diff --git a/lib/gitlab/ci/config/node/variables.rb b/lib/gitlab/ci/config/node/variables.rb new file mode 100644 index 0000000000000000000000000000000000000000..5decd777bdba596a10f761935f2c5ae96dc39332 --- /dev/null +++ b/lib/gitlab/ci/config/node/variables.rb @@ -0,0 +1,22 @@ +module Gitlab + module Ci + class Config + module Node + ## + # Entry that represents environment variables. + # + class Variables < Entry + include Validatable + + validations do + validates :value, variables: true + end + + def value + @config || {} + end + end + end + end + end +end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 35309eec5974f87efdc50c1b94bdac5bba7e66db..97a08758c04e751d38da7d14faa487ced6723274 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -1098,14 +1098,14 @@ EOT config = YAML.dump({ variables: "test", rspec: { script: "test" } }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-value strings") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Variables value should be a hash of key value pairs") end it "returns errors if variables is not a map of key-value strings" do config = YAML.dump({ variables: { test: false }, rspec: { script: "test" } }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-value strings") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Variables value should be a hash of key value pairs") end it "returns errors if job when is not on_success, on_failure or always" do diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb index 84ab1d49d0b523d9b940175a64a072658858cce4..ef4b669c403f957f8cd22520c790064166dd0a1a 100644 --- a/spec/lib/gitlab/ci/config/node/global_spec.rb +++ b/spec/lib/gitlab/ci/config/node/global_spec.rb @@ -24,6 +24,7 @@ describe Gitlab::Ci::Config::Node::Global do { before_script: ['ls', 'pwd'], image: 'ruby:2.2', services: ['postgres:9.1', 'mysql:5.5'], + variables: { VAR: 'value' }, after_script: ['make clean'] } end @@ -35,7 +36,7 @@ describe Gitlab::Ci::Config::Node::Global do end it 'creates node object for each entry' do - expect(global.nodes.count).to eq 4 + expect(global.nodes.count).to eq 5 end it 'creates node object using valid class' do @@ -93,6 +94,12 @@ describe Gitlab::Ci::Config::Node::Global do expect(global.after_script).to eq ['make clean'] end end + + describe '#variables' do + it 'returns variables' do + expect(global.variables).to eq(VAR: 'value') + end + end end end diff --git a/spec/lib/gitlab/ci/config/node/variables_spec.rb b/spec/lib/gitlab/ci/config/node/variables_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..67df70992b4dcb81f846ac778e4b68bba6213f81 --- /dev/null +++ b/spec/lib/gitlab/ci/config/node/variables_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Node::Variables do + let(:entry) { described_class.new(config) } + + describe 'validations' do + context 'when entry config value is correct' do + let(:config) do + { 'VARIABLE_1' => 'value 1', 'VARIABLE_2' => 'value 2' } + end + + describe '#value' do + it 'returns hash with key value strings' do + expect(entry.value).to eq config + end + end + + describe '#errors' do + it 'does not append errors' do + expect(entry.errors).to be_empty + end + end + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + end + + context 'when entry value is not correct' do + let(:config) { [ :VAR, 'test' ] } + + describe '#errors' do + it 'saves errors' do + expect(entry.errors) + .to include /should be a hash of key value pairs/ + end + end + + describe '#valid?' do + it 'is not valid' do + expect(entry).not_to be_valid + end + end + end + + ## + # See #18775 + # + context 'when entry value is not defined' do + let(:config) { nil } + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + + describe '#values' do + it 'returns an empty hash' do + expect(entry.value).to eq({}) + end + end + end + end +end