diff --git a/ee/app/services/security/ingestion/schedule_mark_dropped_as_resolved_service.rb b/ee/app/services/security/ingestion/schedule_mark_dropped_as_resolved_service.rb index a6a77dcd84cdda27397af15ff56010ce2399640a..b6ba1481e144032523776ce081027e84e519a86a 100644 --- a/ee/app/services/security/ingestion/schedule_mark_dropped_as_resolved_service.rb +++ b/ee/app/services/security/ingestion/schedule_mark_dropped_as_resolved_service.rb @@ -9,8 +9,8 @@ module Ingestion # finds any outstanding vulnerabilities not linked to those identifiers, # and marks them as resolved class ScheduleMarkDroppedAsResolvedService - BATCH_SIZE = 1000 - DELAY_INTERVAL = 1.minute.to_i + BATCH_SIZE = 100 + DELAY_INTERVAL = 20.seconds.to_i def self.execute(project_id, scan_type, primary_identifiers) new(project_id, scan_type, primary_identifiers).execute diff --git a/ee/app/workers/vulnerabilities/mark_dropped_as_resolved_worker.rb b/ee/app/workers/vulnerabilities/mark_dropped_as_resolved_worker.rb index fb372de31e2446742180733ae486910722642775..e014b5b11cac6175109146d9a0997dba9ed53541 100644 --- a/ee/app/workers/vulnerabilities/mark_dropped_as_resolved_worker.rb +++ b/ee/app/workers/vulnerabilities/mark_dropped_as_resolved_worker.rb @@ -5,8 +5,6 @@ module Vulnerabilities class MarkDroppedAsResolvedWorker include ApplicationWorker - BATCH_SIZE = 100 - data_consistency :delayed idempotent! deduplicate :until_executing, including_scheduled: true @@ -16,53 +14,62 @@ class MarkDroppedAsResolvedWorker loggable_arguments 1 def perform(_, dropped_identifier_ids) - dropped_identifier_ids.each_slice(BATCH_SIZE) do |identifier_ids| - vulnerability_ids = vulnerability_ids_for(identifier_ids) - vulnerabilities = resolvable_vulnerabilities(vulnerability_ids) + vulnerability_ids = vulnerability_ids_for(dropped_identifier_ids) + vulnerabilities = resolvable_vulnerabilities(vulnerability_ids) - resolve_batch(vulnerabilities) - end + resolve_vulnerabilities(vulnerabilities) end private def vulnerability_ids_for(identifier_ids) ::Vulnerabilities::Identifier.id_in(identifier_ids) - .order(:id) # rubocop:disable CodeReuse/ActiveRecord - .select_primary_finding_vulnerability_ids - .map(&:vulnerability_id) + .order(:id) # rubocop:disable CodeReuse/ActiveRecord -- unusual order call is very specific to this query + .select_primary_finding_vulnerability_ids + .map(&:vulnerability_id) end def resolvable_vulnerabilities(vulnerability_ids) return [] unless vulnerability_ids.present? ::Vulnerability.with_states(:detected) - .with_resolution(true) - .by_ids(vulnerability_ids) + .with_resolution(true) + .by_ids(vulnerability_ids) end - def resolve_batch(vulnerabilities) + def resolve_vulnerabilities(vulnerabilities) return unless vulnerabilities.present? - ::Vulnerability.transaction do - create_state_transitions(vulnerabilities) - current_time = Time.zone.now + current_time = Time.zone.now + state_transitions = build_state_transitions(vulnerabilities, current_time) + # rubocop:disable CodeReuse/ActiveRecord -- `update_all` changes the result of the query, preventing the system note update + vuln_ids_to_be_updated = vulnerabilities.pluck(:id) + # rubocop:enable CodeReuse/ActiveRecord + + ::Vulnerability.transaction do vulnerabilities.update_all( resolved_by_id: Users::Internal.security_bot.id, resolved_at: current_time, updated_at: current_time, state: :resolved) + + Vulnerabilities::StateTransition.bulk_insert!(state_transitions) end + + create_system_notes(Vulnerability.by_ids(vuln_ids_to_be_updated)) end - def create_state_transitions(vulnerabilities) - state_transitions = vulnerabilities.find_each.map do |vulnerability| - create_system_note(vulnerability) - build_state_transition_for(vulnerability) + def build_state_transitions(vulnerabilities, current_time) + vulnerabilities.find_each.map do |vulnerability| + build_state_transition_for(vulnerability, current_time) end + end - Vulnerabilities::StateTransition.bulk_insert!(state_transitions) + def create_system_notes(vulnerabilities) + vulnerabilities.find_each do |vulnerability| + create_system_note(vulnerability) + end end def create_system_note(vulnerability) @@ -73,9 +80,7 @@ def create_system_note(vulnerability) ) end - def build_state_transition_for(vulnerability) - current_time = Time.zone.now - + def build_state_transition_for(vulnerability, current_time) ::Vulnerabilities::StateTransition.new( vulnerability: vulnerability, from_state: vulnerability.state, diff --git a/ee/spec/workers/vulnerabilities/mark_dropped_as_resolved_worker_spec.rb b/ee/spec/workers/vulnerabilities/mark_dropped_as_resolved_worker_spec.rb index 18013f97dadec5aab765393b85ff9e12ea10aba9..3606acba44e36576434e58ed58d6f3621801f612 100644 --- a/ee/spec/workers/vulnerabilities/mark_dropped_as_resolved_worker_spec.rb +++ b/ee/spec/workers/vulnerabilities/mark_dropped_as_resolved_worker_spec.rb @@ -59,13 +59,6 @@ include_examples 'an idempotent worker' do let(:subject) { worker.perform(pipeline.project_id, dropped_identifiers) } - it 'iterates over dropped_identifier_ids in batches' do - expect(worker).to receive(:vulnerability_ids_for).exactly(7).times.and_call_original - expect(::Vulnerability).to receive(:transaction).twice - - subject - end - it 'changes state of dismissable vulnerabilities to resolved' do expect { subject }.to change { dismissable_vulnerability.reload.state } .from('detected')