Skip to content

Replace .latest scope with .merge

What does this MR do and why?

This MR removes a redundant extra join on the security_scans table when Security::FindingsFinder#all_security_findings is called.

This fixes the related issue #384102 (closed) - see !105097 (comment 1186339891) for the original discussion.

Database review

Below are the relevant queries before and after these MR changes, first with deprecate_vulnerabilities_feedback enabled and disabled. The query was generated by calling Security::FindingsFinder.new(Ci::Pipeline.last).execute

Query before and after MR with `deprecate_vulnerabilities_feedback` enabled

Before

SELECT
    "security_findings".*
FROM
    "security_findings"
    INNER JOIN "security_scans" ON "security_findings"."scan_id" = "security_scans"."id"
    INNER JOIN "security_scans" "scans_security_findings" ON "scans_security_findings"."id" = "security_findings"."scan_id"
WHERE
    "security_scans"."pipeline_id" = 568
    AND "security_findings"."deduplicated" = TRUE
    AND "security_scans"."latest" = TRUE
    AND "security_scans"."status" = 1
    AND (NOT EXISTS (
            SELECT
                1
            FROM
                "vulnerabilities"
                INNER JOIN "vulnerability_occurrences" ON "vulnerability_occurrences"."vulnerability_id" = "vulnerabilities"."id"
            WHERE
                "vulnerabilities"."state" = 2
                AND (vulnerability_occurrences.uuid = security_findings.uuid::text)))
ORDER BY
    "security_findings"."severity" DESC,
    "security_findings"."id" ASC
LIMIT 20 OFFSET 0

After

SELECT
    "security_findings".*
FROM
    "security_findings"
    INNER JOIN "security_scans" ON "security_findings"."scan_id" = "security_scans"."id"
WHERE
    "security_scans"."pipeline_id" = 568
    AND "security_findings"."deduplicated" = TRUE
    AND "security_scans"."latest" = TRUE
    AND "security_scans"."status" = 1
    AND (NOT EXISTS (
            SELECT
                1
            FROM
                "vulnerabilities"
                INNER JOIN "vulnerability_occurrences" ON "vulnerability_occurrences"."vulnerability_id" = "vulnerabilities"."id"
            WHERE
                "vulnerabilities"."state" = 2
                AND (vulnerability_occurrences.uuid = security_findings.uuid::text)))
ORDER BY
    "security_findings"."severity" DESC,
    "security_findings"."id" ASC
LIMIT 20 OFFSET 0
Query before and after MR with `deprecate_vulnerabilities_feedback` disabled

Before

SELECT
    "security_findings".*
FROM
    "security_findings"
    INNER JOIN "security_scans" ON "security_findings"."scan_id" = "security_scans"."id"
    INNER JOIN "security_scans" "scans_security_findings" ON "scans_security_findings"."id" = "security_findings"."scan_id"
WHERE
    "security_scans"."pipeline_id" = 568
    AND "security_findings"."deduplicated" = TRUE
    AND "security_scans"."latest" = TRUE
    AND "security_scans"."status" = 1
    AND (NOT EXISTS (
            SELECT
                1
            FROM
                "security_scans"
                INNER JOIN "projects" ON "projects"."id" = "security_scans"."project_id"
                INNER JOIN "vulnerability_feedback" ON "vulnerability_feedback"."project_id" = "projects"."id"
            WHERE (vulnerability_feedback.category = (security_scans.scan_type - 1))
            AND "vulnerability_feedback"."feedback_type" = 0
            AND (security_scans.id = security_findings.scan_id)
            AND (vulnerability_feedback.finding_uuid = security_findings.uuid)))
ORDER BY
    "security_findings"."severity" DESC,
    "security_findings"."id" ASC
LIMIT 20 OFFSET 0

After

SELECT
    "security_findings".*
FROM
    "security_findings"
    INNER JOIN "security_scans" ON "security_findings"."scan_id" = "security_scans"."id"
WHERE
    "security_scans"."pipeline_id" = 568
    AND "security_findings"."deduplicated" = TRUE
    AND "security_scans"."latest" = TRUE
    AND "security_scans"."status" = 1
    AND (NOT EXISTS (
            SELECT
                1
            FROM
                "security_scans"
                INNER JOIN "projects" ON "projects"."id" = "security_scans"."project_id"
                INNER JOIN "vulnerability_feedback" ON "vulnerability_feedback"."project_id" = "projects"."id"
            WHERE (vulnerability_feedback.category = (security_scans.scan_type - 1))
            AND "vulnerability_feedback"."feedback_type" = 0
            AND (security_scans.id = security_findings.scan_id)
            AND (vulnerability_feedback.finding_uuid = security_findings.uuid)))
ORDER BY
    "security_findings"."severity" DESC,
    "security_findings"."id" ASC
LIMIT 20 OFFSET 0

In both cases above, the only difference between the 2 queries is the removal of the redundant join.

--- before-ff-enabled.sql	2023-01-19 11:56:58.054630035 +1300
+++ after-ff-enabled.sql	2023-01-19 11:59:48.840539552 +1300
@@ -3,7 +3,6 @@
 FROM
     "security_findings"
     INNER JOIN "security_scans" ON "security_findings"."scan_id" = "security_scans"."id"
-    INNER JOIN "security_scans" "scans_security_findings" ON "scans_security_findings"."id" = "security_findings"."scan_id"
 WHERE
     "security_scans"."pipeline_id" = 568
     AND "security_findings"."deduplicated" = TRUE
--- before-ff-disabled.sql	2023-01-19 12:05:29.296346186 +1300
+++ after-ff-disabled.sql	2023-01-19 12:04:15.683523130 +1300
@@ -3,7 +3,6 @@
 FROM
     "security_findings"
     INNER JOIN "security_scans" ON "security_findings"."scan_id" = "security_scans"."id"
-    INNER JOIN "security_scans" "scans_security_findings" ON "scans_security_findings"."id" = "security_findings"."scan_id"
 WHERE
     "security_scans"."pipeline_id" = 568
     AND "security_findings"."deduplicated" = TRUE

How to set up and validate locally

  1. Find a pipeline
    pipeline = Ci::Pipeline.last
  2. Enable deprecate_vulnerabilities_feedback for the pipeline project
    Feature.enable(:deprecate_vulnerabilities_feedback, pipeline.project)
  3. Execute the finder to trigger the query and examine the logged SQL
    Security::FindingsFinder.new(pipeline).execute
  4. Disable deprecate_vulnerabilities_feedback for the pipeline project
    Feature.disable(:deprecate_vulnerabilities_feedback, pipeline.project)
  5. Repeat step 3.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #384102 (closed)

Merge request reports

Loading