From cf53d798736f7c86459c3e6635a83875e6101373 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Jul 2016 13:07:03 +0200
Subject: [PATCH 01/62] Extract jobs config to separate key in config hash

---
 lib/gitlab/ci/config/node/global.rb           | 16 ++++++++++++++++
 spec/lib/gitlab/ci/config/node/global_spec.rb | 12 +++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index f92e1eccbcf..0a4315db047 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -36,6 +36,22 @@ module Gitlab
           helpers :before_script, :image, :services, :after_script,
                   :variables, :stages, :types, :cache
 
+          def initialize(config)
+            return super(config) unless config.is_a?(Hash)
+
+            jobs = config.except(*self.class.nodes.keys)
+            global = config.slice(*self.class.nodes.keys)
+
+            super(global.merge(jobs: jobs))
+          end
+
+          ##
+          # Temporary refactoring stub
+          #
+          def jobs
+            @config[:jobs]
+          end
+
           def stages
             stages_defined? ? stages_value : types_value
           end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index c87c9e97bc8..88194bf9453 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -22,7 +22,9 @@ describe Gitlab::Ci::Config::Node::Global do
           variables: { VAR: 'value' },
           after_script: ['make clean'],
           stages: ['build', 'pages'],
-          cache: { key: 'k', untracked: true, paths: ['public/'] } }
+          cache: { key: 'k', untracked: true, paths: ['public/'] },
+          rspec: { script: 'rspec' },
+          spinach: { script: 'spinach' } }
       end
 
       describe '#process!' do
@@ -120,6 +122,14 @@ describe Gitlab::Ci::Config::Node::Global do
               .to eq(key: 'k', untracked: true, paths: ['public/'])
           end
         end
+
+        describe '#jobs' do
+          it 'returns jobs configuration' do
+            expect(global.jobs)
+              .to eq(rspec: { script: 'rspec' },
+                     spinach: { script: 'spinach' })
+          end
+        end
       end
     end
 

From 9686a4f2b2fd010b5a15409ba65ac4eed8e11c7b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Jul 2016 13:19:22 +0200
Subject: [PATCH 02/62] Remove code creating job hash from legacy CI config

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

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 01ef13df57a..8d0bbe1ae56 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -68,8 +68,9 @@ module Ci
 
       @jobs = {}
 
-      @config.except!(*ALLOWED_YAML_KEYS)
-      @config.each { |name, param| add_job(name, param) }
+      @ci_config.jobs.each do |name, param|
+        add_job(name, param)
+      end
 
       raise ValidationError, "Please define at least one job" if @jobs.none?
     end
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index e6cc1529760..ae82c0db3f1 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, :cache, to: :@global
+               :stages, :cache, :jobs, to: :@global
 
       def initialize(config)
         @config = Loader.new(config).load!

From 5b7f211cbba06f7c43b55b4a38704073564a099f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Jul 2016 13:35:50 +0200
Subject: [PATCH 03/62] Add new CI config entry that holds jobs definition

---
 lib/gitlab/ci/config/node/global.rb           | 14 +++-----
 lib/gitlab/ci/config/node/jobs.rb             | 18 ++++++++++
 spec/lib/gitlab/ci/config/node/global_spec.rb |  4 +--
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   | 36 +++++++++++++++++++
 4 files changed, 61 insertions(+), 11 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/jobs.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/jobs_spec.rb

diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 0a4315db047..cb2db4e9757 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -33,11 +33,14 @@ module Gitlab
           node :cache, Node::Cache,
             description: 'Configure caching between build jobs.'
 
+          node :jobs, Node::Jobs,
+            description: 'Definition of jobs for this pipeline.'
+
           helpers :before_script, :image, :services, :after_script,
-                  :variables, :stages, :types, :cache
+                  :variables, :stages, :types, :cache, :jobs
 
           def initialize(config)
-            return super(config) unless config.is_a?(Hash)
+            return super unless config.is_a?(Hash)
 
             jobs = config.except(*self.class.nodes.keys)
             global = config.slice(*self.class.nodes.keys)
@@ -45,13 +48,6 @@ module Gitlab
             super(global.merge(jobs: jobs))
           end
 
-          ##
-          # Temporary refactoring stub
-          #
-          def jobs
-            @config[:jobs]
-          end
-
           def stages
             stages_defined? ? stages_value : types_value
           end
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
new file mode 100644
index 00000000000..2df2d55aca2
--- /dev/null
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -0,0 +1,18 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a set of jobs.
+        #
+        class Jobs < Entry
+          include Validatable
+
+          validations do
+            validates :config, type: Hash
+          end
+        end
+      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 88194bf9453..8f2f9e171d1 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -35,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 8
+          expect(global.nodes.count).to eq 9
         end
 
         it 'creates node object using valid class' do
@@ -139,7 +139,7 @@ describe Gitlab::Ci::Config::Node::Global do
 
       describe '#nodes' do
         it 'instantizes all nodes' do
-          expect(global.nodes.count).to eq 8
+          expect(global.nodes.count).to eq 9
         end
 
         it 'contains undefined nodes' do
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
new file mode 100644
index 00000000000..3afa3de06c6
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Jobs do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { { rspec: { script: 'rspec' } } }
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq(rspec: { script: 'rspec' })
+        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
+      context 'incorrect config value type' do
+        let(:config) { ['incorrect'] }
+
+        describe '#errors' do
+          it 'saves errors' do
+            expect(entry.errors)
+              .to include 'jobs config should be a hash'
+          end
+        end
+      end
+    end
+  end
+end

From e00ae9a87720bccda634dc85c018f5ec1d03a883 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Jul 2016 14:04:22 +0200
Subject: [PATCH 04/62] Add new CI config entry for single job in pipeline

---
 lib/gitlab/ci/config/node/job.rb           | 14 +++++++++
 spec/lib/gitlab/ci/config/node/job_spec.rb | 36 ++++++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 lib/gitlab/ci/config/node/job.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/job_spec.rb

diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
new file mode 100644
index 00000000000..5be8cb39a87
--- /dev/null
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -0,0 +1,14 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a concrete CI/CD job.
+        #
+        class Job < Entry
+          include Configurable
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
new file mode 100644
index 00000000000..7dd25a23c83
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Job do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { { script: 'rspec' } }
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq(script: 'rspec')
+        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
+      context 'incorrect config value type' do
+        let(:config) { ['incorrect'] }
+
+        describe '#errors' do
+          it 'saves errors' do
+            expect(entry.errors)
+              .to include 'job config should be a hash'
+          end
+        end
+      end
+    end
+  end
+end

From 6ae80732bb3b503e2d15acb2cab527c17e22e34b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Jul 2016 14:17:09 +0200
Subject: [PATCH 05/62] Add ability to define nodes in new CI config entry

---
 lib/gitlab/ci/config/node/entry.rb            | 18 +++++++++++-------
 spec/lib/gitlab/ci/config/node/global_spec.rb | 16 ++++++++--------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 9e79e170a4f..7148f4c2a79 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -26,12 +26,16 @@ module Gitlab
             process_nodes!
           end
 
-          def nodes
-            @nodes.values
+          def leaf?
+            nodes.none?
           end
 
-          def leaf?
-            self.class.nodes.none?
+          def nodes
+            self.class.nodes
+          end
+
+          def descendants
+            @nodes.values
           end
 
           def ancestors
@@ -43,7 +47,7 @@ module Gitlab
           end
 
           def errors
-            @validator.messages + nodes.flat_map(&:errors)
+            @validator.messages + @nodes.values.flat_map(&:errors)
           end
 
           def value
@@ -73,13 +77,13 @@ module Gitlab
           private
 
           def compose!
-            self.class.nodes.each do |key, essence|
+            nodes.each do |key, essence|
               @nodes[key] = create_node(key, essence)
             end
           end
 
           def process_nodes!
-            nodes.each(&:process!)
+            @nodes.each_value(&:process!)
           end
 
           def create_node(key, essence)
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 8f2f9e171d1..254cb28190c 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -31,24 +31,24 @@ describe Gitlab::Ci::Config::Node::Global do
         before { global.process! }
 
         it 'creates nodes hash' do
-          expect(global.nodes).to be_an Array
+          expect(global.descendants).to be_an Array
         end
 
         it 'creates node object for each entry' do
-          expect(global.nodes.count).to eq 9
+          expect(global.descendants.count).to eq 9
         end
 
         it 'creates node object using valid class' do
-          expect(global.nodes.first)
+          expect(global.descendants.first)
             .to be_an_instance_of Gitlab::Ci::Config::Node::Script
-          expect(global.nodes.second)
+          expect(global.descendants.second)
             .to be_an_instance_of Gitlab::Ci::Config::Node::Image
         end
 
         it 'sets correct description for nodes' do
-          expect(global.nodes.first.description)
+          expect(global.descendants.first.description)
             .to eq 'Script that will be executed before each job.'
-          expect(global.nodes.second.description)
+          expect(global.descendants.second.description)
             .to eq 'Docker image that will be used to execute jobs.'
         end
       end
@@ -139,11 +139,11 @@ describe Gitlab::Ci::Config::Node::Global do
 
       describe '#nodes' do
         it 'instantizes all nodes' do
-          expect(global.nodes.count).to eq 9
+          expect(global.descendants.count).to eq 9
         end
 
         it 'contains undefined nodes' do
-          expect(global.nodes.first)
+          expect(global.descendants.first)
             .to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
         end
       end

From dbab56a9519039bb6a83974c31b90b1283b8479c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Jul 2016 14:48:17 +0200
Subject: [PATCH 06/62] Create composite job entries in new CI config

---
 lib/gitlab/ci/config/node/jobs.rb            | 14 ++++++++++++++
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb |  4 ++--
 spec/lib/gitlab/ci/config/node/jobs_spec.rb  | 17 +++++++++++++++++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 2df2d55aca2..915b46652f2 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -11,6 +11,20 @@ module Gitlab
           validations do
             validates :config, type: Hash
           end
+
+          def nodes
+            @config
+          end
+
+          private
+
+          def create_node(key, essence)
+            Node::Job.new(essence).tap do |job|
+              job.key = key
+              job.parent = self
+              job.description = "#{key} job definition."
+            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 bad439bc489..49a786191b8 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1043,11 +1043,11 @@ EOT
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: services should be an array of strings")
       end
 
-      it "returns errors if there are unknown parameters" do
+      it "returns error if job configuration is invalid" do
         config = YAML.dump({ extra: "bundle update" })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Unknown parameter: extra")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:extra config should be a hash")
       end
 
       it "returns errors if there are unknown parameters that are hashes, but doesn't have a script" do
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 3afa3de06c6..7f80e11cea3 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -4,6 +4,8 @@ describe Gitlab::Ci::Config::Node::Jobs do
   let(:entry) { described_class.new(config) }
 
   describe 'validations' do
+    before { entry.process! }
+
     context 'when entry config value is correct' do
       let(:config) { { rspec: { script: 'rspec' } } }
 
@@ -33,4 +35,19 @@ describe Gitlab::Ci::Config::Node::Jobs do
       end
     end
   end
+
+  describe '#descendants' do
+    before { entry.process! }
+
+    let(:config) do
+      { rspec: { script: 'rspec' },
+        spinach: { script: 'spinach' } }
+    end
+
+    it 'creates two descendant nodes' do
+      expect(entry.descendants.count).to eq 2
+      expect(entry.descendants)
+        .to all(be_an_instance_of(Gitlab::Ci::Config::Node::Job))
+    end
+  end
 end

From b1b0c18b8c66857964eaaa5704a0744aacb707dd Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Jul 2016 12:58:43 +0200
Subject: [PATCH 07/62] Add hidden job in new CI config that is irrelevant

---
 lib/gitlab/ci/config/node/entry.rb            | 11 ++++-
 lib/gitlab/ci/config/node/hidden_job.rb       | 22 +++++++++
 lib/gitlab/ci/config/node/jobs.rb             | 10 +++-
 lib/gitlab/ci/config/node/validator.rb        |  2 +-
 .../gitlab/ci/config/node/hidden_job_spec.rb  | 48 +++++++++++++++++++
 spec/lib/gitlab/ci/config/node/job_spec.rb    |  6 +++
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   | 23 ++++++---
 7 files changed, 112 insertions(+), 10 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/hidden_job.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/hidden_job_spec.rb

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 7148f4c2a79..fa6569e8bb2 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -54,8 +54,11 @@ module Gitlab
             if leaf?
               @config
             else
-              defined = @nodes.select { |_key, value| value.defined? }
-              Hash[defined.map { |key, node| [key, node.value] }]
+              meaningful = @nodes.select do |_key, value|
+                value.defined? && value.relevant?
+              end
+
+              Hash[meaningful.map { |key, node| [key, node.value] }]
             end
           end
 
@@ -63,6 +66,10 @@ module Gitlab
             true
           end
 
+          def relevant?
+            true
+          end
+
           def self.default
           end
 
diff --git a/lib/gitlab/ci/config/node/hidden_job.rb b/lib/gitlab/ci/config/node/hidden_job.rb
new file mode 100644
index 00000000000..6a559ee8c04
--- /dev/null
+++ b/lib/gitlab/ci/config/node/hidden_job.rb
@@ -0,0 +1,22 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a hidden CI/CD job.
+        #
+        class HiddenJob < Entry
+          include Validatable
+
+          validations do
+            validates :config, type: Hash
+          end
+
+          def relevant?
+            false
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 915b46652f2..a76b7a260c4 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -19,12 +19,20 @@ module Gitlab
           private
 
           def create_node(key, essence)
-            Node::Job.new(essence).tap do |job|
+            fabricate_job(key, essence).tap do |job|
               job.key = key
               job.parent = self
               job.description = "#{key} job definition."
             end
           end
+
+          def fabricate_job(key, essence)
+            if key.to_s.start_with?('.')
+              Node::HiddenJob.new(essence)
+            else
+              Node::Job.new(essence)
+            end
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index 758a6cf4356..dcfeb194374 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -31,7 +31,7 @@ module Gitlab
 
           def location
             predecessors = ancestors.map(&:key).compact
-            current = key || @node.class.name.demodulize.underscore
+            current = key || @node.class.name.demodulize.underscore.humanize
             predecessors.append(current).join(':')
           end
         end
diff --git a/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb b/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb
new file mode 100644
index 00000000000..ab865c3522e
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb
@@ -0,0 +1,48 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::HiddenJob do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { { image: 'ruby:2.2' } }
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq(image: 'ruby:2.2')
+        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
+      context 'incorrect config value type' do
+        let(:config) { ['incorrect'] }
+
+        describe '#errors' do
+          it 'saves errors' do
+            expect(entry.errors)
+              .to include 'hidden job config should be a hash'
+          end
+        end
+      end
+    end
+  end
+
+  describe '#leaf?' do
+    it 'is a leaf' do
+      expect(entry).to be_leaf
+    end
+  end
+
+  describe '#relevant?' do
+    it 'is not a relevant entry' do
+      expect(entry).not_to be_relevant
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 7dd25a23c83..15c7f9bc394 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -33,4 +33,10 @@ describe Gitlab::Ci::Config::Node::Job do
       end
     end
   end
+
+  describe '#relevant?' do
+    it 'is a relevant entry' do
+      expect(entry).to be_relevant
+    end
+  end
 end
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 7f80e11cea3..b2d2a92d9e8 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -36,18 +36,29 @@ describe Gitlab::Ci::Config::Node::Jobs do
     end
   end
 
-  describe '#descendants' do
+  context 'when valid job entries processed' do
     before { entry.process! }
 
     let(:config) do
       { rspec: { script: 'rspec' },
-        spinach: { script: 'spinach' } }
+        spinach: { script: 'spinach' },
+        '.hidden'.to_sym => {} }
     end
 
-    it 'creates two descendant nodes' do
-      expect(entry.descendants.count).to eq 2
-      expect(entry.descendants)
-        .to all(be_an_instance_of(Gitlab::Ci::Config::Node::Job))
+    describe '#descendants' do
+      it 'creates valid descendant nodes' do
+        expect(entry.descendants.count).to eq 3
+        expect(entry.descendants.first(2))
+          .to all(be_an_instance_of(Gitlab::Ci::Config::Node::Job))
+        expect(entry.descendants.last)
+          .to be_an_instance_of(Gitlab::Ci::Config::Node::HiddenJob)
+      end
+    end
+
+    describe '#value' do
+      it 'returns value of visible jobs only' do
+        expect(entry.value.keys).to eq [:rspec, :spinach]
+      end
     end
   end
 end

From 580c4e1841cf4756e86c1ec9eddef56e2bfc9c97 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Jul 2016 13:08:40 +0200
Subject: [PATCH 08/62] Move decision about relevant jobs to new CI config

---
 lib/ci/gitlab_ci_yaml_processor.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 8d0bbe1ae56..d226ebc3229 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -76,8 +76,6 @@ module Ci
     end
 
     def add_job(name, job)
-      return if name.to_s.start_with?('.')
-
       raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
 
       stage = job[:stage] || job[:type] || DEFAULT_STAGE

From 4491bf28e10da258701b316f397c5802f5f9974e Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Jul 2016 14:08:19 +0200
Subject: [PATCH 09/62] Move CI job config validations to new classes

---
 lib/ci/gitlab_ci_yaml_processor.rb            |  2 -
 lib/gitlab/ci/config/node/entry.rb            |  1 +
 lib/gitlab/ci/config/node/jobs.rb             | 15 +++++++
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  |  9 +++-
 spec/lib/gitlab/ci/config/node/global_spec.rb |  7 +++-
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   | 42 +++++++++++++------
 6 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index d226ebc3229..ab77d4df841 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -71,8 +71,6 @@ module Ci
       @ci_config.jobs.each do |name, param|
         add_job(name, param)
       end
-
-      raise ValidationError, "Please define at least one job" if @jobs.none?
     end
 
     def add_job(name, job)
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index fa6569e8bb2..033f9f0e3d1 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -24,6 +24,7 @@ module Gitlab
 
             compose!
             process_nodes!
+            @validator.validate(:processed)
           end
 
           def leaf?
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index a76b7a260c4..e8e78e4088d 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -10,12 +10,27 @@ module Gitlab
 
           validations do
             validates :config, type: Hash
+            validate :jobs_presence, on: :processed
+
+            def jobs_presence
+              unless relevant?
+                errors.add(:config, 'should contain at least one visible job')
+              end
+            end
           end
 
           def nodes
             @config
           end
 
+          def relevant?
+            @nodes.values.any?(&:relevant?)
+          end
+
+          def leaf?
+            false
+          end
+
           private
 
           def create_node(key, essence)
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 49a786191b8..ac058ba1595 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1061,7 +1061,14 @@ EOT
         config = YAML.dump({ before_script: ["bundle update"] })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Please define at least one job")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs config should contain at least one visible job")
+      end
+
+      it "returns errors if there are no visible jobs defined" do
+        config = YAML.dump({ before_script: ["bundle update"], '.hidden'.to_sym => {} })
+        expect do
+          GitlabCiYamlProcessor.new(config, path)
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs config should contain at least one visible job")
       end
 
       it "returns errors if job allow_failure parameter is not an boolean" do
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 254cb28190c..a98de73c06c 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -108,7 +108,10 @@ describe Gitlab::Ci::Config::Node::Global do
           end
 
           context 'when deprecated types key defined' do
-            let(:hash) { { types: ['test', 'deploy'] } }
+            let(:hash) do
+              { types: ['test', 'deploy'],
+                rspec: { script: 'rspec' } }
+            end
 
             it 'returns array of types as stages' do
               expect(global.stages).to eq %w[test deploy]
@@ -174,7 +177,7 @@ describe Gitlab::Ci::Config::Node::Global do
     # details.
     #
     context 'when entires specified but not defined' do
-      let(:hash) { { variables: nil } }
+      let(:hash) { { variables: nil, rspec: { script: 'rspec' } } }
       before { global.process! }
 
       describe '#variables' do
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index b2d2a92d9e8..7ec28b642b4 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -4,17 +4,9 @@ describe Gitlab::Ci::Config::Node::Jobs do
   let(:entry) { described_class.new(config) }
 
   describe 'validations' do
-    before { entry.process! }
-
     context 'when entry config value is correct' do
       let(:config) { { rspec: { script: 'rspec' } } }
 
-      describe '#value' do
-        it 'returns key value' do
-          expect(entry.value).to eq(rspec: { script: 'rspec' })
-        end
-      end
-
       describe '#valid?' do
         it 'is valid' do
           expect(entry).to be_valid
@@ -23,15 +15,34 @@ describe Gitlab::Ci::Config::Node::Jobs do
     end
 
     context 'when entry value is not correct' do
-      context 'incorrect config value type' do
-        let(:config) { ['incorrect'] }
+      describe '#errors' do
+        context 'incorrect config value type' do
+          let(:config) { ['incorrect'] }
 
-        describe '#errors' do
-          it 'saves errors' do
+          it 'returns error about incorrect type' do
             expect(entry.errors)
               .to include 'jobs config should be a hash'
           end
         end
+
+        context 'when no visible jobs present' do
+          let(:config) { { '.hidden'.to_sym => {} } }
+
+          context 'when not processed' do
+            it 'is valid' do
+              expect(entry.errors).to be_empty
+            end
+          end
+
+          context 'when processed' do
+            before { entry.process! }
+
+            it 'returns error about no visible jobs defined' do
+              expect(entry.errors)
+                .to include 'jobs config should contain at least one visible job'
+            end
+          end
+        end
       end
     end
   end
@@ -45,6 +56,13 @@ describe Gitlab::Ci::Config::Node::Jobs do
         '.hidden'.to_sym => {} }
     end
 
+    describe '#value' do
+      it 'returns key value' do
+        expect(entry.value).to eq(rspec: { script: 'rspec' },
+                                  spinach: { script: 'spinach' })
+      end
+    end
+
     describe '#descendants' do
       it 'creates valid descendant nodes' do
         expect(entry.descendants.count).to eq 3

From 6f02da2c4e069ef4ab550dc43176dc0563c95017 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Jul 2016 14:24:31 +0200
Subject: [PATCH 10/62] Simplify legacy CI config processor a little

---
 lib/ci/gitlab_ci_yaml_processor.rb | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index ab77d4df841..f0710690985 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -26,7 +26,6 @@ module Ci
       end
 
       initial_parsing
-      validate!
     rescue Gitlab::Ci::Config::Loader::FormatError => e
       raise ValidationError, e.message
     end
@@ -71,6 +70,10 @@ module Ci
       @ci_config.jobs.each do |name, param|
         add_job(name, param)
       end
+
+      @jobs.each do |name, job|
+        validate_job!(name, job)
+      end
     end
 
     def add_job(name, job)
@@ -108,14 +111,6 @@ module Ci
       }
     end
 
-    def validate!
-      @jobs.each do |name, job|
-        validate_job!(name, job)
-      end
-
-      true
-    end
-
     def validate_job!(name, job)
       validate_job_name!(name)
       validate_job_keys!(name, job)

From b0ae0d730f4eda34cd712477a828dddd888ba474 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 7 Jul 2016 10:23:47 +0200
Subject: [PATCH 11/62] Use only node factory to create CI config entries

---
 lib/gitlab/ci/config/node/factory.rb          | 28 +++++++++++--------
 lib/gitlab/ci/config/node/global.rb           |  4 +--
 lib/gitlab/ci/config/node/jobs.rb             | 20 +++++--------
 .../lib/gitlab/ci/config/node/factory_spec.rb | 12 ++++++++
 4 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 5919a283283..b1457b81a45 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -21,26 +21,30 @@ 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::Undefined.new(@node)
+              fabricate(Node::Undefined, @node)
             else
-              @node.new(@attributes[:value])
+              fabricate(@node, @attributes[:value])
             end
           end
+
+          def self.fabricate(node, value, **attributes)
+            node.new(value).tap do |entry|
+              entry.key = attributes[:key]
+              entry.parent = attributes[:parent]
+              entry.description = attributes[:description]
+            end
+          end
+
+          private
+
+          def fabricate(node, value)
+            self.class.fabricate(node, value, @attributes)
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index cb2db4e9757..64d8e39093d 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -42,8 +42,8 @@ module Gitlab
           def initialize(config)
             return super unless config.is_a?(Hash)
 
-            jobs = config.except(*self.class.nodes.keys)
-            global = config.slice(*self.class.nodes.keys)
+            jobs = config.except(*nodes.keys)
+            global = config.slice(*nodes.keys)
 
             super(global.merge(jobs: jobs))
           end
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index e8e78e4088d..1cd2dc8f5b3 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -33,20 +33,14 @@ module Gitlab
 
           private
 
-          def create_node(key, essence)
-            fabricate_job(key, essence).tap do |job|
-              job.key = key
-              job.parent = self
-              job.description = "#{key} job definition."
-            end
-          end
+          def create_node(key, value)
+            node = key.to_s.start_with?('.') ? Node::HiddenJob : Node::Job
 
-          def fabricate_job(key, essence)
-            if key.to_s.start_with?('.')
-              Node::HiddenJob.new(essence)
-            else
-              Node::Job.new(essence)
-            end
+            attributes = { key: key,
+                           parent: self,
+                           description: "#{key} job definition." }
+
+            Node::Factory.fabricate(node, value, attributes)
           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 91ddef7bfbf..5a26bb6df02 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -5,6 +5,18 @@ describe Gitlab::Ci::Config::Node::Factory do
     let(:factory) { described_class.new(entry_class) }
     let(:entry_class) { Gitlab::Ci::Config::Node::Script }
 
+    describe '.fabricate' do
+      it 'fabricates entry with attributes set' do
+        fabricated = described_class
+          .fabricate(entry_class, ['ls'],
+                     parent: factory, key: :test)
+
+        expect(fabricated.parent).to be factory
+        expect(fabricated.key).to eq :test
+        expect(fabricated.value).to eq ['ls']
+      end
+    end
+
     context 'when setting up a value' do
       it 'creates entry with valid value' do
         entry = factory

