Send a notification email on mirror update errors

The email is sent to project maintainers containing the last mirror
update error. This will allow maintainers to set alarms and react
accordingly.

17 files changed, 240 insertions, 21 deletions

diff --git a/app/finders/remote_mirror_finder.rb b/app/finders/remote_mirror_finder.rb
new file mode 100644
index 00000000000..420db0077aa
--- /dev/null
+++ b/app/finders/remote_mirror_finder.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class RemoteMirrorFinder
+  attr_accessor :params
+
+  def initialize(params)
+    @params = params
+  end
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def execute
+    RemoteMirror.find_by(id: params[:id])
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+end
diff --git a/app/mailers/emails/remote_mirrors.rb b/app/mailers/emails/remote_mirrors.rb
new file mode 100644
index 00000000000..2018eb7260b
--- /dev/null
+++ b/app/mailers/emails/remote_mirrors.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Emails
+  module RemoteMirrors
+    def remote_mirror_update_failed_email(remote_mirror_id, recipient_id)
+      @remote_mirror = RemoteMirrorFinder.new(id: remote_mirror_id).execute
+      @project = @remote_mirror.project
+
+      mail(to: recipient(recipient_id), subject: subject('Remote mirror update failed'))
+    end
+  end
+end
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 88ad4c3e893..099ad779aa5 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -13,6 +13,7 @@ class Notify < BaseMailer
   include Emails::Pipelines
   include Emails::Members
   include Emails::AutoDevops
+  include Emails::RemoteMirrors
 
   helper MergeRequestsHelper
   helper DiffHelper
diff --git a/app/mailers/previews/notify_preview.rb b/app/mailers/previews/notify_preview.rb
index e7e8d96eca4..2ac4610967d 100644
--- a/app/mailers/previews/notify_preview.rb
+++ b/app/mailers/previews/notify_preview.rb
@@ -145,6 +145,10 @@ class NotifyPreview < ActionMailer::Preview
     Notify.autodevops_disabled_email(pipeline, user.email).message
   end
 
+  def remote_mirror_update_failed_email
+    Notify.remote_mirror_update_failed_email(remote_mirror.id, user.id).message
+  end
+
   private
 
   def project
@@ -167,6 +171,10 @@ class NotifyPreview < ActionMailer::Preview
     @pipeline = Ci::Pipeline.last
   end
 
+  def remote_mirror
+    @remote_mirror ||= RemoteMirror.last
+  end
+
   def user
     @user ||= User.last
   end
diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb
index b7b4d0f1be9..5a6895aefab 100644
--- a/app/models/remote_mirror.rb
+++ b/app/models/remote_mirror.rb
@@ -65,10 +65,14 @@ class RemoteMirror < ActiveRecord::Base
       )
     end
 
-    after_transition started: :failed do |remote_mirror, _|
+    after_transition started: :failed do |remote_mirror|
       Gitlab::Metrics.add_event(:remote_mirrors_failed)
 
       remote_mirror.update(last_update_at: Time.now)
+
+      remote_mirror.run_after_commit do
+        RemoteMirrorNotificationWorker.perform_async(remote_mirror.id)
+      end
     end
   end
 
@@ -135,8 +139,8 @@ class RemoteMirror < ActiveRecord::Base
   end
 
   def mark_as_failed(error_message)
-    update_fail
     update_column(:last_error, Gitlab::UrlSanitizer.sanitize(error_message))
+    update_fail
   end
 
   def url=(value)
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 9c236d7f41d..68cdc69023a 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -24,6 +24,10 @@ module NotificationRecipientService
     Builder::MergeRequestUnmergeable.new(*args).notification_recipients
   end
 
+  def self.build_project_maintainers_recipients(*args)
+    Builder::ProjectMaintainers.new(*args).notification_recipients
+  end
+
   module Builder
     class Base
       def initialize(*)
@@ -380,5 +384,24 @@ module NotificationRecipientService
         nil
       end
     end
+
+    class ProjectMaintainers < Base
+      attr_reader :target
+
+      def initialize(target, action:)
+        @target = target
+        @action = action
+      end
+
+      def build!
+        return [] unless project
+
+        add_recipients(project.team.maintainers, :watch, nil)
+      end
+
+      def acting_user
+        nil
+      end
+    end
   end
 end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index e24ef7f9c87..ff035fea216 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -429,26 +429,26 @@ class NotificationService
   end
 
   def pages_domain_verification_succeeded(domain)
