Rename internal argument in ProcessAlertWorker

Second param of ProcessAlertWorker#perform is an alert payload
not the alert itself.

app/services/projects/alerting/notify_service.rb

@@ -10,7 +10,10 @@ module Projects
         return forbidden unless alerts_service_activated?
         return unauthorized unless valid_token?(token)
 
-        process_incident_issues if process_issues?
+        alert = create_alert
+        return bad_request unless alert.persisted?
+
+        process_incident_issues(alert) if process_issues?
         send_alert_email if send_email?
 
         ServiceResponse.success
@@ -22,13 +25,21 @@ module Projects
 
       delegate :alerts_service, :alerts_service_activated?, to: :project
 
+      def am_alert_params
+        Gitlab::AlertManagement::AlertParams.from_generic_alert(project: project, payload: params.to_h)
+      end
+
+      def create_alert
+        AlertManagement::Alert.create(am_alert_params)
+      end
+
       def send_email?
         incident_management_setting.send_email?
       end
 
-      def process_incident_issues
+      def process_incident_issues(alert)
         IncidentManagement::ProcessAlertWorker
-          .perform_async(project.id, parsed_payload)
+          .perform_async(project.id, parsed_payload, alert.id)
       end
 
       def send_alert_email

app/workers/incident_management/process_alert_worker.rb

@@ -7,11 +7,14 @@ module IncidentManagement
     queue_namespace :incident_management
     feature_category :incident_management
 
-    def perform(project_id, alert)
+    def perform(project_id, alert_payload, am_alert_id = nil)
       project = find_project(project_id)
       return unless project
 
-      create_issue(project, alert)
+      new_issue = create_issue(project, alert_payload)
+      return unless am_alert_id && new_issue.persisted?
+
+      link_issue_with_alert(am_alert_id, new_issue.id)
     end
 
     private
@@ -20,10 +23,24 @@ module IncidentManagement
       Project.find_by_id(project_id)
     end
 
-    def create_issue(project, alert)
+    def create_issue(project, alert_payload)
       IncidentManagement::CreateIssueService
-        .new(project, alert)
+        .new(project, alert_payload)
         .execute
     end
+
+    def link_issue_with_alert(alert_id, issue_id)
+      alert = AlertManagement::Alert.find_by_id(alert_id)
+      return unless alert
+
+      return if alert.update(issue_id: issue_id)
+
+      Gitlab::GitLogger.warn(
+        message: 'Cannot link an Issue with Alert',
+        issue_id: issue_id,
+        alert_id: alert_id,
+        alert_errors: alert.errors.messages
+      )
+    end
   end
 end

lib/gitlab/alert_management/alert_params.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module AlertManagement
+    class AlertParams
+      def self.from_generic_alert(project:, payload:)
+        parsed_payload = Gitlab::Alerting::NotificationPayloadParser.call(payload).with_indifferent_access
+        annotations = parsed_payload[:annotations]
+
+        {
+          project_id: project.id,
+          title: annotations[:title],
+          description: annotations[:description],
+          monitoring_tool: annotations[:monitoring_tool],
+          service: annotations[:service],
+          hosts: Array(annotations[:hosts]),
+          payload: payload,
+          started_at: parsed_payload['startsAt']
+        }
+      end
+    end
+  end
+end

spec/lib/gitlab/alert_management/alert_params_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::AlertManagement::AlertParams do
+  let_it_be(:project) { create(:project, :repository, :private) }
+
+  describe '.from_generic_alert' do
+    let(:started_at) { Time.current.change(usec: 0).rfc3339 }
+    let(:payload) do
+      {
+        'title' => 'Alert title',
+        'description' => 'Description',
+        'monitoring_tool' => 'Monitoring tool name',
+        'service' => 'Service',
+        'hosts' => ['gitlab.com'],
+        'start_time' => started_at,
+        'some' => { 'extra' => { 'payload' => 'here' } }
+      }
+    end
+
+    subject { described_class.from_generic_alert(project: project, payload: payload) }
+
+    it 'returns Alert compatible parameters' do
+      is_expected.to eq(
+        project_id: project.id,
+        title: 'Alert title',
+        description: 'Description',
+        monitoring_tool: 'Monitoring tool name',
+        service: 'Service',
+        hosts: ['gitlab.com'],
+        payload: payload,
+        started_at: started_at
+      )
+    end
+
+    context 'when there are no hosts in the payload' do
+      let(:payload) { {} }
+
+      it 'hosts param is an empty array' do
+        expect(subject[:hosts]).to be_empty
+      end
+    end
+  end
+end

spec/services/projects/alerting/notify_service_spec.rb

@@ -12,11 +12,16 @@ describe Projects::Alerting::NotifyService do
 
   shared_examples 'processes incident issues' do |amount|
     let(:create_incident_service) { spy }
+    let(:new_alert) { instance_double(AlertManagement::Alert, id: 503, persisted?: true) }
 
     it 'processes issues' do
+      expect(AlertManagement::Alert)
+        .to receive(:create)
+        .and_return(new_alert)
+
       expect(IncidentManagement::ProcessAlertWorker)
         .to receive(:perform_async)
-        .with(project.id, kind_of(Hash))
+        .with(project.id, kind_of(Hash), new_alert.id)
         .exactly(amount).times
 
       Sidekiq::Testing.inline! do
@@ -59,6 +64,12 @@ describe Projects::Alerting::NotifyService do
     end
   end
 
+  shared_examples 'NotifyService does not create alert' do
+    it 'does not create alert' do
+      expect { subject }.not_to change(AlertManagement::Alert, :count)
+    end
+  end
+
   describe '#execute' do
     let(:token) { 'invalid-token' }
     let(:starts_at) { Time.now.change(usec: 0) }
@@ -88,6 +99,36 @@ describe Projects::Alerting::NotifyService do
             .and_return(incident_management_setting)
         end
 
+        context 'with valid payload' do
+          it 'creates AlertManagement::Alert' do
+            expect { subject }.to change(AlertManagement::Alert, :count).by(1)
+          end
+
+          it 'created alert has all data properly assigned' do
+            subject
+
+            alert = AlertManagement::Alert.last
+            alert_attributes = alert.attributes.except('id', 'iid', 'created_at', 'updated_at')
+
+            expect(alert_attributes).to eq(
+              'project_id' => project.id,
+              'issue_id' => nil,
+              'fingerprint' => nil,
+              'title' => 'alert title',
+              'description' => nil,
+              'monitoring_tool' => nil,
+              'service' => nil,
+              'hosts' => [],
+              'payload' => payload_raw,
+              'severity' => 'critical',
+              'status' => 'triggered',
+              'events' => 1,
+              'started_at' => alert.started_at,
+              'ended_at' => nil
+            )
+          end
+        end
+
         it_behaves_like 'does not process incident issues'
 
         context 'issue enabled' do
@@ -103,6 +144,7 @@ describe Projects::Alerting::NotifyService do
             end
 
             it_behaves_like 'does not process incident issues due to error', http_status: :bad_request
+            it_behaves_like 'NotifyService does not create alert'
           end
         end
 
@@ -115,12 +157,14 @@ describe Projects::Alerting::NotifyService do
 
       context 'with invalid token' do
         it_behaves_like 'does not process incident issues due to error', http_status: :unauthorized
+        it_behaves_like 'NotifyService does not create alert'
       end
 
       context 'with deactivated Alerts Service' do
         let!(:alerts_service) { create(:alerts_service, :inactive, project: project) }
 
         it_behaves_like 'does not process incident issues due to error', http_status: :forbidden
+        it_behaves_like 'NotifyService does not create alert'
       end
     end
   end

spec/workers/incident_management/process_alert_worker_spec.rb

@@ -6,16 +6,24 @@ describe IncidentManagement::ProcessAlertWorker do
   let_it_be(:project) { create(:project) }
 
   describe '#perform' do
-    let(:alert) { :alert }
-    let(:create_issue_service) { spy(:create_issue_service) }
+    let(:alert_management_alert_id) { nil }
+    let(:alert_payload) { { alert: 'payload' } }
+    let(:new_issue) { create(:issue, project: project) }
+    let(:create_issue_service) { instance_double(IncidentManagement::CreateIssueService, execute: new_issue) }
 
-    subject { described_class.new.perform(project.id, alert) }
+    subject { described_class.new.perform(project.id, alert_payload, alert_management_alert_id) }
+
+    before do
+      allow(IncidentManagement::CreateIssueService)
+        .to receive(:new).with(project, alert_payload)
+        .and_return(create_issue_service)
+    end
 
     it 'calls create issue service' do
       expect(Project).to receive(:find_by_id).and_call_original
 
       expect(IncidentManagement::CreateIssueService)
-        .to receive(:new).with(project, :alert)
+        .to receive(:new).with(project, alert_payload)
         .and_return(create_issue_service)
 
       expect(create_issue_service).to receive(:execute)
@@ -26,7 +34,7 @@ describe IncidentManagement::ProcessAlertWorker do
     context 'with invalid project' do
       let(:invalid_project_id) { 0 }
 
-      subject { described_class.new.perform(invalid_project_id, alert) }
+      subject { described_class.new.perform(invalid_project_id, alert_payload) }
 
       it 'does not create issues' do
         expect(Project).to receive(:find_by_id).and_call_original
@@ -35,5 +43,54 @@ describe IncidentManagement::ProcessAlertWorker do
         subject
       end
     end
+
+    context 'when alert_management_alert_id is present' do
+      let!(:alert) { create(:alert_management_alert, project: project) }
+      let(:alert_management_alert_id) { alert.id }
+
+      before do
+        allow(AlertManagement::Alert)
+          .to receive(:find_by_id)
+          .with(alert_management_alert_id)
+          .and_return(alert)
+
+        allow(Gitlab::GitLogger).to receive(:warn).and_call_original
+      end
+
+      context 'when alert can be updated' do
+        it 'updates AlertManagement::Alert#issue_id' do
+          expect { subject }.to change { alert.reload.issue_id }.to(new_issue.id)
+        end
+
+        it 'does not write a warning to log' do
+          subject
+
+          expect(Gitlab::GitLogger).not_to have_received(:warn)
+        end
+      end
+
+      context 'when alert cannot be updated' do
+        before do
+          # invalidate alert
+          too_many_hosts = Array.new(AlertManagement::Alert::HOSTS_MAX_LENGTH + 1) { |_| 'host' }
+          alert.update_columns(hosts: too_many_hosts)
+        end
+
+        it 'updates AlertManagement::Alert#issue_id' do
+          expect { subject }.not_to change { alert.reload.issue_id }
+        end
+
+        it 'writes a worning to log' do
+          subject
+
+          expect(Gitlab::GitLogger).to have_received(:warn).with(
+            message: 'Cannot link an Issue with Alert',
+            issue_id: new_issue.id,
+            alert_id: alert_management_alert_id,
+            alert_errors: { hosts: ['hosts array is over 255 chars'] }
+          )
+        end
+      end
+    end
   end
 end