From 6609589b935147886fbaba187231af7ada846d43 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 6 Jun 2016 09:05:00 +0200
Subject: [PATCH 01/56] Add ci config global and before_script entries

---
 lib/gitlab/ci/config/entry/base_entry.rb          | 15 +++++++++++++++
 lib/gitlab/ci/config/entry/before_script.rb       | 13 +++++++++++++
 lib/gitlab/ci/config/entry/global.rb              | 13 +++++++++++++
 .../gitlab/ci/config/entry/before_script_spec.rb  | 11 +++++++++++
 spec/lib/gitlab/ci/config/entry/global_spec.rb    |  5 +++++
 spec/lib/gitlab/ci/config_spec.rb                 |  3 ++-
 6 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 lib/gitlab/ci/config/entry/base_entry.rb
 create mode 100644 lib/gitlab/ci/config/entry/before_script.rb
 create mode 100644 lib/gitlab/ci/config/entry/global.rb
 create mode 100644 spec/lib/gitlab/ci/config/entry/before_script_spec.rb
 create mode 100644 spec/lib/gitlab/ci/config/entry/global_spec.rb

diff --git a/lib/gitlab/ci/config/entry/base_entry.rb b/lib/gitlab/ci/config/entry/base_entry.rb
new file mode 100644
index 0000000000..3a41487d89
--- /dev/null
+++ b/lib/gitlab/ci/config/entry/base_entry.rb
@@ -0,0 +1,15 @@
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        class BaseEntry
+          def initialize(hash, config, parent = nil)
+            @hash = hash
+            @config = config
+            @parent = parent
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/entry/before_script.rb b/lib/gitlab/ci/config/entry/before_script.rb
new file mode 100644
index 0000000000..b7f15355a5
--- /dev/null
+++ b/lib/gitlab/ci/config/entry/before_script.rb
@@ -0,0 +1,13 @@
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        class BeforeScript < BaseEntry
+          def leaf?
+            true
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/entry/global.rb b/lib/gitlab/ci/config/entry/global.rb
new file mode 100644
index 0000000000..e333ecb949
--- /dev/null
+++ b/lib/gitlab/ci/config/entry/global.rb
@@ -0,0 +1,13 @@
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        class Global < BaseEntry
+          def allowed_keys
+            []
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/entry/before_script_spec.rb b/spec/lib/gitlab/ci/config/entry/before_script_spec.rb
new file mode 100644
index 0000000000..69573af554
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/entry/before_script_spec.rb
@@ -0,0 +1,11 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::BeforeScript do
+  let(:entry) { described_class.new(hash, config) }
+
+  describe '#leaf?' do
+    it 'is a leaf entry' do
+      expect(entry).to be_leaf
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb
new file mode 100644
index 0000000000..8be956bb0e
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Global do
+
+end
diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb
index 52aafbcaaa..211226f9f7 100644
--- a/spec/lib/gitlab/ci/config_spec.rb
+++ b/spec/lib/gitlab/ci/config_spec.rb
@@ -37,7 +37,8 @@ describe Gitlab::Ci::Config do
       describe '.new' do
         it 'raises error' do
           expect { config }.to raise_error(
-            Gitlab::Ci::Config::LoaderError, /Invalid configuration format/
+            Gitlab::Ci::Config::LoaderError,
+            /Invalid configuration format/
           )
         end
       end

From 7f2f683eeb2c3b443f519e2e83dbb3d789a00cf8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 6 Jun 2016 09:24:16 +0200
Subject: [PATCH 02/56] Rename ci config module that holds nodes to Node

---
 lib/gitlab/ci/config/entry/global.rb                | 13 -------------
 .../ci/config/{entry => node}/before_script.rb      |  4 ++--
 .../config/{entry/base_entry.rb => node/entry.rb}   |  8 ++++++--
 lib/gitlab/ci/config/node/global.rb                 | 10 ++++++++++
 spec/lib/gitlab/ci/config/entry/global_spec.rb      |  5 -----
 .../ci/config/{entry => node}/before_script_spec.rb |  2 +-
 spec/lib/gitlab/ci/config/node/global_spec.rb       |  5 +++++
 7 files changed, 24 insertions(+), 23 deletions(-)
 delete mode 100644 lib/gitlab/ci/config/entry/global.rb
 rename lib/gitlab/ci/config/{entry => node}/before_script.rb (69%)
 rename lib/gitlab/ci/config/{entry/base_entry.rb => node/entry.rb} (71%)
 create mode 100644 lib/gitlab/ci/config/node/global.rb
 delete mode 100644 spec/lib/gitlab/ci/config/entry/global_spec.rb
 rename spec/lib/gitlab/ci/config/{entry => node}/before_script_spec.rb (77%)
 create mode 100644 spec/lib/gitlab/ci/config/node/global_spec.rb

diff --git a/lib/gitlab/ci/config/entry/global.rb b/lib/gitlab/ci/config/entry/global.rb
deleted file mode 100644
index e333ecb949..0000000000
--- a/lib/gitlab/ci/config/entry/global.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-module Gitlab
-  module Ci
-    class Config
-      module Entry
-        class Global < BaseEntry
-          def allowed_keys
-            []
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/ci/config/entry/before_script.rb b/lib/gitlab/ci/config/node/before_script.rb
similarity index 69%
rename from lib/gitlab/ci/config/entry/before_script.rb
rename to lib/gitlab/ci/config/node/before_script.rb
index b7f15355a5..bf73c01efb 100644
--- a/lib/gitlab/ci/config/entry/before_script.rb
+++ b/lib/gitlab/ci/config/node/before_script.rb
@@ -1,8 +1,8 @@
 module Gitlab
   module Ci
     class Config
-      module Entry
-        class BeforeScript < BaseEntry
+      module Node
+        class BeforeScript < Entry
           def leaf?
             true
           end
diff --git a/lib/gitlab/ci/config/entry/base_entry.rb b/lib/gitlab/ci/config/node/entry.rb
similarity index 71%
rename from lib/gitlab/ci/config/entry/base_entry.rb
rename to lib/gitlab/ci/config/node/entry.rb
index 3a41487d89..eb1b52a3e5 100644
--- a/lib/gitlab/ci/config/entry/base_entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -1,13 +1,17 @@
 module Gitlab
   module Ci
     class Config
-      module Entry
-        class BaseEntry
+      module Node
+        class Entry
           def initialize(hash, config, parent = nil)
             @hash = hash
             @config = config
             @parent = parent
           end
+
+          def allowed_keys
+            []
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
new file mode 100644
index 0000000000..b3dd6df0a4
--- /dev/null
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -0,0 +1,10 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        class Global < Entry
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb
deleted file mode 100644
index 8be956bb0e..0000000000
--- a/spec/lib/gitlab/ci/config/entry/global_spec.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Ci::Config::Entry::Global do
-
-end
diff --git a/spec/lib/gitlab/ci/config/entry/before_script_spec.rb b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
similarity index 77%
rename from spec/lib/gitlab/ci/config/entry/before_script_spec.rb
rename to spec/lib/gitlab/ci/config/node/before_script_spec.rb
index 69573af554..eb86931c58 100644
--- a/spec/lib/gitlab/ci/config/entry/before_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Ci::Config::Entry::BeforeScript do
+describe Gitlab::Ci::Config::Node::BeforeScript do
   let(:entry) { described_class.new(hash, config) }
 
   describe '#leaf?' do
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
new file mode 100644
index 0000000000..89594fa20c
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Global do
+
+end

From 8048dcc8e693d713a94a7b9361672692f4e5932f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 6 Jun 2016 10:43:11 +0200
Subject: [PATCH 03/56] Implement CI configuration nodes tree processing

---
 lib/gitlab/ci/config/node/before_script.rb    |  7 +++--
 lib/gitlab/ci/config/node/entry.rb            | 22 ++++++++++++--
 lib/gitlab/ci/config/node/global.rb           |  3 ++
 .../ci/config/node/before_script_spec.rb      |  6 ----
 spec/lib/gitlab/ci/config/node/global_spec.rb | 30 +++++++++++++++++++
 5 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/lib/gitlab/ci/config/node/before_script.rb b/lib/gitlab/ci/config/node/before_script.rb
index bf73c01efb..88ebd6bb30 100644
--- a/lib/gitlab/ci/config/node/before_script.rb
+++ b/lib/gitlab/ci/config/node/before_script.rb
@@ -3,8 +3,11 @@ module Gitlab
     class Config
       module Node
         class BeforeScript < Entry
-          def leaf?
-            true
+          def keys
+            {}
+          end
+
+          def validate!
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index eb1b52a3e5..6336535bc0 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -3,14 +3,32 @@ module Gitlab
     class Config
       module Node
         class Entry
+          attr_reader :hash, :config, :parent, :nodes, :errors
+
           def initialize(hash, config, parent = nil)
             @hash = hash
             @config = config
             @parent = parent
+            @nodes = {}
+            @errors = []
           end
 
-          def allowed_keys
-            []
+          def process!
+            keys.each_pair do |key, entry|
+              next unless hash.include?(key)
+              @nodes[key] = entry.new(hash[key], config, self)
+            end
+
+            @nodes.values.each(&:process!)
+            @nodes.values.each(&:validate!)
+          end
+
+          def keys
+            raise NotImplementedError
+          end
+
+          def validate!
+            raise NotImplementedError
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index b3dd6df0a4..81a9d0667b 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -3,6 +3,9 @@ module Gitlab
     class Config
       module Node
         class Global < Entry
+          def keys
+            { before_script: BeforeScript }
+          end
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/before_script_spec.rb b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
index eb86931c58..d4a8eea3ff 100644
--- a/spec/lib/gitlab/ci/config/node/before_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
@@ -2,10 +2,4 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::BeforeScript do
   let(:entry) { described_class.new(hash, config) }
-
-  describe '#leaf?' do
-    it 'is a leaf entry' do
-      expect(entry).to be_leaf
-    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 89594fa20c..e2e8fcfabd 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -1,5 +1,35 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Global do
+  let(:global) { described_class.new(hash, config) }
+  let(:config) { double('Config') }
 
+  describe '#keys' do
+    it 'can contain global config keys' do
+      expect(global.keys).to include :before_script
+    end
+  end
+
+  context 'when hash is valid' do
+    let(:hash) do
+      { before_script: ['ls', 'pwd'] }
+    end
+
+    describe '#process!' do
+      before { global.process! }
+
+      it 'creates nodes hash' do
+        expect(global.nodes).to be_a Hash
+      end
+
+      it 'creates node object for each entry' do
+        expect(global.nodes.count).to eq 1
+      end
+
+      it 'creates node object using valid class' do
+        expect(global.nodes[:before_script])
+          .to be_an_instance_of Gitlab::Ci::Config::Node::BeforeScript
+      end
+    end
+  end
 end

From 251dd571dfc3e6261ed075ecf725dd98ee176b69 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 6 Jun 2016 11:05:15 +0200
Subject: [PATCH 04/56] Extract CI config validation helpers to mixin

---
 lib/ci/gitlab_ci_yaml_processor.rb         | 18 ++-------------
 lib/gitlab/ci/config/validation_helpers.rb | 26 ++++++++++++++++++++++
 2 files changed, 28 insertions(+), 16 deletions(-)
 create mode 100644 lib/gitlab/ci/config/validation_helpers.rb

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 46a923161c..e470ec56b7 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -2,6 +2,8 @@ module Ci
   class GitlabCiYamlProcessor
     class ValidationError < StandardError; end
 
+    include Gitlab::Ci::Config::ValidationHelpers
+
     DEFAULT_STAGES = %w(build test deploy)
     DEFAULT_STAGE = 'test'
     ALLOWED_YAML_KEYS = [:before_script, :after_script, :image, :services, :types, :stages, :variables, :cache]
@@ -276,22 +278,6 @@ module Ci
       end
     end
 
-    def validate_array_of_strings(values)
-      values.is_a?(Array) && values.all? { |value| validate_string(value) }
-    end
-
-    def validate_variables(variables)
-      variables.is_a?(Hash) && variables.all? { |key, value| validate_string(key) && validate_string(value) }
-    end
-
-    def validate_string(value)
-      value.is_a?(String) || value.is_a?(Symbol)
-    end
-
-    def validate_boolean(value)
-      value.in?([true, false])
-    end
-
     def process?(only_params, except_params, ref, tag, trigger_request)
       if only_params.present?
         return false unless matching?(only_params, ref, tag, trigger_request)
diff --git a/lib/gitlab/ci/config/validation_helpers.rb b/lib/gitlab/ci/config/validation_helpers.rb
new file mode 100644
index 0000000000..9e4e9a8332
--- /dev/null
+++ b/lib/gitlab/ci/config/validation_helpers.rb
@@ -0,0 +1,26 @@
+module Gitlab
+  module Ci
+    class Config
+      module ValidationHelpers
+        private
+
+        def validate_array_of_strings(values)
+          values.is_a?(Array) && values.all? { |value| validate_string(value) }
+        end
+
+        def validate_variables(variables)
+          variables.is_a?(Hash) &&
+            variables.all? { |key, value| validate_string(key) && validate_string(value) }
+        end
+
+        def validate_string(value)
+          value.is_a?(String) || value.is_a?(Symbol)
+        end
+
+        def validate_boolean(value)
+          value.in?([true, false])
+        end
+      end
+    end
+  end
+end

From 6dbd1c86a82156dd5ad39b0e2ad119a493dadeae Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 6 Jun 2016 11:20:47 +0200
Subject: [PATCH 05/56] Validate new before script CI configuration entry

---
 lib/gitlab/ci/config/node/before_script.rb    |  3 +++
 lib/gitlab/ci/config/node/entry.rb            | 12 ++++++----
 .../ci/config/node/before_script_spec.rb      | 24 ++++++++++++++++++-
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/lib/gitlab/ci/config/node/before_script.rb b/lib/gitlab/ci/config/node/before_script.rb
index 88ebd6bb30..204e0970a9 100644
--- a/lib/gitlab/ci/config/node/before_script.rb
+++ b/lib/gitlab/ci/config/node/before_script.rb
@@ -8,6 +8,9 @@ module Gitlab
           end
 
           def validate!
+            unless validate_array_of_strings(@value)
+              @errors << 'before_script should be an array of strings'
+            end
           end
         end
       end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 6336535bc0..3220b01ca1 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -3,10 +3,12 @@ module Gitlab
     class Config
       module Node
         class Entry
-          attr_reader :hash, :config, :parent, :nodes, :errors
+          include Config::ValidationHelpers
 
-          def initialize(hash, config, parent = nil)
-            @hash = hash
+          attr_reader :value, :config, :parent, :nodes, :errors
+
+          def initialize(value, config, parent = nil)
+            @value = value
             @config = config
             @parent = parent
             @nodes = {}
@@ -15,8 +17,8 @@ module Gitlab
 
           def process!
             keys.each_pair do |key, entry|
-              next unless hash.include?(key)
-              @nodes[key] = entry.new(hash[key], config, self)
+              next unless @value.include?(key)
+              @nodes[key] = entry.new(@value[key], config, self)
             end
 
             @nodes.values.each(&:process!)
diff --git a/spec/lib/gitlab/ci/config/node/before_script_spec.rb b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
index d4a8eea3ff..e6d0bfd5ea 100644
--- a/spec/lib/gitlab/ci/config/node/before_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
@@ -1,5 +1,27 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::BeforeScript do
-  let(:entry) { described_class.new(hash, config) }
+  let(:entry) { described_class.new(value, config) }
+  let(:config) { double('config') }
+
+  describe '#validate!' do
+    before { entry.validate! }
+
+    context 'when entry value is correct' do
+      let(:value) { ['ls', 'pwd'] }
+
+      it 'does not append errors' do
+        expect(entry.errors).to be_empty
+      end
+    end
+
+    context 'when entry value is not correct' do
+      let(:value) { 'ls' }
+
+      it 'saves errors' do
+        expect(entry.errors)
+          .to include /should be an array of strings/
+      end
+    end
+  end
 end

From a3c0745514ad98df1fbb8a6142f6cc50df76edae Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 6 Jun 2016 11:54:54 +0200
Subject: [PATCH 06/56] Collect errors from all nodes in new CI config

---
 lib/gitlab/ci/config/node/entry.rb            | 18 ++++++++++---
 spec/lib/gitlab/ci/config/node/global_spec.rb | 25 +++++++++++++++++--
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 3220b01ca1..4547531653 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -5,7 +5,7 @@ module Gitlab
         class Entry
           include Config::ValidationHelpers
 
-          attr_reader :value, :config, :parent, :nodes, :errors
+          attr_reader :value, :parent
 
           def initialize(value, config, parent = nil)
             @value = value
@@ -21,8 +21,20 @@ module Gitlab
               @nodes[key] = entry.new(@value[key], config, self)
             end
 
