Merge branch 'review-apps' into 'master'

CHANGELOG

@@ -25,6 +25,8 @@ v 8.12.0 (unreleased)
   - Fix sorting of issues in API
   - Sort project variables by key. !6275 (Diego Souza)
   - Ensure specs on sorting of issues in API are deterministic on MySQL
+  - Added ability to use predefined CI variables for environment name
+  - Added ability to specify URL in environment configuration in gitlab-ci.yml
   - Escape search term before passing it to Regexp.new !6241 (winniehell)
   - Fix pinned sidebar behavior in smaller viewports !6169
   - Fix file permissions change when updating a file on the Gitlab UI !5979

app/models/ci/build.rb

@@ -79,11 +79,14 @@ module Ci
 
       after_transition any => [:success] do |build|
         if build.environment.present?
-          service = CreateDeploymentService.new(build.project, build.user,
-                                                environment: build.environment,
-                                                sha: build.sha,
-                                                ref: build.ref,
-                                                tag: build.tag)
+          service = CreateDeploymentService.new(
+            build.project, build.user,
+            environment: build.environment,
+            sha: build.sha,
+            ref: build.ref,
+            tag: build.tag,
+            options: build.options[:environment],
+            variables: build.variables)
           service.execute(build)
         end
       end

app/models/environment.rb

@@ -4,6 +4,7 @@ class Environment < ActiveRecord::Base
   has_many :deployments
 
   before_validation :nullify_external_url
+  before_save :set_environment_type
 
   validates :name,
             presence: true,
@@ -26,6 +27,17 @@ class Environment < ActiveRecord::Base
     self.external_url = nil if self.external_url.blank?
   end
 
+  def set_environment_type
+    names = name.split('/')
+
+    self.environment_type =
+      if names.many?
+        names.first
+      else
+        nil
+      end
+  end
+
   def includes_commit?(commit)
     return false unless last_deployment
 

app/services/create_deployment_service.rb

@@ -2,9 +2,7 @@ require_relative 'base_service'
 
 class CreateDeploymentService < BaseService
   def execute(deployable = nil)
-    environment = project.environments.find_or_create_by(
-      name: params[:environment]
-    )
+    environment = find_or_create_environment
 
     project.deployments.create(
       environment: environment,
@@ -15,4 +13,38 @@ class CreateDeploymentService < BaseService
       deployable: deployable
     )
   end
+
+  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
+
+  def expanded_url
+    return unless url
+
+    @expanded_url ||= ExpandVariables.expand(url, variables)
+  end
+
+  def name
+    params[:environment]
+  end
+
+  def url
+    options[:url]
+  end
+
+  def options
+    params[:options] || {}
+  end
+
+  def variables
+    params[:variables] || []
+  end
 end

db/migrate/20160907131111_add_environment_type_to_environments.rb

+class AddEnvironmentTypeToEnvironments < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :environments, :environment_type, :string
+  end
+end

db/schema.rb

@@ -390,10 +390,11 @@ ActiveRecord::Schema.define(version: 20160913212128) do
 
   create_table "environments", force: :cascade do |t|
     t.integer  "project_id"
-    t.string   "name",         null: false
+    t.string   "name",             null: false
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "external_url"
+    t.string   "environment_type"
   end
 
   add_index "environments", ["project_id", "name"], name: "index_environments_on_project_id_and_name", using: :btree

doc/ci/yaml/README.md

@@ -90,8 +90,7 @@ builds, including deploy builds. This can be an array or a multi-line string.
 
 ### after_script
 
->**Note:**
-Introduced in GitLab 8.7 and requires Gitlab Runner v1.2
+> Introduced in GitLab 8.7 and requires Gitlab Runner v1.2
 
 `after_script` is used to define the command that will be run after for all
 builds. This has to be an array or a multi-line string.
@@ -135,8 +134,7 @@ Alias for [stages](#stages).
 
 ### variables
 
->**Note:**
-Introduced in GitLab Runner v0.5.0.
+> Introduced in GitLab Runner v0.5.0.
 
 GitLab CI allows you to add variables to `.gitlab-ci.yml` that are set in the
 build environment. The variables are stored in the Git repository and are meant
@@ -158,8 +156,7 @@ Variables can be also defined on [job level](#job-variables).
 
 ### cache
 
->**Note:**
-Introduced in GitLab Runner v0.7.0.
+> Introduced in GitLab Runner v0.7.0.
 
 `cache` is used to specify a list of files and directories which should be
 cached between builds.
@@ -220,8 +217,7 @@ will be always present. For implementation details, please check GitLab Runner.
 
 #### cache:key
 
->**Note:**
-Introduced in GitLab Runner v1.0.0.
+> Introduced in GitLab Runner v1.0.0.
 
 The `key` directive allows you to define the affinity of caching
 between jobs, allowing to have a single cache for all jobs,
@@ -531,8 +527,7 @@ The above script will:
 
 #### Manual actions
 
->**Note:**
-Introduced in GitLab 8.10.
+> Introduced in GitLab 8.10.
 
 Manual actions are a special type of job that are not executed automatically;
 they need to be explicitly started by a user. Manual actions can be started
@@ -543,17 +538,16 @@ An example usage of manual actions is deployment to production.
 
 ### environment
 
->**Note:**
-Introduced in GitLab 8.9.
+> Introduced in GitLab 8.9.
 
-`environment` is used to define that a job deploys to a specific environment.
+`environment` is used to define that a job deploys to a specific [environment].
 This allows easy tracking of all deployments to your environments straight from
 GitLab.
 
 If `environment` is specified and no environment under that name exists, a new
 one will be created automatically.
 
-The `environment` name must contain only letters, digits, '-' and '_'. Common
+The `environment` name must contain only letters, digits, '-', '_', '/', '$', '{', '}' and spaces. Common
 names are `qa`, `staging`, and `production`, but you can use whatever name works
 with your workflow.
 
@@ -571,6 +565,35 @@ deploy to production:
 The `deploy to production` job will be marked as doing deployment to
 `production` environment.
 
+#### dynamic environments
+
+> [Introduced][ce-6323] in GitLab 8.12 and GitLab Runner 1.6.
+
+`environment` can also represent a configuration hash with `name` and `url`.
+These parameters can use any of the defined CI [variables](#variables)
+(including predefined, secure variables and `.gitlab-ci.yml` variables).
+
+The common use case is to create dynamic environments for branches and use them
+as review apps.
+
+---
+
+**Example configurations**
+
+```
+deploy as review app:
+  stage: deploy
+  script: ...
+  environment:
+    name: review-apps/$CI_BUILD_REF_NAME
+    url: https://$CI_BUILD_REF_NAME.review.example.com/
+```
+
+The `deploy as review app` job will be marked as deployment to dynamically
+create the `review-apps/branch-name` environment.
+
+This environment should be accessible under `https://branch-name.review.example.com/`.
+
 ### artifacts
 
 >**Notes:**
@@ -638,8 +661,7 @@ be available for download in the GitLab UI.
 
 #### artifacts:name
 
->**Note:**
-Introduced in GitLab 8.6 and GitLab Runner v1.1.0.
+> Introduced in GitLab 8.6 and GitLab Runner v1.1.0.
 
 The `name` directive allows you to define the name of the created artifacts
 archive. That way, you can have a unique name for every archive which could be
@@ -702,8 +724,7 @@ job:
 
 #### artifacts:when
 
->**Note:**
-Introduced in GitLab 8.9 and GitLab Runner v1.3.0.
+> Introduced in GitLab 8.9 and GitLab Runner v1.3.0.
 
 `artifacts:when` is used to upload artifacts on build failure or despite the
 failure.
@@ -728,8 +749,7 @@ job:
 
 #### artifacts:expire_in
 
->**Note:**
-Introduced in GitLab 8.9 and GitLab Runner v1.3.0.
+> Introduced in GitLab 8.9 and GitLab Runner v1.3.0.
 
 `artifacts:expire_in` is used to delete uploaded artifacts after the specified
 time. By default, artifacts are stored on GitLab forever. `expire_in` allows you
@@ -764,8 +784,7 @@ job:
 
 ### dependencies
 
->**Note:**
-Introduced in GitLab 8.6 and GitLab Runner v1.1.1.
+> Introduced in GitLab 8.6 and GitLab Runner v1.1.1.
 
 This feature should be used in conjunction with [`artifacts`](#artifacts) and
 allows you to define the artifacts to pass between different builds.
@@ -839,9 +858,8 @@ job:
 
 ## Git Strategy
 
->**Note:**
-Introduced in GitLab 8.9 as an experimental feature. May change in future
-releases or be removed completely.
+> Introduced in GitLab 8.9 as an experimental feature. May change in future
+  releases or be removed completely.
 
 You can set the `GIT_STRATEGY` used for getting recent application code. `clone`
 is slower, but makes sure you have a clean directory before every build. `fetch`
@@ -863,8 +881,7 @@ variables:
 
 ## Shallow cloning
 
->**Note:**
-Introduced in GitLab 8.9 as an experimental feature. May change in future
+> Introduced in GitLab 8.9 as an experimental feature. May change in future
 releases or be removed completely.
 
 You can specify the depth of fetching and cloning using `GIT_DEPTH`. This allows
@@ -894,8 +911,7 @@ variables:
 
 ## Hidden keys
 
->**Note:**
-Introduced in GitLab 8.6 and GitLab Runner v1.1.1.
+> Introduced in GitLab 8.6 and GitLab Runner v1.1.1.
 
 Keys that start with a dot (`.`) will be not processed by GitLab CI. You can
 use this feature to ignore jobs, or use the
@@ -923,8 +939,7 @@ Read more about the various [YAML features](https://learnxinyminutes.com/docs/ya
 
 ### Anchors
 
->**Note:**
-Introduced in GitLab 8.6 and GitLab Runner v1.1.1.
+> Introduced in GitLab 8.6 and GitLab Runner v1.1.1.
 
 YAML also has a handy feature called 'anchors', which let you easily duplicate
 content across your document. Anchors can be used to duplicate/inherit
@@ -1067,3 +1082,5 @@ Visit the [examples README][examples] to see a list of examples using GitLab
 CI with various languages.
 
 [examples]: ../examples/README.md
+[ce-6323]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6323
+[environment]: ../environments.md

lib/ci/api/entities.rb

@@ -15,6 +15,15 @@ module Ci
         expose :filename, :size
       end
 
+      class BuildOptions < Grape::Entity
+        expose :image
+        expose :services
+        expose :artifacts
+        expose :cache
+        expose :dependencies
+        expose :after_script
+      end
+
       class Build < Grape::Entity
         expose :id, :ref, :tag, :sha, :status
         expose :name, :token, :stage

lib/ci/gitlab_ci_yaml_processor.rb

@@ -60,7 +60,7 @@ module Ci
         name: job[:name].to_s,
         allow_failure: job[:allow_failure] || false,
         when: job[:when] || 'on_success',
-        environment: job[:environment],
+        environment: job[:environment_name],
         yaml_variables: yaml_variables(name),
         options: {
           image: job[:image],
@@ -69,6 +69,7 @@ module Ci
           cache: job[:cache],
           dependencies: job[:dependencies],
           after_script: job[:after_script],
+          environment: job[:environment],
         }.compact
       }
     end

lib/expand_variables.rb

+module ExpandVariables
+  class << self
+    def expand(value, variables)
+      # Convert hash array to variables
+      if variables.is_a?(Array)
+        variables = variables.reduce({}) do |hash, variable|
+          hash[variable[:key]] = variable[:value]
+          hash
+        end
+      end
+
+      value.gsub(/\$([a-zA-Z_][a-zA-Z0-9_]*)|\${\g<1>}|%\g<1>%/) do
+        variables[$1 || $2]
+      end
+    end
+  end
+end

lib/gitlab/ci/config/node/environment.rb

+module Gitlab
+  module Ci
+    class Config
+      module Node
+        ##
+        # Entry that represents an environment.
+        #
+        class Environment < Entry
+          include Validatable
+
+          ALLOWED_KEYS = %i[name url]
+
+          validations do
+            validate do
+              unless hash? || string?
+                errors.add(:config, 'should be a hash or a string')
+              end
+            end
+
+            validates :name, presence: true
+            validates :name,
+              type: {
+                with: String,
+                message: Gitlab::Regex.environment_name_regex_message }
+
+            validates :name,
+              format: {
+                with: Gitlab::Regex.environment_name_regex,
+                message: Gitlab::Regex.environment_name_regex_message }
+
+            with_options if: :hash? do
+              validates :config, allowed_keys: ALLOWED_KEYS
+
+              validates :url,
+                        length: { maximum: 255 },
+                        addressable_url: true,
+                        allow_nil: true
+            end
+          end
+
+          def hash?
+            @config.is_a?(Hash)
+          end
+
+          def string?
+            @config.is_a?(String)
+          end
+
+          def name
+            value[:name]
+          end
+
+          def url
+            value[:url]
+          end
+
+          def value
+            case @config
+            when String then { name: @config }
+            when Hash then @config
+            else {}
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/config/node/job.rb

@@ -13,7 +13,7 @@ module Gitlab
                             type stage when artifacts cache dependencies before_script
                             after_script variables environment]
 
-          attributes :tags, :allow_failure, :when, :environment, :dependencies
+          attributes :tags, :allow_failure, :when, :dependencies
 
           validations do
             validates :config, allowed_keys: ALLOWED_KEYS
@@ -29,58 +29,53 @@ 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
           end
 
-          node :before_script, Script,
+          node :before_script, Node::Script,
             description: 'Global before script overridden in this job.'
 
-          node :script, Commands,
+          node :script, Node::Commands,
             description: 'Commands that will be executed in this job.'
 
-          node :stage, Stage,
+          node :stage, Node::Stage,
             description: 'Pipeline stage this job will be executed into.'
 
-          node :type, Stage,
+          node :type, Node::Stage,
             description: 'Deprecated: stage this job will be executed into.'
 
-          node :after_script, Script,
+          node :after_script, Node::Script,
             description: 'Commands that will be executed when finishing job.'
 
-          node :cache, Cache,
+          node :cache, Node::Cache,
             description: 'Cache definition for this job.'
 
-          node :image, Image,
+          node :image, Node::Image,
             description: 'Image that will be used to execute this job.'
 
-          node :services, Services,
+          node :services, Node::Services,
             description: 'Services that will be used to execute this job.'
 
-          node :only, Trigger,
+          node :only, Node::Trigger,
             description: 'Refs policy this job will be executed for.'
 
-          node :except, Trigger,
+          node :except, Node::Trigger,
             description: 'Refs policy this job will be executed for.'
 
-          node :variables, Variables,
+          node :variables, Node::Variables,
             description: 'Environment variables available for this job.'
 
-          node :artifacts, Artifacts,
+          node :artifacts, Node::Artifacts,
             description: 'Artifacts configuration for this job.'
 
+          node :environment, Node::Environment,
+               description: 'Environment configuration for this job.'
+
           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
@@ -133,6 +128,8 @@ module Gitlab
               only: only,
               except: except,
               variables: variables_defined? ? variables : nil,
+              environment: environment_defined? ? environment : nil,
+              environment_name: environment_defined? ? environment[:name] : nil,
               artifacts: artifacts,
               after_script: after_script }
           end

lib/gitlab/regex.rb

@@ -96,11 +96,11 @@ module Gitlab
     end
 
     def environment_name_regex
-      @environment_name_regex ||= /\A[a-zA-Z0-9_-]+\z/.freeze
+      @environment_name_regex ||= /\A[a-zA-Z0-9_\\\/\${}. -]+\z/.freeze
     end
 
     def environment_name_regex_message
-      "can contain only letters, digits, '-' and '_'."
+      "can contain only letters, digits, '-', '_', '/', '$', '{', '}', '.' and spaces"
     end
   end
 end

spec/features/environments_spec.rb

@@ -150,7 +150,7 @@ feature 'Environments', feature: true do
 
       context 'for invalid name' do
         before do
-          fill_in('Name', with: 'name with spaces')
+          fill_in('Name', with: 'name,with,commas')
           click_on 'Save'
         end
 

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: environment)
         end
       end
 
@@ -770,15 +784,16 @@ module Ci
         let(:environment) { 1 }
 
         it 'raises error' do
-          expect { builds }.to raise_error("jobs:deploy_to_production environment #{Gitlab::Regex.environment_name_regex_message}")
+          expect { builds }.to raise_error(
+            'jobs:deploy_to_production:environment config should be a hash or a string')
         end
       end
 
       context 'is not a valid string' do
-        let(:environment) { 'production staging' }
+        let(:environment) { 'production:staging' }
 
         it 'raises error' do
-          expect { builds }.to raise_error("jobs:deploy_to_production environment #{Gitlab::Regex.environment_name_regex_message}")
+          expect { builds }.to raise_error("jobs:deploy_to_production:environment name #{Gitlab::Regex.environment_name_regex_message}")
         end
       end
     end

spec/lib/expand_variables_spec.rb

+require 'spec_helper'
+
+describe ExpandVariables do
+  describe '#expand' do
+    subject { described_class.expand(value, variables) }
+
+    tests = [
+      { value: 'key',
+        result: 'key',
+        variables: []
+      },
+      { value: 'key$variable',
+        result: 'key',
+        variables: []
+      },
+      { value: 'key$variable',
+        result: 'keyvalue',
+        variables: [
+          { key: 'variable', value: 'value' }
+        ]
+      },
+      { value: 'key${variable}',
+        result: 'keyvalue',
+        variables: [
+          { key: 'variable', value: 'value' }
+        ]
+      },
+      { value: 'key$variable$variable2',
+        result: 'keyvalueresult',
+        variables: [
+          { key: 'variable', value: 'value' },
+          { key: 'variable2', value: 'result' },
+        ]
+      },
+      { value: 'key${variable}${variable2}',
+        result: 'keyvalueresult',
+        variables: [
+          { key: 'variable', value: 'value' },
+          { key: 'variable2', value: 'result' }
+        ]
+      },
+      { value: 'key$variable2$variable',
+        result: 'keyresultvalue',
+        variables: [
+          { key: 'variable', value: 'value' },
+          { key: 'variable2', value: 'result' },
+        ]
+      },
+      { value: 'key${variable2}${variable}',
+        result: 'keyresultvalue',
+        variables: [
+          { key: 'variable', value: 'value' },
+          { key: 'variable2', value: 'result' }
+        ]
+      },
+      { value: 'review/$CI_BUILD_REF_NAME',
+        result: 'review/feature/add-review-apps',
+        variables: [
+          { key: 'CI_BUILD_REF_NAME', value: 'feature/add-review-apps' }
+        ]
+      },
+    ]
+
+    tests.each do |test|
+      context "#{test[:value]} resolves to #{test[:result]}" do
+        let(:value) { test[:value] }
+        let(:variables) { test[:variables] }
+
+        it { is_expected.to eq(test[:result]) }
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/config/node/environment_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Environment do
+  let(:entry) { described_class.new(config) }
+
+  before { entry.compose! }
+
+  context 'when configuration is a string' do
+    let(:config) { 'production' }
+
+    describe '#string?' do
+      it 'is string configuration' do
+        expect(entry).to be_string
+      end
+    end
+
+    describe '#hash?' do
+      it 'is not hash configuration' do
+        expect(entry).not_to be_hash
+      end
+    end
+
+    describe '#valid?' do
+      it 'is valid' do
+        expect(entry).to be_valid
+      end
+    end
+
+    describe '#value' do
+      it 'returns valid hash' do
+        expect(entry.value).to eq(name: 'production')
+      end
+    end
+
+    describe '#name' do
+      it 'returns environment name' do
+        expect(entry.name).to eq 'production'
+      end
+    end
+
+    describe '#url' do
+      it 'returns environment url' do
+        expect(entry.url).to be_nil
+      end
+    end
+  end
+
+  context 'when configuration is a hash' do
+    let(:config) do
+      { name: 'development', url: 'https://example.gitlab.com' }
+    end
+
+    describe '#string?' do
+      it 'is not string configuration' do
+        expect(entry).not_to be_string
+      end
+    end
+
+    describe '#hash?' do
+      it 'is hash configuration' do
+        expect(entry).to be_hash
+      end
+    end
+
+    describe '#valid?' do
+      it 'is valid' do
+        expect(entry).to be_valid
+      end
+    end
+
+    describe '#value' do
+      it 'returns valid hash' do
+        expect(entry.value).to eq config
+      end
+    end
+
+    describe '#name' do
+      it 'returns environment name' do
+        expect(entry.name).to eq 'development'
+      end
+    end
+
+    describe '#url' do
+      it 'returns environment url' do
+        expect(entry.url).to eq 'https://example.gitlab.com'
+      end
+    end
+  end
+
+  context 'when variables are used for environment' do
+    let(:config) do
+      { name: 'review/$CI_BUILD_REF_NAME',
+        url: 'https://$CI_BUILD_REF_NAME.review.gitlab.com' }
+    end
+
+    describe '#valid?' do
+      it 'is valid' do
+        expect(entry).to be_valid
+      end
+    end
+  end
+
+  context 'when configuration is invalid' do
+    context 'when configuration is an array' do
+      let(:config) { ['env'] }
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+
+      describe '#errors' do
+        it 'contains error about invalid type' do
+          expect(entry.errors)
+            .to include 'environment config should be a hash or a string'
+        end
+      end
+    end
+
+    context 'when environment name is not present' do
+      let(:config) { { url: 'https://example.gitlab.com' } }
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+
+      describe '#errors?' do
+        it 'contains error about missing environment name' do
+          expect(entry.errors)
+            .to include "environment name can't be blank"
+        end
+      end
+    end
+
+    context 'when invalid URL is used' do
+      let(:config) { { name: 'test', url: 'invalid-example.gitlab.com' } }
+
+      describe '#valid?' do
+        it 'is not valid' do
+          expect(entry).not_to be_valid
+        end
+      end
+
+      describe '#errors?' do
+        it 'contains error about invalid URL' do
+          expect(entry.errors)
+            .to include "environment url must be a valid url"
+        end
+      end
+    end
+  end
+end

spec/models/environment_spec.rb

@@ -63,4 +63,20 @@ describe Environment, models: true do
       end
     end
   end
+
+  describe '#environment_type' do
+    subject { environment.environment_type }
+
+    it 'sets a environment type if name has multiple segments' do
+      environment.update!(name: 'production/worker.gitlab.com')
+
+      is_expected.to eq('production')
+    end
+
+    it 'nullifies a type if it\'s a simple name' do
+      environment.update!(name: 'production')
+
+      is_expected.to be_nil
+    end
+  end
 end

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: 'name,with,commas',
           ref: 'master',
           tag: false,
           sha: '97de212e80737a608d939f648d959671fb0a0142',
@@ -56,8 +56,36 @@ describe CreateDeploymentService, services: true do
         expect(subject).not_to be_persisted
       end
     end
+
+    context 'when variables are used' do
+      let(:params) do
+        { environment: 'review-apps/$CI_BUILD_REF_NAME',
+          ref: 'master',
+          tag: false,
+          sha: '97de212e80737a608d939f648d959671fb0a0142',
+          options: {
+            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' }
+          ]
+        }
+      end
+
+      it 'does create a new environment' do
+        expect { subject }.to change { Environment.count }.by(1)
+
+        expect(subject.environment.name).to eq('review-apps/feature-review-apps')
+        expect(subject.environment.external_url).to eq('http://feature-review-apps.review-apps.gitlab.com')
+      end
+
+      it 'does create a new deployment' do
+        expect(subject).to be_persisted
+      end
+    end
   end
-  
+
   describe 'processing of builds' do
     let(:environment) { nil }
     
@@ -95,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
@@ -107,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