Merge branch 'sy-enable-http-endpoints' into 'master'

Enable endpoints corresponding to AlertManagement::HttpIntegrations

See merge request gitlab-org/gitlab!44485

app/controllers/projects/alerting/notifications_controller.rb

@@ -14,7 +14,7 @@ module Projects
 
       def create
         token = extract_alert_manager_token(request)
-        result = notify_service.execute(token)
+        result = notify_service.execute(token, integration)
 
         head result.http_status
       end
@@ -45,6 +45,20 @@ module Projects
         end
       end
 
+      def integration
+        return unless Feature.enabled?(:multiple_http_integrations, project)
+
+        AlertManagement::HttpIntegrationsFinder.new(
+          project,
+          endpoint_identifier: endpoint_identifier,
+          active: true
+        ).execute.first
+      end
+
+      def endpoint_identifier
+        params[:endpoint_identifier] || AlertManagement::HttpIntegration::LEGACY_IDENTIFIER
+      end
+
       def notification_payload
         @notification_payload ||= params.permit![:notification]
       end

app/finders/alert_management/http_integrations_finder.rb

+# frozen_string_literal: true
+
+module AlertManagement
+  class HttpIntegrationsFinder
+    def initialize(project, params)
+      @project = project
+      @params = params
+    end
+
+    def execute
+      @collection = project.alert_management_http_integrations
+
+      filter_by_availability
+      filter_by_endpoint_identifier
+      filter_by_active
+
+      collection
+    end
+
+    private
+
+    attr_reader :project, :params, :collection
+
+    def filter_by_availability
+      return if multiple_alert_http_integrations?
+
+      first_id = project.alert_management_http_integrations
+                        .ordered_by_id
+                        .select(:id)
+                        .at_most(1)
+
+      @collection = collection.id_in(first_id)
+    end
+
+    def filter_by_endpoint_identifier
+      return unless params[:endpoint_identifier]
+
+      @collection = collection.for_endpoint_identifier(params[:endpoint_identifier])
+    end
+
+    def filter_by_active
+      return unless params[:active]
+
+      @collection = collection.active
+    end
+
+    # Overridden in EE
+    def multiple_alert_http_integrations?
+      false
+    end
+  end
+end
+
+::AlertManagement::HttpIntegrationsFinder.prepend_if_ee('EE::AlertManagement::HttpIntegrationsFinder')

app/models/alert_management/http_integration.rb

@@ -2,6 +2,9 @@
 
 module AlertManagement
   class HttpIntegration < ApplicationRecord
+    LEGACY_IDENTIFIER = 'legacy'
+    DEFAULT_NAME_SLUG = 'http-endpoint'
+
     belongs_to :project, inverse_of: :alert_management_http_integrations
 
     attr_encrypted :token,
@@ -9,19 +12,45 @@ module AlertManagement
       key: Settings.attr_encrypted_db_key_base_truncated,
       algorithm: 'aes-256-gcm'
 
+    default_value_for(:endpoint_identifier, allows_nil: false) { SecureRandom.hex(8) }
+    default_value_for(:token) { generate_token }
+
     validates :project, presence: true
     validates :active, inclusion: { in: [true, false] }
-
-    validates :token, presence: true
+    validates :token, presence: true, format: { with: /\A\h{32}\z/ }
     validates :name, presence: true, length: { maximum: 255 }
-    validates :endpoint_identifier, presence: true, length: { maximum: 255 }
+    validates :endpoint_identifier, presence: true, length: { maximum: 255 }, format: { with: /\A[A-Za-z0-9]+\z/ }
     validates :endpoint_identifier, uniqueness: { scope: [:project_id, :active] }, if: :active?
 
     before_validation :prevent_token_assignment
+    before_validation :prevent_endpoint_identifier_assignment
     before_validation :ensure_token
 
+    scope :for_endpoint_identifier, -> (endpoint_identifier) { where(endpoint_identifier: endpoint_identifier) }
+    scope :active, -> { where(active: true) }
+    scope :ordered_by_id, -> { order(:id) }
+
+    def url
+      return ::Gitlab::Routing.url_helpers.project_alerts_notify_url(project, format: :json) if legacy?
+
+      ::Gitlab::Routing.url_helpers.project_alert_http_integration_url(project, name_slug, endpoint_identifier, format: :json)
+    end
+
     private
 
