Move job environment validation to new CI config

5 files changed, 18 insertions, 20 deletions

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 3b6fcd909f4..aa2f7743a5e 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -93,21 +93,10 @@ module Ci
     def validate_job!(name, job)
       raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
 
-      validate_job_types!(name, job)
       validate_job_stage!(name, job) if job[:stage]
       validate_job_dependencies!(name, job) if job[:dependencies]
     end
 
-    def validate_job_types!(name, job)
-      if job[:when] && !job[:when].in?(%w[on_success on_failure always])
-        raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always"
-      end
-
-      if job[:environment] && !validate_environment(job[:environment])
-        raise ValidationError, "#{name} job: environment parameter #{Gitlab::Regex.environment_name_regex_message}"
-      end
-    end
-
     def validate_job_stage!(name, job)
       unless job[:stage].is_a?(String) && job[:stage].in?(@stages)
         raise ValidationError, "#{name} job: stage parameter should be #{@stages.join(", ")}"
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index c0ec2236231..464abd4d2d8 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -9,7 +9,7 @@ module Gitlab
           include Configurable
           include Attributable
 
-          attributes :tags, :allow_failure
+          attributes :tags, :allow_failure, :when, :environment
 
           validations do
             validates :config, allowed_keys:
@@ -24,6 +24,18 @@ module Gitlab
             with_options allow_nil: true do
               validates :tags, array_of_strings: true
               validates :allow_failure, boolean: true
+              validates :when,
+                inclusion: { in: %w[on_success on_failure always],
+                             message: 'should be on_success, on_failure ' \
+                                      'or always' }
+              validates :environment,
+                type: {
+                  with: String,
+                  message: Gitlab::Regex.environment_name_regex_message }
+              validates :environment,
+                format: {
+                  with: Gitlab::Regex.environment_name_regex,
+                  message: Gitlab::Regex.environment_name_regex_message }
             end
           end
 
diff --git a/lib/gitlab/ci/config/node/legacy_validation_helpers.rb b/lib/gitlab/ci/config/node/legacy_validation_helpers.rb
index 4d9a508796a..0c291efe6a5 100644
--- a/lib/gitlab/ci/config/node/legacy_validation_helpers.rb
+++ b/lib/gitlab/ci/config/node/legacy_validation_helpers.rb
@@ -41,10 +41,6 @@ module Gitlab
             false
           end
 
-          def validate_environment(value)
-            value.is_a?(String) && value =~ Gitlab::Regex.environment_name_regex
-          end
-
           def validate_boolean(value)
             value.in?([true, false])
           end
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index b43a0bc0bab..23d5faf6f07 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -69,7 +69,8 @@ module Gitlab
               raise unless type.is_a?(Class)
 
               unless value.is_a?(type)
-                record.errors.add(attribute, "should be a #{type.name}")
+                message = options[:message] || "should be a #{type.name}"
+                record.errors.add(attribute, message)
               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 d354ddb7b13..9735af27aa1 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -754,7 +754,7 @@ module Ci
         let(:environment) { 1 }
 
         it 'raises error' do
-          expect { builds }.to raise_error("deploy_to_production job: environment parameter #{Gitlab::Regex.environment_name_regex_message}")
+          expect { builds }.to raise_error("jobs:deploy_to_production environment #{Gitlab::Regex.environment_name_regex_message}")
         end
       end
 
@@ -762,7 +762,7 @@ module Ci
         let(:environment) { 'production staging' }
 
         it 'raises error' do
-          expect { builds }.to raise_error("deploy_to_production job: environment parameter #{Gitlab::Regex.environment_name_regex_message}")
+          expect { builds }.to raise_error("jobs:deploy_to_production environment #{Gitlab::Regex.environment_name_regex_message}")
         end
       end
     end
@@ -1131,7 +1131,7 @@ EOT
         config = YAML.dump({ rspec: { script: "test", when: 1 } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: when parameter should be on_success, on_failure or always")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec when should be on_success, on_failure or always")
       end
 
       it "returns errors if job artifacts:name is not an a string" do