-            @nodes.values.each(&:process!)
-            @nodes.values.each(&:validate!)
+            nodes.each(&:process!)
+            nodes.each(&:validate!)
+          end
+
+          def errors
+            @errors + nodes.map(&:errors).flatten
+          end
+
+          def valid?
+            errors.none?
+          end
+
+          def nodes
+            @nodes.values
           end
 
           def keys
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index e2e8fcfabd..4b464db35b 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -19,7 +19,7 @@ describe Gitlab::Ci::Config::Node::Global do
       before { global.process! }
 
       it 'creates nodes hash' do
-        expect(global.nodes).to be_a Hash
+        expect(global.nodes).to be_an Array
       end
 
       it 'creates node object for each entry' do
@@ -27,9 +27,30 @@ describe Gitlab::Ci::Config::Node::Global do
       end
 
       it 'creates node object using valid class' do
-        expect(global.nodes[:before_script])
+        expect(global.nodes.first)
           .to be_an_instance_of Gitlab::Ci::Config::Node::BeforeScript
       end
     end
   end
+
+  context 'when hash is not valid' do
+    let(:hash) do
+      { before_script: 'ls' }
+    end
+
+    before { global.process! }
+
+    describe '#valid?' do
+      it 'is not valid' do
+        expect(global).not_to be_valid
+      end
+    end
+
+    describe '#errors' do
+      it 'reports errors from child nodes' do
+        expect(global.errors)
+          .to include 'before_script should be an array of strings'
+      end
+    end
+  end
 end

From 940763e0e72a7f71c6e60f2a1a848f8fe4afaf33 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 6 Jun 2016 12:23:27 +0200
Subject: [PATCH 07/56] Use CI config errors from new processor in legacy one

---
 lib/ci/gitlab_ci_yaml_processor.rb | 12 +++++----
 lib/gitlab/ci/config.rb            |  4 +++
 lib/gitlab/ci/config/node/entry.rb |  2 +-
 spec/lib/gitlab/ci/config_spec.rb  | 40 ++++++++++++++++++++++++------
 4 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index e470ec56b7..4bd2ac4f2d 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -14,7 +14,9 @@ module Ci
     attr_reader :before_script, :after_script, :image, :services, :path, :cache
 
     def initialize(config, path = nil)
-      @config = Gitlab::Ci::Config.new(config).to_hash
+      @ci_config = Gitlab::Ci::Config.new(config)
+      @config = @ci_config.to_hash
+
       @path = path
 
       initial_parsing
@@ -99,6 +101,10 @@ module Ci
     end
 
     def validate!
+      unless @ci_config.valid?
+        raise ValidationError, @ci_config.errors.first
+      end
+
       validate_global!
 
       @jobs.each do |name, job|
@@ -109,10 +115,6 @@ module Ci
     end
 
     def validate_global!
-      unless validate_array_of_strings(@before_script)
-        raise ValidationError, "before_script should be an array of strings"
-      end
-
       unless @after_script.nil? || validate_array_of_strings(@after_script)
         raise ValidationError, "after_script should be an array of strings"
       end
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index 5fc4894311..a042c49add 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -3,6 +3,8 @@ module Gitlab
     class Config
       class LoaderError < StandardError; end
 
+      delegate :valid?, :errors, to: :@global
+
       def initialize(config)
         loader = Loader.new(config)
 
@@ -11,6 +13,8 @@ module Gitlab
         end
 
         @config = loader.load
+        @global = Node::Global.new(@config, self)
+        @global.process!
       end
 
       def to_hash
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 4547531653..e8ed5f54c5 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -18,7 +18,7 @@ module Gitlab
           def process!
             keys.each_pair do |key, entry|
               next unless @value.include?(key)
-              @nodes[key] = entry.new(@value[key], config, self)
+              @nodes[key] = entry.new(@value[key], @config, self)
             end
 
             nodes.each(&:process!)
diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb
index 211226f9f7..ba8a44f4fc 100644
--- a/spec/lib/gitlab/ci/config_spec.rb
+++ b/spec/lib/gitlab/ci/config_spec.rb
@@ -29,17 +29,43 @@ describe Gitlab::Ci::Config do
 
         expect(config.to_hash).to eq hash
       end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(config).to be_valid
+        end
+
+        it 'has no errors' do
+          expect(config.errors).to be_empty
+        end
+      end
     end
 
     context 'when config is invalid' do
-      let(:yml) { '// invalid' }
+      context 'when yml is incorrect' do
+        let(:yml) { '// invalid' }
 
-      describe '.new' do
-        it 'raises error' do
-          expect { config }.to raise_error(
-            Gitlab::Ci::Config::LoaderError,
-            /Invalid configuration format/
-          )
+        describe '.new' do
+          it 'raises error' do
+            expect { config }.to raise_error(
+              Gitlab::Ci::Config::LoaderError,
+              /Invalid configuration format/
+            )
+          end
+        end
+      end
+
+      context 'when config logic is incorrect' do
+        let(:yml) { 'before_script: "ls"' }
+
+        describe '#valid?' do
+          it 'is not valid' do
+            expect(config).not_to be_valid
+          end
+
+          it 'has errors' do
+            expect(config.errors).not_to be_empty
+          end
         end
       end
     end

From b95c60a0715b5639e70b64e04fd4923e8bdd1923 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 7 Jun 2016 11:26:39 +0200
Subject: [PATCH 08/56] Do not process Ci config node when node is a leaf

---
 lib/gitlab/ci/config/node/entry.rb            | 10 ++++++++--
 spec/lib/gitlab/ci/config/node/global_spec.rb |  6 ++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index e8ed5f54c5..007585d401 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -16,6 +16,8 @@ module Gitlab
           end
 
           def process!
+            return if leaf?
+
             keys.each_pair do |key, entry|
               next unless @value.include?(key)
               @nodes[key] = entry.new(@value[key], @config, self)
@@ -29,12 +31,16 @@ module Gitlab
             @errors + nodes.map(&:errors).flatten
           end
 
+          def nodes
+            @nodes.values
+          end
+
           def valid?
             errors.none?
           end
 
-          def nodes
-            @nodes.values
+          def leaf?
+            keys.none?
           end
 
           def keys
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 4b464db35b..06c88b61f0 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -31,6 +31,12 @@ describe Gitlab::Ci::Config::Node::Global do
           .to be_an_instance_of Gitlab::Ci::Config::Node::BeforeScript
       end
     end
+
+    describe '#leaf?' do
+      it 'is not leaf' do
+        expect(global).not_to be_leaf
+      end
+    end
   end
 
   context 'when hash is not valid' do

From 69a3755c5a93395fd2fdfd5bee00e6064d1670f8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 7 Jun 2016 11:58:02 +0200
Subject: [PATCH 09/56] Add Ci config entry that implements Null Object

---
 lib/gitlab/ci/config/node/entry.rb          | 22 +++++++++++----------
 lib/gitlab/ci/config/node/null.rb           | 17 ++++++++++++++++
 spec/lib/gitlab/ci/config/node/null_spec.rb | 17 ++++++++++++++++
 3 files changed, 46 insertions(+), 10 deletions(-)
 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/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 007585d401..af92899af4 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -5,22 +5,28 @@ module Gitlab
         class Entry
           include Config::ValidationHelpers
 
-          attr_reader :value, :parent
+          attr_reader :value, :nodes, :parent
 
           def initialize(value, config, parent = nil)
             @value = value
             @config = config
             @parent = parent
-            @nodes = {}
-            @errors = []
+            @nodes, @errors = [], []
+
+            keys.each_key do |key|
+              instance_variable_set("@#{key}", Null.new(nil, config, self))
+            end
           end
 
           def process!
             return if leaf?
 
-            keys.each_pair do |key, entry|
-              next unless @value.include?(key)
-              @nodes[key] = entry.new(@value[key], @config, self)
+            keys.each do |key, entry_class|
+              next unless @value.has_key?(key)
+
+              entry = entry_class.new(@value[key], @config, self)
+              instance_variable_set("@#{key}", entry)
+              @nodes.append(entry)
             end
 
             nodes.each(&:process!)
@@ -31,10 +37,6 @@ module Gitlab
             @errors + nodes.map(&:errors).flatten
           end
 
-          def nodes
-            @nodes.values
-          end
-
           def valid?
             errors.none?
           end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
new file mode 100644
index 0000000000..6147b0d882
--- /dev/null
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -0,0 +1,17 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        class Null < Entry
+          def keys
+            {}
+          end
+
+          def method_missing(*)
+            nil
+          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 0000000000..42a6789296
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/null_spec.rb
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Null do
+  let(:entry) { described_class.new(double, double) }
+
+  describe '#leaf?' do
+    it 'is leaf node' do
+      expect(entry).to be_leaf
+    end
+  end
+
+  describe '#any_method' do
+    it 'responds with nil' do
+      expect(entry.any_method).to be nil
+    end
+  end
+end

From e8f995ef2631983ffe960464d0fd13a4c5ed8e09 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 7 Jun 2016 12:13:22 +0200
Subject: [PATCH 10/56] Pass root Ci config entry to each subsequent entry

---
 lib/gitlab/ci/config.rb                              |  2 +-
 lib/gitlab/ci/config/node/entry.rb                   | 10 +++++-----
 spec/lib/gitlab/ci/config/node/before_script_spec.rb |  3 +--
 spec/lib/gitlab/ci/config/node/global_spec.rb        |  3 +--
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index a042c49add..62cd514a72 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -13,7 +13,7 @@ module Gitlab
         end
 
         @config = loader.load
-        @global = Node::Global.new(@config, self)
+        @global = Node::Global.new(@config)
         @global.process!
       end
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index af92899af4..e2afeb1b3c 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -7,14 +7,14 @@ module Gitlab
 
           attr_reader :value, :nodes, :parent
 
-          def initialize(value, config, parent = nil)
+          def initialize(value, root = nil, parent = nil)
             @value = value
-            @config = config
+            @root = root
             @parent = parent
             @nodes, @errors = [], []
 
             keys.each_key do |key|
-              instance_variable_set("@#{key}", Null.new(nil, config, self))
+              instance_variable_set("@#{key}", Null.new(nil, root, self))
             end
           end
 
@@ -24,7 +24,7 @@ module Gitlab
             keys.each do |key, entry_class|
               next unless @value.has_key?(key)
 
-              entry = entry_class.new(@value[key], @config, self)
+              entry = entry_class.new(@value[key], @root, self)
               instance_variable_set("@#{key}", entry)
               @nodes.append(entry)
             end
@@ -42,7 +42,7 @@ module Gitlab
           end
 
           def leaf?
-            keys.none?
+            keys.none? # TODO || !@value.is_a?(Hash)
           end
 
           def keys
diff --git a/spec/lib/gitlab/ci/config/node/before_script_spec.rb b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
index e6d0bfd5ea..80c05f3de2 100644
--- a/spec/lib/gitlab/ci/config/node/before_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
@@ -1,8 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::BeforeScript do
-  let(:entry) { described_class.new(value, config) }
-  let(:config) { double('config') }
+  let(:entry) { described_class.new(value, double)}
 
   describe '#validate!' do
     before { entry.validate! }
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 06c88b61f0..c920dd3584 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -1,8 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Global do
-  let(:global) { described_class.new(hash, config) }
-  let(:config) { double('Config') }
+  let(:global) { described_class.new(hash) }
 
   describe '#keys' do
     it 'can contain global config keys' do

From 6bd67f5212de739b3016b0941853ce42f523a0f1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 7 Jun 2016 12:48:26 +0200
Subject: [PATCH 11/56] Do not process new Ci config entry when invalid

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

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index e2afeb1b3c..c07e7cf652 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -16,10 +16,14 @@ module Gitlab
             keys.each_key do |key|
               instance_variable_set("@#{key}", Null.new(nil, root, self))
             end
+
+            unless leaf? || value.is_a?(Hash)
+              @errors << 'should be a configuration entry with hash value'
+            end
           end
 
           def process!
-            return if leaf?
+            return if leaf? || !valid?
 
             keys.each do |key, entry_class|
               next unless @value.has_key?(key)
@@ -42,7 +46,7 @@ module Gitlab
           end
 
           def leaf?
-            keys.none? # TODO || !@value.is_a?(Hash)
+            keys.none?
           end
 
           def keys
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index c920dd3584..f277c457a3 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -58,4 +58,16 @@ describe Gitlab::Ci::Config::Node::Global do
       end
     end
   end
+
+  context 'when value is not a hash' do
+    let(:hash) { [] }
+
+    before { global.process! }
+
+    describe '#valid?' do
+      it 'is not valid' do
+        expect(global).not_to be_valid
+      end
+    end
+  end
 end

From df25c19699ba35682fd92da2b9c451bb4ba1c775 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 7 Jun 2016 12:58:32 +0200
Subject: [PATCH 12/56] Use Ci config validation helpers only where needed

---
 lib/ci/gitlab_ci_yaml_processor.rb            |  2 +-
 lib/gitlab/ci/config/node/before_script.rb    |  2 ++
 lib/gitlab/ci/config/node/entry.rb            |  2 --
 .../ci/config/node/validation_helpers.rb      | 28 +++++++++++++++++++
 lib/gitlab/ci/config/validation_helpers.rb    | 26 -----------------
 5 files changed, 31 insertions(+), 29 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/validation_helpers.rb
 delete mode 100644 lib/gitlab/ci/config/validation_helpers.rb

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 4bd2ac4f2d..c2b941a270 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -2,7 +2,7 @@ module Ci
   class GitlabCiYamlProcessor
     class ValidationError < StandardError; end
 
-    include Gitlab::Ci::Config::ValidationHelpers
+    include Gitlab::Ci::Config::Node::ValidationHelpers
 
     DEFAULT_STAGES = %w(build test deploy)
     DEFAULT_STAGE = 'test'
diff --git a/lib/gitlab/ci/config/node/before_script.rb b/lib/gitlab/ci/config/node/before_script.rb
index 204e0970a9..586eab12a0 100644
--- a/lib/gitlab/ci/config/node/before_script.rb
+++ b/lib/gitlab/ci/config/node/before_script.rb
@@ -3,6 +3,8 @@ module Gitlab
     class Config
       module Node
         class BeforeScript < Entry
+          include ValidationHelpers
+
           def keys
             {}
           end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index c07e7cf652..e95bc7bad4 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -3,8 +3,6 @@ module Gitlab
     class Config
       module Node
         class Entry
-          include Config::ValidationHelpers
-
           attr_reader :value, :nodes, :parent
 
           def initialize(value, root = nil, parent = nil)
diff --git a/lib/gitlab/ci/config/node/validation_helpers.rb b/lib/gitlab/ci/config/node/validation_helpers.rb
new file mode 100644
index 0000000000..4ea26492b6
--- /dev/null
+++ b/lib/gitlab/ci/config/node/validation_helpers.rb
@@ -0,0 +1,28 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        module ValidationHelpers
+          private
+
+          def validate_array_of_strings(values)
+            values.is_a?(Array) && values.all? { |value| validate_string(value) }
+          end
+
+          def validate_variables(variables)
+            variables.is_a?(Hash) &&
+              variables.all? { |key, value| validate_string(key) && validate_string(value) }
+          end
+
+          def validate_string(value)
+            value.is_a?(String) || value.is_a?(Symbol)
+          end
+
+          def validate_boolean(value)
+            value.in?([true, false])
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/validation_helpers.rb b/lib/gitlab/ci/config/validation_helpers.rb
deleted file mode 100644
index 9e4e9a8332..0000000000
--- a/lib/gitlab/ci/config/validation_helpers.rb
+++ /dev/null
@@ -1,26 +0,0 @@
-module Gitlab
-  module Ci
-    class Config
-      module ValidationHelpers
-        private
-
-        def validate_array_of_strings(values)
-          values.is_a?(Array) && values.all? { |value| validate_string(value) }
-        end
-
-        def validate_variables(variables)
-          variables.is_a?(Hash) &&
-            variables.all? { |key, value| validate_string(key) && validate_string(value) }
-        end
-
-        def validate_string(value)
-          value.is_a?(String) || value.is_a?(Symbol)
-        end
-
-        def validate_boolean(value)
-          value.in?([true, false])
-        end
-      end
-    end
-  end
-end

From c2d6d61dac2bf04b649c84ab0f4fe98da906c2c4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 7 Jun 2016 13:19:22 +0200
Subject: [PATCH 13/56] Add DSL for adding nodes in Ci config interface

---
 lib/gitlab/ci/config/node/before_script.rb    |  4 ----
 lib/gitlab/ci/config/node/entry.rb            | 12 +++++++++++-
 lib/gitlab/ci/config/node/global.rb           |  4 +---
 lib/gitlab/ci/config/node/null.rb             |  4 ----
 spec/lib/gitlab/ci/config/node/global_spec.rb |  4 ++++
 5 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/lib/gitlab/ci/config/node/before_script.rb b/lib/gitlab/ci/config/node/before_script.rb
index 586eab12a0..a8c350f3c7 100644
--- a/lib/gitlab/ci/config/node/before_script.rb
+++ b/lib/gitlab/ci/config/node/before_script.rb
@@ -5,10 +5,6 @@ module Gitlab
         class BeforeScript < Entry
           include ValidationHelpers
 
