Merge branch '34008-fix-CI_ENVIRONMENT_URL-2' into 'master'
Don't expand CI_ENVIRONMENT_URL so runner would do Closes #34008 See merge request !12344
This commit is contained in:
commit
6431118d85
|
@ -138,17 +138,6 @@ 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
|
||||
|
@ -192,7 +181,7 @@ module Ci
|
|||
slugified.gsub(/[^a-z0-9]/, '-')[0..62]
|
||||
end
|
||||
|
||||
# Variables whose value does not depend on other variables
|
||||
# Variables whose value does not depend on environment
|
||||
def simple_variables
|
||||
variables = predefined_variables
|
||||
variables += project.predefined_variables
|
||||
|
@ -207,7 +196,8 @@ module Ci
|
|||
variables
|
||||
end
|
||||
|
||||
# All variables, including those dependent on other variables
|
||||
# All variables, including those dependent on environment, which could
|
||||
# contain unexpanded variables.
|
||||
def variables
|
||||
simple_variables.concat(persisted_environment_variables)
|
||||
end
|
||||
|
@ -481,9 +471,10 @@ module Ci
|
|||
|
||||
variables = persisted_environment.predefined_variables
|
||||
|
||||
if url = environment_url
|
||||
variables << { key: 'CI_ENVIRONMENT_URL', value: url, public: true }
|
||||
end
|
||||
# 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
|
||||
end
|
||||
|
@ -506,6 +497,10 @@ module Ci
|
|||
variables
|
||||
end
|
||||
|
||||
def environment_url
|
||||
options&.dig(:environment, :url) || persisted_environment&.external_url
|
||||
end
|
||||
|
||||
def build_attributes_from_config
|
||||
return {} unless pipeline.config_processor
|
||||
|
||||
|
|
|
@ -2,7 +2,7 @@ class CreateDeploymentService
|
|||
attr_reader :job
|
||||
|
||||
delegate :expanded_environment_name,
|
||||
:environment_url,
|
||||
:variables,
|
||||
:project,
|
||||
to: :job
|
||||
|
||||
|
@ -14,7 +14,8 @@ class CreateDeploymentService
|
|||
return unless executable?
|
||||
|
||||
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)
|
||||
|
||||
return unless environment.save
|
||||
|
@ -49,6 +50,17 @@ class CreateDeploymentService
|
|||
@environment_options ||= job.options&.dig(:environment) || {}
|
||||
end
|
||||
|
||||
def expanded_environment_url
|
||||
return @expanded_environment_url if defined?(@expanded_environment_url)
|
||||
|
||||
@expanded_environment_url =
|
||||
ExpandVariables.expand(environment_url, variables) if environment_url
|
||||
end
|
||||
|
||||
def environment_url
|
||||
environment_options[:url]
|
||||
end
|
||||
|
||||
def on_stop
|
||||
environment_options[:on_stop]
|
||||
end
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
title: Fix passing CI_ENVIRONMENT_NAME and CI_ENVIRONMENT_SLUG for CI_ENVIRONMENT_URL
|
||||
merge_request: 12344
|
||||
author:
|
|
@ -451,42 +451,6 @@ 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? }
|
||||
|
||||
|
@ -1292,10 +1256,20 @@ describe Ci::Build, :models do
|
|||
|
||||
context 'when the URL was set from the job' do
|
||||
before do
|
||||
build.update(options: { environment: { url: 'http://host/$CI_JOB_NAME' } })
|
||||
build.update(options: { environment: { url: url } })
|
||||
end
|
||||
|
||||
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
|
||||
|
||||
context 'when the URL was not set from the job, but environment' do
|
||||
|
|
|
@ -122,6 +122,61 @@ describe CreateDeploymentService, services: true do
|
|||
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 $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,
|
||||
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 be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'processing of builds' do
|
||||
shared_examples 'does not create deployment' do
|
||||
it 'does not create a new deployment' do
|
||||
|
|
Loading…
Reference in New Issue