Commit 00244a89 authored by Robert Speicher's avatar Robert Speicher Committed by Robert Speicher

Merge branch 'fail-on-empty-job-name' into 'master'

Require jobs to be named

This properly presents error for .gitlab-ci.yml having empty job name.

See merge request !1662
parent 60e82ef8
...@@ -12,6 +12,7 @@ v 8.1.0 ...@@ -12,6 +12,7 @@ v 8.1.0
- Don't show "Add README" link in an empty repository if user doesn't have access to push (Stan Hu) - Don't show "Add README" link in an empty repository if user doesn't have access to push (Stan Hu)
- Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu) - Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu)
- Speed up load times of issue detail pages by roughly 1.5x - Speed up load times of issue detail pages by roughly 1.5x
- Require CI jobs to be named
- If a merge request is to close an issue, show this on the issue page (Zeger-Jan van de Weg) - If a merge request is to close an issue, show this on the issue page (Zeger-Jan van de Weg)
- Add a system note and update relevant merge requests when a branch is deleted or re-added (Stan Hu) - Add a system note and update relevant merge requests when a branch is deleted or re-added (Stan Hu)
- Make diff file view easier to use on mobile screens (Stan Hu) - Make diff file view easier to use on mobile screens (Stan Hu)
......
...@@ -139,66 +139,74 @@ module Ci ...@@ -139,66 +139,74 @@ module Ci
end end
@jobs.each do |name, job| @jobs.each do |name, job|
validate_job!("#{name} job", job) validate_job!(name, job)
end end
true true
end end
def validate_job!(name, job) def validate_job!(name, job)
if name.blank? || !validate_string(name)
raise ValidationError, "job name should be non-empty string"
end
job.keys.each do |key| job.keys.each do |key|
unless ALLOWED_JOB_KEYS.include? key unless ALLOWED_JOB_KEYS.include? key
raise ValidationError, "#{name}: unknown parameter #{key}" raise ValidationError, "#{name} job: unknown parameter #{key}"
end end
end end
if !job[:script].is_a?(String) && !validate_array_of_strings(job[:script]) if !validate_string(job[:script]) && !validate_array_of_strings(job[:script])
raise ValidationError, "#{name}: script should be a string or an array of a strings" raise ValidationError, "#{name} job: script should be a string or an array of a strings"
end end
if job[:stage] if job[:stage]
unless job[:stage].is_a?(String) && job[:stage].in?(stages) unless job[:stage].is_a?(String) && job[:stage].in?(stages)
raise ValidationError, "#{name}: stage parameter should be #{stages.join(", ")}" raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}"
end end
end end
if job[:image] && !job[:image].is_a?(String) if job[:image] && !validate_string(job[:image])
raise ValidationError, "#{name}: image should be a string" raise ValidationError, "#{name} job: image should be a string"
end end
if job[:services] && !validate_array_of_strings(job[:services]) if job[:services] && !validate_array_of_strings(job[:services])
raise ValidationError, "#{name}: services should be an array of strings" raise ValidationError, "#{name} job: services should be an array of strings"
end end
if job[:tags] && !validate_array_of_strings(job[:tags]) if job[:tags] && !validate_array_of_strings(job[:tags])
raise ValidationError, "#{name}: tags parameter should be an array of strings" raise ValidationError, "#{name} job: tags parameter should be an array of strings"
end end
if job[:only] && !validate_array_of_strings(job[:only]) if job[:only] && !validate_array_of_strings(job[:only])
raise ValidationError, "#{name}: only parameter should be an array of strings" raise ValidationError, "#{name} job: only parameter should be an array of strings"
end end
if job[:except] && !validate_array_of_strings(job[:except]) if job[:except] && !validate_array_of_strings(job[:except])
raise ValidationError, "#{name}: except parameter should be an array of strings" raise ValidationError, "#{name} job: except parameter should be an array of strings"
end end
if job[:allow_failure] && !job[:allow_failure].in?([true, false]) if job[:allow_failure] && !job[:allow_failure].in?([true, false])
raise ValidationError, "#{name}: allow_failure parameter should be an boolean" raise ValidationError, "#{name} job: allow_failure parameter should be an boolean"
end end
if job[:when] && !job[:when].in?(%w(on_success on_failure always)) if job[:when] && !job[:when].in?(%w(on_success on_failure always))
raise ValidationError, "#{name}: when parameter should be on_success, on_failure or always" raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always"
end end
end end
private private
def validate_array_of_strings(values) def validate_array_of_strings(values)
values.is_a?(Array) && values.all? {|tag| tag.is_a?(String)} values.is_a?(Array) && values.all? { |value| validate_string(value) }
end end
def validate_variables(variables) def validate_variables(variables)
variables.is_a?(Hash) && variables.all? {|key, value| key.is_a?(Symbol) && value.is_a?(String)} variables.is_a?(Hash) && variables.all? { |key, value| validate_string(key) && validate_string(value) }
end
def validate_string(value)
value.is_a?(String) || value.is_a?(Symbol)
end end
end end
end end
...@@ -218,6 +218,20 @@ module Ci ...@@ -218,6 +218,20 @@ module Ci
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image should be a string") end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image should be a string")
end end
it "returns errors if job name is blank" do
config = YAML.dump({ '' => { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config)
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string")
end
it "returns errors if job name is non-string" do
config = YAML.dump({ 10 => { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config)
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string")
end
it "returns errors if job image parameter is invalid" do it "returns errors if job image parameter is invalid" do
config = YAML.dump({ rspec: { script: "test", image: ["test"] } }) config = YAML.dump({ rspec: { script: "test", image: ["test"] } })
expect do expect do
......
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