+    def self.generate_token
+      SecureRandom.hex
+    end
+
+    def name_slug
+      (name && Gitlab::Utils.slugify(name)) || DEFAULT_NAME_SLUG
+    end
+
+    def legacy?
+      endpoint_identifier == LEGACY_IDENTIFIER
+    end
+
+    # Blank token assignment triggers token reset
     def prevent_token_assignment
       if token.present? && token_changed?
         self.token = nil
@@ -31,11 +60,13 @@ module AlertManagement
     end
 
     def ensure_token
-      self.token = generate_token if token.blank?
+      self.token = self.class.generate_token if token.blank?
     end
 
-    def generate_token
-      SecureRandom.hex
+    def prevent_endpoint_identifier_assignment
+      if endpoint_identifier_changed? && endpoint_identifier_was.present?
+        self.endpoint_identifier = endpoint_identifier_was
+      end
     end
   end
 end

app/services/projects/alerting/notify_service.rb

@@ -6,9 +6,11 @@ module Projects
       include Gitlab::Utils::StrongMemoize
       include ::IncidentManagement::Settings
 
-      def execute(token)
+      def execute(token, integration = nil)
+        @integration = integration
+
         return bad_request unless valid_payload_size?
-        return forbidden unless alerts_service_activated?
+        return forbidden unless active_integration?
         return unauthorized unless valid_token?(token)
 
         process_alert
@@ -22,6 +24,7 @@ module Projects
 
       private
 
+      attr_reader :integration
       delegate :alerts_service, :alerts_service_activated?, to: :project
 
       def process_alert
@@ -66,10 +69,7 @@ module Projects
         return unless alert.save
 
         alert.execute_services
-        SystemNoteService.create_new_alert(
-          alert,
-          alert.monitoring_tool || 'Generic Alert Endpoint'
-        )
+        SystemNoteService.create_new_alert(alert, notification_source)
       end
 
       def process_incident_issues
@@ -106,11 +106,27 @@ module Projects
         end
       end
 
+      def notification_source
+        alert.monitoring_tool || integration&.name || 'Generic Alert Endpoint'
+      end
+
       def valid_payload_size?
         Gitlab::Utils::DeepSize.new(params).valid?
       end
 
+      def active_integration?
+        if Feature.enabled?(:multiple_http_integrations, project)
+          return true if integration
+        end
+
+        alerts_service_activated?
+      end
+
       def valid_token?(token)
+        if Feature.enabled?(:multiple_http_integrations, project)
+          return token == integration.token if integration
+        end
+
         token == alerts_service.token
       end
 

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

@@ -17,7 +17,7 @@ module Projects
 
         SUPPORTED_VERSION = '4'
 
-        def execute(token)
+        def execute(token, _integration = nil)
           return bad_request unless valid_payload_size?
           return unprocessable_entity unless self.class.processable?(params)
           return unauthorized unless valid_alert_manager_token?(token)

config/feature_flags/development/multiple_http_integrations.yml

+---
+name: multiple_http_integrations
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44485
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/255509
+type: development
+group: group::health
+default_enabled: false

config/routes/project.rb

@@ -436,6 +436,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       end
 
       post 'alerts/notify', to: 'alerting/notifications#create' # rubocop:todo Cop/PutProjectRoutesUnderScope
+      post 'alerts/notify/:name/:endpoint_identifier', # rubocop:todo Cop/PutProjectRoutesUnderScope
+            to: 'alerting/notifications#create',
+            as: :alert_http_integration,
+            constraints: { endpoint_identifier: /[A-Za-z0-9]+/ }
 
       draw :legacy_builds
 

ee/app/finders/ee/alert_management/http_integrations_finder.rb

+# frozen_string_literal: true
+
+module EE
+  module AlertManagement
+    module HttpIntegrationsFinder
+      extend ::Gitlab::Utils::Override
+
+      private
+
+      override :multiple_alert_http_integrations?
+      def multiple_alert_http_integrations?
+        project.feature_available?(:multiple_alert_http_integrations)
+      end
+    end
+  end
+end

ee/app/models/license.rb

@@ -98,6 +98,7 @@ class License < ApplicationRecord
     admin_merge_request_approvers_rules
     merge_trains
     metrics_reports
+    multiple_alert_http_integrations
     multiple_approval_rules
     multiple_group_issue_boards
     object_storage

ee/spec/finders/ee/alert_management/http_integrations_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AlertManagement::HttpIntegrationsFinder do
+  describe '#execute' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:active_integration) { create(:alert_management_http_integration, project: project, endpoint_identifier: 'abc123' ) }
+    let_it_be(:inactive_integration) { create(:alert_management_http_integration, :inactive, project: project, endpoint_identifier: 'abc123' ) }
+    let_it_be(:alt_identifier_integration) { create(:alert_management_http_integration, project: project) }
+    let_it_be(:alt_project_integration) { create(:alert_management_http_integration) }
+
+    before do
+      stub_licensed_features(multiple_alert_http_integrations: true)
+    end
+
+    let(:params) { {} }
+
+    subject(:execute) { described_class.new(project, params).execute }
+
+    context 'empty params' do
+      it { is_expected.to contain_exactly(active_integration, inactive_integration, alt_identifier_integration) }
+    end
+
+    context 'endpoint_identifier given' do
+      let(:params) { { endpoint_identifier: active_integration.endpoint_identifier } }
+
+      it { is_expected.to contain_exactly(active_integration, inactive_integration) }
+
+      context 'but unknown' do
+        let(:params) { { endpoint_identifier: 'unknown' } }
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'but blank' do
+        let(:params) { { endpoint_identifier: nil } }
+
+        it { is_expected.to contain_exactly(active_integration, inactive_integration, alt_identifier_integration) }
+      end
+    end
+
+    context 'active param given' do
+      let(:params) { { active: true } }
+
+      it { is_expected.to contain_exactly(active_integration, alt_identifier_integration) }
+
+      context 'but blank' do
+        let(:params) { { active: nil } }
+
+        it { is_expected.to contain_exactly(active_integration, inactive_integration, alt_identifier_integration) }
+      end
+    end
+  end
+end

ee/spec/requests/rack_attack_spec.rb

@@ -107,4 +107,16 @@ RSpec.describe 'Rack Attack EE throttles' do
       let(:path) { "/#{project.full_path}/alerts/notify" }
     end
   end
+
+  describe 'requests to AlertManagement::HttpIntegration notify endpoint with oauth token' do
+    before do
+      allow_next_instance_of(Projects::Alerting::NotifyService) do |instance|
+        allow(instance).to receive(:execute).and_return(ServiceResponse.success)
+      end
+    end
+
+    it_behaves_like 'incident management rate limiting' do
+      let(:path) { "/#{project.full_path}/alerts/notify/http-integration-name/eddd36969b2d3d6a" }
+    end
+  end
 end

spec/controllers/projects/alerting/notifications_controller_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 RSpec.describe Projects::Alerting::NotificationsController do
   let_it_be(:project) { create(:project) }
   let_it_be(:environment) { create(:environment, project: project) }
+  let(:params) { project_params }
 
   describe 'POST #create' do
     around do |example|
@@ -20,7 +21,7 @@ RSpec.describe Projects::Alerting::NotificationsController do
       end
 
       def make_request
-        post :create, params: project_params, body: payload.to_json, as: :json
+        post :create, params: params, body: payload.to_json, as: :json
       end
 
       context 'when notification service succeeds' do
@@ -53,26 +54,81 @@ RSpec.describe Projects::Alerting::NotificationsController do
 
       context 'bearer token' do
         context 'when set' do
-          it 'extracts bearer token' do
-            request.headers['HTTP_AUTHORIZATION'] = 'Bearer some token'
+          context 'when extractable' do
+            before do
+              request.headers['HTTP_AUTHORIZATION'] = 'Bearer some token'
+            end
 
-            expect(notify_service).to receive(:execute).with('some token')
+            it 'extracts bearer token' do
+              expect(notify_service).to receive(:execute).with('some token', nil)
 
-            make_request
+              make_request
+            end
+
+            context 'with a corresponding integration' do
+              context 'with integration parameters specified' do
+                let_it_be_with_reload(:integration) { create(:alert_management_http_integration, project: project) }
+                let(:params) { project_params(endpoint_identifier: integration.endpoint_identifier, name: integration.name) }
+
+                context 'the integration is active' do
+                  it 'extracts and finds the integration' do
+                    expect(notify_service).to receive(:execute).with('some token', integration)
+
+                    make_request
+                  end
+                end
+
+                context 'when the integration is inactive' do
+                  before do
+                    integration.update!(active: false)
+                  end
+
+                  it 'does not find an integration' do
+                    expect(notify_service).to receive(:execute).with('some token', nil)
+
+                    make_request
+                  end
+                end
+
+                context 'when multiple endpoints are disabled' do
+                  before do
+                    stub_feature_flags(multiple_http_integrations: false)
+                  end
+
+                  it 'does not find an integration' do
+                    expect(notify_service).to receive(:execute).with('some token', nil)
+
+                    make_request
+                  end
+                end
+              end
+
+              context 'without integration parameters specified' do
+                let_it_be(:integration) { create(:alert_management_http_integration, :legacy, project: project) }
+
+                it 'extracts and finds the legacy integration' do
+                  expect(notify_service).to receive(:execute).with('some token', integration)
+
+                  make_request
+                end
+              end
+            end
           end
 
