Fix the has_remediations ingestion logic for vulnerability_reads
We introduced the ingestion logic to update vulnerability_reads.has_remediations
in !129125 (diffs) but during related backend issue verification we observed that this logic has a bug and needs a fix.
Solution proposal:
- Use
return_data
from ingestion task as it is the source of truth for remediations present on DB.
diff --git a/ee/app/services/security/ingestion/tasks/ingest_remediations.rb b/ee/app/services/security/ingestion/tasks/ingest_remediations.rb
index 99b53a9ea624..aa201d9b7899 100644
--- a/ee/app/services/security/ingestion/tasks/ingest_remediations.rb
+++ b/ee/app/services/security/ingestion/tasks/ingest_remediations.rb
@@ -93,28 +93,36 @@ def dissassociate_unfound_remediations
unfound_remediations.delete_all
end
+ def finding_maps_vul_finding_remediations
+ @finding_maps_vul_finding_remediations ||= Vulnerabilities::FindingRemediation.by_finding_id(finding_maps.map(&:finding_id))
+ end
+
# When a new pipeline is run and the new findings set doesn't include some of the older findings,
# the remediation associated with the older findings should be corrected.
def unfound_remediations
- @unfound_remediations ||= Vulnerabilities::FindingRemediation.by_finding_id(finding_maps.map(&:finding_id))
- .id_not_in(return_data.flatten)
+ @unfound_remediations ||= finding_maps_vul_finding_remediations.id_not_in(return_data.flatten)
end
- def update_vulnerability_reads
- finding_ids = finding_maps.map(&:finding_id)
- vulnerability_ids = Vulnerabilities::Finding.id_in(finding_ids).pluck_vulnerability_ids
+ def found_remediations
+ @found_remediations ||= finding_maps_vul_finding_remediations.id_in(return_data.flatten)
+ end
+ def update_vulnerability_reads
if unfound_remediations.present?
unfound_remediations_finding_ids = unfound_remediations.map(&:vulnerability_occurrence_id)
unfound_remediations_vulnerability_ids = Vulnerabilities::Finding.id_in(unfound_remediations_finding_ids)
.pluck_vulnerability_ids
Vulnerabilities::Read.by_vulnerabilities(unfound_remediations_vulnerability_ids).update_all(has_remediations: false)
-
- vulnerability_ids -= unfound_remediations_vulnerability_ids
end
- Vulnerabilities::Read.by_vulnerabilities(vulnerability_ids).update_all(has_remediations: true)
+ return unless found_remediations.present?
+
+ found_remediations_finding_ids = found_remediations.map(&:vulnerability_occurrence_id)
+ found_remediations_vulnerability_ids = Vulnerabilities::Finding.id_in(found_remediations_finding_ids)
+ .pluck_vulnerability_ids
+
+ Vulnerabilities::Read.by_vulnerabilities(found_remediations_vulnerability_ids).update_all(has_remediations: true)
end
def new_report_remediations
- After
step-1
is merged requeue the modified backfill batched migration as diff below following the requeue guidelines
diff --git a/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads.rb b/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads.rb
index 83acd8a27f7b..53855da6ad41 100644
--- a/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads.rb
+++ b/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads.rb
@@ -20,6 +20,10 @@ class BackfillHasRemediationsOfVulnerabilityReads < BatchedMigrationJob
def perform
each_sub_batch do |sub_batch|
+ reset_vulnerability_reads_query = reset_vulnerability_reads_query(sub_batch)
+
+ connection.execute(reset_vulnerability_reads_query)
+
update_query = update_query_for(sub_batch)
connection.execute(update_query)
@@ -28,6 +32,10 @@ def perform
private
+ def reset_vulnerability_reads_query(sub_batch)
+ sub_batch.update(has_remediations: false).to_sql
+ end
+
def update_query_for(sub_batch)
subquery = sub_batch.joins("
INNER JOIN vulnerability_occurrences ON
Verification Steps
- Import demo project and run the pipeline for the main branch.
- The
vulnerability_reads
records created on the imported project should have two records withhas_remediations
set to true and false for each of the records corresponding to the records invulnerability_findings_remediations
. This can be confirmed using GraphQL API by togglinghasRemediations: true/false
and the result should have one vulnerability for each of the hasRemediations true and false value.
query {
project(fullPath: "[full-path-here]") {
name
vulnerabilities(hasRemediations: true) {
nodes {
id
title
description
severity
hasRemediations
}
}
}
}
Edited by Bala Kumar