From 2305483d7d0f398caedabf6a99a94913be75138a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 22 Oct 2015 10:38:47 +0200 Subject: Require jobs to be named --- CHANGELOG | 1 + lib/ci/gitlab_ci_yaml_processor.rb | 38 +++++++++++++++++----------- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 14 ++++++++++ 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 671955d70cf..aa70d49435e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -20,6 +20,7 @@ v 8.1.0 (unreleased) - 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) - 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) - 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) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 0da73e387e1..efcd2faffc7 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -139,66 +139,74 @@ module Ci end @jobs.each do |name, job| - validate_job!("#{name} job", job) + validate_job!(name, job) end true end 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| unless ALLOWED_JOB_KEYS.include? key - raise ValidationError, "#{name}: unknown parameter #{key}" + raise ValidationError, "#{name} job: unknown parameter #{key}" end end - if !job[:script].is_a?(String) && !validate_array_of_strings(job[:script]) - raise ValidationError, "#{name}: script should be a string or an array of a strings" + if !validate_string(job[:script]) && !validate_array_of_strings(job[:script]) + raise ValidationError, "#{name} job: script should be a string or an array of a strings" end if job[:stage] 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 - if job[:image] && !job[:image].is_a?(String) - raise ValidationError, "#{name}: image should be a string" + if job[:image] && !validate_string(job[:image]) + raise ValidationError, "#{name} job: image should be a string" end 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 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 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 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 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 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 private 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 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 diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 2260a6f8130..abdb6b89ac5 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -218,6 +218,20 @@ module Ci end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image should be a string") 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 config = YAML.dump({ rspec: { script: "test", image: ["test"] } }) expect do -- cgit v1.2.1