From c91298d554a2535e0a579e6255fd5640d171e6cf Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 21 Jun 2016 11:43:32 +0200
Subject: [PATCH 01/28] Use generic type validator in new ci configuration

---
 lib/gitlab/ci/config/node/configurable.rb     | 2 +-
 lib/gitlab/ci/config/node/validators.rb       | 9 ++++++---
 spec/lib/gitlab/ci/config/node/global_spec.rb | 6 ++++++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 374ff71d0f..e691ab0c5c 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -19,7 +19,7 @@ module Gitlab
 
           included do
             validations do
-              validates :config, hash: true
+              validates :config, type: Hash
             end
           end
 
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index dc9cdb9a22..a76f041c95 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -13,10 +13,13 @@ module Gitlab
             end
           end
 
-          class HashValidator < ActiveModel::EachValidator
+          class TypeValidator < ActiveModel::EachValidator
             def validate_each(record, attribute, value)
-              unless value.is_a?(Hash)
-                record.errors.add(attribute, 'should be a configuration entry hash')
+              type = options[:with]
+              raise unless type.is_a?(Class)
+
+              unless value.is_a?(type)
+                record.errors.add(attribute, "should be a #{type.name}")
               end
             end
           end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index fddd53a2b5..fc9257d89d 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -106,5 +106,11 @@ describe Gitlab::Ci::Config::Node::Global do
         expect(global).not_to be_valid
       end
     end
+
+    describe '#errors' do
+      it 'returns error about invalid type' do
+        expect(global.errors.first).to match /should be a hash/
+      end
+    end
   end
 end

From 8b550db33e27d95424de31acf20835cc731684a5 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 21 Jun 2016 11:47:05 +0200
Subject: [PATCH 02/28] Add image configuration entry to new ci config

---
 lib/gitlab/ci/config/node/image.rb           | 22 ++++++++++
 spec/lib/gitlab/ci/config/node/image_spec.rb | 46 ++++++++++++++++++++
 2 files changed, 68 insertions(+)
 create mode 100644 lib/gitlab/ci/config/node/image.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/image_spec.rb

diff --git a/lib/gitlab/ci/config/node/image.rb b/lib/gitlab/ci/config/node/image.rb
new file mode 100644
index 0000000000..ff8dd8308a
--- /dev/null
+++ b/lib/gitlab/ci/config/node/image.rb
@@ -0,0 +1,22 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a Docker image.
+        #
+        class Image < Entry
+          include Validatable
+
+          validations do
+            validates :config, type: String
+          end
+
+          def value
+            @config
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/image_spec.rb b/spec/lib/gitlab/ci/config/node/image_spec.rb
new file mode 100644
index 0000000000..0b0821ca55
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/image_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Image do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validation' do
+    context 'when entry config value is correct' do
+      let(:config) { 'ruby:2.2' }
+
+      describe '#value' do
+        it 'returns image string' do
+          expect(entry.value).to eq 'ruby:2.2'
+        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) { ['ruby:2.2'] }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'Image config should be a string'
+        end
+      end
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+    end
+  end
+end

From cd6a2afbbb508e96f67c1f192cee2c1d379b1749 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 21 Jun 2016 12:10:13 +0200
Subject: [PATCH 03/28] Move CI image configuration entry to new CI config

---
 lib/ci/gitlab_ci_yaml_processor.rb            | 20 ++++++-------
 lib/gitlab/ci/config.rb                       |  2 +-
 lib/gitlab/ci/config/node/global.rb           |  3 ++
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  |  2 +-
 spec/lib/gitlab/ci/config/node/global_spec.rb | 28 +++++++++++++------
 5 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index c52d4d6338..f4ef449c84 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -14,7 +14,7 @@ module Ci
     ALLOWED_CACHE_KEYS = [:key, :untracked, :paths]
     ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in]
 
-    attr_reader :after_script, :image, :services, :path, :cache
+    attr_reader :after_script, :services, :path, :cache
 
     def initialize(config, path = nil)
       @ci_config = Gitlab::Ci::Config.new(config)
@@ -22,8 +22,11 @@ module Ci
 
       @path = path
 
-      initial_parsing
+      unless @ci_config.valid?
+        raise ValidationError, @ci_config.errors.first
+      end
 
+      initial_parsing
       validate!
     rescue Gitlab::Ci::Config::Loader::FormatError => e
       raise ValidationError, e.message
@@ -60,6 +63,9 @@ module Ci
     private
 
     def initial_parsing
+      @before_script = @ci_config.before_script
+      @image = @ci_config.image
+
       @after_script = @config[:after_script]
       @image = @config[:image]
       @services = @config[:services]
