From 05ce8a118743a5d896b6b8cc99b40af214ac8cd1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon <grzesiek.bizon@gmail.com> Date: Wed, 22 Jun 2016 11:22:53 +0200 Subject: [PATCH] Handle CI environment variables in a new CI config --- lib/ci/gitlab_ci_yaml_processor.rb | 6 +- lib/gitlab/ci/config.rb | 3 +- lib/gitlab/ci/config/node/global.rb | 3 + lib/gitlab/ci/config/node/validators.rb | 10 +++ lib/gitlab/ci/config/node/variables.rb | 22 ++++++ spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 4 +- spec/lib/gitlab/ci/config/node/global_spec.rb | 9 ++- .../gitlab/ci/config/node/variables_spec.rb | 67 +++++++++++++++++++ 8 files changed, 115 insertions(+), 9 deletions(-) create mode 100644 lib/gitlab/ci/config/node/variables.rb create mode 100644 spec/lib/gitlab/ci/config/node/variables_spec.rb diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index c0f2a258836..436b0127c3d 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 8475e47d2b0..1bea9c21f6a 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 7b8d6d63a08..3bb6f98ed0f 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 a76f041c953..56f7661daf0 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 00000000000..5decd777bdb --- /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 35309eec597..97a08758c04 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 84ab1d49d0b..ef4b669c403 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 00000000000..67df70992b4 --- /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 -- GitLab