From 09490fd0ef5717b28af3f23b85d7887c282883d7 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Thu, 15 Nov 2018 11:19:04 +0000
Subject: [PATCH] Ignore environment validation failure
---
app/models/concerns/deployable.rb | 4 ++
.../ignore-environment-validation-failure.yml | 5 ++
spec/models/concerns/deployable_spec.rb | 21 ++++++++
.../ci/create_pipeline_service_spec.rb | 48 +++++++++++++++++++
4 files changed, 78 insertions(+)
create mode 100644 changelogs/unreleased/ignore-environment-validation-failure.yml
diff --git a/app/models/concerns/deployable.rb b/app/models/concerns/deployable.rb
index 85db01af18d8b..bc12b06b5af10 100644
--- a/app/models/concerns/deployable.rb
+++ b/app/models/concerns/deployable.rb
@@ -13,6 +13,10 @@ def create_deployment
name: expanded_environment_name
)
+ # If we failed to persist envirionment record by validation error, such as name with invalid character,
+ # the job will fall back to a non-environment job.
+ return unless environment.persisted?
+
create_deployment!(
project_id: environment.project_id,
environment: environment,
diff --git a/changelogs/unreleased/ignore-environment-validation-failure.yml b/changelogs/unreleased/ignore-environment-validation-failure.yml
new file mode 100644
index 0000000000000..1b61cf86dc462
--- /dev/null
+++ b/changelogs/unreleased/ignore-environment-validation-failure.yml
@@ -0,0 +1,5 @@
+---
+title: Ignore environment validation failure
+merge_request: 23100
+author:
+type: fixed
diff --git a/spec/models/concerns/deployable_spec.rb b/spec/models/concerns/deployable_spec.rb
index ac79c75a55ee8..6951be903fe43 100644
--- a/spec/models/concerns/deployable_spec.rb
+++ b/spec/models/concerns/deployable_spec.rb
@@ -49,5 +49,26 @@
expect(environment).to be_nil
end
end
+
+ context 'when environment scope contains invalid character' do
+ let(:job) do
+ create(
+ :ci_build,
+ name: 'job:deploy-to-test-site',
+ environment: '$CI_JOB_NAME',
+ options: {
+ environment: {
+ name: '$CI_JOB_NAME',
+ url: 'http://staging.example.com/$CI_JOB_NAME',
+ on_stop: 'stop_review_app'
+ }
+ })
+ end
+
+ it 'does not create a deployment and environment record' do
+ expect(deployment).to be_nil
+ expect(environment).to be_nil
+ end
+ end
end
end
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 193148d403a6b..4d9c5aabbda18 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -608,5 +608,53 @@ def previous_commit_sha_from_ref(ref)
.to eq variables_attributes.map(&:with_indifferent_access)
end
end
+
+ context 'when pipeline has a job with environment' do
+ let(:pipeline) { execute_service }
+
+ before do
+ stub_ci_pipeline_yaml_file(YAML.dump(config))
+ end
+
+ context 'when environment name is valid' do
+ let(:config) do
+ {
+ review_app: {
+ script: 'deploy',
+ environment: {
+ name: 'review/${CI_COMMIT_REF_NAME}',
+ url: 'http://${CI_COMMIT_REF_SLUG}-staging.example.com'
+ }
+ }
+ }
+ end
+
+ it 'has a job with environment' do
+ expect(pipeline.builds.count).to eq(1)
+ expect(pipeline.builds.first.persisted_environment.name).to eq('review/master')
+ expect(pipeline.builds.first.deployment).to be_created
+ end
+ end
+
+ context 'when environment name is invalid' do
+ let(:config) do
+ {
+ 'job:deploy-to-test-site': {
+ script: 'deploy',
+ environment: {
+ name: '${CI_JOB_NAME}',
+ url: 'https://$APP_URL'
+ }
+ }
+ }
+ end
+
+ it 'has a job without environment' do
+ expect(pipeline.builds.count).to eq(1)
+ expect(pipeline.builds.first.persisted_environment).to be_nil
+ expect(pipeline.builds.first.deployment).to be_nil
+ end
+ end
+ end
end
end
--
GitLab