From fea7762485c75003381891bc892bc6049f8a2105 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 7 Jul 2016 12:41:31 +0200
Subject: [PATCH 12/62] Delegate methods to default CI entry if undefined

---
 lib/gitlab/ci/config/node/entry.rb            |  5 ++
 lib/gitlab/ci/config/node/undefined.rb        | 29 +++++++-
 .../lib/gitlab/ci/config/node/factory_spec.rb |  6 +-
 .../gitlab/ci/config/node/undefined_spec.rb   | 67 +++++++++++++------
 4 files changed, 83 insertions(+), 24 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 033f9f0e3d1..8fda37a8922 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -14,6 +14,7 @@ module Gitlab
           def initialize(config)
             @config = config
             @nodes = {}
+
             @validator = self.class.validator.new(self)
             @validator.validate
           end
@@ -71,6 +72,10 @@ module Gitlab
             true
           end
 
+          def attributes
+            { key: @key, parent: @parent, description: @description }
+          end
+
           def self.default
           end
 
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index 699605e1e3a..f152c433c42 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -5,8 +5,9 @@ module Gitlab
         ##
         # This class represents an undefined entry node.
         #
-        # It takes original entry class as configuration and returns default
-        # value of original entry as self value.
+        # It takes original entry class as configuration and creates an object
+        # if original entry has a default value. If there is default value
+        # some methods are delegated to it.
         #
         #
         class Undefined < Entry
@@ -16,13 +17,35 @@ module Gitlab
             validates :config, type: Class
           end
 
+          def initialize(node)
+            super
+
+            unless node.default.nil?
+              @default = fabricate_default(node)
+            end
+          end
+
           def value
-            @config.default
+            @default.value if @default
+          end
+
+          def valid?
+            @default ? @default.valid? : true
+          end
+
+          def errors
+            @default ? @default.errors : []
           end
 
           def defined?
             false
           end
+
+          private
+
+          def fabricate_default(node)
+            Node::Factory.fabricate(node, node.default, attributes)
+          end
         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 5a26bb6df02..5b856d44989 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -9,11 +9,13 @@ describe Gitlab::Ci::Config::Node::Factory do
       it 'fabricates entry with attributes set' do
         fabricated = described_class
           .fabricate(entry_class, ['ls'],
-                     parent: factory, key: :test)
+                     parent: true, key: :test)
 
-        expect(fabricated.parent).to be factory
+        expect(fabricated.parent).to be true
         expect(fabricated.key).to eq :test
         expect(fabricated.value).to eq ['ls']
+        expect(fabricated.attributes)
+          .to eq(parent: true, key: :test, description: nil)
       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 0c6608d906d..417b4a0ad6f 100644
--- a/spec/lib/gitlab/ci/config/node/undefined_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
@@ -2,33 +2,62 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Undefined do
   let(:undefined) { described_class.new(entry) }
-  let(:entry) { Class.new }
+  let(:entry) { spy('Entry') }
 
-  describe '#leaf?' do
-    it 'is leaf node' do
-      expect(undefined).to be_leaf
+  context 'when entry does not have a default value' do
+    before { allow(entry).to receive(:default).and_return(nil) }
+
+    describe '#leaf?' do
+      it 'is leaf node' do
+        expect(undefined).to be_leaf
+      end
+    end
+
+    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 nil' do
+        expect(undefined.value).to eq nil
+      end
     end
   end
 
-  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
+  context 'when entry has a default value' do
     before do
       allow(entry).to receive(:default).and_return('some value')
+      allow(entry).to receive(:value).and_return('some value')
     end
 
-    it 'returns default value for entry' do
-      expect(undefined.value).to eq 'some value'
+    describe '#value' do
+      it 'returns default value for entry' do
+        expect(undefined.value).to eq 'some value'
+      end
+    end
+
+    describe '#errors' do
+      it 'delegates errors to default entry' do
+        expect(entry).to receive(:errors)
+
+        undefined.errors
+      end
+    end
+
+    describe '#valid?' do
+      it 'delegates valid? to default entry' do
+        expect(entry).to receive(:valid?)
+
+        undefined.valid?
+      end
     end
   end
 

From de8b93bbff65844438d7dfbde178746c3585bd92 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 7 Jul 2016 12:56:00 +0200
Subject: [PATCH 13/62] Improve validation of CI config entry if composite

---
 lib/gitlab/ci/config/node/entry.rb | 12 ++++--------
 lib/gitlab/ci/config/node/jobs.rb  |  4 ----
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 8fda37a8922..97e17b89c40 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -20,12 +20,8 @@ module Gitlab
           end
 
           def process!
-            return if leaf?
-            return unless valid?
-
-            compose!
-            process_nodes!
-            @validator.validate(:processed)
+            compose! unless leaf?
+            @validator.validate(:processed) if valid?
           end
 
           def leaf?
@@ -90,12 +86,12 @@ module Gitlab
           private
 
           def compose!
+            return unless valid?
+
             nodes.each do |key, essence|
               @nodes[key] = create_node(key, essence)
             end
-          end
 
-          def process_nodes!
             @nodes.each_value(&:process!)
           end
 
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 1cd2dc8f5b3..71893ba1d88 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -27,10 +27,6 @@ module Gitlab
             @nodes.values.any?(&:relevant?)
           end
 
-          def leaf?
-            false
-          end
-
           private
 
           def create_node(key, value)

From ecdcf04e88f6313ae8773e7b9886bc983adab83d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 7 Jul 2016 13:09:36 +0200
Subject: [PATCH 14/62] Add undefined CI node strategies to handle defaults

---
 lib/gitlab/ci/config/node/undefined.rb | 55 +++++++++++++++++---------
 1 file changed, 37 insertions(+), 18 deletions(-)

diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index f152c433c42..7b18e364675 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -13,28 +13,15 @@ module Gitlab
         class Undefined < Entry
           include Validatable
 
+          delegate :valid?, :errors, :value, to: :@strategy
+
           validations do
             validates :config, type: Class
           end
 
           def initialize(node)
             super
-
-            unless node.default.nil?
-              @default = fabricate_default(node)
-            end
-          end
-
-          def value
-            @default.value if @default
-          end
-
-          def valid?
-            @default ? @default.valid? : true
-          end
-
-          def errors
-            @default ? @default.errors : []
+            @strategy = create_strategy(node, node.default)
           end
 
           def defined?
@@ -43,8 +30,40 @@ module Gitlab
 
           private
 
-          def fabricate_default(node)
-            Node::Factory.fabricate(node, node.default, attributes)
+          def create_strategy(node, default)
+            if default.nil?
+              Undefined::NullStrategy.new
+            else
+              entry = Node::Factory
+                .fabricate(node, default, attributes)
+
+              Undefined::DefaultStrategy.new(entry)
+            end
+          end
+
+          class DefaultStrategy
+            delegate :valid?, :errors, :value, to: :@default
+
+            def initialize(entry)
+              @default = entry
+            end
+          end
+
+          class NullStrategy
+            def initialize(*)
+            end
+
+            def value
+              nil
+            end
+
+            def valid?
+              true
+            end
+
+            def errors
+              []
+            end
           end
         end
       end

From 9410aecca87a1c03f7e7fb1e5e1073460c71b6e5 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 7 Jul 2016 13:39:13 +0200
Subject: [PATCH 15/62] Add scaffold of CI config for the job stage entry

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

diff --git a/lib/gitlab/ci/config/node/stage.rb b/lib/gitlab/ci/config/node/stage.rb
new file mode 100644
index 00000000000..53ceafaa3f4
--- /dev/null
+++ b/lib/gitlab/ci/config/node/stage.rb
@@ -0,0 +1,22 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a stage for a job.
+        #
+        class Stage < Entry
+          include Validatable
+
+          validations do
+            validates :config, key: true
+          end
+
+          def self.default
+            :test
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/stage_spec.rb b/spec/lib/gitlab/ci/config/node/stage_spec.rb
new file mode 100644
index 00000000000..653d613ba6e
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/stage_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Stage do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { :stage1 }
+
+      describe '#value' do
+        it 'returns a stage key' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+
+      context 'when entry config is incorrect' do
+        let(:config) { { test: true } }
+
+        describe '#errors' do
+          it 'reports errors' do
+            expect(entry.errors)
+              .to include 'stage config should be a string or symbol'
+          end
+        end
+
+        describe '#valid?' do
+          it 'is not valid' do
+            expect(entry).not_to be_valid
+          end
+        end
+      end
+    end
+  end
+
+  describe '.default' do
+    it 'returns default stage' do
+      expect(described_class.default).to eq :test
+    end
+  end
+end

From a7ac2f74944430d75b090f78cd9c1cf1d24379f6 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 7 Jul 2016 15:00:35 +0200
Subject: [PATCH 16/62] Simplify CI config entry node factory, use attribs

---
 lib/gitlab/ci/config/node/configurable.rb     |  4 ++-
 lib/gitlab/ci/config/node/entry.rb            | 14 +++++-----
 lib/gitlab/ci/config/node/factory.rb          | 27 +++++++------------
 lib/gitlab/ci/config/node/global.rb           |  4 +++
 lib/gitlab/ci/config/node/jobs.rb             | 10 +++----
 lib/gitlab/ci/config/node/undefined.rb        |  6 ++---
 .../lib/gitlab/ci/config/node/factory_spec.rb | 26 +++++-------------
 7 files changed, 37 insertions(+), 54 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 37936fc8242..88403a9de1e 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -26,7 +26,9 @@ module Gitlab
           private
 
           def create_node(key, factory)
-            factory.with(value: @config[key], key: key, parent: self)
+            factory
+              .value(config[key])
+              .with(key: key, parent: self, global: global)
 
             factory.create!
           end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 97e17b89c40..e8b0160edc1 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -8,13 +8,17 @@ module Gitlab
         class Entry
           class InvalidError < StandardError; end
 
-          attr_reader :config
-          attr_accessor :key, :parent, :description
+          attr_reader :config, :attributes
+          attr_accessor :key, :parent, :global, :description
 
-          def initialize(config)
+          def initialize(config, **attributes)
             @config = config
             @nodes = {}
 
+            (@attributes = attributes).each do |attribute, value|
+              public_send("#{attribute}=", value)
+            end
+
             @validator = self.class.validator.new(self)
             @validator.validate
           end
@@ -68,10 +72,6 @@ module Gitlab
             true
           end
 
-          def attributes
-            { key: @key, parent: @parent, description: @description }
-          end
-
           def self.default
           end
 
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index b1457b81a45..3f2cdf436e3 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -13,38 +13,29 @@ module Gitlab
             @attributes = {}
           end
 
+          def value(value)
+            @value = value
+            self
+          end
+
           def with(attributes)
             @attributes.merge!(attributes)
             self
           end
 
           def create!
-            raise InvalidFactory unless @attributes.has_key?(:value)
+            raise InvalidFactory unless defined?(@value)
 
             ##
             # We assume that unspecified entry is undefined.
             # See issue #18775.
             #
-            if @attributes[:value].nil?
-              fabricate(Node::Undefined, @node)
+            if @value.nil?
+              Node::Undefined.new(@node, @attributes)
             else
-              fabricate(@node, @attributes[:value])
+              @node.new(@value, @attributes)
             end
           end
-
-          def self.fabricate(node, value, **attributes)
-            node.new(value).tap do |entry|
-              entry.key = attributes[:key]
-              entry.parent = attributes[:parent]
-              entry.description = attributes[:description]
-            end
-          end
-
-          private
-
-          def fabricate(node, value)
-            self.class.fabricate(node, value, @attributes)
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 64d8e39093d..dffa3326630 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -51,6 +51,10 @@ module Gitlab
           def stages
             stages_defined? ? stages_value : types_value
           end
+
+          def global
+            self
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 71893ba1d88..d7d61ade36d 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -30,13 +30,13 @@ module Gitlab
           private
 
           def create_node(key, value)
-            node = key.to_s.start_with?('.') ? Node::HiddenJob : Node::Job
+            job_node = key.to_s.start_with?('.') ? Node::HiddenJob : Node::Job
 
-            attributes = { key: key,
-                           parent: self,
-                           description: "#{key} job definition." }
+            job_attributes = { key: key,
+                               parent: self,
+                               description: "#{key} job definition." }
 
-            Node::Factory.fabricate(node, value, attributes)
+            job_node.new(value, attributes.merge(job_attributes))
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index 7b18e364675..fedb9d020be 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -19,7 +19,7 @@ module Gitlab
             validates :config, type: Class
           end
 
-          def initialize(node)
+          def initialize(node, **attributes)
             super
             @strategy = create_strategy(node, node.default)
           end
@@ -34,9 +34,7 @@ module Gitlab
             if default.nil?
               Undefined::NullStrategy.new
             else
-              entry = Node::Factory
-                .fabricate(node, default, attributes)
-
+              entry = node.new(default, attributes)
               Undefined::DefaultStrategy.new(entry)
             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 5b856d44989..c912b1b2044 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -5,24 +5,10 @@ describe Gitlab::Ci::Config::Node::Factory do
     let(:factory) { described_class.new(entry_class) }
     let(:entry_class) { Gitlab::Ci::Config::Node::Script }
 
-    describe '.fabricate' do
-      it 'fabricates entry with attributes set' do
-        fabricated = described_class
-          .fabricate(entry_class, ['ls'],
-                     parent: true, key: :test)
-
-        expect(fabricated.parent).to be true
-        expect(fabricated.key).to eq :test
-        expect(fabricated.value).to eq ['ls']
-        expect(fabricated.attributes)
-          .to eq(parent: true, key: :test, description: nil)
-      end
-    end
-
     context 'when setting up a value' do
       it 'creates entry with valid value' do
         entry = factory
-          .with(value: ['ls', 'pwd'])
+          .value(['ls', 'pwd'])
           .create!
 
         expect(entry.value).to eq ['ls', 'pwd']
@@ -31,7 +17,7 @@ describe Gitlab::Ci::Config::Node::Factory do
       context 'when setting description' do
         it 'creates entry with description' do
           entry = factory
-            .with(value: ['ls', 'pwd'])
+            .value(['ls', 'pwd'])
             .with(description: 'test description')
             .create!
 
@@ -43,7 +29,8 @@ describe Gitlab::Ci::Config::Node::Factory do
       context 'when setting key' do
         it 'creates entry with custom key' do
           entry = factory
-            .with(value: ['ls', 'pwd'], key: 'test key')
+            .value(['ls', 'pwd'])
+            .with(key: 'test key')
             .create!
 
           expect(entry.key).to eq 'test key'
@@ -55,7 +42,8 @@ describe Gitlab::Ci::Config::Node::Factory do
 
         it 'creates entry with valid parent' do
           entry = factory
-            .with(value: 'ls', parent: parent)
+            .value('ls')
+            .with(parent: parent)
             .create!
 
           expect(entry.parent).to eq parent
@@ -74,7 +62,7 @@ describe Gitlab::Ci::Config::Node::Factory do
     context 'when creating entry with nil value' do
       it 'creates an undefined entry' do
         entry = factory
-          .with(value: nil)
+          .value(nil)
           .create!
 
         expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined

From f067202e9b7a4304ffb8d68408880f7eb7fc8b34 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 7 Jul 2016 15:06:05 +0200
Subject: [PATCH 17/62] Improve creating CI config entries for jobs config

---
 lib/gitlab/ci/config/node/jobs.rb | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index d7d61ade36d..cba1fce4a4c 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -29,14 +29,18 @@ module Gitlab
 
           private
 
-          def create_node(key, value)
-            job_node = key.to_s.start_with?('.') ? Node::HiddenJob : Node::Job
+          def create_node(name, config)
+            job_node(name).new(config, job_attributes(name))
+          end
 
-            job_attributes = { key: key,
-                               parent: self,
-                               description: "#{key} job definition." }
+          def job_node(name)
+            name.to_s.start_with?('.') ? Node::HiddenJob : Node::Job
+          end
 
-            job_node.new(value, attributes.merge(job_attributes))
+          def job_attributes(name)
+            @attributes.merge(key: name,
+                              parent: self,
+                              description: "#{name} job definition.")
           end
         end
       end

From 3da57c800bf0c4fe3c45dbea3cff4179f6aa124f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 7 Jul 2016 15:15:44 +0200
Subject: [PATCH 18/62] Require reference to CI config for some entries

---
 lib/gitlab/ci/config/node/stage.rb           |  7 +++++
 spec/lib/gitlab/ci/config/node/stage_spec.rb | 31 ++++++++++++++------
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/lib/gitlab/ci/config/node/stage.rb b/lib/gitlab/ci/config/node/stage.rb
index 53ceafaa3f4..c7e12f291d7 100644
--- a/lib/gitlab/ci/config/node/stage.rb
+++ b/lib/gitlab/ci/config/node/stage.rb
@@ -10,6 +10,13 @@ module Gitlab
 
           validations do
             validates :config, key: true
+
+            validate do |entry|
+              unless entry.global
+                raise Entry::InvalidError,
+                  'This entry needs reference to global configuration'
+              end
+            end
           end
 
           def self.default
diff --git a/spec/lib/gitlab/ci/config/node/stage_spec.rb b/spec/lib/gitlab/ci/config/node/stage_spec.rb
index 653d613ba6e..92150ea5337 100644
--- a/spec/lib/gitlab/ci/config/node/stage_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/stage_spec.rb
@@ -1,11 +1,12 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Stage do
-  let(:entry) { described_class.new(config) }
+  let(:entry) { described_class.new(config, global: global) }
+  let(:global) { spy('Global') }
 
   describe 'validations' do
     context 'when entry config value is correct' do
-      let(:config) { :stage1 }
+      let(:config) { :build }
 
       describe '#value' do
         it 'returns a stage key' do
@@ -18,20 +19,32 @@ describe Gitlab::Ci::Config::Node::Stage do
           expect(entry).to be_valid
         end
       end
+    end
 
-      context 'when entry config is incorrect' do
-        let(:config) { { test: true } }
+    context 'when entry config is incorrect' do
+      describe '#errors' do
+        context 'when reference to global node is not set' do
+          let(:entry) { described_class.new(config) }
 
-        describe '#errors' do
-          it 'reports errors' do
+          it 'raises error' do
+            expect { entry }
+              .to raise_error Gitlab::Ci::Config::Node::Entry::InvalidError
+          end
+        end
+
+        context 'when value has a wrong type' do
+          let(:config) { { test: true } }
+
+          it 'reports errors about wrong type' do
             expect(entry.errors)
               .to include 'stage config should be a string or symbol'
           end
         end
 
-        describe '#valid?' do
-          it 'is not valid' do
-            expect(entry).not_to be_valid
+        context 'when stage is not present in global configuration' do
+          pending 'reports error about missing stage' do
+            expect(entry.errors)
+              .to include 'stage config should be one of test, stage'
           end
         end
       end

From 8baee987beaea8197d28ee9715ef23f5813566e5 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 8 Jul 2016 11:27:36 +0200
Subject: [PATCH 19/62] Extract internal attributes validator for CI entry

---
 lib/gitlab/ci/config/node/stage.rb           | 8 +-------
 lib/gitlab/ci/config/node/validators.rb      | 9 +++++++++
 spec/lib/gitlab/ci/config/node/stage_spec.rb | 6 ++++--
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/lib/gitlab/ci/config/node/stage.rb b/lib/gitlab/ci/config/node/stage.rb
index c7e12f291d7..9c76cf7c0b7 100644
--- a/lib/gitlab/ci/config/node/stage.rb
+++ b/lib/gitlab/ci/config/node/stage.rb
@@ -10,13 +10,7 @@ module Gitlab
 
           validations do
             validates :config, key: true
-
-            validate do |entry|
-              unless entry.global
-                raise Entry::InvalidError,
-                  'This entry needs reference to global configuration'
-              end
-            end
+            validates :global, required_attribute: true
           end
 
           def self.default
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index 7b2f57990b5..6f0e14e2f0a 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -33,6 +33,15 @@ module Gitlab
             end
           end
 
+          class RequiredAttributeValidator < ActiveModel::EachValidator
+            def validate_each(record, attribute, value)
+              if value.nil?
+                raise Entry::InvalidError,
+                  "Entry needs #{attribute} attribute set internally."
+              end
+            end
+          end
+
           class KeyValidator < ActiveModel::EachValidator
             include LegacyValidationHelpers
 
diff --git a/spec/lib/gitlab/ci/config/node/stage_spec.rb b/spec/lib/gitlab/ci/config/node/stage_spec.rb
index 92150ea5337..4047d46c80f 100644
--- a/spec/lib/gitlab/ci/config/node/stage_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/stage_spec.rb
@@ -27,8 +27,10 @@ describe Gitlab::Ci::Config::Node::Stage do
           let(:entry) { described_class.new(config) }
 
           it 'raises error' do
-            expect { entry }
-              .to raise_error Gitlab::Ci::Config::Node::Entry::InvalidError
+            expect { entry }.to raise_error(
+              Gitlab::Ci::Config::Node::Entry::InvalidError,
+              /Entry needs global attribute set internally./
+            )
           end
         end
 

From 159aed1c4220277ece9b4496d460f931cd65e228 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 8 Jul 2016 11:29:03 +0200
Subject: [PATCH 20/62] Extract global CI config entry configuration setup

---
 lib/gitlab/ci/config/node/global.rb | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index dffa3326630..110d982588b 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -39,21 +39,24 @@ module Gitlab
           helpers :before_script, :image, :services, :after_script,
                   :variables, :stages, :types, :cache, :jobs
 
-          def initialize(config)
-            return super unless config.is_a?(Hash)
-
-            jobs = config.except(*nodes.keys)
-            global = config.slice(*nodes.keys)
-
-            super(global.merge(jobs: jobs))
+          def initialize(config, **attributes)
+            super(setup(config), attributes)
+            @global = self
           end
 
           def stages
             stages_defined? ? stages_value : types_value
           end
 
-          def global
-            self
+          private
+
+          def setup(config)
+            return config unless config.is_a?(Hash)
+
+            jobs = config.except(*nodes.keys)
+            global = config.slice(*nodes.keys)
+
+            global.merge(jobs: jobs)
           end
         end
       end

From 1ac62de2c12a26e6f5158cdb4f008a71729b39fc Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 8 Jul 2016 12:51:47 +0200
Subject: [PATCH 21/62] Extract CI entry node validator and improve naming

---
 lib/gitlab/ci/config.rb                     |  1 +
 lib/gitlab/ci/config/node/configurable.rb   |  6 ++--
 lib/gitlab/ci/config/node/entry.rb          | 36 ++++++++++-----------
 lib/gitlab/ci/config/node/jobs.rb           |  6 ++--
 spec/lib/gitlab/ci/config/node/jobs_spec.rb |  5 ++-
 5 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index ae82c0db3f1..20f5f8e2ff8 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -15,6 +15,7 @@ module Gitlab
 
         @global = Node::Global.new(@config)
         @global.process!
+        @global.validate!
       end
 
       def valid?
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 88403a9de1e..e5780c60e70 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -25,7 +25,7 @@ module Gitlab
 
           private
 
-          def create_node(key, factory)
+          def create(key, factory)
             factory
               .value(config[key])
               .with(key: key, parent: self, global: global)
@@ -50,12 +50,12 @@ module Gitlab
             def helpers(*nodes)
               nodes.each do |symbol|
                 define_method("#{symbol}_defined?") do
-                  @nodes[symbol].try(:defined?)
+                  @entries[symbol].try(:defined?)
                 end
 
                 define_method("#{symbol}_value") do
                   raise Entry::InvalidError unless valid?
-                  @nodes[symbol].try(:value)
+                  @entries[symbol].try(:value)
                 end
 
                 alias_method symbol.to_sym, "#{symbol}_value".to_sym
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index e8b0160edc1..67e59ffb86e 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -13,7 +13,7 @@ module Gitlab
 
           def initialize(config, **attributes)
             @config = config
-            @nodes = {}
+            @entries = {}
 
             (@attributes = attributes).each do |attribute, value|
               public_send("#{attribute}=", value)
@@ -24,8 +24,18 @@ module Gitlab
           end
 
           def process!
-            compose! unless leaf?
-            @validator.validate(:processed) if valid?
+            return unless valid?
+
+            nodes.each do |key, essence|
+              @entries[key] = create(key, essence)
+            end
+
+            @entries.each_value(&:process!)
+          end
+
+          def validate!
+            @validator.validate(:after)
+            @entries.each_value(&:validate!)
           end
 
           def leaf?
@@ -37,7 +47,7 @@ module Gitlab
           end
 
           def descendants
-            @nodes.values
+            @entries.values
           end
 
           def ancestors
@@ -49,18 +59,18 @@ module Gitlab
           end
 
           def errors
-            @validator.messages + @nodes.values.flat_map(&:errors)
+            @validator.messages + @entries.values.flat_map(&:errors)
           end
 
           def value
             if leaf?
               @config
             else
-              meaningful = @nodes.select do |_key, value|
+              meaningful = @entries.select do |_key, value|
                 value.defined? && value.relevant?
               end
 
-              Hash[meaningful.map { |key, node| [key, node.value] }]
+              Hash[meaningful.map { |key, entry| [key, entry.value] }]
             end
           end
 
@@ -85,17 +95,7 @@ module Gitlab
 
           private
 
-          def compose!
-            return unless valid?
-
-            nodes.each do |key, essence|
-              @nodes[key] = create_node(key, essence)
-            end
-
-            @nodes.each_value(&:process!)
-          end
-
-          def create_node(key, essence)
+          def create(entry, essence)
             raise NotImplementedError
           end
         end
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index cba1fce4a4c..6199749a508 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -10,7 +10,7 @@ module Gitlab
 
           validations do
             validates :config, type: Hash
