Merge branch...

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

Add email notifications for failed Let's Encrypt integration

See merge request gitlab-org/gitlab!28581

app/mailers/emails/pages_domains.rb

@@ -41,5 +41,16 @@ module Emails
         subject: subject("ACTION REQUIRED: Verification failed for GitLab Pages domain '#{domain.domain}'")
       )
     end
+
+    def pages_domain_auto_ssl_failed_email(domain, recipient)
+      @domain = domain
+      @project = domain.project
+
+      subject_text = _("ACTION REQUIRED: Something went wrong while obtaining the Let's Encrypt certificate for GitLab Pages domain '%{domain}'") % { domain: domain.domain }
+      mail(
+        to: recipient.notification_email_for(@project.group),
+        subject: subject(subject_text)
+      )
+    end
   end
 end

app/services/notification_service.rb

@@ -489,6 +489,12 @@ class NotificationService
     end
   end
 
+  def pages_domain_auto_ssl_failed(domain)
+    project_maintainers_recipients(domain, action: 'disabled').each do |recipient|
+      mailer.pages_domain_auto_ssl_failed_email(domain, recipient.user).deliver_later
+    end
+  end
+
   def issue_due(issue)
     recipients = NotificationRecipients::BuildService.build_recipients(
       issue,

app/services/pages_domains/obtain_lets_encrypt_certificate_service.rb

@@ -57,6 +57,8 @@ module PagesDomains
       pages_domain.save!(validate: false)
 
       acme_order.destroy!
+
+      NotificationService.new.pages_domain_auto_ssl_failed(pages_domain)
     end
 
     def log_error(api_order)

app/views/notify/pages_domain_auto_ssl_failed_email.html.haml

+%p
+  = _("Something went wrong while obtaining the Let's Encrypt certificate.")
+%p
+  #{_('Project')}: #{link_to @project.human_name, project_url(@project)}
+%p
+  #{_('Domain')}: #{link_to @domain.domain, project_pages_domain_url(@project, @domain)}
+%p
+  - docs_url = help_page_url('user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md', anchor: 'troubleshooting')
+  - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: docs_url }
+  - link_end = '</a>'.html_safe
+  = _("Please follow the %{link_start}Let\'s Encrypt troubleshooting instructions%{link_end} to re-obtain your Let's Encrypt certificate.").html_safe % { link_start: link_start, link_end: link_end }

app/views/notify/pages_domain_auto_ssl_failed_email.text.haml

+= _("Something went wrong while obtaining the Let's Encrypt certificate.").html_safe
+
+#{_('Project')}: #{project_url(@project)}
+#{_('Domain')}: #{project_pages_domain_url(@project, @domain)}
+
+- docs_url = help_page_url('user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md', anchor: 'troubleshooting')
+= _("Please follow the Let\'s Encrypt troubleshooting instructions to re-obtain your Let's Encrypt certificate: %{docs_url}.").html_safe % { docs_url: docs_url }

locale/gitlab.pot

@@ -894,6 +894,9 @@ msgstr ""
 msgid "A user with write access to the source branch selected this option"
 msgstr ""
 
+msgid "ACTION REQUIRED: Something went wrong while obtaining the Let's Encrypt certificate for GitLab Pages domain '%{domain}'"
+msgstr ""
+
 msgid "API Help"
 msgstr ""
 
@@ -14912,6 +14915,12 @@ msgstr ""
 msgid "Please fill in a descriptive name for your group."
 msgstr ""
 
+msgid "Please follow the %{link_start}Let's Encrypt troubleshooting instructions%{link_end} to re-obtain your Let's Encrypt certificate."
+msgstr ""
+
+msgid "Please follow the Let's Encrypt troubleshooting instructions to re-obtain your Let's Encrypt certificate: %{docs_url}."
+msgstr ""
+
 msgid "Please migrate all existing projects to hashed storage to avoid security issues and ensure data integrity. %{migrate_link}"
 msgstr ""
 
@@ -18951,6 +18960,9 @@ msgstr ""
 msgid "Something went wrong while moving issues."
 msgstr ""
 
