Expand with all the variables so that things like

CI_ENVIRONMENT_SLUG is also available. It won't be
recursive because we're not putting this value in the variables.

3 files changed, 31 insertions, 20 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 4454057e418..a300536532b 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -471,9 +471,9 @@ module Ci
 
       variables = persisted_environment.predefined_variables
 
-        # Here we're passing unexpanded environment_url for runner to expand,
-        # and we need to make sure that CI_ENVIRONMENT_NAME and
-        # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded.
+      # Here we're passing unexpanded environment_url for runner to expand,
+      # and we need to make sure that CI_ENVIRONMENT_NAME and
+      # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded.
       variables << { key: 'CI_ENVIRONMENT_URL', value: environment_url, public: true } if environment_url
 
       variables
@@ -498,11 +498,7 @@ module Ci
     end
 
     def environment_url
-      return @environment_url if defined?(@environment_url)
-
-      @environment_url =
-        options&.dig(:environment, :url) ||
-          persisted_environment&.external_url
+      options&.dig(:environment, :url) || persisted_environment&.external_url
     end
 
     def build_attributes_from_config
diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb
index 85500d6c9c9..45f68391df8 100644
--- a/app/services/create_deployment_service.rb
+++ b/app/services/create_deployment_service.rb
@@ -2,7 +2,7 @@ class CreateDeploymentService
   attr_reader :job
 
   delegate :expanded_environment_name,
-           :simple_variables,
+           :variables,
            :project,
            to: :job
 
@@ -14,8 +14,10 @@ class CreateDeploymentService
     return unless executable?
 
     ActiveRecord::Base.transaction do
-      environment.external_url = expanded_environment_url if
-        expanded_environment_url
+      if external_url = expanded_environment_url
+        environment.external_url = external_url
+      end
+
       environment.fire_state_event(action)
 
       return unless environment.save
@@ -51,14 +53,8 @@ class CreateDeploymentService
   end
 
   def expanded_environment_url
-    return @expanded_environment_url if defined?(@expanded_environment_url)
-
-    @expanded_environment_url =
-      if unexpanded_url = environment_options[:url]
-        ExpandVariables.expand(unexpanded_url, simple_variables)
-      else
-        environment&.external_url
-      end
+    ExpandVariables.expand(environment_options[:url], variables) if
+      environment_options[:url]
   end
 
   def on_stop
diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb
index 790713cf2e4..dfab6ebf372 100644
--- a/spec/services/create_deployment_service_spec.rb
+++ b/spec/services/create_deployment_service_spec.rb
@@ -135,6 +135,25 @@ describe CreateDeploymentService, services: true do
       it { is_expected.to eq('http://review/master') }
     end
 
+    context 'when yaml environment uses $CI_ENVIRONMENT_SLUG' do
+      let(:job) do
+        create(:ci_build,
+               ref: 'master',
+               environment: 'production',
+               options: { environment: { url: 'http://review/$CI_ENVIRONMENT_SLUG' } })
+      end
+
+      let!(:environment) do
+        create(:environment,
+          project: job.project,
+          name: 'production',
+          slug: 'prod-slug',
+          external_url: 'http://review/old')
+      end
+
+      it { is_expected.to eq('http://review/prod-slug') }
+    end
+
     context 'when yaml environment uses yaml_variables containing symbol keys' do
       let(:job) do
         create(:ci_build,
@@ -153,7 +172,7 @@ describe CreateDeploymentService, services: true do
       end
 
       it 'returns the external_url from persisted environment' do
-        is_expected.to eq(environment.external_url)
+        is_expected.to be_nil
       end
     end
   end