Merge branch '25680-CI_ENVIRONMENT_URL' into 'master'

Add `$CI_ENVIRONMENT_URL` as a job variable

Closes #25680

See merge request !11695

app/models/ci/build.rb

@@ -138,6 +138,17 @@ module Ci
       ExpandVariables.expand(environment, simple_variables) if environment
     end
 
+    def environment_url
+      return @environment_url if defined?(@environment_url)
+
+      @environment_url =
+        if unexpanded_url = options&.dig(:environment, :url)
+          ExpandVariables.expand(unexpanded_url, simple_variables)
+        else
+          persisted_environment&.external_url
+        end
+    end
+
     def has_environment?
       environment.present?
     end
@@ -198,9 +209,7 @@ module Ci
 
     # All variables, including those dependent on other variables
     def variables
-      variables = simple_variables
-      variables += persisted_environment.predefined_variables if persisted_environment.present?
-      variables
+      simple_variables.concat(persisted_environment_variables)
     end
 
     def merge_request
@@ -462,6 +471,18 @@ module Ci
       variables.concat(legacy_variables)
     end
 
+    def persisted_environment_variables
+      return [] unless persisted_environment
+
+      variables = persisted_environment.predefined_variables
+
+      if url = environment_url
+        variables << { key: 'CI_ENVIRONMENT_URL', value: url, public: true }
+      end
+
+      variables
+    end
+
     def legacy_variables
       variables = [
         { key: 'CI_BUILD_ID', value: id.to_s, public: true },

app/services/create_deployment_service.rb

-class CreateDeploymentService < BaseService
-  def execute(deployable = nil)
+class CreateDeploymentService
+  attr_reader :job
+
+  delegate :expanded_environment_name,
+           :environment_url,
+           :project,
+           to: :job
+
+  def initialize(job)
+    @job = job
+  end
+
+  def execute
     return unless executable?
 
     ActiveRecord::Base.transaction do
-      @deployable = deployable
+      environment.external_url = environment_url if environment_url
+      environment.fire_state_event(action)
 
-      @environment = environment
-      @environment.external_url = expanded_url if expanded_url
-      @environment.fire_state_event(action)
+      return unless environment.save
+      return if environment.stopped?
 
-      return unless @environment.save
-      return if @environment.stopped?
-
-      deploy.tap do |deployment|
-        deployment.update_merge_request_metrics!
-      end
+      deploy.tap(&:update_merge_request_metrics!)
     end
   end
 
   private
 
   def executable?
-    project && name.present?
+    project && job.environment.present? && environment
   end
 
   def deploy
     project.deployments.create(
-      environment: @environment,
-      ref: params[:ref],
-      tag: params[:tag],
-      sha: params[:sha],
-      user: current_user,
-      deployable: @deployable,
-      on_stop: options[:on_stop])
+      environment: environment,
+      ref: job.ref,
+      tag: job.tag,
+      sha: job.sha,
+      user: job.user,
+      deployable: job,
+      on_stop: on_stop)
   end
 
   def environment
-    @environment ||= project.environments.find_or_create_by(name: expanded_name)
-  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]
+    @environment ||= job.persisted_environment
   end
 
-  def options
-    params[:options] || {}
+  def environment_options
+    @environment_options ||= job.options&.dig(:environment) || {}
   end
 
-  def variables
-    params[:variables] || []
+  def on_stop
+    environment_options[:on_stop]
   end
 
   def action
-    options[:action] || 'start'
+    environment_options[:action] || 'start'
   end
 end

app/workers/build_success_worker.rb

@@ -11,15 +11,6 @@ class BuildSuccessWorker
   private
 
   def create_deployment(build)
-    service = CreateDeploymentService.new(
-      build.project, build.user,
-      environment: build.environment,
-      sha: build.sha,
-      ref: build.ref,
-      tag: build.tag,
-      options: build.options.to_h[:environment],
-      variables: build.variables)
-
-    service.execute(build)
+    CreateDeploymentService.new(build).execute
   end
 end

changelogs/unreleased/25680-CI_ENVIRONMENT_URL.yml

+---
+title: Add $CI_ENVIRONMENT_URL to predefined variables for pipelines
+merge_request: 11695
+author:

db/fixtures/development/14_pipelines.rb

