Commit 7cef4f19 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Improve valid keys validation for CI config nodes

parent 24b686eb
...@@ -106,7 +106,6 @@ module Ci ...@@ -106,7 +106,6 @@ module Ci
validate_job_types!(name, job) validate_job_types!(name, job)
validate_job_stage!(name, job) if job[:stage] validate_job_stage!(name, job) if job[:stage]
validate_job_artifacts!(name, job) if job[:artifacts]
validate_job_dependencies!(name, job) if job[:dependencies] validate_job_dependencies!(name, job) if job[:dependencies]
end end
...@@ -142,14 +141,6 @@ module Ci ...@@ -142,14 +141,6 @@ module Ci
end end
end end
def validate_job_artifacts!(name, job)
job[:artifacts].keys.each do |key|
unless ALLOWED_ARTIFACTS_KEYS.include? key
raise ValidationError, "#{name} job: artifacts unknown parameter #{key}"
end
end
end
def validate_job_dependencies!(name, job) def validate_job_dependencies!(name, job)
unless validate_array_of_strings(job[:dependencies]) unless validate_array_of_strings(job[:dependencies])
raise ValidationError, "#{name} job: dependencies parameter should be an array of strings" raise ValidationError, "#{name} job: dependencies parameter should be an array of strings"
......
...@@ -13,6 +13,8 @@ module Gitlab ...@@ -13,6 +13,8 @@ module Gitlab
validations do validations do
validates :config, type: Hash validates :config, type: Hash
validates :config,
allowed_keys: %i[name untracked paths when expire_in]
with_options allow_nil: true do with_options allow_nil: true do
validates :name, type: String validates :name, type: String
......
...@@ -8,6 +8,10 @@ module Gitlab ...@@ -8,6 +8,10 @@ module Gitlab
class Cache < Entry class Cache < Entry
include Configurable include Configurable
validations do
validates :config, allowed_keys: %i[key untracked paths]
end
node :key, Node::Key, node :key, Node::Key,
description: 'Cache key used to define a cache affinity.' description: 'Cache key used to define a cache affinity.'
...@@ -16,10 +20,6 @@ module Gitlab ...@@ -16,10 +20,6 @@ module Gitlab
node :paths, Node::Paths, node :paths, Node::Paths,
description: 'Specify which paths should be cached across builds.' description: 'Specify which paths should be cached across builds.'
validations do
validates :config, allowed_keys: true
end
end end
end end
end end
......
...@@ -21,12 +21,6 @@ module Gitlab ...@@ -21,12 +21,6 @@ module Gitlab
'Validator' 'Validator'
end end
def unknown_keys
return [] unless config.is_a?(Hash)
config.keys - @node.class.nodes.keys
end
private private
def location def location
...@@ -35,7 +29,7 @@ module Gitlab ...@@ -35,7 +29,7 @@ module Gitlab
end end
def key_name def key_name
if key.blank? || key.nil? if key.blank?
@node.class.name.demodulize.underscore.humanize @node.class.name.demodulize.underscore.humanize
else else
key key
......
...@@ -5,10 +5,11 @@ module Gitlab ...@@ -5,10 +5,11 @@ module Gitlab
module Validators module Validators
class AllowedKeysValidator < ActiveModel::EachValidator class AllowedKeysValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value) def validate_each(record, attribute, value)
if record.unknown_keys.any? unknown_keys = record.config.try(:keys).to_a - options[:in]
unknown_list = record.unknown_keys.join(', ')
record.errors.add(:config, if unknown_keys.any?
"contains unknown keys: #{unknown_list}") record.errors.add(:config, 'contains unknown keys: ' +
unknown_keys.join(', '))
end end
end end
end end
......
...@@ -21,14 +21,25 @@ describe Gitlab::Ci::Config::Node::Artifacts do ...@@ -21,14 +21,25 @@ describe Gitlab::Ci::Config::Node::Artifacts do
end end
context 'when entry value is not correct' do context 'when entry value is not correct' do
describe '#errors' do
context 'when value of attribute is invalid' do
let(:config) { { name: 10 } } let(:config) { { name: 10 } }
describe '#errors' do it 'reports error' do
it 'saves errors' do
expect(entry.errors) expect(entry.errors)
.to include 'artifacts name should be a string' .to include 'artifacts name should be a string'
end end
end end
context 'when there is uknown key' do
let(:config) { { test: 100 } }
it 'reports error' do
expect(entry.errors)
.to include 'artifacts config contains unknown keys: test'
end
end
end
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment