Skip to content

Add migration to populate pipeline_id in Vulnerability Feedback

Alan (Maciej) Paruszewski requested to merge 254228-background-migration-for-pipeline-id into master

What does this MR do?

Related to #254228 (closed)

This MR adds background migration to populate pipeline_id for vulnerability feedback entities that were created on single Vulnerability page. Lack of value for pipeline_id causes small problems in MR security widget where findings are not marked as dismissed. By default we will set pipeline_id as latest successful pipeline with security reports.

Migration

== 20201026182253 SchedulePopulateVulnerabilityFeedbackPipelineId: migrating ==
-- exec_query("SELECT DISTINCT \"vulnerability_feedback\".\"project_id\"\nFROM \"vulnerability_feedback\"\nWHERE \"vulnerability_feedback\".\"pipeline_id\" IS NULL\nORDER BY \"vulnerability_feedback\".\"project_id\" ASC\n")
   -> 0.0011s
== 20201026182253 SchedulePopulateVulnerabilityFeedbackPipelineId: migrated (0.0157s) 
== 20201026182253 SchedulePopulateVulnerabilityFeedbackPipelineId: reverting ==
== 20201026182253 SchedulePopulateVulnerabilityFeedbackPipelineId: reverted (0.0000s)

Queries

UPDATE
    vulnerability_feedback
SET
    pipeline_id = pipelines_with_reports.id
FROM (
    SELECT
        ci_pipelines.id, ci_pipelines.project_id
    FROM
        ci_pipelines
    WHERE (ci_pipelines.project_id IN ('278964', '278965', '278966', '278967', '278968', '278969', '278970', '278971', '278972', '278973', '278974', '278975', '278976', '278977', '278978', '278979', '278980', '278981', '278982', '278983', '278984', '278985', '278986', '278987', '278988', '278989', '278990', '278991', '278992', '278993', '278994', '278995', '278996', '278997', '278998', '278999', '279000', '279001', '279002', '279003', '279004', '279005', '279006', '279007', '279008', '279009', '279010', '279011', '279012', '279013', '279014', '279015', '279016', '279017', '279018', '279019', '279020', '279021', '279022', '279023', '279024', '279025', '279026', '279027', '279028', '279029', '279030', '279031', '279032', '279033', '279034', '279035', '279036', '279037', '279038', '279039', '279040', '279041', '279042', '279043', '279044', '279045', '279046', '279047', '279048', '279049', '279050', '279051', '279052', '279053', '279054', '279055', '279056', '279057', '279058', '279059', '279060', '279061', '279062', '279063'))
    AND (ci_pipelines.status IN ('success'))
    AND (EXISTS (
            SELECT
                1
            FROM
                ci_builds
            WHERE
                ci_builds.type = 'Ci::Build'
                AND (ci_builds.retried = FALSE
                    OR ci_builds.retried IS NULL)
                AND (EXISTS (
                        SELECT
                            1
                        FROM
                            ci_job_artifacts
                        WHERE (ci_builds.id = ci_job_artifacts.job_id)
                        AND ci_job_artifacts.file_type IN (5, 6, 7, 8, 21, 23, 26)))
                AND (ci_pipelines.id = ci_builds.commit_id)))
    ORDER BY
        ci_pipelines.id DESC
    LIMIT 1) AS pipelines_with_reports
WHERE
    vulnerability_feedback.pipeline_id IS NULL
    AND vulnerability_feedback.project_id = pipelines_with_reports.project_id;

https://explain.depesz.com/s/m0oK ~50ms

UPDATE
    vulnerability_feedback
SET
    pipeline_id = pipelines_with_reports.id
FROM (SELECT "ci_pipelines"."id", "ci_pipelines"."project_id"
    FROM "ci_pipelines"
      INNER JOIN "ci_builds" ON "ci_builds"."commit_id" = "ci_pipelines"."id"
      AND "ci_builds"."type" = 'Ci::Build'
      AND (
        "ci_builds"."retried" = FALSE
        OR "ci_builds"."retried" IS NULL
      )
      INNER JOIN "ci_job_artifacts" ON "ci_job_artifacts"."file_type" IN (19, 26, 1, 17, 9, 7, 8, 6, 16, 4, 10, 101, 15, 12, 11, 24, 25, 5, 21, 22)
      AND "ci_job_artifacts"."job_id" = "ci_builds"."id"
    WHERE "ci_pipelines"."project_id" IN ('278964', '278965', '278966', '278967', '278968', '278969', '278970', '278971', '278972', '278973', '278974', '278975', '278976', '278977', '278978', '278979', '278980', '278981', '278982', '278983', '278984', '278985', '278986', '278987', '278988', '278989', '278990', '278991', '278992', '278993', '278994', '278995', '278996', '278997', '278998', '278999', '279000', '279001', '279002', '279003', '279004', '279005', '279006', '279007', '279008', '279009', '279010', '279011', '279012', '279013', '279014', '279015', '279016', '279017', '279018', '279019', '279020', '279021', '279022', '279023', '279024', '279025', '279026', '279027', '279028', '279029', '279030', '279031', '279032', '279033', '279034', '279035', '279036', '279037', '279038', '279039', '279040', '279041', '279042', '279043', '279044', '279045', '279046', '279047', '279048', '279049', '279050', '279051', '279052', '279053', '279054', '279055', '279056', '279057', '279058', '279059', '279060', '279061', '279062', '279063')
      AND ("ci_pipelines"."status" IN ('success'))
      AND "ci_builds"."name" IN (
        'sast',
        'secret_detection',
        'dependency_scanning',
        'container_scanning',
        'dast'
      )
    ORDER BY "ci_pipelines"."id" DESC
    LIMIT 1
  
) AS pipelines_with_reports
WHERE
    vulnerability_feedback.pipeline_id IS NULL
    AND vulnerability_feedback.project_id = pipelines_with_reports.project_id;

https://explain.depesz.com/s/a7zS ~25ms

Background migration in batches calculation

Time for batch of projects => 2.5s + 1.5s = 4s (cold cache) and 50ms + 25ms = 75ms (warm cache) for GitLab project -> selected 5s as an estimate

Time for executing one batch (100 projects) => ~5s -> selected 2 minutes as an INTERVAL_DELAY, as this is lowest used value currently

Number of batches => currently ~1600 projects with vulnerability feedback with no pipeline_id => 16 batches, 2 minutes each

Time to execute the migration => 32 minutes

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
Edited by Mayra Cabrera

Merge request reports