Validate environment name with regex

4 files changed, 78 insertions, 10 deletions

diff --git a/app/models/environment.rb b/app/models/environment.rb
index 623404ba634..b29cca8fbe2 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -3,7 +3,11 @@ class Environment < ActiveRecord::Base
 
   has_many :deployments
 
-  validates_presence_of :name
+  validates :name,
+            presence: true,
+            length: { within: 0..255 },
+            format: { with: Gitlab::Regex.environment_name_regex,
+                      message: Gitlab::Regex.environment_name_regex_message }
 
   def last_deployment
     deployments.last
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 66f1bcea4ff..b19ce4aaff9 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -214,8 +214,8 @@ module Ci
         raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always"
       end
 
-      if job[:environment] && !validate_string(job[:environment])
-        raise ValidationError, "#{name} job: environment should be a string"
+      if job[:environment] && !validate_environment(job[:environment])
+        raise ValidationError, "#{name} job: environment parameter #{Gitlab::Regex.environment_name_regex_message}"
       end
     end
 
@@ -322,6 +322,10 @@ module Ci
       value.in?([true, false])
     end
 
+    def validate_environment(value)
+      value.is_a?(String) && value =~ Gitlab::Regex.environment_name_regex
+    end
+
     def process?(only_params, except_params, ref, tag, trigger_request)
       if only_params.present?
         return false unless matching?(only_params, ref, tag, trigger_request)
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 1cbd6d945a0..c84c68f96f6 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -100,5 +100,13 @@ module Gitlab
     def container_registry_reference_regex
       git_reference_regex
     end
+
+    def environment_name_regex
+      @environment_name_regex ||= /\A[a-zA-Z0-9_-]+\z/.freeze
+    end
+
+    def environment_name_regex_message
+      "can contain only letters, digits, '-' and '_'."
+    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 304290d6608..530aa79955a 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -26,7 +26,8 @@ module Ci
           tag_list: [],
           options: {},
           allow_failure: false,
-          when: "on_success"
+          when: "on_success",
+          environment: nil,
         })
       end
 
@@ -387,7 +388,8 @@ module Ci
             services: ["mysql"]
           },
           allow_failure: false,
-          when: "on_success"
+          when: "on_success",
+          environment: nil,
         })
       end
 
@@ -415,7 +417,8 @@ module Ci
             services: ["postgresql"]
           },
           allow_failure: false,
-          when: "on_success"
+          when: "on_success",
+          environment: nil,
         })
       end
     end
@@ -599,7 +602,8 @@ module Ci
             }
           },
           when: "on_success",
-          allow_failure: false
+          allow_failure: false,
+          environment: nil,
         })
       end
 
@@ -621,6 +625,51 @@ module Ci
       end
     end
 
+    describe '#environment' do
+      let(:config) do
+        {
+          deploy_to_production: { stage: 'deploy', script: 'test', environment: environment }
+        }
+      end
+
+      let(:processor) { GitlabCiYamlProcessor.new(YAML.dump(config)) }
+      let(:builds) { processor.builds_for_stage_and_ref('deploy', 'master') }
+
+      context 'when a production environment is specified' do
+        let(:environment) { 'production' }
+
+        it 'does return production' do
+          expect(builds.size).to eq(1)
+          expect(builds.first[:environment]).to eq(environment)
+        end
+      end
+
+      context 'when no environment is specified' do
+        let(:environment) { nil }
+
+        it 'does return nil environment' do
+          expect(builds.size).to eq(1)
+          expect(builds.first[:environment]).to be_nil
+        end
+      end
+
+      context 'is not a string' do
+        let(:environment) { 1 }
+
+        it 'raises error' do
+          expect { builds }.to raise_error("deploy_to_production job: environment parameter #{Gitlab::Regex.environment_name_regex_message}")
+        end
+      end
+
+      context 'is not a valid string' do
+        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}")
+        end
+      end
+    end
+
     describe "Dependencies" do
       let(:config) do
         {
@@ -682,7 +731,8 @@ module Ci
             tag_list: [],
             options: {},
             when: "on_success",
-            allow_failure: false
+            allow_failure: false,
+            environment: nil,
           })
         end
       end
@@ -727,7 +777,8 @@ module Ci
             tag_list: [],
             options: {},
             when: "on_success",
-            allow_failure: false
+            allow_failure: false,
+            environment: nil,
           })
           expect(subject.second).to eq({
             except: nil,
@@ -739,7 +790,8 @@ module Ci
             tag_list: [],
             options: {},
             when: "on_success",
-            allow_failure: false
+            allow_failure: false,
+            environment: nil,
           })
         end
       end