-          def keys
-            {}
-          end
-
           def validate!
             unless validate_array_of_strings(@value)
               @errors << 'before_script should be an array of strings'
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index e95bc7bad4..3043dc4c61 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -48,12 +48,22 @@ module Gitlab
           end
 
           def keys
-            raise NotImplementedError
+            self.class.nodes || {}
           end
 
           def validate!
             raise NotImplementedError
           end
+
+          class << self
+            attr_reader :nodes
+
+            private
+
+            def add_node(symbol, entry_class)
+              (@nodes ||= {}).merge!(symbol.to_sym => entry_class)
+            end
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 81a9d0667b..cfa506c28b 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -3,9 +3,7 @@ module Gitlab
     class Config
       module Node
         class Global < Entry
-          def keys
-            { before_script: BeforeScript }
-          end
+          add_node :before_script, BeforeScript
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index 6147b0d882..fc240e16f5 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -3,10 +3,6 @@ module Gitlab
     class Config
       module Node
         class Null < Entry
-          def keys
-            {}
-          end
-
           def method_missing(*)
             nil
           end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index f277c457a3..05e035ada3 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -7,6 +7,10 @@ describe Gitlab::Ci::Config::Node::Global do
     it 'can contain global config keys' do
       expect(global.keys).to include :before_script
     end
+
+    it 'returns a hash' do
+      expect(global.keys).to be_a Hash
+    end
   end
 
   context 'when hash is valid' do

From 70bda3e89bc3828fc8771496ec6d61e41ac3d3ed Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 7 Jun 2016 14:23:47 +0200
Subject: [PATCH 14/56] Implement script in Ci config and use in legacy one

