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

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

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!

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

+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

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

+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

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

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

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

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

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

+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

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

+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

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

+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

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

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

+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

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

+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

lib/gitlab/ci/config/node/null.rb → lib/gitlab/ci/config/node/undefined.rb

@@ -3,22 +3,25 @@ module Gitlab
     class Config
       module Node
         ##
-        # This class represents a configuration entry that is not being used
-        # in configuration file.
+        # This class represents an undefined entry node.
         #
-        # This implements Null Object pattern.
+        # It takes original entry class as configuration and returns default
+        # value of original entry as self value.
         #
-        class Null < Entry
-          def value
-            nil
+        #
+        class Undefined < Entry
+          include Validatable
+
+          validations do
+            validates :config, type: Class
           end
 
-          def validate!
-            nil
+          def value
+            @config.default
           end
 
-          def method_missing(*)
-            nil
+          def defined?
+            false
           end
         end
       end

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

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

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

+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

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -550,8 +550,8 @@ module Ci
               config_processor = GitlabCiYamlProcessor.new(config, path)
 
               ##
-              #  TODO, in next version of CI configuration processor this
-              # should be invalid configuration, see #18775 and #15060
+              # When variables config is empty, we assume this is a valid
+              # configuration, see issue #18775
               #
               expect(config_processor.job_variables(:rspec))
                 .to be_an_instance_of(Array).and be_empty
@@ -590,7 +590,20 @@ module Ci
       end
     end
 