-            validate :jobs_presence, on: :processed
+            validate :jobs_presence, on: :after
 
             def jobs_presence
               unless relevant?
@@ -24,12 +24,12 @@ module Gitlab
           end
 
           def relevant?
-            @nodes.values.any?(&:relevant?)
+            @entries.values.any?(&:relevant?)
           end
 
           private
 
-          def create_node(name, config)
+          def create(name, config)
             job_node(name).new(config, job_attributes(name))
           end
 
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 7ec28b642b4..1ecc3e18d4e 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -35,7 +35,10 @@ describe Gitlab::Ci::Config::Node::Jobs do
           end
 
           context 'when processed' do
-            before { entry.process! }
+            before do
+              entry.process!
+              entry.validate!
+            end
 
             it 'returns error about no visible jobs defined' do
               expect(entry.errors)

From d9142f2c97524fc2d5af7dda79b849d1e23f4910 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 8 Jul 2016 13:31:41 +0200
Subject: [PATCH 22/62] Add CI config known stage validation for job stage

---
 lib/gitlab/ci/config/node/stage.rb           | 13 +++++
 spec/lib/gitlab/ci/config/node/stage_spec.rb | 54 ++++++++++++++++----
 2 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/lib/gitlab/ci/config/node/stage.rb b/lib/gitlab/ci/config/node/stage.rb
index 9c76cf7c0b7..457f6dfa3ba 100644
--- a/lib/gitlab/ci/config/node/stage.rb
+++ b/lib/gitlab/ci/config/node/stage.rb
@@ -11,6 +11,19 @@ module Gitlab
           validations do
             validates :config, key: true
             validates :global, required_attribute: true
+            validate :known_stage, on: :after
+
+            def known_stage
+              unless known?
+                stages_list = global.stages.join(', ')
+                errors.add(:config,
+                           "should be one of defined stages (#{stages_list})")
+              end
+            end
+          end
+
+          def known?
+            @global.stages.include?(@config)
           end
 
           def self.default
diff --git a/spec/lib/gitlab/ci/config/node/stage_spec.rb b/spec/lib/gitlab/ci/config/node/stage_spec.rb
index 4047d46c80f..95b46d76adb 100644
--- a/spec/lib/gitlab/ci/config/node/stage_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/stage_spec.rb
@@ -1,33 +1,33 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Stage do
-  let(:entry) { described_class.new(config, global: global) }
+  let(:stage) { described_class.new(config, global: global) }
   let(:global) { spy('Global') }
 
   describe 'validations' do
-    context 'when entry config value is correct' do
+    context 'when stage config value is correct' do
       let(:config) { :build }
 
       describe '#value' do
         it 'returns a stage key' do
-          expect(entry.value).to eq config
+          expect(stage.value).to eq config
         end
       end
 
       describe '#valid?' do
         it 'is valid' do
-          expect(entry).to be_valid
+          expect(stage).to be_valid
         end
       end
     end
 
-    context 'when entry config is incorrect' do
+    context 'when stage config is incorrect' do
       describe '#errors' do
         context 'when reference to global node is not set' do
-          let(:entry) { described_class.new(config) }
+          let(:stage) { described_class.new(config) }
 
           it 'raises error' do