---
 lib/ci/gitlab_ci_yaml_processor.rb            |  2 +-
 lib/gitlab/ci/config.rb                       |  5 ++++
 lib/gitlab/ci/config/node/before_script.rb    | 10 ++++++++
 lib/gitlab/ci/config/node/entry.rb            |  4 +++
 lib/gitlab/ci/config/node/global.rb           |  4 +++
 .../ci/config/node/before_script_spec.rb      | 25 ++++++++++++++-----
 spec/lib/gitlab/ci/config/node/global_spec.rb | 14 ++++++-----
 7 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index c2b941a270..0483e13b09 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -82,7 +82,7 @@ module Ci
       {
         stage_idx: stages.index(job[:stage]),
         stage: job[:stage],
-        commands: [job[:before_script] || @before_script, job[:script]].flatten.join("\n"),
+        commands: [job[:before_script] || [@ci_config.before_script], job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
         name: name,
         only: job[:only],
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index 62cd514a72..6e3fd2aa60 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -5,6 +5,11 @@ module Gitlab
 
       delegate :valid?, :errors, to: :@global
 
+      ##
+      # Temporary delegations that should be removed after refactoring
+      #
+      delegate :before_script, to: :@global
+
       def initialize(config)
         loader = Loader.new(config)
 
diff --git a/lib/gitlab/ci/config/node/before_script.rb b/lib/gitlab/ci/config/node/before_script.rb
index a8c350f3c7..271cb7b5da 100644
--- a/lib/gitlab/ci/config/node/before_script.rb
+++ b/lib/gitlab/ci/config/node/before_script.rb
@@ -5,6 +5,16 @@ module Gitlab
         class BeforeScript < Entry
           include ValidationHelpers
 
+          def description
+            'Script that is executed before the one defined in a job.'
+          end
+
+          def script
+            raise unless valid?
+
+            @value.join("\n")
+          end
+
           def validate!
             unless validate_array_of_strings(@value)
               @errors << 'before_script should be an array of strings'
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 3043dc4c61..f8f2d0be23 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -55,6 +55,10 @@ module Gitlab
             raise NotImplementedError
           end
 
+          def description
+            raise NotImplementedError
+          end
+
           class << self
             attr_reader :nodes
 
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index cfa506c28b..5912ead21c 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -4,6 +4,10 @@ module Gitlab
       module Node
         class Global < Entry
           add_node :before_script, BeforeScript
+
+          def before_script
+            @before_script.script
+          end
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/before_script_spec.rb b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
index 80c05f3de2..8ccefb9b9b 100644
--- a/spec/lib/gitlab/ci/config/node/before_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
@@ -2,25 +2,38 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::BeforeScript do
   let(:entry) { described_class.new(value, double)}
+  before { entry.validate! }
 
-  describe '#validate!' do
-    before { entry.validate! }
+  context 'when entry value is correct' do
+    let(:value) { ['ls', 'pwd'] }
 
-    context 'when entry value is correct' do
-      let(:value) { ['ls', 'pwd'] }
+    describe '#script' do
+      it 'returns concatenated command' do
+        expect(entry.script).to eq "ls\npwd"
+      end
+    end
 
+    describe '#errors' do
       it 'does not append errors' do
         expect(entry.errors).to be_empty
       end
     end
+  end
 
-    context 'when entry value is not correct' do
-      let(:value) { 'ls' }
+  context 'when entry value is not correct' do
+    let(:value) { 'ls' }
 
+    describe '#errors' do
       it 'saves errors' do
         expect(entry.errors)
           .to include /should be an array of strings/
       end
     end
+
+    describe '#script' do
+      it 'raises error' do
+        expect { entry.script }.to raise_error
+      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 05e035ada3..7f49b89f6d 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -3,6 +3,8 @@ require 'spec_helper'
 describe Gitlab::Ci::Config::Node::Global do
   let(:global) { described_class.new(hash) }
 
+  before { global.process! }
+
   describe '#keys' do
     it 'can contain global config keys' do
       expect(global.keys).to include :before_script
@@ -19,8 +21,6 @@ describe Gitlab::Ci::Config::Node::Global do
     end
 
     describe '#process!' do
-      before { global.process! }
-
       it 'creates nodes hash' do
         expect(global.nodes).to be_an Array
       end
@@ -40,6 +40,12 @@ describe Gitlab::Ci::Config::Node::Global do
         expect(global).not_to be_leaf
       end
     end
+
+    describe '#before_script' do
+      it 'returns correct script' do
+        expect(global.before_script).to eq "ls\npwd"
+      end
+    end
   end
 
   context 'when hash is not valid' do
@@ -47,8 +53,6 @@ describe Gitlab::Ci::Config::Node::Global do
       { before_script: 'ls' }
     end
 
-    before { global.process! }
-
     describe '#valid?' do
       it 'is not valid' do
         expect(global).not_to be_valid
@@ -66,8 +70,6 @@ describe Gitlab::Ci::Config::Node::Global do
   context 'when value is not a hash' do
     let(:hash) { [] }
 
-    before { global.process! }
-
     describe '#valid?' do
       it 'is not valid' do
         expect(global).not_to be_valid

From cba266aabc60aeee64ac2eb7e76b3e9e7012bad4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 8 Jun 2016 11:44:07 +0200
Subject: [PATCH 15/56] Remove old before_script from legacy Ci config

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

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index b37d231e89..c5a820563f 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -11,7 +11,7 @@ module Ci
                         :allow_failure, :type, :stage, :when, :artifacts, :cache,
                         :dependencies, :before_script, :after_script, :variables]
 
-    attr_reader :before_script, :after_script, :image, :services, :path, :cache
+    attr_reader :after_script, :image, :services, :path, :cache
 
     def initialize(config, path = nil)
       @ci_config = Gitlab::Ci::Config.new(config)
@@ -54,7 +54,6 @@ module Ci
     private
 
     def initial_parsing
-      @before_script = @config[:before_script] || []
       @after_script = @config[:after_script]
       @image = @config[:image]
       @services = @config[:services]

From 87fe50f2a0facd5bfdf287195a21932ff2340e1b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 8 Jun 2016 12:32:56 +0200
Subject: [PATCH 16/56] Delegate Ci config entry value to single method

---
 lib/gitlab/ci/config/node/before_script.rb    |  4 +--
 lib/gitlab/ci/config/node/entry.rb            | 34 +++++++++++++------
 lib/gitlab/ci/config/node/global.rb           |  4 ---
 lib/gitlab/ci/config/node/null.rb             |  7 ++++
 .../ci/config/node/before_script_spec.rb      | 10 ++----
 spec/lib/gitlab/ci/config/node/global_spec.rb |  8 +++++
 spec/lib/gitlab/ci/config/node/null_spec.rb   |  6 ++++
 7 files changed, 48 insertions(+), 25 deletions(-)

diff --git a/lib/gitlab/ci/config/node/before_script.rb b/lib/gitlab/ci/config/node/before_script.rb
index 271cb7b5da..be2ceebf3f 100644
--- a/lib/gitlab/ci/config/node/before_script.rb
+++ b/lib/gitlab/ci/config/node/before_script.rb
@@ -9,9 +9,7 @@ module Gitlab
             'Script that is executed before the one defined in a job.'
           end
 
-          def script
-            raise unless valid?
-
+          def value
             @value.join("\n")
           end
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index f8f2d0be23..0767fadcb9 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -3,17 +3,14 @@ module Gitlab
     class Config
       module Node
         class Entry
-          attr_reader :value, :nodes, :parent
+          class InvalidError < StandardError; end
 
           def initialize(value, root = nil, parent = nil)
             @value = value
             @root = root
             @parent = parent
-            @nodes, @errors = [], []
-
-            keys.each_key do |key|
-              instance_variable_set("@#{key}", Null.new(nil, root, self))
-            end
+            @nodes = {}
+            @errors = []
 
             unless leaf? || value.is_a?(Hash)
               @errors << 'should be a configuration entry with hash value'
@@ -24,17 +21,23 @@ module Gitlab
             return if leaf? || !valid?
 
             keys.each do |key, entry_class|
-              next unless @value.has_key?(key)
+              if @value.has_key?(key)
+                entry = entry_class.new(@value[key], @root, self)
+              else
+                entry = Node::Null.new(nil, @root, self)
+              end
 
-              entry = entry_class.new(@value[key], @root, self)
-              instance_variable_set("@#{key}", entry)
-              @nodes.append(entry)
+              @nodes[key] = entry
             end
 
             nodes.each(&:process!)
             nodes.each(&:validate!)
           end
 
+          def nodes
+            @nodes.values
+          end
+
           def errors
             @errors + nodes.map(&:errors).flatten
           end
@@ -51,6 +54,17 @@ module Gitlab
             self.class.nodes || {}
           end
 
+          def method_missing(name, *args)
+            super unless keys.has_key?(name)
+            raise InvalidError unless valid?
+
+            @nodes[name].value
+          end
+
+          def value
+            raise NotImplementedError
+          end
+
           def validate!
             raise NotImplementedError
           end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 5912ead21c..cfa506c28b 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -4,10 +4,6 @@ module Gitlab
       module Node
         class Global < Entry
           add_node :before_script, BeforeScript
-
-          def before_script
-            @before_script.script
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index fc240e16f5..db3fa05c32 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -3,6 +3,13 @@ module Gitlab
     class Config
       module Node
         class Null < Entry
+          def value
+            nil
+          end
+
+          def validate!
+          end
+
           def method_missing(*)
             nil
           end
diff --git a/spec/lib/gitlab/ci/config/node/before_script_spec.rb b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
index 8ccefb9b9b..bc34b9c9b5 100644
--- a/spec/lib/gitlab/ci/config/node/before_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
@@ -7,9 +7,9 @@ describe Gitlab::Ci::Config::Node::BeforeScript do
   context 'when entry value is correct' do
     let(:value) { ['ls', 'pwd'] }
 
-    describe '#script' do
+    describe '#value' do
       it 'returns concatenated command' do
-        expect(entry.script).to eq "ls\npwd"
+        expect(entry.value).to eq "ls\npwd"
       end
     end
 
@@ -29,11 +29,5 @@ describe Gitlab::Ci::Config::Node::BeforeScript do
           .to include /should be an array of strings/
       end
     end
-
-    describe '#script' do
-      it 'raises error' do
-        expect { entry.script }.to raise_error
-      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 7f49b89f6d..66d40be6e6 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -65,6 +65,14 @@ describe Gitlab::Ci::Config::Node::Global do
           .to include 'before_script should be an array of strings'
       end
     end
+
+    describe '#before_script' do
+      it 'raises error' do
+        expect { global.before_script }.to raise_error(
+          Gitlab::Ci::Config::Node::Entry::InvalidError
+        )
+      end
+    end
   end
 
   context 'when value is not a hash' do
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb
index 42a6789296..fa75bdcaa6 100644
--- a/spec/lib/gitlab/ci/config/node/null_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/null_spec.rb
@@ -14,4 +14,10 @@ describe Gitlab::Ci::Config::Node::Null do
       expect(entry.any_method).to be nil
     end
   end
+
+  describe '#value' do
+    it 'returns nill' do
+      expect(entry.value).to be nil
+    end
+  end
 end

From 5065612a0a1a5dd68c075e54f5f5f89c5c025a6b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 8 Jun 2016 13:01:44 +0200
Subject: [PATCH 17/56] Add minor improvements in new Ci config design

---
 lib/gitlab/ci/config/node/entry.rb            | 40 +++++++++++++------
 lib/gitlab/ci/config/node/null.rb             |  1 +
 .../ci/config/node/before_script_spec.rb      | 12 ++++++
 spec/lib/gitlab/ci/config/node/global_spec.rb | 12 ++++++
 4 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 0767fadcb9..302cded664 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -12,22 +12,16 @@ module Gitlab
             @nodes = {}
             @errors = []
 
-            unless leaf? || value.is_a?(Hash)
+            unless leaf? || has_config?
               @errors << 'should be a configuration entry with hash value'
             end
           end
 
           def process!
-            return if leaf? || !valid?
+            return if leaf? || invalid?
 
             keys.each do |key, entry_class|
-              if @value.has_key?(key)
-                entry = entry_class.new(@value[key], @root, self)
-              else
-                entry = Node::Null.new(nil, @root, self)
-              end
-
-              @nodes[key] = entry
+              add_node(key, entry_class)
             end
 
             nodes.each(&:process!)
@@ -38,22 +32,30 @@ module Gitlab
             @nodes.values
           end
 
-          def errors
-            @errors + nodes.map(&:errors).flatten
-          end
-
           def valid?
             errors.none?
           end
 
+          def invalid?
+            !valid?
+          end
+
           def leaf?
             keys.none?
           end
 
+          def has_config?
+            @value.is_a?(Hash)
+          end
+
           def keys
             self.class.nodes || {}
           end
 
+          def errors
+            @errors + nodes.map(&:errors).flatten
+          end
+
           def method_missing(name, *args)
             super unless keys.has_key?(name)
             raise InvalidError unless valid?
@@ -73,6 +75,18 @@ module Gitlab
             raise NotImplementedError
           end
 
+          private
+
+          def add_node(key, entry_class)
+            if @value.has_key?(key)
+              entry = entry_class.new(@value[key], @root, self)
+            else
+              entry = Node::Null.new(nil, @root, self)
+            end
+
+            @nodes[key] = entry
+          end
+
           class << self
             attr_reader :nodes
 
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index db3fa05c32..bf8bc62dc9 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -8,6 +8,7 @@ module Gitlab
           end
 
           def validate!
+            nil
           end
 
           def method_missing(*)
diff --git a/spec/lib/gitlab/ci/config/node/before_script_spec.rb b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
index bc34b9c9b5..b506b9743c 100644
--- a/spec/lib/gitlab/ci/config/node/before_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/before_script_spec.rb
@@ -18,6 +18,12 @@ describe Gitlab::Ci::Config::Node::BeforeScript do
         expect(entry.errors).to be_empty
       end
     end
+
+    describe '#has_config?' do
+      it 'does not have config' do
+        expect(entry).not_to have_config
+      end
+    end
   end
 
   context 'when entry value is not correct' do
@@ -29,5 +35,11 @@ describe Gitlab::Ci::Config::Node::BeforeScript do
           .to include /should be an array of strings/
       end
     end
+
+    describe '#invalid?' do
+      it 'is not valid' do
+        expect(entry).to be_invalid
+      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 66d40be6e6..74a64c6df9 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -35,6 +35,12 @@ describe Gitlab::Ci::Config::Node::Global do
       end
     end
 
+    describe '#has_config?' do
+      it 'has config' do
+        expect(global).to have_config
+      end
+    end
+
     describe '#leaf?' do
       it 'is not leaf' do
         expect(global).not_to be_leaf
@@ -59,6 +65,12 @@ describe Gitlab::Ci::Config::Node::Global do
       end
     end
 
+    describe '#invalid?' do
+      it 'is not valid' do
+        expect(global).to be_invalid
+      end
+    end
+
     describe '#errors' do
       it 'reports errors from child nodes' do
         expect(global.errors)

From 48a59c1a8baf3921f26c8503a9fdd63bf7398f0f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 8 Jun 2016 13:22:39 +0200
Subject: [PATCH 18/56] Rename BeforeScript to Script in new Ci config

---
 lib/gitlab/ci/config/node/global.rb                             | 2 +-
 lib/gitlab/ci/config/node/{before_script.rb => script.rb}       | 2 +-
 spec/lib/gitlab/ci/config/node/global_spec.rb                   | 2 +-
 .../ci/config/node/{before_script_spec.rb => script_spec.rb}    | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
 rename lib/gitlab/ci/config/node/{before_script.rb => script.rb} (93%)
 rename spec/lib/gitlab/ci/config/node/{before_script_spec.rb => script_spec.rb} (94%)

diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index cfa506c28b..2e899b0b2a 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -3,7 +3,7 @@ module Gitlab
     class Config
       module Node
         class Global < Entry
-          add_node :before_script, BeforeScript
+          add_node :before_script, Script
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/before_script.rb b/lib/gitlab/ci/config/node/script.rb
similarity index 93%
rename from lib/gitlab/ci/config/node/before_script.rb
rename to lib/gitlab/ci/config/node/script.rb
index be2ceebf3f..db635f6541 100644
--- a/lib/gitlab/ci/config/node/before_script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -2,7 +2,7 @@ module Gitlab
   module Ci
     class Config
       module Node
-        class BeforeScript < Entry
+        class Script < Entry
           include ValidationHelpers
 
           def description
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 74a64c6df9..ecfd60b273 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -31,7 +31,7 @@ describe Gitlab::Ci::Config::Node::Global do
 
       it 'creates node object using valid class' do
         expect(global.nodes.first)
-          .to be_an_instance_of Gitlab::Ci::Config::Node::BeforeScript
+          .to be_an_instance_of Gitlab::Ci::Config::Node::Script
       end
     end
 
diff --git a/spec/lib/gitlab/ci/config/node/before_script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
similarity index 94%
rename from spec/lib/gitlab/ci/config/node/before_script_spec.rb
rename to spec/lib/gitlab/ci/config/node/script_spec.rb
index b506b9743c..0af97bab16 100644
--- a/spec/lib/gitlab/ci/config/node/before_script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Ci::Config::Node::BeforeScript do
+describe Gitlab::Ci::Config::Node::Script do
   let(:entry) { described_class.new(value, double)}
   before { entry.validate! }
 

From 33cd090b93714e147e59195d24918e8b7c6d4614 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 9 Jun 2016 10:08:49 +0200
Subject: [PATCH 19/56] Move new Ci config configurable DSL to concern

---
 lib/gitlab/ci/config/node/configurable.rb | 39 +++++++++++++++++++++++
 lib/gitlab/ci/config/node/entry.rb        | 34 +++++---------------
 lib/gitlab/ci/config/node/global.rb       |  2 ++
 3 files changed, 49 insertions(+), 26 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/configurable.rb

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
new file mode 100644
index 0000000000..9c04a1cdc0
--- /dev/null
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -0,0 +1,39 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        module Configurable
+          extend ActiveSupport::Concern
+
+          def keys
+            self.class.nodes || {}
+          end
+
+          private
+
+          def add_node(key, entry_class)
+            if @value.has_key?(key)
+              entry = entry_class.new(@value[key], @root, self)
+            else
+              entry = Node::Null.new(nil, @root, self)
+            end
+
+            @nodes[key] = entry
+          end
+
+          class_methods do
+            attr_reader :nodes
+
+            private
+
+            def add_node(symbol, entry_class)
+              node = { symbol.to_sym => entry_class }
+
+              (@nodes ||= {}).merge!(node)
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 302cded664..c45744efdf 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -20,8 +20,8 @@ module Gitlab
           def process!
             return if leaf? || invalid?
 
-            keys.each do |key, entry_class|
-              add_node(key, entry_class)
+            keys.each do |key, entry|
+              add_node(key, entry)
             end
 
             nodes.each(&:process!)
@@ -49,7 +49,7 @@ module Gitlab
           end
 
           def keys
-            self.class.nodes || {}
+            {}
           end
 
           def errors
@@ -60,7 +60,11 @@ module Gitlab
             super unless keys.has_key?(name)
             raise InvalidError unless valid?
 
-            @nodes[name].value
+            @nodes[name].try(:value)
+          end
+
+          def add_node(key, entry)
+            raise NotImplementedError
           end
 
           def value
@@ -74,28 +78,6 @@ module Gitlab
           def description
             raise NotImplementedError
           end
-
-          private
-
-          def add_node(key, entry_class)
-            if @value.has_key?(key)
-              entry = entry_class.new(@value[key], @root, self)
-            else
-              entry = Node::Null.new(nil, @root, self)
-            end
-
-            @nodes[key] = entry
-          end
-
-          class << self
-            attr_reader :nodes
-
-            private
-
-            def add_node(symbol, entry_class)
-              (@nodes ||= {}).merge!(symbol.to_sym => entry_class)
-            end
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 2e899b0b2a..5a176ab5ea 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -3,6 +3,8 @@ module Gitlab
     class Config
       module Node
         class Global < Entry
+          include Configurable
+
           add_node :before_script, Script
         end
       end

From d9d5042fd9edf2abd662566ddc4c65b6a9bdbb08 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 9 Jun 2016 10:28:44 +0200
Subject: [PATCH 20/56] Extract method that composes new Ci config entry

---
 lib/gitlab/ci/config/node/entry.rb            | 10 +++++--
 spec/lib/gitlab/ci/config/node/global_spec.rb | 29 ++++++++++++++++---
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index c45744efdf..bdef2af9ae 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -20,14 +20,18 @@ module Gitlab
           def process!
             return if leaf? || invalid?
 
-            keys.each do |key, entry|
-              add_node(key, entry)
-            end
+            compose!
 
             nodes.each(&:process!)
             nodes.each(&:validate!)
           end
 
+          def compose!
+            keys.each do |key, entry|
+              add_node(key, entry)
+            end
+          end
+
           def nodes
             @nodes.values
           end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index ecfd60b273..606750648d 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -3,8 +3,6 @@ require 'spec_helper'
 describe Gitlab::Ci::Config::Node::Global do
   let(:global) { described_class.new(hash) }
 
-  before { global.process! }
-
   describe '#keys' do
     it 'can contain global config keys' do
       expect(global.keys).to include :before_script
@@ -20,7 +18,18 @@ describe Gitlab::Ci::Config::Node::Global do
       { before_script: ['ls', 'pwd'] }
     end
 
+    describe '#compose!' do
+      before { global.compose! }
+
+      it 'instantiates entry nodes' do
+        expect(global.nodes.first)
+          .to be_an_instance_of Gitlab::Ci::Config::Node::Script
+      end
+    end
+
     describe '#process!' do
+      before { global.process! }
+
       it 'creates nodes hash' do
         expect(global.nodes).to be_an Array
       end
@@ -48,13 +57,25 @@ describe Gitlab::Ci::Config::Node::Global do
     end
 
     describe '#before_script' do
-      it 'returns correct script' do
-        expect(global.before_script).to eq "ls\npwd"
+      context 'when processed' do
+        before { global.process! }
+
+        it 'returns correct script' do
+          expect(global.before_script).to eq "ls\npwd"
+        end
+      end
+
+      context 'when not processed' do
+        it 'returns nil' do
+          expect(global.before_script).to be nil
+        end
       end
     end
   end
 
   context 'when hash is not valid' do
+    before { global.process! }
+
     let(:hash) do
       { before_script: 'ls' }
     end

From 6a319fd28790228295de19d8c786d1a807f73376 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 9 Jun 2016 10:53:56 +0200
Subject: [PATCH 21/56] Make it possible configure Ci entry description

---
 lib/gitlab/ci/config/node/configurable.rb     | 23 ++++++++++++-------
 lib/gitlab/ci/config/node/entry.rb            |  6 ++---
 lib/gitlab/ci/config/node/global.rb           |  3 ++-
 lib/gitlab/ci/config/node/script.rb           |  4 ----
 spec/lib/gitlab/ci/config/node/global_spec.rb |  5 ++++
 5 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 9c04a1cdc0..4b33fe025b 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -11,23 +11,30 @@ module Gitlab
 
           private
 
-          def add_node(key, entry_class)
-            if @value.has_key?(key)
-              entry = entry_class.new(@value[key], @root, self)
-            else
-              entry = Node::Null.new(nil, @root, self)
-            end
+          def add_node(key, metadata)
+            entry = create_entry(key, metadata[:class])
+            entry.description = metadata[:description]
 
             @nodes[key] = entry
           end
 
+          def create_entry(key, entry_class)
+            if @value.has_key?(key)
+              entry_class.new(@value[key], @root, self)
+            else
+              Node::Null.new(nil, @root, self)
+            end
+          end
+
           class_methods do
             attr_reader :nodes
 
             private
 
-            def add_node(symbol, entry_class)
-              node = { symbol.to_sym => entry_class }
+            def add_node(symbol, entry_class, metadata)
+              node = { symbol.to_sym =>
+                       { class: entry_class,
+                         description: metadata[:description] } }
 
               (@nodes ||= {}).merge!(node)
             end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index bdef2af9ae..bbe07d68b3 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -5,6 +5,8 @@ module Gitlab
         class Entry
           class InvalidError < StandardError; end
 
+          attr_accessor :description
+
           def initialize(value, root = nil, parent = nil)
             @value = value
             @root = root
@@ -78,10 +80,6 @@ module Gitlab
           def validate!
             raise NotImplementedError
           end
-
-          def description
-            raise NotImplementedError
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 5a176ab5ea..7411f8c863 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -5,7 +5,8 @@ module Gitlab
         class Global < Entry
           include Configurable
 
-          add_node :before_script, Script
+          add_node :before_script, Script,
+            description: 'Script that will be executed before each job.'
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index db635f6541..34d18ad278 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -5,10 +5,6 @@ module Gitlab
         class Script < Entry
           include ValidationHelpers
 
-          def description
-            'Script that is executed before the one defined in a job.'
-          end
-
           def value
             @value.join("\n")
           end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 606750648d..9cbd62cbf6 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -42,6 +42,11 @@ describe Gitlab::Ci::Config::Node::Global do
         expect(global.nodes.first)
           .to be_an_instance_of Gitlab::Ci::Config::Node::Script
       end
+
+      it 'sets correct description for nodes' do
+        expect(global.nodes.first.description)
+          .to eq 'Script that will be executed before each job.'
+      end
     end
 
     describe '#has_config?' do

From 20ccd4465b0fbba45839256af93cf36c7b45d4e9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 9 Jun 2016 12:35:24 +0200
Subject: [PATCH 22/56] Do not require Ci config node to have a hash value

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

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 4b33fe025b..e0a0b40fc6 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -5,6 +5,14 @@ module Gitlab
         module Configurable
           extend ActiveSupport::Concern
 
+          def initialize(*)
+            super
+
+            unless leaf? || has_config?
+              @errors << 'should be a configuration entry with hash value'
+            end
+          end
+
           def keys
             self.class.nodes || {}
           end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index bbe07d68b3..6b59461a58 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -13,10 +13,6 @@ module Gitlab
             @parent = parent
             @nodes = {}
             @errors = []
-
-            unless leaf? || has_config?
-              @errors << 'should be a configuration entry with hash value'
-            end
           end
 
           def process!

From 99ee39bf6c21eef8cebc431fb79286d5347d1d21 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 9 Jun 2016 13:01:19 +0200
Subject: [PATCH 23/56] Add comments to new CI config classes and modules

---
 lib/gitlab/ci/config.rb                   |  5 +++--
 lib/gitlab/ci/config/node/configurable.rb | 11 +++++++++++
 lib/gitlab/ci/config/node/entry.rb        |  3 +++
 lib/gitlab/ci/config/node/global.rb       |  4 ++++
 lib/gitlab/ci/config/node/null.rb         |  6 ++++++
 lib/gitlab/ci/config/node/script.rb       |  8 ++++++++
 6 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index 2d02036af1..b48d3592f1 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -1,8 +1,9 @@
 module Gitlab
   module Ci
+    ##
+    # Base GitLab CI Configuration facade
+    #
     class Config
-      class LoaderError < StandardError; end
-
       delegate :valid?, :errors, to: :@global
 
       ##
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index e0a0b40fc6..d3ed72649b 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -2,6 +2,17 @@ module Gitlab
   module Ci
     class Config
       module Node
+        ##
+        # This mixin is responsible for adding DSL, which purpose is to
+        # simplifly process of adding child nodes.
+        #
+        # This can be used only if parent node is a configuration entry that
+        # holds a hash as a configuration value, for example:
+        #
+        # job:
+        #   script: ...
+        #   artifacts: ...
+        #
         module Configurable
           extend ActiveSupport::Concern
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 6b59461a58..7d7e6f26cb 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -2,6 +2,9 @@ module Gitlab
   module Ci
     class Config
       module Node
+        ##
+        # Base abstract class for each configuration entry node.
+        #
         class Entry
           class InvalidError < StandardError; end
 
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 7411f8c863..911dc51da4 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -2,6 +2,10 @@ module Gitlab
   module Ci
     class Config
       module Node
+        ##
+        # This class represents a global entry - root node for entire
+        # GitLab CI Configuration file.
+        #
         class Global < Entry
           include Configurable
 
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index bf8bc62dc9..ab7b0abaf2 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -1,6 +1,12 @@
 module Gitlab
   module Ci
     class Config
+      ##
+      # This class represents a configuration entry that is not being used
+      # in configuration file.
+      #
+      # This implements Null Object pattern.
+      #
       module Node
         class Null < Entry
           def value
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index 34d18ad278..84f9ec0eb0 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -2,6 +2,14 @@ module Gitlab
   module Ci
     class Config
       module Node
+        ##
+        # Entry that represents a script.
+        #
+        # Each element in the value array is a command that will be executed
+        # by GitLab Runner. Currently we concatenate this commands with
+        # new line character as a separator what is compatbile with
+        # implementation in Runner.
+        #
         class Script < Entry
           include ValidationHelpers
 

From d7e125116124b9c08c27b4a02f4738619db1d2f5 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 9 Jun 2016 14:59:59 +0200
Subject: [PATCH 24/56] Rename method that returns allowed nodes in Ci config

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

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index d3ed72649b..cf065c7f6f 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -24,7 +24,7 @@ module Gitlab
             end
           end
 
-          def keys
+          def allowed_nodes
             self.class.nodes || {}
           end
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 7d7e6f26cb..19fc997297 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -28,7 +28,7 @@ module Gitlab
           end
 
           def compose!
-            keys.each do |key, entry|
+            allowed_nodes.each do |key, entry|
               add_node(key, entry)
             end
           end
@@ -46,23 +46,23 @@ module Gitlab
           end
 
           def leaf?
-            keys.none?
+            allowed_nodes.none?
           end
 
           def has_config?
             @value.is_a?(Hash)
           end
 
-          def keys
-            {}
-          end
-
           def errors
             @errors + nodes.map(&:errors).flatten
           end
 
+          def allowed_nodes
+            {}
+          end
+
           def method_missing(name, *args)
-            super unless keys.has_key?(name)
+            super unless allowed_nodes.has_key?(name)
             raise InvalidError unless valid?
 
             @nodes[name].try(:value)
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 9cbd62cbf6..1a51528336 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -3,13 +3,13 @@ require 'spec_helper'
 describe Gitlab::Ci::Config::Node::Global do
   let(:global) { described_class.new(hash) }
 
-  describe '#keys' do
+  describe '#allowed_nodes' do
     it 'can contain global config keys' do
-      expect(global.keys).to include :before_script
+      expect(global.allowed_nodes).to include :before_script
     end
 
     it 'returns a hash' do
-      expect(global.keys).to be_a Hash
+      expect(global.allowed_nodes).to be_a Hash
     end
   end
 

From 828a15bccd5a6fe0471e97ebd5c0c0f6f674b9b7 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 10 Jun 2016 10:49:47 +0200
Subject: [PATCH 25/56] Rename method used to allow node in Ci config

---
 lib/gitlab/ci/config/node/configurable.rb | 8 ++++----
 lib/gitlab/ci/config/node/global.rb       | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index cf065c7f6f..c8c917f229 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -25,7 +25,7 @@ module Gitlab
           end
 
           def allowed_nodes
-            self.class.nodes || {}
+            self.class.allowed_nodes || {}
           end
 
           private
@@ -46,16 +46,16 @@ module Gitlab
           end
 
           class_methods do
-            attr_reader :nodes
+            attr_reader :allowed_nodes
 
             private
 
-            def add_node(symbol, entry_class, metadata)
+            def allow_node(symbol, entry_class, metadata)
               node = { symbol.to_sym =>
                        { class: entry_class,
                          description: metadata[:description] } }
 
-              (@nodes ||= {}).merge!(node)
+              (@allowed_nodes ||= {}).merge!(node)
             end
           end
         end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 911dc51da4..044603423d 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -9,7 +9,7 @@ module Gitlab
         class Global < Entry
           include Configurable
 
-          add_node :before_script, Script,
+          allow_node :before_script, Script,
             description: 'Script that will be executed before each job.'
         end
       end

From 12080ba150328963987674d282f435fc0e88b9d6 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 10 Jun 2016 11:20:46 +0200
Subject: [PATCH 26/56] Simplify new ci config entry class interface

---
 lib/gitlab/ci/config/node/configurable.rb     |  6 +-
 lib/gitlab/ci/config/node/entry.rb            | 15 +----
 lib/gitlab/ci/config/node/null.rb             | 12 ++--
 lib/gitlab/ci/config/node/script.rb           |  4 +-
 spec/lib/gitlab/ci/config/node/global_spec.rb | 12 ----
 spec/lib/gitlab/ci/config/node/null_spec.rb   |  2 +-
 spec/lib/gitlab/ci/config/node/script_spec.rb | 65 ++++++++++---------
 7 files changed, 49 insertions(+), 67 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index c8c917f229..120457690d 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -19,7 +19,7 @@ module Gitlab
           def initialize(*)
             super
 
-            unless leaf? || has_config?
+            unless @value.is_a?(Hash)
               @errors << 'should be a configuration entry with hash value'
             end
           end
@@ -39,9 +39,9 @@ module Gitlab
 
           def create_entry(key, entry_class)
             if @value.has_key?(key)
-              entry_class.new(@value[key], @root, self)
+              entry_class.new(@value[key])
             else
-              Node::Null.new(nil, @root, self)
+              Node::Null.new(nil)
             end
           end
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 19fc997297..ed1cdd6f15 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -10,16 +10,15 @@ module Gitlab
 
           attr_accessor :description
 
-          def initialize(value, root = nil, parent = nil)
+          def initialize(value)
             @value = value
-            @root = root
-            @parent = parent
             @nodes = {}
             @errors = []
           end
 
           def process!
-            return if leaf? || invalid?
+            return if leaf?
+            return unless valid?
 
             compose!
 
@@ -41,18 +40,10 @@ module Gitlab
             errors.none?
           end
 
-          def invalid?
-            !valid?
-          end
-
           def leaf?
             allowed_nodes.none?
           end
 
-          def has_config?
-            @value.is_a?(Hash)
-          end
-
           def errors
             @errors + nodes.map(&:errors).flatten
           end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index ab7b0abaf2..4f590f6bec 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -1,13 +1,13 @@
 module Gitlab
   module Ci
     class Config
-      ##
-      # This class represents a configuration entry that is not being used
-      # in configuration file.
-      #
-      # This implements Null Object pattern.
-      #
       module Node
+        ##
+        # This class represents a configuration entry that is not being used
+        # in configuration file.
+        #
+        # This implements Null Object pattern.
+        #
         class Null < Entry
           def value
             nil
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index 84f9ec0eb0..5072bf0db7 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -6,8 +6,8 @@ module Gitlab
         # Entry that represents a script.
         #
         # Each element in the value array is a command that will be executed
-        # by GitLab Runner. Currently we concatenate this commands with
-        # new line character as a separator what is compatbile with
+        # by GitLab Runner. Currently we concatenate these commands with
+        # new line character as a separator, what is compatible with
         # implementation in Runner.
         #
         class Script < Entry
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 1a51528336..2227fcec63 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -49,12 +49,6 @@ describe Gitlab::Ci::Config::Node::Global do
       end
     end
 
-    describe '#has_config?' do
-      it 'has config' do
-        expect(global).to have_config
-      end
-    end
-
     describe '#leaf?' do
       it 'is not leaf' do
         expect(global).not_to be_leaf
@@ -91,12 +85,6 @@ describe Gitlab::Ci::Config::Node::Global do
       end
     end
 
-    describe '#invalid?' do
-      it 'is not valid' do
-        expect(global).to be_invalid
-      end
-    end
-
     describe '#errors' do
       it 'reports errors from child nodes' do
         expect(global.errors)
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb
index fa75bdcaa6..fb6c3b5cbc 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(:entry) { described_class.new(double, double) }
+  let(:entry) { described_class.new(nil) }
 
   describe '#leaf?' do
     it 'is leaf node' do
diff --git a/spec/lib/gitlab/ci/config/node/script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
index 0af97bab16..e4d6481f8a 100644
--- a/spec/lib/gitlab/ci/config/node/script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -1,44 +1,47 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Script do
-  let(:entry) { described_class.new(value, double)}
-  before { entry.validate! }
+  let(:entry) { described_class.new(value) }
 
-  context 'when entry value is correct' do
-    let(:value) { ['ls', 'pwd'] }
+  describe '#validate!' do
+    before { entry.validate! }
 
-    describe '#value' do
-      it 'returns concatenated command' do
-        expect(entry.value).to eq "ls\npwd"
+    context 'when entry value is correct' do
+      let(:value) { ['ls', 'pwd'] }
+
+      describe '#value' do
+        it 'returns concatenated command' do
+          expect(entry.value).to eq "ls\npwd"
+        end
+      end
+
+      describe '#errors' do
+        it 'does not append errors' do
+          expect(entry.errors).to be_empty
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
       end
     end
 
-    describe '#errors' do
-      it 'does not append errors' do
-        expect(entry.errors).to be_empty
+    context 'when entry value is not correct' do
+      let(:value) { 'ls' }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include /should be an array of strings/
+        end
       end
-    end
 
-    describe '#has_config?' do
-      it 'does not have config' do
-        expect(entry).not_to have_config
-      end
-    end
-  end
-
-  context 'when entry value is not correct' do
-    let(:value) { 'ls' }
-
-    describe '#errors' do
-      it 'saves errors' do
-        expect(entry.errors)
-          .to include /should be an array of strings/
-      end
-    end
-
-    describe '#invalid?' do
-      it 'is not valid' do
-        expect(entry).to be_invalid
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
       end
     end
   end

From 5abfc7fa7157e876299d1675f1cc96b78a3feadc Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 10 Jun 2016 11:32:49 +0200
Subject: [PATCH 27/56] Define ci entry accessor instead of method_missing

---
 lib/gitlab/ci/config/node/configurable.rb | 6 ++++++
 lib/gitlab/ci/config/node/entry.rb        | 7 -------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 120457690d..b72bc0d592 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -55,6 +55,12 @@ module Gitlab
                        { class: entry_class,
                          description: metadata[:description] } }
 
