diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index 26028547fa0e43406886fd25d55932050ef29c24..adfd097736e3e0347dd6dd889ed13b4aa38b658e 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -17,11 +17,11 @@ module Gitlab
end
def valid?
- errors.none?
+ @global.valid?
end
def errors
- @global.errors.map(&:to_s)
+ @global.errors
end
def to_hash
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 2f4abbf9ffee4bbdddb701aed29c4cfd2a535488..23d3f9f3277a61c806cc1e49896df4d518100fd4 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -16,21 +16,27 @@ module Gitlab
module Configurable
extend ActiveSupport::Concern
+ included do
+ validations do
+ validate :hash_config_value
+
+ def hash_config_value
+ unless self.config.is_a?(Hash)
+ errors.add(:config, 'should be a configuration entry hash')
+ end
+ end
+ end
+ end
+
def allowed_nodes
self.class.allowed_nodes || {}
end
private
- def prevalidate!
- unless @value.is_a?(Hash)
- add_error('should be a configuration entry with hash value')
- end
- end
-
def create_node(key, factory)
- factory.with(value: @value[key], key: key)
- factory.nullify! unless @value.has_key?(key)
+ factory.with(value: @config[key], key: key)
+ factory.nullify! unless @config.has_key?(key)
factory.create!
end
@@ -47,7 +53,6 @@ module Gitlab
define_method(symbol) do
raise Entry::InvalidError unless valid?
-
@nodes[symbol].try(:value)
end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index f41e191d611d9db5330a196d9f22e6b08cd1629f..823c8bb5d13d37dfcf31a1c3e722b2a63bd69f62 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -7,16 +7,15 @@ module Gitlab
#
class Entry
class InvalidError < StandardError; end
+ include Validatable
- attr_writer :key
- attr_accessor :description
+ attr_reader :config
+ attr_accessor :key, :description
- def initialize(value)
- @value = value
+ def initialize(config)
+ @config = config
@nodes = {}
- @errors = []
-
- prevalidate!
+ @validator = self.class.validator.new(self)
end
def process!
@@ -24,19 +23,13 @@ module Gitlab
return unless valid?
compose!
-
- nodes.each(&:process!)
- nodes.each(&:validate!)
+ process_nodes!
end
def nodes
@nodes.values
end
- def valid?
- errors.none?
- end
-
def leaf?
allowed_nodes.none?
end
@@ -45,37 +38,35 @@ module Gitlab
@key || self.class.name.demodulize.underscore
end
- def errors
- @errors + nodes.map(&:errors).flatten
+ def valid?
+ errors.none?
end
- def add_error(message)
- @errors << Error.new(message, self)
+ def errors
+ @validator.full_errors +
+ nodes.map(&:errors).flatten
end
def allowed_nodes
{}
end
- def validate!
- raise NotImplementedError
- end
-
def value
raise NotImplementedError
end
private
- def prevalidate!
- end
-
def compose!
allowed_nodes.each do |key, essence|
@nodes[key] = create_node(key, essence)
end
end
+ def process_nodes!
+ nodes.each(&:process!)
+ end
+
def create_node(key, essence)
raise NotImplementedError
end
diff --git a/lib/gitlab/ci/config/node/error.rb b/lib/gitlab/ci/config/node/error.rb
deleted file mode 100644
index 5b8d0288e4e726b60eda951494b278f391974c97..0000000000000000000000000000000000000000
--- a/lib/gitlab/ci/config/node/error.rb
+++ /dev/null
@@ -1,26 +0,0 @@
-module Gitlab
- module Ci
- class Config
- module Node
- class Error
- def initialize(message, parent)
- @message = message
- @parent = parent
- end
-
- def key
- @parent.key
- end
-
- def to_s
- "#{key}: #{@message}"
- end
-
- def ==(other)
- other.to_s == to_s
- 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 314877a035c26e558aae22043b6a73e9e8a12f15..18592acdac6312f6b031262a1ece19971664516f 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -11,17 +11,21 @@ module Gitlab
# implementation in Runner.
#
class Script < Entry
- include ValidationHelpers
+ validations do
+ include ValidationHelpers
- def value
- @value.join("\n")
- end
+ validate :array_of_strings
- def validate!
- unless validate_array_of_strings(@value)
- add_error('should be an array of strings')
+ def array_of_strings
+ unless validate_array_of_strings(self.config)
+ errors.add(:config, 'should be an array of strings')
+ end
end
end
+
+ def value
+ @config.join("\n")
+ end
end
end
end
diff --git a/lib/gitlab/ci/config/node/validatable.rb b/lib/gitlab/ci/config/node/validatable.rb
new file mode 100644
index 0000000000000000000000000000000000000000..a0617d21ad88eda5fc77ede607c48b7946544441
--- /dev/null
+++ b/lib/gitlab/ci/config/node/validatable.rb
@@ -0,0 +1,30 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ module Validatable
+ extend ActiveSupport::Concern
+
+ class_methods do
+ def validator
+ validator = Class.new(Node::Validator)
+ validator.include(ActiveModel::Validations)
+
+ if defined?(@validations)
+ @validations.each { |rules| validator.class_eval(&rules) }
+ end
+
+ validator
+ end
+
+ private
+
+ def validations(&block)
+ (@validations ||= []).append(block)
+ end
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
new file mode 100644
index 0000000000000000000000000000000000000000..891b19c9743680d604a7031b2f540c471f9c95c2
--- /dev/null
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -0,0 +1,25 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ class Validator < SimpleDelegator
+ def initialize(node)
+ @node = node
+ super(node)
+ validate
+ end
+
+ def full_errors
+ errors.full_messages.map do |error|
+ "#{@node.key} #{error}".humanize
+ end
+ end
+
+ def self.name
+ 'Validator'
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 40ee1eb64ca2e86a03437d783c182ece5ae7928d..42cbb55569442d08cc3dc9cdabb00454471530ed 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -820,7 +820,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: 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
diff --git a/spec/lib/gitlab/ci/config/node/configurable_spec.rb b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
index 47c68f96dc8ab603743f67ee38aa0e0b8dff4742..8ec7919f4d457f4a644dde3621393a25f1779e23 100644
--- a/spec/lib/gitlab/ci/config/node/configurable_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
@@ -4,6 +4,7 @@ describe Gitlab::Ci::Config::Node::Configurable do
let(:node) { Class.new }
before do
+ node.include(Gitlab::Ci::Config::Node::Validatable)
node.include(described_class)
end
diff --git a/spec/lib/gitlab/ci/config/node/error_spec.rb b/spec/lib/gitlab/ci/config/node/error_spec.rb
deleted file mode 100644
index 764fdfdb821a21a7694768373e8bde1fc5de631d..0000000000000000000000000000000000000000
--- a/spec/lib/gitlab/ci/config/node/error_spec.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Ci::Config::Node::Error do
- let(:error) { described_class.new(message, parent) }
- let(:message) { 'some error' }
- let(:parent) { spy('parent') }
-
- before do
- allow(parent).to receive(:key).and_return('parent_key')
- end
-
- describe '#key' do
- it 'returns underscored class name' do
- expect(error.key).to eq 'parent_key'
- end
- end
-
- describe '#to_s' do
- it 'returns valid error message' do
- expect(error.to_s).to eq 'parent_key: some error'
- end
- end
-end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 89a7183b65591343a6794180962b107af267a8f9..8bb76cf920ae54a9670e146f5ec9ffac3782d713 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -85,7 +85,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: should be an array of strings'
+ .to include 'Before script config should be an array of strings'
end
end
diff --git a/spec/lib/gitlab/ci/config/node/script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
index ff7016046cc8645f2def66fce4fcbb1cbcee1620..6af6aa15eef2dbe618bc20bdda64b9cfb3531d1b 100644
--- a/spec/lib/gitlab/ci/config/node/script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -1,13 +1,13 @@
require 'spec_helper'
describe Gitlab::Ci::Config::Node::Script do
- let(:entry) { described_class.new(value) }
+ let(:entry) { described_class.new(config) }
- describe '#validate!' do
- before { entry.validate! }
+ describe '#process!' do
+ before { entry.process! }
- context 'when entry value is correct' do
- let(:value) { ['ls', 'pwd'] }
+ context 'when entry config value is correct' do
+ let(:config) { ['ls', 'pwd'] }
describe '#value' do
it 'returns concatenated command' do
@@ -29,12 +29,12 @@ describe Gitlab::Ci::Config::Node::Script do
end
context 'when entry value is not correct' do
- let(:value) { 'ls' }
+ let(:config) { 'ls' }
describe '#errors' do
it 'saves errors' do
expect(entry.errors)
- .to include 'script: should be an array of strings'
+ .to include 'Script config should be an array of strings'
end
end