Skip to content

Do not create `vulnerability_reads` records for vulnerabilities that do not exist on default branch

Mehmet Emin INAC requested to merge 324858_dont_create_vulnerability_reads_if_the_vulnerability_does_not_exist_on_default_branch into master

What does this MR do and why?

Updates the existing trigger function to skip creating a vulnerability_reads record if the vulnerability does not exist on the default branch and introduces a new trigger function to create a vulnerability_reads record if a vulnerability gets updated as present_on_default_branch.

Related to Change Vulnerability Report to show Vulnerabili... (#324858 - closed).

Database review

DDL changes

This MR updates an existing trigger function and introduces a new one.

Updating the existing trigger function
rake db:migrate:up 
main: == 20220712025712 UpdateInsertOrUpdateVulnerabilityReadsFunction: migrating ===
main: -- execute("CREATE OR REPLACE FUNCTION insert_or_update_vulnerability_reads() RETURNS trigger\n    LANGUAGE plpgsql\n    AS $$\nDECLARE\n  severity smallint;\n  state smallint;\n  report_type smallint;\n  resolved_on_default_branch boolean;\n  present_on_default_branch 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  INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id)\n    VALUES (NEW.vulnerability_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))\n    ON CONFLICT(vulnerability_id) DO NOTHING;\n  RETURN NULL;\nEND\n$$;\n")
main:    -> 0.0093s
main: == 20220712025712 UpdateInsertOrUpdateVulnerabilityReadsFunction: migrated (0.0119s)
rake db:migrate:down 
main: == 20220712025712 UpdateInsertOrUpdateVulnerabilityReadsFunction: reverting ===
main: -- execute("CREATE OR REPLACE FUNCTION insert_or_update_vulnerability_reads() RETURNS trigger\n    LANGUAGE plpgsql\n    AS $$\nDECLARE\n  severity smallint;\n  state smallint;\n  report_type smallint;\n  resolved_on_default_branch 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\n  INTO\n    severity, state, report_type, resolved_on_default_branch\n  FROM\n     vulnerabilities\n  WHERE\n    vulnerabilities.id = NEW.vulnerability_id;\n\n  INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id)\n    VALUES (NEW.vulnerability_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))\n    ON CONFLICT(vulnerability_id) DO NOTHING;\n  RETURN NULL;\nEND\n$$;\n")
main:    -> 0.0218s
main: == 20220712025712 UpdateInsertOrUpdateVulnerabilityReadsFunction: reverted (0.0270s)
The new trigger function
rake db:migrate:up 
main: == 20220712031923 CreateVulnerabilityReadsForAnExistingVulnerabilityRecord: migrating
main: -- execute("CREATE FUNCTION insert_vulnerability_reads_from_vulnerability() RETURNS trigger\n    LANGUAGE plpgsql\n    AS $$\nDECLARE\n  scanner_id bigint;\n  uuid uuid;\n  location_image text;\n  cluster_agent_id text;\n  casted_cluster_agent_id bigint;\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)\n  INTO\n    scanner_id, uuid, location_image, cluster_agent_id, casted_cluster_agent_id\n  FROM\n     vulnerability_occurrences v_o\n  WHERE\n    v_o.vulnerability_id = NEW.id\n  LIMIT 1;\n\n  INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id)\n    VALUES (NEW.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)\n    ON CONFLICT(vulnerability_id) DO NOTHING;\n  RETURN NULL;\nEND\n$$;\n")
main:    -> 0.0223s
main: -- execute("CREATE TRIGGER trigger_insert_vulnerability_reads_from_vulnerability\nAFTER UPDATE ON vulnerabilities\nFOR EACH ROW\nWHEN (\n  OLD.present_on_default_branch IS NOT true AND NEW.present_on_default_branch IS true\n)\nEXECUTE PROCEDURE insert_vulnerability_reads_from_vulnerability();\n")
main:    -> 0.0040s
main: == 20220712031923 CreateVulnerabilityReadsForAnExistingVulnerabilityRecord: migrated (0.0305s)
rake db:migrate:down 
main: == 20220712031923 CreateVulnerabilityReadsForAnExistingVulnerabilityRecord: reverting
main: -- execute("DROP TRIGGER IF EXISTS trigger_insert_vulnerability_reads_from_vulnerability ON vulnerabilities")
main:    -> 0.0062s
main: -- execute("DROP FUNCTION IF EXISTS insert_vulnerability_reads_from_vulnerability()")
main:    -> 0.0013s
main: == 20220712031923 CreateVulnerabilityReadsForAnExistingVulnerabilityRecord: reverted (0.0117s)
Updating the existing trigger called `trigger_update_vulnerability_reads_on_vulnerability_update`
rake db:migrate:up 
main: == 20220714142424 UpdateTriggerUpdateVulnerabilityReadsOnVulnerabilityUpdate: migrating
main: -- execute("DROP TRIGGER IF EXISTS trigger_update_vulnerability_reads_on_vulnerability_update ON vulnerabilities")
main:    -> 0.0038s
main: -- execute("CREATE TRIGGER trigger_update_vulnerability_reads_on_vulnerability_update\nAFTER UPDATE ON vulnerabilities\nFOR EACH ROW\nWHEN (\n  OLD.present_on_default_branch IS TRUE AND\n  (\n    OLD.severity IS DISTINCT FROM NEW.severity OR\n    OLD.state IS DISTINCT FROM NEW.state OR\n    OLD.resolved_on_default_branch IS DISTINCT FROM NEW.resolved_on_default_branch\n  )\n)\nEXECUTE PROCEDURE update_vulnerability_reads_from_vulnerability();\n")
main:    -> 0.0020s
main: == 20220714142424 UpdateTriggerUpdateVulnerabilityReadsOnVulnerabilityUpdate: migrated (0.0081s)
rake db:migrate:down 
main: == 20220714142424 UpdateTriggerUpdateVulnerabilityReadsOnVulnerabilityUpdate: reverting
main: -- execute("DROP TRIGGER IF EXISTS trigger_update_vulnerability_reads_on_vulnerability_update ON vulnerabilities")
main:    -> 0.0032s
main: -- execute("CREATE TRIGGER trigger_update_vulnerability_reads_on_vulnerability_update\nAFTER UPDATE ON vulnerabilities\nFOR EACH ROW\nWHEN (\n  OLD.severity IS DISTINCT FROM NEW.severity OR\n  OLD.state IS DISTINCT FROM NEW.state OR\n  OLD.resolved_on_default_branch IS DISTINCT FROM NEW.resolved_on_default_branch\n)\nEXECUTE PROCEDURE update_vulnerability_reads_from_vulnerability();\n")
main:    -> 0.0015s
main: == 20220714142424 UpdateTriggerUpdateVulnerabilityReadsOnVulnerabilityUpdate: reverted (0.0069s)

Query plans

The new trigger uses 2 queries;

Select attributes of `vulnerability_occurrences` into variables

The query used in trigger;

SELECT
  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)
