Skip to content

Deleted `vulnerability_remediations` leave behind `uploads` records

Michael Becker requested to merge 462724-deleted-vulnerability_remediations-records-leave-behind-uploads-records into master

What does this MR do and why?

During Clean up orphaned remediations (#378269 - closed) • Michael Becker • 16.2, we added a cron worker that deletes orphaned remediations from the database. An "orphaned remediation" is a remediation that is no longer associated with a vulnerability_occurrence (aka finding).

Because of the use of delete_all rather than destroy_all, we ended up with orphaned Uploads instead of orphaned remediations.

This change moves the deletion logic to a new service. This service:

  1. iterates over orphaned remediations in batches
  2. for each batch, finds all Uploads associated with it and calls the "fast_destroy" methods to remove the associated files
  3. then calls delete_all on the batch of remediations

The specs for this new service have been expanded to also assert the Uploads are being deleted

Note

This change only prevents new orphaned Uploads going forward. Cleaning those up will likely need to be done in a migration in a follow-up

SQL

each_batch Getting bounds for a batch 
/* lower bound */
SELECT
    "vulnerability_remediations"."id"
FROM
    "vulnerability_remediations"
    LEFT OUTER JOIN "vulnerability_findings_remediations" ON "vulnerability_findings_remediations"."vulnerability_remediation_id" = "vulnerability_remediations"."id"
WHERE
    "vulnerability_findings_remediations"."id" IS NULL
ORDER BY
    "vulnerability_remediations"."id" ASC
LIMIT 1;

SELECT
    "vulnerability_remediations"."id"
FROM
    "vulnerability_remediations"
    LEFT OUTER JOIN "vulnerability_findings_remediations" ON "vulnerability_findings_remediations"."vulnerability_remediation_id" = "vulnerability_remediations"."id"
WHERE
    "vulnerability_findings_remediations"."id" IS NULL
ORDER BY
    "vulnerability_remediations"."id" ASC
LIMIT 1 OFFSET 1000;

original query

SELECT
    "vulnerability_remediations"."id"
FROM
    "vulnerability_remediations"
    LEFT OUTER JOIN "vulnerability_findings_remediations" ON "vulnerability_findings_remediations"."vulnerability_remediation_id" = "vulnerability_remediations"."id"
WHERE
    "vulnerability_findings_remediations"."id" IS NULL
LIMIT 1000;

uploads query

SELECT
    "uploads".*
FROM
    "uploads"
WHERE
    "uploads"."model_type" = 'Vulnerabilities::Remediation'
    AND "uploads"."model_id" IN (
        SELECT
            "vulnerability_remediations"."id"
        FROM
            "vulnerability_remediations"
        LEFT OUTER JOIN "vulnerability_findings_remediations" ON "vulnerability_findings_remediations"."vulnerability_remediation_id" = "vulnerability_remediations"."id"
WHERE
    "vulnerability_findings_remediations"."id" IS NULL
LIMIT 1000)
AND "uploads"."uploader" = 'AttachmentUploader'
AND "uploads"."store" = 1
ORDER BY
    "uploads"."id" ASC;

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

local testing steps

  1. in a separate terminal window, tail the sidekiq logs

     tail -f log/sidekiq*| grep --line-buffered 'Vulnerabilities::OrphanedRemediationsCleanupWorker' | jq -cR 'fromjson?'|jq

  2. use FactoryBot to create a bunch of remediations

    # with_findings
FactoryBot.create_list(:vulnerabilities_remediation, 4, findings: FactoryBot.create_list(:vulnerabilities_finding, 1));nil
# without_findings
FactoryBot.create_list(:vulnerabilities_remediation, 4, findings: [], project: Project.first);nil

Vulnerabilities::Remediation.count

# => 8

  3. modify the cron schedule to run every minute

    diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index befbe06033b8..6e66be06b9ad 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -812 +812 @@
-  Settings.cron_jobs['vulnerability_orphaned_remediations_cleanup_worker']['cron'] ||= '15 3 */3 * *'
+  Settings.cron_jobs['vulnerability_orphaned_remediations_cleanup_worker']['cron'] ||= '*/1 * * * *'

  4. gdk restart

  5. wait for the job to run, you should see logs like the following:

    Click to expand 
    {
  "severity": "INFO",
  "time": "2023-07-11T22:21:02.288Z",
  "retry": 0,
  "queue": "default",
  "backtrace": true,
  "version": 0,
  "queue_namespace": "cronjob",
  "args": [],
  "class": "Vulnerabilities::OrphanedRemediationsCleanupWorker",
  "jid": "82ac3276385963276c6719f2",
  "created_at": "2023-07-11T22:21:02.274Z",
  "meta.caller_id": "Cronjob",
  "correlation_id": "3587ca2199e15da531670dbc5b468fb5",
  "meta.root_caller_id": "Cronjob",
  "meta.feature_category": "vulnerability_management",
  "worker_data_consistency": "always",
  "idempotency_key": "resque:gitlab:duplicate:default:e28051869e793708bf6d7f453756748b3a630753ba8c4a71b23a17d4b48c80bb",
  "size_limiter": "validated",
  "enqueued_at": "2023-07-11T22:21:02.275Z",
  "job_size_bytes": 2,
  "pid": 482581,
  "message": "Vulnerabilities::OrphanedRemediationsCleanupWorker JID-82ac3276385963276c6719f2: start",
  "job_status": "start",
  "scheduling_latency_s": 0.013081
}
{
  "severity": "INFO",
  "time": "2023-07-11T22:21:02.405Z",
  "retry": 0,
  "queue": "default",
  "backtrace": true,
  "version": 0,
  "queue_namespace": "cronjob",
  "args": [],
  "class": "Vulnerabilities::OrphanedRemediationsCleanupWorker",
  "jid": "82ac3276385963276c6719f2",
  "created_at": "2023-07-11T22:21:02.274Z",
  "meta.caller_id": "Cronjob",
  "correlation_id": "3587ca2199e15da531670dbc5b468fb5",
  "meta.root_caller_id": "Cronjob",
  "meta.feature_category": "vulnerability_management",
  "worker_data_consistency": "always",
  "idempotency_key": "resque:gitlab:duplicate:default:e28051869e793708bf6d7f453756748b3a630753ba8c4a71b23a17d4b48c80bb",
  "size_limiter": "validated",
  "enqueued_at": "2023-07-11T22:21:02.275Z",
  "job_size_bytes": 2,
  "pid": 482581,
  "message": "Vulnerabilities::OrphanedRemediationsCleanupWorker JID-82ac3276385963276c6719f2: done: 0.117322 sec",
  "job_status": "done",
  "scheduling_latency_s": 0.013081,
  "redis_calls": 2,
  "redis_duration_s": 0.00131,
  "redis_read_bytes": 2,
  "redis_write_bytes": 186,
  "redis_queues_calls": 2,
  "redis_queues_duration_s": 0.00131,
  "redis_queues_read_bytes": 2,
  "redis_queues_write_bytes": 186,
  "db_count": 1,
  "db_write_count": 1,
  "db_cached_count": 0,
  "db_replica_count": 0,
  "db_primary_count": 1,
  "db_main_count": 1,
  "db_ci_count": 0,
  "db_main_replica_count": 0,
  "db_ci_replica_count": 0,
  "db_replica_cached_count": 0,
  "db_primary_cached_count": 0,
  "db_main_cached_count": 0,
  "db_ci_cached_count": 0,
  "db_main_replica_cached_count": 0,
  "db_ci_replica_cached_count": 0,
  "db_replica_wal_count": 0,
  "db_primary_wal_count": 0,
  "db_main_wal_count": 0,
  "db_ci_wal_count": 0,
  "db_main_replica_wal_count": 0,
  "db_ci_replica_wal_count": 0,
  "db_replica_wal_cached_count": 0,
  "db_primary_wal_cached_count": 0,
  "db_main_wal_cached_count": 0,
  "db_ci_wal_cached_count": 0,
  "db_main_replica_wal_cached_count": 0,
  "db_ci_replica_wal_cached_count": 0,
  "db_replica_duration_s": 0,
  "db_primary_duration_s": 0.003,
  "db_main_duration_s": 0.003,
  "db_ci_duration_s": 0,
  "db_main_replica_duration_s": 0,
  "db_ci_replica_duration_s": 0,
  "cpu_s": 0.003803,
  "worker_id": "sidekiq_0",
  "rate_limiting_gates": [],
  "duration_s": 0.117322,
  "completed_at": "2023-07-11T22:21:02.405Z",
  "load_balancing_strategy": "primary",
  "db_duration_s": 0.001666,
  "urgency": "low",
  "target_duration_s": 300
}
{
  "severity": "INFO",
  "time": "2023-07-11T22:22:05.451Z",
  "retry": 0,
  "queue": "default",
  "backtrace": true,
  "version": 0,
  "queue_namespace": "cronjob",
  "args": [],
  "class": "Vulnerabilities::OrphanedRemediationsCleanupWorker",
  "jid": "f48afdaa29493f6851f60bfd",
  "created_at": "2023-07-11T22:22:05.438Z",
  "meta.caller_id": "Cronjob",
  "correlation_id": "f2f5b759df6e60f76add1a3c29c41c48",
  "meta.root_caller_id": "Cronjob",
  "meta.feature_category": "vulnerability_management",
  "worker_data_consistency": "always",
  "idempotency_key": "resque:gitlab:duplicate:default:e28051869e793708bf6d7f453756748b3a630753ba8c4a71b23a17d4b48c80bb",
  "size_limiter": "validated",
  "enqueued_at": "2023-07-11T22:22:05.439Z",
  "job_size_bytes": 2,
  "pid": 482581,
  "message": "Vulnerabilities::OrphanedRemediationsCleanupWorker JID-f48afdaa29493f6851f60bfd: start",
  "job_status": "start",
  "scheduling_latency_s": 0.012039
}

  6. see if all the remediations you created are deleted

        Vulnerabilities::Remediation.count
    # => 4
    Upload.where(model_type: Vulnerabilities::Remediation).count
    # => 4

Related to #462724 (closed)

Changelog: fixed
EE: true

Edited by Michael Becker

Merge request reports