Merge branch...

Merge branch '218560-allow-generic-endpoint-to-receive-alerts-from-external-prometheus' into 'master'

Allow generic endpoint to receive alerts from external Prometheus

See merge request gitlab-org/gitlab!32676

app/controllers/projects/alerting/notifications_controller.rb

@@ -29,12 +29,22 @@ module Projects
       end
 
       def notify_service
-        Projects::Alerting::NotifyService
-          .new(project, current_user, notification_payload)
+        notify_service_class.new(project, current_user, notification_payload)
+      end
+
+      def notify_service_class
+        # We are tracking the consolidation of these services in
+        # https://gitlab.com/groups/gitlab-org/-/epics/3360
+        # to get rid of this workaround.
+        if Projects::Prometheus::Alerts::NotifyService.processable?(notification_payload)
+          Projects::Prometheus::Alerts::NotifyService
+        else
+          Projects::Alerting::NotifyService
+        end
       end
 
       def notification_payload
-        params.permit![:notification]
+        @notification_payload ||= params.permit![:notification]
       end
     end
   end

app/services/projects/prometheus/alerts/notify_service.rb

@@ -7,9 +7,19 @@ module Projects
         include Gitlab::Utils::StrongMemoize
         include IncidentManagement::Settings
 
+        # This set of keys identifies a payload as a valid Prometheus
+        # payload and thus processable by this service. See also
+        # https://prometheus.io/docs/alerting/configuration/#webhook_config
+        REQUIRED_PAYLOAD_KEYS = %w[
+          version groupKey status receiver groupLabels commonLabels
+          commonAnnotations externalURL alerts
+        ].to_set.freeze
+
+        SUPPORTED_VERSION = '4'
+
         def execute(token)
           return bad_request unless valid_payload_size?
-          return unprocessable_entity unless valid_version?
+          return unprocessable_entity unless self.class.processable?(params)
           return unauthorized unless valid_alert_manager_token?(token)
 
           process_prometheus_alerts
@@ -20,6 +30,14 @@ module Projects
           ServiceResponse.success
         end
 
+        def self.processable?(params)
+          # Workaround for https://gitlab.com/gitlab-org/gitlab/-/issues/220496
+          return false unless params
+
+          REQUIRED_PAYLOAD_KEYS.subset?(params.keys.to_set) &&
+            params['version'] == SUPPORTED_VERSION
+        end
+
         private
 
         def valid_payload_size?
@@ -42,12 +60,10 @@ module Projects
           params['alerts']
         end
 
-        def valid_version?
-          params['version'] == '4'
-        end
-
         def valid_alert_manager_token?(token)
-          valid_for_manual?(token) || valid_for_managed?(token)
+          valid_for_manual?(token) ||
+            valid_for_alerts_endpoint?(token) ||
+            valid_for_managed?(token)
         end
 
         def valid_for_manual?(token)
@@ -61,6 +77,13 @@ module Projects
           end
         end
 
+        def valid_for_alerts_endpoint?(token)
+          return false unless project.alerts_service_activated?
+
+          # Here we are enforcing the existence of the token
+          compare_token(token, project.alerts_service.token)
+        end
+
         def valid_for_managed?(token)
           prometheus_application = available_prometheus_application(project)
           return false unless prometheus_application

changelogs/unreleased/218560-allow-generic-endpoint-to-receive-alerts-from-external-prometheus.yml

+---
+title: Allow generic endpoint to receive alerts from external Prometheus
+merge_request: 32676
+author:
+type: added

doc/user/project/integrations/generic_alerts.md

@@ -18,6 +18,9 @@ create an issue with the payload in the body of the issue. You can always
 The entire payload will be posted in the issue discussion as a comment
 authored by the GitLab Alert Bot.
 
+NOTE: **Note**
+In GitLab versions 13.1 and greater, you can configure [External Prometheus instances](prometheus.md#external-prometheus-instances) to use this endpoint.
+
 ## Setting up generic alerts
 
 To set up the generic alerts integration:

doc/user/project/integrations/prometheus.md

@@ -981,6 +981,9 @@ receivers:
 
 In order for GitLab to associate your alerts with an [environment](../../../ci/environments/index.md), you need to configure a `gitlab_environment_name` label on the alerts you set up in Prometheus. The value of this should match the name of your Environment in GitLab.
 
+NOTE: **Note**
+In GitLab versions 13.1 and greater, you can configure your manually configured Prometheus server to use the [Generic alerts integration](generic_alerts.md).
+
 ### Taking action on incidents **(ULTIMATE)**
 
 >- [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/4925) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 11.11.

ee/spec/services/projects/prometheus/alerts/notify_service_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Projects::Prometheus::Alerts::NotifyService do
+  include PrometheusHelpers
+
   let_it_be(:project, reload: true) { create(:project) }
 
   let(:service) { described_class.new(project, nil, payload) }
@@ -94,7 +96,7 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
   context 'with valid payload' do
     let(:alert_firing) { create(:prometheus_alert, project: project) }
     let(:alert_resolved) { create(:prometheus_alert, project: project) }
-    let(:payload_raw) { payload_for(firing: [alert_firing], resolved: [alert_resolved]) }
+    let(:payload_raw) { prometheus_alert_payload(firing: [alert_firing], resolved: [alert_resolved]) }
     let(:payload) { ActionController::Parameters.new(payload_raw).permit! }
     let(:payload_alert_firing) { payload_raw['alerts'].first }
     let(:token) { 'token' }
@@ -135,51 +137,4 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
       end
     end
   end
-
-  private
-
-  def payload_for(firing: [], resolved: [])
-    status = firing.any? ? 'firing' : 'resolved'
-    alerts = firing + resolved
-    alert_name = alerts.first.title
-    prometheus_metric_id = alerts.first.prometheus_metric_id.to_s
-
-    alerts_map = \
-      firing.map { |alert| map_alert_payload('firing', alert) } +
-      resolved.map { |alert| map_alert_payload('resolved', alert) }
-
-    # See https://prometheus.io/docs/alerting/configuration/#%3Cwebhook_config%3E
-    {
-      'version' => '4',
-      'receiver' => 'gitlab',
-      'status' => status,
-      'alerts' => alerts_map,
-      'groupLabels' => {
-        'alertname' => alert_name
-      },
-      'commonLabels' => {
-        'alertname' => alert_name,
-        'gitlab' => 'hook',
-        'gitlab_alert_id' => prometheus_metric_id
-      },
-      'commonAnnotations' => {},
-      'externalURL' => '',
-      'groupKey' => "{}:{alertname=\'#{alert_name}\'}"
-    }
-  end
-
-  def map_alert_payload(status, alert)
-    {
-      'status' => status,
-      'labels' => {
-        'alertname' => alert.title,
-        'gitlab' => 'hook',
-        'gitlab_alert_id' => alert.prometheus_metric_id.to_s
-      },
-      'annotations' => {},
-      'startsAt' => '2018-09-24T08:57:31.095725221Z',
-      'endsAt' => '0001-01-01T00:00:00Z',
-      'generatorURL' => 'http://prometheus-prometheus-server-URL'
-    }
-  end
 end

spec/controllers/projects/alerting/notifications_controller_spec.rb

@@ -7,85 +7,96 @@ RSpec.describe Projects::Alerting::NotificationsController do
   let_it_be(:environment) { create(:environment, project: project) }
 
   describe 'POST #create' do
-    let(:service_response) { ServiceResponse.success }
-    let(:notify_service) { instance_double(Projects::Alerting::NotifyService, execute: service_response) }
-
     around do |example|
       ForgeryProtection.with_forgery_protection { example.run }
     end
 
-    before do
-      allow(Projects::Alerting::NotifyService).to receive(:new).and_return(notify_service)
-    end
+    shared_examples 'process alert payload' do |notify_service_class|
+      let(:service_response) { ServiceResponse.success }
+      let(:notify_service) { instance_double(notify_service_class, execute: service_response) }
 
-    def make_request(body = {})
-      post :create, params: project_params, body: body.to_json, as: :json
-    end
+      before do
+        allow(notify_service_class).to receive(:new).and_return(notify_service)
+      end
 
-    context 'when notification service succeeds' do
-      let(:payload) do
-        {
-          title: 'Alert title',
-          hosts: 'https://gitlab.com'
-        }
+      def make_request
+        post :create, params: project_params, body: payload.to_json, as: :json
       end
 
-      let(:permitted_params) { ActionController::Parameters.new(payload).permit! }
+      context 'when notification service succeeds' do
+        let(:permitted_params) { ActionController::Parameters.new(payload).permit! }
 
-      it 'responds with ok' do
-        make_request
+        it 'responds with ok' do
+          make_request
 
-        expect(response).to have_gitlab_http_status(:ok)
-      end
+          expect(response).to have_gitlab_http_status(:ok)
+        end
 
-      it 'does not pass excluded parameters to the notify service' do
-        make_request(payload)
+        it 'does not pass excluded parameters to the notify service' do
+          make_request
 
-        expect(Projects::Alerting::NotifyService)
-          .to have_received(:new)
-          .with(project, nil, permitted_params)
+          expect(notify_service_class)
+            .to have_received(:new)
+            .with(project, nil, permitted_params)
+        end
       end
-    end
 
-    context 'when notification service fails' do
-      let(:service_response) { ServiceResponse.error(message: 'Unauthorized', http_status: :unauthorized) }
+      context 'when notification service fails' do
+        let(:service_response) { ServiceResponse.error(message: 'Unauthorized', http_status: :unauthorized) }
 
-      it 'responds with the service response' do
-        make_request
+        it 'responds with the service response' do
+          make_request
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+          expect(response).to have_gitlab_http_status(:unauthorized)
+        end
       end
-    end
 
-    context 'bearer token' do
-      context 'when set' do
-        it 'extracts bearer token' do
-          request.headers['HTTP_AUTHORIZATION'] = 'Bearer some token'
+      context 'bearer token' do
+        context 'when set' do
+          it 'extracts bearer token' do
+            request.headers['HTTP_AUTHORIZATION'] = 'Bearer some token'
 
-          expect(notify_service).to receive(:execute).with('some token')
+            expect(notify_service).to receive(:execute).with('some token')
 
-          make_request
-        end
+            make_request
+          end
 
-        it 'pass nil if cannot extract a non-bearer token' do
-          request.headers['HTTP_AUTHORIZATION'] = 'some token'
+          it 'pass nil if cannot extract a non-bearer token' do
+            request.headers['HTTP_AUTHORIZATION'] = 'some token'
 
-          expect(notify_service).to receive(:execute).with(nil)
+            expect(notify_service).to receive(:execute).with(nil)
 
-          make_request
+            make_request
+          end
         end
-      end
 
-      context 'when missing' do
-        it 'passes nil' do
-          expect(notify_service).to receive(:execute).with(nil)
+        context 'when missing' do
+          it 'passes nil' do
+            expect(notify_service).to receive(:execute).with(nil)
 
-          make_request
+            make_request
+          end
         end
       end
     end
+
+    context 'generic alert payload' do
+      it_behaves_like 'process alert payload', Projects::Alerting::NotifyService do
+        let(:payload) { { title: 'Alert title' } }
+      end
+    end
+
+    context 'Prometheus alert payload' do
+      include PrometheusHelpers
+
+      it_behaves_like 'process alert payload', Projects::Prometheus::Alerts::NotifyService do
+        let(:payload) { prometheus_alert_payload }
+      end
+    end
   end
 
+  private
+
   def project_params(opts = {})
     opts.reverse_merge(namespace_id: project.namespace, project_id: project)
   end

spec/factories/services.rb

@@ -45,9 +45,13 @@ FactoryBot.define do
   end
 
   factory :alerts_service do
+    active
     project
     type { 'AlertsService' }
-    active { true }
+
+    trait :active do
+      active { true }
+    end
 
     trait :inactive do
       active { false }

spec/services/projects/prometheus/alerts/notify_service_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Projects::Prometheus::Alerts::NotifyService do
+  include PrometheusHelpers
+
   let_it_be(:project, reload: true) { create(:project) }
 
   let(:service) { described_class.new(project, nil, payload) }
@@ -92,9 +94,10 @@ describe Projects::Prometheus::Alerts::NotifyService do
   end
 
   context 'with valid payload' do
-    let(:alert_firing) { create(:prometheus_alert, project: project) }
-    let(:alert_resolved) { create(:prometheus_alert, project: project) }
-    let(:payload_raw) { payload_for(firing: [alert_firing], resolved: [alert_resolved]) }
+    let_it_be(:alert_firing) { create(:prometheus_alert, project: project) }
+    let_it_be(:alert_resolved) { create(:prometheus_alert, project: project) }
+    let_it_be(:cluster) { create(:cluster, :provided_by_user, projects: [project]) }
+    let(:payload_raw) { prometheus_alert_payload(firing: [alert_firing], resolved: [alert_resolved]) }
     let(:payload) { ActionController::Parameters.new(payload_raw).permit! }
     let(:payload_alert_firing) { payload_raw['alerts'].first }
     let(:token) { 'token' }
@@ -116,9 +119,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
 
       with_them do
         before do
-          cluster = create(:cluster, :provided_by_user,
-                           projects: [project],
-                           enabled: cluster_enabled)
+          cluster.update!(enabled: cluster_enabled)
 
           if status
             create(:clusters_applications_prometheus, status,
@@ -179,6 +180,39 @@ describe Projects::Prometheus::Alerts::NotifyService do
       end
     end
 
+    context 'with generic alerts integration' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:alerts_service, :token, :result) do
+        :active   | :valid    | :success
+        :active   | :invalid  | :failure
+        :active   | nil       | :failure
+        :inactive | :valid    | :failure
+        nil       | nil       | :failure
+      end
+
+      with_them do
+        let(:valid) { project.alerts_service.token }
+        let(:invalid) { 'invalid token' }
+        let(:token_input) { public_send(token) if token }
+
+        before do
+          if alerts_service
+            create(:alerts_service, alerts_service, project: project)
+          end
+        end
+
+        case result = params[:result]
+        when :success
+          it_behaves_like 'notifies alerts'
+        when :failure
+          it_behaves_like 'no notifications', http_status: :unauthorized
+        else
+          raise "invalid result: #{result.inspect}"
+        end
+      end
+    end
+
     context 'alert emails' do
       before do
         create(:prometheus_service, project: project)
@@ -227,7 +261,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
 
       context 'with multiple firing alerts and resolving alerts' do
         let(:payload_raw) do
-          payload_for(firing: [alert_firing, alert_firing], resolved: [alert_resolved])
+          prometheus_alert_payload(firing: [alert_firing, alert_firing], resolved: [alert_resolved])
         end
 
         it 'processes Prometheus alerts' do
@@ -258,7 +292,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
 
         context 'multiple firing alerts' do
           let(:payload_raw) do
-            payload_for(firing: [alert_firing, alert_firing], resolved: [])
+            prometheus_alert_payload(firing: [alert_firing, alert_firing], resolved: [])
           end
 
           it_behaves_like 'processes incident issues', 2
@@ -266,7 +300,7 @@ describe Projects::Prometheus::Alerts::NotifyService do
 
         context 'without firing alerts' do
           let(:payload_raw) do
-            payload_for(firing: [], resolved: [alert_resolved])
+            prometheus_alert_payload(firing: [], resolved: [alert_resolved])
           end
 
           it_behaves_like 'processes incident issues', 1
@@ -284,24 +318,17 @@ describe Projects::Prometheus::Alerts::NotifyService do
   end
 
   context 'with invalid payload' do
-    context 'without version' do
+    context 'when payload is not processable' do
       let(:payload) { {} }
 
-      it_behaves_like 'no notifications', http_status: :unprocessable_entity
-    end
-
-    context 'when version is not "4"' do
-      let(:payload) { { 'version' => '5' } }
+      before do
+        allow(described_class).to receive(:processable?).with(payload)
+          .and_return(false)
+      end
 
       it_behaves_like 'no notifications', http_status: :unprocessable_entity
     end
 
-    context 'with missing alerts' do
-      let(:payload) { { 'version' => '4' } }
-
-      it_behaves_like 'no notifications', http_status: :unauthorized
-    end
-
     context 'when the payload is too big' do
       let(:payload) { { 'the-payload-is-too-big' => true } }
       let(:deep_size_object) { instance_double(Gitlab::Utils::DeepSize, valid?: false) }
@@ -328,50 +355,39 @@ describe Projects::Prometheus::Alerts::NotifyService do
     end
   end
 
-  private
-
-  def payload_for(firing: [], resolved: [])
-    status = firing.any? ? 'firing' : 'resolved'
-    alerts = firing + resolved
-    alert_name = alerts.first.title
-    prometheus_metric_id = alerts.first.prometheus_metric_id.to_s
-
-    alerts_map = \
-      firing.map { |alert| map_alert_payload('firing', alert) } +
-      resolved.map { |alert| map_alert_payload('resolved', alert) }
-
-    # See https://prometheus.io/docs/alerting/configuration/#%3Cwebhook_config%3E
-    {
-      'version' => '4',
-      'receiver' => 'gitlab',
-      'status' => status,
-      'alerts' => alerts_map,
-      'groupLabels' => {
-        'alertname' => alert_name
-      },
-      'commonLabels' => {
-        'alertname' => alert_name,
-        'gitlab' => 'hook',
-        'gitlab_alert_id' => prometheus_metric_id
-      },
-      'commonAnnotations' => {},
-      'externalURL' => '',
-      'groupKey' => "{}:{alertname=\'#{alert_name}\'}"
-    }
-  end
+  describe '.processable?' do
+    let(:valid_payload) { prometheus_alert_payload }
+
+    subject { described_class.processable?(payload) }
+
+    context 'with valid payload' do
+      let(:payload) { valid_payload }
+
+      it { is_expected.to eq(true) }
+
+      context 'containing unrelated keys' do
+        let(:payload) { valid_payload.merge('unrelated' => 'key') }
 
-  def map_alert_payload(status, alert)
-    {
-      'status' => status,
-      'labels' => {
-        'alertname' => alert.title,
-        'gitlab' => 'hook',
-        'gitlab_alert_id' => alert.prometheus_metric_id.to_s
-      },
-      'annotations' => {},
-      'startsAt' => '2018-09-24T08:57:31.095725221Z',
-      'endsAt' => '0001-01-01T00:00:00Z',
-      'generatorURL' => 'http://prometheus-prometheus-server-URL'
-    }
+        it { is_expected.to eq(true) }
+      end
+    end
+
+    context 'with invalid payload' do
+      where(:missing_key) do
+        described_class::REQUIRED_PAYLOAD_KEYS.to_a
+      end
+
+      with_them do
+        let(:payload) { valid_payload.except(missing_key) }
+
+        it { is_expected.to eq(false) }
+      end
+    end
+
+    context 'with unsupported version' do
+      let(:payload) { valid_payload.merge('version' => '5') }
+
+      it { is_expected.to eq(false) }
+    end
   end
 end

spec/support/helpers/prometheus_helpers.rb

@@ -236,4 +236,51 @@ module PrometheusHelpers
       ]
     }
   end
