Clean up DastSite on DastSiteProfile destroy (!38653) · Merge requests · GitLab.org / GitLab

What does this MR do?

Problem

when a dast_site_profile is destroyed it may end up leaving around a dangling dast_site. over time we could end up with many dangling dast_sites.

Related Issue(s)

#233868 (closed)

Proposed Solution

i've naively added an after_destroy hook to clean up associated dast_sites when the last dast_site_profile association is destroyed but i'm open to feedback about alternative approaches or ways what i've got here can be improved.

Context

dast_sites and dast_site_profiles are in separate tables in order to allow us to share site validation between profiles. right now there's no ability to list individual dast_sites in the ui, so i want to avoid creating orphan records if possible.

Notes

note that the delete mutation is behind a feature flag, so no changelog entry is necessary.

Schema

`dast_site_profiles`

belongs_to :project
belongs_to :dast_site

                                        Table "public.dast_site_profiles"
    Column    |           Type           | Collation | Nullable |                    Default
--------------+--------------------------+-----------+----------+------------------------------------------------
 id           | bigint                   |           | not null | nextval('dast_site_profiles_id_seq'::regclass)
 project_id   | bigint                   |           | not null |
 dast_site_id | bigint                   |           | not null |
 created_at   | timestamp with time zone |           | not null |
 updated_at   | timestamp with time zone |           | not null |
 name         | text                     |           | not null |
Indexes:
    "dast_site_profiles_pkey" PRIMARY KEY, btree (id)
    "index_dast_site_profiles_on_project_id_and_name" UNIQUE, btree (project_id, name)
    "index_dast_site_profiles_on_dast_site_id" btree (dast_site_id)
Check constraints:
    "check_6cfab17b48" CHECK (char_length(name) <= 255)
Foreign-key constraints:
    "fk_rails_747dc64abc" FOREIGN KEY (dast_site_id) REFERENCES dast_sites(id) ON DELETE CASCADE
    "fk_rails_83e309d69e" FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE

`dast_sites`

belongs_to :project
has_many :dast_site_profiles

                                       Table "public.dast_sites"
   Column   |           Type           | Collation | Nullable |                Default
------------+--------------------------+-----------+----------+----------------------------------------
 id         | bigint                   |           | not null | nextval('dast_sites_id_seq'::regclass)
 project_id | bigint                   |           | not null |
 created_at | timestamp with time zone |           | not null |
 updated_at | timestamp with time zone |           | not null |
 url        | text                     |           | not null |
Indexes:
    "dast_sites_pkey" PRIMARY KEY, btree (id)
    "index_dast_sites_on_project_id_and_url" UNIQUE, btree (project_id, url)
Check constraints:
    "check_46df8b449c" CHECK (char_length(url) <= 255)
Foreign-key constraints:
    "fk_rails_6febb6ea9c" FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE
Referenced by:
    TABLE "dast_site_profiles" CONSTRAINT "fk_rails_747dc64abc" FOREIGN KEY (dast_site_id) REFERENCES dast_sites(id) ON DELETE CASCADE

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
Tested in all supported browsers
Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

Label as security and @ mention @gitlab-com/gl-security/appsec
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
Security reports checked/validated by a reviewer from the AppSec team

Clean up DastSite on DastSiteProfile destroy