Merge branch '34564-vulnerability-issue-links' into 'master'

Data model updates to allow linking of Vulnerabilities to Issues

See merge request gitlab-org/gitlab!19852

changelogs/unreleased/34564-vulnerability-issue-links.yml

+---
+title: Update the DB schema to allow linking between Vulnerabilities and Issues
+merge_request: 19852
+author:
+type: added

db/migrate/20191115091425_create_vulnerability_issue_links.rb

+# frozen_string_literal: true
+
+class CreateVulnerabilityIssueLinks < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    create_table :vulnerability_issue_links do |t|
+      # index: false because idx_vulnerability_issue_links_on_vulnerability_id_and_issue_id refers the same column
+      t.references :vulnerability, null: false, index: false, foreign_key: { on_delete: :cascade }
+      # index: true is implied
+      t.references :issue, null: false, foreign_key: { on_delete: :cascade }
+      t.integer 'link_type', limit: 2, null: false, default: 1 # 'related'
+      t.index %i[vulnerability_id issue_id],
+              name: 'idx_vulnerability_issue_links_on_vulnerability_id_and_issue_id',
+              unique: true # only one link (and of only one type) is allowed
+      t.index %i[vulnerability_id link_type],
+              name: 'idx_vulnerability_issue_links_on_vulnerability_id_and_link_type',
+              where: 'link_type = 2',
+              unique: true # only one 'created' link per vulnerability is allowed
+      t.timestamps_with_timezone
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_11_14_173624) do
+ActiveRecord::Schema.define(version: 2019_11_15_091425) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -4018,6 +4018,17 @@ ActiveRecord::Schema.define(version: 2019_11_14_173624) do
     t.index ["project_id", "fingerprint"], name: "index_vulnerability_identifiers_on_project_id_and_fingerprint", unique: true
   end
 
+  create_table "vulnerability_issue_links", force: :cascade do |t|
+    t.bigint "vulnerability_id", null: false
+    t.bigint "issue_id", null: false
+    t.integer "link_type", limit: 2, default: 1, null: false
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.index ["issue_id"], name: "index_vulnerability_issue_links_on_issue_id"
+    t.index ["vulnerability_id", "issue_id"], name: "idx_vulnerability_issue_links_on_vulnerability_id_and_issue_id", unique: true
+    t.index ["vulnerability_id", "link_type"], name: "idx_vulnerability_issue_links_on_vulnerability_id_and_link_type", unique: true, where: "(link_type = 2)"
+  end
+
   create_table "vulnerability_occurrence_identifiers", force: :cascade do |t|
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
@@ -4547,6 +4558,8 @@ ActiveRecord::Schema.define(version: 2019_11_14_173624) do
   add_foreign_key "vulnerability_feedback", "users", column: "author_id", on_delete: :cascade
   add_foreign_key "vulnerability_feedback", "users", column: "comment_author_id", name: "fk_94f7c8a81e", on_delete: :nullify
   add_foreign_key "vulnerability_identifiers", "projects", on_delete: :cascade
+  add_foreign_key "vulnerability_issue_links", "issues", on_delete: :cascade
+  add_foreign_key "vulnerability_issue_links", "vulnerabilities", on_delete: :cascade
   add_foreign_key "vulnerability_occurrence_identifiers", "vulnerability_identifiers", column: "identifier_id", on_delete: :cascade
   add_foreign_key "vulnerability_occurrence_identifiers", "vulnerability_occurrences", column: "occurrence_id", on_delete: :cascade
   add_foreign_key "vulnerability_occurrence_pipelines", "ci_pipelines", column: "pipeline_id", on_delete: :cascade

ee/app/models/ee/issue.rb

@@ -37,6 +37,9 @@ module EE
       has_and_belongs_to_many :prometheus_alert_events, join_table: :issues_prometheus_alert_events
       has_many :prometheus_alerts, through: :prometheus_alert_events
 
+      has_many :vulnerability_links, class_name: 'Vulnerabilities::IssueLink', inverse_of: :issue
+      has_many :related_vulnerabilities, through: :vulnerability_links, source: :vulnerability
+
       validates :weight, allow_nil: true, numericality: { greater_than_or_equal_to: 0 }
 
       after_create :update_generic_alert_title, if: :generic_alert_with_default_title?

ee/app/models/vulnerabilities/issue_link.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  class IssueLink < ApplicationRecord
+    self.table_name = 'vulnerability_issue_links'
+
+    belongs_to :vulnerability
+    belongs_to :issue
+
+    enum link_type: { related: 1, created: 2 } # 'related' is the default value
+
+    validates :vulnerability, :issue, presence: true
+  end
+end