+              define_method(symbol) do
+                raise Entry::InvalidError unless valid?
+
+                @nodes[symbol].try(:value)
+              end
+
               (@allowed_nodes ||= {}).merge!(node)
             end
           end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index ed1cdd6f15..f7649784c2 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -52,13 +52,6 @@ module Gitlab
             {}
           end
 
-          def method_missing(name, *args)
-            super unless allowed_nodes.has_key?(name)
-            raise InvalidError unless valid?
-
-            @nodes[name].try(:value)
-          end
-
           def add_node(key, entry)
             raise NotImplementedError
           end

From cc373a35504bc1f92f1a040c87a712a6480757ec Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 10 Jun 2016 14:01:07 +0200
Subject: [PATCH 28/56] Add factory for fabricating new ci config nodes

---
 lib/gitlab/ci/config/node/configurable.rb     | 24 +++------
 lib/gitlab/ci/config/node/entry.rb            | 10 ++--
 lib/gitlab/ci/config/node/factory.rb          | 44 +++++++++++++++++
 .../lib/gitlab/ci/config/node/factory_spec.rb | 49 +++++++++++++++++++
 4 files changed, 106 insertions(+), 21 deletions(-)
 create mode 100644 lib/gitlab/ci/config/node/factory.rb
 create mode 100644 spec/lib/gitlab/ci/config/node/factory_spec.rb

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index b72bc0d592..650c6efba6 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -30,19 +30,10 @@ module Gitlab
 
           private
 
-          def add_node(key, metadata)
-            entry = create_entry(key, metadata[:class])
-            entry.description = metadata[:description]
-
-            @nodes[key] = entry
-          end
-
-          def create_entry(key, entry_class)
-            if @value.has_key?(key)
-              entry_class.new(@value[key])
-            else
-              Node::Null.new(nil)
-            end
+          def create_node(key, factory)
+            factory.with_value(@value[key])
+            factory.null_node unless @value.has_key?(key)
+            factory.create!
           end
 
           class_methods do
@@ -51,9 +42,8 @@ module Gitlab
             private
 
             def allow_node(symbol, entry_class, metadata)
-              node = { symbol.to_sym =>
-                       { class: entry_class,
-                         description: metadata[:description] } }
+              factory = Node::Factory.new(entry_class)
+                .with_description(metadata[:description])
 
               define_method(symbol) do
                 raise Entry::InvalidError unless valid?
@@ -61,7 +51,7 @@ module Gitlab
                 @nodes[symbol].try(:value)
               end
 
-              (@allowed_nodes ||= {}).merge!(node)
+              (@allowed_nodes ||= {}).merge!(symbol => factory)
             end
           end
         end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index f7649784c2..2f327fa9bf 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -27,8 +27,8 @@ module Gitlab
           end
 
           def compose!
-            allowed_nodes.each do |key, entry|
-              add_node(key, entry)
+            allowed_nodes.each do |key, factory|
+              @nodes[key] = create_node(key, factory.dup)
             end
           end
 
@@ -52,7 +52,7 @@ module Gitlab
             {}
           end
 
-          def add_node(key, entry)
+          def validate!
             raise NotImplementedError
           end
 
@@ -60,7 +60,9 @@ module Gitlab
             raise NotImplementedError
           end
 
-          def validate!
+          private
+
+          def create_node(key, factory)
             raise NotImplementedError
           end
         end
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
new file mode 100644
index 0000000000..969af45272
--- /dev/null
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -0,0 +1,44 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Factory class responsible for fabricating node entry objects.
+        #
+        # It uses Fluent Interface pattern to set all necessary attributes.
+        #
+        class Factory
+          class InvalidFactory < StandardError; end
+
+          def initialize(entry_class)
+            @entry_class = entry_class
+            @attributes = {}
+          end
+
+          def with_value(value)
+            @attributes[:value] = value
+            self
+          end
+
+          def with_description(description)
+            @attributes[:description] = description
+            self
+          end
+
+          def null_node
+            @entry_class = Node::Null
+            self
+          end
+
+          def create!
+            raise InvalidFactory unless @attributes.has_key?(:value)
+
+            @entry_class.new(@attributes[:value]).tap do |entry|
+              entry.description = @attributes[:description]
+            end
+          end
+        end
+      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
new file mode 100644
index 0000000000..73d760d1b0
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -0,0 +1,49 @@
+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 }
+
+    context 'when value setting value' do
+      it 'creates entry with valid value' do
+        entry = factory
+          .with_value(['ls', 'pwd'])
+          .create!
+
+        expect(entry.value).to eq "ls\npwd"
+      end
+
+      context 'when setting description' do
+        it 'creates entry with description' do
+          entry = factory
+            .with_value(['ls', 'pwd'])
+            .with_description('test description')
+            .create!
+
+          expect(entry.value).to eq "ls\npwd"
+          expect(entry.description).to eq 'test description'
+        end
+      end
+    end
+
+    context 'when not setting value' do
+      it 'raises error' do
+        expect { factory.create! }.to raise_error(
+          Gitlab::Ci::Config::Node::Factory::InvalidFactory
+        )
+      end
+    end
+
+    context 'when creating a null entry' do
+      it 'creates a null entry' do
+        entry = factory
+          .with_value(nil)
+          .null_node
+          .create!
+
+        expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Null
+      end
+    end
+  end
+end

From 7c8f3b0cfc38838755a21641e402b3ef7a1f9d0b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 13 Jun 2016 08:50:12 +0200
Subject: [PATCH 29/56] Duplicate CI config node factory on class level

---
 lib/gitlab/ci/config/node/configurable.rb     |  4 ++-
 lib/gitlab/ci/config/node/entry.rb            |  6 ++--
 .../ci/config/node/configurable_spec.rb       | 35 +++++++++++++++++++
 3 files changed, 41 insertions(+), 4 deletions(-)
 create mode 100644 spec/lib/gitlab/ci/config/node/configurable_spec.rb

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 650c6efba6..f2383e07aa 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -37,7 +37,9 @@ module Gitlab
           end
 
           class_methods do
-            attr_reader :allowed_nodes
+            def allowed_nodes
+              Hash[@allowed_nodes.map { |key, factory| [key, factory.dup] } ]
+            end
 
             private
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 2f327fa9bf..e5692e7294 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -27,8 +27,8 @@ module Gitlab
           end
 
           def compose!
-            allowed_nodes.each do |key, factory|
-              @nodes[key] = create_node(key, factory.dup)
+            allowed_nodes.each do |key, essence|
+              @nodes[key] = create_node(key, essence)
             end
           end
 
@@ -62,7 +62,7 @@ module Gitlab
 
           private
 
-          def create_node(key, factory)
+          def create_node(key, essence)
             raise NotImplementedError
           end
         end
diff --git a/spec/lib/gitlab/ci/config/node/configurable_spec.rb b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
new file mode 100644
index 0000000000..47c68f96dc
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
@@ -0,0 +1,35 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Configurable do
+  let(:node) { Class.new }
+
+  before do
+    node.include(described_class)
+  end
+
+  describe 'allowed nodes' do
+    before do
+      node.class_eval do
+        allow_node :object, Object, description: 'test object'
+      end
+    end
+
+    describe '#allowed_nodes' do
+      it 'has valid allowed nodes' do
+        expect(node.allowed_nodes).to include :object
+      end
+
+      it 'creates a node factory' do
+        expect(node.allowed_nodes[:object])
+          .to be_an_instance_of Gitlab::Ci::Config::Node::Factory
+      end
+
+      it 'returns a duplicated factory object' do
+        first_factory = node.allowed_nodes[:object]
+        second_factory = node.allowed_nodes[:object]
+
+        expect(first_factory).not_to be_equal(second_factory)
+      end
+    end
+  end
+end

From 11c0d022835cafc1d52e18580d0e1523a83bbdd2 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 13 Jun 2016 09:14:23 +0200
Subject: [PATCH 30/56] Simplify ci config node factory

---
 lib/gitlab/ci/config/node/configurable.rb      |  6 +++---
 lib/gitlab/ci/config/node/factory.rb           | 11 +++--------
 spec/lib/gitlab/ci/config/node/factory_spec.rb | 10 +++++-----
 3 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index f2383e07aa..86cc33e11b 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -31,8 +31,8 @@ module Gitlab
           private
 
           def create_node(key, factory)
-            factory.with_value(@value[key])
-            factory.null_node unless @value.has_key?(key)
+            factory.with(value: @value[key])
+            factory.nullify! unless @value.has_key?(key)
             factory.create!
           end
 
@@ -45,7 +45,7 @@ module Gitlab
 
             def allow_node(symbol, entry_class, metadata)
               factory = Node::Factory.new(entry_class)
-                .with_description(metadata[:description])
+                .with(description: metadata[:description])
 
               define_method(symbol) do
                 raise Entry::InvalidError unless valid?
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 969af45272..787ca006f5 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -15,17 +15,12 @@ module Gitlab
             @attributes = {}
           end
 
-          def with_value(value)
-            @attributes[:value] = value
+          def with(attributes)
+            @attributes.merge!(attributes)
             self
           end
 
-          def with_description(description)
-            @attributes[:description] = description
-            self
-          end
-
-          def null_node
+          def nullify!
             @entry_class = Node::Null
             self
           end
diff --git a/spec/lib/gitlab/ci/config/node/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb
index 73d760d1b0..d681aa3245 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -8,7 +8,7 @@ describe Gitlab::Ci::Config::Node::Factory do
     context 'when value setting value' do
       it 'creates entry with valid value' do
         entry = factory
-          .with_value(['ls', 'pwd'])
+          .with(value: ['ls', 'pwd'])
           .create!
 
         expect(entry.value).to eq "ls\npwd"
@@ -17,8 +17,8 @@ describe Gitlab::Ci::Config::Node::Factory do
       context 'when setting description' do
         it 'creates entry with description' do
           entry = factory
-            .with_value(['ls', 'pwd'])
-            .with_description('test description')
+            .with(value: ['ls', 'pwd'])
+            .with(description: 'test description')
             .create!
 
           expect(entry.value).to eq "ls\npwd"
@@ -38,8 +38,8 @@ describe Gitlab::Ci::Config::Node::Factory do
     context 'when creating a null entry' do
       it 'creates a null entry' do
         entry = factory
-          .with_value(nil)
-          .null_node
+          .with(value: nil)
+          .nullify!
           .create!
 
         expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Null

From 03d2bf141cde7bb12f88f25bcb08a612e65044c4 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Mon, 13 Jun 2016 13:06:40 +0100
Subject: [PATCH 31/56] Fix description and GFM pipelines conflicting

Consider this command:

    bundle exec rails r "include GitlabMarkdownHelper
    puts markdown('<span>this is a span</span>', pipeline: :description)
    puts markdown('<span>this is a span</span>')"

And the same in the opposite order:

    bundle exec rails r "include GitlabMarkdownHelper
    puts markdown('<span>this is a span</span>')
    puts markdown('<span>this is a span</span>', pipeline: :description)"

