Merge branch '291986-avoid-passing-nil-to-user-less-alert-services' into 'master'

Resolve "Avoid passing `nil` users to user-less alert services"

See merge request gitlab-org/gitlab!49929

app/controllers/projects/alerting/notifications_controller.rb

@@ -31,7 +31,7 @@ module Projects
       end
 
       def notify_service
-        notify_service_class.new(project, current_user, notification_payload)
+        notify_service_class.new(project, notification_payload)
       end
 
       def notify_service_class

app/controllers/projects/prometheus/alerts_controller.rb

@@ -73,7 +73,7 @@ module Projects
 
       def notify_service
         Projects::Prometheus::Alerts::NotifyService
-          .new(project, current_user, params.permit!)
+          .new(project, params.permit!)
       end
 
       def create_service

app/services/alert_management/process_prometheus_alert_service.rb

 # frozen_string_literal: true
 
 module AlertManagement
-  class ProcessPrometheusAlertService < BaseService
+  class ProcessPrometheusAlertService
+    include BaseServiceUtility
     include Gitlab::Utils::StrongMemoize
     include ::IncidentManagement::Settings
 
+    def initialize(project, payload)
+      @project = project
+      @payload = payload
+    end
+
     def execute
       return bad_request unless incoming_payload.has_required_attributes?
 
@@ -19,6 +25,8 @@ module AlertManagement
 
     private
 
+    attr_reader :project, :payload
+
     def process_alert_management_alert
       if incoming_payload.resolved?
         process_resolved_alert_management_alert
@@ -127,7 +135,7 @@ module AlertManagement
       strong_memoize(:incoming_payload) do
         Gitlab::AlertManagement::Payload.parse(
           project,
-          params,
+          payload,
           monitoring_tool: Gitlab::AlertManagement::Payload::MONITORING_TOOLS[:prometheus]
         )
       end

app/services/clusters/applications/prometheus_health_check_service.rb

@@ -65,7 +65,7 @@ module Clusters
         notification_payload = build_notification_payload(project)
         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)
       end

app/services/projects/alerting/notify_service.rb

@@ -2,10 +2,16 @@
 
 module Projects
   module Alerting
-    class NotifyService < BaseService
+    class NotifyService
+      include BaseServiceUtility
       include Gitlab::Utils::StrongMemoize
       include ::IncidentManagement::Settings
 
+      def initialize(project, payload)
+        @project = project
+        @payload = payload
+      end
+
       def execute(token, integration = nil)
         @integration = integration
 
@@ -24,7 +30,7 @@ module Projects
 
       private
 
-      attr_reader :integration
+      attr_reader :project, :payload, :integration
 
       def process_alert
         if alert.persisted?
@@ -101,7 +107,7 @@ module Projects
 
       def incoming_payload
         strong_memoize(:incoming_payload) do
-          Gitlab::AlertManagement::Payload.parse(project, params.to_h)
+          Gitlab::AlertManagement::Payload.parse(project, payload.to_h)
         end
       end
 
@@ -110,7 +116,7 @@ module Projects
       end
 
       def valid_payload_size?
-        Gitlab::Utils::DeepSize.new(params).valid?
+        Gitlab::Utils::DeepSize.new(payload).valid?
       end
 
       def active_integration?

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

@@ -3,7 +3,7 @@
 module Projects
   module Prometheus
     module Alerts
-      class NotifyService < BaseService
+      class NotifyService
         include Gitlab::Utils::StrongMemoize
         include ::IncidentManagement::Settings
 
@@ -17,9 +17,14 @@ module Projects
 
         SUPPORTED_VERSION = '4'
 
+        def initialize(project, payload)
+          @project = project
+          @payload = payload
+        end
+
         def execute(token, integration = nil)
           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)
 
           process_prometheus_alerts
@@ -27,18 +32,20 @@ module Projects
           ServiceResponse.success
         end
 
-        def self.processable?(params)
+        def self.processable?(payload)
           # 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) &&
-            params['version'] == SUPPORTED_VERSION
+          REQUIRED_PAYLOAD_KEYS.subset?(payload.keys.to_set) &&
+            payload['version'] == SUPPORTED_VERSION
         end
 
         private
 
+        attr_reader :project, :payload
+
         def valid_payload_size?
-          Gitlab::Utils::DeepSize.new(params).valid?
+          Gitlab::Utils::DeepSize.new(payload).valid?
         end
 
         def firings
@@ -50,7 +57,7 @@ module Projects
         end
 
         def alerts
-          params['alerts']
+          payload['alerts']
         end
 
         def valid_alert_manager_token?(token, integration)
@@ -121,7 +128,7 @@ module Projects
         def process_prometheus_alerts
           alerts.each do |alert|
             AlertManagement::ProcessPrometheusAlertService
-              .new(project, nil, alert.to_h)
+              .new(project, alert.to_h)
               .execute
           end
         end

ee/app/services/alert_management/network_alert_service.rb

@@ -2,12 +2,17 @@
 
 module AlertManagement
   # Create alerts coming K8 through gitlab-agent
-  class NetworkAlertService < BaseService
+  class NetworkAlertService
     include Gitlab::Utils::StrongMemoize
     include ::IncidentManagement::Settings
 
     MONITORING_TOOL = Gitlab::AlertManagement::Payload::MONITORING_TOOLS.fetch(:cilium)
 
+    def initialize(project, payload)
+      @project = project
+      @payload = payload
+    end
+
     # Users of this service need to check the agent token before calling `execute`.
     # https://gitlab.com/gitlab-org/gitlab/-/issues/292707 will handle token within the service.
     def execute
@@ -24,8 +29,10 @@ module AlertManagement
 
     private
 
+    attr_reader :project, :payload
+
     def valid_payload_size?
-      Gitlab::Utils::DeepSize.new(params).valid?
+      Gitlab::Utils::DeepSize.new(payload).valid?
     end
 
     def process_request
@@ -78,7 +85,7 @@ module AlertManagement
       strong_memoize(:incoming_payload) do
         Gitlab::AlertManagement::Payload.parse(
           project,
-          params,
+          payload,
           monitoring_tool: MONITORING_TOOL
         )
       end

ee/lib/ee/api/internal/kubernetes.rb

@@ -25,7 +25,7 @@ module EE
 
                   forbidden! unless project.feature_available?(:cilium_alerts)
 
-                  result = ::AlertManagement::NetworkAlertService.new(agent.project, nil, params[:alert]).execute
+                  result = ::AlertManagement::NetworkAlertService.new(agent.project, params[:alert]).execute
 
                   status result.http_status
                 end

ee/spec/services/alert_management/network_alert_service_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe AlertManagement::NetworkAlertService do
   let_it_be(:environment) { create(:environment, project: project) }
 
   describe '#execute' do
-    let(:service) { described_class.new(project, nil, payload) }
+    let(:service) { described_class.new(project, payload) }
     let(:tool) { Gitlab::AlertManagement::Payload::MONITORING_TOOLS[:cilium] }
     let(:starts_at) { Time.current.change(usec: 0) }
     let(:ended_at) { nil }

ee/spec/services/projects/alerting/notify_service_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe Projects::Alerting::NotifyService do
   let_it_be(:project, refind: true) { create(:project) }
 
   describe '#execute' do
-    let(:service) { described_class.new(project, nil, payload) }
+    let(:service) { described_class.new(project, payload) }
     let(:token) { integration.token }
     let(:payload) do
       {

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

@@ -7,7 +7,7 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
 
   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!(:setting) do

spec/controllers/projects/alerting/notifications_controller_spec.rb

@@ -38,7 +38,7 @@ RSpec.describe Projects::Alerting::NotificationsController do
 
           expect(notify_service_class)
             .to have_received(:new)
-            .with(project, nil, permitted_params)
+            .with(project, permitted_params)
         end
       end
 

spec/controllers/projects/prometheus/alerts_controller_spec.rb

@@ -168,7 +168,7 @@ RSpec.describe Projects::Prometheus::AlertsController do
 
       expect(Projects::Prometheus::Alerts::NotifyService)
         .to receive(:new)
-        .with(project, nil, duck_type(:permitted?))
+        .with(project, duck_type(:permitted?))
         .and_return(notify_service)
     end
 

spec/services/alert_management/process_prometheus_alert_service_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe AlertManagement::ProcessPrometheusAlertService do
   end
 
   describe '#execute' do
-    let(:service) { described_class.new(project, nil, payload) }
+    let(:service) { described_class.new(project, payload) }
     let(:auto_close_incident) { true }
     let(:create_issue) { true }
     let(:send_email) { true }

spec/services/projects/alerting/notify_service_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe Projects::Alerting::NotifyService do
     let(:token) { 'invalid-token' }
     let(:starts_at) { Time.current.change(usec: 0) }
     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(:environment) { create(:environment, project: project) }
     let(:ended_at) { nil }

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

@@ -7,7 +7,7 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
 
   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!(:setting) do
@@ -218,7 +218,7 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do
         it 'processes Prometheus alerts' do
           expect(AlertManagement::ProcessPrometheusAlertService)
             .to receive(:new)
-            .with(project, nil, kind_of(Hash))
+            .with(project, kind_of(Hash))
             .exactly(3).times
             .and_return(process_service)
           expect(process_service).to receive(:execute).exactly(3).times