Fix Security features tables cross-joining `ci_builds` to map pipeline -> security scans
From !62092 (closed) we have seen that VulnerabilityCountingService
needs to join between non ci_*
tables and ci_*
tables.
But this will not be possible once we move ci_builds to a separate database.
The particular query comes from Pipeline#security_findings
:
SELECT "security_findings".* FROM "security_findings"
INNER JOIN "security_scans" ON "security_findings"."scan_id" = "security_scans"."id"
INNER JOIN "ci_builds" ON "security_scans"."build_id" = "ci_builds"."id"
WHERE "ci_builds"."type" = 'Ci::Build' AND "ci_builds"."commit_id" = 5
This also affects Pipeline#has_security_findings?
which is used in other places.
Options
- De-normalize
pipeline_id
ontosecurity_scans
table - Change/refactor this feature such that we do not need these joins
- Introduce another de-normalized join table that contains the columns we need to join without going to the
ci_*
table
Sharding team recommendation
Add a new column security_scans.pipeline_id
:
And then you will update:
module EE
module Ci
module Pipeline
prepended do
has_many :security_scans, class_name: 'Security::Scan'
Such that the new query will be:
SELECT "security_findings".* FROM "security_findings"
INNER JOIN "security_scans" ON "security_findings"."scan_id" = "security_scans"."id"
WHERE "security_scans"."pipeline_id" = 5
This will be more performant and will remove the joins into ci_builds
.
This will be similar to #333414 (closed) and can follow the similarly linked backfill approach of !37713 (merged) . It's also possible we could backfill both columns in 1 go.
Implementation plan:
-
Migration for adding new column -
Migration for adding index and foreign key -
Background migration for backfilling data from ci_builds
tosecurity_scans
-
Make sure we are inserting data for new column on save
Edited by Subashis Chakraborty