diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 2fd76bc3690..29a52b9da17 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -16,8 +16,11 @@ module Gitlab dependencies needs before_script after_script variables environment coverage retry parallel extends].freeze + REQUIRED_BY_NEEDS = %i[stage].freeze + validations do validates :config, allowed_keys: ALLOWED_KEYS + validates :config, required_keys: REQUIRED_BY_NEEDS, if: :has_needs? validates :config, presence: true validates :script, presence: true validates :name, presence: true diff --git a/lib/gitlab/config/entry/attributable.rb b/lib/gitlab/config/entry/attributable.rb index 560fe63df0e..87bd257f69a 100644 --- a/lib/gitlab/config/entry/attributable.rb +++ b/lib/gitlab/config/entry/attributable.rb @@ -18,6 +18,10 @@ module Gitlab config[attribute] end + + define_method("has_#{attribute}?") do + config.is_a?(Hash) && config.key?(attribute) + end end end end diff --git a/lib/gitlab/config/entry/validators.rb b/lib/gitlab/config/entry/validators.rb index 6796fcce75f..0289e675c6b 100644 --- a/lib/gitlab/config/entry/validators.rb +++ b/lib/gitlab/config/entry/validators.rb @@ -26,6 +26,17 @@ module Gitlab end end + class RequiredKeysValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + present_keys = options[:in] - value.try(:keys).to_a + + if present_keys.any? + record.errors.add(attribute, "missing required keys: " + + present_keys.join(', ')) + end + end + end + class AllowedValuesValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) unless options[:in].include?(value.to_s) diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 800ef122203..415ade7a096 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -89,14 +89,23 @@ describe Gitlab::Ci::Config::Entry::Job do context 'when has needs' do let(:config) do - { script: 'echo', needs: ['another-job'] } + { + stage: 'test', + script: 'echo', + needs: ['another-job'] + } end it { expect(entry).to be_valid } context 'when has dependencies' do let(:config) do - { script: 'echo', dependencies: ['another-job'], needs: ['another-job'] } + { + stage: 'test', + script: 'echo', + dependencies: ['another-job'], + needs: ['another-job'] + } end it { expect(entry).to be_valid } @@ -256,7 +265,11 @@ describe Gitlab::Ci::Config::Entry::Job do context 'when has needs' do context 'that are not a array of strings' do let(:config) do - { script: 'echo', needs: 'build-job' } + { + stage: 'test', + script: 'echo', + needs: 'build-job' + } end it 'returns error about invalid type' do @@ -267,7 +280,12 @@ describe Gitlab::Ci::Config::Entry::Job do context 'when have dependencies that are not subset of needs' do let(:config) do - { script: 'echo', dependencies: ['another-job'], needs: ['build-job'] } + { + stage: 'test', + script: 'echo', + dependencies: ['another-job'], + needs: ['build-job'] + } end it 'returns error about invalid data' do @@ -275,6 +293,20 @@ describe Gitlab::Ci::Config::Entry::Job do expect(entry.errors).to include 'job dependencies the another-job should be part of needs' end end + + context 'when stage: is missing' do + let(:config) do + { + script: 'echo', + needs: ['build-job'] + } + end + + it 'returns error about invalid data' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'job config missing required keys: stage' + end + end end end end diff --git a/spec/lib/gitlab/config/entry/attributable_spec.rb b/spec/lib/gitlab/config/entry/attributable_spec.rb index 82614bb8238..6b548d5c4a8 100644 --- a/spec/lib/gitlab/config/entry/attributable_spec.rb +++ b/spec/lib/gitlab/config/entry/attributable_spec.rb @@ -25,7 +25,9 @@ describe Gitlab::Config::Entry::Attributable do end it 'returns the value of config' do + expect(instance).to have_name expect(instance.name).to eq 'some name' + expect(instance).to have_test expect(instance.test).to eq 'some test' end @@ -42,6 +44,7 @@ describe Gitlab::Config::Entry::Attributable do end it 'returns nil' do + expect(instance).not_to have_test expect(instance.test).to be_nil end end