Commit 1a193d04 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Don't expand CI_ENVIRONMENT_URL so runner would do

And make sure CI_ENVIRONMENT_URL comes last so all
variables would be available whenever the runner is
trying to expand it.

This is an alternative to !12333
parent 8f537e5c
...@@ -138,11 +138,11 @@ module Ci ...@@ -138,11 +138,11 @@ module Ci
ExpandVariables.expand(environment, simple_variables) if environment ExpandVariables.expand(environment, simple_variables) if environment
end end
def environment_url def expanded_environment_url
return @environment_url if defined?(@environment_url) return @expanded_environment_url if defined?(@expanded_environment_url)
@environment_url = @expanded_environment_url =
if unexpanded_url = options&.dig(:environment, :url) if unexpanded_url = environment_url
ExpandVariables.expand(unexpanded_url, simple_variables) ExpandVariables.expand(unexpanded_url, simple_variables)
else else
persisted_environment&.external_url persisted_environment&.external_url
...@@ -506,6 +506,10 @@ module Ci ...@@ -506,6 +506,10 @@ module Ci
variables variables
end end
def environment_url
options&.dig(:environment, :url)
end
def build_attributes_from_config def build_attributes_from_config
return {} unless pipeline.config_processor return {} unless pipeline.config_processor
......
...@@ -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,
:environment_url, :expanded_environment_url,
:project, :project,
to: :job to: :job
...@@ -14,7 +14,8 @@ class CreateDeploymentService ...@@ -14,7 +14,8 @@ class CreateDeploymentService
return unless executable? return unless executable?
ActiveRecord::Base.transaction do ActiveRecord::Base.transaction do
environment.external_url = environment_url if environment_url environment.external_url = expanded_environment_url if
expanded_environment_url
environment.fire_state_event(action) environment.fire_state_event(action)
return unless environment.save return unless environment.save
......
...@@ -451,8 +451,8 @@ describe Ci::Build, :models do ...@@ -451,8 +451,8 @@ describe Ci::Build, :models do
end end
end end
describe '#environment_url' do describe '#expanded_environment_url' do
subject { job.environment_url } subject { job.expanded_environment_url }
context 'when yaml environment uses $CI_COMMIT_REF_NAME' do context 'when yaml environment uses $CI_COMMIT_REF_NAME' do
let(:job) do let(:job) do
...@@ -1292,10 +1292,20 @@ describe Ci::Build, :models do ...@@ -1292,10 +1292,20 @@ describe Ci::Build, :models do
context 'when the URL was set from the job' do context 'when the URL was set from the job' do
before do before do
build.update(options: { environment: { url: 'http://host/$CI_JOB_NAME' } }) build.update(options: { environment: { url: url } })
end end
it_behaves_like 'containing environment variables' it_behaves_like 'containing environment variables'
context 'when variables are used in the URL, it does not expand' do
let(:url) { 'http://$CI_PROJECT_NAME-$CI_ENVIRONMENT_SLUG' }
it_behaves_like 'containing environment variables'
it 'puts $CI_ENVIRONMENT_URL in the last so all other variables are available to be used when runners are trying to expand it' do
expect(subject.last).to eq(environment_variables.last)
end
end
end end
context 'when the URL was not set from the job, but environment' do context 'when the URL was not set from the job, but environment' 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