Merge branch 'invalid-environment-not-being-created-should-be-surfaced-to-user' into 'master'

Surface the error for invalid environment creation

See merge request gitlab-org/gitlab!69537

app/models/concerns/enums/ci/commit_status.rb

@@ -26,6 +26,7 @@ module Enums
           pipeline_loop_detected: 17,
           no_matching_runner: 18, # not used anymore, but cannot be deleted because of old data
           trace_size_exceeded: 19,
+          environment_creation_failure: 20,
           insufficient_bridge_permissions: 1_001,
           downstream_bridge_project_not_found: 1_002,
           invalid_bridge_trigger: 1_003,

app/presenters/commit_status_presenter.rb

 # frozen_string_literal: true
 class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
+  include ActionView::Helpers::UrlHelper
+
   CALLOUT_FAILURE_MESSAGES = {
     unknown_failure: 'There is an unknown failure, please try again',
     script_failure: nil,
@@ -27,7 +29,12 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
     user_blocked: 'The user who created this job is blocked',
     ci_quota_exceeded: 'No more CI minutes available',
     no_matching_runner: 'No matching runner available',
-    trace_size_exceeded: 'The job log size limit was reached'
+    trace_size_exceeded: 'The job log size limit was reached',
+    environment_creation_failure: 'This job could not be executed because it would create an environment with an invalid parameter.'
+  }.freeze
+
+  TROUBLESHOOTING_DOC = {
+    environment_creation_failure: { path: 'ci/environments/index', anchor: 'a-deployment-job-failed-with-this-job-could-not-be-executed-because-it-would-create-an-environment-with-an-invalid-parameter-error' }
   }.freeze
 
   private_constant :CALLOUT_FAILURE_MESSAGES
@@ -39,7 +46,13 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
   end
 
   def callout_failure_message
-    self.class.callout_failure_messages.fetch(failure_reason.to_sym)
+    message = self.class.callout_failure_messages.fetch(failure_reason.to_sym)
+
+    if doc = TROUBLESHOOTING_DOC[failure_reason.to_sym]
+      message += " #{link_to('How do I fix it?', help_page_path(doc[:path], anchor: doc[:anchor]))}"
+    end
+
+    message
   end
 end
 

config/feature_flags/development/surface_environment_creation_failure.yml

+---
+name: surface_environment_creation_failure
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69537
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340169
+milestone: '14.3'
+type: development
+group: group::release
+default_enabled: false

config/feature_flags/development/surface_environment_creation_failure_override.yml

+---
+name: surface_environment_creation_failure_override
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69537
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340169
+milestone: '14.3'
+type: development
+group: group::release
+default_enabled: false

doc/ci/environments/index.md

