Merge branch 'ignore-environment-validation-failure' into 'master'

Ignore environment validation failure

See merge request gitlab-org/gitlab-ce!23100

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,

changelogs/unreleased/ignore-environment-validation-failure.yml

+---
+title: Ignore environment validation failure
+merge_request: 23100
+author:
+type: fixed

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

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