Merge branch 'refactor/ci-config-move-global-entries' into 'master'

Move global ci entries handling from legacy to new config

## What does this MR do?

This MR moves responsibility of handling global CI config entries (like `image`, `services`), from legacy `GitlabCiYamlProcessor` to new CI Config

## Why was this MR needed?

This is the next iteration of CI configuration refactoring

## What are the relevant issue numbers?

#15060

## Does this MR meet the acceptance criteria?

- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)

See merge request !4820

19 files changed, 364 insertions, 139 deletions

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index c52d4d63382..01ef13df57a 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -4,7 +4,6 @@ module Ci
 
     include Gitlab::Ci::Config::Node::LegacyValidationHelpers
 
-    DEFAULT_STAGES = %w(build test deploy)
     DEFAULT_STAGE = 'test'
     ALLOWED_YAML_KEYS = [:before_script, :after_script, :image, :services, :types, :stages, :variables, :cache]
     ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services,
@@ -14,7 +13,7 @@ module Ci
     ALLOWED_CACHE_KEYS = [:key, :untracked, :paths]
     ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in]
 
-    attr_reader :after_script, :image, :services, :path, :cache
+    attr_reader :path, :cache, :stages
 
     def initialize(config, path = nil)
       @ci_config = Gitlab::Ci::Config.new(config)
@@ -22,8 +21,11 @@ module Ci
 
       @path = path
 
-      initial_parsing
+      unless @ci_config.valid?
+        raise ValidationError, @ci_config.errors.first
+      end
 
+      initial_parsing
       validate!
     rescue Gitlab::Ci::Config::Loader::FormatError => e
       raise ValidationError, e.message
@@ -42,10 +44,6 @@ module Ci
       end
     end
 
-    def stages
-      @stages || DEFAULT_STAGES
-    end
-
     def global_variables
       @variables
     end
@@ -60,12 +58,14 @@ module Ci
     private
 
     def initial_parsing
-      @after_script = @config[:after_script]
-      @image = @config[:image]
-      @services = @config[:services]
-      @stages = @config[:stages] || @config[:types]
-      @variables = @config[:variables] || {}
-      @cache = @config[:cache]
+      @before_script = @ci_config.before_script
+      @image = @ci_config.image
+      @after_script = @ci_config.after_script
+      @services = @ci_config.services
+      @variables = @ci_config.variables
+      @stages = @ci_config.stages
+      @cache = @ci_config.cache
+
       @jobs = {}
 
       @config.except!(*ALLOWED_YAML_KEYS)
@@ -85,9 +85,14 @@ module Ci
 
     def build_job(name, job)
       {
-        stage_idx: stages.index(job[:stage]),
+        stage_idx: @stages.index(job[:stage]),
         stage: job[:stage],
-        commands: [job[:before_script] || [@ci_config.before_script], job[:script]].flatten.compact.join("\n"),
+        ##
+        # Refactoring note:
+        #  - before script behaves differently than after script
+        #  - after script returns an array of commands
+        #  - before script should be a concatenated command
+        commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
         name: name,
         only: job[:only],
@@ -107,12 +112,6 @@ module Ci
     end
 
     def validate!
-      unless @ci_config.valid?
-        raise ValidationError, @ci_config.errors.first
-      end
-
-      validate_global!
-
       @jobs.each do |name, job|
         validate_job!(name, job)
       end
@@ -120,50 +119,6 @@ module Ci
       true
     end
 
-    def validate_global!
-      unless @after_script.nil? || validate_array_of_strings(@after_script)
-        raise ValidationError, "after_script should be an array of strings"
-      end
-
-      unless @image.nil? || @image.is_a?(String)
-        raise ValidationError, "image should be a string"
-      end
-
-      unless @services.nil? || validate_array_of_strings(@services)
-        raise ValidationError, "services should be an array of strings"
-      end
-
-      unless @stages.nil? || validate_array_of_strings(@stages)
-        raise ValidationError, "stages should be an array of strings"
-      end
-
-      unless @variables.nil? || validate_variables(@variables)
-        raise ValidationError, "variables should be a map of key-value strings"
-      end
-
-      validate_global_cache! if @cache
-    end
-
-    def validate_global_cache!
-      @cache.keys.each do |key|
-        unless ALLOWED_CACHE_KEYS.include? key
-          raise ValidationError, "#{name} cache unknown parameter #{key}"
-        end
-      end
-
-      if @cache[:key] && !validate_string(@cache[:key])
-        raise ValidationError, "cache:key parameter should be a string"
-      end
-
-      if @cache[:untracked] && !validate_boolean(@cache[:untracked])
-        raise ValidationError, "cache:untracked parameter should be an boolean"
-      end
-
-      if @cache[:paths] && !validate_array_of_strings(@cache[:paths])
-        raise ValidationError, "cache:paths parameter should be an array of strings"
-      end
-    end
-
     def validate_job!(name, job)
       validate_job_name!(name)
       validate_job_keys!(name, job)
@@ -240,8 +195,8 @@ module Ci
     end
 
     def validate_job_stage!(name, job)
-      unless job[:stage].is_a?(String) && job[:stage].in?(stages)
-        raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}"
+      unless job[:stage].is_a?(String) && job[:stage].in?(@stages)
+        raise ValidationError, "#{name} job: stage parameter should be #{@stages.join(", ")}"
       end
     end
 
@@ -305,12 +260,12 @@ module Ci
         raise ValidationError, "#{name} job: dependencies parameter should be an array of strings"
       end
 
-      stage_index = stages.index(job[:stage])
+      stage_index = @stages.index(job[:stage])
 
       job[:dependencies].each do |dependency|
         raise ValidationError, "#{name} job: undefined dependency: #{dependency}" unless @jobs[dependency.to_sym]
 
-        unless stages.index(@jobs[dependency.to_sym][:stage]) < stage_index
+        unless @stages.index(@jobs[dependency.to_sym][:stage]) < stage_index
           raise ValidationError, "#{name} job: dependency #{dependency} is not defined in prior stages"
         end
       end
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index adfd097736e..e6cc1529760 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -7,7 +7,8 @@ module Gitlab
       ##
       # Temporary delegations that should be removed after refactoring
       #
-      delegate :before_script, to: :@global
+      delegate :before_script, :image, :services, :after_script, :variables,
+               :stages, :cache, to: :@global
 
       def initialize(config)
         @config = Loader.new(config).load!
diff --git a/lib/gitlab/ci/config/node/boolean.rb b/lib/gitlab/ci/config/node/boolean.rb
new file mode 100644
index 00000000000..84b03ee7832
--- /dev/null
+++ b/lib/gitlab/ci/config/node/boolean.rb
@@ -0,0 +1,18 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a boolean value.
+        #
+        class Boolean < Entry
+          include Validatable
+
+          validations do
+            validates :config, boolean: true
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
new file mode 100644
index 00000000000..cdf8ba2e35d
--- /dev/null
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -0,0 +1,27 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a cache configuration
+        #
+        class Cache < Entry
+          include Configurable
+
+          node :key, Node::Key,
+            description: 'Cache key used to define a cache affinity.'
+
+          node :untracked, Node::Boolean,
+            description: 'Cache all untracked files.'
+
+          node :paths, Node::Paths,
+            description: 'Specify which paths should be cached across builds.'
+
+          validations do
+            validates :config, allowed_keys: true
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 374ff71d0f5..37936fc8242 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -19,35 +19,45 @@ module Gitlab
 
           included do
             validations do
-              validates :config, hash: true
+              validates :config, type: Hash
             end
           end
 
           private
 
           def create_node(key, factory)
-            factory.with(value: @config[key], key: key)
-            factory.nullify! unless @config.has_key?(key)
+            factory.with(value: @config[key], key: key, parent: self)
+
             factory.create!
           end
 
           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])
 
-              define_method(symbol) do
-                raise Entry::InvalidError unless valid?
-                @nodes[symbol].try(:value)
-              end
+              (@nodes ||= {}).merge!(symbol.to_sym => factory)
+            end
 
-              (@allowed_nodes ||= {}).merge!(symbol => factory)
+            def helpers(*nodes)
+              nodes.each do |symbol|
+                define_method("#{symbol}_defined?") do
+                  @nodes[symbol].try(:defined?)
+                end
+
+                define_method("#{symbol}_value") do
+                  raise Entry::InvalidError unless valid?
+                  @nodes[symbol].try(:value)
+                end
+
+                alias_method symbol.to_sym, "#{symbol}_value".to_sym
+              end
             end
           end
         end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index f044ef965e9..9e79e170a4f 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -9,7 +9,7 @@ module Gitlab
           class InvalidError < StandardError; end
 
           attr_reader :config
-          attr_accessor :key, :description
+          attr_accessor :key, :parent, :description
 
           def initialize(config)
             @config = config
@@ -34,8 +34,8 @@ module Gitlab
             self.class.nodes.none?
           end
 
-          def key
-            @key || self.class.name.demodulize.underscore
+          def ancestors
+            @parent ? @parent.ancestors + [@parent] : []
           end
 
           def valid?
@@ -43,12 +43,23 @@ module Gitlab
           end
 
           def errors
-            @validator.full_errors +
-              nodes.map(&:errors).flatten
+            @validator.messages + nodes.flat_map(&:errors)
           end
 
           def value
-            raise NotImplementedError
+            if leaf?
+              @config
+            else
+              defined = @nodes.select { |_key, value| value.defined? }
+              Hash[defined.map { |key, node| [key, node.value] }]
+            end
+          end
+
+          def defined?
+            true
+          end
+
+          def self.default
           end
 
           def self.nodes
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 025ae40ef94..5919a283283 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -5,13 +5,11 @@ module Gitlab
         ##
         # Factory class responsible for fabricating node entry objects.
         #
-        # It uses Fluent Interface pattern to set all necessary attributes.
-        #
         class Factory
           class InvalidFactory < StandardError; end
 
-          def initialize(entry_class)
-            @entry_class = entry_class
+          def initialize(node)
+            @node = node
             @attributes = {}
           end
 
@@ -20,17 +18,27 @@ module Gitlab
             self
           end
 
-          def nullify!
-            @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]
+            fabricate.tap do |entry|
               entry.key = @attributes[:key]
+              entry.parent = @attributes[:parent]
+              entry.description = @attributes[:description]
+            end
+          end
+
+          private
+
+          def fabricate
+            ##
+            # We assume that unspecified entry is undefined.
+            # See issue #18775.
+            #
+            if @attributes[:value].nil?
+              Node::Undefined.new(@node)
+            else
+              @node.new(@attributes[:value])
             end
           end
         end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 044603423d5..f92e1eccbcf 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -9,8 +9,36 @@ module Gitlab
         class Global < Entry
           include Configurable
 
-          allow_node :before_script, Script,
+          node :before_script, Node::Script,
             description: 'Script that will be executed before each job.'
+
+          node :image, Node::Image,
+            description: 'Docker image that will be used to execute jobs.'
+
+          node :services, Node::Services,
+            description: 'Docker images that will be linked to the container.'
+
+          node :after_script, Node::Script,
+            description: 'Script that will be executed after each job.'
+
+          node :variables, Node::Variables,
+            description: 'Environment variables that will be used.'
+
+          node :stages, Node::Stages,
+            description: 'Configuration of stages for this pipeline.'
+
+          node :types, Node::Stages,
+            description: 'Deprecated: stages for this pipeline.'
+
+          node :cache, Node::Cache,
+            description: 'Configure caching between build jobs.'
+
+          helpers :before_script, :image, :services, :after_script,
+                  :variables, :stages, :types, :cache
+
+          def stages
+            stages_defined? ? stages_value : types_value
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/image.rb b/lib/gitlab/ci/config/node/image.rb
new file mode 100644
index 00000000000..5d3c7c5eab0
--- /dev/null
+++ b/lib/gitlab/ci/config/node/image.rb
@@ -0,0 +1,18 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a Docker image.
+        #
+        class Image < Entry
+          include Validatable
+
+          validations do
+            validates :config, type: String
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/key.rb b/lib/gitlab/ci/config/node/key.rb
new file mode 100644
index 00000000000..f8b461ca098
--- /dev/null
+++ b/lib/gitlab/ci/config/node/key.rb
@@ -0,0 +1,18 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a key.
+        #
+        class Key < Entry
+          include Validatable
+
+          validations do
+            validates :config, key: true
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
deleted file mode 100644
index 4f590f6bec8..00000000000
--- a/lib/gitlab/ci/config/node/null.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-module Gitlab
-  module Ci
-    class Config
-      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
-          end
-
-          def validate!
-            nil
-          end
-
-          def method_missing(*)
-            nil
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/ci/config/node/paths.rb b/lib/gitlab/ci/config/node/paths.rb
new file mode 100644
index 00000000000..3c6d3a52966
--- /dev/null
+++ b/lib/gitlab/ci/config/node/paths.rb
@@ -0,0 +1,18 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents an array of paths.
+        #
+        class Paths < Entry
+          include Validatable
+
+          validations do
+            validates :config, array_of_strings: true
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index c044f5c5e71..39328f0fade 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -5,21 +5,12 @@ module Gitlab
         ##
         # Entry that represents a script.
         #
-        # Each element in the value array is a command that will be executed
-        # by GitLab Runner. Currently we concatenate these commands with
-        # new line character as a separator, what is compatible with
-        # implementation in Runner.
-        #
         class Script < Entry
           include Validatable
 
           validations do
             validates :config, array_of_strings: true
           end
-
-          def value
-            @config.join("\n")
-          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/services.rb b/lib/gitlab/ci/config/node/services.rb
new file mode 100644
index 00000000000..481e2b66adc
--- /dev/null
+++ b/lib/gitlab/ci/config/node/services.rb
@@ -0,0 +1,18 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a configuration of Docker services.
+        #
+        class Services < Entry
+          include Validatable
+
+          validations do
+            validates :config, array_of_strings: true
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/stages.rb b/lib/gitlab/ci/config/node/stages.rb
new file mode 100644
index 00000000000..b1fe45357ff
--- /dev/null
+++ b/lib/gitlab/ci/config/node/stages.rb
@@ -0,0 +1,22 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents a configuration for pipeline stages.
+        #
+        class Stages < Entry
+          include Validatable
+
+          validations do
+            validates :config, array_of_strings: true
+          end
+
+          def self.default
+            %w[build test deploy]
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
new file mode 100644
index 00000000000..699605e1e3a
--- /dev/null
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -0,0 +1,30 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # This class represents an undefined entry node.
+        #
+        # It takes original entry class as configuration and returns default
+        # value of original entry as self value.
+        #
+        #
+        class Undefined < Entry
+          include Validatable
+
+          validations do
+            validates :config, type: Class
+          end
+
+          def value
+            @config.default
+          end
+
+          def defined?
+            false
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index 02edc9219c3..758a6cf4356 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -11,15 +11,29 @@ module Gitlab
             @node = node
           end
 
-          def full_errors
+          def messages
             errors.full_messages.map do |error|
-              "#{@node.key} #{error}".humanize
+              "#{location} #{error}".downcase
             end
           end
 
           def self.name
             'Validator'
           end
+
+          def unknown_keys
+            return [] unless config.is_a?(Hash)
+
+            config.keys - @node.class.nodes.keys
+          end
+
+          private
+
+          def location
+            predecessors = ancestors.map(&:key).compact
+            current = key || @node.class.name.demodulize.underscore
+            predecessors.append(current).join(':')
+          end
         end
       end
     end
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index dc9cdb9a220..7b2f57990b5 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -3,6 +3,16 @@ module Gitlab
     class Config
       module Node
         module Validators
+          class AllowedKeysValidator < ActiveModel::EachValidator
+            def validate_each(record, attribute, value)
+              if record.unknown_keys.any?
+                unknown_list = record.unknown_keys.join(', ')
+                record.errors.add(:config,
+                                  "contains unknown keys: #{unknown_list}")
+              end
+            end
+          end
+
           class ArrayOfStringsValidator < ActiveModel::EachValidator
             include LegacyValidationHelpers
 
@@ -13,10 +23,43 @@ module Gitlab
             end
           end
 
-          class HashValidator < ActiveModel::EachValidator
+          class BooleanValidator < ActiveModel::EachValidator
+            include LegacyValidationHelpers
+
+            def validate_each(record, attribute, value)
+              unless validate_boolean(value)
+                record.errors.add(attribute, 'should be a boolean value')
+              end
+            end
+          end
+
+          class KeyValidator < ActiveModel::EachValidator
+            include LegacyValidationHelpers
+
+            def validate_each(record, attribute, value)
+              unless validate_string(value)
+                record.errors.add(attribute, 'should be a string or symbol')
+              end
+            end
+          end
+
+          class TypeValidator < ActiveModel::EachValidator
+            def validate_each(record, attribute, value)
+              type = options[:with]
+              raise unless type.is_a?(Class)
+
+              unless value.is_a?(type)
+                record.errors.add(attribute, "should be a #{type.name}")
+              end
+            end
+          end
+
+          class VariablesValidator < ActiveModel::EachValidator
+            include LegacyValidationHelpers
+
             def validate_each(record, attribute, value)
-              unless value.is_a?(Hash)
-                record.errors.add(attribute, 'should be a configuration entry hash')
+              unless validate_variables(value)
+                record.errors.add(attribute, 'should be a hash of key value pairs')
               end
             end
           end
diff --git a/lib/gitlab/ci/config/node/variables.rb b/lib/gitlab/ci/config/node/variables.rb
new file mode 100644
index 00000000000..5f813f81f55
--- /dev/null
+++ b/lib/gitlab/ci/config/node/variables.rb
@@ -0,0 +1,22 @@
+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents environment variables.
+        #
+        class Variables < Entry
+          include Validatable
+
+          validations do
+            validates :config, variables: true
+          end
+
+          def self.default
+            {}
+          end
+        end
+      end
+    end
+  end
+end