Skip to content

Fix the flakiness of SetReportTypeForVulnerabilities migration

Victor Zagorodny requested to merge 36182-fix-flaky-migration-set-report-type into master

What does this MR do?

The initial idea is that many Occurrences (backed by vulnerability_occurrences table and now called Findings, more on that here) belong to one Vulnerability via vulnerability_id foreign key. Vulnerabilities get created from Findings, so there's always the first Finding that belongs to a particular Vulnerability. Later, other Findings may be attached to that Vulnerability leveraging the same foreign key vulnerability_id. Almost all data fields from a Finding is copied to a Vulnerability upon the creation of the latter.

The report_type column for Vulnerabilities was introduced later than the respective table was created to there was a need to bulk-copy the report_type values from Findings to Vulnerabilities. The report_type should be copied to a Vulnerability from the first Finding that belongs to it i.e. from which it was created.

It was detected by tests, that the order of fetching records from vulnerability_occurrences table in FROM clause of SetReportTypeForVulnerabilities migration is unstable. This was not obvious at the time when this migration was introduced.

This MR fixes that migration. We ensure that each Vulnerability is guaranteed to have its report_type copied from the first Finding that belongs to its i.e. from the Finding that created this Vulnerability.

I've tested both previous and current implementation of this migration for flakiness (200 specs' runs for each version). The initial version has failed twice per 200 runs which the second one did not fail.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Closes #36182 (closed).

Edited by Rémy Coutable

Merge request reports