Merge branch '204908-ensure-correct-prometheus-alert-found' into 'master'

Resolve "Firing prometheus alerts configured by gitlab are not found by environment"

Closes #204908

See merge request gitlab-org/gitlab!29119

app/models/prometheus_alert.rb

@@ -56,7 +56,8 @@ class PrometheusAlert < ApplicationRecord
       "for" => "5m",
       "labels" => {
         "gitlab" => "hook",
-        "gitlab_alert_id" => prometheus_metric_id
+        "gitlab_alert_id" => prometheus_metric_id,
+        "gitlab_prometheus_alert_id" => id
       }
     }
   end

changelogs/unreleased/204908-ensure-correct-prometheus-alert-found.yml

+---
+title: Prevent wrong environment being used when processing Prometheus alert
+merge_request: 29119
+author:
+type: fixed

lib/gitlab/alerting/alert.rb

@@ -21,6 +21,12 @@ module Gitlab
         end
       end
 
+      def gitlab_prometheus_alert_id
+        strong_memoize(:gitlab_prometheus_alert_id) do
+          payload&.dig('labels', 'gitlab_prometheus_alert_id')
+        end
+      end
+
       def title
         strong_memoize(:title) do
           gitlab_alert&.title || parse_title_from_payload
@@ -120,12 +126,19 @@ module Gitlab
       end
 
       def parse_gitlab_alert_from_payload
-        return unless metric_id
+        alerts_found = matching_gitlab_alerts
+
+        return if alerts_found.blank? || alerts_found.size > 1
+
+        alerts_found.first
+      end
+
+      def matching_gitlab_alerts
+        return unless metric_id || gitlab_prometheus_alert_id
 
         Projects::Prometheus::AlertsFinder
-          .new(project: project, metric: metric_id)
+          .new(project: project, metric: metric_id, id: gitlab_prometheus_alert_id)
           .execute
-          .first
       end
 
       def parse_title_from_payload

spec/lib/gitlab/alerting/alert_spec.rb

@@ -9,11 +9,14 @@ describe Gitlab::Alerting::Alert do
   let(:payload) { {} }
 
   shared_context 'gitlab alert' do
-    let(:gitlab_alert_id) { gitlab_alert.prometheus_metric_id.to_s }
     let!(:gitlab_alert) { create(:prometheus_alert, project: project) }
+    let(:gitlab_alert_id) { gitlab_alert.id }
 
     before do
-      payload['labels'] = { 'gitlab_alert_id' => gitlab_alert_id }
+      payload['labels'] = {
+        'gitlab_alert_id' => gitlab_alert.prometheus_metric_id.to_s,
+        'gitlab_prometheus_alert_id' => gitlab_alert_id
+      }
     end
   end
 
@@ -68,6 +71,41 @@ describe Gitlab::Alerting::Alert do
 
       it { is_expected.to be_nil }
     end
+
+    context 'when two alerts with the same metric exist' do
+      include_context 'gitlab alert'
+
+      let!(:second_gitlab_alert) do
+        create(:prometheus_alert,
+          project: project,
+          prometheus_metric_id: gitlab_alert.prometheus_metric_id
+        )
+      end
+
+      context 'alert id given in params' do
+        before do
+          payload['labels'] = {
+            'gitlab_alert_id' => gitlab_alert.prometheus_metric_id.to_s,
+            'gitlab_prometheus_alert_id' => second_gitlab_alert.id
+          }
+        end
+
+        it { is_expected.to eq(second_gitlab_alert) }
+      end
+
+      context 'metric id given in params' do
+        # This tests the case when two alerts are found, as metric id
+        # is not unique.
+
+        # Note the metric id was incorrectly named as 'gitlab_alert_id'
+        # in PrometheusAlert#to_param.
+        before do
+          payload['labels'] = { 'gitlab_alert_id' => gitlab_alert.prometheus_metric_id }
+        end
+
+        it { is_expected.to be_nil }
+      end
+    end
   end
 
   describe '#title' do

spec/models/prometheus_alert_spec.rb

@@ -96,7 +96,8 @@ describe PrometheusAlert do
         "for" => "5m",
         "labels" => {
           "gitlab" => "hook",
-          "gitlab_alert_id" => metric.id
+          "gitlab_alert_id" => metric.id,
+          "gitlab_prometheus_alert_id" => subject.id
         })
     end
   end