+
+  def prometheus_alert_payload(firing: [], resolved: [])
+    status = firing.any? ? 'firing' : 'resolved'
+    alerts = firing + resolved
+    alert_name = alerts.first&.title || ''
+    prometheus_metric_id = alerts.first&.prometheus_metric_id&.to_s
+
+    alerts_map = \
+      firing.map { |alert| prometheus_map_alert_payload('firing', alert) } +
+      resolved.map { |alert| prometheus_map_alert_payload('resolved', alert) }
+
+    # See https://prometheus.io/docs/alerting/configuration/#%3Cwebhook_config%3E
+    {
+      'version' => '4',
+      'receiver' => 'gitlab',
+      'status' => status,
+      'alerts' => alerts_map,
+      'groupLabels' => {
+        'alertname' => alert_name
+      },
+      'commonLabels' => {
+        'alertname' => alert_name,
+        'gitlab' => 'hook',
+        'gitlab_alert_id' => prometheus_metric_id
+      },
+      'commonAnnotations' => {},
+      'externalURL' => '',
+      'groupKey' => "{}:{alertname=\'#{alert_name}\'}"
+    }
+  end
+
+  private
+
+  def prometheus_map_alert_payload(status, alert)
+    {
+      'status' => status,
+      'labels' => {
+        'alertname' => alert.title,
+        'gitlab' => 'hook',
+        'gitlab_alert_id' => alert.prometheus_metric_id.to_s
+      },
+      'annotations' => {},
+      'startsAt' => '2018-09-24T08:57:31.095725221Z',
+      'endsAt' => '0001-01-01T00:00:00Z',
+      'generatorURL' => 'http://prometheus-prometheus-server-URL'
+    }
+  end
 end