Commit 0d3631ac authored by Lin Jen-Shin's avatar Lin Jen-Shin

Move expanded_environment_url to CreateDeploymentService

Because that's the only place we need it.
parent df095d7d
...@@ -138,17 +138,6 @@ module Ci ...@@ -138,17 +138,6 @@ module Ci
ExpandVariables.expand(environment, simple_variables) if environment ExpandVariables.expand(environment, simple_variables) if environment
end end
def expanded_environment_url
return @expanded_environment_url if defined?(@expanded_environment_url)
@expanded_environment_url =
if unexpanded_url = environment_url
ExpandVariables.expand(unexpanded_url, simple_variables)
else
persisted_environment&.external_url
end
end
def has_environment? def has_environment?
environment.present? environment.present?
end end
...@@ -482,15 +471,10 @@ module Ci ...@@ -482,15 +471,10 @@ module Ci
variables = persisted_environment.predefined_variables variables = persisted_environment.predefined_variables
if url = environment_url # Here we're passing unexpanded environment_url for runner to expand,
# Note that CI_ENVIRONMENT_URL should be the last variable, because # and we need to make sure that CI_ENVIRONMENT_NAME and
# here we're passing unexpanded environment_url for runner to expand, # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded.
# and the runner would expand in order. In order to make sure that variables << { key: 'CI_ENVIRONMENT_URL', value: environment_url, public: true } if environment_url
# CI_ENVIRONMENT_URL has everything available, such as variables
# from Environment#predefined_variables, we need to make sure it's
# the last variable.
variables << { key: 'CI_ENVIRONMENT_URL', value: url, public: true }
end
variables variables
end end
...@@ -514,7 +498,11 @@ module Ci ...@@ -514,7 +498,11 @@ module Ci
end end
def environment_url def environment_url
options&.dig(:environment, :url) return @environment_url if defined?(@environment_url)
@environment_url =
options&.dig(:environment, :url) ||
persisted_environment&.external_url
end end
def build_attributes_from_config def build_attributes_from_config
......
...@@ -2,7 +2,7 @@ class CreateDeploymentService ...@@ -2,7 +2,7 @@ class CreateDeploymentService
attr_reader :job attr_reader :job
delegate :expanded_environment_name, delegate :expanded_environment_name,
:expanded_environment_url, :simple_variables,
:project, :project,
to: :job to: :job
...@@ -50,6 +50,17 @@ class CreateDeploymentService ...@@ -50,6 +50,17 @@ class CreateDeploymentService
@environment_options ||= job.options&.dig(:environment) || {} @environment_options ||= job.options&.dig(:environment) || {}
end 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
end
def on_stop def on_stop
environment_options[:on_stop] environment_options[:on_stop]
end end
......
...@@ -451,42 +451,6 @@ describe Ci::Build, :models do ...@@ -451,42 +451,6 @@ describe Ci::Build, :models do
end end
end end
describe '#expanded_environment_url' do
subject { job.expanded_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 describe '#starts_environment?' do
subject { build.starts_environment? } subject { build.starts_environment? }
......
...@@ -122,6 +122,42 @@ describe CreateDeploymentService, services: true do ...@@ -122,6 +122,42 @@ describe CreateDeploymentService, services: true do
end end
end end
describe '#expanded_environment_url' do
subject { service.send(:expanded_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 'processing of builds' do describe 'processing of builds' do
shared_examples 'does not create deployment' do shared_examples 'does not create deployment' do
it 'does not create a new deployment' do it 'does not create a new deployment' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment