Merge branch '25680-CI_ENVIRONMENT_URL' into 'master'

Add `$CI_ENVIRONMENT_URL` as a job variable

Closes #25680

See merge request !11695

5 files changed, 238 insertions, 165 deletions

diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index f5e99fdf00b..0bb5a86d9b9 100644
--- a/spec/factories/ci/builds.rb
+++ b/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
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index e2406290c6c..22ee469dd86 100644
--- a/spec/models/ci/build_spec.rb
+++ b/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,7 +1223,56 @@ describe Ci::Build, :models do
         ]
       end
 
-      it { environment_variables.each { |v| is_expected.to include(v) } }
+      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
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 06fbd7bad90..597c3947e71 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/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
diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb
index f35d7a33548..5398b5c3f7e 100644
--- a/spec/services/create_deployment_service_spec.rb
+++ b/spec/services/create_deployment_service_spec.rb
@@ -1,156 +1,117 @@
 require 'spec_helper'
 
 describe CreateDeploymentService, services: true do
-  let(:project) { create(:empty_project) }
   let(:user) { create(:user) }
+  let(:options) { nil }
+
+  let(:job) do
+    create(:ci_build,
+      ref: 'master',
+      tag: false,
+      environment: 'production',
+      options: { environment: options })
+  end
 
-  let(:service) { described_class.new(project, user, params) }
+  let(:project) { job.project }
 
-  describe '#execute' do
-    let(:options) { nil }
-    let(:params) do
-      {
-        environment: 'production',
-        ref: 'master',
-        tag: false,
-        sha: '97de212e80737a608d939f648d959671fb0a0142',
-        options: options
-      }
-    end
+  let!(:environment) do
+    create(:environment, project: project, name: 'production')
+  end
 
-    subject { service.execute }
+  let(:service) { described_class.new(job) }
 
-    context 'when no environments exist' do
-      it 'does create a new environment' do
-        expect { subject }.to change { Environment.count }.by(1)
-      end
+  describe '#execute' do
+    subject { service.execute }
 
-      it 'does create a deployment' do
+    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') }
-
-      it 'does not create a new environment' do
-        expect { subject }.not_to change { Environment.count }
-      end
+    context 'when environment does not exist' do
+      let(:environment) {}
 
-      it 'does create a deployment' do
-        expect(subject).to be_persisted
+      it 'does not create a deployment' do
+        expect do
+          expect(subject).to be_nil
+        end.not_to change { Deployment.count }
       end
+    end
 
-      context 'and start action is defined' do
-        let(:options) { { action: 'start' } }
+    context 'when start action is defined' do
+      let(:options) { { action: 'start' } }
 
-        context 'and environment is stopped' do
-          before do
-            environment.stop
-          end
+      context 'and environment is stopped' do
+        before do
+          environment.stop
+        end
 
-          it 'makes environment available' do
-            subject
+        it 'makes environment available' do
+          subject
 
-            expect(environment.reload).to be_available
-          end
+          expect(environment.reload).to be_available
+        end
 
-          it 'does create a deployment' do
-            expect(subject).to be_persisted
-          end
+        it 'creates a deployment' do
+          expect(subject).to be_persisted
         end
       end
+    end
 
-      context 'and stop action is defined' do
-        let(:options) { { action: 'stop' } }
-
-        context 'and environment is available' do
-          before do
-            environment.start
-          end
-
-          it 'makes environment stopped' do
-            subject
-
-            expect(environment.reload).to be_stopped
-          end
+    context 'when stop action is defined' do
+      let(:options) { { action: 'stop' } }
 
-          it 'does not create a deployment' do
-            expect(subject).to be_nil
-          end
+      context 'and environment is available' do
+        before do
+          environment.start
         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 'makes environment stopped' do
+          subject
 
-      it 'does not create a new environment' do
-        expect { subject }.not_to change { Environment.count }
-      end
+          expect(environment.reload).to be_stopped
+        end
 
-      it 'does not create a deployment' do
-        expect(subject).to be_nil
+        it 'does not create a deployment' do
+          expect(subject).to be_nil
+        end
       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
-
-        it 'updates external url' do
-          subject
+      it 'does not create a new environment' do
+        expect { subject }.not_to change { Environment.count }
+      end
 
-          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 'updates external url' do
+        subject
 
-        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
diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb
index 66545127a44..6e1eb5c678d 100644
--- a/spec/support/cycle_analytics_helpers.rb
+++ b/spec/support/cycle_analytics_helpers.rb
@@ -51,12 +51,43 @@ module CycleAnalyticsHelpers
   end
 
   def deploy_master(environment: 'production')
-    CreateDeploymentService.new(project, user, {
-                                  environment: environment,
-                                  ref: 'master',
-                                  tag: false,
-                                  sha: project.repository.commit('master').sha
-                                }).execute
+    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,
+      name: 'dummy',
+      pipeline: dummy_pipeline)
   end
 end