Commit 9c682c3c authored by Sarah Yasonik's avatar Sarah Yasonik Committed by Tiger Watson

Enable endpoints corresponding to AlertManagement::HttpIntegrations

This adds the routes for new alert HTTP integrations, including
support for endpoint throttling and alert creation.
parent cbfc5023
...@@ -14,7 +14,7 @@ module Projects ...@@ -14,7 +14,7 @@ module Projects
def create def create
token = extract_alert_manager_token(request) token = extract_alert_manager_token(request)
result = notify_service.execute(token) result = notify_service.execute(token, integration)
head result.http_status head result.http_status
end end
...@@ -45,6 +45,20 @@ module Projects ...@@ -45,6 +45,20 @@ module Projects
end end
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 def notification_payload
@notification_payload ||= params.permit![:notification] @notification_payload ||= params.permit![:notification]
end end
......
# 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')
...@@ -2,6 +2,9 @@ ...@@ -2,6 +2,9 @@
module AlertManagement module AlertManagement
class HttpIntegration < ApplicationRecord class HttpIntegration < ApplicationRecord
LEGACY_IDENTIFIER = 'legacy'
DEFAULT_NAME_SLUG = 'http-endpoint'
belongs_to :project, inverse_of: :alert_management_http_integrations belongs_to :project, inverse_of: :alert_management_http_integrations
attr_encrypted :token, attr_encrypted :token,
...@@ -9,19 +12,45 @@ module AlertManagement ...@@ -9,19 +12,45 @@ module AlertManagement
key: Settings.attr_encrypted_db_key_base_truncated, key: Settings.attr_encrypted_db_key_base_truncated,
algorithm: 'aes-256-gcm' 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 :project, presence: true
validates :active, inclusion: { in: [true, false] } validates :active, inclusion: { in: [true, false] }
validates :token, presence: true, format: { with: /\A\h{32}\z/ }
validates :token, presence: true
validates :name, presence: true, length: { maximum: 255 } 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? validates :endpoint_identifier, uniqueness: { scope: [:project_id, :active] }, if: :active?
before_validation :prevent_token_assignment before_validation :prevent_token_assignment
before_validation :prevent_endpoint_identifier_assignment
before_validation :ensure_token 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 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 def prevent_token_assignment
if token.present? && token_changed? if token.present? && token_changed?
self.token = nil self.token = nil
...@@ -31,11 +60,13 @@ module AlertManagement ...@@ -31,11 +60,13 @@ module AlertManagement
end end
def ensure_token def ensure_token
self.token = generate_token if token.blank? self.token = self.class.generate_token if token.blank?
end end
def generate_token def prevent_endpoint_identifier_assignment
SecureRandom.hex if endpoint_identifier_changed? && endpoint_identifier_was.present?
self.endpoint_identifier = endpoint_identifier_was
end
end end
end end
end end
...@@ -6,9 +6,11 @@ module Projects ...@@ -6,9 +6,11 @@ module Projects
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
include ::IncidentManagement::Settings include ::IncidentManagement::Settings
def execute(token) def execute(token, integration = nil)
@integration = integration
return bad_request unless valid_payload_size? return bad_request unless valid_payload_size?
return forbidden unless alerts_service_activated? return forbidden unless active_integration?
return unauthorized unless valid_token?(token) return unauthorized unless valid_token?(token)
process_alert process_alert
...@@ -22,6 +24,7 @@ module Projects ...@@ -22,6 +24,7 @@ module Projects
private private
attr_reader :integration
delegate :alerts_service, :alerts_service_activated?, to: :project delegate :alerts_service, :alerts_service_activated?, to: :project
def process_alert def process_alert
...@@ -66,10 +69,7 @@ module Projects ...@@ -66,10 +69,7 @@ module Projects
return unless alert.save return unless alert.save
alert.execute_services alert.execute_services
SystemNoteService.create_new_alert( SystemNoteService.create_new_alert(alert, notification_source)
alert,
alert.monitoring_tool || 'Generic Alert Endpoint'
)
end end
def process_incident_issues def process_incident_issues
...@@ -106,11 +106,27 @@ module Projects ...@@ -106,11 +106,27 @@ module Projects
end end
end end
def notification_source
alert.monitoring_tool || integration&.name || 'Generic Alert Endpoint'
end
def valid_payload_size? def valid_payload_size?
Gitlab::Utils::DeepSize.new(params).valid? Gitlab::Utils::DeepSize.new(params).valid?
end end
def active_integration?
if Feature.enabled?(:multiple_http_integrations, project)
return true if integration
end
alerts_service_activated?
end
def valid_token?(token) def valid_token?(token)
if Feature.enabled?(:multiple_http_integrations, project)
return token == integration.token if integration
end
token == alerts_service.token token == alerts_service.token
end end
......
...@@ -17,7 +17,7 @@ module Projects ...@@ -17,7 +17,7 @@ module Projects
SUPPORTED_VERSION = '4' SUPPORTED_VERSION = '4'
def execute(token) def execute(token, _integration = nil)
return bad_request unless valid_payload_size? return bad_request unless valid_payload_size?
return unprocessable_entity unless self.class.processable?(params) return unprocessable_entity unless self.class.processable?(params)
return unauthorized unless valid_alert_manager_token?(token) return unauthorized unless valid_alert_manager_token?(token)
......
---
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
...@@ -436,6 +436,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -436,6 +436,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end end
post 'alerts/notify', to: 'alerting/notifications#create' # rubocop:todo Cop/PutProjectRoutesUnderScope 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 draw :legacy_builds
......
# 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
...@@ -98,6 +98,7 @@ class License < ApplicationRecord ...@@ -98,6 +98,7 @@ class License < ApplicationRecord
admin_merge_request_approvers_rules admin_merge_request_approvers_rules
merge_trains merge_trains
metrics_reports metrics_reports
multiple_alert_http_integrations
multiple_approval_rules multiple_approval_rules
multiple_group_issue_boards multiple_group_issue_boards
object_storage object_storage
......
# 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
...@@ -107,4 +107,16 @@ RSpec.describe 'Rack Attack EE throttles' do ...@@ -107,4 +107,16 @@ RSpec.describe 'Rack Attack EE throttles' do
let(:path) { "/#{project.full_path}/alerts/notify" } let(:path) { "/#{project.full_path}/alerts/notify" }
end end
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 end
...@@ -5,6 +5,7 @@ require 'spec_helper' ...@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe Projects::Alerting::NotificationsController do RSpec.describe Projects::Alerting::NotificationsController do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:environment) { create(:environment, project: project) } let_it_be(:environment) { create(:environment, project: project) }
let(:params) { project_params }
describe 'POST #create' do describe 'POST #create' do
around do |example| around do |example|
...@@ -20,7 +21,7 @@ RSpec.describe Projects::Alerting::NotificationsController do ...@@ -20,7 +21,7 @@ RSpec.describe Projects::Alerting::NotificationsController do
end end
def make_request 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 end
context 'when notification service succeeds' do context 'when notification service succeeds' do
...@@ -53,26 +54,81 @@ RSpec.describe Projects::Alerting::NotificationsController do ...@@ -53,26 +54,81 @@ RSpec.describe Projects::Alerting::NotificationsController do
context 'bearer token' do context 'bearer token' do
context 'when set' do context 'when set' do
it 'extracts bearer token' do context 'when extractable' do
before do
request.headers['HTTP_AUTHORIZATION'] = 'Bearer some token' request.headers['HTTP_AUTHORIZATION'] = 'Bearer some token'
end
it 'extracts bearer token' do
expect(notify_service).to receive(:execute).with('some token', nil)
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)
expect(notify_service).to receive(:execute).with('some token') 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 make_request
end end
end
end
end
it 'pass nil if cannot extract a non-bearer token' do context 'when inextractable' do
it 'passes nil for a non-bearer token' do
request.headers['HTTP_AUTHORIZATION'] = 'some token' 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 end
end
context 'when missing' do context 'when missing' do
it 'passes nil' do it 'passes nil' do
expect(notify_service).to receive(:execute).with(nil) expect(notify_service).to receive(:execute).with(nil, nil)
make_request make_request
end end
......
...@@ -10,5 +10,11 @@ FactoryBot.define do ...@@ -10,5 +10,11 @@ FactoryBot.define do
trait :inactive do trait :inactive do
active { false } active { false }
end end
trait :legacy do
endpoint_identifier { 'legacy' }
end
initialize_with { new(**attributes) }
end end
end end
# 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
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe AlertManagement::HttpIntegration do RSpec.describe AlertManagement::HttpIntegration do
include ::Gitlab::Routing.url_helpers
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
subject(:integration) { build(:alert_management_http_integration) } subject(:integration) { build(:alert_management_http_integration) }
...@@ -15,19 +17,17 @@ RSpec.describe AlertManagement::HttpIntegration do ...@@ -15,19 +17,17 @@ RSpec.describe AlertManagement::HttpIntegration do
it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:name) } 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_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 context 'when active' do
# Using `create` instead of `build` the integration so `token` is set. # Using `create` instead of `build` the integration so `token` is set.
# Uniqueness spec saves integration with `validate: false` otherwise. # 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) } it { is_expected.to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id, :active) }
end end
context 'when inactive' do 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) } it { is_expected.not_to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id, :active) }
end end
...@@ -51,10 +51,6 @@ RSpec.describe AlertManagement::HttpIntegration do ...@@ -51,10 +51,6 @@ RSpec.describe AlertManagement::HttpIntegration do
context 'when unsaved' do context 'when unsaved' do
context 'when unassigned' do context 'when unassigned' do
before do
integration.valid?
end
it_behaves_like 'valid token' it_behaves_like 'valid token'
end end
...@@ -89,4 +85,75 @@ RSpec.describe AlertManagement::HttpIntegration do ...@@ -89,4 +85,75 @@ RSpec.describe AlertManagement::HttpIntegration do
end end
end 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 end
...@@ -34,13 +34,11 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -34,13 +34,11 @@ RSpec.describe Projects::Alerting::NotifyService do
let(:payload) { ActionController::Parameters.new(payload_raw).permit! } let(:payload) { ActionController::Parameters.new(payload_raw).permit! }
subject { service.execute(token) } subject { service.execute(token, nil) }
context 'with activated Alerts Service' do
let_it_be_with_reload(:alerts_service) { create(:alerts_service, project: project) }
shared_examples 'notifcations are handled correctly' do
context 'with valid token' 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(:incident_management_setting) { double(send_email?: email_enabled, create_issue?: issue_enabled, auto_close_incident?: auto_close_enabled) }
let(:email_enabled) { false } let(:email_enabled) { false }
let(:issue_enabled) { false } let(:issue_enabled) { false }
...@@ -197,7 +195,7 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -197,7 +195,7 @@ RSpec.describe Projects::Alerting::NotifyService do
it 'creates a system note corresponding to alert creation' do it 'creates a system note corresponding to alert creation' do
expect { subject }.to change(Note, :count).by(1) 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 end
end end
...@@ -247,15 +245,42 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -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 process incident issues due to error', http_status: :unauthorized
it_behaves_like 'does not an create alert management alert' it_behaves_like 'does not an create alert management alert'
end 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 context 'with deactivated Alerts Service' do
before do before do
alerts_service.update!(active: false) integration.update!(active: false)
end end
it_behaves_like 'does not process incident issues due to error', http_status: :forbidden it_behaves_like 'does not process incident issues due to error', http_status: :forbidden
it_behaves_like 'does not an create alert management alert' it_behaves_like 'does not an create alert management alert'
end end
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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment