Require parent when using node factory in CI config

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

@@ -28,7 +28,8 @@ module Gitlab
           def create(key, factory)
             factory
               .value(@config[key])
-              .with(key: key, parent: self, global: @global)
+              .parent(self)
+              .with(key: key)
 
             factory.create!
           end
@@ -40,11 +41,11 @@ module Gitlab
 
             private
 
-            def node(symbol, entry_class, metadata)
+            def node(key, node, metadata)
-              factory = Node::Factory.new(entry_class)
+              factory = Node::Factory.new(node)
                 .with(description: metadata[:description])
 
-              (@nodes ||= {}).merge!(symbol.to_sym => factory)
+              (@nodes ||= {}).merge!(key.to_sym => factory)
             end
 
             def helpers(*nodes)

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

@@ -18,6 +18,11 @@ module Gitlab
             self
           end
 
+          def parent(parent)
+            @parent = parent
+            self
+          end
+
           def with(attributes)
             @attributes.merge!(attributes)
             self
@@ -25,15 +30,19 @@ module Gitlab
 
           def create!
             raise InvalidFactory unless defined?(@value)
+            raise InvalidFactory unless defined?(@parent)
+
+            attributes = { parent: @parent, global: @parent.global }
+            attributes.merge!(@attributes)
 
             ##
             # We assume that unspecified entry is undefined.
             # See issue #18775.
             #
             if @value.nil?
-              Node::Undefined.new(@node, @attributes)
+              Node::Undefined.new(@node, attributes)
             else
-              @node.new(@value, @attributes)
+              @node.new(@value, attributes)
             end
           end
         end

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

@@ -53,9 +53,10 @@ module Gitlab
 
           def compose_jobs!
             factory = Node::Factory.new(Node::Jobs)
-            factory.value(@config.except(*nodes.keys))
+              .value(@config.except(*nodes.keys))
-            factory.with(key: :jobs, parent: self, global: self)
+              .parent(self)
-            factory.with(description: 'Jobs definition for this pipeline')
+              .with(key: :jobs, global: self)
+              .with(description: 'Jobs definition for this pipeline')
 
             @entries[:jobs] = factory.create!
           end

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

@@ -30,14 +30,14 @@ module Gitlab
           private
 
           def create(name, config)
-            Node::Factory.new(job_node(name))
+            Node::Factory.new(node(name))
               .value(config || {})
-              .with(key: name, parent: self, global: @global)
+              .parent(self)
-              .with(description: "#{name} job definition.")
+              .with(key: name, description: "#{name} job definition.")
               .create!
           end
 
-          def job_node(name)
+          def node(name)
             if name.to_s.start_with?('.')
               Node::HiddenJob
             else

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

@@ -2,22 +2,40 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Config::Node::Factory do
   describe '#create!' do
-    let(:factory) { described_class.new(entry_class) }
+    let(:factory) { described_class.new(node) }
-    let(:entry_class) { Gitlab::Ci::Config::Node::Script }
+    let(:node) { Gitlab::Ci::Config::Node::Script }
+    let(:parent) { double('parent') }
+    let(:global) { double('global') }
 
-    context 'when setting up a value' do
+    before do
+      allow(parent).to receive(:global).and_return(global)
+    end
+
+    context 'when setting a concrete value' do
       it 'creates entry with valid value' do
         entry = factory
           .value(['ls', 'pwd'])
+          .parent(parent)
           .create!
 
         expect(entry.value).to eq ['ls', 'pwd']
       end
 
+      it 'sets parent and global attributes' do
+        entry = factory
+          .value('ls')
+          .parent(parent)
+          .create!
+
+        expect(entry.global).to eq global
+        expect(entry.parent).to eq parent
+      end
+
       context 'when setting description' do
         it 'creates entry with description' do
           entry = factory
             .value(['ls', 'pwd'])
+            .parent(parent)
             .with(description: 'test description')
             .create!
 
@@ -30,6 +48,7 @@ describe Gitlab::Ci::Config::Node::Factory do
         it 'creates entry with custom key' do
           entry = factory
             .value(['ls', 'pwd'])
+            .parent(parent)
             .with(key: 'test key')
             .create!
 
@@ -38,20 +57,21 @@ describe Gitlab::Ci::Config::Node::Factory do
       end
 
       context 'when setting a parent' do
-        let(:parent) { Object.new }
+        let(:object) { Object.new }
 
         it 'creates entry with valid parent' do
           entry = factory
             .value('ls')
-            .with(parent: parent)
+            .parent(parent)
+            .with(parent: object)
             .create!
 
-          expect(entry.parent).to eq parent
+          expect(entry.parent).to eq object
         end
       end
     end
 
-    context 'when not setting up a value' do
+    context 'when not setting a value' do
       it 'raises error' do
         expect { factory.create! }.to raise_error(
           Gitlab::Ci::Config::Node::Factory::InvalidFactory
@@ -59,10 +79,19 @@ describe Gitlab::Ci::Config::Node::Factory do
       end
     end
 
+    context 'when not setting parent object' do
+      it 'raises error' do
+        expect { factory.value('ls').create! }.to raise_error(
+          Gitlab::Ci::Config::Node::Factory::InvalidFactory
+        )
+      end
+    end
+
     context 'when creating entry with nil value' do
       it 'creates an undefined entry' do
         entry = factory
           .value(nil)
+          .parent(parent)
           .create!
 
         expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined