Commit dd40c7d8 authored by Vitali Tatarintev's avatar Vitali Tatarintev

Make NotifyService's user-less services

Prevent passing a `nil` user to  `Alerting::NotifyService` and
`Prometheus::Alerts::NotifyService` services
parent 86f27d94
...@@ -31,7 +31,7 @@ module Projects ...@@ -31,7 +31,7 @@ module Projects
end end
def notify_service def notify_service
notify_service_class.new(project, current_user, notification_payload) notify_service_class.new(project, notification_payload)
end end
def notify_service_class def notify_service_class
......
...@@ -73,7 +73,7 @@ module Projects ...@@ -73,7 +73,7 @@ module Projects
def notify_service def notify_service
Projects::Prometheus::Alerts::NotifyService Projects::Prometheus::Alerts::NotifyService
.new(project, current_user, params.permit!) .new(project, params.permit!)
end end
def create_service def create_service
......
...@@ -65,7 +65,7 @@ module Clusters ...@@ -65,7 +65,7 @@ module Clusters
notification_payload = build_notification_payload(project) notification_payload = build_notification_payload(project)
integration = project.alert_management_http_integrations.active.first integration = project.alert_management_http_integrations.active.first
Projects::Alerting::NotifyService.new(project, nil, notification_payload).execute(integration&.token, integration) Projects::Alerting::NotifyService.new(project, notification_payload).execute(integration&.token, integration)
@logger.info(message: 'Successfully notified of Prometheus newly unhealthy', cluster_id: @cluster.id, project_id: project.id) @logger.info(message: 'Successfully notified of Prometheus newly unhealthy', cluster_id: @cluster.id, project_id: project.id)
end end
......
...@@ -2,10 +2,16 @@ ...@@ -2,10 +2,16 @@
module Projects module Projects
module Alerting module Alerting
class NotifyService < BaseService class NotifyService
include BaseServiceUtility
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
include ::IncidentManagement::Settings include ::IncidentManagement::Settings
def initialize(project, payload)
@project = project
@payload = payload
end
def execute(token, integration = nil) def execute(token, integration = nil)
@integration = integration @integration = integration
...@@ -24,7 +30,7 @@ module Projects ...@@ -24,7 +30,7 @@ module Projects
private private
attr_reader :integration attr_reader :project, :payload, :integration
def process_alert def process_alert
if alert.persisted? if alert.persisted?
...@@ -101,7 +107,7 @@ module Projects ...@@ -101,7 +107,7 @@ module Projects
def incoming_payload def incoming_payload
strong_memoize(:incoming_payload) do strong_memoize(:incoming_payload) do
Gitlab::AlertManagement::Payload.parse(project, params.to_h) Gitlab::AlertManagement::Payload.parse(project, payload.to_h)
end end
end end
...@@ -110,7 +116,7 @@ module Projects ...@@ -110,7 +116,7 @@ module Projects
end end
def valid_payload_size? def valid_payload_size?
Gitlab::Utils::DeepSize.new(params).valid? Gitlab::Utils::DeepSize.new(payload).valid?
end end
def active_integration? def active_integration?
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Projects module Projects
module Prometheus module Prometheus
module Alerts module Alerts
class NotifyService < BaseService class NotifyService
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
include ::IncidentManagement::Settings include ::IncidentManagement::Settings
...@@ -17,9 +17,14 @@ module Projects ...@@ -17,9 +17,14 @@ module Projects
SUPPORTED_VERSION = '4' SUPPORTED_VERSION = '4'
def initialize(project, payload)
@project = project
@payload = payload
end
def execute(token, integration = nil) 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?(payload)
return unauthorized unless valid_alert_manager_token?(token, integration) return unauthorized unless valid_alert_manager_token?(token, integration)
process_prometheus_alerts process_prometheus_alerts
...@@ -27,18 +32,20 @@ module Projects ...@@ -27,18 +32,20 @@ module Projects
ServiceResponse.success ServiceResponse.success
end end
def self.processable?(params) def self.processable?(payload)
# Workaround for https://gitlab.com/gitlab-org/gitlab/-/issues/220496 # Workaround for https://gitlab.com/gitlab-org/gitlab/-/issues/220496
return false unless params return false unless payload
REQUIRED_PAYLOAD_KEYS.subset?(params.keys.to_set) && REQUIRED_PAYLOAD_KEYS.subset?(payload.keys.to_set) &&
params['version'] == SUPPORTED_VERSION payload['version'] == SUPPORTED_VERSION
end end
private private
attr_reader :project, :payload
def valid_payload_size? def valid_payload_size?
Gitlab::Utils::DeepSize.new(params).valid? Gitlab::Utils::DeepSize.new(payload).valid?
end end
def firings def firings
...@@ -50,7 +57,7 @@ module Projects ...@@ -50,7 +57,7 @@ module Projects
end end
def alerts def alerts
params['alerts'] payload['alerts']
end end
def valid_alert_manager_token?(token, integration) def valid_alert_manager_token?(token, integration)
......
...@@ -6,7 +6,7 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -6,7 +6,7 @@ RSpec.describe Projects::Alerting::NotifyService do
let_it_be(:project, refind: true) { create(:project) } let_it_be(:project, refind: true) { create(:project) }
describe '#execute' do describe '#execute' do
let(:service) { described_class.new(project, nil, payload) } let(:service) { described_class.new(project, payload) }
let(:token) { integration.token } let(:token) { integration.token }
let(:payload) do let(:payload) do
{ {
......
...@@ -7,7 +7,7 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do ...@@ -7,7 +7,7 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
let_it_be(:project, reload: true) { create(:project) } let_it_be(:project, reload: true) { create(:project) }
let(:service) { described_class.new(project, nil, payload) } let(:service) { described_class.new(project, payload) }
let(:token_input) { 'token' } let(:token_input) { 'token' }
let!(:setting) do let!(:setting) do
......
...@@ -38,7 +38,7 @@ RSpec.describe Projects::Alerting::NotificationsController do ...@@ -38,7 +38,7 @@ RSpec.describe Projects::Alerting::NotificationsController do
expect(notify_service_class) expect(notify_service_class)
.to have_received(:new) .to have_received(:new)
.with(project, nil, permitted_params) .with(project, permitted_params)
end end
end end
......
...@@ -168,7 +168,7 @@ RSpec.describe Projects::Prometheus::AlertsController do ...@@ -168,7 +168,7 @@ RSpec.describe Projects::Prometheus::AlertsController do
expect(Projects::Prometheus::Alerts::NotifyService) expect(Projects::Prometheus::Alerts::NotifyService)
.to receive(:new) .to receive(:new)
.with(project, nil, duck_type(:permitted?)) .with(project, duck_type(:permitted?))
.and_return(notify_service) .and_return(notify_service)
end end
......
...@@ -13,7 +13,7 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -13,7 +13,7 @@ RSpec.describe Projects::Alerting::NotifyService do
let(:token) { 'invalid-token' } let(:token) { 'invalid-token' }
let(:starts_at) { Time.current.change(usec: 0) } let(:starts_at) { Time.current.change(usec: 0) }
let(:fingerprint) { 'testing' } let(:fingerprint) { 'testing' }
let(:service) { described_class.new(project, nil, payload) } let(:service) { described_class.new(project, payload) }
let_it_be(:environment) { create(:environment, project: project) } let_it_be(:environment) { create(:environment, project: project) }
let(:environment) { create(:environment, project: project) } let(:environment) { create(:environment, project: project) }
let(:ended_at) { nil } let(:ended_at) { nil }
......
...@@ -7,7 +7,7 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do ...@@ -7,7 +7,7 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
let_it_be(:project, reload: true) { create(:project) } let_it_be(:project, reload: true) { create(:project) }
let(:service) { described_class.new(project, nil, payload) } let(:service) { described_class.new(project, payload) }
let(:token_input) { 'token' } let(:token_input) { 'token' }
let!(:setting) do let!(:setting) do
......
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