INTO
  scanner_id, uuid, location_image, cluster_agent_id, casted_cluster_agent_id
FROM
   vulnerability_occurrences v_o
WHERE
  v_o.vulnerability_id = NEW.id
LIMIT 1;

Similar query to check the execution plan;

SELECT
  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)
FROM
   vulnerability_occurrences v_o
WHERE
  v_o.vulnerability_id = 1
LIMIT 1;
 Limit  (cost=0.56..3.60 rows=1 width=117) (actual time=0.101..0.101 rows=1 loops=1)
   Buffers: shared hit=8
   I/O Timings: read=0.000 write=0.000
   ->  Index Scan using index_vulnerability_occurrences_on_vulnerability_id on public.vulnerability_occurrences v_o  (cost=0.56..3.60 rows=1 width=117) (actual time=0.099..0.099 rows=1 loops=1)
         Index Cond: (v_o.vulnerability_id = 1)
         Buffers: shared hit=8
         I/O Timings: read=0.000 write=0.000

https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/11046/commands/39575

Insert into `vulnerability_reads` 
 ModifyTable on public.vulnerability_reads  (cost=0.00..0.01 rows=1 width=128) (actual time=8.484..8.485 rows=0 loops=1)
   Buffers: shared hit=1 read=6
   I/O Timings: read=8.371 write=0.000
   ->  Result  (cost=0.00..0.01 rows=1 width=128) (actual time=0.010..0.011 rows=1 loops=1)
         Buffers: shared hit=1
         I/O Timings: read=0.000 write=0.000

https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/11046/commands/39581

⚠ Incident Mitigation

Rollback the DDL changes introduced by this MR;

rake db:migrate:down:main VERSION=20220714142424
rake db:migrate:down:main VERSION=20220712031923
rake db:migrate:down:main VERSION=20220712025712

And do the same for the CI database.

MR acceptance checklist

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

Edited by Mehmet Emin INAC

Merge request reports