Merge branch...

Merge branch '30146-let-s-encrypt-integration-doesn-t-scale-and-does-not-give-any-feedback-to-user-on-errors-4' into 'master'

Add warning about failing to obtain LE certificates to pages setting page

See merge request gitlab-org/gitlab!27779

app/presenters/pages_domain_presenter.rb

+# frozen_string_literal: true
+
+class PagesDomainPresenter < Gitlab::View::Presenter::Delegated
+  presents :pages_domain
+
+  def needs_verification?
+    Gitlab::CurrentSettings.pages_domain_verification_enabled? && unverified?
+  end
+
+  def show_auto_ssl_failed_warning?
+    return false unless Feature.enabled?(:pages_letsencrypt_errors, pages_domain.project)
+
+    # validations prevents auto ssl from working, so there is no need to show that warning until
+    return false if needs_verification?
+
+    ::Gitlab::LetsEncrypt.enabled? && auto_ssl_failed
+  end
+end

app/views/projects/pages/_list.html.haml

@@ -6,6 +6,7 @@
       Domains (#{@domains.count})
     %ul.list-group.list-group-flush.pages-domain-list{ class: ("has-verification-status" if verification_enabled) }
       - @domains.each do |domain|
+        - domain = Gitlab::View::Presenter::Factory.new(domain, current_user: current_user).fabricate!
         %li.pages-domain-list-item.list-group-item.d-flex.justify-content-between
           - if verification_enabled
             - tooltip, status = domain.unverified? ? [s_('GitLabPages|Unverified'), 'failed'] : [s_('GitLabPages|Verified'), 'success']
@@ -13,20 +14,27 @@
               = sprite_icon("status_#{status}", size: 16 )
           .domain-name
             = external_link(domain.url, domain.url)
-            - if domain.subject
+            - if domain.certificate
               %div
                 %span.badge.badge-gray
-                  = s_('GitLabPages|Certificate: %{subject}') % { subject: domain.subject }
+                  = s_('GitLabPages|Certificate: %{subject}') % { subject: domain.pages_domain.subject }
                 - if domain.expired?
                   %span.badge.badge-danger
                     = s_('GitLabPages|Expired')
           %div
             = link_to s_('GitLabPages|Edit'), project_pages_domain_path(@project, domain), class: "btn btn-sm btn-grouped btn-success btn-inverted"
             = link_to s_('GitLabPages|Remove'), project_pages_domain_path(@project, domain), data: { confirm: s_('GitLabPages|Are you sure?')}, method: :delete, class: "btn btn-remove btn-sm btn-grouped"
-        - if verification_enabled && domain.unverified?
+        - if domain.needs_verification?
           %li.list-group-item.bs-callout-warning
             - details_link_start = "<a href='#{project_pages_domain_path(@project, domain)}'>".html_safe
             - details_link_end = '</a>'.html_safe
             = s_('GitLabPages|%{domain} is not verified. To learn how to verify ownership, visit your %{link_start}domain details%{link_end}.').html_safe % { domain: domain.domain,
                 link_start: details_link_start,
                 link_end: details_link_end }
+        - if domain.show_auto_ssl_failed_warning?
+          %li.list-group-item.bs-callout-warning
+            - details_link_start = "<a href='#{project_pages_domain_path(@project, domain)}'>".html_safe
+            - details_link_end = '</a>'.html_safe
+            = s_("GitLabPages|Something went wrong while obtaining Let's Encrypt certificate for %{domain}. To retry visit your %{link_start}domain details%{link_end}.").html_safe % { domain: domain.domain,
+                link_start: details_link_start,
+                link_end: details_link_end }

locale/gitlab.pot

@@ -9723,6 +9723,9 @@ msgstr ""
 msgid "GitLabPages|Save"
 msgstr ""
 
+msgid "GitLabPages|Something went wrong while obtaining Let's Encrypt certificate for %{domain}. To retry visit your %{link_start}domain details%{link_end}."
+msgstr ""
+
 msgid "GitLabPages|Support for domains and certificates is disabled.  Ask your system's administrator to enable it."
 msgstr ""
 

spec/presenters/pages_domain_presenter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe PagesDomainPresenter do
+  using RSpec::Parameterized::TableSyntax
+  include LetsEncryptHelpers
+
+  let(:presenter) { Gitlab::View::Presenter::Factory.new(domain).fabricate! }
+
+  describe 'needs_validation?' do
+    where(:pages_verification_enabled, :traits, :expected) do
+      false | :unverified | false
+      false | []          | false
+      true  | :unverified | true
+      true  | []          | false
+    end
+
+    with_them do
+      before do
+        stub_application_setting(pages_domain_verification_enabled: pages_verification_enabled)
+      end
+
+      let(:domain) { create(:pages_domain, *traits) }
+
+      it { expect(presenter.needs_verification?).to eq(expected) }
+    end
+  end
+
+  describe 'show_auto_ssl_failed_warning?' do
+    subject { presenter.show_auto_ssl_failed_warning? }
+
+    let(:domain) { create(:pages_domain) }
+
+    before do
+      stub_lets_encrypt_settings
+    end
+
+    it { is_expected.to eq(false) }
+
+    context "when we failed to obtain Let's Encrypt's certificate" do
+      before do
+        domain.update!(auto_ssl_failed: true)
+      end
+
+      it { is_expected.to eq(true) }
+
+      context 'when lets_encrypt_error feature flag is disabled' do
+        before do
+          stub_feature_flags(pages_letsencrypt_errors: false)
+        end
+
+        it { is_expected.to eq(false) }
+      end
+
+      context "when Let's Encrypt integration is disabled" do
+        before do
+          allow(::Gitlab::LetsEncrypt).to receive(:enabled?).and_return false
+        end
+
+        it { is_expected.to eq(false) }
+      end
+
+      context "when domain is unverified" do
+        before do
+          domain.update!(verified_at: nil)
+        end
+
+        it { is_expected.to eq(false) }
+      end
+    end
+  end
+end

spec/views/projects/pages/show.html.haml_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'projects/pages/show' do
+  include LetsEncryptHelpers
+
+  let(:project) { create(:project, :repository) }
+  let(:user) { create(:user) }
+  let(:domain) { create(:pages_domain, project: project) }
+
+  before do
+    allow(project).to receive(:pages_deployed?).and_return(true)
+    stub_pages_setting(external_https: true)
+    stub_lets_encrypt_settings
+    project.add_maintainer(user)
+
+    assign(:project, project)
+    allow(view).to receive(:current_user).and_return(user)
+    assign(:domains, [domain])
+  end
+
+  describe 'validation warning' do
+    let(:warning_message) do
+      "#{domain.domain} is not verified. To learn how to verify ownership, "\
+      "visit your domain details."
+    end
+
+    it "doesn't show auto ssl error warning" do
+      render
+
+      expect(rendered).not_to have_content(warning_message)
+    end
+
+    context "when domain is not verified" do
+      before do
+        domain.update!(verified_at: nil)
+      end
+
+      it 'shows auto ssl error warning' do
+        render
+
+        expect(rendered).to have_content(warning_message)
+      end
+    end
+  end
+
+  describe "warning about failed Let's Encrypt" do
+    let(:error_message) do
+      "Something went wrong while obtaining Let's Encrypt certificate for #{domain.domain}. "\
+      "To retry visit your domain details."
+    end
+
+    it "doesn't show auto ssl error warning" do
+      render
+
+      expect(rendered).not_to have_content(error_message)
+    end
+
+    context "when we failed to obtain Let's Encrypt's certificate" do
+      before do
+        domain.update!(auto_ssl_failed: true)
+      end
+
+      it 'shows auto ssl error warning' do
+        render
+
+        expect(rendered).to have_content(error_message)
+      end
+    end
+  end
+end