-          it 'pass nil if cannot extract a non-bearer token' do
-            request.headers['HTTP_AUTHORIZATION'] = 'some token'
+          context 'when inextractable' do
+            it 'passes nil for 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, nil)
 
-            make_request
+              make_request
+            end
           end
         end
 
         context 'when missing' do
           it 'passes nil' do
-            expect(notify_service).to receive(:execute).with(nil)
+            expect(notify_service).to receive(:execute).with(nil, nil)
 
             make_request
           end

spec/factories/alert_management/http_integrations.rb

@@ -10,5 +10,11 @@ FactoryBot.define do
     trait :inactive do
       active { false }
     end
+
+    trait :legacy do
+      endpoint_identifier { 'legacy' }
+    end
+
+    initialize_with { new(**attributes) }
   end
 end

spec/finders/alert_management/http_integrations_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AlertManagement::HttpIntegrationsFinder do
+  let_it_be(:project) { create(:project) }
+  let_it_be_with_reload(:integration) { create(:alert_management_http_integration, project: project ) }
+  let_it_be(:extra_integration) { create(:alert_management_http_integration, project: project ) }
+  let_it_be(:alt_project_integration) { create(:alert_management_http_integration) }
+
+  let(:params) { {} }
+
+  describe '#execute' do
+    subject(:execute) { described_class.new(project, params).execute }
+
+    context 'empty params' do
+      it { is_expected.to contain_exactly(integration) }
+    end
+
+    context 'endpoint_identifier param given' do
+      let(:params) { { endpoint_identifier: integration.endpoint_identifier } }
+
+      it { is_expected.to contain_exactly(integration) }
+
+      context 'matches an unavailable integration' do
+        let(:params) { { endpoint_identifier: extra_integration.endpoint_identifier } }
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'but unknown' do
+        let(:params) { { endpoint_identifier: 'unknown' } }
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'but blank' do
+        let(:params) { { endpoint_identifier: nil } }
+
+        it { is_expected.to contain_exactly(integration) }
+      end
+    end
+
+    context 'active param given' do
+      let(:params) { { active: true } }
+
+      it { is_expected.to contain_exactly(integration) }
+
+      context 'when integration is disabled' do
+        before do
+          integration.update!(active: false)
+        end
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'but blank' do
+        let(:params) { { active: nil } }
+
+        it { is_expected.to contain_exactly(integration) }
+      end
+    end
+
+    context 'project has no integrations' do
+      subject(:execute) { described_class.new(create(:project), params).execute }
+
+      it { is_expected.to be_empty }
+    end
+  end
+end

spec/models/alert_management/http_integration_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe AlertManagement::HttpIntegration do
+  include ::Gitlab::Routing.url_helpers
+
   let_it_be(:project) { create(:project) }
 
   subject(:integration) { build(:alert_management_http_integration) }
@@ -15,19 +17,17 @@ RSpec.describe AlertManagement::HttpIntegration do
     it { is_expected.to validate_presence_of(:project) }
     it { is_expected.to validate_presence_of(:name) }
     it { is_expected.to validate_length_of(:name).is_at_most(255) }
-    it { is_expected.to validate_presence_of(:endpoint_identifier) }
-    it { is_expected.to validate_length_of(:endpoint_identifier).is_at_most(255) }
 
     context 'when active' do
       # Using `create` instead of `build` the integration so `token` is set.
       # Uniqueness spec saves integration with `validate: false` otherwise.
-      subject { create(:alert_management_http_integration) }
+      subject { create(:alert_management_http_integration, :legacy) }
 
       it { is_expected.to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id, :active) }
     end
 
     context 'when inactive' do
-      subject { create(:alert_management_http_integration, :inactive) }
+      subject { create(:alert_management_http_integration, :legacy, :inactive) }
 
       it { is_expected.not_to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id, :active) }
     end
@@ -51,10 +51,6 @@ RSpec.describe AlertManagement::HttpIntegration do
 
     context 'when unsaved' do
       context 'when unassigned' do
