Update vulnerability_reads trigger to set has_merge_request

What does this MR do and why?

We currently have postgres triggers on the vulnerability_merge_request_links table which set the has_merge_request column on a vulnerability_reads record when a vulnerability_merge_request_link is created or updated. However, vulnerability_reads records are only created when the vulnerability becomes present on the default branch. If a merge_request_link is created before the vulnerability becomes present on the default branch, then the vulnerability_merge_request_links triggers will do nothing since the vulnerability_reads record does not yet exist. To account for this case, we need to check if the vulnerability has merge_request links when the vulnerability_reads record is created, and set has_merge_request accordingly. vulnerability_reads records are created via postgres triggers on the vulnerabilities and vulnerability_occurrences tables.

This MR updates the existing triggers we already use for setting vulnerability_reads.has_issues to also set vulnerability_reads.has_merge_request and cover the non default branch scenario.

Also note that we chose to use triggers for this column has_merge_request alone as discussed in !128372 (comment 1509313269) and we have an issue to move-away from using triggers on a whole for the various application related logic implemented with triggers for vulnerability_reads table in Migrate `Vulnerability::Read` create / update l... (#393912 - closed)

Migrations

Up

$ bin/rails db:migrate:up:main VERSION=20230901170145
main: == [advisory_lock_connection] object_id: 226880, pg_backend_pid: 51664
main: == 20230901170145 UpdateVulnerabilityReadsTriggerToSetHasMergeRequest: migrating
main: -- execute("CREATE OR REPLACE FUNCTION insert_or_update_vulnerability_reads()\n  RETURNS trigger\n  LANGUAGE plpgsql\nAS $$\nDECLARE\n  severity smallint;\n  state smallint;\n  report_type smallint;\n  resolved_on_default_branch boolean;\n  present_on_default_branch boolean;\n  namespace_id bigint;\n  has_issues boolean;\n  has_merge_request boolean;\nBEGIN\n  IF (NEW.vulnerability_id IS NULL AND (TG_OP = 'INSERT' OR TG_OP = 'UPDATE')) THEN\n    RETURN NULL;\n  END IF;\n\n  IF (TG_OP = 'UPDATE' AND OLD.vulnerability_id IS NOT NULL AND NEW.vulnerability_id IS NOT NULL) THEN\n    RETURN NULL;\n  END IF;\n\n  SELECT\n    vulnerabilities.severity, vulnerabilities.state, vulnerabilities.report_type, vulnerabilities.resolved_on_default_branch, vulnerabilities.present_on_default_branch\n  INTO\n    severity, state, report_type, resolved_on_default_branch, present_on_default_branch\n  FROM\n    vulnerabilities\n  WHERE\n    vulnerabilities.id = NEW.vulnerability_id;\n\n  IF present_on_default_branch IS NOT true THEN\n    RETURN NULL;\n  END IF;\n\n  SELECT\n    projects.namespace_id\n  INTO\n    namespace_id\n  FROM\n    projects\n  WHERE\n    projects.id = NEW.project_id;\n\n  SELECT\n    EXISTS (SELECT 1 FROM vulnerability_issue_links WHERE vulnerability_issue_links.vulnerability_id = NEW.vulnerability_id)\n  INTO\n    has_issues;\n\n  SELECT\n    EXISTS (SELECT 1 FROM vulnerability_merge_request_links WHERE vulnerability_merge_request_links.vulnerability_id = NEW.vulnerability_id)\n  INTO\n    has_merge_request;\n\n  INSERT INTO vulnerability_reads (vulnerability_id, namespace_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues, has_merge_request)\n    VALUES (NEW.vulnerability_id, namespace_id, NEW.project_id, NEW.scanner_id, report_type, severity, state, resolved_on_default_branch, NEW.uuid::uuid, NEW.location->>'image', NEW.location->'kubernetes_resource'->>'agent_id', CAST(NEW.location->'kubernetes_resource'->>'agent_id' AS bigint), has_issues, has_merge_request)\n    ON CONFLICT(vulnerability_id) DO NOTHING;\n  RETURN NULL;\nEND\n$$\n")
main:    -> 0.0019s
main: -- execute("CREATE OR REPLACE FUNCTION insert_vulnerability_reads_from_vulnerability()\n  RETURNS trigger\n  LANGUAGE plpgsql\nAS $$\nDECLARE\n  scanner_id bigint;\n  uuid uuid;\n  location_image text;\n  cluster_agent_id text;\n  casted_cluster_agent_id bigint;\n  namespace_id bigint;\n  has_issues boolean;\n  has_merge_request boolean;\nBEGIN\n  SELECT\n    v_o.scanner_id, v_o.uuid, v_o.location->>'image', v_o.location->'kubernetes_resource'->>'agent_id', CAST(v_o.location->'kubernetes_resource'->>'agent_id' AS bigint), projects.namespace_id\n  INTO\n    scanner_id, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, namespace_id\n  FROM\n    vulnerability_occurrences v_o\n  INNER JOIN projects ON projects.id = v_o.project_id\n  WHERE\n    v_o.vulnerability_id = NEW.id\n  LIMIT 1;\n\n  SELECT\n    EXISTS (SELECT 1 FROM vulnerability_issue_links WHERE vulnerability_issue_links.vulnerability_id = NEW.id)\n  INTO\n    has_issues;\n\n  SELECT\n    EXISTS (SELECT 1 FROM vulnerability_merge_request_links WHERE vulnerability_merge_request_links.vulnerability_id = NEW.id)\n  INTO\n    has_merge_request;\n\n  INSERT INTO vulnerability_reads (vulnerability_id, namespace_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues, has_merge_request)\n    VALUES (NEW.id, namespace_id, NEW.project_id, scanner_id, NEW.report_type, NEW.severity, NEW.state, NEW.resolved_on_default_branch, uuid::uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues, has_merge_request)\n    ON CONFLICT(vulnerability_id) DO NOTHING;\n  RETURN NULL;\nEND\n$$\n")
main:    -> 0.0004s
main: == 20230901170145 UpdateVulnerabilityReadsTriggerToSetHasMergeRequest: migrated (0.0054s)

main: == [advisory_lock_connection] object_id: 226880, pg_backend_pid: 51664

Down

$ bin/rails db:migrate:down:main VERSION=20230901170145
main: == [advisory_lock_connection] object_id: 226940, pg_backend_pid: 52204
main: == 20230901170145 UpdateVulnerabilityReadsTriggerToSetHasMergeRequest: reverting
main: -- execute("CREATE OR REPLACE FUNCTION insert_or_update_vulnerability_reads()\n  RETURNS trigger\n  LANGUAGE plpgsql\nAS $$\nDECLARE\n  severity smallint;\n  state smallint;\n  report_type smallint;\n  resolved_on_default_branch boolean;\n  present_on_default_branch boolean;\n  namespace_id bigint;\n  has_issues boolean;\nBEGIN\n  IF (NEW.vulnerability_id IS NULL AND (TG_OP = 'INSERT' OR TG_OP = 'UPDATE')) THEN\n    RETURN NULL;\n  END IF;\n\n  IF (TG_OP = 'UPDATE' AND OLD.vulnerability_id IS NOT NULL AND NEW.vulnerability_id IS NOT NULL) THEN\n    RETURN NULL;\n  END IF;\n\n  SELECT\n    vulnerabilities.severity, vulnerabilities.state, vulnerabilities.report_type, vulnerabilities.resolved_on_default_branch, vulnerabilities.present_on_default_branch\n  INTO\n    severity, state, report_type, resolved_on_default_branch, present_on_default_branch\n  FROM\n    vulnerabilities\n  WHERE\n    vulnerabilities.id = NEW.vulnerability_id;\n\n  IF present_on_default_branch IS NOT true THEN\n    RETURN NULL;\n  END IF;\n\n  SELECT\n    projects.namespace_id\n  INTO\n    namespace_id\n  FROM\n    projects\n  WHERE\n    projects.id = NEW.project_id;\n\n  SELECT\n    EXISTS (SELECT 1 FROM vulnerability_issue_links WHERE vulnerability_issue_links.vulnerability_id = NEW.vulnerability_id)\n  INTO\n    has_issues;\n\n  INSERT INTO vulnerability_reads (vulnerability_id, namespace_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues)\n    VALUES (NEW.vulnerability_id, namespace_id, NEW.project_id, NEW.scanner_id, report_type, severity, state, resolved_on_default_branch, NEW.uuid::uuid, NEW.location->>'image', NEW.location->'kubernetes_resource'->>'agent_id', CAST(NEW.location->'kubernetes_resource'->>'agent_id' AS bigint), has_issues)\n    ON CONFLICT(vulnerability_id) DO NOTHING;\n  RETURN NULL;\nEND\n$$\n")
main:    -> 0.0021s
main: -- execute("CREATE OR REPLACE FUNCTION insert_vulnerability_reads_from_vulnerability()\n  RETURNS trigger\n  LANGUAGE plpgsql\nAS $$\nDECLARE\n  scanner_id bigint;\n  uuid uuid;\n  location_image text;\n  cluster_agent_id text;\n  casted_cluster_agent_id bigint;\n  namespace_id bigint;\n  has_issues boolean;\nBEGIN\n  SELECT\n    v_o.scanner_id, v_o.uuid, v_o.location->>'image', v_o.location->'kubernetes_resource'->>'agent_id', CAST(v_o.location->'kubernetes_resource'->>'agent_id' AS bigint), projects.namespace_id\n  INTO\n    scanner_id, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, namespace_id\n  FROM\n    vulnerability_occurrences v_o\n  INNER JOIN projects ON projects.id = v_o.project_id\n  WHERE\n    v_o.vulnerability_id = NEW.id\n  LIMIT 1;\n\n  SELECT\n    EXISTS (SELECT 1 FROM vulnerability_issue_links WHERE vulnerability_issue_links.vulnerability_id = NEW.id)\n  INTO\n    has_issues;\n\n  INSERT INTO vulnerability_reads (vulnerability_id, namespace_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues)\n    VALUES (NEW.id, namespace_id, NEW.project_id, scanner_id, NEW.report_type, NEW.severity, NEW.state, NEW.resolved_on_default_branch, uuid::uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues)\n    ON CONFLICT(vulnerability_id) DO NOTHING;\n  RETURN NULL;\nEND\n$$\n")
main:    -> 0.0005s
main: == 20230901170145 UpdateVulnerabilityReadsTriggerToSetHasMergeRequest: reverted (0.0058s)

main: == [advisory_lock_connection] object_id: 226940, pg_backend_pid: 52204

How to set up and validate locally

Because of this Bug with create MergeRequest action on a non de... (#421428 - closed) it is currently not possible to test this directly in UI and we do not want any surprises when the bug is addressed likely with !130806 (merged) (inprogress).

To test we can modify code with the proposed fix and create a Merge Request for a vulnerability on a non-default branch:

1. Apply the patch in local

--- a/ee/app/services/vulnerabilities/security_finding/create_merge_request_service.rb
+++ b/ee/app/services/vulnerabilities/security_finding/create_merge_request_service.rb
@@ -45,7 +45,7 @@ def find_or_create_vulnerability
           Vulnerabilities::FindOrCreateFromSecurityFindingService
             .new(project: project, current_user: current_user, params: {
               security_finding_uuid: params[:security_finding].uuid
-            }, state: 'confirmed')
+            }, state: 'confirmed', present_on_default_branch: false)
         ).payload[:vulnerability]
       end

2. Clone https://gitlab.com/gitlab-org/govern/threat-insights-demos/issue-390071-verification/ and run pipeline for branch remediate/test-vulnerability-1-D20230321T163025.

3. Goto the pipeline security tab and click on the first vulnerability and click resolve with MergeRequest button.

4. Then merge branch remediate/test-vulnerability-1-D20230321T163025 into main. Now the vulnerability_reads latest record should have has_merge_request value set for it.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

• I have evaluated the MR acceptance checklist for this MR.

Related to #421736 (closed)