@@ -816,3 +816,41 @@ To ensure the `action: stop` can always run when needed, you can:
       action: stop
     when: manual
   ```
+
+### A deployment job failed with "This job could not be executed because it would create an environment with an invalid parameter" error
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/21182) in GitLab 14.3.
+
+FLAG:
+On self-managed GitLab, by default this bug fix is not available. To make it available per project or for your entire instance, ask an administrator to [enable the `surface_environment_creation_failure` flag](../../administration/feature_flags.md). On GitLab.com, this bug fix is not available, but will be rolled out shortly.
+
+If your project is configured to [create a dynamic environment](#create-a-dynamic-environment),
+such as a [Review App](../review_apps/index.md), you might encounter this error
+because the dynamically generated parameter is invalid for the system.
+
+For example, if you have the following in your `.gitlab-ci.yml`:
+
+```yaml
+review:
+  script: deploy review app
+  environment: review/$CI_COMMIT_REF_NAME
+```
+
+When you create a new merge request with a branch name `bug-fix!`,
+the `review` job tries to create an environment with `review/bug-fix!`.
+However, the `!` is an invalid character for environments, so the
+deployment job fails since it was about to run without an environment.
+
+To fix this, you can:
+
+- Re-create your feature branch without the invalid characters,
+  such as `bug-fix`.
+- Replace the `CI_COMMIT_REF_NAME`
+  [predefined variable](../variables/predefined_variables.md) with
+  `CI_COMMIT_REF_SLUG` which strips any invalid characters:
+
+  ```yaml
+  review:
+    script: deploy review app
+    environment: review/$CI_COMMIT_REF_SLUG
+  ```

lib/gitlab/ci/pipeline/seed/build.rb

@@ -106,10 +106,15 @@ module Gitlab
 
             environment = Seed::Environment.new(build).to_resource
 
-            # If there is a validation error on environment creation, such as
-            # the name contains invalid character, the build falls back to a
-            # non-environment job.
             unless environment.persisted?
+              if Feature.enabled?(:surface_environment_creation_failure, build.project, default_enabled: :yaml) &&
+                 Feature.disabled?(:surface_environment_creation_failure_override, build.project)
+                return { status: :failed, failure_reason: :environment_creation_failure }
+              end
+
+              # If there is a validation error on environment creation, such as
+              # the name contains invalid character, the build falls back to a
+              # non-environment job.
               Gitlab::ErrorTracking.track_exception(
                 EnvironmentCreationFailure.new,
                 project_id: build.project_id,

lib/gitlab/ci/status/build/failed.rb

@@ -32,7 +32,8 @@ module Gitlab
             user_blocked: 'pipeline user was blocked',
             ci_quota_exceeded: 'no more CI minutes available',
             no_matching_runner: 'no matching runner available',
-            trace_size_exceeded: 'log size limit exceeded'
+            trace_size_exceeded: 'log size limit exceeded',
+            environment_creation_failure: 'environment creation failure'
           }.freeze
 
           private_constant :REASONS

spec/lib/gitlab/ci/pipeline/seed/build_spec.rb

@@ -440,17 +440,30 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
         context 'when the environment name is invalid' do
           let(:attributes) { { name: 'deploy', ref: 'master', environment: '!!!' } }
 
-          it_behaves_like 'non-deployment job'
-          it_behaves_like 'ensures environment inexistence'
+          it 'fails the job with a failure reason and does not create an environment' do
+            expect(subject).to be_failed
+            expect(subject).to be_environment_creation_failure
+            expect(subject.metadata.expanded_environment_name).to be_nil
+            expect(Environment.exists?(name: expected_environment_name)).to eq(false)
+          end
+
+          context 'when surface_environment_creation_failure feature flag is disabled' do
+            before do
+              stub_feature_flags(surface_environment_creation_failure: false)
+            end
 
-          it 'tracks an exception' do
-            expect(Gitlab::ErrorTracking).to receive(:track_exception)
-              .with(an_instance_of(described_class::EnvironmentCreationFailure),
-                    project_id: project.id,
-                    reason: %q{Name can contain only letters, digits, '-', '_', '/', '$', '{', '}', '.', and spaces, but it cannot start or end with '/'})
-              .once
+            it_behaves_like 'non-deployment job'
+            it_behaves_like 'ensures environment inexistence'
 
-            subject
+            it 'tracks an exception' do
+              expect(Gitlab::ErrorTracking).to receive(:track_exception)
+                .with(an_instance_of(described_class::EnvironmentCreationFailure),
+                      project_id: project.id,
+                      reason: %q{Name can contain only letters, digits, '-', '_', '/', '$', '{', '}', '.', and spaces, but it cannot start or end with '/'})
+                .once
+
+              subject
+            end
           end
         end
       end

spec/presenters/commit_status_presenter_spec.rb

@@ -15,6 +15,25 @@ RSpec.describe CommitStatusPresenter do
     expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated)
   end
 
+  describe '#callout_failure_message' do
+    subject { presenter.callout_failure_message }
+
+    context 'when troubleshooting doc is available' do
+      let(:failure_reason) { :environment_creation_failure }
+
+      before do
+        build.failure_reason = failure_reason
+      end
+
+      it 'appends the troubleshooting link' do
+        doc = described_class::TROUBLESHOOTING_DOC[failure_reason]
+
+        expect(subject).to eq("#{described_class.callout_failure_messages[failure_reason]} " \
+                              "<a href=\"#{presenter.help_page_path(doc[:path], anchor: doc[:anchor])}\">How do I fix it?</a>")
+      end
+    end
+  end
+
   describe 'covers all failure reasons' do
     let(:message) { presenter.callout_failure_message }
 

spec/spec_helper.rb

@@ -308,6 +308,10 @@ RSpec.configure do |config|
       # For more information check https://gitlab.com/gitlab-org/gitlab/-/issues/339348
       stub_feature_flags(new_header_search: false)
 
+      # Disable the override flag in order to enable the feature by default.
+      # See https://docs.gitlab.com/ee/development/feature_flags/#selectively-disable-by-actor
+      stub_feature_flags(surface_environment_creation_failure_override: false)
+
       allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_return(enable_rugged)
     else
       unstub_all_feature_flags