-    describe "Caches" do
+    describe 'cache' do
+      context 'when cache definition has unknown keys' do
+        it 'raises relevant validation error' do
+          config = YAML.dump(
+            { cache: { untracked: true, invalid: 'key' },
+              rspec: { script: 'rspec' } })
+
+          expect { GitlabCiYamlProcessor.new(config) }.to raise_error(
+            GitlabCiYamlProcessor::ValidationError,
+            'cache config contains unknown keys: invalid'
+          )
+        end
+      end
+
       it "returns cache when defined globally" do
         config = YAML.dump({
                              cache: { paths: ["logs/", "binaries/"], untracked: true, key: 'key' },
@@ -950,7 +963,7 @@ EOT
         config = YAML.dump({ before_script: "bundle update", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Before script config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script config should be an array of strings")
       end
 
       it "returns errors if job before_script parameter is not an array of strings" do
@@ -964,7 +977,7 @@ EOT
         config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script config should be an array of strings")
       end
 
       it "returns errors if job after_script parameter is not an array of strings" do
@@ -978,7 +991,7 @@ EOT
         config = YAML.dump({ image: ["test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image config should be a string")
       end
 
       it "returns errors if job name is blank" do
@@ -1006,14 +1019,14 @@ EOT
         config = YAML.dump({ services: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services config should be an array of strings")
       end
 
       it "returns errors if services parameter is not an array of strings" do
         config = YAML.dump({ services: [10, "test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services config should be an array of strings")
       end
 
       it "returns errors if job services parameter is not an array" do
@@ -1080,31 +1093,31 @@ EOT
       end
 
       it "returns errors if stages is not an array" do
-        config = YAML.dump({ types: "test", rspec: { script: "test" } })
+        config = YAML.dump({ stages: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages config should be an array of strings")
       end
 
       it "returns errors if stages is not an array of strings" do
-        config = YAML.dump({ types: [true, "test"], rspec: { script: "test" } })
+        config = YAML.dump({ stages: [true, "test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages config should be an array of strings")
       end
 
       it "returns errors if variables is not a map" do
         config = YAML.dump({ variables: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-value strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables config should be a hash of key value pairs")
       end
 
       it "returns errors if variables is not a map of key-value strings" do
         config = YAML.dump({ variables: { test: false }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-value strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables config should be a hash of key value pairs")
       end
 
       it "returns errors if job when is not on_success, on_failure or always" do
@@ -1160,21 +1173,21 @@ EOT
         config = YAML.dump({ cache: { untracked: "string" }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:untracked parameter should be an boolean")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:untracked config should be a boolean value")
       end
 
       it "returns errors if cache:paths is not an array of strings" do
         config = YAML.dump({ cache: { paths: "string" }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:paths parameter should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:paths config should be an array of strings")
       end
 
       it "returns errors if cache:key is not a string" do
         config = YAML.dump({ cache: { key: 1 }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:key parameter should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:key config should be a string or symbol")
       end
 
       it "returns errors if job cache:key is not an a string" do

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Boolean do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is valid' do
+      let(:config) { false }
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq false
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not valid' do
+      let(:config) { ['incorrect'] }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'boolean config should be a boolean value'
+        end
+      end
+    end
+  end
+end

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Cache do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    before { entry.process! }
+
+    context 'when entry config value is correct' do
+      let(:config) do
+        { key: 'some key',
+          untracked: true,
+          paths: ['some/path/'] }
+      end
+
+      describe '#value' do
+        it 'returns hash value' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not correct' do
+      describe '#errors' do
+        context 'when is not a hash' do
+          let(:config) { 'ls' }
+
+          it 'reports errors with config value' do
+            expect(entry.errors)
+              .to include 'cache config should be a hash'
+          end
+        end
+
+        context 'when descendants are invalid' do
+          let(:config) { { key: 1 } }
+
+          it 'reports error with descendants' do
+            expect(entry.errors)
+              .to include 'key config should be a string or symbol'
+          end
+        end
+
+        context 'when there is an unknown key present' do
+          let(:config) { { invalid: true } }
+
+          it 'reports error with descendants' do
+            expect(entry.errors)
+              .to include 'cache config contains unknown keys: invalid'
+          end
+        end
+      end
+    end
+  end
+end

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

@@ -7,10 +7,42 @@ describe Gitlab::Ci::Config::Node::Configurable do
     node.include(described_class)
   end
 
+  describe 'validations' do
+    let(:validator) { node.validator.new(instance) }
+
+    before do
+      node.class_eval do
+        attr_reader :config
+
+        def initialize(config)
+          @config = config
+        end
+      end
+
+      validator.validate
+    end
+
+    context 'when node validator is invalid' do
+      let(:instance) { node.new('ls') }
+
+      it 'returns invalid validator' do
+        expect(validator).to be_invalid
+      end
+    end
+
+    context 'when node instance is valid' do
+      let(:instance) { node.new(key: 'value') }
+
+      it 'returns valid validator' do
+        expect(validator).to be_valid
+      end
+    end
+  end
+
   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
 

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

@@ -5,13 +5,13 @@ describe Gitlab::Ci::Config::Node::Factory do
     let(:factory) { described_class.new(entry_class) }
     let(:entry_class) { Gitlab::Ci::Config::Node::Script }
 
-    context 'when value setting value' do
+    context 'when setting up a value' do
       it 'creates entry with valid value' do
         entry = factory
           .with(value: ['ls', 'pwd'])
           .create!
 
-        expect(entry.value).to eq "ls\npwd"
+        expect(entry.value).to eq ['ls', 'pwd']
       end
 
       context 'when setting description' do
@@ -21,7 +21,7 @@ describe Gitlab::Ci::Config::Node::Factory do
             .with(description: 'test description')
             .create!
 
-          expect(entry.value).to eq "ls\npwd"
+          expect(entry.value).to eq ['ls', 'pwd']
           expect(entry.description).to eq 'test description'
         end
       end
@@ -35,9 +35,21 @@ describe Gitlab::Ci::Config::Node::Factory do
           expect(entry.key).to eq 'test key'
         end
       end
+
+      context 'when setting a parent' do
+        let(:parent) { Object.new }
+
+        it 'creates entry with valid parent' do
+          entry = factory
+            .with(value: 'ls', parent: parent)
+            .create!
+
+          expect(entry.parent).to eq parent
+        end
+      end
     end
 
-    context 'when not setting value' do
+    context 'when not setting up a value' do
       it 'raises error' do
         expect { factory.create! }.to raise_error(
           Gitlab::Ci::Config::Node::Factory::InvalidFactory
@@ -45,14 +57,13 @@ describe Gitlab::Ci::Config::Node::Factory do
       end
     end
 
-    context 'when creating a null entry' do
-      it 'creates a null entry' do
+    context 'when creating entry with nil value' do
+      it 'creates an undefined entry' do
         entry = factory
           .with(value: nil)
-          .nullify!
           .create!
 
-        expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Null
+        expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
       end
     end
   end

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

@@ -13,57 +13,163 @@ describe Gitlab::Ci::Config::Node::Global do
     end
   end
 
-  describe '#key' do
-    it 'returns underscored class name' do
-      expect(global.key).to eq 'global'
-    end
-  end
-
   context 'when hash is valid' do
-    let(:hash) do
-      { before_script: ['ls', 'pwd'] }
-    end
+    context 'when all entries defined' do
+      let(:hash) do
+        { before_script: ['ls', 'pwd'],
+          image: 'ruby:2.2',
+          services: ['postgres:9.1', 'mysql:5.5'],
+          variables: { VAR: 'value' },
+          after_script: ['make clean'],
+          stages: ['build', 'pages'],
+          cache: { key: 'k', untracked: true, paths: ['public/'] } }
+      end
 
-    describe '#process!' do
-      before { global.process! }
+      describe '#process!' do
+        before { global.process! }
 
-      it 'creates nodes hash' do
-        expect(global.nodes).to be_an Array
+        it 'creates nodes hash' do
+          expect(global.nodes).to be_an Array
+        end
+
+        it 'creates node object for each entry' do
+          expect(global.nodes.count).to eq 8
+        end
+
+        it 'creates node object using valid class' do
+          expect(global.nodes.first)
+            .to be_an_instance_of Gitlab::Ci::Config::Node::Script
+          expect(global.nodes.second)
+            .to be_an_instance_of Gitlab::Ci::Config::Node::Image
+        end
+
+        it 'sets correct description for nodes' do
+          expect(global.nodes.first.description)
+            .to eq 'Script that will be executed before each job.'
+          expect(global.nodes.second.description)
+            .to eq 'Docker image that will be used to execute jobs.'
+        end
       end
 
-      it 'creates node object for each entry' do
-        expect(global.nodes.count).to eq 1
+      describe '#leaf?' do
+        it 'is not leaf' do
+          expect(global).not_to be_leaf
+        end
       end
 
-      it 'creates node object using valid class' do
-        expect(global.nodes.first)
-          .to be_an_instance_of Gitlab::Ci::Config::Node::Script
+      context 'when not processed' do
+        describe '#before_script' do
+          it 'returns nil' do
+            expect(global.before_script).to be nil
+          end
+        end
       end
 
-      it 'sets correct description for nodes' do
-        expect(global.nodes.first.description)
-          .to eq 'Script that will be executed before each job.'
+      context 'when processed' do
+        before { global.process! }
+
+        describe '#before_script' do
+          it 'returns correct script' do
+            expect(global.before_script).to eq ['ls', 'pwd']
+          end
+        end
+
+        describe '#image' do
+          it 'returns valid image' do
+            expect(global.image).to eq 'ruby:2.2'
+          end
+        end
+
+        describe '#services' do
+          it 'returns array of services' do
+            expect(global.services).to eq ['postgres:9.1', 'mysql:5.5']
+          end
+        end
+
+        describe '#after_script' do
+          it 'returns after script' do
+            expect(global.after_script).to eq ['make clean']
+          end
+        end
+
+        describe '#variables' do
+          it 'returns variables' do
+            expect(global.variables).to eq(VAR: 'value')
+          end
+        end
+
+        describe '#stages' do
+          context 'when stages key defined' do
+            it 'returns array of stages' do
+              expect(global.stages).to eq %w[build pages]
+            end
+          end
+
+          context 'when deprecated types key defined' do
+            let(:hash) { { types: ['test', 'deploy'] } }
+
+            it 'returns array of types as stages' do
+              expect(global.stages).to eq %w[test deploy]
+            end
+          end
+        end
+
+        describe '#cache' do
+          it 'returns cache configuration' do
+            expect(global.cache)
+              .to eq(key: 'k', untracked: true, paths: ['public/'])
+          end
+        end
       end
     end
 
-    describe '#leaf?' do
-      it 'is not leaf' do
-        expect(global).not_to be_leaf
+    context 'when most of entires not defined' do
+      let(:hash) { { cache: { key: 'a' }, rspec: {} } }
+      before { global.process! }
+
+      describe '#nodes' do
+        it 'instantizes all nodes' do
+          expect(global.nodes.count).to eq 8
+        end
+
+        it 'contains undefined nodes' do
+          expect(global.nodes.first)
+            .to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
+        end
       end
-    end
 
-    describe '#before_script' do
-      context 'when processed' do
-        before { global.process! }
+      describe '#variables' do
+        it 'returns default value for variables' do
+          expect(global.variables).to eq({})
+        end
+      end
 
-        it 'returns correct script' do
-          expect(global.before_script).to eq "ls\npwd"
+      describe '#stages' do
+        it 'returns an array of default stages' do
+          expect(global.stages).to eq %w[build test deploy]
         end
       end
 
-      context 'when not processed' do
-        it 'returns nil' do
-          expect(global.before_script).to be nil
+      describe '#cache' do
+        it 'returns correct cache definition' do
+          expect(global.cache).to eq(key: 'a')
+        end
+      end
+    end
+
+    ##
+    # When nodes are specified but not defined, we assume that
+    # configuration is valid, and we asume that entry is simply undefined,
+    # despite the fact, that key is present. See issue #18775 for more
+    # details.
+    #
+    context 'when entires specified but not defined' do
+      let(:hash) { { variables: nil } }
+      before { global.process! }
+
+      describe '#variables' do
+        it 'undefined entry returns a default value' do
+          expect(global.variables).to eq({})
         end
       end
     end
@@ -85,7 +191,7 @@ describe Gitlab::Ci::Config::Node::Global do
     describe '#errors' do
       it 'reports errors from child nodes' do
         expect(global.errors)
-          .to include 'Before script config should be an array of strings'
+          .to include 'before_script config should be an array of strings'
       end
     end
 
@@ -106,5 +212,17 @@ describe Gitlab::Ci::Config::Node::Global do
         expect(global).not_to be_valid
       end
     end
+
+    describe '#errors' do
+      it 'returns error about invalid type' do
+        expect(global.errors.first).to match /should be a hash/
+      end
+    end
+  end
+
+  describe '#defined?' do
+    it 'is concrete entry that is defined' do
+      expect(global.defined?).to be true
+    end
   end
 end

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Image do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validation' do
+    context 'when entry config value is correct' do
+      let(:config) { 'ruby:2.2' }
+
+      describe '#value' do
+        it 'returns image string' do
+          expect(entry.value).to eq 'ruby:2.2'
+        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
+
+    context 'when entry value is not correct' do
+      let(:config) { ['ruby:2.2'] }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'image config should be a string'
+        end
+      end
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+    end
+  end
+end

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Key do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { 'test' }
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq 'test'
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not correct' do
+      let(:config) { [ 'incorrect' ] }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'key config should be a string or symbol'
+        end
+      end
+    end
+  end
+end

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

-require 'spec_helper'
-
-describe Gitlab::Ci::Config::Node::Null do
-  let(:entry) { described_class.new(nil) }
-
-  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
-
-  describe '#value' do
-    it 'returns nil' do
-      expect(entry.value).to be nil
-    end
-  end
-end

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Paths do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is valid' do
+      let(:config) { ['some/file', 'some/path/'] }
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not valid' do
+      let(:config) { [ 1 ] }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'paths config should be an array of strings'
+        end
+      end
+    end
+  end
+end

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

@@ -10,8 +10,8 @@ describe Gitlab::Ci::Config::Node::Script do
       let(:config) { ['ls', 'pwd'] }
 
       describe '#value' do
-        it 'returns concatenated command' do
-          expect(entry.value).to eq "ls\npwd"
+        it 'returns array of strings' do
+          expect(entry.value).to eq config
         end
       end
 
@@ -34,7 +34,7 @@ describe Gitlab::Ci::Config::Node::Script do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Script config should be an array of strings'
+            .to include 'script config should be an array of strings'
         end
       end
 

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Services do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { ['postgres:9.1', 'mysql:5.5'] }
+
+      describe '#value' do
+        it 'returns array of services as is' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not correct' do
+      let(:config) { 'ls' }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'services config should be an array of strings'
+        end
+      end
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+    end
+  end
+end

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Stages do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) { [:stage1, :stage2] }
+
+      describe '#value' do
+        it 'returns array of stages' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not correct' do
+      let(:config) { { test: true } }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include 'stages config should be an array of strings'
+        end
+      end
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+    end
+  end
+
+  describe '.default' do
+    it 'returns default stages' do
+      expect(described_class.default).to eq %w[build test deploy]
+    end
+  end
+end

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Undefined do
+  let(:undefined) { described_class.new(entry) }
+  let(:entry) { Class.new }
+
+  describe '#leaf?' do
+    it 'is leaf node' do
+      expect(undefined).to be_leaf
+    end
+  end
+
+  describe '#valid?' do
+    it 'is always valid' do
+      expect(undefined).to be_valid
+    end
+  end
+
+  describe '#errors' do
+    it 'is does not contain errors' do
+      expect(undefined.errors).to be_empty
+    end
+  end
+
+  describe '#value' do
+    before do
+      allow(entry).to receive(:default).and_return('some value')
+    end
+
+    it 'returns default value for entry' do
+      expect(undefined.value).to eq 'some value'
+    end
+  end
+
+  describe '#undefined?' do
+    it 'is not a defined entry' do
+      expect(undefined.defined?).to be false
+    end
+  end
+end

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

@@ -5,7 +5,18 @@ describe Gitlab::Ci::Config::Node::Validator do
   let(:validator_instance) { validator.new(node) }
   let(:node) { spy('node') }
 
-  shared_examples 'delegated validator' do
+  before do
+    allow(node).to receive(:key).and_return('node')
+    allow(node).to receive(:ancestors).and_return([])
+  end
+
+  describe 'delegated validator' do
+    before do
+      validator.class_eval do
+        validates :test_attribute, presence: true
+      end
+    end
+
     context 'when node is valid' do
       before do
         allow(node).to receive(:test_attribute).and_return('valid value')
@@ -19,7 +30,7 @@ describe Gitlab::Ci::Config::Node::Validator do
       it 'returns no errors' do
         validator_instance.validate
 
-        expect(validator_instance.full_errors).to be_empty
+        expect(validator_instance.messages).to be_empty
       end
     end
 
@@ -36,32 +47,9 @@ describe Gitlab::Ci::Config::Node::Validator do
       it 'returns errors' do
         validator_instance.validate
 
-        expect(validator_instance.full_errors).not_to be_empty
+        expect(validator_instance.messages)
+          .to include "node test attribute can't be blank"
       end
     end
   end
-
-  describe 'attributes validations' do
-    before do
-      validator.class_eval do
-        validates :test_attribute, presence: true
-      end
-    end
-
-    it_behaves_like 'delegated validator'
-  end
-
-  describe 'interface validations' do
-    before do
-      validator.class_eval do
-        validate do
-          unless @node.test_attribute == 'valid value'
-            errors.add(:test_attribute, 'invalid value')
-          end
-        end
-      end
-    end
-
-    it_behaves_like 'delegated validator'
-  end
 end

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

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Variables do
+  let(:entry) { described_class.new(config) }
+
+  describe 'validations' do
+    context 'when entry config value is correct' do
+      let(:config) do
+        { 'VARIABLE_1' => 'value 1', 'VARIABLE_2' => 'value 2' }
+      end
+
+      describe '#value' do
+        it 'returns hash with key value strings' do
+          expect(entry.value).to eq config
+        end
+      end
+
+      describe '#errors' do
+        it 'does not append errors' do
+          expect(entry.errors).to be_empty
+        end
+      end
+
+      describe '#valid?' do
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+    end
+
+    context 'when entry value is not correct' do
+      let(:config) { [ :VAR, 'test' ] }
+
+      describe '#errors' do
+        it 'saves errors' do
+          expect(entry.errors)
+            .to include /should be a hash of key value pairs/
+        end
+      end
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+    end
+  end
+end

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