Improve CI job entry validations in new config

5 files changed, 31 insertions, 24 deletions

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 61075d3b923..e18d5b907b4 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -102,7 +102,6 @@ module Ci
     def validate_job!(name, job)
       raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
 
-      validate_job_name!(name)
       validate_job_keys!(name, job)
       validate_job_types!(name, job)
 
@@ -112,12 +111,6 @@ module Ci
       validate_job_dependencies!(name, job) if job[:dependencies]
     end
 
-    def validate_job_name!(name)
-      if name.blank? || !validate_string(name)
-        raise ValidationError, "job name should be non-empty string"
-      end
-    end
-
     def validate_job_keys!(name, job)
       ##
       # TODO, remove refactoring keys
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 8bd752b0e2a..b33d743e2c3 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -55,8 +55,10 @@ module Gitlab
                 end
 
                 define_method("#{symbol}_value") do
-                  return unless valid?
-                  @entries[symbol].value if @entries[symbol]
+                  if @entries[symbol]
+                    return unless @entries[symbol].valid?
+                    @entries[symbol].value
+                  end
                 end
 
                 alias_method symbol.to_sym, "#{symbol}_value".to_sym
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index bb1c3386bd4..822b428926a 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -10,7 +10,12 @@ module Gitlab
 
           validations do
             validates :config, presence: true
-            validates :global, required: true, on: :processed
+
+            with_options on: :processed do
+              validates :global, required: true
+              validates :name, presence: true
+              validates :name, type: Symbol
+            end
           end
 
           node :before_script, Script,
@@ -30,11 +35,11 @@ module Gitlab
 
           helpers :before_script, :script, :stage, :type, :after_script
 
+          def name
+            @key
+          end
+
           def value
-            ##
-            # TODO, refactoring step: do not expose internal configuration,
-            # return only hash value without merging it to internal config.
-            #
             @config.merge(to_hash.compact)
           end
 
@@ -53,10 +58,9 @@ module Gitlab
           private
 
           def to_hash
-            { before_script: before_script,
-              script: script,
-              commands: commands,
+            { script: script,
               stage: stage,
+              commands: commands,
               after_script: after_script }
           end
 
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 548441df37c..3c1851b9fea 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -10,11 +10,12 @@ module Gitlab
 
           validations do
             validates :config, type: Hash
-            validate :jobs_presence, on: :processed
 
-            def jobs_presence
-              unless relevant?
-                errors.add(:config, 'should contain at least one visible job')
+            with_options on: :processed do
+              validate do
+                unless has_visible_job?
+                  errors.add(:config, 'should contain at least one visible job')
+                end
               end
             end
           end
@@ -23,7 +24,7 @@ module Gitlab
             @config
           end
 
-          def relevant?
+          def has_visible_job?
             @entries.values.any?(&:relevant?)
           end
 
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index dcfeb194374..ca000f245aa 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -31,8 +31,15 @@ module Gitlab
 
           def location
             predecessors = ancestors.map(&:key).compact
-            current = key || @node.class.name.demodulize.underscore.humanize
-            predecessors.append(current).join(':')
+            predecessors.append(key_name).join(':')
+          end
+
+          def key_name
+            if key.blank? || key.nil?
+              @node.class.name.demodulize.underscore.humanize
+            else
+              key
+            end
           end
         end
       end