@@ -87,7 +93,7 @@ module Ci
       {
         stage_idx: stages.index(job[:stage]),
         stage: job[:stage],
-        commands: [job[:before_script] || [@ci_config.before_script], job[:script]].flatten.compact.join("\n"),
+        commands: [job[:before_script] || [@before_script], job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
         name: name,
         only: job[:only],
@@ -107,10 +113,6 @@ module Ci
     end
 
     def validate!
-      unless @ci_config.valid?
-        raise ValidationError, @ci_config.errors.first
-      end
-
       validate_global!
 
       @jobs.each do |name, job|
@@ -125,10 +127,6 @@ module Ci
         raise ValidationError, "after_script should be an array of strings"
       end
 
-      unless @image.nil? || @image.is_a?(String)
-        raise ValidationError, "image should be a string"
-      end
-
       unless @services.nil? || validate_array_of_strings(@services)
         raise ValidationError, "services should be an array of strings"
       end
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index adfd097736..d02902a110 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -7,7 +7,7 @@ module Gitlab
       ##
       # Temporary delegations that should be removed after refactoring
       #
-      delegate :before_script, to: :@global
+      delegate :before_script, :image, 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 044603423d..fa5f75beb9 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -11,6 +11,9 @@ module Gitlab
 
           allow_node :before_script, Script,
             description: 'Script that will be executed before each job.'
+
+          allow_node :image, Image,
+            description: 'Docker image that will be used to execute jobs.'
         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 200ca6aeee..4c7070ad05 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -979,7 +979,7 @@ EOT
         config = YAML.dump({ image: ["test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Image config should be a string")
       end
 
       it "returns errors if job name is blank" do
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index fc9257d89d..ae911d81c4 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -21,7 +21,8 @@ describe Gitlab::Ci::Config::Node::Global do
 
   context 'when hash is valid' do
     let(:hash) do
-      { before_script: ['ls', 'pwd'] }
+      { before_script: ['ls', 'pwd'],
+        image: 'ruby:2.2' }
     end
 
     describe '#process!' do
@@ -32,17 +33,21 @@ describe Gitlab::Ci::Config::Node::Global do
       end
 
       it 'creates node object for each entry' do
-        expect(global.nodes.count).to eq 1
+        expect(global.nodes.count).to eq 2
       end
 
       it 'creates node object using valid class' do
         expect(global.nodes.first)
           .to be_an_instance_of Gitlab::Ci::Config::Node::Script
+        expect(global.nodes.second)
+          .to be_an_instance_of Gitlab::Ci::Config::Node::Image
       end
 
       it 'sets correct description for nodes' do
         expect(global.nodes.first.description)
           .to eq 'Script that will be executed before each job.'
+        expect(global.nodes.second.description)
+          .to eq 'Docker image that will be used to execute jobs.'
       end
     end
 
@@ -51,19 +56,26 @@ describe Gitlab::Ci::Config::Node::Global do
         expect(global).not_to be_leaf
       end
     end
+    context 'when not processed' do
+      describe '#before_script' do
+        it 'returns nil' do
+          expect(global.before_script).to be nil
+        end
+      end
+    end
 
-    describe '#before_script' do
-      context 'when processed' do
-        before { global.process! }
+    context 'when processed' do
+      before { global.process! }
 
+      describe '#before_script' do
         it 'returns correct script' do
           expect(global.before_script).to eq "ls\npwd"
         end
       end
 
-      context 'when not processed' do
-        it 'returns nil' do
-          expect(global.before_script).to be nil
+      describe '#image' do
+        it 'returns valid image' do
+          expect(global.image).to eq 'ruby:2.2'
         end
       end
     end

From fc00c545b27ab2f4bf713ae246c197f809dd4c11 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 21 Jun 2016 12:40:52 +0200
Subject: [PATCH 04/28] Handle CI services config in new CI config classes

---
 lib/ci/gitlab_ci_yaml_processor.rb            |  8 +---
 lib/gitlab/ci/config.rb                       |  2 +-
 lib/gitlab/ci/config/node/global.rb           |  3 ++
 lib/gitlab/ci/config/node/services.rb         | 22 ++++++++++
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  |  4 +-
 spec/lib/gitlab/ci/config/node/global_spec.rb | 12 +++++-
 .../gitlab/ci/config/node/services_spec.rb    | 42 +++++++++++++++++++
 7 files changed, 82 insertions(+), 11 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/services.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/services_spec.rb

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index f4ef449c84..2cb46448e7 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -14,7 +14,7 @@ module Ci
     ALLOWED_CACHE_KEYS = [:key, :untracked, :paths]
     ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in]
 
-    attr_reader :after_script, :services, :path, :cache
+    attr_reader :after_script, :path, :cache
 
     def initialize(config, path = nil)
       @ci_config = Gitlab::Ci::Config.new(config)
@@ -68,7 +68,7 @@ module Ci
 
       @after_script = @config[:after_script]
       @image = @config[:image]
-      @services = @config[:services]
+      @services = @ci_config.services
       @stages = @config[:stages] || @config[:types]
       @variables = @config[:variables] || {}
       @cache = @config[:cache]
@@ -127,10 +127,6 @@ module Ci
         raise ValidationError, "after_script should be an array of strings"
       end
 
-      unless @services.nil? || validate_array_of_strings(@services)
-        raise ValidationError, "services should be an array of strings"
-      end
-
       unless @stages.nil? || validate_array_of_strings(@stages)
         raise ValidationError, "stages should be an array of strings"
       end
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index d02902a110..fb93d36c48 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -7,7 +7,7 @@ module Gitlab
       ##
       # Temporary delegations that should be removed after refactoring
       #
-      delegate :before_script, :image, to: :@global
+      delegate :before_script, :image, :services, 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 fa5f75beb9..03ed7808d2 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -14,6 +14,9 @@ module Gitlab
 
           allow_node :image, Image,
             description: 'Docker image that will be used to execute jobs.'
+
+          allow_node :services, Services,
+            description: 'Docker images that will be linked to the container.'
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/services.rb b/lib/gitlab/ci/config/node/services.rb
new file mode 100644
index 0000000000..d9898d9a4a
--- /dev/null
+++ b/lib/gitlab/ci/config/node/services.rb
@@ -0,0 +1,22 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a configuration of Docker services.
+        #
+        class Services < Entry
+          include Validatable
+
+          validations do
+            validates :config, array_of_strings: 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 4c7070ad05..bed174e249 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1007,14 +1007,14 @@ EOT
         config = YAML.dump({ services: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Services config should be an array of strings")
       end
 
       it "returns errors if services parameter is not an array of strings" do
         config = YAML.dump({ services: [10, "test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Services config should be an array of strings")
       end
 
       it "returns errors if job services parameter is not an array" do
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index ae911d81c4..7c8ddffc08 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -22,7 +22,8 @@ describe Gitlab::Ci::Config::Node::Global do
   context 'when hash is valid' do
     let(:hash) do
       { before_script: ['ls', 'pwd'],
-        image: 'ruby:2.2' }
+        image: 'ruby:2.2',
+        services: ['postgres:9.1', 'mysql:5.5'] }
     end
 
     describe '#process!' do
@@ -33,7 +34,7 @@ describe Gitlab::Ci::Config::Node::Global do
       end
 
       it 'creates node object for each entry' do
-        expect(global.nodes.count).to eq 2
+        expect(global.nodes.count).to eq 3
       end
 
       it 'creates node object using valid class' do
@@ -56,6 +57,7 @@ describe Gitlab::Ci::Config::Node::Global do
         expect(global).not_to be_leaf
       end
     end
+
     context 'when not processed' do
       describe '#before_script' do
         it 'returns nil' do
@@ -78,6 +80,12 @@ describe Gitlab::Ci::Config::Node::Global do
           expect(global.image).to eq 'ruby:2.2'
         end
       end
+
+      describe '#services' do
+        it 'returns array of services' do
+          expect(global.services).to eq ['postgres:9.1', 'mysql:5.5']
+        end
+      end
     end
   end
 
diff --git a/spec/lib/gitlab/ci/config/node/services_spec.rb b/spec/lib/gitlab/ci/config/node/services_spec.rb
new file mode 100644
index 0000000000..bda4b976cb
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/services_spec.rb
@@ -0,0 +1,42 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Services do
+  let(:entry) { described_class.new(config) }
+
+  describe '#process!' do
+    before { entry.process! }
+
+    context 'when entry config value is correct' do
+      let(:config) { ['postgres:9.1', 'mysql:5.5'] }
+
+      describe '#value' do
+        it 'returns array of services as is' do
+          expect(entry.value).to eq config
+        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) { 'ls' }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'Services config should be an array of strings'
+        end
+      end
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+    end
+  end
+end

From d399128955756fe7a4651d6595ae31406055dfb8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 21 Jun 2016 13:02:14 +0200
Subject: [PATCH 05/28] Handle after script CI config in new classes

This also makes Script to return an array of commands instead of
concatented command, which is our current direction.
---
 lib/ci/gitlab_ci_yaml_processor.rb             | 13 ++++---------
 lib/gitlab/ci/config.rb                        |  2 +-
 lib/gitlab/ci/config/node/global.rb            |  3 +++
 lib/gitlab/ci/config/node/script.rb            |  7 +------
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb   |  2 +-
 spec/lib/gitlab/ci/config/node/factory_spec.rb |  4 ++--
 spec/lib/gitlab/ci/config/node/global_spec.rb  | 13 ++++++++++---
 spec/lib/gitlab/ci/config/node/script_spec.rb  |  4 ++--
 8 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 2cb46448e7..c0f2a25883 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -14,7 +14,7 @@ module Ci
     ALLOWED_CACHE_KEYS = [:key, :untracked, :paths]
     ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in]
 
-    attr_reader :after_script, :path, :cache
+    attr_reader :path, :cache
 
     def initialize(config, path = nil)
       @ci_config = Gitlab::Ci::Config.new(config)
@@ -65,10 +65,9 @@ module Ci
     def initial_parsing
       @before_script = @ci_config.before_script
       @image = @ci_config.image
-
-      @after_script = @config[:after_script]
-      @image = @config[:image]
+      @after_script = @ci_config.after_script
       @services = @ci_config.services
+
       @stages = @config[:stages] || @config[:types]
       @variables = @config[:variables] || {}
       @cache = @config[:cache]
@@ -93,7 +92,7 @@ module Ci
       {
         stage_idx: stages.index(job[:stage]),
         stage: job[:stage],
-        commands: [job[:before_script] || [@before_script], job[:script]].flatten.compact.join("\n"),
+        commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
         name: name,
         only: job[:only],
@@ -123,10 +122,6 @@ module Ci
     end
 
     def validate_global!
-      unless @after_script.nil? || validate_array_of_strings(@after_script)
-        raise ValidationError, "after_script should be an array of strings"
-      end
-
       unless @stages.nil? || validate_array_of_strings(@stages)
         raise ValidationError, "stages should be an array of strings"
       end
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index fb93d36c48..8475e47d2b 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -7,7 +7,7 @@ module Gitlab
       ##
       # Temporary delegations that should be removed after refactoring
       #
-      delegate :before_script, :image, :services, to: :@global
+      delegate :before_script, :image, :services, :after_script, 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 03ed7808d2..7b8d6d63a0 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -17,6 +17,9 @@ module Gitlab
 
           allow_node :services, Services,
             description: 'Docker images that will be linked to the container.'
+
+          allow_node :after_script, Script,
+            description: 'Script that will be executed after each job.'
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index c044f5c5e7..7bbd6291c2 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -5,11 +5,6 @@ module Gitlab
         ##
         # Entry that represents a script.
         #
-        # Each element in the value array is a command that will be executed
-        # by GitLab Runner. Currently we concatenate these commands with
-        # new line character as a separator, what is compatible with
-        # implementation in Runner.
-        #
         class Script < Entry
           include Validatable
 
@@ -18,7 +13,7 @@ module Gitlab
           end
 
           def value
-            @config.join("\n")
+            @config
           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 bed174e249..35309eec59 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -965,7 +965,7 @@ EOT
         config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "After script config should be an array of strings")
       end
 
       it "returns errors if job after_script parameter is not an array of strings" do
diff --git a/spec/lib/gitlab/ci/config/node/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb
index 01a707a6bd..10462db769 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -11,7 +11,7 @@ describe Gitlab::Ci::Config::Node::Factory do
           .with(value: ['ls', 'pwd'])
           .create!
 
-        expect(entry.value).to eq "ls\npwd"
+        expect(entry.value).to eq ['ls', 'pwd']
       end
 
       context 'when setting description' do
@@ -21,7 +21,7 @@ describe Gitlab::Ci::Config::Node::Factory do
             .with(description: 'test description')
             .create!
 
-          expect(entry.value).to eq "ls\npwd"
+          expect(entry.value).to eq ['ls', 'pwd']
           expect(entry.description).to eq 'test description'
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 7c8ddffc08..84ab1d49d0 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -23,7 +23,8 @@ describe Gitlab::Ci::Config::Node::Global do
     let(:hash) do
       { before_script: ['ls', 'pwd'],
         image: 'ruby:2.2',
-        services: ['postgres:9.1', 'mysql:5.5'] }
+        services: ['postgres:9.1', 'mysql:5.5'],
+        after_script: ['make clean'] }
     end
 
     describe '#process!' do
@@ -34,7 +35,7 @@ describe Gitlab::Ci::Config::Node::Global do
       end
 
       it 'creates node object for each entry' do
-        expect(global.nodes.count).to eq 3
+        expect(global.nodes.count).to eq 4
       end
 
       it 'creates node object using valid class' do
@@ -71,7 +72,7 @@ describe Gitlab::Ci::Config::Node::Global do
 
       describe '#before_script' do
         it 'returns correct script' do
-          expect(global.before_script).to eq "ls\npwd"
+          expect(global.before_script).to eq ['ls', 'pwd']
         end
       end
 
@@ -86,6 +87,12 @@ describe Gitlab::Ci::Config::Node::Global do
           expect(global.services).to eq ['postgres:9.1', 'mysql:5.5']
         end
       end
+
+      describe '#after_script' do
+        it 'returns after script' do
+          expect(global.after_script).to eq ['make clean']
+        end
+      end
     end
   end
 
diff --git a/spec/lib/gitlab/ci/config/node/script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
index 6af6aa15ee..abd43aa1ee 100644
--- a/spec/lib/gitlab/ci/config/node/script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -10,8 +10,8 @@ describe Gitlab::Ci::Config::Node::Script do
       let(:config) { ['ls', 'pwd'] }
 
       describe '#value' do
-        it 'returns concatenated command' do
-          expect(entry.value).to eq "ls\npwd"
+        it 'returns array of strings' do
+          expect(entry.value).to eq config
         end
       end
 

From 97ec24f0b07d78dee1fa079479abc3b8ddf2e844 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 21 Jun 2016 13:12:58 +0200
Subject: [PATCH 06/28] Return CI entry config as value by default

---
 lib/gitlab/ci/config/node/entry.rb    | 5 +++--
 lib/gitlab/ci/config/node/image.rb    | 4 ----
 lib/gitlab/ci/config/node/script.rb   | 4 ----
 lib/gitlab/ci/config/node/services.rb | 4 ----
 4 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index f044ef965e..91f3fd0e23 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -9,7 +9,8 @@ module Gitlab
           class InvalidError < StandardError; end
 
           attr_reader :config
-          attr_accessor :key, :description
+          attr_accessor :description
+          attr_writer :key
 
           def initialize(config)
             @config = config
@@ -48,7 +49,7 @@ module Gitlab
           end
 
           def value
-            raise NotImplementedError
+            @config
           end
 
           def self.nodes
diff --git a/lib/gitlab/ci/config/node/image.rb b/lib/gitlab/ci/config/node/image.rb
index ff8dd8308a..5d3c7c5eab 100644
--- a/lib/gitlab/ci/config/node/image.rb
+++ b/lib/gitlab/ci/config/node/image.rb
@@ -11,10 +11,6 @@ module Gitlab
           validations do
             validates :config, type: String
           end
-
-          def value
-            @config
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index 7bbd6291c2..39328f0fad 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -11,10 +11,6 @@ module Gitlab
           validations do
             validates :config, array_of_strings: true
           end
-
-          def value
-            @config
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/services.rb b/lib/gitlab/ci/config/node/services.rb
index d9898d9a4a..481e2b66ad 100644
--- a/lib/gitlab/ci/config/node/services.rb
+++ b/lib/gitlab/ci/config/node/services.rb
@@ -11,10 +11,6 @@ module Gitlab
           validations do
             validates :config, array_of_strings: true
           end
-
-          def value
-            @config
-          end
         end
       end
     end

From 04ecfca386b70800d3876d3f11ff7451e95d9087 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 22 Jun 2016 10:44:33 +0200
Subject: [PATCH 07/28] Rename CI config null node entry to undefined node

---
 lib/gitlab/ci/config/node/configurable.rb       |  2 +-
 lib/gitlab/ci/config/node/factory.rb            |  6 ++----
 .../ci/config/node/{null.rb => undefined.rb}    | 17 ++++++-----------
 spec/lib/gitlab/ci/config/node/factory_spec.rb  |  6 +++---
 .../node/{null_spec.rb => undefined_spec.rb}    |  8 ++++----
 5 files changed, 16 insertions(+), 23 deletions(-)
 rename lib/gitlab/ci/config/node/{null.rb => undefined.rb} (57%)
 rename spec/lib/gitlab/ci/config/node/{null_spec.rb => undefined_spec.rb} (60%)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index e691ab0c5c..f26924fab2 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -27,7 +27,7 @@ module Gitlab
 
           def create_node(key, factory)
             factory.with(value: @config[key], key: key)
-            factory.nullify! unless @config.has_key?(key)
+            factory.undefine! unless @config.has_key?(key)
             factory.create!
           end
 
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 025ae40ef9..dbf027b6d8 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -5,8 +5,6 @@ module Gitlab
         ##
         # Factory class responsible for fabricating node entry objects.
         #
-        # It uses Fluent Interface pattern to set all necessary attributes.
-        #
         class Factory
           class InvalidFactory < StandardError; end
 
@@ -20,8 +18,8 @@ module Gitlab
             self
           end
 
-          def nullify!
-            @entry_class = Node::Null
+          def undefine!
+            @entry_class = Node::Undefined
             self
           end
 
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/undefined.rb
similarity index 57%
rename from lib/gitlab/ci/config/node/null.rb
rename to lib/gitlab/ci/config/node/undefined.rb
index 4f590f6bec..a812d803f3 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -3,20 +3,15 @@ module Gitlab
     class Config
       module Node
         ##
-        # This class represents a configuration entry that is not being used
+        # This class represents a configuration entry that is not defined
         # in configuration file.
         #
-        # This implements Null Object pattern.
+        # This implements a Null Object pattern.
         #
-        class Null < Entry
-          def value
-            nil
-          end
-
-          def validate!
-            nil
-          end
-
+        # It can be initialized using a default value of entry that is not
+        # present in configuration.
+        #
+        class Undefined < Entry
           def method_missing(*)
             nil
           end
diff --git a/spec/lib/gitlab/ci/config/node/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb
index 10462db769..8e13e243d4 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -45,14 +45,14 @@ describe Gitlab::Ci::Config::Node::Factory do
       end
     end
 
-    context 'when creating a null entry' do
+    context 'when creating undefined entry' do
       it 'creates a null entry' do
         entry = factory
           .with(value: nil)
-          .nullify!
+          .undefine!
           .create!
 
-        expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Null
+        expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
       end
     end
   end
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
similarity index 60%
rename from spec/lib/gitlab/ci/config/node/null_spec.rb
rename to spec/lib/gitlab/ci/config/node/undefined_spec.rb
index 36101c6246..2d01a8a6ec 100644
--- a/spec/lib/gitlab/ci/config/node/null_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
-describe Gitlab::Ci::Config::Node::Null do
-  let(:entry) { described_class.new(nil) }
+describe Gitlab::Ci::Config::Node::Undefined do
+  let(:entry) { described_class.new('some value') }
 
   describe '#leaf?' do
     it 'is leaf node' do
@@ -16,8 +16,8 @@ describe Gitlab::Ci::Config::Node::Null do
   end
 
   describe '#value' do
-    it 'returns nil' do
-      expect(entry.value).to be nil
+    it 'returns configured value' do
+      expect(entry.value).to eq 'some value'
     end
   end
 end

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 08/28] 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 c0f2a25883..436b0127c3 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 8475e47d2b..1bea9c21f6 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 7b8d6d63a0..3bb6f98ed0 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 a76f041c95..56f7661daf 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 0000000000..5decd777bd
--- /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 35309eec59..97a08758c0 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 84ab1d49d0..ef4b669c40 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 0000000000..67df70992b
--- /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

From bc2348f2e4365099e2a99df3d8e2a55fe7d138f4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 22 Jun 2016 14:26:33 +0200
Subject: [PATCH 09/28] Return default config value when entry is undefined

---
 lib/gitlab/ci/config/node/configurable.rb     |  6 +--
 lib/gitlab/ci/config/node/entry.rb            |  3 ++
 lib/gitlab/ci/config/node/factory.rb          |  9 ++--
 lib/gitlab/ci/config/node/global.rb           | 10 ++--
 lib/gitlab/ci/config/node/undefined.rb        | 18 ++++---
 lib/gitlab/ci/config/node/variables.rb        |  6 ++-
 .../ci/config/node/configurable_spec.rb       |  2 +-
 .../gitlab/ci/config/node/undefined_spec.rb   | 25 ++++++---
 spec/lib/gitlab/ci/config_spec.rb             | 54 +++++++++----------
 9 files changed, 78 insertions(+), 55 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index f26924fab2..25b5c2c9e2 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -33,12 +33,12 @@ module Gitlab
 
           class_methods do
             def nodes
-              Hash[@allowed_nodes.map { |key, factory| [key, factory.dup] }]
+              Hash[@nodes.map { |key, factory| [key, factory.dup] }]
             end
 
             private
 
-            def allow_node(symbol, entry_class, metadata)
+            def node(symbol, entry_class, metadata)
               factory = Node::Factory.new(entry_class)
                 .with(description: metadata[:description])
 
@@ -47,7 +47,7 @@ module Gitlab
                 @nodes[symbol].try(:value)
               end
 
-              (@allowed_nodes ||= {}).merge!(symbol => factory)
+              (@nodes ||= {}).merge!(symbol => factory)
             end
           end
         end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 91f3fd0e23..444a276d5c 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -52,6 +52,9 @@ module Gitlab
             @config
           end
 
+          def self.default
+          end
+
           def self.nodes
             {}
           end
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index dbf027b6d8..2271c386df 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -8,8 +8,8 @@ module Gitlab
         class Factory
           class InvalidFactory < StandardError; end
 
-          def initialize(entry_class)
-            @entry_class = entry_class
+          def initialize(node)
+            @node = node
             @attributes = {}
           end
 
@@ -19,14 +19,15 @@ module Gitlab
           end
 
           def undefine!
-            @entry_class = Node::Undefined
+            @attributes[:value] = @node.dup
+            @node = Node::Undefined
             self
           end
 
           def create!
             raise InvalidFactory unless @attributes.has_key?(:value)
 
-            @entry_class.new(@attributes[:value]).tap do |entry|
+            @node.new(@attributes[:value]).tap do |entry|
               entry.description = @attributes[:description]
               entry.key = @attributes[:key]
             end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 3bb6f98ed0..b5d177c528 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -9,19 +9,19 @@ module Gitlab
         class Global < Entry
           include Configurable
 
-          allow_node :before_script, Script,
+          node :before_script, Script,
             description: 'Script that will be executed before each job.'
 
-          allow_node :image, Image,
+          node :image, Image,
             description: 'Docker image that will be used to execute jobs.'
 
-          allow_node :services, Services,
+          node :services, Services,
             description: 'Docker images that will be linked to the container.'
 
-          allow_node :after_script, Script,
+          node :after_script, Script,
             description: 'Script that will be executed after each job.'
 
-          allow_node :variables, Variables,
+          node :variables, Variables,
             description: 'Environment variables that will be used.'
         end
       end
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index a812d803f3..e8a69b810e 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -3,17 +3,21 @@ module Gitlab
     class Config
       module Node
         ##
-        # This class represents a configuration entry that is not defined
-        # in configuration file.
+        # This class represents an undefined entry node.
         #
-        # This implements a Null Object pattern.
+        # It takes original entry class as configuration and returns default
+        # value of original entry as self value.
         #
-        # It can be initialized using a default value of entry that is not
-        # present in configuration.
         #
         class Undefined < Entry
-          def method_missing(*)
-            nil
+          include Validatable
+
+          validations do
+            validates :config, type: Class
+          end
+
+          def value
+            @config.default
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/variables.rb b/lib/gitlab/ci/config/node/variables.rb
index 5decd777bd..fd3ce8715a 100644
--- a/lib/gitlab/ci/config/node/variables.rb
+++ b/lib/gitlab/ci/config/node/variables.rb
@@ -13,7 +13,11 @@ module Gitlab
           end
 
           def value
-            @config || {}
+            @config || self.class.default
+          end
+
+          def self.default
+            {}
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/configurable_spec.rb b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
index 9bbda6e739..2ac436cb4b 100644
--- a/spec/lib/gitlab/ci/config/node/configurable_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
@@ -10,7 +10,7 @@ describe Gitlab::Ci::Config::Node::Configurable do
   describe 'configured nodes' do
     before do
       node.class_eval do
-        allow_node :object, Object, description: 'test object'
+        node :object, Object, description: 'test object'
       end
     end
 
diff --git a/spec/lib/gitlab/ci/config/node/undefined_spec.rb b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
index 2d01a8a6ec..5ded0504a3 100644
--- a/spec/lib/gitlab/ci/config/node/undefined_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
@@ -1,23 +1,34 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Undefined do
-  let(:entry) { described_class.new('some value') }
+  let(:undefined) { described_class.new(entry) }
+  let(:entry) { Class.new }
 
   describe '#leaf?' do
     it 'is leaf node' do
-      expect(entry).to be_leaf
+      expect(undefined).to be_leaf
     end
   end
 
-  describe '#any_method' do
-    it 'responds with nil' do
-      expect(entry.any_method).to be nil
+  describe '#valid?' do
+    it 'is always valid' do
+      expect(undefined).to be_valid
+    end
+  end
+
+  describe '#errors' do
+    it 'is does not contain errors' do
+      expect(undefined.errors).to be_empty
     end
   end
 
   describe '#value' do
-    it 'returns configured value' do
-      expect(entry.value).to eq 'some value'
+    before do
+      allow(entry).to receive(:default).and_return('some value')
+    end
+
+    it 'returns default value for entry that is undefined' do
+      expect(undefined.value).to eq 'some value'
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb
index 2a5d132db7..bc5a5e4310 100644
--- a/spec/lib/gitlab/ci/config_spec.rb
+++ b/spec/lib/gitlab/ci/config_spec.rb
@@ -40,38 +40,38 @@ describe Gitlab::Ci::Config do
         end
       end
     end
+  end
 
-    context 'when config is invalid' do
-      context 'when yml is incorrect' do
-        let(:yml) { '// invalid' }
+  context 'when config is invalid' do
+    context 'when yml is incorrect' do
+      let(:yml) { '// invalid' }
 
-        describe '.new' do
-          it 'raises error' do
-            expect { config }.to raise_error(
-              Gitlab::Ci::Config::Loader::FormatError,
-              /Invalid configuration format/
-            )
-          end
+      describe '.new' do
+        it 'raises error' do
+          expect { config }.to raise_error(
+            Gitlab::Ci::Config::Loader::FormatError,
+            /Invalid configuration format/
+          )
+        end
+      end
+    end
+
+    context 'when config logic is incorrect' do
+      let(:yml) { 'before_script: "ls"' }
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(config).not_to be_valid
+        end
+
+        it 'has errors' do
+          expect(config.errors).not_to be_empty
         end
       end
 
-      context 'when config logic is incorrect' do
-        let(:yml) { 'before_script: "ls"' }
-
-        describe '#valid?' do
-          it 'is not valid' do
-            expect(config).not_to be_valid
-          end
-
-          it 'has errors' do
-            expect(config.errors).not_to be_empty
-          end
-        end
-
-        describe '#errors' do
-          it 'returns an array of strings' do
-            expect(config.errors).to all(be_an_instance_of(String))
-          end
+      describe '#errors' do
+        it 'returns an array of strings' do
+          expect(config.errors).to all(be_an_instance_of(String))
         end
       end
     end

From 2240807c1aaa7d7df313dde9775e3ec99f7ad1b3 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 23 Jun 2016 10:07:42 +0200
Subject: [PATCH 10/28] Assume that unspecified CI config is undefined

We assume that when someone adds a key for the configuration entry, but
does not provide a valid value, which causes entry to be `nil`, then
entry should be considered as the undefined one. We also assume this is
semantically correct, this is also backwards compatible with legacy CI
config processor.

See issue #18775 for more details.
---
 lib/gitlab/ci/config/node/configurable.rb     |   1 -
 lib/gitlab/ci/config/node/factory.rb          |  18 +-
 lib/gitlab/ci/config/node/variables.rb        |   6 +-
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  |   8 +-
 .../lib/gitlab/ci/config/node/factory_spec.rb |   5 +-
 spec/lib/gitlab/ci/config/node/global_spec.rb | 177 +++++++++++-------
 .../gitlab/ci/config/node/services_spec.rb    |   4 +-
 .../gitlab/ci/config/node/variables_spec.rb   |  19 --
 8 files changed, 128 insertions(+), 110 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 25b5c2c9e2..0fb9092daf 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -27,7 +27,6 @@ module Gitlab
 
           def create_node(key, factory)
             factory.with(value: @config[key], key: key)
-            factory.undefine! unless @config.has_key?(key)
             factory.create!
           end
 
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 2271c386df..647b0c82a7 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -18,16 +18,20 @@ module Gitlab
             self
           end
 
-          def undefine!
-            @attributes[:value] = @node.dup
-            @node = Node::Undefined
-            self
-          end
-
           def create!
             raise InvalidFactory unless @attributes.has_key?(:value)
 
-            @node.new(@attributes[:value]).tap do |entry|
+            ##
+            # We assume unspecified entry is undefined.
+            # See issue #18775.
+            #
+            if @attributes[:value].nil?
+              node, value = Node::Undefined, @node
+            else
+              node, value = @node, @attributes[:value]
+            end
+
+            node.new(value).tap do |entry|
               entry.description = @attributes[:description]
               entry.key = @attributes[:key]
             end
diff --git a/lib/gitlab/ci/config/node/variables.rb b/lib/gitlab/ci/config/node/variables.rb
index fd3ce8715a..5f813f81f5 100644
--- a/lib/gitlab/ci/config/node/variables.rb
+++ b/lib/gitlab/ci/config/node/variables.rb
@@ -9,11 +9,7 @@ module Gitlab
           include Validatable
 
           validations do
-            validates :value, variables: true
-          end
-
-          def value
-            @config || self.class.default
+            validates :config, variables: true
           end
 
           def self.default
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 97a08758c0..eb20f5f4c0 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -551,8 +551,8 @@ module Ci
               config_processor = GitlabCiYamlProcessor.new(config, path)
 
               ##
-              #  TODO, in next version of CI configuration processor this
-              # should be invalid configuration, see #18775 and #15060
+              # When variables config is empty, we asumme this is a correct,
+              # see issue #18775
               #
               expect(config_processor.job_variables(:rspec))
                 .to be_an_instance_of(Array).and be_empty
@@ -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 value should be a hash of key value pairs")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Variables config 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 value should be a hash of key value pairs")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Variables config 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/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb
index 8e13e243d4..dd5f6e62b3 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -45,11 +45,10 @@ describe Gitlab::Ci::Config::Node::Factory do
       end
     end
 
-    context 'when creating undefined entry' do
-      it 'creates a null entry' do
+    context 'when creating entry with nil value' do
+      it 'creates an undefined entry' do
         entry = factory
           .with(value: nil)
-          .undefine!
           .create!
 
         expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index ef4b669c40..36a5b8041f 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -20,84 +20,125 @@ describe Gitlab::Ci::Config::Node::Global do
   end
 
   context 'when hash is valid' do
-    let(:hash) do
-      { before_script: ['ls', 'pwd'],
-        image: 'ruby:2.2',
-        services: ['postgres:9.1', 'mysql:5.5'],
-        variables: { VAR: 'value' },
-        after_script: ['make clean'] }
+    context 'when all entries defined' do
+      let(:hash) do
+        { before_script: ['ls', 'pwd'],
+          image: 'ruby:2.2',
+          services: ['postgres:9.1', 'mysql:5.5'],
+          variables: { VAR: 'value' },
+          after_script: ['make clean'] }
+      end
+
+      describe '#process!' do
+        before { global.process! }
+
+        it 'creates nodes hash' do
+          expect(global.nodes).to be_an Array
+        end
+
+        it 'creates node object for each entry' do
+          expect(global.nodes.count).to eq 5
+        end
+
+        it 'creates node object using valid class' do
+          expect(global.nodes.first)
+            .to be_an_instance_of Gitlab::Ci::Config::Node::Script
+          expect(global.nodes.second)
+            .to be_an_instance_of Gitlab::Ci::Config::Node::Image
+        end
+
+        it 'sets correct description for nodes' do
+          expect(global.nodes.first.description)
+            .to eq 'Script that will be executed before each job.'
+          expect(global.nodes.second.description)
+            .to eq 'Docker image that will be used to execute jobs.'
+        end
+      end
+
+      describe '#leaf?' do
+        it 'is not leaf' do
+          expect(global).not_to be_leaf
+        end
+      end
+
+      context 'when not processed' do
+        describe '#before_script' do
+          it 'returns nil' do
+            expect(global.before_script).to be nil
+          end
+        end
+      end
+
+      context 'when processed' do
+        before { global.process! }
+
+        describe '#before_script' do
+          it 'returns correct script' do
+            expect(global.before_script).to eq ['ls', 'pwd']
+          end
+        end
+
+        describe '#image' do
+          it 'returns valid image' do
+            expect(global.image).to eq 'ruby:2.2'
+          end
+        end
+
+        describe '#services' do
+          it 'returns array of services' do
+            expect(global.services).to eq ['postgres:9.1', 'mysql:5.5']
+          end
+        end
+
+        describe '#after_script' do
+          it 'returns after script' 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
 
-    describe '#process!' do
+    context 'when most of entires not defined' do
+      let(:hash) { { rspec: {} } }
       before { global.process! }
 
-      it 'creates nodes hash' do
-        expect(global.nodes).to be_an Array
-      end
-
-      it 'creates node object for each entry' do
-        expect(global.nodes.count).to eq 5
-      end
-
-      it 'creates node object using valid class' do
-        expect(global.nodes.first)
-          .to be_an_instance_of Gitlab::Ci::Config::Node::Script
-        expect(global.nodes.second)
-          .to be_an_instance_of Gitlab::Ci::Config::Node::Image
-      end
-
-      it 'sets correct description for nodes' do
-        expect(global.nodes.first.description)
-          .to eq 'Script that will be executed before each job.'
-        expect(global.nodes.second.description)
-          .to eq 'Docker image that will be used to execute jobs.'
-      end
-    end
-
-    describe '#leaf?' do
-      it 'is not leaf' do
-        expect(global).not_to be_leaf
-      end
-    end
-
-    context 'when not processed' do
-      describe '#before_script' do
-        it 'returns nil' do
-          expect(global.before_script).to be nil
+      describe '#nodes' do
+        it 'instantizes all nodes' do
+          expect(global.nodes.count).to eq 5
         end
-      end
-    end
 
-    context 'when processed' do
-      before { global.process! }
-
-      describe '#before_script' do
-        it 'returns correct script' do
-          expect(global.before_script).to eq ['ls', 'pwd']
-        end
-      end
-
-      describe '#image' do
-        it 'returns valid image' do
-          expect(global.image).to eq 'ruby:2.2'
-        end
-      end
-
-      describe '#services' do
-        it 'returns array of services' do
-          expect(global.services).to eq ['postgres:9.1', 'mysql:5.5']
-        end
-      end
-
-      describe '#after_script' do
-        it 'returns after script' do
-          expect(global.after_script).to eq ['make clean']
+        it 'contains undefined nodes' do
+          expect(global.nodes.last)
+            .to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
         end
       end
 
       describe '#variables' do
-        it 'returns variables' do
-          expect(global.variables).to eq(VAR: 'value')
+        it 'returns default value for variables' do
+          expect(global.variables).to eq({})
+        end
+      end
+    end
+
+    ##
+    # When nodes are specified but not defined, we assume that
+    # configuration is valid, and we asume that entry is simply undefined,
+    # despite the fact, that key is present. See issue #18775 for more
+    # details.
+    #
+    context 'when entires specified but not defined' do
+      let(:hash) { { variables: nil } }
+      before { global.process! }
+
+      describe '#variables' do
+        it 'undefined entry returns a default value' do
+          expect(global.variables).to eq({})
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/services_spec.rb b/spec/lib/gitlab/ci/config/node/services_spec.rb
index bda4b976cb..e38f6f6923 100644
--- a/spec/lib/gitlab/ci/config/node/services_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/services_spec.rb
@@ -3,9 +3,7 @@ require 'spec_helper'
 describe Gitlab::Ci::Config::Node::Services do
   let(:entry) { described_class.new(config) }
 
-  describe '#process!' do
-    before { entry.process! }
-
+  describe 'validations' do
     context 'when entry config value is correct' do
       let(:config) { ['postgres:9.1', 'mysql:5.5'] }
 
diff --git a/spec/lib/gitlab/ci/config/node/variables_spec.rb b/spec/lib/gitlab/ci/config/node/variables_spec.rb
index 67df70992b..4b6d971ec7 100644
--- a/spec/lib/gitlab/ci/config/node/variables_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/variables_spec.rb
@@ -44,24 +44,5 @@ describe Gitlab::Ci::Config::Node::Variables do
         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

From 29b96d92c163d71fe5a0fdf37d6a3c57c51141cd Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 23 Jun 2016 13:51:07 +0200
Subject: [PATCH 11/28] Move CI stages configuration to new CI config

---
 lib/ci/gitlab_ci_yaml_processor.rb            | 25 ++++------
 lib/gitlab/ci/config.rb                       |  2 +-
 lib/gitlab/ci/config/node/configurable.rb     | 12 +++--
 lib/gitlab/ci/config/node/entry.rb            |  4 ++
 lib/gitlab/ci/config/node/factory.rb          |  2 +-
 lib/gitlab/ci/config/node/global.rb           | 10 ++++
 lib/gitlab/ci/config/node/stages.rb           | 22 +++++++++
 lib/gitlab/ci/config/node/undefined.rb        |  4 ++
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  |  8 ++--
 spec/lib/gitlab/ci/config/node/global_spec.rb | 35 ++++++++++++--
 spec/lib/gitlab/ci/config/node/stages_spec.rb | 46 +++++++++++++++++++
 .../gitlab/ci/config/node/undefined_spec.rb   |  6 +++
 12 files changed, 147 insertions(+), 29 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/stages.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/stages_spec.rb

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 436b0127c3..f0c3eae661 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -4,7 +4,6 @@ module Ci
 
     include Gitlab::Ci::Config::Node::LegacyValidationHelpers
 
-    DEFAULT_STAGES = %w(build test deploy)
     DEFAULT_STAGE = 'test'
     ALLOWED_YAML_KEYS = [:before_script, :after_script, :image, :services, :types, :stages, :variables, :cache]
     ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services,
@@ -46,7 +45,7 @@ module Ci
     end
 
     def stages
-      @stages || DEFAULT_STAGES
+      @stages
     end
 
     def global_variables
@@ -68,8 +67,8 @@ module Ci
       @after_script = @ci_config.after_script
       @services = @ci_config.services
       @variables = @ci_config.variables
+      @stages = @ci_config.stages
 
-      @stages = @config[:stages] || @config[:types]
       @cache = @config[:cache]
       @jobs = {}
 
@@ -90,7 +89,7 @@ module Ci
 
     def build_job(name, job)
       {
-        stage_idx: stages.index(job[:stage]),
+        stage_idx: @stages.index(job[:stage]),
         stage: job[:stage],
         commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
@@ -112,7 +111,7 @@ module Ci
     end
 
     def validate!
-      validate_global!
+      validate_global_cache! if @cache
 
       @jobs.each do |name, job|
         validate_job!(name, job)
@@ -121,14 +120,6 @@ module Ci
       true
     end
 
-    def validate_global!
-      unless @stages.nil? || validate_array_of_strings(@stages)
-        raise ValidationError, "stages should be an array of strings"
-      end
-
-      validate_global_cache! if @cache
-    end
-
     def validate_global_cache!
       @cache.keys.each do |key|
         unless ALLOWED_CACHE_KEYS.include? key
@@ -225,8 +216,8 @@ module Ci
     end
 
     def validate_job_stage!(name, job)
-      unless job[:stage].is_a?(String) && job[:stage].in?(stages)
-        raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}"
+      unless job[:stage].is_a?(String) && job[:stage].in?(@stages)
+        raise ValidationError, "#{name} job: stage parameter should be #{@stages.join(", ")}"
       end
     end
 
@@ -290,12 +281,12 @@ module Ci
         raise ValidationError, "#{name} job: dependencies parameter should be an array of strings"
       end
 
-      stage_index = stages.index(job[:stage])
+      stage_index = @stages.index(job[:stage])
 
       job[:dependencies].each do |dependency|
         raise ValidationError, "#{name} job: undefined dependency: #{dependency}" unless @jobs[dependency.to_sym]
 
-        unless stages.index(@jobs[dependency.to_sym][:stage]) < stage_index
+        unless @stages.index(@jobs[dependency.to_sym][:stage]) < stage_index
           raise ValidationError, "#{name} job: dependency #{dependency} is not defined in prior stages"
         end
       end
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index 1bea9c21f6..61a2d2069a 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -8,7 +8,7 @@ module Gitlab
       # Temporary delegations that should be removed after refactoring
       #
       delegate :before_script, :image, :services, :after_script, :variables,
-               to: :@global
+               :stages, to: :@global
 
       def initialize(config)
         @config = Loader.new(config).load!
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 0fb9092daf..61e4f1cee2 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -38,14 +38,20 @@ module Gitlab
             private
 
             def node(symbol, entry_class, metadata)
-              factory = Node::Factory.new(entry_class)
-                .with(description: metadata[:description])
+              define_method("#{symbol}_defined?") do
+                @nodes[symbol].try(:defined?)
+              end
 
-              define_method(symbol) do
+              define_method("#{symbol}_value") do
                 raise Entry::InvalidError unless valid?
                 @nodes[symbol].try(:value)
               end
 
+              alias_method symbol.to_sym, "#{symbol}_value".to_sym
+
+              factory = Node::Factory.new(entry_class)
+                .with(description: metadata[:description])
+
               (@nodes ||= {}).merge!(symbol => factory)
             end
           end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 444a276d5c..e6f738b179 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -52,6 +52,10 @@ module Gitlab
             @config
           end
 
+          def defined?
+            true
+          end
+
           def self.default
           end
 
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 647b0c82a7..39b5784af2 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -22,7 +22,7 @@ module Gitlab
             raise InvalidFactory unless @attributes.has_key?(:value)
 
             ##
-            # We assume unspecified entry is undefined.
+            # We assume that unspecified entry is undefined.
             # See issue #18775.
             #
             if @attributes[:value].nil?
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index b5d177c528..88f9bb3f43 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -23,6 +23,16 @@ module Gitlab
 
           node :variables, Variables,
             description: 'Environment variables that will be used.'
+
+          node :stages, Stages,
+            description: 'Configuration of stages for this pipeline.'
+
+          node :types, Stages,
+            description: 'Stages for this pipeline (deprecated key).'
+
+          def stages
+            stages_defined? ? stages_value : types_value
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/stages.rb b/lib/gitlab/ci/config/node/stages.rb
new file mode 100644
index 0000000000..88d88252bc
--- /dev/null
+++ b/lib/gitlab/ci/config/node/stages.rb
@@ -0,0 +1,22 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a configuration for pipeline stages.
+        #
+        class Stages < Entry
+          include Validatable
+
+          validations do
+            validates :config, array_of_strings: true
+          end
+
+          def self.default
+            %w(build test deploy)
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index e8a69b810e..699605e1e3 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -19,6 +19,10 @@ module Gitlab
           def value
             @config.default
           end
+
+          def defined?
+            false
+          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 eb20f5f4c0..3f732d2ca2 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1081,17 +1081,17 @@ EOT
       end
 
       it "returns errors if stages is not an array" do
-        config = YAML.dump({ types: "test", rspec: { script: "test" } })
+        config = YAML.dump({ stages: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Stages config should be an array of strings")
       end
 
       it "returns errors if stages is not an array of strings" do
-        config = YAML.dump({ types: [true, "test"], rspec: { script: "test" } })
+        config = YAML.dump({ stages: [true, "test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Stages config should be an array of strings")
       end
 
       it "returns errors if variables is not a map" do
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 36a5b8041f..6aef6b913c 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -26,7 +26,8 @@ describe Gitlab::Ci::Config::Node::Global do
           image: 'ruby:2.2',
           services: ['postgres:9.1', 'mysql:5.5'],
           variables: { VAR: 'value' },
-          after_script: ['make clean'] }
+          after_script: ['make clean'],
+          stages: ['build', 'pages'] }
       end
 
       describe '#process!' do
@@ -37,7 +38,7 @@ describe Gitlab::Ci::Config::Node::Global do
         end
 
         it 'creates node object for each entry' do
-          expect(global.nodes.count).to eq 5
+          expect(global.nodes.count).to eq 7
         end
 
         it 'creates node object using valid class' do
@@ -101,6 +102,22 @@ describe Gitlab::Ci::Config::Node::Global do
             expect(global.variables).to eq(VAR: 'value')
           end
         end
+
+        describe '#stages' do
+          context 'when stages key defined' do
+            it 'returns array of stages' do
+              expect(global.stages).to eq %w[build pages]
+            end
+          end
+
+          context 'when deprecated types key defined' do
+            let(:hash) { { types: ['test', 'deploy'] } }
+
+            it 'returns array of types as stages' do
+              expect(global.stages).to eq %w[test deploy]
+            end
+          end
+        end
       end
     end
 
@@ -110,7 +127,7 @@ describe Gitlab::Ci::Config::Node::Global do
 
       describe '#nodes' do
         it 'instantizes all nodes' do
-          expect(global.nodes.count).to eq 5
+          expect(global.nodes.count).to eq 7
         end
 
         it 'contains undefined nodes' do
@@ -124,6 +141,12 @@ describe Gitlab::Ci::Config::Node::Global do
           expect(global.variables).to eq({})
         end
       end
+
+      describe '#stages' do
+        it 'returns an array of default stages' do
+          expect(global.stages).to eq %w[build test deploy]
+        end
+      end
     end
 
     ##
@@ -188,4 +211,10 @@ describe Gitlab::Ci::Config::Node::Global do
       end
     end
   end
+
+  describe '#defined?' do
+    it 'is concrete entry that is defined' do
+      expect(global.defined?).to be true
+    end
+  end
 end
diff --git a/spec/lib/gitlab/ci/config/node/stages_spec.rb b/spec/lib/gitlab/ci/config/node/stages_spec.rb
new file mode 100644
index 0000000000..dbf2eb8993
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/stages_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Stages do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { [:stage1, :stage2] }
+
+      describe '#value' do
+        it 'returns array of stages' do
+          expect(entry.value).to eq config
+        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) { { test: true } }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'Stages config should be an array of strings'
+        end
+      end
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+    end
+  end
+
+  describe '.default' do
+    it 'returns default stages' do
+      expect(described_class.default).to eq %w[build test deploy]
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/undefined_spec.rb b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
index 5ded0504a3..4318dfe6e5 100644
--- a/spec/lib/gitlab/ci/config/node/undefined_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
@@ -31,4 +31,10 @@ describe Gitlab::Ci::Config::Node::Undefined do
       expect(undefined.value).to eq 'some value'
     end
   end
+
+  describe '#undefined?' do
+    it 'is not a concrete entry that is defined' do
+      expect(undefined.defined?).to be false
+    end
+  end
 end

From 1f320edb7721bcc86d26add7ba3fcbd185a1ca06 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 23 Jun 2016 13:51:34 +0200
Subject: [PATCH 12/28] Minor refactorings in new CI configuration classes

---
 lib/ci/gitlab_ci_yaml_processor.rb               | 5 +++++
 lib/gitlab/ci/config/node/configurable.rb        | 2 +-
 lib/gitlab/ci/config/node/entry.rb               | 3 +--
 lib/gitlab/ci/config/node/validator.rb           | 2 +-
 spec/lib/gitlab/ci/config/node/validator_spec.rb | 4 ++--
 5 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index f0c3eae661..e471019568 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -91,6 +91,11 @@ module Ci
       {
         stage_idx: @stages.index(job[:stage]),
         stage: job[:stage],
+        ##
+        # Refactoring note:
+        #  - before script behaves differently than after script
+        #  - after script returns an array of commands
+        #  - before script should be a concatenated command
         commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
         name: name,
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 61e4f1cee2..590cf3d7b7 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -52,7 +52,7 @@ module Gitlab
               factory = Node::Factory.new(entry_class)
                 .with(description: metadata[:description])
 
-              (@nodes ||= {}).merge!(symbol => factory)
+              (@nodes ||= {}).merge!(symbol.to_sym => factory)
             end
           end
         end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index e6f738b179..08d8020f8e 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -44,8 +44,7 @@ module Gitlab
           end
 
           def errors
-            @validator.full_errors +
-              nodes.map(&:errors).flatten
+            @validator.messages + nodes.flat_map(&:errors)
           end
 
           def value
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index 02edc9219c..5f62d68710 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -11,7 +11,7 @@ module Gitlab
             @node = node
           end
 
-          def full_errors
+          def messages
             errors.full_messages.map do |error|
               "#{@node.key} #{error}".humanize
             end
diff --git a/spec/lib/gitlab/ci/config/node/validator_spec.rb b/spec/lib/gitlab/ci/config/node/validator_spec.rb
index ad875d5538..aa55ce90b3 100644
--- a/spec/lib/gitlab/ci/config/node/validator_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/validator_spec.rb
@@ -19,7 +19,7 @@ describe Gitlab::Ci::Config::Node::Validator do
       it 'returns no errors' do
         validator_instance.validate
 
-        expect(validator_instance.full_errors).to be_empty
+        expect(validator_instance.messages).to be_empty
       end
     end
 
@@ -36,7 +36,7 @@ describe Gitlab::Ci::Config::Node::Validator do
       it 'returns errors' do
         validator_instance.validate
 
-        expect(validator_instance.full_errors).not_to be_empty
+        expect(validator_instance.messages).not_to be_empty
       end
     end
   end

From 823970b570463bb011fbdc1117a1450310763da0 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 24 Jun 2016 08:25:10 +0200
Subject: [PATCH 13/28] Fix ci config cache validation in legacy processor

---
 lib/ci/gitlab_ci_yaml_processor.rb           |  4 ++--
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index e471019568..33492775fe 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -127,8 +127,8 @@ module Ci
 
     def validate_global_cache!
       @cache.keys.each do |key|
-        unless ALLOWED_CACHE_KEYS.include? key
-          raise ValidationError, "#{name} cache unknown parameter #{key}"
+        unless ALLOWED_CACHE_KEYS.include?(key)
+          raise ValidationError, "Cache config has unknown parameter: #{key}"
         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 3f732d2ca2..6ef6a59f18 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -591,7 +591,20 @@ module Ci
       end
     end
 
-    describe "Caches" do
+    describe 'cache' do
+      context 'when cache definition has unknown keys' do
+        it 'raises relevant validation error' do
+          config = YAML.dump(
+            { cache: { untracked: true, invalid: 'key' },
+              rspec: { script: 'rspec' } })
+
+          expect { GitlabCiYamlProcessor.new(config) }.to raise_error(
+            GitlabCiYamlProcessor::ValidationError,
+            'Cache config has unknown parameter: invalid'
+          )
+        end
+      end
+
       it "returns cache when defined globally" do
         config = YAML.dump({
                              cache: { paths: ["logs/", "binaries/"], untracked: true, key: 'key' },

From 04ece6664a04e7c352582100bdd6e8d78c3ea7cc Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 24 Jun 2016 08:58:09 +0200
Subject: [PATCH 14/28] Add ci config class that represents a key value

---
 lib/gitlab/ci/config/node/key.rb           | 18 ++++++++++++
 lib/gitlab/ci/config/node/validators.rb    | 10 +++++++
 spec/lib/gitlab/ci/config/node/key_spec.rb | 34 ++++++++++++++++++++++
 3 files changed, 62 insertions(+)
 create mode 100644 lib/gitlab/ci/config/node/key.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/key_spec.rb

diff --git a/lib/gitlab/ci/config/node/key.rb b/lib/gitlab/ci/config/node/key.rb
new file mode 100644
index 0000000000..f8b461ca09
--- /dev/null
+++ b/lib/gitlab/ci/config/node/key.rb
@@ -0,0 +1,18 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a key.
+        #
+        class Key < Entry
+          include Validatable
+
+          validations do
+            validates :config, key: true
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index 56f7661daf..f2b3a8a3f8 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -13,6 +13,16 @@ module Gitlab
             end
           end
 
+          class KeyValidator < ActiveModel::EachValidator
+            include LegacyValidationHelpers
+
+            def validate_each(record, attribute, value)
+              unless validate_string(value)
+                record.errors.add(attribute, 'should be a string or symbol')
+              end
+            end
+          end
+
           class TypeValidator < ActiveModel::EachValidator
             def validate_each(record, attribute, value)
               type = options[:with]
diff --git a/spec/lib/gitlab/ci/config/node/key_spec.rb b/spec/lib/gitlab/ci/config/node/key_spec.rb
new file mode 100644
index 0000000000..23e7fc4620
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/key_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Key do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { 'test' }
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq 'test'
+        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) { [ 'incorrect' ] }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'Key config should be a string or symbol'
+        end
+      end
+    end
+  end
+end

From e017e1b62904fe323be359f1ac406c951dcd4ccd Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 24 Jun 2016 09:49:54 +0200
Subject: [PATCH 15/28] Add ci config class that represents a boolean value

---
 lib/gitlab/ci/config/node/boolean.rb          | 18 ++++++++++
 lib/gitlab/ci/config/node/validators.rb       | 10 ++++++
 .../lib/gitlab/ci/config/node/boolean_spec.rb | 34 +++++++++++++++++++
 3 files changed, 62 insertions(+)
 create mode 100644 lib/gitlab/ci/config/node/boolean.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/boolean_spec.rb

diff --git a/lib/gitlab/ci/config/node/boolean.rb b/lib/gitlab/ci/config/node/boolean.rb
new file mode 100644
index 0000000000..84b03ee783
--- /dev/null
+++ b/lib/gitlab/ci/config/node/boolean.rb
@@ -0,0 +1,18 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a boolean value.
+        #
+        class Boolean < Entry
+          include Validatable
+
+          validations do
+            validates :config, boolean: true
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index f2b3a8a3f8..4082c161e8 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -13,6 +13,16 @@ module Gitlab
             end
           end
 
+          class BooleanValidator < ActiveModel::EachValidator
+            include LegacyValidationHelpers
+
+            def validate_each(record, attribute, value)
+              unless validate_boolean(value)
+                record.errors.add(attribute, 'should be a boolean value')
+              end
+            end
+          end
+
           class KeyValidator < ActiveModel::EachValidator
             include LegacyValidationHelpers
 
diff --git a/spec/lib/gitlab/ci/config/node/boolean_spec.rb b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
new file mode 100644
index 0000000000..97f13b2d5f
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Boolean do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is valid' do
+      let(:config) { false }
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq false
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not valid' do
+      let(:config) { [ 'incorrect' ] }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'Boolean config should be a boolean value'
+        end
+      end
+    end
+  end
+end

From ce4478ed86e7487ea6bb45703561d1d5539ef5b4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 24 Jun 2016 09:54:52 +0200
Subject: [PATCH 16/28] Add ci config entry that represents array of paths

---
 lib/gitlab/ci/config/node/paths.rb           | 18 +++++++++++
 spec/lib/gitlab/ci/config/node/paths_spec.rb | 34 ++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 lib/gitlab/ci/config/node/paths.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/paths_spec.rb

diff --git a/lib/gitlab/ci/config/node/paths.rb b/lib/gitlab/ci/config/node/paths.rb
new file mode 100644
index 0000000000..3c6d3a5296
--- /dev/null
+++ b/lib/gitlab/ci/config/node/paths.rb
@@ -0,0 +1,18 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents an array of paths.
+        #
+        class Paths < Entry
+          include Validatable
+
+          validations do
+            validates :config, array_of_strings: true
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/paths_spec.rb b/spec/lib/gitlab/ci/config/node/paths_spec.rb
new file mode 100644
index 0000000000..0d95ad8abd
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/paths_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Paths do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is valid' do
+      let(:config) { ['some/file', 'some/path/'] }
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not valid' do
+      let(:config) { [ 1 ] }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'Paths config should be an array of strings'
+        end
+      end
+    end
+  end
+end

From 56e88b8c28282976be258ba53a9f82662cc74703 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 24 Jun 2016 10:13:55 +0200
Subject: [PATCH 17/28] Add new ci config entry that handles cache config

---
 lib/gitlab/ci/config/node/cache.rb            | 39 ++++++++++++
 lib/gitlab/ci/config/node/configurable.rb     | 28 +++++----
 lib/gitlab/ci/config/node/global.rb           |  3 +
 spec/lib/gitlab/ci/config/node/cache_spec.rb  | 60 +++++++++++++++++++
 .../ci/config/node/configurable_spec.rb       | 33 ++++++++++
 5 files changed, 151 insertions(+), 12 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/cache.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/cache_spec.rb

diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
new file mode 100644
index 0000000000..c6508e59c4
--- /dev/null
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -0,0 +1,39 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a cache configuration
+        #
+        class Cache < Entry
+          include Configurable
+
+          validations do
+            validate :allowed_keys
+
+            def unknown_keys
+              return [] unless @node.config.is_a?(Hash)
+
+              @node.config.keys - @node.class.nodes.keys
+            end
+
+            def allowed_keys
+              if unknown_keys.any?
+                errors.add(:config, "contains unknown keys #{unknown_keys}")
+              end
+            end
+          end
+
+          node :key, Node::Key,
+            description: 'Cache key used to define a cache affinity.'
+
+          node :untracked, Boolean,
+            description: 'Cache all untracked files.'
+
+          node :paths, Paths,
+            description: 'Specify which paths should be cached across builds.'
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 590cf3d7b7..46a473ad09 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -32,28 +32,32 @@ module Gitlab
 
           class_methods do
             def nodes
-              Hash[@nodes.map { |key, factory| [key, factory.dup] }]
+              Hash[(@nodes || {}).map { |key, factory| [key, factory.dup] }]
             end
 
             private
 
             def node(symbol, entry_class, metadata)
-              define_method("#{symbol}_defined?") do
-                @nodes[symbol].try(:defined?)
-              end
-
-              define_method("#{symbol}_value") do
-                raise Entry::InvalidError unless valid?
-                @nodes[symbol].try(:value)
-              end
-
-              alias_method symbol.to_sym, "#{symbol}_value".to_sym
-
               factory = Node::Factory.new(entry_class)
                 .with(description: metadata[:description])
 
               (@nodes ||= {}).merge!(symbol.to_sym => factory)
             end
+
+            def helpers(*nodes)
+              nodes.each do |symbol|
+                define_method("#{symbol}_defined?") do
+                  @nodes[symbol].try(:defined?)
+                end
+
+                define_method("#{symbol}_value") do
+                  raise Entry::InvalidError unless valid?
+                  @nodes[symbol].try(:value)
+                end
+
+                alias_method symbol.to_sym, "#{symbol}_value".to_sym
+              end
+            end
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 88f9bb3f43..4ca379712c 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -30,6 +30,9 @@ module Gitlab
           node :types, Stages,
             description: 'Stages for this pipeline (deprecated key).'
 
+          helpers :before_script, :image, :services, :after_script, :variables,
+                  :stages, :types
+
           def stages
             stages_defined? ? stages_value : types_value
           end
diff --git a/spec/lib/gitlab/ci/config/node/cache_spec.rb b/spec/lib/gitlab/ci/config/node/cache_spec.rb
new file mode 100644
index 0000000000..d6428f6b99
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/cache_spec.rb
@@ -0,0 +1,60 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Cache do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    before { entry.process! }
+
+    context 'when entry config value is correct' do
+      let(:config) do
+        { key: 'some key',
+          untracked: true,
+          paths: ['some/path/'] }
+      end
+
+      describe '#value' do
+        it 'returns hash value' do
+          expect(entry.value).to eq config
+        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
+      describe '#errors' do
+        context 'when is not a hash' do
+          let(:config) { 'ls' }
+
+          it 'reports errors with config value' do
+            expect(entry.errors)
+              .to include 'Cache config should be a hash'
+          end
+        end
+
+        context 'when descendants are invalid' do
+          let(:config) { { key: 1 } }
+
+          it 'reports error with descendants' do
+            expect(entry.errors)
+              .to include 'Key config should be a string or symbol'
+          end
+        end
+
+        context 'when there is an unknown key present' do
+          let(:config) { { invalid: true } }
+
+          it 'reports error with descendants' do
+            expect(entry.errors)
+              .to include 'Cache config contains unknown keys [:invalid]'
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/configurable_spec.rb b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
index 2ac436cb4b..4a1550517f 100644
--- a/spec/lib/gitlab/ci/config/node/configurable_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
@@ -7,6 +7,39 @@ describe Gitlab::Ci::Config::Node::Configurable do
     node.include(described_class)
   end
 
+  describe 'validations' do
+    let(:validator) { node.validator.new(instance) }
+
+    before do
+      node.class_eval do
+        attr_reader :config
+
+        def initialize(config)
+          @config = config
+        end
+      end
+
+      validator.validate
+    end
+
+
+    context 'when node validator is invalid' do
+      let(:instance) { node.new('ls') }
+
+      it 'returns invalid validator' do
+        expect(validator).to be_invalid
+      end
+    end
+
+    context 'when node instance is valid' do
+      let(:instance) { node.new(key: 'value') }
+
+      it 'returns valid validator' do
+        expect(validator).to be_valid
+      end
+    end
+  end
+
   describe 'configured nodes' do
     before do
       node.class_eval do

From c019585cb83b1852451184663085e6f0e0d12024 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 27 Jun 2016 14:12:47 +0200
Subject: [PATCH 18/28] Validate interface only with CI node validator

---
 lib/gitlab/ci/config/node/cache.rb            | 37 ++++++++++---------
 lib/gitlab/ci/config/node/validator.rb        |  3 +-
 .../gitlab/ci/config/node/validator_spec.rb   | 32 ++++------------
 3 files changed, 28 insertions(+), 44 deletions(-)

diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
index c6508e59c4..251d7aa909 100644
--- a/lib/gitlab/ci/config/node/cache.rb
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -8,23 +8,7 @@ module Gitlab
         class Cache < Entry
           include Configurable
 
-          validations do
-            validate :allowed_keys
-
-            def unknown_keys
-              return [] unless @node.config.is_a?(Hash)
-
-              @node.config.keys - @node.class.nodes.keys
-            end
-
-            def allowed_keys
-              if unknown_keys.any?
-                errors.add(:config, "contains unknown keys #{unknown_keys}")
-              end
-            end
-          end
-
-          node :key, Node::Key,
+          node :key, Key,
             description: 'Cache key used to define a cache affinity.'
 
           node :untracked, Boolean,
@@ -32,6 +16,25 @@ module Gitlab
 
           node :paths, Paths,
             description: 'Specify which paths should be cached across builds.'
+
+          validations do
+            validate :keys
+
+            def unknown_keys
+              return [] unless config.is_a?(Hash)
+              config.keys - allowed_keys
+            end
+
+            def keys
+              if unknown_keys.any?
+                errors.add(:config, "contains unknown keys #{unknown_keys}")
+              end
+            end
+          end
+
+          def allowed_keys
+            self.class.nodes.keys
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index 5f62d68710..18e795d2c4 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -8,12 +8,11 @@ module Gitlab
 
           def initialize(node)
             super(node)
-            @node = node
           end
 
           def messages
             errors.full_messages.map do |error|
-              "#{@node.key} #{error}".humanize
+              "#{key} #{error}".humanize
             end
           end
 
diff --git a/spec/lib/gitlab/ci/config/node/validator_spec.rb b/spec/lib/gitlab/ci/config/node/validator_spec.rb
index aa55ce90b3..c293faa33a 100644
--- a/spec/lib/gitlab/ci/config/node/validator_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/validator_spec.rb
@@ -5,7 +5,13 @@ describe Gitlab::Ci::Config::Node::Validator do
   let(:validator_instance) { validator.new(node) }
   let(:node) { spy('node') }
 
-  shared_examples 'delegated validator' do
+  describe 'delegated validator' do
+    before do
+      validator.class_eval do
+        validates :test_attribute, presence: true
+      end
+    end
+
     context 'when node is valid' do
       before do
         allow(node).to receive(:test_attribute).and_return('valid value')
@@ -40,28 +46,4 @@ describe Gitlab::Ci::Config::Node::Validator do
       end
     end
   end
-
-  describe 'attributes validations' do
-    before do
-      validator.class_eval do
-        validates :test_attribute, presence: true
-      end
-    end
-
-    it_behaves_like 'delegated validator'
-  end
-
-  describe 'interface validations' do
-    before do
-      validator.class_eval do
-        validate do
-          unless @node.test_attribute == 'valid value'
-            errors.add(:test_attribute, 'invalid value')
-          end
-        end
-      end
-    end
-
-    it_behaves_like 'delegated validator'
-  end
 end

From 7c511c2f55f3e181983253d8b3ae74cd84e6844c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 29 Jun 2016 09:05:14 +0200
Subject: [PATCH 19/28] Make it possible to set parent in CI config node

---
 lib/gitlab/ci/config/node/entry.rb             |  4 ++--
 lib/gitlab/ci/config/node/factory.rb           |  3 ++-
 spec/lib/gitlab/ci/config/node/factory_spec.rb | 16 ++++++++++++++--
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 08d8020f8e..f22dac4483 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -8,9 +8,9 @@ module Gitlab
         class Entry
           class InvalidError < StandardError; end
 
-          attr_reader :config
-          attr_accessor :description
           attr_writer :key
+          attr_reader :config
+          attr_accessor :parent, :description
 
           def initialize(config)
             @config = config
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 39b5784af2..85e28f345f 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -32,8 +32,9 @@ module Gitlab
             end
 
             node.new(value).tap do |entry|
-              entry.description = @attributes[:description]
               entry.key = @attributes[:key]
+              entry.parent = @attributes[:parent]
+              entry.description = @attributes[:description]
             end
           end
         end
diff --git a/spec/lib/gitlab/ci/config/node/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb
index dd5f6e62b3..91ddef7bfb 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -5,7 +5,7 @@ describe Gitlab::Ci::Config::Node::Factory do
     let(:factory) { described_class.new(entry_class) }
     let(:entry_class) { Gitlab::Ci::Config::Node::Script }
 
-    context 'when value setting value' do
+    context 'when setting up a value' do
       it 'creates entry with valid value' do
         entry = factory
           .with(value: ['ls', 'pwd'])
@@ -35,9 +35,21 @@ describe Gitlab::Ci::Config::Node::Factory do
           expect(entry.key).to eq 'test key'
         end
       end
+
+      context 'when setting a parent' do
+        let(:parent) { Object.new }
+
+        it 'creates entry with valid parent' do
+          entry = factory
+            .with(value: 'ls', parent: parent)
+            .create!
+
+          expect(entry.parent).to eq parent
+        end
+      end
     end
 
-    context 'when not setting value' do
+    context 'when not setting up a value' do
       it 'raises error' do
         expect { factory.create! }.to raise_error(
           Gitlab::Ci::Config::Node::Factory::InvalidFactory

From 92312786f13c72188abbbe4f0b6cbdd36de2331d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 29 Jun 2016 09:10:23 +0200
Subject: [PATCH 20/28] Add CI config entry location info to error message

This CI config entry location in configuration Hash.
---
 lib/gitlab/ci/config/node/entry.rb               | 7 +------
 lib/gitlab/ci/config/node/validator.rb           | 9 ++++++++-
 spec/lib/gitlab/ci/config/node/global_spec.rb    | 6 ------
 spec/lib/gitlab/ci/config/node/validator_spec.rb | 7 ++++++-
 4 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index f22dac4483..17d04fbdfe 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -8,9 +8,8 @@ module Gitlab
         class Entry
           class InvalidError < StandardError; end
 
-          attr_writer :key
           attr_reader :config
-          attr_accessor :parent, :description
+          attr_accessor :key, :parent, :description
 
           def initialize(config)
             @config = config
@@ -35,10 +34,6 @@ module Gitlab
             self.class.nodes.none?
           end
 
-          def key
-            @key || self.class.name.demodulize.underscore
-          end
-
           def valid?
             errors.none?
           end
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index 18e795d2c4..d898d52154 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -8,17 +8,24 @@ module Gitlab
 
           def initialize(node)
             super(node)
+            @node = node
           end
 
           def messages
             errors.full_messages.map do |error|
-              "#{key} #{error}".humanize
+              "#{location} #{error}".humanize
             end
           end
 
           def self.name
             'Validator'
           end
+
+          private
+
+          def location
+            key || @node.class.name.demodulize.underscore
+          end
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 6aef6b913c..cf7ab13c8b 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -13,12 +13,6 @@ describe Gitlab::Ci::Config::Node::Global do
     end
   end
 
-  describe '#key' do
-    it 'returns underscored class name' do
-      expect(global.key).to eq 'global'
-    end
-  end
-
   context 'when hash is valid' do
     context 'when all entries defined' do
       let(:hash) do
diff --git a/spec/lib/gitlab/ci/config/node/validator_spec.rb b/spec/lib/gitlab/ci/config/node/validator_spec.rb
index c293faa33a..87a1bbf55c 100644
--- a/spec/lib/gitlab/ci/config/node/validator_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/validator_spec.rb
@@ -5,6 +5,10 @@ describe Gitlab::Ci::Config::Node::Validator do
   let(:validator_instance) { validator.new(node) }
   let(:node) { spy('node') }
 
+  before do
+    allow(node).to receive(:key).and_return('node')
+  end
+
   describe 'delegated validator' do
     before do
       validator.class_eval do
@@ -42,7 +46,8 @@ describe Gitlab::Ci::Config::Node::Validator do
       it 'returns errors' do
         validator_instance.validate
 
-        expect(validator_instance.messages).not_to be_empty
+        expect(validator_instance.messages)
+          .to include "Node test attribute can't be blank"
       end
     end
   end

From f4421817de474cda3598eac8cad0752d324608e1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 29 Jun 2016 09:39:04 +0200
Subject: [PATCH 21/28] Add global cache config entry to new CI config

---
 lib/gitlab/ci/config/node/cache.rb            |  3 ++-
 lib/gitlab/ci/config/node/configurable.rb     |  5 +++-
 lib/gitlab/ci/config/node/entry.rb            |  4 +++
 lib/gitlab/ci/config/node/global.rb           |  7 +++--
 lib/gitlab/ci/config/node/validator.rb        |  6 +++--
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  | 26 +++++++++----------
 .../lib/gitlab/ci/config/node/boolean_spec.rb |  2 +-
 spec/lib/gitlab/ci/config/node/cache_spec.rb  |  6 ++---
 spec/lib/gitlab/ci/config/node/global_spec.rb | 26 ++++++++++++++-----
 spec/lib/gitlab/ci/config/node/image_spec.rb  |  2 +-
 spec/lib/gitlab/ci/config/node/key_spec.rb    |  2 +-
 spec/lib/gitlab/ci/config/node/paths_spec.rb  |  2 +-
 spec/lib/gitlab/ci/config/node/script_spec.rb |  2 +-
 .../gitlab/ci/config/node/services_spec.rb    |  2 +-
 spec/lib/gitlab/ci/config/node/stages_spec.rb |  2 +-
 .../gitlab/ci/config/node/validator_spec.rb   |  3 ++-
 16 files changed, 64 insertions(+), 36 deletions(-)

diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
index 251d7aa909..01a9ef511e 100644
--- a/lib/gitlab/ci/config/node/cache.rb
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -27,7 +27,8 @@ module Gitlab
 
             def keys
               if unknown_keys.any?
-                errors.add(:config, "contains unknown keys #{unknown_keys}")
+                unknown_list = unknown_keys.join(', ')
+                errors.add(:config, "contains unknown keys: #{unknown_list}")
               end
             end
           end
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 46a473ad09..4889a21a23 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -26,7 +26,10 @@ module Gitlab
           private
 
           def create_node(key, factory)
-            factory.with(value: @config[key], key: key)
+            factory.with(value: @config[key])
+            factory.with(parent: self)
+            factory.with(key: key)
+
             factory.create!
           end
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 17d04fbdfe..985d170519 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -34,6 +34,10 @@ module Gitlab
             self.class.nodes.none?
           end
 
+          def ancestors
+            @parent ? @parent.ancestors + [@parent] : []
+          end
+
           def valid?
             errors.none?
           end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 4ca379712c..65919ef1ee 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -30,8 +30,11 @@ module Gitlab
           node :types, Stages,
             description: 'Stages for this pipeline (deprecated key).'
 
-          helpers :before_script, :image, :services, :after_script, :variables,
-                  :stages, :types
+          node :cache, Cache,
+            description: 'Configure caching between build jobs.'
+
+          helpers :before_script, :image, :services, :after_script,
+                  :variables, :stages, :types, :cache
 
           def stages
             stages_defined? ? stages_value : types_value
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index d898d52154..94a8af4d08 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -13,7 +13,7 @@ module Gitlab
 
           def messages
             errors.full_messages.map do |error|
-              "#{location} #{error}".humanize
+              "#{location} #{error}".downcase
             end
           end
 
@@ -24,7 +24,9 @@ module Gitlab
           private
 
           def location
-            key || @node.class.name.demodulize.underscore
+            predecessors = ancestors.map(&:key).compact
+            current = key || @node.class.name.demodulize.underscore
+            predecessors.append(current).join(':')
           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 147301b312..262a91fedf 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -600,7 +600,7 @@ module Ci
 
           expect { GitlabCiYamlProcessor.new(config) }.to raise_error(
             GitlabCiYamlProcessor::ValidationError,
-            'Cache config has unknown parameter: invalid'
+            'cache config contains unknown keys: invalid'
           )
         end
       end
@@ -964,7 +964,7 @@ EOT
         config = YAML.dump({ before_script: "bundle update", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Before script config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script config should be an array of strings")
       end
 
       it "returns errors if job before_script parameter is not an array of strings" do
@@ -978,7 +978,7 @@ EOT
         config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "After script config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script config should be an array of strings")
       end
 
       it "returns errors if job after_script parameter is not an array of strings" do
@@ -992,7 +992,7 @@ EOT
         config = YAML.dump({ image: ["test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Image config should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image config should be a string")
       end
 
       it "returns errors if job name is blank" do
@@ -1020,14 +1020,14 @@ EOT
         config = YAML.dump({ services: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Services config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services config should be an array of strings")
       end
 
       it "returns errors if services parameter is not an array of strings" do
         config = YAML.dump({ services: [10, "test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Services config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services config should be an array of strings")
       end
 
       it "returns errors if job services parameter is not an array" do
@@ -1097,28 +1097,28 @@ EOT
         config = YAML.dump({ stages: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Stages config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages config should be an array of strings")
       end
 
       it "returns errors if stages is not an array of strings" do
         config = YAML.dump({ stages: [true, "test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Stages config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages config should be an array of strings")
       end
 
       it "returns errors if variables is not a map" do
         config = YAML.dump({ variables: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Variables config should be a hash of key value pairs")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables config 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 config should be a hash of key value pairs")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables config should be a hash of key value pairs")
       end
 
       it "returns errors if job when is not on_success, on_failure or always" do
@@ -1174,21 +1174,21 @@ EOT
         config = YAML.dump({ cache: { untracked: "string" }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:untracked parameter should be an boolean")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:untracked config should be a boolean value")
       end
 
       it "returns errors if cache:paths is not an array of strings" do
         config = YAML.dump({ cache: { paths: "string" }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:paths parameter should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:paths config should be an array of strings")
       end
 
       it "returns errors if cache:key is not a string" do
         config = YAML.dump({ cache: { key: 1 }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:key parameter should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:key config should be a string or symbol")
       end
 
       it "returns errors if job cache:key is not an a string" do
diff --git a/spec/lib/gitlab/ci/config/node/boolean_spec.rb b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
index 97f13b2d5f..32639296e6 100644
--- a/spec/lib/gitlab/ci/config/node/boolean_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Boolean do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Boolean config should be a boolean value'
+            .to include 'boolean config should be a boolean value'
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/cache_spec.rb b/spec/lib/gitlab/ci/config/node/cache_spec.rb
index d6428f6b99..50f619ce26 100644
--- a/spec/lib/gitlab/ci/config/node/cache_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/cache_spec.rb
@@ -33,7 +33,7 @@ describe Gitlab::Ci::Config::Node::Cache do
 
           it 'reports errors with config value' do
             expect(entry.errors)
-              .to include 'Cache config should be a hash'
+              .to include 'cache config should be a hash'
           end
         end
 
@@ -42,7 +42,7 @@ describe Gitlab::Ci::Config::Node::Cache do
 
           it 'reports error with descendants' do
             expect(entry.errors)
-              .to include 'Key config should be a string or symbol'
+              .to include 'key config should be a string or symbol'
           end
         end
 
@@ -51,7 +51,7 @@ describe Gitlab::Ci::Config::Node::Cache do
 
           it 'reports error with descendants' do
             expect(entry.errors)
-              .to include 'Cache config contains unknown keys [:invalid]'
+              .to include 'cache config contains unknown keys: invalid'
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index cf7ab13c8b..c87c9e97bc 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -21,7 +21,8 @@ describe Gitlab::Ci::Config::Node::Global do
           services: ['postgres:9.1', 'mysql:5.5'],
           variables: { VAR: 'value' },
           after_script: ['make clean'],
-          stages: ['build', 'pages'] }
+          stages: ['build', 'pages'],
+          cache: { key: 'k', untracked: true, paths: ['public/'] } }
       end
 
       describe '#process!' do
@@ -32,7 +33,7 @@ describe Gitlab::Ci::Config::Node::Global do
         end
 
         it 'creates node object for each entry' do
-          expect(global.nodes.count).to eq 7
+          expect(global.nodes.count).to eq 8
         end
 
         it 'creates node object using valid class' do
@@ -112,20 +113,27 @@ describe Gitlab::Ci::Config::Node::Global do
             end
           end
         end
+
+        describe '#cache' do
+          it 'returns cache configuration' do
+            expect(global.cache)
+              .to eq(key: 'k', untracked: true, paths: ['public/'])
+          end
+        end
       end
     end
 
     context 'when most of entires not defined' do
-      let(:hash) { { rspec: {} } }
+      let(:hash) { { cache: { key: 'a' }, rspec: {} } }
       before { global.process! }
 
       describe '#nodes' do
         it 'instantizes all nodes' do
-          expect(global.nodes.count).to eq 7
+          expect(global.nodes.count).to eq 8
         end
 
         it 'contains undefined nodes' do
-          expect(global.nodes.last)
+          expect(global.nodes.first)
             .to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
         end
       end
@@ -141,6 +149,12 @@ describe Gitlab::Ci::Config::Node::Global do
           expect(global.stages).to eq %w[build test deploy]
         end
       end
+
+      describe '#cache' do
+        it 'returns correct cache definition' do
+          expect(global.cache).to eq(key: 'a')
+        end
+      end
     end
 
     ##
@@ -177,7 +191,7 @@ describe Gitlab::Ci::Config::Node::Global do
     describe '#errors' do
       it 'reports errors from child nodes' do
         expect(global.errors)
-          .to include 'Before script config should be an array of strings'
+          .to include 'before_script config should be an array of strings'
       end
     end
 
diff --git a/spec/lib/gitlab/ci/config/node/image_spec.rb b/spec/lib/gitlab/ci/config/node/image_spec.rb
index 0b0821ca55..d11bb39f32 100644
--- a/spec/lib/gitlab/ci/config/node/image_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/image_spec.rb
@@ -32,7 +32,7 @@ describe Gitlab::Ci::Config::Node::Image do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Image config should be a string'
+            .to include 'image config should be a string'
         end
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/key_spec.rb b/spec/lib/gitlab/ci/config/node/key_spec.rb
index 23e7fc4620..8cda43173f 100644
--- a/spec/lib/gitlab/ci/config/node/key_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/key_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Key do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Key config should be a string or symbol'
+            .to include 'key config should be a string or symbol'
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/paths_spec.rb b/spec/lib/gitlab/ci/config/node/paths_spec.rb
index 0d95ad8abd..6fd744b397 100644
--- a/spec/lib/gitlab/ci/config/node/paths_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/paths_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Paths do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Paths config should be an array of strings'
+            .to include 'paths config should be an array of strings'
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
index abd43aa1ee..ee7395362a 100644
--- a/spec/lib/gitlab/ci/config/node/script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -34,7 +34,7 @@ describe Gitlab::Ci::Config::Node::Script do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Script config should be an array of strings'
+            .to include 'script config should be an array of strings'
         end
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/services_spec.rb b/spec/lib/gitlab/ci/config/node/services_spec.rb
index e38f6f6923..be0fe46bef 100644
--- a/spec/lib/gitlab/ci/config/node/services_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/services_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Services do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Services config should be an array of strings'
+            .to include 'services config should be an array of strings'
         end
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/stages_spec.rb b/spec/lib/gitlab/ci/config/node/stages_spec.rb
index dbf2eb8993..1a3818d899 100644
--- a/spec/lib/gitlab/ci/config/node/stages_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/stages_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Stages do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Stages config should be an array of strings'
+            .to include 'stages config should be an array of strings'
         end
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/validator_spec.rb b/spec/lib/gitlab/ci/config/node/validator_spec.rb
index 87a1bbf55c..090fd63b84 100644
--- a/spec/lib/gitlab/ci/config/node/validator_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/validator_spec.rb
@@ -7,6 +7,7 @@ describe Gitlab::Ci::Config::Node::Validator do
 
   before do
     allow(node).to receive(:key).and_return('node')
+    allow(node).to receive(:ancestors).and_return([])
   end
 
   describe 'delegated validator' do
@@ -47,7 +48,7 @@ describe Gitlab::Ci::Config::Node::Validator do
         validator_instance.validate
 
         expect(validator_instance.messages)
-          .to include "Node test attribute can't be blank"
+          .to include "node test attribute can't be blank"
       end
     end
   end

From 7759242ae5da221ebe02e9e4b79be3e6aadc9bc6 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 29 Jun 2016 09:49:46 +0200
Subject: [PATCH 22/28] Move global CI cache configuration to new CI classes

---
 lib/ci/gitlab_ci_yaml_processor.rb | 30 ++----------------------------
 lib/gitlab/ci/config.rb            |  2 +-
 2 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 33492775fe..01ef13df57 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -13,7 +13,7 @@ module Ci
     ALLOWED_CACHE_KEYS = [:key, :untracked, :paths]
     ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in]
 
-    attr_reader :path, :cache
+    attr_reader :path, :cache, :stages
 
     def initialize(config, path = nil)
       @ci_config = Gitlab::Ci::Config.new(config)
@@ -44,10 +44,6 @@ module Ci
       end
     end
 
-    def stages
-      @stages
-    end
-
     def global_variables
       @variables
     end
@@ -68,8 +64,8 @@ module Ci
       @services = @ci_config.services
       @variables = @ci_config.variables
       @stages = @ci_config.stages
+      @cache = @ci_config.cache
 
-      @cache = @config[:cache]
       @jobs = {}
 
       @config.except!(*ALLOWED_YAML_KEYS)
@@ -116,8 +112,6 @@ module Ci
     end
 
     def validate!
-      validate_global_cache! if @cache
-
       @jobs.each do |name, job|
         validate_job!(name, job)
       end
@@ -125,26 +119,6 @@ module Ci
       true
     end
 
-    def validate_global_cache!
-      @cache.keys.each do |key|
-        unless ALLOWED_CACHE_KEYS.include?(key)
-          raise ValidationError, "Cache config has unknown parameter: #{key}"
-        end
-      end
-
-      if @cache[:key] && !validate_string(@cache[:key])
-        raise ValidationError, "cache:key parameter should be a string"
-      end
-
-      if @cache[:untracked] && !validate_boolean(@cache[:untracked])
-        raise ValidationError, "cache:untracked parameter should be an boolean"
-      end
-
-      if @cache[:paths] && !validate_array_of_strings(@cache[:paths])
-        raise ValidationError, "cache:paths parameter should be an array of strings"
-      end
-    end
-
     def validate_job!(name, job)
       validate_job_name!(name)
       validate_job_keys!(name, job)
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index 61a2d2069a..e6cc152976 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -8,7 +8,7 @@ module Gitlab
       # Temporary delegations that should be removed after refactoring
       #
       delegate :before_script, :image, :services, :after_script, :variables,
-               :stages, to: :@global
+               :stages, :cache, to: :@global
 
       def initialize(config)
         @config = Loader.new(config).load!

From b85d4969a973862414560bd23b5ff4192dfaa372 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 29 Jun 2016 10:02:02 +0200
Subject: [PATCH 23/28] Return compound value if CI config node is composite

---
 lib/gitlab/ci/config/node/entry.rb | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 985d170519..8fece12232 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -47,7 +47,12 @@ module Gitlab
           end
 
           def value
-            @config
+            if leaf?
+              @config
+            else
+              defined = @nodes.select { |_key, value| value.defined? }
+              Hash[(defined).map { |key, node| [key, node.value] }]
+            end
           end
 
           def defined?

From c8c930f3ff6e1218e7614e46874bb5279bc30fe9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 29 Jun 2016 10:46:30 +0200
Subject: [PATCH 24/28] Add CI config entry validator for allowed keys

---
 lib/gitlab/ci/config/node/cache.rb      | 18 +-----------------
 lib/gitlab/ci/config/node/entry.rb      |  2 +-
 lib/gitlab/ci/config/node/validator.rb  |  5 +++++
 lib/gitlab/ci/config/node/validators.rb | 10 ++++++++++
 4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
index 01a9ef511e..d81b2121a9 100644
--- a/lib/gitlab/ci/config/node/cache.rb
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -18,23 +18,7 @@ module Gitlab
             description: 'Specify which paths should be cached across builds.'
 
           validations do
-            validate :keys
-
-            def unknown_keys
-              return [] unless config.is_a?(Hash)
-              config.keys - allowed_keys
-            end
-
-            def keys
-              if unknown_keys.any?
-                unknown_list = unknown_keys.join(', ')
-                errors.add(:config, "contains unknown keys: #{unknown_list}")
-              end
-            end
-          end
-
-          def allowed_keys
-            self.class.nodes.keys
+            validates :config, allowed_keys: true
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 8fece12232..9e79e170a4 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -51,7 +51,7 @@ module Gitlab
               @config
             else
               defined = @nodes.select { |_key, value| value.defined? }
-              Hash[(defined).map { |key, node| [key, node.value] }]
+              Hash[defined.map { |key, node| [key, node.value] }]
             end
           end
 
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index 94a8af4d08..1ba2e1dc59 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -21,6 +21,11 @@ module Gitlab
             'Validator'
           end
 
+          def unknown_keys
+            return [] unless config.is_a?(Hash)
+            config.keys - @node.class.nodes.keys
+          end
+
           private
 
           def location
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index 4082c161e8..7b2f57990b 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -3,6 +3,16 @@ module Gitlab
     class Config
       module Node
         module Validators
+          class AllowedKeysValidator < ActiveModel::EachValidator
+            def validate_each(record, attribute, value)
+              if record.unknown_keys.any?
+                unknown_list = record.unknown_keys.join(', ')
+                record.errors.add(:config,
+                                  "contains unknown keys: #{unknown_list}")
+              end
+            end
+          end
+
           class ArrayOfStringsValidator < ActiveModel::EachValidator
             include LegacyValidationHelpers
 

From 7ef11ce3de797aa8ad0c39245e78aedd91ffa84c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 29 Jun 2016 12:20:45 +0200
Subject: [PATCH 25/28] Explicitly define entry node class in new CI config

---
 lib/gitlab/ci/config/node/cache.rb  |  6 +++---
 lib/gitlab/ci/config/node/global.rb | 16 ++++++++--------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
index d81b2121a9..cdf8ba2e35 100644
--- a/lib/gitlab/ci/config/node/cache.rb
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -8,13 +8,13 @@ module Gitlab
         class Cache < Entry
           include Configurable
 
-          node :key, Key,
+          node :key, Node::Key,
             description: 'Cache key used to define a cache affinity.'
 
-          node :untracked, Boolean,
+          node :untracked, Node::Boolean,
             description: 'Cache all untracked files.'
 
-          node :paths, Paths,
+          node :paths, Node::Paths,
             description: 'Specify which paths should be cached across builds.'
 
           validations do
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 65919ef1ee..fec2fe564a 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -9,28 +9,28 @@ module Gitlab
         class Global < Entry
           include Configurable
 
-          node :before_script, Script,
+          node :before_script, Node::Script,
             description: 'Script that will be executed before each job.'
 
-          node :image, Image,
+          node :image, Node::Image,
             description: 'Docker image that will be used to execute jobs.'
 
-          node :services, Services,
+          node :services, Node::Services,
             description: 'Docker images that will be linked to the container.'
 
-          node :after_script, Script,
+          node :after_script, Node::Script,
             description: 'Script that will be executed after each job.'
 
-          node :variables, Variables,
+          node :variables, Node::Variables,
             description: 'Environment variables that will be used.'
 
-          node :stages, Stages,
+          node :stages, Node::Stages,
             description: 'Configuration of stages for this pipeline.'
 
-          node :types, Stages,
+          node :types, Node::Stages,
             description: 'Stages for this pipeline (deprecated key).'
 
-          node :cache, Cache,
+          node :cache, Node::Cache,
             description: 'Configure caching between build jobs.'
 
           helpers :before_script, :image, :services, :after_script,

From b4f03e8b1e94863949c567a305c8072b34d7e6a1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 30 Jun 2016 12:59:17 +0200
Subject: [PATCH 26/28] Improve description of CI types node and in specs

---
 lib/gitlab/ci/config/node/global.rb              | 2 +-
 spec/lib/gitlab/ci/config/node/undefined_spec.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index fec2fe564a..f92e1eccbc 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -28,7 +28,7 @@ module Gitlab
             description: 'Configuration of stages for this pipeline.'
 
           node :types, Node::Stages,
-            description: 'Stages for this pipeline (deprecated key).'
+            description: 'Deprecated: stages for this pipeline.'
 
           node :cache, Node::Cache,
             description: 'Configure caching between build jobs.'
diff --git a/spec/lib/gitlab/ci/config/node/undefined_spec.rb b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
index 4318dfe6e5..0c6608d906 100644
--- a/spec/lib/gitlab/ci/config/node/undefined_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
@@ -27,13 +27,13 @@ describe Gitlab::Ci::Config::Node::Undefined do
       allow(entry).to receive(:default).and_return('some value')
     end
 
-    it 'returns default value for entry that is undefined' do
+    it 'returns default value for entry' do
       expect(undefined.value).to eq 'some value'
     end
   end
 
   describe '#undefined?' do
-    it 'is not a concrete entry that is defined' do
+    it 'is not a defined entry' do
       expect(undefined.defined?).to be false
     end
   end

From eb151e77ff389c3e22454145cfd55cfaa4be7948 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 4 Jul 2016 11:29:59 +0200
Subject: [PATCH 27/28] Extract CI configuration entry node factory method

---
 lib/gitlab/ci/config/node/factory.rb | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 85e28f345f..5919a28328 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -21,20 +21,24 @@ module Gitlab
           def create!
             raise InvalidFactory unless @attributes.has_key?(:value)
 
+            fabricate.tap do |entry|
+              entry.key = @attributes[:key]
+              entry.parent = @attributes[:parent]
+              entry.description = @attributes[:description]
+            end
+          end
+
+          private
+
+          def fabricate
             ##
             # We assume that unspecified entry is undefined.
             # See issue #18775.
             #
             if @attributes[:value].nil?
-              node, value = Node::Undefined, @node
+              Node::Undefined.new(@node)
             else
-              node, value = @node, @attributes[:value]
-            end
-
-            node.new(value).tap do |entry|
-              entry.key = @attributes[:key]
-              entry.parent = @attributes[:parent]
-              entry.description = @attributes[:description]
+              @node.new(@attributes[:value])
             end
           end
         end

From bfad4c61f10f689868817cf0b94cddaa1de22240 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 4 Jul 2016 11:37:28 +0200
Subject: [PATCH 28/28] Add minor improvements in readability in CI config

---
 lib/gitlab/ci/config/node/configurable.rb           | 4 +---
 lib/gitlab/ci/config/node/stages.rb                 | 2 +-
 lib/gitlab/ci/config/node/validator.rb              | 1 +
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb        | 4 ++--
 spec/lib/gitlab/ci/config/node/boolean_spec.rb      | 2 +-
 spec/lib/gitlab/ci/config/node/configurable_spec.rb | 1 -
 6 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 4889a21a23..37936fc824 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -26,9 +26,7 @@ module Gitlab
           private
 
           def create_node(key, factory)
-            factory.with(value: @config[key])
-            factory.with(parent: self)
-            factory.with(key: key)
+            factory.with(value: @config[key], key: key, parent: self)
 
             factory.create!
           end
diff --git a/lib/gitlab/ci/config/node/stages.rb b/lib/gitlab/ci/config/node/stages.rb
index 88d88252bc..b1fe45357f 100644
--- a/lib/gitlab/ci/config/node/stages.rb
+++ b/lib/gitlab/ci/config/node/stages.rb
@@ -13,7 +13,7 @@ module Gitlab
           end
 
           def self.default
-            %w(build test deploy)
+            %w[build test deploy]
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index 1ba2e1dc59..758a6cf435 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -23,6 +23,7 @@ module Gitlab
 
           def unknown_keys
             return [] unless config.is_a?(Hash)
+
             config.keys - @node.class.nodes.keys
           end
 
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 262a91fedf..33b9d5f8f2 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -551,8 +551,8 @@ module Ci
               config_processor = GitlabCiYamlProcessor.new(config, path)
 
               ##
-              # When variables config is empty, we asumme this is a correct,
-              # see issue #18775
+              # When variables config is empty, we assume this is a valid
+              # configuration, see issue #18775
               #
               expect(config_processor.job_variables(:rspec))
                 .to be_an_instance_of(Array).and be_empty
diff --git a/spec/lib/gitlab/ci/config/node/boolean_spec.rb b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
index 32639296e6..deafa8bf8a 100644
--- a/spec/lib/gitlab/ci/config/node/boolean_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
@@ -21,7 +21,7 @@ describe Gitlab::Ci::Config::Node::Boolean do
     end
 
     context 'when entry value is not valid' do
-      let(:config) { [ 'incorrect' ] }
+      let(:config) { ['incorrect'] }
 
       describe '#errors' do
         it 'saves errors' do
diff --git a/spec/lib/gitlab/ci/config/node/configurable_spec.rb b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
index 4a1550517f..c468ecf957 100644
--- a/spec/lib/gitlab/ci/config/node/configurable_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
@@ -22,7 +22,6 @@ describe Gitlab::Ci::Config::Node::Configurable do
       validator.validate
     end
 
-
     context 'when node validator is invalid' do
       let(:instance) { node.new('ls') }