Update support for dynamic environments

7 files changed, 49 insertions, 32 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 47dedef38d0..d5724af4cce 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -86,7 +86,7 @@ module Ci
             ref: build.ref,
             tag: build.tag,
             options: build.options[:environment],
-            variables: variables)
+            variables: build.variables)
           service.execute(build)
         end
       end
diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb
index 577ba731583..e6667132e27 100644
--- a/app/services/create_deployment_service.rb
+++ b/app/services/create_deployment_service.rb
@@ -2,13 +2,7 @@ require_relative 'base_service'
 
 class CreateDeploymentService < BaseService
   def execute(deployable = nil)
-    environment = project.environments.find_or_create_by(
-      name: expanded_name
-    )
-
-    if expanded_url
-      environment.external_url = expanded_url
-    end
+    environment = find_or_create_environment
 
     project.deployments.create(
       environment: environment,
@@ -22,6 +16,12 @@ class CreateDeploymentService < BaseService
 
   private
 
+  def find_or_create_environment
+    project.environments.find_or_create_by(name: expanded_name) do |environment|
+      environment.external_url = expanded_url
+    end
+  end
+
   def expanded_name
     ExpandVariables.expand(name, variables)
   end
@@ -41,7 +41,7 @@ class CreateDeploymentService < BaseService
   end
 
   def options
-    params[:environment] || {}
+    params[:options] || {}
   end
 
   def variables
diff --git a/lib/expand_variables.rb b/lib/expand_variables.rb
index 669735cc56c..7b1533d0d32 100644
--- a/lib/expand_variables.rb
+++ b/lib/expand_variables.rb
@@ -1,6 +1,6 @@
 module ExpandVariables
   class << self
-    def expand_variables(value, variables)
+    def expand(value, variables)
       # Convert hash array to variables
       if variables.is_a?(Array)
         variables = variables.reduce({}) do |hash, variable|
diff --git a/lib/gitlab/ci/config/node/environment.rb b/lib/gitlab/ci/config/node/environment.rb
index 629c17e6250..85302589ce6 100644
--- a/lib/gitlab/ci/config/node/environment.rb
+++ b/lib/gitlab/ci/config/node/environment.rb
@@ -8,7 +8,11 @@ module Gitlab
         class Environment < Entry
           include Validatable
 
+          ALLOWED_KEYS = %i[name url]
+
           validations do
+            validates :config, allowed_keys: ALLOWED_KEYS, if: :hash?
+
             validates :name, presence: true
 
             validates :url,
@@ -32,9 +36,9 @@ module Gitlab
           end
 
           def name
-            case
-            when string? then @config
-            when hash? then @config[:name]
+            case @config.type
+            when String then @config
+            when Hash then @config[:name]
             end
           end
 
@@ -43,9 +47,9 @@ module Gitlab
           end
 
           def value
-            case
-            when string? then { name: @config }
-            when hash? then @config
+            case @config.type
+            when String then { name: @config }
+            when Hash then @config
             end
           end
         end
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index e90e80171a4..3ab34d23d37 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -29,14 +29,6 @@ module Gitlab
                 inclusion: { in: %w[on_success on_failure always manual],
                              message: 'should be on_success, on_failure, ' \
                                       'always or manual' }
-              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 }
 
               validates :dependencies, array_of_strings: true
             end
@@ -83,7 +75,7 @@ module Gitlab
 
           helpers :before_script, :script, :stage, :type, :after_script,
                   :cache, :image, :services, :only, :except, :variables,
-                  :artifacts, :commands
+                  :artifacts, :commands, :environment
 
           def compose!(deps = nil)
             super do
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index af192664b33..2ad33007b8a 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -754,6 +754,20 @@ module Ci
         it 'does return production' do
           expect(builds.size).to eq(1)
           expect(builds.first[:environment]).to eq(environment)
+          expect(builds.first[:options]).to include(environment: { name: environment })
+        end
+      end
+
+      context 'when hash is specified' do
+        let(:environment) do
+          { name: 'production',
+            url: 'http://production.gitlab.com' }
+        end
+
+        it 'does return production and URL' do
+          expect(builds.size).to eq(1)
+          expect(builds.first[:environment]).to eq(environment[:name])
+          expect(builds.first[:options]).to include(environment)
         end
       end
 
diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb
index c8c741c0e88..7ebfddc45d5 100644
--- a/spec/services/create_deployment_service_spec.rb
+++ b/spec/services/create_deployment_service_spec.rb
@@ -41,7 +41,7 @@ describe CreateDeploymentService, services: true do
 
     context 'for environment with invalid name' do
       let(:params) do
-        { environment: 'name with spaces',
+        { environment: '..',
           ref: 'master',
           tag: false,
           sha: '97de212e80737a608d939f648d959671fb0a0142',
@@ -64,10 +64,8 @@ describe CreateDeploymentService, services: true do
           tag: false,
           sha: '97de212e80737a608d939f648d959671fb0a0142',
           options: {
-            environment: {
-              name: 'review-apps/$CI_BUILD_REF_NAME',
-              url: 'http://$CI_BUILD_REF_NAME.review-apps.gitlab.com'
-            }
+            name: 'review-apps/$CI_BUILD_REF_NAME',
+            url: 'http://$CI_BUILD_REF_NAME.review-apps.gitlab.com'
           },
           variables: [
             { key: 'CI_BUILD_REF_NAME', value: 'feature-review-apps' }
@@ -83,7 +81,7 @@ describe CreateDeploymentService, services: true do
       end
 
       it 'does create a new deployment' do
-        expect(subject).not_to be_persisted
+        expect(subject).to be_persisted
       end
     end
   end
@@ -125,6 +123,12 @@ describe CreateDeploymentService, services: true do
 
         expect(Deployment.last.deployable).to eq(deployable)
       end
+
+      it 'create environment has URL set' do
+        subject
+
+        expect(Deployment.last.environment.external_url).not_to be_nil
+      end
     end
 
     context 'without environment specified' do
@@ -137,7 +141,10 @@ describe CreateDeploymentService, services: true do
     
     context 'when environment is specified' do
       let(:pipeline) { create(:ci_pipeline, project: project) }
-      let(:build) { create(:ci_build, pipeline: pipeline, environment: 'production') }
+      let(:build) { create(:ci_build, pipeline: pipeline, environment: 'production', options: options) }
+      let(:options) do
+        { environment: { name: 'production', url: 'http://gitlab.com' } }
+      end
 
       context 'when build succeeds' do
         it_behaves_like 'does create environment and deployment' do