Create Merge Request From Remediated Vulnerability Solution
What does this MR do?
Adds support for creating a Merge Request from a Vulnerability Solution
- Add
MergeRequests::CreateFromVulnerabilityDataService
- Add new Vulnerabilities::Feedback#feedback_type of 'merge_request'
Issue: https://gitlab.com/gitlab-org/gitlab-ee/issues/9224
https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/9326
Followup to reverted MR:Issue covering revert: https://gitlab.com/gitlab-org/gitlab-ee/issues/10181
It looks like the issue with the original migration was due to MySQL's inability to rollback an index due to foreign key constraints:
Caused by:
Mysql2::Error: Cannot drop index 'index_vulnerability_feedback_on_merge_request_id': needed in a foreign key constraint
So I've updated the index migration with explicit up/down steps. This took some trial and error as I found a few references in the existing codebase but both of the docs on foreign keys and migrations have no explicit mention of this issue:
- Adding foreign-key constraints https://docs.gitlab.com/ee/development/migration_style_guide.html#adding-foreign-key-constraints
- Foreign Keys & Associations https://docs.gitlab.com/ee/development/foreign_keys.html
It would be great to update these accordingly.
Database checklist
-
Conforms to the database guides
When adding migrations:
-
Updated db/schema.rb
-
Added a down
method so the migration can be reverted -
Added the output of the migration(s) to the MR body -
Added tests for the migration in spec/migrations
if necessary (e.g. when migrating data)
❯ be rails db:migrate
== 20190129013538 AddMergeRequestIdToVulnerabilityFeedback: migrating =========
-- add_column(:vulnerability_feedback, :merge_request_id, :integer)
-> 0.0010s
== 20190129013538 AddMergeRequestIdToVulnerabilityFeedback: migrated (0.0010s)
== 20190301182031 AddMergeRequestIdIndexOnVulnerabilityFeedback: migrating ====
-- transaction_open?()
-> 0.0000s
-- foreign_keys(:vulnerability_feedback)
-> 0.0020s
-- execute("ALTER TABLE vulnerability_feedback\nADD CONSTRAINT fk_563ff1912e\nFOREIGN KEY (merge_request_id)\nREFERENCES merge_requests (id)\nON DELETE SET NULL\nNOT VALID;\n")
-> 0.0015s
-- execute("SET statement_timeout TO 0")
-> 0.0003s
-- execute("ALTER TABLE vulnerability_feedback VALIDATE CONSTRAINT fk_563ff1912e;")
-> 0.0096s
-- execute("RESET ALL")
-> 0.0004s
-- transaction_open?()
-> 0.0000s
-- index_exists?(:vulnerability_feedback, :merge_request_id, {:algorithm=>:concurrently})
-> 0.0029s
-- execute("SET statement_timeout TO 0")
-> 0.0003s
-- add_index(:vulnerability_feedback, :merge_request_id, {:algorithm=>:concurrently})
-> 0.0028s
-- execute("RESET ALL")
-> 0.0003s
== 20190301182031 AddMergeRequestIdIndexOnVulnerabilityFeedback: migrated (0.0207s)
When adding foreign keys to existing tables:
-
Included a migration to remove orphaned rows in the source table before adding the foreign key -
Removed any instances of dependent: ...
that may no longer be necessary
General checklist
-
Changelog entry added, if necessary -
Documentation created/updated -
Tests added for this feature/bug -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides
Edited by Lucas Charles