-    recipients_for_pages_domain(domain).each do |user|
-      mailer.pages_domain_verification_succeeded_email(domain, user).deliver_later
+    project_maintainers_recipients(domain, action: 'succeeded').each do |recipient|
+      mailer.pages_domain_verification_succeeded_email(domain, recipient.user).deliver_later
     end
   end
 
   def pages_domain_verification_failed(domain)
-    recipients_for_pages_domain(domain).each do |user|
-      mailer.pages_domain_verification_failed_email(domain, user).deliver_later
+    project_maintainers_recipients(domain, action: 'failed').each do |recipient|
+      mailer.pages_domain_verification_failed_email(domain, recipient.user).deliver_later
     end
   end
 
   def pages_domain_enabled(domain)
-    recipients_for_pages_domain(domain).each do |user|
-      mailer.pages_domain_enabled_email(domain, user).deliver_later
+    project_maintainers_recipients(domain, action: 'enabled').each do |recipient|
+      mailer.pages_domain_enabled_email(domain, recipient.user).deliver_later
     end
   end
 
   def pages_domain_disabled(domain)
-    recipients_for_pages_domain(domain).each do |user|
-      mailer.pages_domain_disabled_email(domain, user).deliver_later
+    project_maintainers_recipients(domain, action: 'disabled').each do |recipient|
+      mailer.pages_domain_disabled_email(domain, recipient.user).deliver_later
     end
   end
 
@@ -474,6 +474,14 @@ class NotificationService
     mailer.send(:repository_cleanup_failure_email, project, user, error).deliver_later
   end
 
+  def remote_mirror_update_failed(remote_mirror)
+    recipients = project_maintainers_recipients(remote_mirror, action: 'update_failed')
+
+    recipients.each do |recipient|
+      mailer.remote_mirror_update_failed_email(remote_mirror.id, recipient.user.id).deliver_later
+    end
+  end
+
   protected
 
   def new_resource_email(target, method)
@@ -569,12 +577,8 @@ class NotificationService
 
   private
 
-  def recipients_for_pages_domain(domain)
-    project = domain.project
-
-    return [] unless project
-
-    notifiable_users(project.team.maintainers, :watch, target: project)
+  def project_maintainers_recipients(target, action:)
+    NotificationRecipientService.build_project_maintainers_recipients(target, action: action)
   end
 
   def notifiable?(*args)
