Return default config value when entry is undefined

9 files changed, 74 insertions, 51 deletions

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