Skip to content

Create IssueLink for Vulnerabilities that do not have them, attempt 3

What does this MR do?

This MR is a follow up to !39986 (merged) which was reverted because of a deployment failure in staging !40716 (merged)

The main difference is removal of unique_by option from insert_all call which should make the UPSERT respect all unique indices and do nothing on conflicts.

Pasted below are database query details from the previous merge request.

index_vulnerability_feedback_on_issue_id_not_null index is already present on staging

psql output
gitlabhq_production=> \d vulnerability_feedback;
                                           Table "public.vulnerability_feedback"
       Column        |           Type           | Collation | Nullable |                      Default
---------------------+--------------------------+-----------+----------+----------------------------------------------------
 id                  | integer                  |           | not null | nextval('vulnerability_feedback_id_seq'::regclass)
 created_at          | timestamp with time zone |           | not null |
 updated_at          | timestamp with time zone |           | not null |
 feedback_type       | smallint                 |           | not null |
 category            | smallint                 |           | not null |
 project_id          | integer                  |           | not null |
 author_id           | integer                  |           | not null |
 pipeline_id         | integer                  |           |          |
 issue_id            | integer                  |           |          |
 project_fingerprint | character varying(40)    |           | not null |
 merge_request_id    | integer                  |           |          |
 comment_author_id   | integer                  |           |          |
 comment             | text                     |           |          |
 comment_timestamp   | timestamp with time zone |           |          |
Indexes:
    "vulnerability_feedback_pkey" PRIMARY KEY, btree (id)
    "vulnerability_feedback_unique_idx" UNIQUE, btree (project_id, category, feedback_type, project_fingerprint)
    "index_vulnerability_feedback_on_author_id" btree (author_id)
    "index_vulnerability_feedback_on_comment_author_id" btree (comment_author_id)
    "index_vulnerability_feedback_on_issue_id" btree (issue_id)
    "index_vulnerability_feedback_on_issue_id_not_null" btree (id) WHERE issue_id IS NOT NULL
    "index_vulnerability_feedback_on_merge_request_id" btree (merge_request_id)
    "index_vulnerability_feedback_on_pipeline_id" btree (pipeline_id)
Foreign-key constraints:
    "fk_563ff1912e" FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE SET NULL
    "fk_94f7c8a81e" FOREIGN KEY (comment_author_id) REFERENCES users(id) ON DELETE SET NULL
    "fk_rails_20976e6fd9" FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE SET NULL
    "fk_rails_472f69b043" FOREIGN KEY (author_id) REFERENCES users(id) ON DELETE CASCADE
    "fk_rails_8c77e5891a" FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE SET NULL
    "fk_rails_debd54e456" FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE
Publications:
    "mypub"

Records to be modified count

Around 2530 as of 2020-08-26

SQL
SELECT COUNT(*)
FROM
    vulnerability_feedback
    JOIN vulnerability_occurrences vo ON vo.project_id = vulnerability_feedback.project_id
        AND vo.report_type = vulnerability_feedback.category
        AND encode(vo.project_fingerprint, 'hex') = vulnerability_feedback.project_fingerprint
WHERE (issue_id IS NOT NULL)
AND (vo.vulnerability_id IS NOT NULL)

Vulnerabilities::Feedback.where("issue_id IS NOT NULL") query details

With the specialized indices created for the migration in !38898 (merged) and index_vulnerability_feedback_on_issue_id_not_null one batch should take around 43ms.

Summary
Time: 42.943 ms  
  - planning: 31.105 ms  
  - execution: 11.838 ms  
    - I/O read: 0.000 ms  
    - I/O write: 0.000 ms  
  
Shared buffers:  
  - hits: 9430 (~73.70 MiB) from the buffer pool  
  - reads: 0 from the OS file cache, including disk I/O  
  - dirtied: 0  
  - writes: 0  

Plan for Vulnerabilities::Finding without vulnerability_id

We are exploring ways to fix this problem in #239015 (closed)

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

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/223770

Edited by Michał Zając

Merge request reports