+msgid "Something went wrong while obtaining the Let's Encrypt certificate."
+msgstr ""
+
 msgid "Something went wrong while performing the action."
 msgstr ""
 

spec/mailers/emails/pages_domains_spec.rb

@@ -23,13 +23,20 @@ describe Emails::PagesDomains do
         is_expected.to have_subject(email_subject)
         is_expected.to have_body_text(project.human_name)
         is_expected.to have_body_text(domain.domain)
-        is_expected.to have_body_text domain.url
         is_expected.to have_body_text project_pages_domain_url(project, domain)
-        is_expected.to have_body_text help_page_url('user/project/pages/custom_domains_ssl_tls_certification/index.md', anchor: link_anchor)
       end
     end
   end
 
+  shared_examples 'a pages domain verification email' do
+    it_behaves_like 'a pages domain email'
+
+    it 'has the expected content' do
+      is_expected.to have_body_text domain.url
+      is_expected.to have_body_text help_page_url('user/project/pages/custom_domains_ssl_tls_certification/index.md', anchor: link_anchor)
+    end
+  end
+
   shared_examples 'notification about upcoming domain removal' do
     context 'when domain is not scheduled for removal' do
       it 'asks user to remove it' do
@@ -56,7 +63,7 @@ describe Emails::PagesDomains do
 
     subject { Notify.pages_domain_enabled_email(domain, user) }
 
-    it_behaves_like 'a pages domain email'
+    it_behaves_like 'a pages domain verification email'
 
     it { is_expected.to have_body_text 'has been enabled' }
   end
@@ -67,7 +74,7 @@ describe Emails::PagesDomains do
 
     subject { Notify.pages_domain_disabled_email(domain, user) }
 
-    it_behaves_like 'a pages domain email'
+    it_behaves_like 'a pages domain verification email'
 
     it_behaves_like 'notification about upcoming domain removal'
 
@@ -80,7 +87,7 @@ describe Emails::PagesDomains do
 
     subject { Notify.pages_domain_verification_succeeded_email(domain, user) }
 
-    it_behaves_like 'a pages domain email'
+    it_behaves_like 'a pages domain verification email'
 
     it { is_expected.to have_body_text 'successfully verified' }
   end
@@ -94,10 +101,18 @@ describe Emails::PagesDomains do
     it_behaves_like 'a pages domain email'
 
     it_behaves_like 'notification about upcoming domain removal'
+  end
+
+  describe '#pages_domain_auto_ssl_failed_email' do
+    let(:email_subject) { "#{project.path} | ACTION REQUIRED: Something went wrong while obtaining the Let's Encrypt certificate for GitLab Pages domain '#{domain.domain}'" }
+
+    subject { Notify.pages_domain_auto_ssl_failed_email(domain, user) }
+
+    it_behaves_like 'a pages domain email'
 
-    it 'says verification has failed and when the domain is enabled until' do
-      is_expected.to have_body_text 'Verification has failed'
-      is_expected.to have_body_text domain.enabled_until.strftime('%F %T')
+    it 'says that we failed to obtain certificate' do
+      is_expected.to have_body_text "Something went wrong while obtaining the Let's Encrypt certificate."
+      is_expected.to have_body_text help_page_url('user/project/pages/custom_domains_ssl_tls_certification/lets_encrypt_integration.md', anchor: 'troubleshooting')
     end
   end
 end

spec/services/notification_service_spec.rb

@@ -2604,6 +2604,7 @@ describe NotificationService, :mailer do
       pages_domain_disabled
       pages_domain_verification_succeeded
       pages_domain_verification_failed
+      pages_domain_auto_ssl_failed
     ].each do |sym|
       describe "##{sym}" do
         subject(:notify!) { notification.send(sym, domain) }

spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb

@@ -180,5 +180,13 @@ describe PagesDomains::ObtainLetsEncryptCertificateService do
 
       expect(PagesDomainAcmeOrder.find_by_id(existing_order.id)).to be_nil
     end
+
+    it 'sends notification' do
+      expect_next_instance_of(NotificationService) do |notification_service|
+        expect(notification_service).to receive(:pages_domain_auto_ssl_failed).with(pages_domain)
+      end
+
+      service.execute
+    end
   end
 end