@@ -112,6 +112,10 @@ class Gitlab::Seeder::Pipelines
 
       setup_artifacts(build)
       setup_build_log(build)
+
+      build.project.environments.
+        find_or_create_by(name: build.expanded_environment_name)
+
       build.save
     end
   end

db/fixtures/development/17_cycle_analytics.rb

@@ -212,12 +212,9 @@ class Gitlab::Seeder::CycleAnalytics
     merge_requests.each do |merge_request|
       Timecop.travel 12.hours.from_now
 
-      CreateDeploymentService.new(merge_request.project, @user, {
-                                    environment: 'production',
-                                    ref: 'master',
-                                    tag: false,
-                                    sha: @project.repository.commit('master').sha
-                                  }).execute
+      job = merge_request.head_pipeline.builds.where.not(environment: nil).last
+
+      CreateDeploymentService.new(job).execute
     end
   end
 end

doc/ci/environments.md

@@ -94,6 +94,12 @@ the name given in `.gitlab-ci.yml` (with any variables expanded), while the
 second is a "cleaned-up" version of the name, suitable for use in URLs, DNS,
 etc.
 
+>**Note:**
+Starting with GitLab 9.3, the environment URL is exposed to the Runner via
+`$CI_ENVIRONMENT_URL`. The URL would be expanded from `.gitlab-ci.yml`, or if
+the URL was not defined there, the external URL from the environment would be
+used.
+
 To sum up, with the above `.gitlab-ci.yml` we have achieved that:
 
 - All branches will run the `test` and `build` jobs.

doc/ci/variables/README.md

@@ -43,6 +43,7 @@ future GitLab releases.**
 | **CI_DEBUG_TRACE**              | all    | 1.7    | Whether [debug tracing](#debug-tracing) is enabled |
 | **CI_ENVIRONMENT_NAME**         | 8.15   | all    | The name of the environment for this job |
 | **CI_ENVIRONMENT_SLUG**         | 8.15   | all    | A simplified version of the environment name, suitable for inclusion in DNS, URLs, Kubernetes labels, etc. |
+| **CI_ENVIRONMENT_URL**          | 9.3    | all    | The URL of the environment for this job |
 | **CI_JOB_ID**                   | 9.0    | all    | The unique id of the current job that GitLab CI uses internally |
 | **CI_JOB_MANUAL**               | 8.12   | all    | The flag to indicate that job was manually started |
 | **CI_JOB_NAME**                 | 9.0    | 0.5    | The name of the job as defined in `.gitlab-ci.yml` |

spec/factories/ci/builds.rb

@@ -64,7 +64,8 @@ FactoryGirl.define do
     trait :teardown_environment do
       environment 'staging'
       options environment: { name: 'staging',
-                             action: 'stop' }
+                             action: 'stop',
+                             url: 'http://staging.example.com/$CI_JOB_NAME' }
     end
 
     trait :allowed_to_fail do

spec/models/ci/build_spec.rb

@@ -427,6 +427,42 @@ describe Ci::Build, :models do
       end
     end
 
+    describe '#environment_url' do
+      subject { job.environment_url }
+
+      context 'when yaml environment uses $CI_COMMIT_REF_NAME' do
+        let(:job) do
+          create(:ci_build,
+                 ref: 'master',
+                 options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } })
+        end
+
+        it { is_expected.to eq('http://review/master') }
+      end
+
+      context 'when yaml environment uses yaml_variables containing symbol keys' do
+        let(:job) do
+          create(:ci_build,
+                 yaml_variables: [{ key: :APP_HOST, value: 'host' }],
+                 options: { environment: { url: 'http://review/$APP_HOST' } })
+        end
+
+        it { is_expected.to eq('http://review/host') }
+      end
+
+      context 'when yaml environment does not have url' do
+        let(:job) { create(:ci_build, environment: 'staging') }
+
+        let!(:environment) do
+          create(:environment, project: job.project, name: job.environment)
+        end
+
+        it 'returns the external_url from persisted environment' do
+          is_expected.to eq(environment.external_url)
+        end
+      end
+    end
+
     describe '#starts_environment?' do
       subject { build.starts_environment? }
 
@@ -918,6 +954,10 @@ describe Ci::Build, :models do
 
       it { is_expected.to eq(environment) }
     end