Before this change, they would both output:

    <p><span>this is a span</span></p>
    <p>this is a span</p>

That's because `span` is added to the list of whitelisted elements in
the `SanitizationFilter`, but this method tries not to make the same
changes multiple times. Unfortunately,
`HTML::Pipeline::SanitizationFilter::LIMITED`, which is used by the
`DescriptionPipeline`, uses the same Ruby objects for all of its hash
values _except_ `:elements`.

That means that whichever of `DescriptionPipeline` and `GfmPipeline` is
called first would have `span` in its whitelisted elements, and the
second wouldn't.

Fix this by creating an entirely separate hash, before either pipeline
is invoked.
---
 lib/banzai/pipeline/description_pipeline.rb | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/lib/banzai/pipeline/description_pipeline.rb b/lib/banzai/pipeline/description_pipeline.rb
index f239586765..042fb2e6e1 100644
--- a/lib/banzai/pipeline/description_pipeline.rb
+++ b/lib/banzai/pipeline/description_pipeline.rb
@@ -1,23 +1,16 @@
 module Banzai
   module Pipeline
     class DescriptionPipeline < FullPipeline
+      WHITELIST = Banzai::Filter::SanitizationFilter::LIMITED.deep_dup.merge(
+        elements: Banzai::Filter::SanitizationFilter::LIMITED[:elements] - %w(pre code img ol ul li)
+      )
+
       def self.transform_context(context)
         super(context).merge(
           # SanitizationFilter
-          whitelist: whitelist
+          whitelist: WHITELIST
         )
       end
-
-      private
-
-      def self.whitelist
-        # Descriptions are more heavily sanitized, allowing only a few elements.
-        # See http://git.io/vkuAN
-        whitelist = Banzai::Filter::SanitizationFilter::LIMITED
-        whitelist[:elements] -= %w(pre code img ol ul li)
-
-        whitelist
-      end
     end
   end
 end

From 17eb51594d220658686ac25b183b661db0936e6c Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Tue, 7 Jun 2016 21:33:11 -0700
Subject: [PATCH 32/56] Fix some grammar

---
 doc/container_registry/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/container_registry/README.md b/doc/container_registry/README.md