-        before do
-          integration.valid?
-        end
-
         it_behaves_like 'valid token'
       end
 
@@ -89,4 +85,75 @@ RSpec.describe AlertManagement::HttpIntegration do
       end
     end
   end
+
+  describe '#endpoint_identifier' do
+    subject { integration.endpoint_identifier }
+
+    context 'when defined on initialize' do
+      let(:integration) { described_class.new }
+
+      it { is_expected.to match(/\A\h{16}\z/) }
+    end
+
+    context 'when included in initialization args' do
+      let(:integration) { described_class.new(endpoint_identifier: 'legacy') }
+
+      it { is_expected.to eq('legacy') }
+    end
+
+    context 'when reassigning' do
+      let(:integration) { create(:alert_management_http_integration) }
+      let!(:starting_identifier) { subject }
+
+      it 'does not allow reassignment' do
+        integration.endpoint_identifier = 'newValidId'
+        integration.save!
+
+        expect(integration.reload.endpoint_identifier).to eq(starting_identifier)
+      end
+    end
+  end
+
+  describe '#url' do
+    subject { integration.url }
+
+    it do
+      is_expected.to eq(
+        project_alert_http_integration_url(
+          integration.project,
+          'datadog',
+          integration.endpoint_identifier,
+          format: :json
+        )
+      )
+    end
+
+    context 'when name is not defined' do
+      let(:integration) { described_class.new(project: project) }
+
+      it do
+        is_expected.to eq(
+          project_alert_http_integration_url(
+            integration.project,
+            'http-endpoint',
+            integration.endpoint_identifier,
+            format: :json
+          )
+        )
+      end
+    end
+
+    context 'for a legacy integration' do
+      let(:integration) { build(:alert_management_http_integration, :legacy) }
+
+      it do
+        is_expected.to eq(
+          project_alerts_notify_url(
+            integration.project,
+            format: :json
+          )
+        )
+      end
+    end
+  end
 end

spec/services/projects/alerting/notify_service_spec.rb

@@ -34,13 +34,11 @@ RSpec.describe Projects::Alerting::NotifyService do
 
     let(:payload) { ActionController::Parameters.new(payload_raw).permit! }
 
-    subject { service.execute(token) }
-
-    context 'with activated Alerts Service' do
-      let_it_be_with_reload(:alerts_service) { create(:alerts_service, project: project) }
+    subject { service.execute(token, nil) }
 
+    shared_examples 'notifcations are handled correctly' do
       context 'with valid token' do
-        let(:token) { alerts_service.token }
+        let(:token) { integration.token }
         let(:incident_management_setting) { double(send_email?: email_enabled, create_issue?: issue_enabled, auto_close_incident?: auto_close_enabled) }
         let(:email_enabled) { false }
         let(:issue_enabled) { false }
@@ -197,7 +195,7 @@ RSpec.describe Projects::Alerting::NotifyService do
 
             it 'creates a system note corresponding to alert creation' do
               expect { subject }.to change(Note, :count).by(1)
-              expect(Note.last.note).to include('Generic Alert Endpoint')
+              expect(Note.last.note).to include(source)
             end
           end
         end
@@ -247,15 +245,42 @@ RSpec.describe Projects::Alerting::NotifyService do
         it_behaves_like 'does not process incident issues due to error', http_status: :unauthorized
         it_behaves_like 'does not an create alert management alert'
       end
+    end
+
+    context 'with an Alerts Service' do
+      let_it_be_with_reload(:integration) { create(:alerts_service, project: project) }
+
+      it_behaves_like 'notifcations are handled correctly' do
+        let(:source) { 'Generic Alert Endpoint' }
+      end
 
       context 'with deactivated Alerts Service' do
         before do
-          alerts_service.update!(active: false)
+          integration.update!(active: false)
         end
 
         it_behaves_like 'does not process incident issues due to error', http_status: :forbidden
         it_behaves_like 'does not an create alert management alert'
       end
     end
+
+    context 'with an HTTP Integration' do
+      let_it_be_with_reload(:integration) { create(:alert_management_http_integration, project: project) }
+
+      subject { service.execute(token, integration) }
+
+      it_behaves_like 'notifcations are handled correctly' do
+        let(:source) { integration.name }
+      end
+
+      context 'with deactivated HTTP Integration' do
+        before do
+          integration.update!(active: false)
+        end
+
+        it_behaves_like 'does not process incident issues due to error', http_status: :unauthorized
+        it_behaves_like 'does not an create alert management alert'
+      end
+    end
   end
 end