-            expect { entry }.to raise_error(
+            expect { stage }.to raise_error(
               Gitlab::Ci::Config::Node::Entry::InvalidError,
               /Entry needs global attribute set internally./
             )
@@ -38,21 +38,53 @@ describe Gitlab::Ci::Config::Node::Stage do
           let(:config) { { test: true } }
 
           it 'reports errors about wrong type' do
-            expect(entry.errors)
+            expect(stage.errors)
               .to include 'stage config should be a string or symbol'
           end
         end
 
         context 'when stage is not present in global configuration' do
-          pending 'reports error about missing stage' do
-            expect(entry.errors)
-              .to include 'stage config should be one of test, stage'
+          let(:config) { :unknown }
+
+          before do
+            allow(global)
+              .to receive(:stages).and_return([:test, :deploy])
+          end
+
+          it 'reports error about missing stage' do
+            stage.validate!
+
+            expect(stage.errors)
+              .to include 'stage config should be one of ' \
+                          'defined stages (test, deploy)'
           end
         end
       end
     end
   end
 
+  describe '#known?' do
+    before do
+      allow(global).to receive(:stages).and_return([:test, :deploy])
+    end
+
+    context 'when stage is not known' do
+      let(:config) { :unknown }
+
+      it 'returns false' do
+        expect(stage.known?).to be false
+      end
+    end
+
+    context 'when stage is known' do
+      let(:config) { :test }
+
+      it 'returns false' do
+        expect(stage.known?).to be true
+      end
+    end
+  end
+
   describe '.default' do
     it 'returns default stage' do
       expect(described_class.default).to eq :test

From 1b624ba4eb423c39671b9f0440018e9783aa844b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 8 Jul 2016 13:36:01 +0200
Subject: [PATCH 23/62] Improve name of CI entry validation context hook

---
 lib/gitlab/ci/config/node/entry.rb | 2 +-
 lib/gitlab/ci/config/node/jobs.rb  | 2 +-
 lib/gitlab/ci/config/node/stage.rb | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 67e59ffb86e..2b16a81f88d 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -34,7 +34,7 @@ module Gitlab
           end
 
           def validate!
-            @validator.validate(:after)
+            @validator.validate(:processed)
             @entries.each_value(&:validate!)
           end
 
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 6199749a508..f6acc25e4fb 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -10,7 +10,7 @@ module Gitlab
 
           validations do
             validates :config, type: Hash
-            validate :jobs_presence, on: :after
+            validate :jobs_presence, on: :processed
 
             def jobs_presence
               unless relevant?
diff --git a/lib/gitlab/ci/config/node/stage.rb b/lib/gitlab/ci/config/node/stage.rb
index 457f6dfa3ba..c15f46bc7a5 100644
--- a/lib/gitlab/ci/config/node/stage.rb
+++ b/lib/gitlab/ci/config/node/stage.rb
@@ -11,7 +11,7 @@ module Gitlab
           validations do
             validates :config, key: true
             validates :global, required_attribute: true
-            validate :known_stage, on: :after
+            validate :known_stage, on: :processed
 
             def known_stage
               unless known?

From ccbdb4022ac87f7c30e970922be64bcea0b406e9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 9 Jul 2016 14:56:41 +0200
Subject: [PATCH 24/62] Integrate CI job stage entry into CI configuration

---
 lib/gitlab/ci/config/node/entry.rb            | 18 +++++++++-----
 lib/gitlab/ci/config/node/job.rb              | 24 +++++++++++++++++++
 lib/gitlab/ci/config/node/stage.rb            |  4 ++--
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  |  6 ++---
 spec/lib/gitlab/ci/config/node/global_spec.rb |  4 ++--
 spec/lib/gitlab/ci/config/node/job_spec.rb    | 12 ++++++++--
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   |  7 +++---
 spec/lib/gitlab/ci/config/node/stage_spec.rb  | 18 ++++++++------
 8 files changed, 68 insertions(+), 25 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 2b16a81f88d..1940c39087b 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -20,21 +20,21 @@ module Gitlab
             end
 
             @validator = self.class.validator.new(self)
-            @validator.validate
+            @validator.validate(:new)
           end
 
           def process!
             return unless valid?
 
-            nodes.each do |key, essence|
-              @entries[key] = create(key, essence)
-            end
-
+            compose!
             @entries.each_value(&:process!)
           end
 
           def validate!
-            @validator.validate(:processed)
+            if @validator.valid?(:new)
+              @validator.validate(:processed)
+            end
+
             @entries.each_value(&:validate!)
           end
 
@@ -95,6 +95,12 @@ module Gitlab
 
           private
 
+          def compose!
+            nodes.each do |key, essence|
+              @entries[key] = create(key, essence)
+            end
+          end
+
           def create(entry, essence)
             raise NotImplementedError
           end
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 5be8cb39a87..4a9cc28d763 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -7,6 +7,30 @@ module Gitlab
         #
         class Job < Entry
           include Configurable
+
+          node :stage, Stage,
+            description: 'Pipeline stage this job will be executed into.'
+
+          node :type, Stage,
+            description: 'Deprecated: stage this job will be executed into.'
+
+          helpers :stage, :type
+
+          def value
+            @config.merge(stage: stage_value)
+          end
+
+          private
+
+          def compose!
+            super
+
+            if type_defined? && !stage_defined?
+              @entries[:stage] = @entries[:type]
+            end
+
+            @entries.delete(:type)
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/stage.rb b/lib/gitlab/ci/config/node/stage.rb
index c15f46bc7a5..e8fae65a2a9 100644
--- a/lib/gitlab/ci/config/node/stage.rb
+++ b/lib/gitlab/ci/config/node/stage.rb
@@ -9,7 +9,7 @@ module Gitlab
           include Validatable
 
           validations do
-            validates :config, key: true
+            validates :config, type: String
             validates :global, required_attribute: true
             validate :known_stage, on: :processed
 
@@ -27,7 +27,7 @@ module Gitlab
           end
 
           def self.default
-            :test
+            'test'
           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 ac058ba1595..03477e1ca13 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1082,21 +1082,21 @@ EOT
         config = YAML.dump({ rspec: { script: "test", type: 1 } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:type config should be a string")
       end
 
       it "returns errors if job stage is not a pre-defined stage" do
         config = YAML.dump({ rspec: { script: "test", type: "acceptance" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:type config should be one of defined stages (build, test, deploy)")
       end
 
       it "returns errors if job stage is not a defined stage" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", type: "acceptance" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:type config should be one of defined stages (build, test)")
       end
 
       it "returns errors if stages 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 a98de73c06c..0c56b59db0c 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -129,8 +129,8 @@ describe Gitlab::Ci::Config::Node::Global do
         describe '#jobs' do
           it 'returns jobs configuration' do
             expect(global.jobs)
-              .to eq(rspec: { script: 'rspec' },
-                     spinach: { script: 'spinach' })
+              .to eq(rspec: { script: 'rspec', stage: 'test' },
+                     spinach: { script: 'spinach', stage: 'test' })
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 15c7f9bc394..2a4296448fb 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -1,15 +1,23 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Job do
-  let(:entry) { described_class.new(config) }
+  let(:entry) { described_class.new(config, global: global) }
+  let(:global) { spy('Global') }
 
   describe 'validations' do
+    before do
+      entry.process!
+      entry.validate!
+    end
+
     context 'when entry config value is correct' do
       let(:config) { { script: 'rspec' } }
 
       describe '#value' do
         it 'returns key value' do
-          expect(entry.value).to eq(script: 'rspec')
+          expect(entry.value)
+            .to eq(script: 'rspec',
+                   stage: 'test')
         end
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 1ecc3e18d4e..52018958dcf 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Jobs do
-  let(:entry) { described_class.new(config) }
+  let(:entry) { described_class.new(config, global: spy) }
 
   describe 'validations' do
     context 'when entry config value is correct' do
@@ -61,8 +61,9 @@ describe Gitlab::Ci::Config::Node::Jobs do
 
     describe '#value' do
       it 'returns key value' do
-        expect(entry.value).to eq(rspec: { script: 'rspec' },
-                                  spinach: { script: 'spinach' })
+        expect(entry.value)
+          .to eq(rspec: { script: 'rspec', stage: 'test' },
+                 spinach: { script: 'spinach', stage: 'test' })
       end
     end
 
diff --git a/spec/lib/gitlab/ci/config/node/stage_spec.rb b/spec/lib/gitlab/ci/config/node/stage_spec.rb
index 95b46d76adb..6deeca1a6c0 100644
--- a/spec/lib/gitlab/ci/config/node/stage_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/stage_spec.rb
@@ -6,7 +6,11 @@ describe Gitlab::Ci::Config::Node::Stage do
 
   describe 'validations' do
     context 'when stage config value is correct' do
-      let(:config) { :build }
+      let(:config) { 'build' }
+
+      before do
+        allow(global).to receive(:stages).and_return(%w[build])
+      end
 
       describe '#value' do
         it 'returns a stage key' do
@@ -39,16 +43,16 @@ describe Gitlab::Ci::Config::Node::Stage do
 
           it 'reports errors about wrong type' do
             expect(stage.errors)
-              .to include 'stage config should be a string or symbol'
+              .to include 'stage config should be a string'
           end
         end
 
         context 'when stage is not present in global configuration' do
-          let(:config) { :unknown }
+          let(:config) { 'unknown' }
 
           before do
             allow(global)
-              .to receive(:stages).and_return([:test, :deploy])
+              .to receive(:stages).and_return(%w[test deploy])
           end
 
           it 'reports error about missing stage' do
@@ -65,7 +69,7 @@ describe Gitlab::Ci::Config::Node::Stage do
 
   describe '#known?' do
     before do
-      allow(global).to receive(:stages).and_return([:test, :deploy])
+      allow(global).to receive(:stages).and_return(%w[test deploy])
     end
 
     context 'when stage is not known' do
@@ -77,7 +81,7 @@ describe Gitlab::Ci::Config::Node::Stage do
     end
 
     context 'when stage is known' do
-      let(:config) { :test }
+      let(:config) { 'test' }
 
       it 'returns false' do
         expect(stage.known?).to be true
@@ -87,7 +91,7 @@ describe Gitlab::Ci::Config::Node::Stage do
 
   describe '.default' do
     it 'returns default stage' do
-      expect(described_class.default).to eq :test
+      expect(described_class.default).to eq 'test'
     end
   end
 end

From 9edced40dd7398d1aa553a5454f95ae629da2276 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 9 Jul 2016 16:51:26 +0200
Subject: [PATCH 25/62] Use node factory to assemble global CI config entry

---
 lib/gitlab/ci/config/node/configurable.rb     |  4 +--
 lib/gitlab/ci/config/node/global.rb           | 36 +++++++++++--------
 spec/lib/gitlab/ci/config/node/global_spec.rb |  4 +--
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index e5780c60e70..7a43d494d3d 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -27,8 +27,8 @@ module Gitlab
 
           def create(key, factory)
             factory
-              .value(config[key])
-              .with(key: key, parent: self, global: global)
+              .value(@config[key])
+              .with(key: key, parent: self, global: @global)
 
             factory.create!
           end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 110d982588b..a2649e2c905 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -33,30 +33,38 @@ module Gitlab
           node :cache, Node::Cache,
             description: 'Configure caching between build jobs.'
 
-          node :jobs, Node::Jobs,
-            description: 'Definition of jobs for this pipeline.'
-
           helpers :before_script, :image, :services, :after_script,
                   :variables, :stages, :types, :cache, :jobs
 
-          def initialize(config, **attributes)
-            super(setup(config), attributes)
+          def initialize(*)
+            super
             @global = self
           end
 
-          def stages
-            stages_defined? ? stages_value : types_value
-          end
-
           private
 
-          def setup(config)
-            return config unless config.is_a?(Hash)
+          def compose!
+            super
 
-            jobs = config.except(*nodes.keys)
-            global = config.slice(*nodes.keys)
+            compose_stages!
+            compose_jobs!
+          end
 
-            global.merge(jobs: jobs)
+          def compose_stages!
+            factory = Node::Factory.new(Node::Jobs)
+            factory.value(@config.except(*nodes.keys))
+            factory.with(key: :jobs, parent: self, global: self)
+            factory.with(description: 'Jobs definition for this pipeline')
+
+            @entries[:jobs] = factory.create!
+          end
+
+          def compose_jobs!
+            if types_defined? && !stages_defined?
+              @entries[:stages] = @entries[:types]
+            end
+
+            @entries.delete(:types)
           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 0c56b59db0c..10e5f05a2d5 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -35,7 +35,7 @@ describe Gitlab::Ci::Config::Node::Global do
         end
 
         it 'creates node object for each entry' do
-          expect(global.descendants.count).to eq 9
+          expect(global.descendants.count).to eq 8
         end
 
         it 'creates node object using valid class' do
@@ -142,7 +142,7 @@ describe Gitlab::Ci::Config::Node::Global do
 
       describe '#nodes' do
         it 'instantizes all nodes' do
-          expect(global.descendants.count).to eq 9
+          expect(global.descendants.count).to eq 8
         end
 
         it 'contains undefined nodes' do

From e9b42067fd4065e57a68e4b37732dda91444e3f7 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 9 Jul 2016 17:06:53 +0200
Subject: [PATCH 26/62] Remove CI job stage code from legacy config processor

---
 lib/ci/gitlab_ci_yaml_processor.rb | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index f0710690985..1ffbd0020bb 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -57,6 +57,9 @@ module Ci
     private
 
     def initial_parsing
+      ##
+      # Global config
+      #
       @before_script = @ci_config.before_script
       @image = @ci_config.image
       @after_script = @ci_config.after_script
@@ -65,24 +68,16 @@ module Ci
       @stages = @ci_config.stages
       @cache = @ci_config.cache
 
-      @jobs = {}
-
-      @ci_config.jobs.each do |name, param|
-        add_job(name, param)
-      end
+      ##
+      # Jobs
+      #
+      @jobs = @ci_config.jobs
 
       @jobs.each do |name, job|
         validate_job!(name, job)
       end
     end
 
-    def add_job(name, job)
-      raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
-
-      stage = job[:stage] || job[:type] || DEFAULT_STAGE
-      @jobs[name] = { stage: stage }.merge(job)
-    end
-
     def build_job(name, job)
       {
         stage_idx: @stages.index(job[:stage]),
@@ -112,12 +107,13 @@ module Ci
     end
 
     def validate_job!(name, job)
+      raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
+
       validate_job_name!(name)
       validate_job_keys!(name, job)
       validate_job_types!(name, job)
       validate_job_script!(name, job)
 
-      validate_job_stage!(name, job) if job[:stage]
       validate_job_variables!(name, job) if job[:variables]
       validate_job_cache!(name, job) if job[:cache]
       validate_job_artifacts!(name, job) if job[:artifacts]
@@ -186,12 +182,6 @@ module Ci
       end
     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(", ")}"
-      end
-    end
-
     def validate_job_variables!(name, job)
       unless validate_variables(job[:variables])
         raise ValidationError,

From a80a01e8411cee9e0f7d24ddddb65dca0c7a7fdf Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 9 Jul 2016 17:38:03 +0200
Subject: [PATCH 27/62] Add comment for deprecated CI config `types` entry

---
 lib/ci/gitlab_ci_yaml_processor.rb  |  4 +---
 lib/gitlab/ci/config/node/global.rb | 11 +++++++++--
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 1ffbd0020bb..40d1b475013 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -17,9 +17,7 @@ module Ci
 
     def initialize(config, path = nil)
       @ci_config = Gitlab::Ci::Config.new(config)
-      @config = @ci_config.to_hash
-
-      @path = path
+      @config, @path = @ci_config.to_hash, path
 
       unless @ci_config.valid?
         raise ValidationError, @ci_config.errors.first
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index a2649e2c905..f59a967b1ef 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -50,7 +50,7 @@ module Gitlab
             compose_jobs!
           end
 
-          def compose_stages!
+          def compose_jobs!
             factory = Node::Factory.new(Node::Jobs)
             factory.value(@config.except(*nodes.keys))
             factory.with(key: :jobs, parent: self, global: self)
@@ -59,7 +59,14 @@ module Gitlab
             @entries[:jobs] = factory.create!
           end
 
-          def compose_jobs!
+          def compose_stages!
+            ##
+            # Deprecated `:types` key workaround - if types are defined and
+            # stages are not defined we use types definition as stages.
+            #
+            # Otherwise we use stages in favor of types, and remove types from
+            # processing.
+            #
             if types_defined? && !stages_defined?
               @entries[:stages] = @entries[:types]
             end

From 2480701436bf84281e4afd65eb0d4c2d642754b9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 9 Jul 2016 18:43:26 +0200
Subject: [PATCH 28/62] Extend CI job entries fabrication and validation

---
 lib/gitlab/ci/config/node/global.rb           |  1 +
 lib/gitlab/ci/config/node/hidden_job.rb       |  1 +
 lib/gitlab/ci/config/node/job.rb              |  4 +++
 lib/gitlab/ci/config/node/jobs.rb             | 18 ++++++-----
 spec/lib/gitlab/ci/config/node/global_spec.rb |  2 +-
 .../gitlab/ci/config/node/hidden_job_spec.rb  | 10 +++++++
 spec/lib/gitlab/ci/config/node/job_spec.rb    | 10 +++++++
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   | 30 +++++++++----------
 8 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index f59a967b1ef..f5500e27439 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -38,6 +38,7 @@ module Gitlab
 
           def initialize(*)
             super
+
             @global = self
           end
 
diff --git a/lib/gitlab/ci/config/node/hidden_job.rb b/lib/gitlab/ci/config/node/hidden_job.rb
index 6a559ee8c04..073044b66f8 100644
--- a/lib/gitlab/ci/config/node/hidden_job.rb
+++ b/lib/gitlab/ci/config/node/hidden_job.rb
@@ -10,6 +10,7 @@ module Gitlab
 
           validations do
             validates :config, type: Hash
+            validates :config, presence: true
           end
 
           def relevant?
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 4a9cc28d763..28a3f407605 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -8,6 +8,10 @@ module Gitlab
         class Job < Entry
           include Configurable
 
+          validations do
+            validates :config, presence: true
+          end
+
           node :stage, Stage,
             description: 'Pipeline stage this job will be executed into.'
 
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index f6acc25e4fb..7a164b69aff 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -30,17 +30,19 @@ module Gitlab
           private
 
           def create(name, config)
-            job_node(name).new(config, job_attributes(name))
+            Node::Factory.new(job_node(name))
+              .value(config || {})
+              .with(key: name, parent: self, global: @global)
+              .with(description: "#{name} job definition.")
+              .create!
           end
 
           def job_node(name)
-            name.to_s.start_with?('.') ? Node::HiddenJob : Node::Job
-          end
-
-          def job_attributes(name)
-            @attributes.merge(key: name,
-                              parent: self,
-                              description: "#{name} job definition.")
+            if name.to_s.start_with?('.')
+              Node::HiddenJob
+            else
+              Node::Job
+            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 10e5f05a2d5..3e1c197fe61 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -137,7 +137,7 @@ describe Gitlab::Ci::Config::Node::Global do
     end
 
     context 'when most of entires not defined' do
-      let(:hash) { { cache: { key: 'a' }, rspec: {} } }
+      let(:hash) { { cache: { key: 'a' }, rspec: { script: %w[ls] } } }
       before { global.process! }
 
       describe '#nodes' do
diff --git a/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb b/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb
index ab865c3522e..cc44e2cc054 100644
--- a/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb
@@ -31,6 +31,16 @@ describe Gitlab::Ci::Config::Node::HiddenJob do
           end
         end
       end
+
+      context 'when config is empty' do
+        let(:config) { {} }
+
+        describe '#valid' do
+          it 'is invalid' do
+            expect(entry).not_to be_valid
+          end
+        end
+      end
     end
   end
 
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 2a4296448fb..f841936ee6b 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -39,6 +39,16 @@ describe Gitlab::Ci::Config::Node::Job do
           end
         end
       end
+
+      context 'when config is empty' do
+        let(:config) { {} }
+
+        describe '#valid' do
+          it 'is invalid' do
+            expect(entry).not_to be_valid
+          end
+        end
+      end
     end
   end
 
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 52018958dcf..b0171174157 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -4,6 +4,11 @@ describe Gitlab::Ci::Config::Node::Jobs do
   let(:entry) { described_class.new(config, global: spy) }
 
   describe 'validations' do
+    before do
+      entry.process!
+      entry.validate!
+    end
+
     context 'when entry config value is correct' do
       let(:config) { { rspec: { script: 'rspec' } } }
 
@@ -25,25 +30,20 @@ describe Gitlab::Ci::Config::Node::Jobs do
           end
         end
 
-        context 'when no visible jobs present' do
-          let(:config) { { '.hidden'.to_sym => {} } }
+        context 'when job is unspecified' do
+          let(:config) { { rspec: nil } }
 
-          context 'when not processed' do
-            it 'is valid' do
-              expect(entry.errors).to be_empty
-            end
+          it 'is not valid' do
+            expect(entry).not_to be_valid
           end
+        end
 
-          context 'when processed' do
-            before do
-              entry.process!
-              entry.validate!
-            end
+        context 'when no visible jobs present' do
+          let(:config) { { '.hidden'.to_sym => { script: [] } } }
 
-            it 'returns error about no visible jobs defined' do
-              expect(entry.errors)
-                .to include 'jobs config should contain at least one visible job'
-            end
+          it 'returns error about no visible jobs defined' do
+            expect(entry.errors)
+              .to include 'jobs config should contain at least one visible job'
           end
         end
       end

From 3c5b1da2a1f15be9e032ec23f56de0af8002ec6b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 10 Jul 2016 13:54:39 +0200
Subject: [PATCH 29/62] Add before_script node to CI job entry config

---
 lib/gitlab/ci/config/node/job.rb             | 18 +++++++-
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb |  2 +-
 spec/lib/gitlab/ci/config/node/job_spec.rb   | 46 ++++++++++++++------
 3 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 28a3f407605..2f62248bb00 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -12,20 +12,34 @@ module Gitlab
             validates :config, presence: true
           end
 
+          node :before_script, Script,
+            description: 'Global before script overridden in this job.'
+
           node :stage, Stage,
             description: 'Pipeline stage this job will be executed into.'
 
           node :type, Stage,
             description: 'Deprecated: stage this job will be executed into.'
 
-          helpers :stage, :type
+          helpers :before_script, :stage, :type
 
           def value
-            @config.merge(stage: stage_value)
+            raise InvalidError unless valid?
+
+            ##
+            # TODO, refactoring step: do not expose internal configuration,
+            # return only hash value without merging it to internal config.
+            #
+            @config.merge(to_hash.compact)
           end
 
           private
 
+          def to_hash
+            { before_script: before_script,
+              stage: stage }
+          end
+
           def compose!
             super
 
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 03477e1ca13..230106b74ae 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -970,7 +970,7 @@ EOT
         config = YAML.dump({ rspec: { script: "test", before_script: [10, "test"] } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: before_script should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:before_script config should be an array of strings")
       end
 
       it "returns errors if after_script parameter is invalid" do
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index f841936ee6b..032fbb9c27f 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -4,23 +4,15 @@ describe Gitlab::Ci::Config::Node::Job do
   let(:entry) { described_class.new(config, global: global) }
   let(:global) { spy('Global') }
 
-  describe 'validations' do
-    before do
-      entry.process!
-      entry.validate!
-    end
+  before do
+    entry.process!
+    entry.validate!
+  end
 
+  describe 'validations' do
     context 'when entry config value is correct' do
       let(:config) { { script: 'rspec' } }
 
-      describe '#value' do
-        it 'returns key value' do
-          expect(entry.value)
-            .to eq(script: 'rspec',
-                   stage: 'test')
-        end
-      end
-
       describe '#valid?' do
         it 'is valid' do
           expect(entry).to be_valid
@@ -33,7 +25,7 @@ describe Gitlab::Ci::Config::Node::Job do
         let(:config) { ['incorrect'] }
 
         describe '#errors' do
-          it 'saves errors' do
+          it 'reports error about a config type' do
             expect(entry.errors)
               .to include 'job config should be a hash'
           end
@@ -52,6 +44,32 @@ describe Gitlab::Ci::Config::Node::Job do
     end
   end
 
+  describe '#value' do
+    context 'when entry is correct' do
+      let(:config) do
+        { before_script: %w[ls pwd],
+          script: 'rspec' }
+      end
+
+      it 'returns correct value' do
+        expect(entry.value)
+          .to eq(before_script: %w[ls pwd],
+                 script: 'rspec',
+                 stage: 'test')
+      end
+    end
+
+    context 'when entry is incorrect' do
+      let(:config) { {} }
+
+      it 'raises error' do
+        expect { entry.value }.to raise_error(
+          Gitlab::Ci::Config::Node::Entry::InvalidError
+        )
+      end
+    end
+  end
+
   describe '#relevant?' do
     it 'is a relevant entry' do
       expect(entry).to be_relevant

From 489e9be4e83621ae6f3db311398bf32e1065d1a8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 10 Jul 2016 14:35:53 +0200
Subject: [PATCH 30/62] Add CI job script node in new config processor

---
 lib/gitlab/ci/config/node/job.rb              | 10 ++--
 lib/gitlab/ci/config/node/job_script.rb       | 31 ++++++++++++
 spec/lib/gitlab/ci/config/node/global_spec.rb |  4 +-
 .../gitlab/ci/config/node/job_script_spec.rb  | 49 +++++++++++++++++++
 spec/lib/gitlab/ci/config/node/job_spec.rb    |  2 +-
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   |  4 +-
 6 files changed, 92 insertions(+), 8 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/job_script.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/job_script_spec.rb

diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 2f62248bb00..fc6fe8015d2 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -15,13 +15,16 @@ module Gitlab
           node :before_script, Script,
             description: 'Global before script overridden in this job.'
 
+          node :script, JobScript,
+            description: 'Commands that will be executed in this job.'
+
           node :stage, Stage,
             description: 'Pipeline stage this job will be executed into.'
 
           node :type, Stage,
             description: 'Deprecated: stage this job will be executed into.'
 
-          helpers :before_script, :stage, :type
+          helpers :before_script, :script, :stage, :type
 
           def value
             raise InvalidError unless valid?
@@ -36,8 +39,9 @@ module Gitlab
           private
 
           def to_hash
-            { before_script: before_script,
-              stage: stage }
+            { before_script: before_script_value,
+              script: script_value,
+              stage: stage_value }
           end
 
           def compose!
diff --git a/lib/gitlab/ci/config/node/job_script.rb b/lib/gitlab/ci/config/node/job_script.rb
new file mode 100644
index 00000000000..c3a05dffdd3
--- /dev/null
+++ b/lib/gitlab/ci/config/node/job_script.rb
@@ -0,0 +1,31 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a job script.
+        #
+        class JobScript < Entry
+          include Validatable
+
+          validations do
+            include LegacyValidationHelpers
+
+            validate :string_or_array_of_strings
+
+            def string_or_array_of_strings
+              unless validate_string(config) || validate_array_of_strings(config)
+                errors.add(:config,
+                           'should be a string or an array of strings')
+              end
+            end
+          end
+
+          def value
+            [@config].flatten
+          end
+        end
+      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 3e1c197fe61..786fc5bb6ce 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -129,8 +129,8 @@ describe Gitlab::Ci::Config::Node::Global do
         describe '#jobs' do
           it 'returns jobs configuration' do
             expect(global.jobs)
-              .to eq(rspec: { script: 'rspec', stage: 'test' },
-                     spinach: { script: 'spinach', stage: 'test' })
+              .to eq(rspec: { script: %w[rspec], stage: 'test' },
+                     spinach: { script: %w[spinach], stage: 'test' })
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/job_script_spec.rb b/spec/lib/gitlab/ci/config/node/job_script_spec.rb
new file mode 100644
index 00000000000..6e2ecb6e0ad
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/job_script_spec.rb
@@ -0,0 +1,49 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::JobScript do
+  let(:entry) { described_class.new(config) }
+
+  context 'when entry config value is an array' do
+    let(:config) { ['ls', 'pwd'] }
+
+    describe '#value' do
+      it 'returns array of 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
+  end
+
+  context 'when entry config value is a string' do
+    let(:config) { 'ls' }
+
+    describe '#value' do
+      it 'returns array with single element' do
+        expect(entry.value).to eq ['ls']
+      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 'job script config should be a ' \
+                      'string or an array of strings'
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 032fbb9c27f..3bcac73809d 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -54,7 +54,7 @@ describe Gitlab::Ci::Config::Node::Job do
       it 'returns correct value' do
         expect(entry.value)
           .to eq(before_script: %w[ls pwd],
-                 script: 'rspec',
+                 script: %w[rspec],
                  stage: 'test')
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index b0171174157..255646a001a 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -62,8 +62,8 @@ describe Gitlab::Ci::Config::Node::Jobs do
     describe '#value' do
       it 'returns key value' do
         expect(entry.value)
-          .to eq(rspec: { script: 'rspec', stage: 'test' },
-                 spinach: { script: 'spinach', stage: 'test' })
+          .to eq(rspec: { script: %w[rspec], stage: 'test' },
+                 spinach: { script: %w[spinach], stage: 'test' })
       end
     end
 

From 500b61e14f384eec545c207fa9324906daf2e148 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 10 Jul 2016 14:41:14 +0200
Subject: [PATCH 31/62] Move after script CI job confg to new processor

---
 lib/gitlab/ci/config/node/job.rb             | 8 ++++++--
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 2 +-
 spec/lib/gitlab/ci/config/node/job_spec.rb   | 6 ++++--
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index fc6fe8015d2..050e00a5151 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -24,7 +24,10 @@ module Gitlab
           node :type, Stage,
             description: 'Deprecated: stage this job will be executed into.'
 
-          helpers :before_script, :script, :stage, :type
+          node :after_script, Script,
+            description: 'Commands that will be executed when finishing job.'
+
+          helpers :before_script, :script, :stage, :type, :after_script
 
           def value
             raise InvalidError unless valid?
@@ -41,7 +44,8 @@ module Gitlab
           def to_hash
             { before_script: before_script_value,
               script: script_value,
-              stage: stage_value }
+              stage: stage_value,
+              after_script: after_script_value }
           end
 
           def compose!
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 230106b74ae..a80b0ce5a57 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -984,7 +984,7 @@ EOT
         config = YAML.dump({ rspec: { script: "test", after_script: [10, "test"] } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: after_script should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:after_script config should be an array of strings")
       end
 
       it "returns errors if image parameter is invalid" do
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 3bcac73809d..77efc73632d 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -48,14 +48,16 @@ describe Gitlab::Ci::Config::Node::Job do
     context 'when entry is correct' do
       let(:config) do
         { before_script: %w[ls pwd],
-          script: 'rspec' }
+          script: 'rspec',
+          after_script: %w[cleanup] }
       end
 
       it 'returns correct value' do
         expect(entry.value)
           .to eq(before_script: %w[ls pwd],
                  script: %w[rspec],
-                 stage: 'test')
+                 stage: 'test',
+                 after_script: %w[cleanup])
       end
     end
 

From 6aefb9e99983494b129a011ee0fce57c1398f612 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 10 Jul 2016 14:43:01 +0200
Subject: [PATCH 32/62] Remove CI job script validation from legacy config

---
 lib/ci/gitlab_ci_yaml_processor.rb | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 40d1b475013..ed8dd0f9e47 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -110,7 +110,6 @@ module Ci
       validate_job_name!(name)
       validate_job_keys!(name, job)
       validate_job_types!(name, job)
-      validate_job_script!(name, job)
 
       validate_job_variables!(name, job) if job[:variables]
       validate_job_cache!(name, job) if job[:cache]
@@ -166,20 +165,6 @@ module Ci
       end
     end
 
-    def validate_job_script!(name, job)
-      if !validate_string(job[:script]) && !validate_array_of_strings(job[:script])
-        raise ValidationError, "#{name} job: script should be a string or an array of a strings"
-      end
-
-      if job[:before_script] && !validate_array_of_strings(job[:before_script])
-        raise ValidationError, "#{name} job: before_script should be an array of strings"
-      end
-
-      if job[:after_script] && !validate_array_of_strings(job[:after_script])
-        raise ValidationError, "#{name} job: after_script should be an array of strings"
-      end
-    end
-
     def validate_job_variables!(name, job)
       unless validate_variables(job[:variables])
         raise ValidationError,

From 8f7c98ee2a34cb063428bea81f1420579549a1a5 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 10 Jul 2016 20:26:37 +0200
Subject: [PATCH 33/62] Rename CI config job script entry node to commands

---
 lib/gitlab/ci/config/node/{job_script.rb => commands.rb}  | 8 ++++++--
 lib/gitlab/ci/config/node/job.rb                          | 2 +-
 .../config/node/{job_script_spec.rb => commands_spec.rb}  | 4 ++--
 3 files changed, 9 insertions(+), 5 deletions(-)
 rename lib/gitlab/ci/config/node/{job_script.rb => commands.rb} (75%)
 rename spec/lib/gitlab/ci/config/node/{job_script_spec.rb => commands_spec.rb} (90%)

diff --git a/lib/gitlab/ci/config/node/job_script.rb b/lib/gitlab/ci/config/node/commands.rb
similarity index 75%
rename from lib/gitlab/ci/config/node/job_script.rb
rename to lib/gitlab/ci/config/node/commands.rb
index c3a05dffdd3..f7e6950001e 100644
--- a/lib/gitlab/ci/config/node/job_script.rb
+++ b/lib/gitlab/ci/config/node/commands.rb
@@ -5,7 +5,7 @@ module Gitlab
         ##
         # Entry that represents a job script.
         #
-        class JobScript < Entry
+        class Commands < Entry
           include Validatable
 
           validations do
@@ -14,11 +14,15 @@ module Gitlab
             validate :string_or_array_of_strings
 
             def string_or_array_of_strings
-              unless validate_string(config) || validate_array_of_strings(config)
+              unless config_valid?
                 errors.add(:config,
                            'should be a string or an array of strings')
               end
             end
+
+            def config_valid?
+              validate_string(config) || validate_array_of_strings(config)
+            end
           end
 
           def value
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 050e00a5151..9a019216ca3 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -15,7 +15,7 @@ module Gitlab
           node :before_script, Script,
             description: 'Global before script overridden in this job.'
 
-          node :script, JobScript,
+          node :script, Commands,
             description: 'Commands that will be executed in this job.'
 
           node :stage, Stage,
diff --git a/spec/lib/gitlab/ci/config/node/job_script_spec.rb b/spec/lib/gitlab/ci/config/node/commands_spec.rb
similarity index 90%
rename from spec/lib/gitlab/ci/config/node/job_script_spec.rb
rename to spec/lib/gitlab/ci/config/node/commands_spec.rb
index 6e2ecb6e0ad..e373c40706f 100644
--- a/spec/lib/gitlab/ci/config/node/job_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/commands_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Ci::Config::Node::JobScript do
+describe Gitlab::Ci::Config::Node::Commands do
   let(:entry) { described_class.new(config) }
 
   context 'when entry config value is an array' do
@@ -41,7 +41,7 @@ describe Gitlab::Ci::Config::Node::JobScript do
     describe '#errors' do
       it 'saves errors' do
         expect(entry.errors)
-          .to include 'job script config should be a ' \
+          .to include 'commands config should be a ' \
                       'string or an array of strings'
       end
     end

From 80587064eb798f5bf2b18dbb8e65e5a55d1db085 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sun, 10 Jul 2016 20:59:18 +0200
Subject: [PATCH 34/62] Require parent when using node factory in CI config

---
 lib/gitlab/ci/config/node/configurable.rb     |  9 ++--
 lib/gitlab/ci/config/node/factory.rb          | 13 +++++-
 lib/gitlab/ci/config/node/global.rb           |  7 +--
 lib/gitlab/ci/config/node/jobs.rb             |  8 ++--
 .../lib/gitlab/ci/config/node/factory_spec.rb | 43 ++++++++++++++++---
 5 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 7a43d494d3d..36d7d20c110 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -28,7 +28,8 @@ module Gitlab
           def create(key, factory)
             factory
               .value(@config[key])
-              .with(key: key, parent: self, global: @global)
+              .parent(self)
+              .with(key: key)
 
             factory.create!
           end
@@ -40,11 +41,11 @@ module Gitlab
 
             private
 
-            def node(symbol, entry_class, metadata)
-              factory = Node::Factory.new(entry_class)
+            def node(key, node, metadata)
+              factory = Node::Factory.new(node)
                 .with(description: metadata[:description])
 
-              (@nodes ||= {}).merge!(symbol.to_sym => factory)
+              (@nodes ||= {}).merge!(key.to_sym => factory)
             end
 
             def helpers(*nodes)
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 3f2cdf436e3..3488aec4a22 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -18,6 +18,11 @@ module Gitlab
             self
           end
 
+          def parent(parent)
+            @parent = parent
+            self
+          end
+
           def with(attributes)
             @attributes.merge!(attributes)
             self
@@ -25,15 +30,19 @@ module Gitlab
 
           def create!
             raise InvalidFactory unless defined?(@value)
+            raise InvalidFactory unless defined?(@parent)
+
+            attributes = { parent: @parent, global: @parent.global }
+            attributes.merge!(@attributes)
 
             ##
             # We assume that unspecified entry is undefined.
             # See issue #18775.
             #
             if @value.nil?
-              Node::Undefined.new(@node, @attributes)
+              Node::Undefined.new(@node, attributes)
             else
-              @node.new(@value, @attributes)
+              @node.new(@value, attributes)
             end
           end
         end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index f5500e27439..4a958735599 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -53,9 +53,10 @@ module Gitlab
 
           def compose_jobs!
             factory = Node::Factory.new(Node::Jobs)
-            factory.value(@config.except(*nodes.keys))
-            factory.with(key: :jobs, parent: self, global: self)
-            factory.with(description: 'Jobs definition for this pipeline')
+              .value(@config.except(*nodes.keys))
+              .parent(self)
+              .with(key: :jobs, global: self)
+              .with(description: 'Jobs definition for this pipeline')
 
             @entries[:jobs] = factory.create!
           end
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 7a164b69aff..548441df37c 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -30,14 +30,14 @@ module Gitlab
           private
 
           def create(name, config)
-            Node::Factory.new(job_node(name))
+            Node::Factory.new(node(name))
               .value(config || {})
-              .with(key: name, parent: self, global: @global)
-              .with(description: "#{name} job definition.")
+              .parent(self)
+              .with(key: name, description: "#{name} job definition.")
               .create!
           end
 
-          def job_node(name)
+          def node(name)
             if name.to_s.start_with?('.')
               Node::HiddenJob
             else
diff --git a/spec/lib/gitlab/ci/config/node/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb
index c912b1b2044..bc6bf32ffbf 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -2,22 +2,40 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Factory do
   describe '#create!' do
-    let(:factory) { described_class.new(entry_class) }
-    let(:entry_class) { Gitlab::Ci::Config::Node::Script }
+    let(:factory) { described_class.new(node) }
+    let(:node) { Gitlab::Ci::Config::Node::Script }
+    let(:parent) { double('parent') }
+    let(:global) { double('global') }
 
-    context 'when setting up a value' do
+    before do
+      allow(parent).to receive(:global).and_return(global)
+    end
+
+    context 'when setting a concrete value' do
       it 'creates entry with valid value' do
         entry = factory
           .value(['ls', 'pwd'])
+          .parent(parent)
           .create!
 
         expect(entry.value).to eq ['ls', 'pwd']
       end
 
+      it 'sets parent and global attributes' do
+        entry = factory
+          .value('ls')
+          .parent(parent)
+          .create!
+
+        expect(entry.global).to eq global
+        expect(entry.parent).to eq parent
+      end
+
       context 'when setting description' do
         it 'creates entry with description' do
           entry = factory
             .value(['ls', 'pwd'])
+            .parent(parent)
             .with(description: 'test description')
             .create!
 
@@ -30,6 +48,7 @@ describe Gitlab::Ci::Config::Node::Factory do
         it 'creates entry with custom key' do
           entry = factory
             .value(['ls', 'pwd'])
+            .parent(parent)
             .with(key: 'test key')
             .create!
 
@@ -38,20 +57,21 @@ describe Gitlab::Ci::Config::Node::Factory do
       end
 
       context 'when setting a parent' do
-        let(:parent) { Object.new }
+        let(:object) { Object.new }
 
         it 'creates entry with valid parent' do
           entry = factory
             .value('ls')
-            .with(parent: parent)
+            .parent(parent)
+            .with(parent: object)
             .create!
 
-          expect(entry.parent).to eq parent
+          expect(entry.parent).to eq object
         end
       end
     end
 
-    context 'when not setting up a value' do
+    context 'when not setting a value' do
       it 'raises error' do
         expect { factory.create! }.to raise_error(
           Gitlab::Ci::Config::Node::Factory::InvalidFactory
@@ -59,10 +79,19 @@ describe Gitlab::Ci::Config::Node::Factory do
       end
     end
 
+    context 'when not setting parent object' do
+      it 'raises error' do
+        expect { factory.value('ls').create! }.to raise_error(
+          Gitlab::Ci::Config::Node::Factory::InvalidFactory
+        )
+      end
+    end
+
     context 'when creating entry with nil value' do
       it 'creates an undefined entry' do
         entry = factory
           .value(nil)
+          .parent(parent)
           .create!
 
         expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined

From 8c9c3eda7a305c43d7cf3d50b868292d0b612b5b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 12 Jul 2016 12:56:21 +0200
Subject: [PATCH 35/62] Prevalidate CI entries recursively on processed

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

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 1940c39087b..02e4f7693da 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -31,10 +31,9 @@ module Gitlab
           end
 
           def validate!
-            if @validator.valid?(:new)
-              @validator.validate(:processed)
-            end
+            return unless valid?
 
+            @validator.validate(:processed)
             @entries.each_value(&:validate!)
           end
 
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index a80b0ce5a57..1017f79cc6e 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1065,7 +1065,7 @@ EOT
       end
 
       it "returns errors if there are no visible jobs defined" do
-        config = YAML.dump({ before_script: ["bundle update"], '.hidden'.to_sym => {} })
+        config = YAML.dump({ before_script: ["bundle update"], '.hidden'.to_sym => { script: 'ls' } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs config should contain at least one visible job")

From d41d3301474ffd7022e41daad4ddf67590ac9f95 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 12 Jul 2016 13:03:19 +0200
Subject: [PATCH 36/62] Add CI config node that is unspecified null entry

---
 lib/gitlab/ci/config/node/null.rb           | 30 +++++++++++++++++++++
 spec/lib/gitlab/ci/config/node/null_spec.rb | 29 ++++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 lib/gitlab/ci/config/node/null.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/null_spec.rb

diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
new file mode 100644
index 00000000000..c7bbc16939a
--- /dev/null
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -0,0 +1,30 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # This class represents an undefined and unspecified node.
+        #
+        # Implements the Null Object pattern.
+        #
+        class Null < Entry
+          def initialize(config = nil, **attributes)
+            super
+          end
+
+          def value
+            nil
+          end
+
+          def valid?
+            true
+          end
+
+          def errors
+            []
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb
new file mode 100644
index 00000000000..63b23ed0bd1
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/null_spec.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Null do
+  let(:null) { described_class.new }
+
+  describe '#leaf?' do
+    it 'is leaf node' do
+      expect(null).to be_leaf
+    end
+  end
+
+  describe '#valid?' do
+    it 'is always valid' do
+      expect(null).to be_valid
+    end
+  end
+
+  describe '#errors' do
+    it 'is does not contain errors' do
+      expect(null.errors).to be_empty
+    end
+  end
+
+  describe '#value' do
+    it 'returns nil' do
+      expect(null.value).to eq nil
+    end
+  end
+end

From 06641a3fee4ebdaada3007a51866a7fb927d21de Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 12 Jul 2016 14:10:18 +0200
Subject: [PATCH 37/62] Simplify undefined node definition in CI config

---
 lib/gitlab/ci/config/node/configurable.rb     |  2 +-
 lib/gitlab/ci/config/node/entry.rb            |  4 +-
 lib/gitlab/ci/config/node/factory.rb          | 12 +++-
 lib/gitlab/ci/config/node/null.rb             | 12 ++--
 lib/gitlab/ci/config/node/undefined.rb        | 59 ++-------------
 spec/lib/gitlab/ci/config/node/global_spec.rb |  4 +-
 spec/lib/gitlab/ci/config/node/null_spec.rb   | 14 +++-
 .../gitlab/ci/config/node/undefined_spec.rb   | 71 +++++--------------
 8 files changed, 60 insertions(+), 118 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 36d7d20c110..29de2d7d0b5 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -51,7 +51,7 @@ module Gitlab
             def helpers(*nodes)
               nodes.each do |symbol|
                 define_method("#{symbol}_defined?") do
-                  @entries[symbol].try(:defined?)
+                  @entries[symbol].specified?
                 end
 
                 define_method("#{symbol}_value") do
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 02e4f7693da..c95e85200d2 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -66,14 +66,14 @@ module Gitlab
               @config
             else
               meaningful = @entries.select do |_key, value|
-                value.defined? && value.relevant?
+                value.specified? && value.relevant?
               end
 
               Hash[meaningful.map { |key, entry| [key, entry.value] }]
             end
           end
 
-          def defined?
+          def specified?
             true
           end
 
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 3488aec4a22..602660acb93 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -40,11 +40,21 @@ module Gitlab
             # See issue #18775.
             #
             if @value.nil?
-              Node::Undefined.new(@node, attributes)
+              Node::Undefined.new(fabricate_undefined(attributes))
             else
               @node.new(@value, attributes)
             end
           end
+
+          private
+
+          def fabricate_undefined(attributes)
+            if @node.default.nil?
+              Node::Null.new(nil, attributes)
+            else
+              @node.new(@node.default, attributes)
+            end
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index c7bbc16939a..880d29f663d 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -8,10 +8,6 @@ module Gitlab
         # Implements the Null Object pattern.
         #
         class Null < Entry
-          def initialize(config = nil, **attributes)
-            super
-          end
-
           def value
             nil
           end
@@ -23,6 +19,14 @@ module Gitlab
           def errors
             []
           end
+
+          def specified?
+            false
+          end
+
+          def relevant?
+            false
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index fedb9d020be..384774c9b69 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -3,66 +3,19 @@ module Gitlab
     class Config
       module Node
         ##
-        # This class represents an undefined entry node.
+        # This class represents an undefined and unspecified entry node.
         #
-        # It takes original entry class as configuration and creates an object
-        # if original entry has a default value. If there is default value
-        # some methods are delegated to it.
+        # It decorates original entry adding method that idicates it is
+        # unspecified.
         #
-        #
-        class Undefined < Entry
-          include Validatable
-
-          delegate :valid?, :errors, :value, to: :@strategy
-
-          validations do
-            validates :config, type: Class
-          end
-
-          def initialize(node, **attributes)
+        class Undefined < SimpleDelegator
+          def initialize(entry)
             super
-            @strategy = create_strategy(node, node.default)
           end
 
-          def defined?
+          def specified?
             false
           end
-
-          private
-
-          def create_strategy(node, default)
-            if default.nil?
-              Undefined::NullStrategy.new
-            else
-              entry = node.new(default, attributes)
-              Undefined::DefaultStrategy.new(entry)
-            end
-          end
-
-          class DefaultStrategy
-            delegate :valid?, :errors, :value, to: :@default
-
-            def initialize(entry)
-              @default = entry
-            end
-          end
-
-          class NullStrategy
-            def initialize(*)
-            end
-
-            def value
-              nil
-            end
-
-            def valid?
-              true
-            end
-
-            def errors
-              []
-            end
-          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 786fc5bb6ce..1945b0326cc 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -233,9 +233,9 @@ describe Gitlab::Ci::Config::Node::Global do
     end
   end
 
-  describe '#defined?' do
+  describe '#specified?' do
     it 'is concrete entry that is defined' do
-      expect(global.defined?).to be true
+      expect(global.specified?).to be true
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb
index 63b23ed0bd1..1ab5478dcfa 100644
--- a/spec/lib/gitlab/ci/config/node/null_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/null_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Null do
-  let(:null) { described_class.new }
+  let(:null) { described_class.new(nil) }
 
   describe '#leaf?' do
     it 'is leaf node' do
@@ -26,4 +26,16 @@ describe Gitlab::Ci::Config::Node::Null do
       expect(null.value).to eq nil
     end
   end
+
+  describe '#relevant?' do
+    it 'is not relevant' do
+      expect(null.relevant?).to eq false
+    end
+  end
+
+  describe '#specified?' do
+    it 'is not defined' do
+      expect(null.specified?).to eq false
+    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 417b4a0ad6f..2d43e1c1a9d 100644
--- a/spec/lib/gitlab/ci/config/node/undefined_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
@@ -4,66 +4,29 @@ describe Gitlab::Ci::Config::Node::Undefined do
   let(:undefined) { described_class.new(entry) }
   let(:entry) { spy('Entry') }
 
-  context 'when entry does not have a default value' do
-    before { allow(entry).to receive(:default).and_return(nil) }
-
-    describe '#leaf?' do
-      it 'is leaf node' do
-        expect(undefined).to be_leaf
-      end
-    end
-
-    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 nil' do
-        expect(undefined.value).to eq nil
-      end
+  describe '#valid?' do
+    it 'delegates method to entry' do
+      expect(undefined.valid).to eq entry
     end
   end
 
-  context 'when entry has a default value' do
-    before do
-      allow(entry).to receive(:default).and_return('some value')
-      allow(entry).to receive(:value).and_return('some value')
-    end
-
-    describe '#value' do
-      it 'returns default value for entry' do
-        expect(undefined.value).to eq 'some value'
-      end
-    end
-
-    describe '#errors' do
-      it 'delegates errors to default entry' do
-        expect(entry).to receive(:errors)
-
-        undefined.errors
-      end
-    end
-
-    describe '#valid?' do
-      it 'delegates valid? to default entry' do
-        expect(entry).to receive(:valid?)
-
-        undefined.valid?
-      end
+  describe '#errors' do
+    it 'delegates method to entry' do
+      expect(undefined.errors).to eq entry
     end
   end
 
-  describe '#undefined?' do
-    it 'is not a defined entry' do
-      expect(undefined.defined?).to be false
+  describe '#value' do
+    it 'delegates method to entry' do
+      expect(undefined.value).to eq entry
+    end
+  end
+
+  describe '#specified?' do
+    it 'is always false' do
+      allow(entry).to receive(:specified?).and_return(true)
+
+      expect(undefined.specified?).to be false
     end
   end
 end

From 61f7bede79a006c7b44e88a3385d175c5ad2a863 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 12 Jul 2016 14:40:51 +0200
Subject: [PATCH 38/62] Fix using `try` on delegators in CI config entries

See:

https://github.com/rails/rails/commit/af53280a4b5b3323ac87dc60deb2b1b781197b2b
---
 lib/gitlab/ci/config/node/configurable.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 29de2d7d0b5..da2ef4d5503 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -51,12 +51,12 @@ module Gitlab
             def helpers(*nodes)
               nodes.each do |symbol|
                 define_method("#{symbol}_defined?") do
-                  @entries[symbol].specified?
+                  @entries[symbol].specified? if @entries[symbol]
                 end
 
                 define_method("#{symbol}_value") do
                   raise Entry::InvalidError unless valid?
-                  @entries[symbol].try(:value)
+                  @entries[symbol].value if @entries[symbol]
                 end
 
                 alias_method symbol.to_sym, "#{symbol}_value".to_sym

From b228787f5afa34b153e6b52d6b0d88248cc3e099 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 12 Jul 2016 14:58:48 +0200
Subject: [PATCH 39/62] Do not raise when getting value of invalid CI node

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

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index da2ef4d5503..8bd752b0e2a 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -55,7 +55,7 @@ module Gitlab
                 end
 
                 define_method("#{symbol}_value") do
-                  raise Entry::InvalidError unless valid?
+                  return unless valid?
                   @entries[symbol].value if @entries[symbol]
                 end
 
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 1945b0326cc..3ffbe9c2e97 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -209,10 +209,8 @@ describe Gitlab::Ci::Config::Node::Global do
     end
 
     describe '#before_script' do
-      it 'raises error' do
-        expect { global.before_script }.to raise_error(
-          Gitlab::Ci::Config::Node::Entry::InvalidError
-        )
+      it 'returns nil' do
+        expect(global.before_script).to be_nil
       end
     end
   end

From de4c9a273867864a9033dab0624e0cfd72201384 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 13 Jul 2016 12:22:33 +0200
Subject: [PATCH 40/62] Improve CI stage configuration entry validations

---
 lib/gitlab/ci/config/node/stage.rb           | 16 +++++++++-------
 lib/gitlab/ci/config/node/validators.rb      |  2 +-
 spec/lib/gitlab/ci/config/node/stage_spec.rb |  4 ++--
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/lib/gitlab/ci/config/node/stage.rb b/lib/gitlab/ci/config/node/stage.rb
index e8fae65a2a9..909358ea170 100644
--- a/lib/gitlab/ci/config/node/stage.rb
+++ b/lib/gitlab/ci/config/node/stage.rb
@@ -10,14 +10,16 @@ module Gitlab
 
           validations do
             validates :config, type: String
-            validates :global, required_attribute: true
-            validate :known_stage, on: :processed
 
-            def known_stage
-              unless known?
-                stages_list = global.stages.join(', ')
-                errors.add(:config,
-                           "should be one of defined stages (#{stages_list})")
+            with_options on: :processed do
+              validates :global, required: true
+
+              validate do
+                unless known?
+                  errors.add(:config,
+                             'should be one of defined stages ' \
+                             "(#{global.stages.join(', ')})")
+                end
               end
             end
           end
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index 6f0e14e2f0a..d33b407af68 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -33,7 +33,7 @@ module Gitlab
             end
           end
 
-          class RequiredAttributeValidator < ActiveModel::EachValidator
+          class RequiredValidator < ActiveModel::EachValidator
             def validate_each(record, attribute, value)
               if value.nil?
                 raise Entry::InvalidError,
diff --git a/spec/lib/gitlab/ci/config/node/stage_spec.rb b/spec/lib/gitlab/ci/config/node/stage_spec.rb
index 6deeca1a6c0..004012f8b38 100644
--- a/spec/lib/gitlab/ci/config/node/stage_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/stage_spec.rb
@@ -28,10 +28,10 @@ describe Gitlab::Ci::Config::Node::Stage do
     context 'when stage config is incorrect' do
       describe '#errors' do
         context 'when reference to global node is not set' do
-          let(:stage) { described_class.new(config) }
+          let(:stage) { described_class.new('test') }
 
           it 'raises error' do
-            expect { stage }.to raise_error(
+            expect { stage.validate! }.to raise_error(
               Gitlab::Ci::Config::Node::Entry::InvalidError,
               /Entry needs global attribute set internally./
             )

From 097550f08a2a92dd1efff5da97cff0228afde57b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 13 Jul 2016 13:42:57 +0200
Subject: [PATCH 41/62] Fabricate CI entry with value, set attributes later

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

diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 602660acb93..339548e0feb 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -32,27 +32,39 @@ module Gitlab
             raise InvalidFactory unless defined?(@value)
             raise InvalidFactory unless defined?(@parent)
 
-            attributes = { parent: @parent, global: @parent.global }
-            attributes.merge!(@attributes)
-
             ##
             # We assume that unspecified entry is undefined.
             # See issue #18775.
             #
             if @value.nil?
-              Node::Undefined.new(fabricate_undefined(attributes))
+              Node::Undefined.new(
+                fabricate_undefined
+              )
             else
-              @node.new(@value, attributes)
+              fabricate(@node, @value)
             end
           end
 
           private
 
-          def fabricate_undefined(attributes)
+          def fabricate_undefined
+            ##
+            # If node has a default value we fabricate concrete node
+            # with default value.
+            #
             if @node.default.nil?
-              Node::Null.new(nil, attributes)
+              fabricate(Node::Null)
             else
-              @node.new(@node.default, attributes)
+              fabricate(@node, @node.default)
+            end
+          end
+
+          def fabricate(node, value = nil)
+            node.new(value).tap do |entry|
+              entry.key = @attributes[:key]
+              entry.parent = @attributes[:parent] || @parent
+              entry.global = @attributes[:global] || @parent.global
+              entry.description = @attributes[:description]
             end
           end
         end

From 6920390aad683dcc73109be5a23b647c918f9309 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 13 Jul 2016 14:38:10 +0200
Subject: [PATCH 42/62] Add before script and commands to CI job entry

---
 lib/gitlab/ci/config/node/job.rb           |  15 ++-
 spec/lib/gitlab/ci/config/node/job_spec.rb | 117 +++++++++++++++++++--
 2 files changed, 124 insertions(+), 8 deletions(-)

diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 9a019216ca3..5ee91ebcf0b 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -10,6 +10,7 @@ module Gitlab
 
           validations do
             validates :config, presence: true
+            validates :global, required: true, on: :processed
           end
 
           node :before_script, Script,
@@ -30,8 +31,6 @@ module Gitlab
           helpers :before_script, :script, :stage, :type, :after_script
 
           def value
-            raise InvalidError unless valid?
-
             ##
             # TODO, refactoring step: do not expose internal configuration,
             # return only hash value without merging it to internal config.
@@ -39,6 +38,18 @@ module Gitlab
             @config.merge(to_hash.compact)
           end
 
+          def before_script
+            if before_script_defined?
+              before_script_value.to_a
+            else
+              @global.before_script.to_a
+            end
+          end
+
+          def commands
+            (before_script + script).join("\n")
+          end
+
           private
 
           def to_hash
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 77efc73632d..635362611a0 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -60,14 +60,62 @@ describe Gitlab::Ci::Config::Node::Job do
                  after_script: %w[cleanup])
       end
     end
+  end
 
-    context 'when entry is incorrect' do
-      let(:config) { {} }
+  describe '#before_script' do
+    context 'when global entry has before script' do
+      before do
+        allow(global).to receive(:before_script)
+          .and_return(%w[ls pwd])
+      end
 
-      it 'raises error' do
-        expect { entry.value }.to raise_error(
-          Gitlab::Ci::Config::Node::Entry::InvalidError
-        )
+      context 'when before script is overridden' do
+        let(:config) do
+          { before_script: %w[whoami],
+            script: 'rspec' }
+        end
+
+        it 'returns correct script' do
+          expect(entry.before_script).to eq %w[whoami]
+        end
+      end
+
+      context 'when before script is not overriden' do
+        let(:config) do
+          { script: %w[spinach] }
+        end
+
+        it 'returns correct script' do
+          expect(entry.before_script).to eq %w[ls pwd]
+        end
+      end
+    end
+
+    context 'when global entry does not have before script' do
+      before do
+        allow(global).to receive(:before_script)
+          .and_return(nil)
+      end
+
+      context 'when job has before script' do
+        let(:config) do
+          { before_script: %w[whoami],
+            script: 'rspec' }
+        end
+
+        it 'returns correct script' do
+          expect(entry.before_script).to eq %w[whoami]
+        end
+      end
+
+      context 'when job does not have before script' do
+        let(:config) do
+          { script: %w[ls test] }
+        end
+
+        it 'returns correct script' do
+          expect(entry.before_script).to eq []
+        end
       end
     end
   end
@@ -77,4 +125,61 @@ describe Gitlab::Ci::Config::Node::Job do
       expect(entry).to be_relevant
     end
   end
+
+  describe '#commands' do
+    context 'when global entry has before script' do
+      before do
+        allow(global).to receive(:before_script)
+          .and_return(%w[ls pwd])
+      end
+
+      context 'when before script is overridden' do
+        let(:config) do
+          { before_script: %w[whoami],
+            script: 'rspec' }
+        end
+
+        it 'returns correct commands' do
+          expect(entry.commands).to eq "whoami\nrspec"
+        end
+      end
+
+      context 'when before script is not overriden' do
+        let(:config) do
+          { script: %w[rspec spinach] }
+        end
+
+        it 'returns correct commands' do
+          expect(entry.commands).to eq "ls\npwd\nrspec\nspinach"
+        end
+      end
+    end
+
+    context 'when global entry does not have before script' do
+      before do
+        allow(global).to receive(:before_script)
+          .and_return(nil)
+      end
+      context 'when job has before script' do
+        let(:config) do
+          { before_script: %w[whoami],
+            script: 'rspec' }
+        end
+
+        it 'returns correct commands' do
+          expect(entry.commands).to eq "whoami\nrspec"
+        end
+      end
+
+      context 'when job does not have before script' do
+        let(:config) do
+          { script: %w[ls test] }
+        end
+
+        it 'returns correct commands' do
+          expect(entry.commands).to eq "ls\ntest"
+        end
+      end
+    end
+  end
 end

From 036e297ca3c39f90aebc76d5acb2e01f32364d0d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 13 Jul 2016 15:04:12 +0200
Subject: [PATCH 43/62] Expose CI job commands and use in legacy processor

---
 lib/ci/gitlab_ci_yaml_processor.rb            | 13 ++++++-------
 lib/gitlab/ci/config/node/job.rb              | 15 ++++++++-------
 spec/lib/gitlab/ci/config/node/global_spec.rb | 12 +++++++++---
 spec/lib/gitlab/ci/config/node/job_spec.rb    |  3 ++-
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   | 11 ++++++++---
 5 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index ed8dd0f9e47..61075d3b923 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -80,12 +80,7 @@ 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"),
+        commands: job[:commands],
         tag_list: job[:tags] || [],
         name: name,
         only: job[:only],
@@ -124,8 +119,12 @@ module Ci
     end
 
     def validate_job_keys!(name, job)
+      ##
+      # TODO, remove refactoring keys
+      #
+      refactoring_keys = [:commands]
       job.keys.each do |key|
-        unless ALLOWED_JOB_KEYS.include? key
+        unless (ALLOWED_JOB_KEYS + refactoring_keys).include? key
           raise ValidationError, "#{name} job: unknown parameter #{key}"
         end
       end
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 5ee91ebcf0b..bb1c3386bd4 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -40,23 +40,24 @@ module Gitlab
 
           def before_script
             if before_script_defined?
-              before_script_value.to_a
+              before_script_value
             else
-              @global.before_script.to_a
+              @global.before_script
             end
           end
 
           def commands
-            (before_script + script).join("\n")
+            [before_script, script].compact.join("\n")
           end
 
           private
 
           def to_hash
-            { before_script: before_script_value,
-              script: script_value,
-              stage: stage_value,
-              after_script: after_script_value }
+            { before_script: before_script,
+              script: script,
+              commands: commands,
+              stage: stage,
+              after_script: after_script }
           end
 
           def compose!
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 3ffbe9c2e97..f46359f7ee6 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -23,7 +23,7 @@ describe Gitlab::Ci::Config::Node::Global do
           after_script: ['make clean'],
           stages: ['build', 'pages'],
           cache: { key: 'k', untracked: true, paths: ['public/'] },
-          rspec: { script: 'rspec' },
+          rspec: { script: %w[rspec ls] },
           spinach: { script: 'spinach' } }
       end
 
@@ -129,8 +129,14 @@ describe Gitlab::Ci::Config::Node::Global do
         describe '#jobs' do
           it 'returns jobs configuration' do
             expect(global.jobs)
-              .to eq(rspec: { script: %w[rspec], stage: 'test' },
-                     spinach: { script: %w[spinach], stage: 'test' })
+              .to eq(rspec: { before_script: %w[ls pwd],
+                              script: %w[rspec ls],
+                              commands: "ls\npwd\nrspec\nls",
+                              stage: 'test' },
+                     spinach: { before_script: %w[ls pwd],
+                                script: %w[spinach],
+                                commands: "ls\npwd\nspinach",
+                                stage: 'test' })
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 635362611a0..816c0f275d6 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -56,6 +56,7 @@ describe Gitlab::Ci::Config::Node::Job do
         expect(entry.value)
           .to eq(before_script: %w[ls pwd],
                  script: %w[rspec],
+                 commands: "ls\npwd\nrspec",
                  stage: 'test',
                  after_script: %w[cleanup])
       end
@@ -114,7 +115,7 @@ describe Gitlab::Ci::Config::Node::Job do
         end
 
         it 'returns correct script' do
-          expect(entry.before_script).to eq []
+          expect(entry.before_script).to be_nil
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 255646a001a..60ab1d2150d 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Jobs do
-  let(:entry) { described_class.new(config, global: spy) }
+  let(:entry) { described_class.new(config, global: global) }
+  let(:global) { double('global', before_script: nil, stages: %w[test]) }
 
   describe 'validations' do
     before do
@@ -62,8 +63,12 @@ describe Gitlab::Ci::Config::Node::Jobs do
     describe '#value' do
       it 'returns key value' do
         expect(entry.value)
-          .to eq(rspec: { script: %w[rspec], stage: 'test' },
-                 spinach: { script: %w[spinach], stage: 'test' })
+          .to eq(rspec: { script: %w[rspec],
+                          commands: 'rspec',
+                          stage: 'test' },
+                 spinach: { script: %w[spinach],
+                            commands: 'spinach',
+                            stage: 'test' })
       end
     end
 

From 56ae9f6ba933939ced7c8e0eea5abbb34a0a68be Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 14 Jul 2016 13:14:09 +0200
Subject: [PATCH 44/62] Improve CI job entry validations in new config

---
 lib/ci/gitlab_ci_yaml_processor.rb            |  7 -------
 lib/gitlab/ci/config/node/configurable.rb     |  6 ++++--
 lib/gitlab/ci/config/node/job.rb              | 20 +++++++++++--------
 lib/gitlab/ci/config/node/jobs.rb             | 11 +++++-----
 lib/gitlab/ci/config/node/validator.rb        | 11 ++++++++--
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  |  4 ++--
 spec/lib/gitlab/ci/config/node/global_spec.rb | 14 ++++++-------
 spec/lib/gitlab/ci/config/node/job_spec.rb    | 16 ++++++++++++---
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   |  4 ++--
 9 files changed, 54 insertions(+), 39 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 61075d3b923..e18d5b907b4 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -102,7 +102,6 @@ module Ci
     def validate_job!(name, job)
       raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
 
-      validate_job_name!(name)
       validate_job_keys!(name, job)
       validate_job_types!(name, job)
 
@@ -112,12 +111,6 @@ module Ci
       validate_job_dependencies!(name, job) if job[:dependencies]
     end
 
-    def validate_job_name!(name)
-      if name.blank? || !validate_string(name)
-        raise ValidationError, "job name should be non-empty string"
-      end
-    end
-
     def validate_job_keys!(name, job)
       ##
       # TODO, remove refactoring keys
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 8bd752b0e2a..b33d743e2c3 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -55,8 +55,10 @@ module Gitlab
                 end
 
                 define_method("#{symbol}_value") do
-                  return unless valid?
-                  @entries[symbol].value if @entries[symbol]
+                  if @entries[symbol]
+                    return unless @entries[symbol].valid?
+                    @entries[symbol].value
+                  end
                 end
 
                 alias_method symbol.to_sym, "#{symbol}_value".to_sym
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index bb1c3386bd4..822b428926a 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -10,7 +10,12 @@ module Gitlab
 
           validations do
             validates :config, presence: true
-            validates :global, required: true, on: :processed
+
+            with_options on: :processed do
+              validates :global, required: true
+              validates :name, presence: true
+              validates :name, type: Symbol
+            end
           end
 
           node :before_script, Script,
@@ -30,11 +35,11 @@ module Gitlab
 
           helpers :before_script, :script, :stage, :type, :after_script
 
+          def name
+            @key
+          end
+
           def value
-            ##
-            # TODO, refactoring step: do not expose internal configuration,
-            # return only hash value without merging it to internal config.
-            #
             @config.merge(to_hash.compact)
           end
 
@@ -53,10 +58,9 @@ module Gitlab
           private
 
           def to_hash
-            { before_script: before_script,
-              script: script,
-              commands: commands,
+            { script: script,
               stage: stage,
+              commands: commands,
               after_script: after_script }
           end
 
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 548441df37c..3c1851b9fea 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -10,11 +10,12 @@ module Gitlab
 
           validations do
             validates :config, type: Hash
-            validate :jobs_presence, on: :processed
 
-            def jobs_presence
-              unless relevant?
-                errors.add(:config, 'should contain at least one visible job')
+            with_options on: :processed do
+              validate do
+                unless has_visible_job?
+                  errors.add(:config, 'should contain at least one visible job')
+                end
               end
             end
           end
@@ -23,7 +24,7 @@ module Gitlab
             @config
           end
 
-          def relevant?
+          def has_visible_job?
             @entries.values.any?(&:relevant?)
           end
 
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index dcfeb194374..ca000f245aa 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -31,8 +31,15 @@ module Gitlab
 
           def location
             predecessors = ancestors.map(&:key).compact
-            current = key || @node.class.name.demodulize.underscore.humanize
-            predecessors.append(current).join(':')
+            predecessors.append(key_name).join(':')
+          end
+
+          def key_name
+            if key.blank? || key.nil?
+              @node.class.name.demodulize.underscore.humanize
+            else
+              key
+            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 1017f79cc6e..e88f5cfc6dd 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -998,14 +998,14 @@ EOT
         config = YAML.dump({ '' => { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:job name can't be blank")
       end
 
       it "returns errors if job name is non-string" do
         config = YAML.dump({ 10 => { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:10 name should be a symbol")
       end
 
       it "returns errors if job image parameter is invalid" do
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index f46359f7ee6..fa5ff016995 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -129,14 +129,12 @@ describe Gitlab::Ci::Config::Node::Global do
         describe '#jobs' do
           it 'returns jobs configuration' do
             expect(global.jobs)
-              .to eq(rspec: { before_script: %w[ls pwd],
-                              script: %w[rspec ls],
-                              commands: "ls\npwd\nrspec\nls",
-                              stage: 'test' },
-                     spinach: { before_script: %w[ls pwd],
-                                script: %w[spinach],
-                                commands: "ls\npwd\nspinach",
-                                stage: 'test' })
+              .to eq(rspec: { script: %w[rspec ls],
+                              stage: 'test',
+                              commands: "ls\npwd\nrspec\nls" },
+                     spinach: { script: %w[spinach],
+                                stage: 'test',
+                                commands: "ls\npwd\nspinach" })
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 816c0f275d6..2ac7509cb6d 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -1,8 +1,9 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Job do
-  let(:entry) { described_class.new(config, global: global) }
-  let(:global) { spy('Global') }
+  let(:entry) { described_class.new(config, attributes) }
+  let(:attributes) { { key: :rspec, global: global } }
+  let(:global) { double('global', stages: %w[test]) }
 
   before do
     entry.process!
@@ -18,6 +19,15 @@ describe Gitlab::Ci::Config::Node::Job do
           expect(entry).to be_valid
         end
       end
+
+      context 'when job name is empty' do
+        let(:attributes) { { key: '', global: global } }
+
+        it 'reports error' do
+          expect(entry.errors)
+            .to include "job name can't be blank"
+        end
+      end
     end
 
     context 'when entry value is not correct' do
@@ -27,7 +37,7 @@ describe Gitlab::Ci::Config::Node::Job do
         describe '#errors' do
           it 'reports error about a config type' do
             expect(entry.errors)
-              .to include 'job config should be a hash'
+              .to include 'rspec config should be a hash'
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 60ab1d2150d..fe7ae61ed81 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -34,8 +34,8 @@ describe Gitlab::Ci::Config::Node::Jobs do
         context 'when job is unspecified' do
           let(:config) { { rspec: nil } }
 
-          it 'is not valid' do
-            expect(entry).not_to be_valid
+          it 'reports error' do
+            expect(entry.errors).to include "rspec config can't be blank"
           end
         end
 

From f7c80e9f31944c0001c9bef23d1a8efe33e4adce Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 14 Jul 2016 15:05:46 +0200
Subject: [PATCH 45/62] Revert references to global node in CI job entry

---
 lib/ci/gitlab_ci_yaml_processor.rb            |   8 +-
 lib/gitlab/ci/config/node/job.rb              |  20 +--
 spec/lib/gitlab/ci/config/node/global_spec.rb |   6 +-
 spec/lib/gitlab/ci/config/node/job_spec.rb    | 116 ------------------
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   |   2 -
 5 files changed, 8 insertions(+), 144 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index e18d5b907b4..144f9cd7b74 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -80,7 +80,7 @@ module Ci
       {
         stage_idx: @stages.index(job[:stage]),
         stage: job[:stage],
-        commands: job[:commands],
+        commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
         name: name,
         only: job[:only],
@@ -112,12 +112,8 @@ module Ci
     end
 
     def validate_job_keys!(name, job)
-      ##
-      # TODO, remove refactoring keys
-      #
-      refactoring_keys = [:commands]
       job.keys.each do |key|
-        unless (ALLOWED_JOB_KEYS + refactoring_keys).include? key
+        unless ALLOWED_JOB_KEYS.include? key
           raise ValidationError, "#{name} job: unknown parameter #{key}"
         end
       end
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 822b428926a..f01a46a8ddc 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -43,25 +43,13 @@ module Gitlab
             @config.merge(to_hash.compact)
           end
 
-          def before_script
-            if before_script_defined?
-              before_script_value
-            else
-              @global.before_script
-            end
-          end
-
-          def commands
-            [before_script, script].compact.join("\n")
-          end
-
           private
 
           def to_hash
-            { script: script,
-              stage: stage,
-              commands: commands,
-              after_script: after_script }
+            { before_script: before_script_value,
+              script: script_value,
+              stage: stage_value,
+              after_script: after_script_value }
           end
 
           def compose!
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index fa5ff016995..dfcaebe35be 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -130,11 +130,9 @@ describe Gitlab::Ci::Config::Node::Global do
           it 'returns jobs configuration' do
             expect(global.jobs)
               .to eq(rspec: { script: %w[rspec ls],
-                              stage: 'test',
-                              commands: "ls\npwd\nrspec\nls" },
+                              stage: 'test'  },
                      spinach: { script: %w[spinach],
-                                stage: 'test',
-                                commands: "ls\npwd\nspinach" })
+                                stage: 'test' })
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 2ac7509cb6d..ee3eea9c27a 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -66,131 +66,15 @@ describe Gitlab::Ci::Config::Node::Job do
         expect(entry.value)
           .to eq(before_script: %w[ls pwd],
                  script: %w[rspec],
-                 commands: "ls\npwd\nrspec",
                  stage: 'test',
                  after_script: %w[cleanup])
       end
     end
   end
 
-  describe '#before_script' do
-    context 'when global entry has before script' do
-      before do
-        allow(global).to receive(:before_script)
-          .and_return(%w[ls pwd])
-      end
-
-      context 'when before script is overridden' do
-        let(:config) do
-          { before_script: %w[whoami],
-            script: 'rspec' }
-        end
-
-        it 'returns correct script' do
-          expect(entry.before_script).to eq %w[whoami]
-        end
-      end
-
-      context 'when before script is not overriden' do
-        let(:config) do
-          { script: %w[spinach] }
-        end
-
-        it 'returns correct script' do
-          expect(entry.before_script).to eq %w[ls pwd]
-        end
-      end
-    end
-
-    context 'when global entry does not have before script' do
-      before do
-        allow(global).to receive(:before_script)
-          .and_return(nil)
-      end
-
-      context 'when job has before script' do
-        let(:config) do
-          { before_script: %w[whoami],
-            script: 'rspec' }
-        end
-
-        it 'returns correct script' do
-          expect(entry.before_script).to eq %w[whoami]
-        end
-      end
-
-      context 'when job does not have before script' do
-        let(:config) do
-          { script: %w[ls test] }
-        end
-
-        it 'returns correct script' do
-          expect(entry.before_script).to be_nil
-        end
-      end
-    end
-  end
-
   describe '#relevant?' do
     it 'is a relevant entry' do
       expect(entry).to be_relevant
     end
   end
-
-  describe '#commands' do
-    context 'when global entry has before script' do
-      before do
-        allow(global).to receive(:before_script)
-          .and_return(%w[ls pwd])
-      end
-
-      context 'when before script is overridden' do
-        let(:config) do
-          { before_script: %w[whoami],
-            script: 'rspec' }
-        end
-
-        it 'returns correct commands' do
-          expect(entry.commands).to eq "whoami\nrspec"
-        end
-      end
-
-      context 'when before script is not overriden' do
-        let(:config) do
-          { script: %w[rspec spinach] }
-        end
-
-        it 'returns correct commands' do
-          expect(entry.commands).to eq "ls\npwd\nrspec\nspinach"
-        end
-      end
-    end
-
-    context 'when global entry does not have before script' do
-      before do
-        allow(global).to receive(:before_script)
-          .and_return(nil)
-      end
-      context 'when job has before script' do
-        let(:config) do
-          { before_script: %w[whoami],
-            script: 'rspec' }
-        end
-
-        it 'returns correct commands' do
-          expect(entry.commands).to eq "whoami\nrspec"
-        end
-      end
-
-      context 'when job does not have before script' do
-        let(:config) do
-          { script: %w[ls test] }
-        end
-
-        it 'returns correct commands' do
-          expect(entry.commands).to eq "ls\ntest"
-        end
-      end
-    end
-  end
 end
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index fe7ae61ed81..40837b5f857 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -64,10 +64,8 @@ describe Gitlab::Ci::Config::Node::Jobs do
       it 'returns key value' do
         expect(entry.value)
           .to eq(rspec: { script: %w[rspec],
-                          commands: 'rspec',
                           stage: 'test' },
                  spinach: { script: %w[spinach],
-                            commands: 'spinach',
                             stage: 'test' })
       end
     end

From 3e16b015b969a4d5d28240e76bffd382b0772f49 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 14 Jul 2016 15:23:52 +0200
Subject: [PATCH 46/62] Revert logical validation in CI job stage entry

---
 lib/ci/gitlab_ci_yaml_processor.rb           |  7 ++
 lib/gitlab/ci/config/node/job.rb             |  1 -
 lib/gitlab/ci/config/node/stage.rb           | 16 -----
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb |  4 +-
 spec/lib/gitlab/ci/config/node/stage_spec.rb | 71 ++------------------
 5 files changed, 15 insertions(+), 84 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 144f9cd7b74..0217a905eac 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -105,6 +105,7 @@ module Ci
       validate_job_keys!(name, job)
       validate_job_types!(name, job)
 
+      validate_job_stage!(name, job) if job[:stage]
       validate_job_variables!(name, job) if job[:variables]
       validate_job_cache!(name, job) if job[:cache]
       validate_job_artifacts!(name, job) if job[:artifacts]
@@ -153,6 +154,12 @@ module Ci
       end
     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(", ")}"
+      end
+    end
+
     def validate_job_variables!(name, job)
       unless validate_variables(job[:variables])
         raise ValidationError,
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index f01a46a8ddc..cca9791fc8e 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -12,7 +12,6 @@ module Gitlab
             validates :config, presence: true
 
             with_options on: :processed do
-              validates :global, required: true
               validates :name, presence: true
               validates :name, type: Symbol
             end
diff --git a/lib/gitlab/ci/config/node/stage.rb b/lib/gitlab/ci/config/node/stage.rb
index 909358ea170..cbc97641f5a 100644
--- a/lib/gitlab/ci/config/node/stage.rb
+++ b/lib/gitlab/ci/config/node/stage.rb
@@ -10,22 +10,6 @@ module Gitlab
 
           validations do
             validates :config, type: String
-
-            with_options on: :processed do
-              validates :global, required: true
-
-              validate do
-                unless known?
-                  errors.add(:config,
-                             'should be one of defined stages ' \
-                             "(#{global.stages.join(', ')})")
-                end
-              end
-            end
-          end
-
-          def known?
-            @global.stages.include?(@config)
           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 e88f5cfc6dd..daa02faf6fb 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1089,14 +1089,14 @@ EOT
         config = YAML.dump({ rspec: { script: "test", type: "acceptance" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:type config should be one of defined stages (build, test, deploy)")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy")
       end
 
       it "returns errors if job stage is not a defined stage" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", type: "acceptance" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:type config should be one of defined stages (build, test)")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test")
       end
 
       it "returns errors if stages is not an array" do
diff --git a/spec/lib/gitlab/ci/config/node/stage_spec.rb b/spec/lib/gitlab/ci/config/node/stage_spec.rb
index 004012f8b38..fb9ec70762a 100644
--- a/spec/lib/gitlab/ci/config/node/stage_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/stage_spec.rb
@@ -1,17 +1,12 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Stage do
-  let(:stage) { described_class.new(config, global: global) }
-  let(:global) { spy('Global') }
+  let(:stage) { described_class.new(config) }
 
   describe 'validations' do
     context 'when stage config value is correct' do
       let(:config) { 'build' }
 
-      before do
-        allow(global).to receive(:stages).and_return(%w[build])
-      end
-
       describe '#value' do
         it 'returns a stage key' do
           expect(stage.value).to eq config
@@ -25,66 +20,12 @@ describe Gitlab::Ci::Config::Node::Stage do
       end
     end
 
-    context 'when stage config is incorrect' do
-      describe '#errors' do
-        context 'when reference to global node is not set' do
-          let(:stage) { described_class.new('test') }
+    context 'when value has a wrong type' do
+      let(:config) { { test: true } }
 
-          it 'raises error' do
-            expect { stage.validate! }.to raise_error(
-              Gitlab::Ci::Config::Node::Entry::InvalidError,
-              /Entry needs global attribute set internally./
-            )
-          end
-        end
-
-        context 'when value has a wrong type' do
-          let(:config) { { test: true } }
-
-          it 'reports errors about wrong type' do
-            expect(stage.errors)
-              .to include 'stage config should be a string'
-          end
-        end
-
-        context 'when stage is not present in global configuration' do
-          let(:config) { 'unknown' }
-
-          before do
-            allow(global)
-              .to receive(:stages).and_return(%w[test deploy])
-          end
-
-          it 'reports error about missing stage' do
-            stage.validate!
-
-            expect(stage.errors)
-              .to include 'stage config should be one of ' \
-                          'defined stages (test, deploy)'
-          end
-        end
-      end
-    end
-  end
-
-  describe '#known?' do
-    before do
-      allow(global).to receive(:stages).and_return(%w[test deploy])
-    end
-
-    context 'when stage is not known' do
-      let(:config) { :unknown }
-
-      it 'returns false' do
-        expect(stage.known?).to be false
-      end
-    end
-
-    context 'when stage is known' do
-      let(:config) { 'test' }
-
-      it 'returns false' do
-        expect(stage.known?).to be true
+      it 'reports errors about wrong type' do
+        expect(stage.errors)
+          .to include 'stage config should be a string'
       end
     end
   end

From 5923741fe690a688591ad36da894b3103954a437 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 14 Jul 2016 15:45:29 +0200
Subject: [PATCH 47/62] Remove references to global entry in new CI config

---
 lib/gitlab/ci/config/node/entry.rb             | 2 +-
 lib/gitlab/ci/config/node/factory.rb           | 1 -
 spec/lib/gitlab/ci/config/node/factory_spec.rb | 8 +-------
 spec/lib/gitlab/ci/config/node/job_spec.rb     | 6 ++----
 spec/lib/gitlab/ci/config/node/jobs_spec.rb    | 3 +--
 5 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index c95e85200d2..9640103ea22 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -9,7 +9,7 @@ module Gitlab
           class InvalidError < StandardError; end
 
           attr_reader :config, :attributes
-          attr_accessor :key, :parent, :global, :description
+          attr_accessor :key, :parent, :description
 
           def initialize(config, **attributes)
             @config = config
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 339548e0feb..b509c5edf59 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -63,7 +63,6 @@ module Gitlab
             node.new(value).tap do |entry|
               entry.key = @attributes[:key]
               entry.parent = @attributes[:parent] || @parent
-              entry.global = @attributes[:global] || @parent.global
               entry.description = @attributes[:description]
             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 bc6bf32ffbf..4e6f2419e13 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -5,11 +5,6 @@ describe Gitlab::Ci::Config::Node::Factory do
     let(:factory) { described_class.new(node) }
     let(:node) { Gitlab::Ci::Config::Node::Script }
     let(:parent) { double('parent') }
-    let(:global) { double('global') }
-
-    before do
-      allow(parent).to receive(:global).and_return(global)
-    end
 
     context 'when setting a concrete value' do
       it 'creates entry with valid value' do
@@ -21,13 +16,12 @@ describe Gitlab::Ci::Config::Node::Factory do
         expect(entry.value).to eq ['ls', 'pwd']
       end
 
-      it 'sets parent and global attributes' do
+      it 'sets parent  attributes' do
         entry = factory
           .value('ls')
           .parent(parent)
           .create!
 
-        expect(entry.global).to eq global
         expect(entry.parent).to eq parent
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index ee3eea9c27a..4c7ac9949cc 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -1,9 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Job do
-  let(:entry) { described_class.new(config, attributes) }
-  let(:attributes) { { key: :rspec, global: global } }
-  let(:global) { double('global', stages: %w[test]) }
+  let(:entry) { described_class.new(config, key: :rspec) }
 
   before do
     entry.process!
@@ -21,7 +19,7 @@ describe Gitlab::Ci::Config::Node::Job do
       end
 
       context 'when job name is empty' do
-        let(:attributes) { { key: '', global: global } }
+        let(:entry) { described_class.new(config, key: ''.to_sym) }
 
         it 'reports error' do
           expect(entry.errors)
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 40837b5f857..c4c130abb6d 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -1,8 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Jobs do
-  let(:entry) { described_class.new(config, global: global) }
-  let(:global) { double('global', before_script: nil, stages: %w[test]) }
+  let(:entry) { described_class.new(config) }
 
   describe 'validations' do
     before do

From 615c9730e7783e82287d2b65f58da6336d3d2410 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 14 Jul 2016 16:01:18 +0200
Subject: [PATCH 48/62] Remove job cache configfrom legacy yaml processor

---
 lib/ci/gitlab_ci_yaml_processor.rb           | 21 --------------------
 lib/gitlab/ci/config/node/job.rb             |  6 +++++-
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb |  6 +++---
 3 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 0217a905eac..3e4767cc9f6 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -107,7 +107,6 @@ module Ci
 
       validate_job_stage!(name, job) if job[:stage]
       validate_job_variables!(name, job) if job[:variables]
-      validate_job_cache!(name, job) if job[:cache]
       validate_job_artifacts!(name, job) if job[:artifacts]
       validate_job_dependencies!(name, job) if job[:dependencies]
     end
@@ -167,26 +166,6 @@ module Ci
       end
     end
 
-    def validate_job_cache!(name, job)
-      job[:cache].keys.each do |key|
-        unless ALLOWED_CACHE_KEYS.include? key
-          raise ValidationError, "#{name} job: cache unknown parameter #{key}"
-        end
-      end
-
-      if job[:cache][:key] && !validate_string(job[:cache][:key])
-        raise ValidationError, "#{name} job: cache:key parameter should be a string"
-      end
-
-      if job[:cache][:untracked] && !validate_boolean(job[:cache][:untracked])
-        raise ValidationError, "#{name} job: cache:untracked parameter should be an boolean"
-      end
-
-      if job[:cache][:paths] && !validate_array_of_strings(job[:cache][:paths])
-        raise ValidationError, "#{name} job: cache:paths parameter should be an array of strings"
-      end
-    end
-
     def validate_job_artifacts!(name, job)
       job[:artifacts].keys.each do |key|
         unless ALLOWED_ARTIFACTS_KEYS.include? key
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index cca9791fc8e..483be2a21cc 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -32,7 +32,10 @@ module Gitlab
           node :after_script, Script,
             description: 'Commands that will be executed when finishing job.'
 
-          helpers :before_script, :script, :stage, :type, :after_script
+          node :cache, Cache,
+            description: 'Cache definition for this job.'
+
+          helpers :before_script, :script, :stage, :type, :after_script, :cache
 
           def name
             @key
@@ -48,6 +51,7 @@ module Gitlab
             { before_script: before_script_value,
               script: script_value,
               stage: stage_value,
+              cache: cache_value,
               after_script: after_script_value }
           end
 
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index daa02faf6fb..c9602bcca22 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1201,21 +1201,21 @@ EOT
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", cache: { key: 1 } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: cache:key parameter should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:cache:key config should be a string or symbol")
       end
 
       it "returns errors if job cache:untracked is not an array of strings" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", cache: { untracked: "string" } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: cache:untracked parameter should be an boolean")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:cache:untracked config should be a boolean value")
       end
 
       it "returns errors if job cache:paths is not an array of strings" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", cache: { paths: "string" } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: cache:paths parameter should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:cache:paths config should be an array of strings")
       end
 
       it "returns errors if job dependencies is not an array of strings" do

From 41bcbdd8c2412769a376cd37541ad6e65a1af1f2 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 15 Jul 2016 21:07:51 +0200
Subject: [PATCH 49/62] Add metadata to new CI config and expose job name

---
 lib/ci/gitlab_ci_yaml_processor.rb            |  4 +-
 lib/gitlab/ci/config/node/configurable.rb     |  3 +-
 lib/gitlab/ci/config/node/entry.rb            |  7 +---
 lib/gitlab/ci/config/node/factory.rb          | 10 ++---
 lib/gitlab/ci/config/node/global.rb           |  5 +--
 lib/gitlab/ci/config/node/job.rb              | 20 +++++-----
 lib/gitlab/ci/config/node/jobs.rb             |  9 +++--
 .../lib/gitlab/ci/config/node/factory_spec.rb | 37 +++++++------------
 spec/lib/gitlab/ci/config/node/global_spec.rb | 13 ++++---
 spec/lib/gitlab/ci/config/node/job_spec.rb    |  9 +++--
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   | 12 +++---
 11 files changed, 60 insertions(+), 69 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 3e4767cc9f6..0704e8f1683 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -82,7 +82,7 @@ module Ci
         stage: job[:stage],
         commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
-        name: name,
+        name: job[:name],
         only: job[:only],
         except: job[:except],
         allow_failure: job[:allow_failure] || false,
@@ -113,7 +113,7 @@ module Ci
 
     def validate_job_keys!(name, job)
       job.keys.each do |key|
-        unless ALLOWED_JOB_KEYS.include? key
+        unless (ALLOWED_JOB_KEYS + %i[name]).include? key
           raise ValidationError, "#{name} job: unknown parameter #{key}"
         end
       end
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index b33d743e2c3..10b2db86d24 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -28,8 +28,7 @@ module Gitlab
           def create(key, factory)
             factory
               .value(@config[key])
-              .parent(self)
-              .with(key: key)
+              .with(key: key, parent: self)
 
             factory.create!
           end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 9640103ea22..011c3be849e 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -11,13 +11,10 @@ module Gitlab
           attr_reader :config, :attributes
           attr_accessor :key, :parent, :description
 
-          def initialize(config, **attributes)
+          def initialize(config, **metadata)
             @config = config
             @entries = {}
-
-            (@attributes = attributes).each do |attribute, value|
-              public_send("#{attribute}=", value)
-            end
+            @metadata = metadata
 
             @validator = self.class.validator.new(self)
             @validator.validate(:new)
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index b509c5edf59..707b052e6a8 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -10,6 +10,7 @@ module Gitlab
 
           def initialize(node)
             @node = node
+            @metadata = {}
             @attributes = {}
           end
 
@@ -18,8 +19,8 @@ module Gitlab
             self
           end
 
-          def parent(parent)
-            @parent = parent
+          def metadata(metadata)
+            @metadata.merge!(metadata)
             self
           end
 
@@ -30,7 +31,6 @@ module Gitlab
 
           def create!
             raise InvalidFactory unless defined?(@value)
-            raise InvalidFactory unless defined?(@parent)
 
             ##
             # We assume that unspecified entry is undefined.
@@ -60,9 +60,9 @@ module Gitlab
           end
 
           def fabricate(node, value = nil)
-            node.new(value).tap do |entry|
+            node.new(value, @metadata).tap do |entry|
               entry.key = @attributes[:key]
-              entry.parent = @attributes[:parent] || @parent
+              entry.parent = @attributes[:parent]
               entry.description = @attributes[:description]
             end
           end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 4a958735599..bedacd904cc 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -54,9 +54,8 @@ module Gitlab
           def compose_jobs!
             factory = Node::Factory.new(Node::Jobs)
               .value(@config.except(*nodes.keys))
-              .parent(self)
-              .with(key: :jobs, global: self)
-              .with(description: 'Jobs definition for this pipeline')
+              .with(key: :jobs, parent: self,
+                    description: 'Jobs definition for this pipeline')
 
             @entries[:jobs] = factory.create!
           end
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 483be2a21cc..9280412a638 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -10,11 +10,8 @@ module Gitlab
 
           validations do
             validates :config, presence: true
-
-            with_options on: :processed do
-              validates :name, presence: true
-              validates :name, type: Symbol
-            end
+            validates :name, presence: true
+            validates :name, type: Symbol
           end
 
           node :before_script, Script,
@@ -38,7 +35,7 @@ module Gitlab
           helpers :before_script, :script, :stage, :type, :after_script, :cache
 
           def name
-            @key
+            @metadata[:name]
           end
 
           def value
@@ -48,11 +45,12 @@ module Gitlab
           private
 
           def to_hash
-            { before_script: before_script_value,
-              script: script_value,
-              stage: stage_value,
-              cache: cache_value,
-              after_script: after_script_value }
+            { name: name,
+              before_script: before_script,
+              script: script,
+              stage: stage,
+              cache: cache,
+              after_script: after_script }
           end
 
           def compose!
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 3c1851b9fea..3cabcd6b763 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -31,14 +31,15 @@ module Gitlab
           private
 
           def create(name, config)
-            Node::Factory.new(node(name))
+            Node::Factory.new(job_class(name))
               .value(config || {})
-              .parent(self)
-              .with(key: name, description: "#{name} job definition.")
+              .metadata(name: name)
+              .with(key: name, parent: self,
+                    description: "#{name} job definition.")
               .create!
           end
 
-          def node(name)
+          def job_class(name)
             if name.to_s.start_with?('.')
               Node::HiddenJob
             else
diff --git a/spec/lib/gitlab/ci/config/node/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb
index 4e6f2419e13..d26185ba585 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -4,32 +4,20 @@ describe Gitlab::Ci::Config::Node::Factory do
   describe '#create!' do
     let(:factory) { described_class.new(node) }
     let(:node) { Gitlab::Ci::Config::Node::Script }
-    let(:parent) { double('parent') }
 
     context 'when setting a concrete value' do
       it 'creates entry with valid value' do
         entry = factory
           .value(['ls', 'pwd'])
-          .parent(parent)
           .create!
 
         expect(entry.value).to eq ['ls', 'pwd']
       end
 
-      it 'sets parent  attributes' do
-        entry = factory
-          .value('ls')
-          .parent(parent)
-          .create!
-
-        expect(entry.parent).to eq parent
-      end
-
       context 'when setting description' do
         it 'creates entry with description' do
           entry = factory
             .value(['ls', 'pwd'])
-            .parent(parent)
             .with(description: 'test description')
             .create!
 
@@ -42,7 +30,6 @@ describe Gitlab::Ci::Config::Node::Factory do
         it 'creates entry with custom key' do
           entry = factory
             .value(['ls', 'pwd'])
-            .parent(parent)
             .with(key: 'test key')
             .create!
 
@@ -56,7 +43,6 @@ describe Gitlab::Ci::Config::Node::Factory do
         it 'creates entry with valid parent' do
           entry = factory
             .value('ls')
-            .parent(parent)
             .with(parent: object)
             .create!
 
@@ -73,23 +59,28 @@ describe Gitlab::Ci::Config::Node::Factory do
       end
     end
 
-    context 'when not setting parent object' do
-      it 'raises error' do
-        expect { factory.value('ls').create! }.to raise_error(
-          Gitlab::Ci::Config::Node::Factory::InvalidFactory
-        )
-      end
-    end
-
     context 'when creating entry with nil value' do
       it 'creates an undefined entry' do
         entry = factory
           .value(nil)
-          .parent(parent)
           .create!
 
         expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
       end
     end
+
+    context 'when passing metadata' do
+      let(:node) { spy('node') }
+
+      it 'passes metadata as a parameter' do
+        factory
+          .value('some value')
+          .metadata(some: 'hash')
+          .create!
+
+        expect(node).to have_received(:new)
+          .with('some value', { some: 'hash' })
+      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 dfcaebe35be..2a071b57c72 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -128,11 +128,14 @@ describe Gitlab::Ci::Config::Node::Global do
 
         describe '#jobs' do
           it 'returns jobs configuration' do
-            expect(global.jobs)
-              .to eq(rspec: { script: %w[rspec ls],
-                              stage: 'test'  },
-                     spinach: { script: %w[spinach],
-                                stage: 'test' })
+            expect(global.jobs).to eq(
+              rspec: { name: :rspec,
+                       script: %w[rspec ls],
+                       stage: 'test' },
+              spinach: { name: :spinach,
+                         script: %w[spinach],
+                         stage: 'test' }
+            )
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 4c7ac9949cc..b2559e6e73c 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Job do
-  let(:entry) { described_class.new(config, key: :rspec) }
+  let(:entry) { described_class.new(config, name: :rspec) }
 
   before do
     entry.process!
@@ -19,7 +19,7 @@ describe Gitlab::Ci::Config::Node::Job do
       end
 
       context 'when job name is empty' do
-        let(:entry) { described_class.new(config, key: ''.to_sym) }
+        let(:entry) { described_class.new(config, name: ''.to_sym) }
 
         it 'reports error' do
           expect(entry.errors)
@@ -35,7 +35,7 @@ describe Gitlab::Ci::Config::Node::Job do
         describe '#errors' do
           it 'reports error about a config type' do
             expect(entry.errors)
-              .to include 'rspec config should be a hash'
+              .to include 'job config should be a hash'
           end
         end
       end
@@ -62,7 +62,8 @@ describe Gitlab::Ci::Config::Node::Job do
 
       it 'returns correct value' do
         expect(entry.value)
-          .to eq(before_script: %w[ls pwd],
+          .to eq(name: :rspec,
+                 before_script: %w[ls pwd],
                  script: %w[rspec],
                  stage: 'test',
                  after_script: %w[cleanup])
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index c4c130abb6d..4f08f2f9b69 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -61,11 +61,13 @@ describe Gitlab::Ci::Config::Node::Jobs do
 
     describe '#value' do
       it 'returns key value' do
-        expect(entry.value)
-          .to eq(rspec: { script: %w[rspec],
-                          stage: 'test' },
-                 spinach: { script: %w[spinach],
-                            stage: 'test' })
+        expect(entry.value).to eq(
+          rspec: { name: :rspec,
+                   script: %w[rspec],
+                   stage: 'test' },
+          spinach: { name: :spinach,
+                     script: %w[spinach],
+                     stage: 'test' })
       end
     end
 

From 4bb60b0789a31061cbc81af90b7d5dc558f985b3 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 15 Jul 2016 21:39:26 +0200
Subject: [PATCH 50/62] Simplify CI config and remove logical validation

---
 lib/gitlab/ci/config.rb                     |  1 -
 lib/gitlab/ci/config/node/entry.rb          | 11 ++---------
 lib/gitlab/ci/config/node/global.rb         | 11 +----------
 lib/gitlab/ci/config/node/jobs.rb           | 16 +++++++---------
 spec/lib/gitlab/ci/config/node/job_spec.rb  |  5 +----
 spec/lib/gitlab/ci/config/node/jobs_spec.rb |  5 +----
 6 files changed, 12 insertions(+), 37 deletions(-)

diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index 20f5f8e2ff8..ae82c0db3f1 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -15,7 +15,6 @@ module Gitlab
 
         @global = Node::Global.new(@config)
         @global.process!
-        @global.validate!
       end
 
       def valid?
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 011c3be849e..559688c1bca 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -8,13 +8,13 @@ module Gitlab
         class Entry
           class InvalidError < StandardError; end
 
-          attr_reader :config, :attributes
+          attr_reader :config, :metadata
           attr_accessor :key, :parent, :description
 
           def initialize(config, **metadata)
             @config = config
-            @entries = {}
             @metadata = metadata
+            @entries = {}
 
             @validator = self.class.validator.new(self)
             @validator.validate(:new)
@@ -27,13 +27,6 @@ module Gitlab
             @entries.each_value(&:process!)
           end
 
-          def validate!
-            return unless valid?
-
-            @validator.validate(:processed)
-            @entries.each_value(&:validate!)
-          end
-
           def leaf?
             nodes.none?
           end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index bedacd904cc..3b0d0113d61 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -36,19 +36,13 @@ module Gitlab
           helpers :before_script, :image, :services, :after_script,
                   :variables, :stages, :types, :cache, :jobs
 
-          def initialize(*)
-            super
-
-            @global = self
-          end
-
           private
 
           def compose!
             super
 
-            compose_stages!
             compose_jobs!
+            compose_stages!
           end
 
           def compose_jobs!
@@ -65,9 +59,6 @@ module Gitlab
             # Deprecated `:types` key workaround - if types are defined and
             # stages are not defined we use types definition as stages.
             #
-            # Otherwise we use stages in favor of types, and remove types from
-            # processing.
-            #
             if types_defined? && !stages_defined?
               @entries[:stages] = @entries[:types]
             end
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 3cabcd6b763..77ff3459958 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -11,23 +11,21 @@ module Gitlab
           validations do
             validates :config, type: Hash
 
-            with_options on: :processed do
-              validate do
-                unless has_visible_job?
-                  errors.add(:config, 'should contain at least one visible job')
-                end
+            validate do
+              unless has_visible_job?
+                errors.add(:config, 'should contain at least one visible job')
               end
             end
+
+            def has_visible_job?
+              config.any? { |key, _| !key.to_s.start_with?('.') }
+            end
           end
 
           def nodes
             @config
           end
 
-          def has_visible_job?
-            @entries.values.any?(&:relevant?)
-          end
-
           private
 
           def create(name, config)
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index b2559e6e73c..2721908c5d7 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -3,10 +3,7 @@ require 'spec_helper'
 describe Gitlab::Ci::Config::Node::Job do
   let(:entry) { described_class.new(config, name: :rspec) }
 
-  before do
-    entry.process!
-    entry.validate!
-  end
+  before { entry.process! }
 
   describe 'validations' do
     context 'when entry config value is correct' do
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 4f08f2f9b69..b8d9c70479c 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -4,10 +4,7 @@ describe Gitlab::Ci::Config::Node::Jobs do
   let(:entry) { described_class.new(config) }
 
   describe 'validations' do
-    before do
-      entry.process!
-      entry.validate!
-    end
+    before { entry.process! }
 
     context 'when entry config value is correct' do
       let(:config) { { rspec: { script: 'rspec' } } }

From 17084d42aa4f2a9d58d6b6d30656d5b7cfffe007 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 15 Jul 2016 22:35:29 +0200
Subject: [PATCH 51/62] Simplify abstract class for CI config entry nodes

---
 lib/gitlab/ci/config/node/configurable.rb     | 12 ++++---
 lib/gitlab/ci/config/node/entry.rb            | 17 +---------
 lib/gitlab/ci/config/node/global.rb           |  2 +-
 lib/gitlab/ci/config/node/jobs.rb             | 31 +++++++++----------
 spec/lib/gitlab/ci/config/node/global_spec.rb | 14 ++++++---
 5 files changed, 33 insertions(+), 43 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 10b2db86d24..93a9a253322 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -25,12 +25,14 @@ module Gitlab
 
           private
 
-          def create(key, factory)
-            factory
-              .value(@config[key])
-              .with(key: key, parent: self)
+          def compose!
+            self.class.nodes.each do |key, factory|
+              factory
+                .value(@config[key])
+                .with(key: key, parent: self)
 
-            factory.create!
+              @entries[key] = factory.create!
+            end
           end
 
           class_methods do
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 559688c1bca..813e394e51b 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -28,11 +28,7 @@ module Gitlab
           end
 
           def leaf?
-            nodes.none?
-          end
-
-          def nodes
-            self.class.nodes
+            @entries.none?
           end
 
           def descendants
@@ -74,10 +70,6 @@ module Gitlab
           def self.default
           end
 
-          def self.nodes
-            {}
-          end
-
           def self.validator
             Validator
           end
@@ -85,13 +77,6 @@ module Gitlab
           private
 
           def compose!
-            nodes.each do |key, essence|
-              @entries[key] = create(key, essence)
-            end
-          end
-
-          def create(entry, essence)
-            raise NotImplementedError
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 3b0d0113d61..b545b78a940 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -47,7 +47,7 @@ module Gitlab
 
           def compose_jobs!
             factory = Node::Factory.new(Node::Jobs)
-              .value(@config.except(*nodes.keys))
+              .value(@config.except(*self.class.nodes.keys))
               .with(key: :jobs, parent: self,
                     description: 'Jobs definition for this pipeline')
 
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 77ff3459958..908c8f4f120 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -22,27 +22,24 @@ module Gitlab
             end
           end
 
-          def nodes
-            @config
-          end
-
           private
 
-          def create(name, config)
-            Node::Factory.new(job_class(name))
-              .value(config || {})
-              .metadata(name: name)
-              .with(key: name, parent: self,
-                    description: "#{name} job definition.")
-              .create!
+          def compose!
+            @config.each do |name, config|
+              node = hidden?(name) ? Node::HiddenJob : Node::Job
+
+              factory = Node::Factory.new(node)
+                .value(config || {})
+                .metadata(name: name)
+                .with(key: name, parent: self,
+                      description: "#{name} job definition.")
+
+              @entries[name] = factory.create!
+            end
           end
 
-          def job_class(name)
-            if name.to_s.start_with?('.')
-              Node::HiddenJob
-            else
-              Node::Job
-            end
+          def hidden?(name)
+            name.to_s.start_with?('.')
           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 2a071b57c72..2f87d270b36 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -51,11 +51,11 @@ describe Gitlab::Ci::Config::Node::Global do
           expect(global.descendants.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
+        describe '#leaf?' do
+          it 'is not leaf' do
+            expect(global).not_to be_leaf
+          end
         end
       end
 
@@ -65,6 +65,12 @@ describe Gitlab::Ci::Config::Node::Global do
             expect(global.before_script).to be nil
           end
         end
+
+        describe '#leaf?' do
+          it 'is leaf' do
+            expect(global).to be_leaf
+          end
+        end
       end
 
       context 'when processed' do

From 27e88efceb9d59affebf93be040b0a9b0bf31b2f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 18 Jul 2016 09:54:38 +0200
Subject: [PATCH 52/62] Move job image and services nodes to new CI config

---
 lib/ci/gitlab_ci_yaml_processor.rb           |  8 --------
 lib/gitlab/ci/config/node/job.rb             | 11 ++++++++++-
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb |  8 ++++----
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 0704e8f1683..b8d84de2dbe 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -120,14 +120,6 @@ module Ci
     end
 
     def validate_job_types!(name, job)
-      if job[:image] && !validate_string(job[:image])
-        raise ValidationError, "#{name} job: image should be a string"
-      end
-
-      if job[:services] && !validate_array_of_strings(job[:services])
-        raise ValidationError, "#{name} job: services should be an array of strings"
-      end
-
       if job[:tags] && !validate_array_of_strings(job[:tags])
         raise ValidationError, "#{name} job: tags parameter should be an array of strings"
       end
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 9280412a638..1c28969be14 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -32,7 +32,14 @@ module Gitlab
           node :cache, Cache,
             description: 'Cache definition for this job.'
 
-          helpers :before_script, :script, :stage, :type, :after_script, :cache
+          node :image, Image,
+            description: 'Image that will be used to execute this job.'
+
+          node :services, Services,
+            description: 'Services that will be used to execute this job.'
+
+          helpers :before_script, :script, :stage, :type, :after_script,
+                  :cache, :image, :services
 
           def name
             @metadata[:name]
@@ -48,6 +55,8 @@ module Gitlab
             { name: name,
               before_script: before_script,
               script: script,
+              image: image,
+              services: services,
               stage: stage,
               cache: cache,
               after_script: after_script }
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index d629bd252e2..6e6898e758c 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1012,7 +1012,7 @@ EOT
         config = YAML.dump({ rspec: { script: "test", image: ["test"] } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: image should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:image config should be a string")
       end
 
       it "returns errors if services parameter is not an array" do
@@ -1033,14 +1033,14 @@ EOT
         config = YAML.dump({ rspec: { script: "test", services: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: services should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:services config should be an array of strings")
       end
 
       it "returns errors if job services parameter is not an array of strings" do
         config = YAML.dump({ rspec: { script: "test", services: [10, "test"] } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: services should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:services config should be an array of strings")
       end
 
       it "returns error if job configuration is invalid" do
@@ -1054,7 +1054,7 @@ EOT
         config = YAML.dump({ extra: { services: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Unknown parameter: extra")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:extra:services config should be an array of strings")
       end
 
       it "returns errors if there are no jobs defined" do

From 1bf9e34713b414f0e1ac8bbfe464a4cd5300581f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 18 Jul 2016 12:37:42 +0200
Subject: [PATCH 53/62] Move except and only job nodes to new CI config

---
 lib/gitlab/ci/config/node/job.rb             | 10 +++-
 lib/gitlab/ci/config/node/while.rb           | 26 +++++++++
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb |  4 +-
 spec/lib/gitlab/ci/config/node/while_spec.rb | 56 ++++++++++++++++++++
 4 files changed, 93 insertions(+), 3 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/while.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/while_spec.rb

diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 1c28969be14..401611def17 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -38,8 +38,14 @@ module Gitlab
           node :services, Services,
             description: 'Services that will be used to execute this job.'
 
+          node :only, While,
+            description: 'Refs policy this job will be executed for.'
+
+          node :except, While,
+            description: 'Refs policy this job will be executed for.'
+
           helpers :before_script, :script, :stage, :type, :after_script,
-                  :cache, :image, :services
+                  :cache, :image, :services, :only, :except
 
           def name
             @metadata[:name]
@@ -59,6 +65,8 @@ module Gitlab
               services: services,
               stage: stage,
               cache: cache,
+              only: only,
+              except: except,
               after_script: after_script }
           end
 
diff --git a/lib/gitlab/ci/config/node/while.rb b/lib/gitlab/ci/config/node/while.rb
new file mode 100644
index 00000000000..84d4352624d
--- /dev/null
+++ b/lib/gitlab/ci/config/node/while.rb
@@ -0,0 +1,26 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a ref and trigger policy for the job.
+        #
+        class While < Entry
+          include Validatable
+
+          validations do
+            include LegacyValidationHelpers
+
+            validate :array_of_strings_or_regexps
+
+            def array_of_strings_or_regexps
+              unless validate_array_of_strings_or_regexps(config)
+                errors.add(:config, 'should be an array of strings or regexps')
+              end
+            end
+          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 6e6898e758c..429ffd6ef35 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -163,7 +163,7 @@ module Ci
 
           shared_examples 'raises an error' do
             it do
-              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'rspec job: only parameter should be an array of strings or regexps')
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'jobs:rspec:only config should be an array of strings or regexps')
             end
           end
 
@@ -319,7 +319,7 @@ module Ci
 
           shared_examples 'raises an error' do
             it do
-              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'rspec job: except parameter should be an array of strings or regexps')
+              expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'jobs:rspec:except config should be an array of strings or regexps')
             end
           end
 
diff --git a/spec/lib/gitlab/ci/config/node/while_spec.rb b/spec/lib/gitlab/ci/config/node/while_spec.rb
new file mode 100644
index 00000000000..aac2ed7b3db
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/while_spec.rb
@@ -0,0 +1,56 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::While do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is valid' do
+      context 'when config is a branch or tag name' do
+        let(:config) { %w[master feature/branch] }
+
+        describe '#valid?' do
+          it 'is valid' do
+            expect(entry).to be_valid
+          end
+        end
+
+        describe '#value' do
+          it 'returns key value' do
+            expect(entry.value).to eq config
+          end
+        end
+      end
+
+      context 'when config is a regexp' do
+        let(:config) { ['/^issue-.*$/'] }
+
+        describe '#valid?' do
+          it 'is valid' do
+            expect(entry).to be_valid
+          end
+        end
+      end
+
+      context 'when config is a special keyword' do
+        let(:config) { %w[tags triggers branches] }
+
+        describe '#valid?' do
+          it 'is valid' do
+            expect(entry).to be_valid
+          end
+        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 'while config should be an array of strings or regexps'
+        end
+      end
+    end
+  end
+end

From 47fa9f33ca552e085df2158db41b614a79f3651f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 18 Jul 2016 13:09:00 +0200
Subject: [PATCH 54/62] Move job variables config entry to new CI config

---
 lib/ci/gitlab_ci_yaml_processor.rb           | 16 ----------------
 lib/gitlab/ci/config/node/job.rb             |  6 +++++-
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb |  2 +-
 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index b8d84de2dbe..7306fd110bd 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -106,7 +106,6 @@ module Ci
       validate_job_types!(name, job)
 
       validate_job_stage!(name, job) if job[:stage]
-      validate_job_variables!(name, job) if job[:variables]
       validate_job_artifacts!(name, job) if job[:artifacts]
       validate_job_dependencies!(name, job) if job[:dependencies]
     end
@@ -124,14 +123,6 @@ module Ci
         raise ValidationError, "#{name} job: tags parameter should be an array of strings"
       end
 
-      if job[:only] && !validate_array_of_strings_or_regexps(job[:only])
-        raise ValidationError, "#{name} job: only parameter should be an array of strings or regexps"
-      end
-
-      if job[:except] && !validate_array_of_strings_or_regexps(job[:except])
-        raise ValidationError, "#{name} job: except parameter should be an array of strings or regexps"
-      end
-
       if job[:allow_failure] && !validate_boolean(job[:allow_failure])
         raise ValidationError, "#{name} job: allow_failure parameter should be an boolean"
       end
@@ -151,13 +142,6 @@ module Ci
       end
     end
 
-    def validate_job_variables!(name, job)
-      unless validate_variables(job[:variables])
-        raise ValidationError,
-          "#{name} job: variables should be a map of key-value strings"
-      end
-    end
-
     def validate_job_artifacts!(name, job)
       job[:artifacts].keys.each do |key|
         unless ALLOWED_ARTIFACTS_KEYS.include? key
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 401611def17..d2113556a08 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -44,8 +44,11 @@ module Gitlab
           node :except, While,
             description: 'Refs policy this job will be executed for.'
 
+          node :variables, Variables,
+            description: 'Environment variables available for this job.'
+
           helpers :before_script, :script, :stage, :type, :after_script,
-                  :cache, :image, :services, :only, :except
+                  :cache, :image, :services, :only, :except, :variables
 
           def name
             @metadata[:name]
@@ -67,6 +70,7 @@ module Gitlab
               cache: cache,
               only: only,
               except: except,
+              variables: variables_defined? ? variables : nil,
               after_script: after_script }
           end
 
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 429ffd6ef35..fe2c109f463 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -534,7 +534,7 @@ module Ci
 
               expect { GitlabCiYamlProcessor.new(config, path) }
                 .to raise_error(GitlabCiYamlProcessor::ValidationError,
-                                 /job: variables should be a map/)
+                                 /jobs:rspec:variables config should be a hash of key value pairs/)
             end
           end
 

From 24b686ebb64e2f5a02d812e9aa726f1ba0868c2e Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 18 Jul 2016 14:15:01 +0200
Subject: [PATCH 55/62] Move job artifacts configuration new CI config code

---
 lib/ci/gitlab_ci_yaml_processor.rb            | 20 ---------
 lib/gitlab/ci/config/node/artifacts.rb        | 32 ++++++++++++++
 lib/gitlab/ci/config/node/attributable.rb     | 23 ++++++++++
 lib/gitlab/ci/config/node/job.rb              |  7 ++-
 lib/gitlab/ci/config/node/validators.rb       | 10 +++++
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  | 12 +++---
 .../gitlab/ci/config/node/artifacts_spec.rb   | 34 +++++++++++++++
 .../ci/config/node/attributable_spec.rb       | 43 +++++++++++++++++++
 8 files changed, 154 insertions(+), 27 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/artifacts.rb
 create mode 100644 lib/gitlab/ci/config/node/attributable.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/artifacts_spec.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/attributable_spec.rb

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 7306fd110bd..ec85cf1bd3d 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -148,26 +148,6 @@ module Ci
           raise ValidationError, "#{name} job: artifacts unknown parameter #{key}"
         end
       end
-
-      if job[:artifacts][:name] && !validate_string(job[:artifacts][:name])
-        raise ValidationError, "#{name} job: artifacts:name parameter should be a string"
-      end
-
-      if job[:artifacts][:untracked] && !validate_boolean(job[:artifacts][:untracked])
-        raise ValidationError, "#{name} job: artifacts:untracked parameter should be an boolean"
-      end
-
-      if job[:artifacts][:paths] && !validate_array_of_strings(job[:artifacts][:paths])
-        raise ValidationError, "#{name} job: artifacts:paths parameter should be an array of strings"
-      end
-
-      if job[:artifacts][:when] && !job[:artifacts][:when].in?(%w[on_success on_failure always])
-        raise ValidationError, "#{name} job: artifacts:when parameter should be on_success, on_failure or always"
-      end
-
-      if job[:artifacts][:expire_in] && !validate_duration(job[:artifacts][:expire_in])
-        raise ValidationError, "#{name} job: artifacts:expire_in parameter should be a duration"
-      end
     end
 
     def validate_job_dependencies!(name, job)
diff --git a/lib/gitlab/ci/config/node/artifacts.rb b/lib/gitlab/ci/config/node/artifacts.rb
new file mode 100644
index 00000000000..7b3eb7e1992
--- /dev/null
+++ b/lib/gitlab/ci/config/node/artifacts.rb
@@ -0,0 +1,32 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a configuration of job artifacts.
+        #
+        class Artifacts < Entry
+          include Validatable
+          include Attributable
+
+          attributes :name, :untracked, :paths, :when, :expire_in
+
+          validations do
+            validates :config, type: Hash
+
+            with_options allow_nil: true do
+              validates :name, type: String
+              validates :untracked, boolean: true
+              validates :paths, array_of_strings: true
+              validates :when,
+                inclusion: { in: %w[on_success on_failure always],
+                             message: 'should be on_success, on_failure ' \
+                                      'or always' }
+              validates :expire_in, duration: true
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/attributable.rb b/lib/gitlab/ci/config/node/attributable.rb
new file mode 100644
index 00000000000..6e935c025e4
--- /dev/null
+++ b/lib/gitlab/ci/config/node/attributable.rb
@@ -0,0 +1,23 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        module Attributable
+          extend ActiveSupport::Concern
+
+          class_methods do
+            def attributes(*attributes)
+              attributes.each do |attribute|
+                define_method(attribute) do
+                  return unless config.is_a?(Hash)
+
+                  config[attribute]
+                end
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index d2113556a08..a6d7f16769c 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -47,8 +47,12 @@ module Gitlab
           node :variables, Variables,
             description: 'Environment variables available for this job.'
 
+          node :artifacts, Artifacts,
+            description: 'Artifacts configuration for this job.'
+
           helpers :before_script, :script, :stage, :type, :after_script,
-                  :cache, :image, :services, :only, :except, :variables
+                  :cache, :image, :services, :only, :except, :variables,
+                  :artifacts
 
           def name
             @metadata[:name]
@@ -71,6 +75,7 @@ module Gitlab
               only: only,
               except: except,
               variables: variables_defined? ? variables : nil,
+              artifacts: artifacts,
               after_script: after_script }
           end
 
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index d33b407af68..5568be80166 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -33,6 +33,16 @@ module Gitlab
             end
           end
 
+          class DurationValidator < ActiveModel::EachValidator
+            include LegacyValidationHelpers
+
+            def validate_each(record, attribute, value)
+              unless validate_duration(value)
+                record.errors.add(attribute, 'should be a duration')
+              end
+            end
+          end
+
           class RequiredValidator < ActiveModel::EachValidator
             def validate_each(record, attribute, value)
               if value.nil?
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index fe2c109f463..28849bcf3f4 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1138,42 +1138,42 @@ EOT
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { name: 1 } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:name parameter should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts name should be a string")
       end
 
       it "returns errors if job artifacts:when is not an a predefined value" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { when: 1 } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:when parameter should be on_success, on_failure or always")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts when should be on_success, on_failure or always")
       end
 
       it "returns errors if job artifacts:expire_in is not an a string" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { expire_in: 1 } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:expire_in parameter should be a duration")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts expire in should be a duration")
       end
 
       it "returns errors if job artifacts:expire_in is not an a valid duration" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { expire_in: "7 elephants" } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:expire_in parameter should be a duration")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts expire in should be a duration")
       end
 
       it "returns errors if job artifacts:untracked is not an array of strings" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { untracked: "string" } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:untracked parameter should be an boolean")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts untracked should be a boolean value")
       end
 
       it "returns errors if job artifacts:paths is not an array of strings" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { paths: "string" } } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:paths parameter should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:artifacts paths should be an array of strings")
       end
 
       it "returns errors if cache:untracked is not an array of strings" do
diff --git a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
new file mode 100644
index 00000000000..4973f7d599a
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Artifacts do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validation' do
+    context 'when entry config value is correct' do
+      let(:config) { { paths: %w[public/] } }
+
+      describe '#value' do
+        it 'returns image string' 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) { { name: 10 } }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'artifacts name should be a string'
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/node/attributable_spec.rb b/spec/lib/gitlab/ci/config/node/attributable_spec.rb
new file mode 100644
index 00000000000..24d9daafd88
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/attributable_spec.rb
@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Attributable do
+  let(:node) { Class.new }
+  let(:instance) { node.new }
+
+  before do
+    node.include(described_class)
+
+    node.class_eval do
+      attributes :name, :test
+    end
+  end
+
+  context 'config is a hash' do
+    before do
+      allow(instance)
+        .to receive(:config)
+        .and_return({ name: 'some name', test: 'some test' })
+    end
+
+    it 'returns the value of config' do
+      expect(instance.name).to eq 'some name'
+      expect(instance.test).to eq 'some test'
+    end
+
+    it 'returns no method error for unknown attributes' do
+      expect { instance.unknown }.to raise_error(NoMethodError)
+    end
+  end
+
+  context 'config is not a hash' do
+    before do
+      allow(instance)
+        .to receive(:config)
+        .and_return('some test')
+    end
+
+    it 'returns nil' do
+      expect(instance.test).to be_nil
+    end
+  end
+end

From 7cef4f1908d99743bf1dfb9c1cfdd6b2936b2b3d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 18 Jul 2016 15:38:06 +0200
Subject: [PATCH 56/62] Improve valid keys validation for CI config nodes

---
 lib/ci/gitlab_ci_yaml_processor.rb            |  9 --------
 lib/gitlab/ci/config/node/artifacts.rb        |  2 ++
 lib/gitlab/ci/config/node/cache.rb            |  8 +++----
 lib/gitlab/ci/config/node/validator.rb        |  8 +------
 lib/gitlab/ci/config/node/validators.rb       |  9 ++++----
 .../gitlab/ci/config/node/artifacts_spec.rb   | 21 ++++++++++++++-----
 6 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index ec85cf1bd3d..6901dfbd15d 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -106,7 +106,6 @@ module Ci
       validate_job_types!(name, job)
 
       validate_job_stage!(name, job) if job[:stage]
-      validate_job_artifacts!(name, job) if job[:artifacts]
       validate_job_dependencies!(name, job) if job[:dependencies]
     end
 
@@ -142,14 +141,6 @@ module Ci
       end
     end
 
-    def validate_job_artifacts!(name, job)
-      job[:artifacts].keys.each do |key|
-        unless ALLOWED_ARTIFACTS_KEYS.include? key
-          raise ValidationError, "#{name} job: artifacts unknown parameter #{key}"
-        end
-      end
-    end
-
     def validate_job_dependencies!(name, job)
       unless validate_array_of_strings(job[:dependencies])
         raise ValidationError, "#{name} job: dependencies parameter should be an array of strings"
diff --git a/lib/gitlab/ci/config/node/artifacts.rb b/lib/gitlab/ci/config/node/artifacts.rb
index 7b3eb7e1992..2c301cf2917 100644
--- a/lib/gitlab/ci/config/node/artifacts.rb
+++ b/lib/gitlab/ci/config/node/artifacts.rb
@@ -13,6 +13,8 @@ module Gitlab
 
           validations do
             validates :config, type: Hash
+            validates :config,
+              allowed_keys: %i[name untracked paths when expire_in]
 
             with_options allow_nil: true do
               validates :name, type: String
diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
index cdf8ba2e35d..21d96b220b8 100644
--- a/lib/gitlab/ci/config/node/cache.rb
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -8,6 +8,10 @@ module Gitlab
         class Cache < Entry
           include Configurable
 
+          validations do
+            validates :config, allowed_keys: %i[key untracked paths]
+          end
+
           node :key, Node::Key,
             description: 'Cache key used to define a cache affinity.'
 
@@ -16,10 +20,6 @@ module Gitlab
 
           node :paths, Node::Paths,
             description: 'Specify which paths should be cached across builds.'
-
-          validations do
-            validates :config, allowed_keys: true
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index ca000f245aa..43c7e102b50 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -21,12 +21,6 @@ module Gitlab
             'Validator'
           end
 
-          def unknown_keys
-            return [] unless config.is_a?(Hash)
-
-            config.keys - @node.class.nodes.keys
-          end
-
           private
 
           def location
@@ -35,7 +29,7 @@ module Gitlab
           end
 
           def key_name
-            if key.blank? || key.nil?
+            if key.blank?
               @node.class.name.demodulize.underscore.humanize
             else
               key
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index 5568be80166..b43a0bc0bab 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -5,10 +5,11 @@ module Gitlab
         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}")
+              unknown_keys = record.config.try(:keys).to_a - options[:in]
+
+              if unknown_keys.any?
+                record.errors.add(:config, 'contains unknown keys: ' +
+                                            unknown_keys.join(', '))
               end
             end
           end
diff --git a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
index 4973f7d599a..418a88cabac 100644
--- a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
@@ -21,12 +21,23 @@ describe Gitlab::Ci::Config::Node::Artifacts do
     end
 
     context 'when entry value is not correct' do
-      let(:config) { { name: 10 } }
-
       describe '#errors' do
-        it 'saves errors' do
-          expect(entry.errors)
-            .to include 'artifacts name should be a string'
+        context 'when value of attribute is invalid' do
+          let(:config) { { name: 10 } }
+
+          it 'reports error' do
+            expect(entry.errors)
+              .to include 'artifacts name should be a string'
+          end
+        end
+
+        context 'when there is uknown key' do
+          let(:config) { { test: 100 } }
+
+          it 'reports error' do
+            expect(entry.errors)
+              .to include 'artifacts config contains unknown keys: test'
+          end
         end
       end
     end

From 6d466733a2ab90f2a4d9904e7bfe1ef887568210 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 18 Jul 2016 15:46:24 +0200
Subject: [PATCH 57/62] Validate allowed keys only in new CI config

---
 lib/ci/gitlab_ci_yaml_processor.rb         | 19 -------------------
 lib/gitlab/ci/config/node/job.rb           |  5 +++++
 spec/lib/gitlab/ci/config/node/job_spec.rb | 10 ++++++++++
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 6901dfbd15d..5c54489ab3f 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -4,15 +4,6 @@ module Ci
 
     include Gitlab::Ci::Config::Node::LegacyValidationHelpers
 
-    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,
-                        :allow_failure, :type, :stage, :when, :artifacts, :cache,
-                        :dependencies, :before_script, :after_script, :variables,
-                        :environment]
-    ALLOWED_CACHE_KEYS = [:key, :untracked, :paths]
-    ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in]
-
     attr_reader :path, :cache, :stages
 
     def initialize(config, path = nil)
@@ -102,21 +93,11 @@ module Ci
     def validate_job!(name, job)
       raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
 
-      validate_job_keys!(name, job)
       validate_job_types!(name, job)
-
       validate_job_stage!(name, job) if job[:stage]
       validate_job_dependencies!(name, job) if job[:dependencies]
     end
 
-    def validate_job_keys!(name, job)
-      job.keys.each do |key|
-        unless (ALLOWED_JOB_KEYS + %i[name]).include? key
-          raise ValidationError, "#{name} job: unknown parameter #{key}"
-        end
-      end
-    end
-
     def validate_job_types!(name, job)
       if job[:tags] && !validate_array_of_strings(job[:tags])
         raise ValidationError, "#{name} job: tags parameter should be an array of strings"
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index a6d7f16769c..48569a7e890 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -9,6 +9,11 @@ module Gitlab
           include Configurable
 
           validations do
+            validates :config, allowed_keys:
+              %i[tags script only except type image services allow_failure
+                 type stage when artifacts cache dependencies before_script
+                 after_script variables environment]
+
             validates :config, presence: true
             validates :name, presence: true
             validates :name, type: Symbol
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 2721908c5d7..1484fb60dd8 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -46,6 +46,16 @@ describe Gitlab::Ci::Config::Node::Job do
           end
         end
       end
+
+      context 'when unknown keys detected' do
+        let(:config) { { unknown: true } }
+
+        describe '#valid' do
+          it 'is not valid' do
+            expect(entry).not_to be_valid
+          end
+        end
+      end
     end
   end
 

From 943ae747eac04e19c2614a5b48f41387c6d150d3 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 18 Jul 2016 16:33:20 +0200
Subject: [PATCH 58/62] Move tags and allow_failure CI entries to new config

---
 lib/ci/gitlab_ci_yaml_processor.rb           | 8 --------
 lib/gitlab/ci/config/node/job.rb             | 8 ++++++++
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 5c54489ab3f..3b6fcd909f4 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -99,14 +99,6 @@ module Ci
     end
 
     def validate_job_types!(name, job)
-      if job[:tags] && !validate_array_of_strings(job[:tags])
-        raise ValidationError, "#{name} job: tags parameter should be an array of strings"
-      end
-
-      if job[:allow_failure] && !validate_boolean(job[:allow_failure])
-        raise ValidationError, "#{name} job: allow_failure parameter should be an boolean"
-      end
-
       if job[:when] && !job[:when].in?(%w[on_success on_failure always])
         raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always"
       end
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 48569a7e890..c0ec2236231 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -7,6 +7,9 @@ module Gitlab
         #
         class Job < Entry
           include Configurable
+          include Attributable
+
+          attributes :tags, :allow_failure
 
           validations do
             validates :config, allowed_keys:
@@ -17,6 +20,11 @@ module Gitlab
             validates :config, presence: true
             validates :name, presence: true
             validates :name, type: Symbol
+
+            with_options allow_nil: true do
+              validates :tags, array_of_strings: true
+              validates :allow_failure, boolean: true
+            end
           end
 
           node :before_script, Script,
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 28849bcf3f4..d354ddb7b13 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -956,7 +956,7 @@ EOT
         config = YAML.dump({ rspec: { script: "test", tags: "mysql" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: tags parameter should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec tags should be an array of strings")
       end
 
       it "returns errors if before_script parameter is invalid" do
@@ -1075,7 +1075,7 @@ EOT
         config = YAML.dump({ rspec: { script: "test", allow_failure: "string" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: allow_failure parameter should be an boolean")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec allow failure should be a boolean value")
       end
 
       it "returns errors if job stage is not a string" do

From bb8bf6427d80cb4858318a44e395a2d1cd9115b7 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 19 Jul 2016 13:08:28 +0200
Subject: [PATCH 59/62] Move job environment validation to new CI config

---
 lib/ci/gitlab_ci_yaml_processor.rb                 | 11 -----------
 lib/gitlab/ci/config/node/job.rb                   | 14 +++++++++++++-
 .../ci/config/node/legacy_validation_helpers.rb    |  4 ----
 lib/gitlab/ci/config/node/validators.rb            |  3 ++-
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb       |  6 +++---
 5 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 3b6fcd909f4..aa2f7743a5e 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -93,21 +93,10 @@ module Ci
     def validate_job!(name, job)
       raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
 
-      validate_job_types!(name, job)
       validate_job_stage!(name, job) if job[:stage]
       validate_job_dependencies!(name, job) if job[:dependencies]
     end
 
-    def validate_job_types!(name, job)
-      if job[:when] && !job[:when].in?(%w[on_success on_failure always])
-        raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always"
-      end
-
-      if job[:environment] && !validate_environment(job[:environment])
-        raise ValidationError, "#{name} job: environment parameter #{Gitlab::Regex.environment_name_regex_message}"
-      end
-    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(", ")}"
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index c0ec2236231..464abd4d2d8 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -9,7 +9,7 @@ module Gitlab
           include Configurable
           include Attributable
 
-          attributes :tags, :allow_failure
+          attributes :tags, :allow_failure, :when, :environment
 
           validations do
             validates :config, allowed_keys:
@@ -24,6 +24,18 @@ module Gitlab
             with_options allow_nil: true do
               validates :tags, array_of_strings: true
               validates :allow_failure, boolean: true
+              validates :when,
+                inclusion: { in: %w[on_success on_failure always],
+                             message: 'should be on_success, on_failure ' \
+                                      'or always' }
+              validates :environment,
+                type: {
+                  with: String,
+                  message: Gitlab::Regex.environment_name_regex_message }
+              validates :environment,
+                format: {
+                  with: Gitlab::Regex.environment_name_regex,
+                  message: Gitlab::Regex.environment_name_regex_message }
             end
           end
 
diff --git a/lib/gitlab/ci/config/node/legacy_validation_helpers.rb b/lib/gitlab/ci/config/node/legacy_validation_helpers.rb
index 4d9a508796a..0c291efe6a5 100644
--- a/lib/gitlab/ci/config/node/legacy_validation_helpers.rb
+++ b/lib/gitlab/ci/config/node/legacy_validation_helpers.rb
@@ -41,10 +41,6 @@ module Gitlab
             false
           end
 
-          def validate_environment(value)
-            value.is_a?(String) && value =~ Gitlab::Regex.environment_name_regex
-          end
-
           def validate_boolean(value)
             value.in?([true, false])
           end
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index b43a0bc0bab..23d5faf6f07 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -69,7 +69,8 @@ module Gitlab
               raise unless type.is_a?(Class)
 
               unless value.is_a?(type)
-                record.errors.add(attribute, "should be a #{type.name}")
+                message = options[:message] || "should be a #{type.name}"
+                record.errors.add(attribute, message)
               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 d354ddb7b13..9735af27aa1 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -754,7 +754,7 @@ module Ci
         let(:environment) { 1 }
 
         it 'raises error' do
-          expect { builds }.to raise_error("deploy_to_production job: environment parameter #{Gitlab::Regex.environment_name_regex_message}")
+          expect { builds }.to raise_error("jobs:deploy_to_production environment #{Gitlab::Regex.environment_name_regex_message}")
         end
       end
 
@@ -762,7 +762,7 @@ module Ci
         let(:environment) { 'production staging' }
 
         it 'raises error' do
-          expect { builds }.to raise_error("deploy_to_production job: environment parameter #{Gitlab::Regex.environment_name_regex_message}")
+          expect { builds }.to raise_error("jobs:deploy_to_production environment #{Gitlab::Regex.environment_name_regex_message}")
         end
       end
     end
@@ -1131,7 +1131,7 @@ EOT
         config = YAML.dump({ rspec: { script: "test", when: 1 } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: when parameter should be on_success, on_failure or always")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec when should be on_success, on_failure or always")
       end
 
       it "returns errors if job artifacts:name is not an a string" do

From f83bccfe4f98281ed80c95189c5f7ed77799b2b3 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 20 Jul 2016 10:59:49 +0200
Subject: [PATCH 60/62] Add minor readability, style improvements in CI config

---
 lib/ci/gitlab_ci_yaml_processor.rb               |  5 ++---
 lib/gitlab/ci/config/node/artifacts.rb           |  7 ++++---
 lib/gitlab/ci/config/node/attributable.rb        |  2 +-
 lib/gitlab/ci/config/node/entry.rb               |  4 ++--
 lib/gitlab/ci/config/node/job.rb                 |  9 +++++----
 lib/gitlab/ci/config/node/jobs.rb                | 10 +++++-----
 lib/gitlab/ci/config/node/undefined.rb           |  2 +-
 spec/lib/gitlab/ci/config/node/artifacts_spec.rb |  2 +-
 8 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 013813ef00b..24601fdfe85 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -8,7 +8,8 @@ module Ci
 
     def initialize(config, path = nil)
       @ci_config = Gitlab::Ci::Config.new(config)
-      @config, @path = @ci_config.to_hash, path
+      @config = @ci_config.to_hash
+      @path = path
 
       unless @ci_config.valid?
         raise ValidationError, @ci_config.errors.first
@@ -120,8 +121,6 @@ module Ci
     end
 
     def validate_job!(name, job)
-      raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
-
       validate_job_stage!(name, job) if job[:stage]
       validate_job_dependencies!(name, job) if job[:dependencies]
     end
diff --git a/lib/gitlab/ci/config/node/artifacts.rb b/lib/gitlab/ci/config/node/artifacts.rb
index 2c301cf2917..844bd2fe998 100644
--- a/lib/gitlab/ci/config/node/artifacts.rb
+++ b/lib/gitlab/ci/config/node/artifacts.rb
@@ -9,12 +9,13 @@ module Gitlab
           include Validatable
           include Attributable
 
-          attributes :name, :untracked, :paths, :when, :expire_in
+          ALLOWED_KEYS = %i[name untracked paths when expire_in]
+
+          attributes ALLOWED_KEYS
 
           validations do
             validates :config, type: Hash
-            validates :config,
-              allowed_keys: %i[name untracked paths when expire_in]
+            validates :config, allowed_keys: ALLOWED_KEYS
 
             with_options allow_nil: true do
               validates :name, type: String
diff --git a/lib/gitlab/ci/config/node/attributable.rb b/lib/gitlab/ci/config/node/attributable.rb
index 6e935c025e4..221b666f9f6 100644
--- a/lib/gitlab/ci/config/node/attributable.rb
+++ b/lib/gitlab/ci/config/node/attributable.rb
@@ -7,7 +7,7 @@ module Gitlab
 
           class_methods do
             def attributes(*attributes)
-              attributes.each do |attribute|
+              attributes.flatten.each do |attribute|
                 define_method(attribute) do
                   return unless config.is_a?(Hash)
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 813e394e51b..0c782c422b5 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -24,7 +24,7 @@ module Gitlab
             return unless valid?
 
             compose!
-            @entries.each_value(&:process!)
+            descendants.each(&:process!)
           end
 
           def leaf?
@@ -44,7 +44,7 @@ module Gitlab
           end
 
           def errors
-            @validator.messages + @entries.values.flat_map(&:errors)
+            @validator.messages + descendants.flat_map(&:errors)
           end
 
           def value
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index aea9fef8229..dc0813a8c18 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -9,13 +9,14 @@ module Gitlab
           include Configurable
           include Attributable
 
+          ALLOWED_KEYS = %i[tags script only except type image services allow_failure
+                            type stage when artifacts cache dependencies before_script
+                            after_script variables environment]
+
           attributes :tags, :allow_failure, :when, :environment
 
           validations do
-            validates :config, allowed_keys:
-              %i[tags script only except type image services allow_failure
-                 type stage when artifacts cache dependencies before_script
-                 after_script variables environment]
+            validates :config, allowed_keys: ALLOWED_KEYS
 
             validates :config, presence: true
             validates :name, presence: true
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 908c8f4f120..51683c82ceb 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -18,10 +18,14 @@ module Gitlab
             end
 
             def has_visible_job?
-              config.any? { |key, _| !key.to_s.start_with?('.') }
+              config.any? { |name, _| !hidden?(name) }
             end
           end
 
+          def hidden?(name)
+            name.to_s.start_with?('.')
+          end
+
           private
 
           def compose!
@@ -37,10 +41,6 @@ module Gitlab
               @entries[name] = factory.create!
             end
           end
-
-          def hidden?(name)
-            name.to_s.start_with?('.')
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index 384774c9b69..84dab61e7e9 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -5,7 +5,7 @@ module Gitlab
         ##
         # This class represents an undefined and unspecified entry node.
         #
-        # It decorates original entry adding method that idicates it is
+        # It decorates original entry adding method that indicates it is
         # unspecified.
         #
         class Undefined < SimpleDelegator
diff --git a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
index 418a88cabac..beed29b18ae 100644
--- a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
@@ -31,7 +31,7 @@ describe Gitlab::Ci::Config::Node::Artifacts do
           end
         end
 
-        context 'when there is uknown key' do
+        context 'when there is an unknown key present' do
           let(:config) { { test: 100 } }
 
           it 'reports error' do

From dff10976da42cc729069cddf0f032347fe2f6b14 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 20 Jul 2016 14:15:18 +0200
Subject: [PATCH 61/62] Move job dependencies entry to the new CI config

---
 lib/ci/gitlab_ci_yaml_processor.rb           | 24 ++++++++------------
 lib/gitlab/ci/config/node/job.rb             |  4 +++-
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb |  2 +-
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 24601fdfe85..a2e8bd22a52 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -98,21 +98,22 @@ module Ci
       @jobs = @ci_config.jobs
 
       @jobs.each do |name, job|
-        validate_job!(name, job)
+        # logical validation for job
+
+        validate_job_stage!(name, job)
+        validate_job_dependencies!(name, job)
       end
     end
 
     def yaml_variables(name)
-      variables = global_variables.merge(job_variables(name))
+      variables = (@variables || {})
+        .merge(job_variables(name))
+
       variables.map do |key, value|
         { key: key, value: value, public: true }
       end
     end
 
-    def global_variables
-      @variables || {}
-    end
-
     def job_variables(name)
       job = @jobs[name.to_sym]
       return {} unless job
@@ -120,21 +121,16 @@ module Ci
       job[:variables] || {}
     end
 
-    def validate_job!(name, job)
-      validate_job_stage!(name, job) if job[:stage]
-      validate_job_dependencies!(name, job) if job[:dependencies]
-    end
-
     def validate_job_stage!(name, job)
+      return unless job[:stage]
+
       unless job[:stage].is_a?(String) && job[:stage].in?(@stages)
         raise ValidationError, "#{name} job: stage parameter should be #{@stages.join(", ")}"
       end
     end
 
     def validate_job_dependencies!(name, job)
-      unless validate_array_of_strings(job[:dependencies])
-        raise ValidationError, "#{name} job: dependencies parameter should be an array of strings"
-      end
+      return unless job[:dependencies]
 
       stage_index = @stages.index(job[:stage])
 
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index dc0813a8c18..ace79d829f2 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -13,7 +13,7 @@ module Gitlab
                             type stage when artifacts cache dependencies before_script
                             after_script variables environment]
 
-          attributes :tags, :allow_failure, :when, :environment
+          attributes :tags, :allow_failure, :when, :environment, :dependencies
 
           validations do
             validates :config, allowed_keys: ALLOWED_KEYS
@@ -37,6 +37,8 @@ module Gitlab
                 format: {
                   with: Gitlab::Regex.environment_name_regex,
                   message: Gitlab::Regex.environment_name_regex_message }
+
+              validates :dependencies, array_of_strings: true
             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 5785b7e59fb..61490555ff5 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1239,7 +1239,7 @@ EOT
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", dependencies: "string" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: dependencies parameter should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec dependencies should be an array of strings")
       end
     end
 

From a42cce1b966046c21ec48b18435d38e68a20f7fa Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 29 Jul 2016 12:30:38 +0200
Subject: [PATCH 62/62] Improve code, remove unused validator, improve names

---
 lib/gitlab/ci/config/node/cache.rb                   |  4 +++-
 lib/gitlab/ci/config/node/commands.rb                | 12 +++++-------
 lib/gitlab/ci/config/node/configurable.rb            |  7 +++----
 lib/gitlab/ci/config/node/global.rb                  |  4 ++--
 lib/gitlab/ci/config/node/job.rb                     |  4 ++--
 lib/gitlab/ci/config/node/null.rb                    |  2 +-
 lib/gitlab/ci/config/node/{while.rb => trigger.rb}   |  4 ++--
 lib/gitlab/ci/config/node/undefined.rb               |  6 +-----
 lib/gitlab/ci/config/node/validators.rb              |  9 ---------
 spec/lib/gitlab/ci/config/node/artifacts_spec.rb     |  2 +-
 .../config/node/{while_spec.rb => trigger_spec.rb}   |  4 ++--
 11 files changed, 22 insertions(+), 36 deletions(-)
 rename lib/gitlab/ci/config/node/{while.rb => trigger.rb} (83%)
 rename spec/lib/gitlab/ci/config/node/{while_spec.rb => trigger_spec.rb} (90%)

diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
index 21d96b220b8..b4bda2841ac 100644
--- a/lib/gitlab/ci/config/node/cache.rb
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -8,8 +8,10 @@ module Gitlab
         class Cache < Entry
           include Configurable
 
+          ALLOWED_KEYS = %i[key untracked paths]
+
           validations do
-            validates :config, allowed_keys: %i[key untracked paths]
+            validates :config, allowed_keys: ALLOWED_KEYS
           end
 
           node :key, Node::Key,
diff --git a/lib/gitlab/ci/config/node/commands.rb b/lib/gitlab/ci/config/node/commands.rb
index f7e6950001e..d7657ae314b 100644
--- a/lib/gitlab/ci/config/node/commands.rb
+++ b/lib/gitlab/ci/config/node/commands.rb
@@ -11,22 +11,20 @@ module Gitlab
           validations do
             include LegacyValidationHelpers
 
-            validate :string_or_array_of_strings
-
-            def string_or_array_of_strings
-              unless config_valid?
+            validate do
+              unless string_or_array_of_strings?(config)
                 errors.add(:config,
                            'should be a string or an array of strings')
               end
             end
 
-            def config_valid?
-              validate_string(config) || validate_array_of_strings(config)
+            def string_or_array_of_strings?(field)
+              validate_string(field) || validate_array_of_strings(field)
             end
           end
 
           def value
-            [@config].flatten
+            Array(@config)
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 93a9a253322..aedc28fe1d0 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -56,10 +56,9 @@ module Gitlab
                 end
 
                 define_method("#{symbol}_value") do
-                  if @entries[symbol]
-                    return unless @entries[symbol].valid?
-                    @entries[symbol].value
-                  end
+                  return unless @entries[symbol] && @entries[symbol].valid?
+
+                  @entries[symbol].value
                 end
 
                 alias_method symbol.to_sym, "#{symbol}_value".to_sym
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index b545b78a940..ccd539fb003 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -42,7 +42,7 @@ module Gitlab
             super
 
             compose_jobs!
-            compose_stages!
+            compose_deprecated_entries!
           end
 
           def compose_jobs!
@@ -54,7 +54,7 @@ module Gitlab
             @entries[:jobs] = factory.create!
           end
 
-          def compose_stages!
+          def compose_deprecated_entries!
             ##
             # Deprecated `:types` key workaround - if types are defined and
             # stages are not defined we use types definition as stages.
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index ace79d829f2..e84737acbb9 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -66,10 +66,10 @@ module Gitlab
           node :services, Services,
             description: 'Services that will be used to execute this job.'
 
-          node :only, While,
+          node :only, Trigger,
             description: 'Refs policy this job will be executed for.'
 
-          node :except, While,
+          node :except, Trigger,
             description: 'Refs policy this job will be executed for.'
 
           node :variables, Variables,
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index 880d29f663d..88a5f53f13c 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -3,7 +3,7 @@ module Gitlab
     class Config
       module Node
         ##
-        # This class represents an undefined and unspecified node.
+        # This class represents an undefined node.
         #
         # Implements the Null Object pattern.
         #
diff --git a/lib/gitlab/ci/config/node/while.rb b/lib/gitlab/ci/config/node/trigger.rb
similarity index 83%
rename from lib/gitlab/ci/config/node/while.rb
rename to lib/gitlab/ci/config/node/trigger.rb
index 84d4352624d..d8b31975088 100644
--- a/lib/gitlab/ci/config/node/while.rb
+++ b/lib/gitlab/ci/config/node/trigger.rb
@@ -3,9 +3,9 @@ module Gitlab
     class Config
       module Node
         ##
-        # Entry that represents a ref and trigger policy for the job.
+        # Entry that represents a trigger policy for the job.
         #
-        class While < Entry
+        class Trigger < Entry
           include Validatable
 
           validations do
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index 84dab61e7e9..45fef8c3ae5 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -3,16 +3,12 @@ module Gitlab
     class Config
       module Node
         ##
-        # This class represents an undefined and unspecified entry node.
+        # This class represents an unspecified entry node.
         #
         # It decorates original entry adding method that indicates it is
         # unspecified.
         #
         class Undefined < SimpleDelegator
-          def initialize(entry)
-            super
-          end
-
           def specified?
             false
           end
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index 23d5faf6f07..e20908ad3cb 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -44,15 +44,6 @@ module Gitlab
             end
           end
 
-          class RequiredValidator < ActiveModel::EachValidator
-            def validate_each(record, attribute, value)
-              if value.nil?
-                raise Entry::InvalidError,
-                  "Entry needs #{attribute} attribute set internally."
-              end
-            end
-          end
-
           class KeyValidator < ActiveModel::EachValidator
             include LegacyValidationHelpers
 
diff --git a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
index beed29b18ae..c09a0a9c793 100644
--- a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb
@@ -8,7 +8,7 @@ describe Gitlab::Ci::Config::Node::Artifacts do
       let(:config) { { paths: %w[public/] } }
 
       describe '#value' do
-        it 'returns image string' do
+        it 'returns artifacs configuration' do
           expect(entry.value).to eq config
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/while_spec.rb b/spec/lib/gitlab/ci/config/node/trigger_spec.rb
similarity index 90%
rename from spec/lib/gitlab/ci/config/node/while_spec.rb
rename to spec/lib/gitlab/ci/config/node/trigger_spec.rb
index aac2ed7b3db..a4a3e36754e 100644
--- a/spec/lib/gitlab/ci/config/node/while_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/trigger_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Ci::Config::Node::While do
+describe Gitlab::Ci::Config::Node::Trigger do
   let(:entry) { described_class.new(config) }
 
   describe 'validations' do
@@ -48,7 +48,7 @@ describe Gitlab::Ci::Config::Node::While do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'while config should be an array of strings or regexps'
+            .to include 'trigger config should be an array of strings or regexps'
         end
       end
     end