+
+    context 'when there is no environment' do
+      it { is_expected.to be_nil }
+    end
   end
 
   describe '#play' do
@@ -1176,11 +1216,6 @@ describe Ci::Build, :models do
     end
 
     context 'when build has an environment' do
-      before do
-        build.update(environment: 'production')
-        create(:environment, project: build.project, name: 'production', slug: 'prod-slug')
-      end
-
       let(:environment_variables) do
         [
           { key: 'CI_ENVIRONMENT_NAME', value: 'production', public: true },
@@ -1188,9 +1223,58 @@ describe Ci::Build, :models do
         ]
       end
 
+      let!(:environment) do
+        create(:environment,
+          project: build.project,
+          name: 'production',
+          slug: 'prod-slug',
+          external_url: '')
+      end
+
+      before do
+        build.update(environment: 'production')
+      end
+
+      shared_examples 'containing environment variables' do
         it { environment_variables.each { |v| is_expected.to include(v) } }
       end
 
+      context 'when no URL was set' do
+        it_behaves_like 'containing environment variables'
+
+        it 'does not have CI_ENVIRONMENT_URL' do
+          keys = subject.map { |var| var[:key] }
+
+          expect(keys).not_to include('CI_ENVIRONMENT_URL')
+        end
+      end
+
+      context 'when an URL was set' do
+        let(:url) { 'http://host/test' }
+
+        before do
+          environment_variables <<
+            { key: 'CI_ENVIRONMENT_URL', value: url, public: true }
+        end
+
+        context 'when the URL was set from the job' do
+          before do
+            build.update(options: { environment: { url: 'http://host/$CI_JOB_NAME' } })
+          end
+
+          it_behaves_like 'containing environment variables'
+        end
+
+        context 'when the URL was not set from the job, but environment' do
+          before do
+            environment.update(external_url: url)
+          end
+
+          it_behaves_like 'containing environment variables'
+        end
+      end
+    end
+
     context 'when build started manually' do
       before do
         build.update_attributes(when: :manual)

spec/services/ci/create_pipeline_service_spec.rb

@@ -296,5 +296,20 @@ describe Ci::CreatePipelineService, services: true do
         expect(Environment.find_by(name: "review/master")).not_to be_nil
       end
     end
+
+    context 'when environment with invalid name' do
+      before do
+        config = YAML.dump(deploy: { environment: { name: 'name,with,commas' }, script: 'ls' })
+        stub_ci_pipeline_yaml_file(config)
+      end
+
+      it 'does not create an environment' do
+        expect do
+          result = execute_service
+
+          expect(result).to be_persisted
+        end.not_to change { Environment.count }
+      end
+    end
   end
 end

spec/services/create_deployment_service_spec.rb

 require 'spec_helper'
 
 describe CreateDeploymentService, services: true do
-  let(:project) { create(:empty_project) }
   let(:user) { create(:user) }
-
-  let(:service) { described_class.new(project, user, params) }
-
-  describe '#execute' do
   let(:options) { nil }
-    let(:params) do
-      {
-        environment: 'production',
+
+  let(:job) do
+    create(:ci_build,
       ref: 'master',
       tag: false,
-        sha: '97de212e80737a608d939f648d959671fb0a0142',
-        options: options
-      }
+      environment: 'production',
+      options: { environment: options })
   end
 
-    subject { service.execute }
+  let(:project) { job.project }
 
-    context 'when no environments exist' do
-      it 'does create a new environment' do
-        expect { subject }.to change { Environment.count }.by(1)
+  let!(:environment) do
+    create(:environment, project: project, name: 'production')
   end
 
-      it 'does create a deployment' do
+  let(:service) { described_class.new(job) }
+
+  describe '#execute' do
+    subject { service.execute }
+
+    context 'when environment exists' do
+      it 'creates a deployment' do
         expect(subject).to be_persisted
       end
     end
 
-    context 'when environment exist' do
-      let!(:environment) { create(:environment, project: project, name: 'production') }
+    context 'when environment does not exist' do
+      let(:environment) {}
 
-      it 'does not create a new environment' do
-        expect { subject }.not_to change { Environment.count }
+      it 'does not create a deployment' do
+        expect do
+          expect(subject).to be_nil
+        end.not_to change { Deployment.count }
       end
-
-      it 'does create a deployment' do
-        expect(subject).to be_persisted
     end
 
-      context 'and start action is defined' do
+    context 'when start action is defined' do
       let(:options) { { action: 'start' } }
 
       context 'and environment is stopped' do
@@ -55,13 +53,13 @@ describe CreateDeploymentService, services: true do
           expect(environment.reload).to be_available
         end
 
-          it 'does create a deployment' do
+        it 'creates a deployment' do
           expect(subject).to be_persisted
         end
       end
     end
 
-      context 'and stop action is defined' do
+    context 'when stop action is defined' do
       let(:options) { { action: 'stop' } }
 
       context 'and environment is available' do
@@ -80,58 +78,22 @@ describe CreateDeploymentService, services: true do
         end
       end
     end
-    end
-
-    context 'for environment with invalid name' do
-      let(:params) do
-        {
-          environment: 'name,with,commas',
-          ref: 'master',
-          tag: false,
-          sha: '97de212e80737a608d939f648d959671fb0a0142'
-        }
-      end
-
-      it 'does not create a new environment' do
-        expect { subject }.not_to change { Environment.count }
-      end
-
-      it 'does not create a deployment' do
-        expect(subject).to be_nil
-      end
-    end
 
     context 'when variables are used' do
-      let(:params) do
-        {
-          environment: 'review-apps/$CI_COMMIT_REF_NAME',
-          ref: 'master',
-          tag: false,
-          sha: '97de212e80737a608d939f648d959671fb0a0142',
-          options: {
-            name: 'review-apps/$CI_COMMIT_REF_NAME',
-            url: 'http://$CI_COMMIT_REF_NAME.review-apps.gitlab.com'
-          },
-          variables: [
-            { key: 'CI_COMMIT_REF_NAME', value: 'feature-review-apps' }
-          ]
-        }
+      let(:options) do
+        { name: 'review-apps/$CI_COMMIT_REF_NAME',
+          url: 'http://$CI_COMMIT_REF_NAME.review-apps.gitlab.com' }
       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')
+      before do
+        environment.update(name: 'review-apps/master')
+        job.update(environment: 'review-apps/$CI_COMMIT_REF_NAME')
       end
 
-      it 'does create a new deployment' do
+      it 'creates a new deployment' do
         expect(subject).to be_persisted
       end
 
-      context 'and environment exist' do
-        let!(:environment) { create(:environment, project: project, name: 'review-apps/feature-review-apps') }
-
       it 'does not create a new environment' do
         expect { subject }.not_to change { Environment.count }
       end
@@ -139,18 +101,17 @@ describe CreateDeploymentService, services: true do
       it 'updates external url' do
         subject
 
-          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
+        expect(subject.environment.name).to eq('review-apps/master')
+        expect(subject.environment.external_url).to eq('http://master.review-apps.gitlab.com')
       end
     end
 
     context 'when project was removed' do
-      let(:project) { nil }
+      let(:environment) {}
+
+      before do
+        job.update(project: nil)
+      end
 
       it 'does not create deployment or environment' do
         expect { subject }.not_to raise_error
@@ -162,34 +123,26 @@ describe CreateDeploymentService, services: true do
   end
 
   describe 'processing of builds' do
-    let(:environment) { nil }
-
-    shared_examples 'does not create environment and deployment' do
-      it 'does not create a new environment' do
-        expect { subject }.not_to change { Environment.count }
-      end
-
+    shared_examples 'does not create deployment' do
       it 'does not create a new deployment' do
         expect { subject }.not_to change { Deployment.count }
       end
 
       it 'does not call a service' do
         expect_any_instance_of(described_class).not_to receive(:execute)
+
         subject
       end
     end
 
-    shared_examples 'does create environment and deployment' do
-      it 'does create a new environment' do
-        expect { subject }.to change { Environment.count }.by(1)
-      end
-
-      it 'does create a new deployment' do
+    shared_examples 'creates deployment' do
+      it 'creates a new deployment' do
         expect { subject }.to change { Deployment.count }.by(1)
       end
 
-      it 'does call a service' do
+      it 'calls a service' do
         expect_any_instance_of(described_class).to receive(:execute)
+
         subject
       end
 
@@ -199,7 +152,7 @@ describe CreateDeploymentService, services: true do
         expect(Deployment.last.deployable).to eq(deployable)
       end
 
-      it 'create environment has URL set' do
+      it 'updates environment URL' do
         subject
 
         expect(Deployment.last.environment.external_url).not_to be_nil
@@ -207,41 +160,39 @@ describe CreateDeploymentService, services: true do
     end
 
     context 'without environment specified' do
-      let(:build) { create(:ci_build, project: project) }
+      let(:job) { create(:ci_build) }
 
-      it_behaves_like 'does not create environment and deployment' do
-        subject { build.success }
+      it_behaves_like 'does not create deployment' do
+        subject { job.success }
       end
     end
 
     context 'when environment is specified' do
-      let(:pipeline) { create(:ci_pipeline, project: project) }
-      let(:build) { create(:ci_build, pipeline: pipeline, environment: 'production', options: options) }
+      let(:deployable) { job }
+
       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
-          let(:deployable) { build }
-
-          subject { build.success }
+      context 'when job succeeds' do
+        it_behaves_like 'creates deployment' do
+          subject { job.success }
         end
       end
 
-      context 'when build fails' do
-        it_behaves_like 'does not create environment and deployment' do
-          subject { build.drop }
+      context 'when job fails' do
+        it_behaves_like 'does not create deployment' do
+          subject { job.drop }
         end
       end
 
-      context 'when build is retried' do
-        it_behaves_like 'does create environment and deployment' do
+      context 'when job is retried' do
+        it_behaves_like 'creates deployment' do
           before do
             project.add_developer(user)
           end
 
-          let(:deployable) { Ci::Build.retry(build, user) }
+          let(:deployable) { Ci::Build.retry(job, user) }
 
           subject { deployable.success }
         end
@@ -250,15 +201,6 @@ describe CreateDeploymentService, services: true do
   end
 
   describe "merge request metrics" do
-    let(:params) do
-      {
-        environment: 'production',
-        ref: 'master',
-        tag: false,
-        sha: '97de212e80737a608d939f648d959671fb0a0142b'
-      }
-    end
-
     let(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: project) }
 
     context "while updating the 'first_deployed_to_production_at' time" do
@@ -273,8 +215,8 @@ describe CreateDeploymentService, services: true do
         end
 
         it "doesn't set the time if the deploy's environment is not 'production'" do
-          staging_params = params.merge(environment: 'staging')
-          service = described_class.new(project, user, staging_params)
+          job.update(environment: 'staging')
+          service = described_class.new(job)
           service.execute
 
           expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil
@@ -298,7 +240,7 @@ describe CreateDeploymentService, services: true do
             expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_like_time(time)
 
             # Current deploy
-            service = described_class.new(project, user, params)
+            service = described_class.new(job)
             Timecop.freeze(time + 12.hours) { service.execute }
 
             expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_like_time(time)
@@ -318,7 +260,7 @@ describe CreateDeploymentService, services: true do
             expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil
 
             # Current deploy
-            service = described_class.new(project, user, params)
+            service = described_class.new(job)
             Timecop.freeze(time + 12.hours) { service.execute }
 
             expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil

spec/support/cycle_analytics_helpers.rb

@@ -51,12 +51,43 @@ module CycleAnalyticsHelpers
   end
 
   def deploy_master(environment: 'production')
-    CreateDeploymentService.new(project, user, {
+    dummy_job =
+      case environment
+      when 'production'
+        dummy_production_job
+      when 'staging'
+        dummy_staging_job
+      else
+        raise ArgumentError
+      end
+
+    CreateDeploymentService.new(dummy_job).execute
+  end
+
+  def dummy_production_job
+    @dummy_job ||= new_dummy_job('production')
+  end
+
+  def dummy_staging_job
+    @dummy_job ||= new_dummy_job('staging')
+  end
+
+  def dummy_pipeline
+    @dummy_pipeline ||=
+      Ci::Pipeline.new(sha: project.repository.commit('master').sha)
+  end
+
+  def new_dummy_job(environment)
+    project.environments.find_or_create_by(name: environment)
+
+    Ci::Build.new(
+      project: project,
+      user: user,
       environment: environment,
       ref: 'master',
       tag: false,
-                                  sha: project.repository.commit('master').sha
-                                }).execute
+      name: 'dummy',
+      pipeline: dummy_pipeline)
   end
 end