index 4df24ef13c..a7dac54a1a 100644
--- a/doc/container_registry/README.md
+++ b/doc/container_registry/README.md
@@ -82,7 +82,7 @@ Make sure that your GitLab Runner is configured to allow building docker images.
 You have to check the [Using Docker Build documentation](../../ci/docker/using_docker_build.md).
 
 You can use [docker:dind](https://hub.docker.com/_/docker/) to build your images,
-and this is how `.gitlab-ci.yml` should look like:
+and this is how your `.gitlab-ci.yml` should look:
 
 ```
  build_image:
@@ -98,7 +98,7 @@ and this is how `.gitlab-ci.yml` should look like:
 
 You have to use the credentials of the special `gitlab-ci-token` user with its
 password stored in `$CI_BUILD_TOKEN` in order to push to the Registry connected
-to your project. This allows you to automated building and deployment of your
+to your project. This allows you to automate building and deployment of your
 Docker images.
 
 ## Limitations

From 4209212bb80c8d92955bcc71fa8e6973b44cf59a Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Tue, 7 Jun 2016 21:33:54 -0700
Subject: [PATCH 33/56] Add docker bind-mount as an option

---
 doc/ci/docker/using_docker_build.md | 78 +++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 10 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index ca52a483a5..f98b2860e2 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -4,14 +4,14 @@ GitLab CI allows you to use Docker Engine to build and test docker-based project
 
 **This also allows to you to use `docker-compose` and other docker-enabled tools.**
 
-This is one of new trends in Continuous Integration/Deployment to:
+This is one of the new trends in Continuous Integration/Deployment to:
 
-1. create application image,
-1. run test against created image,
-1. push image to remote registry,
-1. deploy server from pushed image
+1. create an application image,
+1. run tests against the created image,
+1. push image to a remote registry,
+1. deploy server from the pushed image
 
-It's also useful in case when your application already has the `Dockerfile` that can be used to create and test image:
+It's also useful when your application already has the `Dockerfile` that can be used to create and test an image:
 ```bash
 $ docker build -t my-image dockerfiles/
 $ docker run my-docker-image /script/to/run/tests
@@ -19,10 +19,7 @@ $ docker tag my-image my-registry:5000/my-image
 $ docker push my-registry:5000/my-image
 ```
 
-However, this requires special configuration of GitLab Runner to enable `docker` support during build.
-**This requires running GitLab Runner in privileged mode which can be harmful when untrusted code is run.**
-
-There are two methods to enable the use of `docker build` and `docker run` during build.
+However, this requires special configuration of GitLab Runner to enable `docker` support during builds. There are three methods to enable the use of `docker build` and `docker run` during builds.
 
 ## 1. Use shell executor
 
@@ -150,5 +147,66 @@ In order to do that follow the steps:
 
 An example project using this approach can be found here: https://gitlab.com/gitlab-examples/docker.
 
+## 3. Bind Docker socket
+
+The third approach is to bind-mount `/var/run/docker.sock` into the container so that docker is available in the context of that image.
+
+In order to do that follow the steps:
+
+1. Install [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/#installation).
+
+1. Register GitLab Runner from the command line to use `docker` and `privileged`
+   mode:
+
+    ```bash
+    sudo gitlab-runner register -n \
+      --url https://gitlab.com/ci \
+      --token RUNNER_TOKEN \
+      --executor docker \
+      --description "My Docker Runner" \
+      --docker-image "docker:latest" \
+      --docker-volumes /var/run/docker.sock:/var/run/docker.sock
+    ```
+
+    The above command will register a new Runner to use the special
+    `docker:latest` image which is provided by Docker. **Notice that it's using
+    the Docker daemon of the runner itself, and any containers spawned by docker commands will be siblings of the runner rather than children of the runner.** This may have complications and limitations that are unsuitable for your workflow.
+
+    The above command will create a `config.toml` entry similar to this:
+
+    ```
+    [[runners]]
+      url = "https://gitlab.com/ci"
+      token = TOKEN
+      executor = "docker"
+      [runners.docker]
+        tls_verify = false
+        image = "docker:latest"
+        privileged = false
+        disable_cache = false
+        volumes = ["/usr/local/bin/docker:/usr/bin/docker", "/cache"]
+      [runners.cache]
+        Insecure = false
+    ```
+
+1. You can now use `docker` from build script (note that you don't need to include the `docker:dind` service as in the option above):
+
+    ```yaml
+    image: docker:latest
+
+    before_script:
+    - docker info
+
+    build:
+      stage: build
+      script:
+      - docker build -t my-docker-image .
+      - docker run my-docker-image /script/to/run/tests
+    ```
+
+1. However, by sharing the docker daemon, you are effectively disabling all
+   the security mechanisms of containers and exposing your host to privilege
+   escalation which can lead to container breakout.
+
 [docker-in-docker]: https://blog.docker.com/2013/09/docker-can-now-run-within-docker/
 [docker-cap]: https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities

From d7664c7223cbd9e91e21beaf6ceb9ea7c2f294d8 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Tue, 7 Jun 2016 21:34:30 -0700
Subject: [PATCH 34/56] Add example using GitLab Container Registry

---
 doc/ci/docker/using_docker_build.md | 72 +++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index f98b2860e2..fe2c5207cd 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -208,5 +208,77 @@ In order to do that follow the steps:
    the security mechanisms of containers and exposing your host to privilege
    escalation which can lead to container breakout.
 
+## Using the GitLab Container Registry
+
+Once you've built a Docker image, you can push it up to the built-in [GitLab Container Registry](../../container_registry/README.md).
+
+```
+ build:
+   stage: build
+   script:
+     - docker login -u gitlab-ci-token -p $CI_BUILD_TOKEN registry.example.com
+     - docker build -t registry.example.com/group/project:latest .
+     - docker push registry.example.com/group/project:latest
+```
+
+Here's a more elaborate example that splits up the tasks into 4 stages,
+including two tests that run in parallel. The build is stored in the container
+registry and used by subsequent stages, downloading the image
+when needed. Changes to `master` also get tagged as `latest` and deployed using
+an application-specific deploy script:
+
+```yaml
+image: docker:git
+services:
+- docker:dind
+
+stages:
+- build
+- test
+- release
+- deploy
+
+variables:
+  CONTAINER_TEST_IMAGE: registry.example.com/my-group/my-project:$CI_BUILD_REF_NAME
+  CONTAINER_RELEASE_IMAGE: registry.example.com/my-group/my-project:latest
+
+before_script:
+  - docker login -u gitlab-ci-token -p $CI_BUILD_TOKEN registry.example.com
+
+build:
+  stage: build
+  script:
+    - docker build --pull -t $CONTAINER_TEST_IMAGE .
+    - docker push $CONTAINER_TEST_IMAGE
+
+test1:
+  stage: test
+  script:
+    - docker pull $CONTAINER_TEST_IMAGE
+    - docker run $CONTAINER_TEST_IMAGE /script/to/run/tests
+
+test2:
+  stage: test
+  script:
+    - docker pull $CONTAINER_TEST_IMAGE
+    - docker run $CONTAINER_TEST_IMAGE /script/to/run/another/test
+
+release-image:
+  stage: release
+  script:
+    - docker pull $CONTAINER_TEST_IMAGE
+    - docker tag $CONTAINER_TEST_IMAGE $CONTAINER_RELEASE_IMAGE
+    - docker push $CONTAINER_RELEASE_IMAGE
+  only:
+    - master
+
+deploy:
+  stage: deploy
+  script:
+    - ./deploy.sh
+  only:
+    - master
+```
+
 [docker-in-docker]: https://blog.docker.com/2013/09/docker-can-now-run-within-docker/
 [docker-cap]: https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities

From 6841e76b45b44da9f749538dbae2bb1fc63d8ee4 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Tue, 7 Jun 2016 22:09:15 -0700
Subject: [PATCH 35/56] Add notes

---
 doc/ci/docker/using_docker_build.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index fe2c5207cd..a5f3736626 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -280,5 +280,11 @@ deploy:
     - master
 ```
 
+Notes:
+1. You must log in to the container registry before running commands. Putting this in `before_script` will run it before each build job.
+1. Using `docker build --pull` makes sure that Docker fetches any changes to base images before building just in case your cache is stale. It takes slightly longer, but means you don’t get stuck without security patches to base images.
+1. Doing an explicit `docker pull` before each `docker run` makes sure to fetch the latest image that was just built. This is especially important if you are using multiple runners that cache images locally. Using the git SHA in your image tag makes this less necessary since each build will be unique and you shouldn't ever have a stale image, but it's still possible if you re-build a given commit after a dependency has changed.
+1. You don't want to build directly to `latest` in case there are multiple builds happening simultaneously.
+
 [docker-in-docker]: https://blog.docker.com/2013/09/docker-can-now-run-within-docker/
 [docker-cap]: https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities

From d9cbe019866843132225d440754a20da0e937d00 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Tue, 7 Jun 2016 22:25:33 -0700
Subject: [PATCH 36/56] Moar commas

---
 doc/ci/docker/using_docker_build.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index a5f3736626..4620eeac2b 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -76,7 +76,7 @@ The second approach is to use the special Docker image with all tools installed
 (`docker` and `docker-compose`) and run the build script in context of that
 image in privileged mode.
 
-In order to do that follow the steps:
+In order to do that, follow the steps:
 
 1. Install [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/#installation).
 
@@ -151,7 +151,7 @@ An example project using this approach can be found here: https://gitlab.com/git
 
 The third approach is to bind-mount `/var/run/docker.sock` into the container so that docker is available in the context of that image.
 
-In order to do that follow the steps:
+In order to do that, follow the steps:
 
 1. Install [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/#installation).
 

From 84128441081869ff2bd260a92a7c0b43d68ca415 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Tue, 7 Jun 2016 22:26:59 -0700
Subject: [PATCH 37/56] Fix instructions

---
 doc/ci/docker/using_docker_build.md | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 4620eeac2b..62e48a6d8d 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -155,8 +155,7 @@ In order to do that, follow the steps:
 
 1. Install [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/#installation).
 
-1. Register GitLab Runner from the command line to use `docker` and `privileged`
-   mode:
+1. Register GitLab Runner from the command line to use `docker` and share `/var/run/docker.sock`:
 
     ```bash
     sudo gitlab-runner register -n \

From 9b30f26b988f69995d1f548f79020c23dfe1a9ea Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Tue, 7 Jun 2016 23:39:38 -0700
Subject: [PATCH 38/56] Fix runner CLI instructions

---
 doc/ci/docker/using_docker_build.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 62e48a6d8d..3af4afbbef 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -84,9 +84,9 @@ In order to do that, follow the steps:
    mode:
 
     ```bash
-    sudo gitlab-runner register -n \
+    sudo gitlab-ci-multi-runner register -n \
       --url https://gitlab.com/ci \
-      --token RUNNER_TOKEN \
+      --registration-token REGISTRATION_TOKEN \
       --executor docker \
       --description "My Docker Runner" \
       --docker-image "docker:latest" \
@@ -158,9 +158,9 @@ In order to do that, follow the steps:
 1. Register GitLab Runner from the command line to use `docker` and share `/var/run/docker.sock`:
 
     ```bash
-    sudo gitlab-runner register -n \
+    sudo gitlab-ci-multi-runner register -n \
       --url https://gitlab.com/ci \
-      --token RUNNER_TOKEN \
+      --registration-token REGISTRATION_TOKEN \
       --executor docker \
       --description "My Docker Runner" \
       --docker-image "docker:latest" \

From 6ca1370c92dcf074af73562fb0fd613c8af45ce1 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Tue, 7 Jun 2016 23:50:26 -0700
Subject: [PATCH 39/56] Fix more instructions

---
 doc/ci/docker/using_docker_build.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 3af4afbbef..aae3701050 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -24,16 +24,16 @@ However, this requires special configuration of GitLab Runner to enable `docker`
 ## 1. Use shell executor
 
 The simplest approach is to install GitLab Runner in `shell` execution mode.
-GitLab Runner then executes build scripts as `gitlab-runner` user.
+GitLab Runner then executes build scripts as the `gitlab-runner` user.
 
 1. Install [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/#installation).
 
 1. During GitLab Runner installation select `shell` as method of executing build scripts or use command:
 
     ```bash
-    $ sudo gitlab-runner register -n \
+    $ sudo gitlab-ci-multi-runner register -n \
       --url https://gitlab.com/ci \
-      --token RUNNER_TOKEN \
+      --registration-token REGISTRATION_TOKEN \
       --executor shell
       --description "My Runner"
     ```

From db656a3987131816d47897b2424821b19ca147b0 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Wed, 8 Jun 2016 00:28:23 -0700
Subject: [PATCH 40/56] Fix more references to old gitlab-runner

---
 doc/ci/docker/using_docker_images.md | 2 +-
 doc/ci/examples/php.md               | 4 ++--
 doc/ci/runners/README.md             | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/doc/ci/docker/using_docker_images.md b/doc/ci/docker/using_docker_images.md
index 56ac2195c4..a849905ac6 100644
--- a/doc/ci/docker/using_docker_images.md
+++ b/doc/ci/docker/using_docker_images.md
@@ -23,7 +23,7 @@ To use GitLab Runner with docker you need to register a new runner to use the
 `docker` executor:
 
 ```bash
-gitlab-runner register \
+gitlab-ci-multi-runner register \
   --url "https://gitlab.com/" \
   --registration-token "PROJECT_REGISTRATION_TOKEN" \
   --description "docker-ruby-2.1" \
diff --git a/doc/ci/examples/php.md b/doc/ci/examples/php.md
index 2695301450..17e1c64bb8 100644
--- a/doc/ci/examples/php.md
+++ b/doc/ci/examples/php.md
@@ -263,10 +263,10 @@ terminal execute:
 
 ```bash
 # Check using docker executor
-gitlab-runner exec docker test:app
+gitlab-ci-multi-runner exec docker test:app
 
 # Check using shell executor
-gitlab-runner exec shell test:app
+gitlab-ci-multi-runner exec shell test:app
 ```
 
 ## Example project
diff --git a/doc/ci/runners/README.md b/doc/ci/runners/README.md
index b42d7a62eb..400784da61 100644
--- a/doc/ci/runners/README.md
+++ b/doc/ci/runners/README.md
@@ -63,10 +63,10 @@ instance.
 Now simply register the runner as any runner:
 
 ```
-sudo gitlab-runner register
+sudo gitlab-ci-multi-runner register
 ```
 
-Shared runners are enabled by default as of GitLab 8.2, but can be disabled with the 
+Shared runners are enabled by default as of GitLab 8.2, but can be disabled with the
 `DISABLE SHARED RUNNERS` button. Previous versions of GitLab defaulted shared runners to
 disabled.
 
@@ -93,7 +93,7 @@ setup a specific runner for this project.
 To register the runner, run the command below and follow instructions:
 
 ```
-sudo gitlab-runner register
+sudo gitlab-ci-multi-runner register
 ```
 
 ###  Making an existing Shared Runner Specific

From e97af053eb24391df926cb7f7ca20d67a4ff03d0 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Wed, 8 Jun 2016 11:10:06 -0700
Subject: [PATCH 41/56] Fix docker volume

---
 doc/ci/docker/using_docker_build.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index aae3701050..5df1fdd84c 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -183,7 +183,7 @@ In order to do that, follow the steps:
         image = "docker:latest"
         privileged = false
         disable_cache = false
-        volumes = ["/usr/local/bin/docker:/usr/bin/docker", "/cache"]
+        volumes = ["/var/run/docker.sock", "/cache"]
       [runners.cache]
         Insecure = false
     ```

From 46114eddf0a2fc07f932fe45948a48896abbeb78 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Wed, 8 Jun 2016 11:39:31 -0700
Subject: [PATCH 42/56] Add more pros and cons for each docker approach

---
 doc/ci/docker/using_docker_build.md | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 5df1fdd84c..17ba953ca7 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -19,7 +19,7 @@ $ docker tag my-image my-registry:5000/my-image
 $ docker push my-registry:5000/my-image
 ```
 
-However, this requires special configuration of GitLab Runner to enable `docker` support during builds. There are three methods to enable the use of `docker build` and `docker run` during builds.
+However, this requires special configuration of GitLab Runner to enable `docker` support during builds. There are three methods to enable the use of `docker build` and `docker run` during builds; each with their own tradeoffs.
 
 ## 1. Use shell executor
 
@@ -67,7 +67,7 @@ GitLab Runner then executes build scripts as the `gitlab-runner` user.
 
 5. You can now use `docker` command and install `docker-compose` if needed.
 
-6. However, by adding `gitlab-runner` to `docker` group you are effectively granting `gitlab-runner` full root permissions.
+However, by adding `gitlab-runner` to `docker` group you are effectively granting `gitlab-runner` full root permissions.
 For more information please checkout [On Docker security: `docker` group considered harmful](https://www.andreas-jung.com/contents/on-docker-security-docker-group-considered-harmful).
 
 ## 2. Use docker-in-docker executor
@@ -138,12 +138,16 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-1. However, by enabling `--docker-privileged` you are effectively disabling all
-   the security mechanisms of containers and exposing your host to privilege
-   escalation which can lead to container breakout.
+However, by enabling `--docker-privileged` you are effectively disabling all
+the security mechanisms of containers and exposing your host to privilege
+escalation which can lead to container breakout. For more information, check out the official Docker documentation on
+[Runtime privilege and Linux capabilities][docker-cap].
 
-   For more information, check out the official Docker documentation on
-   [Runtime privilege and Linux capabilities][docker-cap].
+Using docker-in-docker, each build is in a clean environment without the past
+history. Concurrent builds work fine because every build get it's own instance of docker engine so they won't conflict with each other. But this also means builds can be slower because there's no caching of layers.
+
+By default `docker:dind` uses ``--storage-driver vfs` which is the slowest form
+offered.
 
 An example project using this approach can be found here: https://gitlab.com/gitlab-examples/docker.
 
@@ -203,9 +207,14 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-1. However, by sharing the docker daemon, you are effectively disabling all
-   the security mechanisms of containers and exposing your host to privilege
-   escalation which can lead to container breakout.
+However, by sharing the docker daemon, you are effectively disabling all
+the security mechanisms of containers and exposing your host to privilege
+escalation which can lead to container breakout. For example, if a project
+ran `docker rm -f $(docker ps -a -q)` it would remove the GitLab Runner
+containers.
+
+Also, concurrent builds may not work; if your tests
+create containers with specific names, they may conflict with each other.
 
 ## Using the GitLab Container Registry
 

From 1c02ef9c144f3a8d40e31a21d82b5628e72d48e6 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Wed, 8 Jun 2016 12:00:17 -0700
Subject: [PATCH 43/56] Drop some 'however's

---
 doc/ci/docker/using_docker_build.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 17ba953ca7..cc820d8114 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -19,7 +19,7 @@ $ docker tag my-image my-registry:5000/my-image
 $ docker push my-registry:5000/my-image
 ```
 
-However, this requires special configuration of GitLab Runner to enable `docker` support during builds. There are three methods to enable the use of `docker build` and `docker run` during builds; each with their own tradeoffs.
+This requires special configuration of GitLab Runner to enable `docker` support during builds. There are three methods to enable the use of `docker build` and `docker run` during builds; each with their own tradeoffs.
 
 ## 1. Use shell executor
 
@@ -67,7 +67,7 @@ GitLab Runner then executes build scripts as the `gitlab-runner` user.
 
 5. You can now use `docker` command and install `docker-compose` if needed.
 
-However, by adding `gitlab-runner` to `docker` group you are effectively granting `gitlab-runner` full root permissions.
+By adding `gitlab-runner` to `docker` group you are effectively granting `gitlab-runner` full root permissions.
 For more information please checkout [On Docker security: `docker` group considered harmful](https://www.andreas-jung.com/contents/on-docker-security-docker-group-considered-harmful).
 
 ## 2. Use docker-in-docker executor
@@ -138,7 +138,7 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-However, by enabling `--docker-privileged` you are effectively disabling all
+By enabling `--docker-privileged` you are effectively disabling all
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For more information, check out the official Docker documentation on
 [Runtime privilege and Linux capabilities][docker-cap].
@@ -207,7 +207,7 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-However, by sharing the docker daemon, you are effectively disabling all
+By sharing the docker daemon, you are effectively disabling all
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For example, if a project
 ran `docker rm -f $(docker ps -a -q)` it would remove the GitLab Runner

From b393478f63ad2f4381996dc08111fc3393bf762e Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Wed, 8 Jun 2016 12:11:44 -0700
Subject: [PATCH 44/56] Refactor notes

---
 doc/ci/docker/using_docker_build.md | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index cc820d8114..5af6d36e83 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -67,7 +67,8 @@ GitLab Runner then executes build scripts as the `gitlab-runner` user.
 
 5. You can now use `docker` command and install `docker-compose` if needed.
 
-By adding `gitlab-runner` to `docker` group you are effectively granting `gitlab-runner` full root permissions.
+Notes:
+* By adding `gitlab-runner` to `docker` group you are effectively granting `gitlab-runner` full root permissions.
 For more information please checkout [On Docker security: `docker` group considered harmful](https://www.andreas-jung.com/contents/on-docker-security-docker-group-considered-harmful).
 
 ## 2. Use docker-in-docker executor
@@ -138,15 +139,16 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-By enabling `--docker-privileged` you are effectively disabling all
+Notes:
+* By enabling `--docker-privileged` you are effectively disabling all
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For more information, check out the official Docker documentation on
 [Runtime privilege and Linux capabilities][docker-cap].
 
-Using docker-in-docker, each build is in a clean environment without the past
+* Using docker-in-docker, each build is in a clean environment without the past
 history. Concurrent builds work fine because every build get it's own instance of docker engine so they won't conflict with each other. But this also means builds can be slower because there's no caching of layers.
 
-By default `docker:dind` uses ``--storage-driver vfs` which is the slowest form
+* By default, `docker:dind` uses ``--storage-driver vfs` which is the slowest form
 offered.
 
 An example project using this approach can be found here: https://gitlab.com/gitlab-examples/docker.
@@ -207,15 +209,21 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-By sharing the docker daemon, you are effectively disabling all
+Notes:
+* By sharing the docker daemon, you are effectively disabling all
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For example, if a project
 ran `docker rm -f $(docker ps -a -q)` it would remove the GitLab Runner
 containers.
 
-Also, concurrent builds may not work; if your tests
+* Concurrent builds may not work; if your tests
 create containers with specific names, they may conflict with each other.
 
+* Sharing files and directories from the source repo into containers may not
+work as expected since volume mounting is done in the context of the host
+machine, not the build container.
+e.g. `docker run --rm -t -i -v $(pwd)/src:/home/app/src test-image:latest run_app_tests`
+
 ## Using the GitLab Container Registry
 
 Once you've built a Docker image, you can push it up to the built-in [GitLab Container Registry](../../container_registry/README.md).

From b0cbeb18d1864ab36fb17c69d963321d745924fa Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Wed, 8 Jun 2016 14:11:15 -0700
Subject: [PATCH 45/56] Remove unnecessary message

---
 doc/ci/docker/using_docker_build.md | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 5af6d36e83..c44b1d7a0c 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -117,10 +117,6 @@ In order to do that, follow the steps:
         Insecure = false
     ```
 
-    If you want to use the Shared Runners available on your GitLab CE/EE
-    installation in order to build Docker images, then make sure that your
-    Shared Runners configuration has the `privileged` mode set to `true`.
-
 1. You can now use `docker` from build script:
 
     ```yaml

From 6f834ecaa94a1da230c933c981b33634d937d8dd Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Wed, 8 Jun 2016 14:17:03 -0700
Subject: [PATCH 46/56] Reformat notes

---
 doc/ci/docker/using_docker_build.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index c44b1d7a0c..697b9f1016 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -67,7 +67,7 @@ GitLab Runner then executes build scripts as the `gitlab-runner` user.
 
 5. You can now use `docker` command and install `docker-compose` if needed.
 
-Notes:
+### Notes
 * By adding `gitlab-runner` to `docker` group you are effectively granting `gitlab-runner` full root permissions.
 For more information please checkout [On Docker security: `docker` group considered harmful](https://www.andreas-jung.com/contents/on-docker-security-docker-group-considered-harmful).
 
@@ -135,7 +135,7 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-Notes:
+### Notes
 * By enabling `--docker-privileged` you are effectively disabling all
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For more information, check out the official Docker documentation on
@@ -205,7 +205,7 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-Notes:
+### Notes
 * By sharing the docker daemon, you are effectively disabling all
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For example, if a project
@@ -292,7 +292,7 @@ deploy:
     - master
 ```
 
-Notes:
+### Notes
 1. You must log in to the container registry before running commands. Putting this in `before_script` will run it before each build job.
 1. Using `docker build --pull` makes sure that Docker fetches any changes to base images before building just in case your cache is stale. It takes slightly longer, but means you don’t get stuck without security patches to base images.
 1. Doing an explicit `docker pull` before each `docker run` makes sure to fetch the latest image that was just built. This is especially important if you are using multiple runners that cache images locally. Using the git SHA in your image tag makes this less necessary since each build will be unique and you shouldn't ever have a stale image, but it's still possible if you re-build a given commit after a dependency has changed.

From 35ce04ef2e02e5b176c57567f2ddf82871af7639 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Wed, 8 Jun 2016 14:40:56 -0700
Subject: [PATCH 47/56] Move registry CI example to CI docs

---
 doc/ci/docker/using_docker_build.md | 22 ++++++++++++++++++----
 doc/container_registry/README.md    | 23 ++---------------------
 2 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 697b9f1016..33b1624d00 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -73,7 +73,8 @@ For more information please checkout [On Docker security: `docker` group conside
 
 ## 2. Use docker-in-docker executor
 
-The second approach is to use the special Docker image with all tools installed
+The second approach is to use the special docker-in-docker (dind)
+[Docker image](https://hub.docker.com/_/docker/) with all tools installed
 (`docker` and `docker-compose`) and run the build script in context of that
 image in privileged mode.
 
@@ -222,10 +223,18 @@ e.g. `docker run --rm -t -i -v $(pwd)/src:/home/app/src test-image:latest run_ap
 
 ## Using the GitLab Container Registry
 
-Once you've built a Docker image, you can push it up to the built-in [GitLab Container Registry](../../container_registry/README.md).
+> **Note:**
+This feature requires GitLab 8.8 and GitLab Runner 1.2.
 
-```
+Once you've built a Docker image, you can push it up to the built-in [GitLab Container Registry](../../container_registry/README.md). For example, if you're using
+docker-in-docker on your runners, this is how your `.gitlab-ci.yml` could look:
+
+
+```yaml
  build:
+   image: docker:git
+   services:
+   - docker:dind
    stage: build
    script:
      - docker login -u gitlab-ci-token -p $CI_BUILD_TOKEN registry.example.com
@@ -233,7 +242,12 @@ Once you've built a Docker image, you can push it up to the built-in [GitLab Con
      - docker push registry.example.com/group/project:latest
 ```
 
-Here's a more elaborate example that splits up the tasks into 4 stages,
+You have to use the credentials of the special `gitlab-ci-token` user with its
+password stored in `$CI_BUILD_TOKEN` in order to push to the Registry connected
+to your project. This allows you to automate building and deployment of your
+Docker images.
+
+Here's a more elaborate example that splits up the tasks into 4 pipeline stages,
 including two tests that run in parallel. The build is stored in the container
 registry and used by subsequent stages, downloading the image
 when needed. Changes to `master` also get tagged as `latest` and deployed using
diff --git a/doc/container_registry/README.md b/doc/container_registry/README.md
index a7dac54a1a..1b46543449 100644
--- a/doc/container_registry/README.md
+++ b/doc/container_registry/README.md
@@ -79,27 +79,8 @@ delete them.
 This feature requires GitLab 8.8 and GitLab Runner 1.2.
 
 Make sure that your GitLab Runner is configured to allow building docker images.
-You have to check the [Using Docker Build documentation](../../ci/docker/using_docker_build.md).
-
-You can use [docker:dind](https://hub.docker.com/_/docker/) to build your images,
-and this is how your `.gitlab-ci.yml` should look:
-
-```
- build_image:
-   image: docker:git
-   services:
-   - docker:dind
-   stage: build
-   script:
-     - docker login -u gitlab-ci-token -p $CI_BUILD_TOKEN registry.example.com
-     - docker build -t registry.example.com/group/project:latest .
-     - docker push registry.example.com/group/project:latest
-```
-
-You have to use the credentials of the special `gitlab-ci-token` user with its
-password stored in `$CI_BUILD_TOKEN` in order to push to the Registry connected
-to your project. This allows you to automate building and deployment of your
-Docker images.
+You have to check the [Using Docker Build documentation](../ci/docker/using_docker_build.md).
+Then see the CI documentation on [Using the GitLab Container Registry](../ci/docker/using_docker_build.md#using-the-gitlab-container-registry).
 
 ## Limitations
 

From a7caea9e3e6b624ada8d3dbabf13c2f9ad79b463 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Wed, 8 Jun 2016 15:41:27 -0700
Subject: [PATCH 48/56] Use docker:latest

---
 doc/ci/docker/using_docker_build.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 33b1624d00..d5bc1d7406 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -232,7 +232,7 @@ docker-in-docker on your runners, this is how your `.gitlab-ci.yml` could look:
 
 ```yaml
  build:
-   image: docker:git
+   image: docker:latest
    services:
    - docker:dind
    stage: build
@@ -254,7 +254,7 @@ when needed. Changes to `master` also get tagged as `latest` and deployed using
 an application-specific deploy script:
 
 ```yaml
-image: docker:git
+image: docker:latest
 services:
 - docker:dind
 

From f95791d4118e7a1ad85ab0f287784c5639182560 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Mon, 13 Jun 2016 22:32:01 -0700
Subject: [PATCH 49/56] Make Achilleas' suggested changes

---
 doc/ci/docker/using_docker_build.md | 56 ++++++++++++++---------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index d5bc1d7406..7729159765 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -4,12 +4,12 @@ GitLab CI allows you to use Docker Engine to build and test docker-based project
 
 **This also allows to you to use `docker-compose` and other docker-enabled tools.**
 
-This is one of the new trends in Continuous Integration/Deployment to:
+One of the new trends in Continuous Integration/Deployment is to:
 
 1. create an application image,
 1. run tests against the created image,
-1. push image to a remote registry,
-1. deploy server from the pushed image
+1. push image to a remote registry, and
+1. deploy to a server from the pushed image.
 
 It's also useful when your application already has the `Dockerfile` that can be used to create and test an image:
 ```bash
@@ -19,9 +19,13 @@ $ docker tag my-image my-registry:5000/my-image
 $ docker push my-registry:5000/my-image
 ```
 
-This requires special configuration of GitLab Runner to enable `docker` support during builds. There are three methods to enable the use of `docker build` and `docker run` during builds; each with their own tradeoffs.
+This requires special configuration of GitLab Runner to enable `docker` support during builds.
 
-## 1. Use shell executor
+## Runner Configuration
+
+There are three methods to enable the use of `docker build` and `docker run` during builds; each with their own tradeoffs.
+
+### Use shell executor
 
 The simplest approach is to install GitLab Runner in `shell` execution mode.
 GitLab Runner then executes build scripts as the `gitlab-runner` user.
@@ -67,11 +71,11 @@ GitLab Runner then executes build scripts as the `gitlab-runner` user.
 
 5. You can now use `docker` command and install `docker-compose` if needed.
 
-### Notes
+> **Note:**
 * By adding `gitlab-runner` to `docker` group you are effectively granting `gitlab-runner` full root permissions.
-For more information please checkout [On Docker security: `docker` group considered harmful](https://www.andreas-jung.com/contents/on-docker-security-docker-group-considered-harmful).
+For more information please check out [On Docker security: `docker` group considered harmful](https://www.andreas-jung.com/contents/on-docker-security-docker-group-considered-harmful).
 
-## 2. Use docker-in-docker executor
+### Use docker-in-docker executor
 
 The second approach is to use the special docker-in-docker (dind)
 [Docker image](https://hub.docker.com/_/docker/) with all tools installed
@@ -118,7 +122,7 @@ In order to do that, follow the steps:
         Insecure = false
     ```
 
-1. You can now use `docker` from build script:
+1. You can now use `docker` in the build script:
 
     ```yaml
     image: docker:latest
@@ -136,21 +140,19 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-### Notes
-* By enabling `--docker-privileged` you are effectively disabling all
+> **Notes:**
+> * By enabling `--docker-privileged`, you are effectively disabling all
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For more information, check out the official Docker documentation on
 [Runtime privilege and Linux capabilities][docker-cap].
-
-* Using docker-in-docker, each build is in a clean environment without the past
-history. Concurrent builds work fine because every build get it's own instance of docker engine so they won't conflict with each other. But this also means builds can be slower because there's no caching of layers.
-
-* By default, `docker:dind` uses ``--storage-driver vfs` which is the slowest form
+> * Using docker-in-docker, each build is in a clean environment without the past
+history. Concurrent builds work fine because every build gets it's own instance of docker engine so they won't conflict with each other. But this also means builds can be slower because there's no caching of layers.
+> * By default, `docker:dind` uses `--storage-driver vfs` which is the slowest form
 offered.
 
 An example project using this approach can be found here: https://gitlab.com/gitlab-examples/docker.
 
-## 3. Bind Docker socket
+### Use Docker socket binding
 
 The third approach is to bind-mount `/var/run/docker.sock` into the container so that docker is available in the context of that image.
 
@@ -172,14 +174,14 @@ In order to do that, follow the steps:
 
     The above command will register a new Runner to use the special
     `docker:latest` image which is provided by Docker. **Notice that it's using
-    the Docker daemon of the runner itself, and any containers spawned by docker commands will be siblings of the runner rather than children of the runner.** This may have complications and limitations that are unsuitable for your workflow.
+    the Docker daemon of the Runner itself, and any containers spawned by docker commands will be siblings of the Runner rather than children of the runner.** This may have complications and limitations that are unsuitable for your workflow.
 
     The above command will create a `config.toml` entry similar to this:
 
     ```
     [[runners]]
       url = "https://gitlab.com/ci"
-      token = TOKEN
+      token = REGISTRATION_TOKEN
       executor = "docker"
       [runners.docker]
         tls_verify = false
@@ -191,7 +193,7 @@ In order to do that, follow the steps:
         Insecure = false
     ```
 
-1. You can now use `docker` from build script (note that you don't need to include the `docker:dind` service as in the option above):
+1. You can now use `docker` in the build script (note that you don't need to include the `docker:dind` service as when using the Docker in Docker executor):
 
     ```yaml
     image: docker:latest
@@ -206,16 +208,14 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-### Notes
+While the above method avoids using Docker in privileged mode, you should be aware of the following implications:
 * By sharing the docker daemon, you are effectively disabling all
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For example, if a project
 ran `docker rm -f $(docker ps -a -q)` it would remove the GitLab Runner
 containers.
-
 * Concurrent builds may not work; if your tests
 create containers with specific names, they may conflict with each other.
-
 * Sharing files and directories from the source repo into containers may not
 work as expected since volume mounting is done in the context of the host
 machine, not the build container.
@@ -306,11 +306,11 @@ deploy:
     - master
 ```
 
-### Notes
-1. You must log in to the container registry before running commands. Putting this in `before_script` will run it before each build job.
-1. Using `docker build --pull` makes sure that Docker fetches any changes to base images before building just in case your cache is stale. It takes slightly longer, but means you don’t get stuck without security patches to base images.
-1. Doing an explicit `docker pull` before each `docker run` makes sure to fetch the latest image that was just built. This is especially important if you are using multiple runners that cache images locally. Using the git SHA in your image tag makes this less necessary since each build will be unique and you shouldn't ever have a stale image, but it's still possible if you re-build a given commit after a dependency has changed.
-1. You don't want to build directly to `latest` in case there are multiple builds happening simultaneously.
+Some things you should be aware of when using the Container Registry:
+* You must log in to the container registry before running commands. Putting this in `before_script` will run it before each build job.
+* Using `docker build --pull` makes sure that Docker fetches any changes to base images before building just in case your cache is stale. It takes slightly longer, but means you don’t get stuck without security patches to base images.
+* Doing an explicit `docker pull` before each `docker run` makes sure to fetch the latest image that was just built. This is especially important if you are using multiple runners that cache images locally. Using the git SHA in your image tag makes this less necessary since each build will be unique and you shouldn't ever have a stale image, but it's still possible if you re-build a given commit after a dependency has changed.
+* You don't want to build directly to `latest` in case there are multiple builds happening simultaneously.
 
 [docker-in-docker]: https://blog.docker.com/2013/09/docker-can-now-run-within-docker/
 [docker-cap]: https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities

From 4c571041de6989d71f09fc326f8d6bee731f0b19 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Mon, 13 Jun 2016 22:36:28 -0700
Subject: [PATCH 50/56] Make minor grammar change

---
 doc/ci/docker/using_docker_build.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 7729159765..09a2d8b596 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -72,8 +72,8 @@ GitLab Runner then executes build scripts as the `gitlab-runner` user.
 5. You can now use `docker` command and install `docker-compose` if needed.
 
 > **Note:**
-* By adding `gitlab-runner` to `docker` group you are effectively granting `gitlab-runner` full root permissions.
-For more information please check out [On Docker security: `docker` group considered harmful](https://www.andreas-jung.com/contents/on-docker-security-docker-group-considered-harmful).
+* By adding `gitlab-runner` to the `docker` group you are effectively granting `gitlab-runner` full root permissions.
+For more information please read [On Docker security: `docker` group considered harmful](https://www.andreas-jung.com/contents/on-docker-security-docker-group-considered-harmful).
 
 ### Use docker-in-docker executor
 

From aefb08cb6a8bd15415b641c385e790f941b72ced Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Mon, 13 Jun 2016 22:42:46 -0700
Subject: [PATCH 51/56] Clarify dind example

---
 doc/ci/docker/using_docker_build.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 09a2d8b596..36ff4dcf05 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -122,7 +122,7 @@ In order to do that, follow the steps:
         Insecure = false
     ```
 
-1. You can now use `docker` in the build script:
+1. You can now use `docker` in the build script (note the inclusion of the `docker:dind` service):
 
     ```yaml
     image: docker:latest
@@ -141,7 +141,7 @@ In order to do that, follow the steps:
     ```
 
 > **Notes:**
-> * By enabling `--docker-privileged`, you are effectively disabling all
+> * By enabling `--docker-privileged`, you are effectively disabling all of
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For more information, check out the official Docker documentation on
 [Runtime privilege and Linux capabilities][docker-cap].

From 8df7d90d5a92b7d8aa26ac07b7391b4e86d63499 Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Mon, 13 Jun 2016 22:45:43 -0700
Subject: [PATCH 52/56] De-note-ify

---
 doc/ci/docker/using_docker_build.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 36ff4dcf05..39eea740d1 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -140,14 +140,14 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-> **Notes:**
-> * By enabling `--docker-privileged`, you are effectively disabling all of
+Docker-in-Docker works well, and is our recommended configuration, but it is not without its own challenges:
+* By enabling `--docker-privileged`, you are effectively disabling all of
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For more information, check out the official Docker documentation on
 [Runtime privilege and Linux capabilities][docker-cap].
-> * Using docker-in-docker, each build is in a clean environment without the past
+* Using docker-in-docker, each build is in a clean environment without the past
 history. Concurrent builds work fine because every build gets it's own instance of docker engine so they won't conflict with each other. But this also means builds can be slower because there's no caching of layers.
-> * By default, `docker:dind` uses `--storage-driver vfs` which is the slowest form
+* By default, `docker:dind` uses `--storage-driver vfs` which is the slowest form
 offered.
 
 An example project using this approach can be found here: https://gitlab.com/gitlab-examples/docker.

From 6ed7fcad29d0b96b4513c2961c342d0309eda07e Mon Sep 17 00:00:00 2001
From: Mark Pundsack <markpundsack@users.noreply.github.com>
Date: Mon, 13 Jun 2016 22:47:54 -0700
Subject: [PATCH 53/56] Remove our

---
 doc/ci/docker/using_docker_build.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md
index 39eea740d1..7f83f84645 100644
--- a/doc/ci/docker/using_docker_build.md
+++ b/doc/ci/docker/using_docker_build.md
@@ -140,7 +140,7 @@ In order to do that, follow the steps:
       - docker run my-docker-image /script/to/run/tests
     ```
 
-Docker-in-Docker works well, and is our recommended configuration, but it is not without its own challenges:
+Docker-in-Docker works well, and is the recommended configuration, but it is not without its own challenges:
 * By enabling `--docker-privileged`, you are effectively disabling all of
 the security mechanisms of containers and exposing your host to privilege
 escalation which can lead to container breakout. For more information, check out the official Docker documentation on

From 59eeec3ff87ce175e34ac96e86c9690c5290502b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 13 Jun 2016 14:03:11 +0200
Subject: [PATCH 54/56] Make method that composes ci config entry private

---
 lib/gitlab/ci/config/node/configurable.rb     |  2 +-
 lib/gitlab/ci/config/node/entry.rb            | 12 ++++++------
 spec/lib/gitlab/ci/config/node/global_spec.rb |  9 ---------
 spec/lib/gitlab/ci/config/node/null_spec.rb   |  2 +-
 4 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 86cc33e11b..7587c8c34c 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -38,7 +38,7 @@ module Gitlab
 
           class_methods do
             def allowed_nodes
-              Hash[@allowed_nodes.map { |key, factory| [key, factory.dup] } ]
+              Hash[@allowed_nodes.map { |key, factory| [key, factory.dup] }]
             end
 
             private
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index e5692e7294..507312e0c0 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -26,12 +26,6 @@ module Gitlab
             nodes.each(&:validate!)
           end
 
-          def compose!
-            allowed_nodes.each do |key, essence|
-              @nodes[key] = create_node(key, essence)
-            end
-          end
-
           def nodes
             @nodes.values
           end
@@ -62,6 +56,12 @@ module Gitlab
 
           private
 
+          def compose!
+            allowed_nodes.each do |key, essence|
+              @nodes[key] = create_node(key, essence)
+            end
+          end
+
           def create_node(key, essence)
             raise NotImplementedError
           end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 2227fcec63..b197217243 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -18,15 +18,6 @@ describe Gitlab::Ci::Config::Node::Global do
       { before_script: ['ls', 'pwd'] }
     end
 
-    describe '#compose!' do
-      before { global.compose! }
-
-      it 'instantiates entry nodes' do
-        expect(global.nodes.first)
-          .to be_an_instance_of Gitlab::Ci::Config::Node::Script
-      end
-    end
-
     describe '#process!' do
       before { global.process! }
 
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb
index fb6c3b5cbc..36101c6246 100644
--- a/spec/lib/gitlab/ci/config/node/null_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/null_spec.rb
@@ -16,7 +16,7 @@ describe Gitlab::Ci::Config::Node::Null do
   end
 
   describe '#value' do
-    it 'returns nill' do
+    it 'returns nil' do
       expect(entry.value).to be nil
     end
   end

From 30e946ce8a9272b3de1a64498965933804b7bb6d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 14 Jun 2016 11:28:20 +0200
Subject: [PATCH 55/56] Validate ci config entry value before processing nodes

---
 lib/gitlab/ci/config/node/configurable.rb | 14 ++++++--------
 lib/gitlab/ci/config/node/entry.rb        |  5 +++++
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 7587c8c34c..d60f87f3f9 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -16,20 +16,18 @@ module Gitlab
         module Configurable
           extend ActiveSupport::Concern
 
-          def initialize(*)
-            super
-
-            unless @value.is_a?(Hash)
-              @errors << 'should be a configuration entry with hash value'
-            end
-          end
-
           def allowed_nodes
             self.class.allowed_nodes || {}
           end
 
           private
 
+          def prevalidate!
+            unless @value.is_a?(Hash)
+              @errors << 'should be a configuration entry with hash value'
+            end
+          end
+
           def create_node(key, factory)
             factory.with(value: @value[key])
             factory.nullify! unless @value.has_key?(key)
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 507312e0c0..52758a962f 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -14,6 +14,8 @@ module Gitlab
             @value = value
             @nodes = {}
             @errors = []
+
+            prevalidate!
           end
 
           def process!
@@ -56,6 +58,9 @@ module Gitlab
 
           private
 
+          def prevalidate!
+          end
+
           def compose!
             allowed_nodes.each do |key, essence|
               @nodes[key] = create_node(key, essence)

From 120fbbd4875f340b5c863b7e0e3eabcb2796e15d Mon Sep 17 00:00:00 2001
From: Paco Guzman <pacoguzmanp@gmail.com>
Date: Mon, 13 Jun 2016 18:41:37 +0200
Subject: [PATCH 56/56] Measure CPU time for instrumented methods

---
 CHANGELOG                                       |  1 +
 doc/development/instrumentation.md              | 11 ++++++-----
 lib/gitlab/metrics/instrumentation.rb           | 11 +++++++----
 spec/lib/gitlab/metrics/instrumentation_spec.rb |  4 ++--
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 2aed8eb322..e71a154d1d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -77,6 +77,7 @@ v 8.9.0 (unreleased)
   - All classes in the Banzai::ReferenceParser namespace are now instrumented
   - Remove deprecated issues_tracker and issues_tracker_id from project model
   - Allow users to create confidential issues in private projects
+  - Measure CPU time for instrumented methods
 
 v 8.8.5 (unreleased)
   - Ensure branch cleanup regardless of whether the GitHub import process succeeds
diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md
index 9168c70945..50d2866ca4 100644
--- a/doc/development/instrumentation.md
+++ b/doc/development/instrumentation.md
@@ -97,15 +97,16 @@ def #{name}(#{args_signature})
   trans = Gitlab::Metrics::Instrumentation.transaction
 
   if trans
-    start    = Time.now
-    retval   = super
-    duration = (Time.now - start) * 1000.0
+    start     = Time.now
+    cpu_start = Gitlab::Metrics::System.cpu_time
+    retval    = super
+    duration  = (Time.now - start) * 1000.0
 
     if duration >= Gitlab::Metrics.method_call_threshold
-      trans.increment(:method_duration, duration)
+      cpu_duration = Gitlab::Metrics::System.cpu_time - cpu_start
 
       trans.add_metric(Gitlab::Metrics::Instrumentation::SERIES,
-                       { duration: duration },
+                       { duration: duration, cpu_duration: cpu_duration },
                        method: #{label.inspect})
     end
 
diff --git a/lib/gitlab/metrics/instrumentation.rb b/lib/gitlab/metrics/instrumentation.rb
index 0f115893a1..ad9ce3fa44 100644
--- a/lib/gitlab/metrics/instrumentation.rb
+++ b/lib/gitlab/metrics/instrumentation.rb
@@ -149,13 +149,16 @@ module Gitlab
             trans = Gitlab::Metrics::Instrumentation.transaction
 
             if trans
-              start    = Time.now
-              retval   = super
-              duration = (Time.now - start) * 1000.0
+              start     = Time.now
+              cpu_start = Gitlab::Metrics::System.cpu_time
+              retval    = super
+              duration  = (Time.now - start) * 1000.0
 
               if duration >= Gitlab::Metrics.method_call_threshold
+                cpu_duration = Gitlab::Metrics::System.cpu_time - cpu_start
+
                 trans.add_metric(Gitlab::Metrics::Instrumentation::SERIES,
-                                 { duration: duration },
+                                 { duration: duration, cpu_duration: cpu_duration },
                                  method: #{label.inspect})
               end
 
diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb
index 220e86924a..c6e979b69a 100644
--- a/spec/lib/gitlab/metrics/instrumentation_spec.rb
+++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb
@@ -57,7 +57,7 @@ describe Gitlab::Metrics::Instrumentation do
           and_return(transaction)
 
         expect(transaction).to receive(:add_metric).
-          with(described_class::SERIES, an_instance_of(Hash),
+          with(described_class::SERIES, hash_including(:duration, :cpu_duration),
                method: 'Dummy.foo')
 
         @dummy.foo
@@ -137,7 +137,7 @@ describe Gitlab::Metrics::Instrumentation do
           and_return(transaction)
 
         expect(transaction).to receive(:add_metric).
-          with(described_class::SERIES, an_instance_of(Hash),
+          with(described_class::SERIES, hash_including(:duration, :cpu_duration),
                method: 'Dummy#bar')
 
         @dummy.new.bar