diff options
author | Rémy Coutable <remy@rymai.me> | 2016-07-05 11:37:16 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-07-05 11:37:16 +0300 |
commit | 06c7d6f3a863a1ac8d9f47fed8423387d6e672a6 (patch) | |
tree | c3c182c203626e4fd67f13b2df25998f46d57f79 /lib | |
parent | ba9ef7f3935cfaa42fcdb2317567cc383c7e9c22 (diff) | |
parent | bfad4c61f10f689868817cf0b94cddaa1de22240 (diff) |
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
Diffstat (limited to 'lib')
-rw-r--r-- | lib/ci/gitlab_ci_yaml_processor.rb | 93 | ||||
-rw-r--r-- | lib/gitlab/ci/config.rb | 3 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/boolean.rb | 18 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/cache.rb | 27 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/configurable.rb | 30 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/entry.rb | 23 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/factory.rb | 30 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/global.rb | 30 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/image.rb | 18 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/key.rb | 18 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/null.rb | 27 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/paths.rb | 18 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/script.rb | 9 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/services.rb | 18 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/stages.rb | 22 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/undefined.rb | 30 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/validator.rb | 18 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/validators.rb | 49 | ||||
-rw-r--r-- | lib/gitlab/ci/config/node/variables.rb | 22 |
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 |