Handle CI environment variables in a new CI config

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
 

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!

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

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

lib/gitlab/ci/config/node/variables.rb

+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

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

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
 

spec/lib/gitlab/ci/config/node/variables_spec.rb

+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