diff --git a/app/views/notify/remote_mirror_update_failed_email.html.haml b/app/views/notify/remote_mirror_update_failed_email.html.haml
new file mode 100644
index 00000000000..4fb0a4c5a8a
--- /dev/null
+++ b/app/views/notify/remote_mirror_update_failed_email.html.haml
@@ -0,0 +1,46 @@
+%tr.alert{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;" }
+  %td{ style: "padding:10px;border-radius:3px;font-size:14px;line-height:1.3;text-align:center;overflow:hidden;background-color:#d22f57;color:#ffffff;" }
+    %table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;margin:0 auto;" }
+      %tbody
+        %tr
+          %td{ style: "vertical-align:middle;color:#ffffff;text-align:center;padding-right:5px;line-height:1;" }
+            %img{ alt: "✖", height: "13", src: image_url('mailers/ci_pipeline_notif_v1/icon-x-red-inverted.gif'), style: "display:block;", width: "13" }/
+          %td{ style: "vertical-align:middle;color:#ffffff;text-align:center;" }
+            A remote mirror update has failed.
+%tr.spacer{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;" }
+  %td{ style: "height:18px;font-size:18px;line-height:18px;" }
+    &nbsp;
+%tr.section{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;" }
+  %td{ style: "padding:0 15px;border:1px solid #ededed;border-radius:3px;overflow:hidden;" }
+    %table.table-info{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:100%;" }
+      %tbody{ style: "font-size:15px;line-height:1.4;color:#8c8c8c;" }
+        %tr
+          %td{ style: "font-weight:300;padding:14px 0;margin:0;" } Project
+          %td{ style: "font-weight:500;padding:14px 0;margin:0;color:#333333;width:75%;padding-left:5px;" }
+            - namespace_url = @project.group ? group_url(@project.group) : user_url(@project.namespace.owner)
+            %a.muted{ href: namespace_url, style: "color:#333333;text-decoration:none;" }
+              = @project.owner_name
+            \/
+            %a.muted{ href: project_url(@project), style: "color:#333333;text-decoration:none;" }
+              = @project.name
+        %tr
+          %td{ style: "font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Remote mirror
+          %td{ style: "font-weight:500;padding:14px 0;margin:0;color:#333333;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
+            = @remote_mirror.safe_url
+        %tr
+          %td{ style: "font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Last update at
+          %td{ style: "font-weight:500;padding:14px 0;margin:0;color:#333333;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
+            = @remote_mirror.last_update_at
+
+%tr.table-warning{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;" }
+  %td{ style: "border: 1px solid #ededed; border-bottom: 0; border-radius: 4px 4px 0 0; overflow: hidden; background-color: #fdf4f6; color: #d22852; font-size: 14px; line-height: 1.4; text-align: center; padding: 8px 16px;" }
+    Logs may contain sensitive data. Please consider before forwarding this email.
+%tr.section{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;" }
+  %td{ style: "padding: 0 16px; border: 1px solid #ededed; border-radius: 4px; overflow: hidden; border-top: 0; border-radius: 0 0 4px 4px;" }
+    %table.builds{ border: "0", cellpadding: "0", cellspacing: "0", style: "width: 100%; border-collapse: collapse;" }
+      %tbody
+        %tr.build-log
+          %td{ colspan: "2", style: "padding: 0 0 16px;" }
+          %pre{ style: "font-family: Monaco,'Lucida Console','Courier New',Courier,monospace; background-color: #fafafa; border-radius: 4px; overflow: hidden; white-space: pre-wrap; word-break: break-all; font-size:13px; line-height: 1.4; padding: 16px 8px; color: #333333; margin: 0;" }
+            = @remote_mirror.last_error
+
diff --git a/app/views/notify/remote_mirror_update_failed_email.text.erb b/app/views/notify/remote_mirror_update_failed_email.text.erb
new file mode 100644
index 00000000000..c6f29f0ad1c
--- /dev/null
+++ b/app/views/notify/remote_mirror_update_failed_email.text.erb
@@ -0,0 +1,7 @@
+A remote mirror update has failed.
+
+Project: <%= @project.human_name %> ( <%= project_url(@project) %> )
+Remote mirror: <%= @remote_mirror.safe_url %>
+Last update at: <%= @remote_mirror.last_update_at %>
+Last error:
+<%= @remote_mirror.last_error %>
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index dfce00a10a1..bc26b3f8ef2 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -124,6 +124,7 @@
 - propagate_service_template
 - reactive_caching
 - rebase
+- remote_mirror_notification
 - repository_fork
 - repository_import
 - repository_remove_remote
diff --git a/app/workers/remote_mirror_notification_worker.rb b/app/workers/remote_mirror_notification_worker.rb
new file mode 100644
index 00000000000..70c2e857d09
--- /dev/null
+++ b/app/workers/remote_mirror_notification_worker.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class RemoteMirrorNotificationWorker
+  include ApplicationWorker
+
+  def perform(remote_mirror_id)
+    remote_mirror = RemoteMirrorFinder.new(id: remote_mirror_id).execute
+
+    # We check again if there's an error because a newer run since this job was
+    # fired could've completed successfully.
+    return unless remote_mirror && remote_mirror.last_error.present?
+
+    NotificationService.new.remote_mirror_update_failed(remote_mirror)
+  end
+end
diff --git a/app/workers/repository_update_remote_mirror_worker.rb b/app/workers/repository_update_remote_mirror_worker.rb
index 9d4e67deb9c..a06db4b08cc 100644
--- a/app/workers/repository_update_remote_mirror_worker.rb
+++ b/app/workers/repository_update_remote_mirror_worker.rb
@@ -16,7 +16,7 @@ class RepositoryUpdateRemoteMirrorWorker
   end
 
   def perform(remote_mirror_id, scheduled_time)
-    remote_mirror = RemoteMirror.find(remote_mirror_id)
+    remote_mirror = RemoteMirrorFinder.new(id: remote_mirror_id).execute
     return if remote_mirror.updated_since?(scheduled_time)
 
     raise UpdateAlreadyInProgressError if remote_mirror.update_in_progress?
diff --git a/changelogs/unreleased/remote-mirror-update-failed-notification.yml b/changelogs/unreleased/remote-mirror-update-failed-notification.yml
new file mode 100644
index 00000000000..50ec8624ae5
--- /dev/null
+++ b/changelogs/unreleased/remote-mirror-update-failed-notification.yml
@@ -0,0 +1,5 @@
+---
+title: Send a notification email to project maintainers when a mirror update fails
+merge_request: 23595
+author:
+type: added
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index 5985569bef4..3ee32678f34 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -84,3 +84,4 @@
   - [object_pool, 1]
   - [repository_cleanup, 1]
   - [delete_stored_files, 1]
+  - [remote_mirror_notification, 2]
diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb
index b12ca79847c..5d3c25062d5 100644
--- a/spec/models/remote_mirror_spec.rb
+++ b/spec/models/remote_mirror_spec.rb
@@ -1,6 +1,6 @@
 require 'rails_helper'
 
-describe RemoteMirror do
+describe RemoteMirror, :mailer do
   include GitHelpers
 
   describe 'URL validation' do
@@ -137,6 +137,43 @@ describe RemoteMirror do
     end
   end
 
+  describe '#mark_as_failed' do
+    let(:remote_mirror) { create(:remote_mirror) }
+    let(:error_message) { 'http://user:pass@test.com/root/repoC.git/' }
+    let(:sanitized_error_message) { 'http://*****:*****@test.com/root/repoC.git/' }
+
+    subject do
+      remote_mirror.update_start
+      remote_mirror.mark_as_failed(error_message)
+    end
+
+    it 'sets the update_status to failed' do
+      subject
+
+      expect(remote_mirror.reload.update_status).to eq('failed')
+    end
+
+    it 'saves the sanitized error' do
+      subject
+
+      expect(remote_mirror.last_error).to eq(sanitized_error_message)
+    end
+
+    context 'notifications' do
+      let(:user) { create(:user) }
+
+      before do
+        remote_mirror.project.add_maintainer(user)
+      end
+
+      it 'notifies the project maintainers' do
+        perform_enqueued_jobs { subject }
+
+        should_email(user)
+      end
+    end
+  end
+
   context 'when remote mirror gets destroyed' do
     it 'removes remote' do
       mirror = create_mirror(url: 'http://foo:bar@test.com')
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 0f6c2604984..68ac3a00ab0 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -2167,6 +2167,39 @@ describe NotificationService, :mailer do
     end
   end
 
+  context 'Remote mirror notifications' do
+    describe '#remote_mirror_update_failed' do
+      let(:project) { create(:project) }
+      let(:remote_mirror) { create(:remote_mirror, project: project) }
+      let(:u_blocked) { create(:user, :blocked) }
+      let(:u_silence) { create_user_with_notification(:disabled, 'silent-maintainer', project) }
+      let(:u_owner)   { project.owner }
+      let(:u_maintainer1) { create(:user) }
+      let(:u_maintainer2) { create(:user) }
+      let(:u_developer) { create(:user) }
+
+      before do
+        project.add_maintainer(u_blocked)
+        project.add_maintainer(u_silence)
+        project.add_maintainer(u_maintainer1)
+        project.add_maintainer(u_maintainer2)
+        project.add_developer(u_developer)
+
+        # Mock remote update
+        allow(project.repository).to receive(:async_remove_remote)
+        allow(project.repository).to receive(:add_remote)
+
+        reset_delivered_emails!
+      end
+
+      it 'emails current watching maintainers' do
+        notification.remote_mirror_update_failed(remote_mirror)
+
+        should_only_email(u_maintainer1, u_maintainer2, u_owner)
+      end
+    end
+  end
+
   def build_team(project)
     @u_watcher               = create_global_setting_for(create(:user), :watch)
     @u_participating         = create_global_setting_for(create(:user), :participating)
diff --git a/spec/workers/repository_update_remote_mirror_worker_spec.rb b/spec/workers/repository_update_remote_mirror_worker_spec.rb
index 4f1ad2474f5..d73b0b53713 100644
--- a/spec/workers/repository_update_remote_mirror_worker_spec.rb
+++ b/spec/workers/repository_update_remote_mirror_worker_spec.rb
@@ -25,12 +25,19 @@ describe RepositoryUpdateRemoteMirrorWorker do
       it 'sets status as failed when update remote mirror service executes with errors' do
         error_message = 'fail!'
 
-        expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :error, message: error_message)
+        expect_next_instance_of(Projects::UpdateRemoteMirrorService) do |service|
+          expect(service).to receive(:execute).with(remote_mirror).and_return(status: :error, message: error_message)
+        end
+
+        # Mock the finder so that it returns an object we can set expectations on
+        expect_next_instance_of(RemoteMirrorFinder) do |finder|
+          expect(finder).to receive(:execute).and_return(remote_mirror)
+        end
+        expect(remote_mirror).to receive(:mark_as_failed).with(error_message)
+
         expect do
           subject.perform(remote_mirror.id, Time.now)
         end.to raise_error(RepositoryUpdateRemoteMirrorWorker::UpdateError, error_message)
-
-        expect(remote_mirror.reload.update_status).to eq('failed')
       end
 
       it 'does nothing if last_update_started_at is higher than the time the job was scheduled in' do