ee/app/models/vulnerability.rb

@@ -23,6 +23,8 @@ class Vulnerability < ApplicationRecord
   belongs_to :closed_by, class_name: 'User'
 
   has_many :findings, class_name: 'Vulnerabilities::Occurrence', inverse_of: :vulnerability
+  has_many :issue_links, class_name: 'Vulnerabilities::IssueLink', inverse_of: :vulnerability
+  has_many :related_issues, through: :issue_links, source: :issue
 
   enum state: { opened: 1, closed: 2, resolved: 3 }
   enum severity: Vulnerabilities::Occurrence::SEVERITY_LEVELS, _prefix: :severity

ee/spec/factories/vulnerabilities/issue_links.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :vulnerabilities_issue_link, class: Vulnerabilities::IssueLink do
+    vulnerability
+    issue
+
+    trait :created do
+      link_type { :created }
+    end
+
+    trait :related do
+      link_type { :related }
+    end
+  end
+end

ee/spec/models/issue_spec.rb

@@ -113,6 +113,8 @@ describe Issue do
     it { is_expected.to have_and_belong_to_many(:prometheus_alert_events) }
     it { is_expected.to have_and_belong_to_many(:self_managed_prometheus_alert_events) }
     it { is_expected.to have_many(:prometheus_alerts) }
+    it { is_expected.to have_many(:vulnerability_links).class_name('Vulnerabilities::IssueLink').inverse_of(:issue) }
+    it { is_expected.to have_many(:related_vulnerabilities).through(:vulnerability_links).source(:vulnerability) }
 
     describe 'versions.most_recent' do
       it 'returns the most recent version' do

ee/spec/models/vulnerabilities/issue_link_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Vulnerabilities::IssueLink do
+  describe 'associations and fields' do
+    it { is_expected.to belong_to(:vulnerability) }
+    it { is_expected.to belong_to(:issue) }
+    it { is_expected.to define_enum_for(:link_type).with_values(related: 1, created: 2) }
+
+    it 'provides the "related" as default link_type' do
+      expect(create(:vulnerabilities_issue_link).link_type).to eq 'related'
+    end
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:vulnerability) }
+    it { is_expected.to validate_presence_of(:issue) }
+  end
+
+  context 'when there is a link between the same vulnerability and issue' do
+    let!(:existing_link) { create(:vulnerabilities_issue_link) }
+
+    it 'raises the uniqueness violation error' do
+      expect do
+        create(:vulnerabilities_issue_link,
+          issue: existing_link.issue,
+          vulnerability: existing_link.vulnerability)
+      end.to raise_error(ActiveRecord::RecordNotUnique)
+    end
+  end
+
+  context 'when there is an existing "created" issue link for vulnerability' do
+    let!(:existing_link) { create(:vulnerabilities_issue_link, :created) }
+
+    it 'prevents the creation of a new "created" issue link' do
+      expect do
+        create(:vulnerabilities_issue_link,
+               :created,
+               vulnerability: existing_link.vulnerability,
+               issue: create(:issue))
+      end.to raise_error(ActiveRecord::RecordNotUnique)
+    end
+
+    it 'allows the creation of a new "related" issue link' do
+      expect do
+        create(:vulnerabilities_issue_link,
+               :related,
+               vulnerability: existing_link.vulnerability,
+               issue: create(:issue))
+      end.not_to raise_error
+    end
+  end
+end

ee/spec/models/vulnerability_spec.rb

@@ -28,6 +28,8 @@ describe Vulnerability do
     it { is_expected.to belong_to(:milestone) }
     it { is_expected.to belong_to(:epic) }
     it { is_expected.to have_many(:findings).class_name('Vulnerabilities::Occurrence').inverse_of(:vulnerability) }
+    it { is_expected.to have_many(:issue_links).class_name('Vulnerabilities::IssueLink').inverse_of(:vulnerability) }
+    it { is_expected.to have_many(:related_issues).through(:issue_links).source(:issue) }
     it { is_expected.to belong_to(:author).class_name('User') }
     it { is_expected.to belong_to(:updated_by).class_name('User') }
     it { is_expected.to belong_to(:last_edited_by).class_name('User') }

spec/lib/gitlab/import_export/all_models.yml

@@ -30,6 +30,8 @@ issues:
 - prometheus_alert_events
 - self_managed_prometheus_alert_events
 - zoom_meetings
+- vulnerability_links
+- related_vulnerabilities
 events:
 - author
 - project