Skip to content

Reduce denormalization of dast_site_tokens table

Philip Cunningham requested to merge philipcunningham-normalise-dast-site-tokens-341116 into master

What does this MR do and why?

we discovered we could end up with orphaned dast_site_tokens, so we added an application-level callback to ensure that no new orphaned records could be created. this merge request simplifies this by promoting the association between a dast_site_token and a dast_site to the database-level via a fk rather than a common, unique,url.

Related Issue(s)

Database

there are currently 212 dast_site_token and 895 dast_sites on gitlab.com.

Migrations

$ rails db:migrate:up VERSION=20211011040203 && rails db:migrate:up VERSION=20211011044147 && rails db:migrate:up VERSION=20211012000408 && rails db:migrate:up VERSION=20211012001017 && rails db:migrate:up VERSION=20211012004556
== 20211011040203 AddDastSiteIdToDastSiteTokens: migrating ====================
-- add_column(:dast_site_tokens, :dast_site_id, :bigint, {:null=>false, :default=>0})
   -> 0.0039s
== 20211011040203 AddDastSiteIdToDastSiteTokens: migrated (0.0039s) ===========

== 20211011044147 CleanUpOrphanedDastSiteTokens: migrating ====================
== 20211011044147 CleanUpOrphanedDastSiteTokens: migrated (0.0359s) ===========

== 20211012000408 RemoveDefaultFromDastSiteTokensDastSiteId: migrating ========
-- change_column_default(:dast_site_tokens, :dast_site_id, nil)
   -> 0.0039s
== 20211012000408 RemoveDefaultFromDastSiteTokensDastSiteId: migrated (0.0039s)

== 20211012001017 AddIndexAndFkToDastSiteTokensDastSiteId: migrating ==========
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:dast_site_tokens, :dast_site_id, {:name=>:index_dast_site_tokens_on_dast_site_id, :unique=>true, :algorithm=>:concurrently})
   -> 0.0035s
-- execute("SET statement_timeout TO 0")
   -> 0.0006s
-- add_index(:dast_site_tokens, :dast_site_id, {:name=>:index_dast_site_tokens_on_dast_site_id, :unique=>true, :algorithm=>:concurrently})
   -> 0.0050s
-- execute("RESET statement_timeout")
   -> 0.0006s
-- add_foreign_key(:dast_site_tokens, :dast_sites, {:on_delete=>:cascade, :validate=>false})
   -> 0.0047s
== 20211012001017 AddIndexAndFkToDastSiteTokensDastSiteId: migrated (0.0166s) =

== 20211012004556 DropNotNullConstraintFromUrlOnDastSiteTokens: migrating =====
-- change_column_null(:dast_site_tokens, :url, true)
   -> 0.0022s
== 20211012004556 DropNotNullConstraintFromUrlOnDastSiteTokens: migrated (0.0022s)
rails db:migrate:down VERSION=20211012004556 && rails db:migrate:down VERSION=20211012001017 && rails db:migrate:down VERSION=20211012000408 && rails db:migrate:down VERSION=20211011044147 && rails db:migrate:down VERSION=20211011040203
== 20211012004556 DropNotNullConstraintFromUrlOnDastSiteTokens: reverting =====
-- change_column_null(:dast_site_tokens, :url, false)
   -> 0.0024s
== 20211012004556 DropNotNullConstraintFromUrlOnDastSiteTokens: reverted (0.0041s)

== 20211012001017 AddIndexAndFkToDastSiteTokensDastSiteId: reverting ==========
-- foreign_keys(:dast_site_tokens)
   -> 0.0039s
-- remove_foreign_key(:dast_site_tokens, {:column=>:dast_site_id})
   -> 0.0043s
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:dast_site_tokens, :dast_site_id, {:name=>:index_dast_site_tokens_on_dast_site_id, :algorithm=>:concurrently})
   -> 0.0039s
-- execute("SET statement_timeout TO 0")
   -> 0.0007s
-- remove_index(:dast_site_tokens, {:name=>:index_dast_site_tokens_on_dast_site_id, :algorithm=>:concurrently, :column=>:dast_site_id})
   -> 0.0050s
-- execute("RESET statement_timeout")
   -> 0.0006s
== 20211012001017 AddIndexAndFkToDastSiteTokensDastSiteId: reverted (0.0206s) =

== 20211012000408 RemoveDefaultFromDastSiteTokensDastSiteId: reverting ========
-- change_column_default(:dast_site_tokens, :dast_site_id, 0)
   -> 0.0043s
== 20211012000408 RemoveDefaultFromDastSiteTokensDastSiteId: reverted (0.0044s)

== 20211011044147 CleanUpOrphanedDastSiteTokens: reverting ====================
== 20211011044147 CleanUpOrphanedDastSiteTokens: reverted (0.0000s) ===========

== 20211011040203 AddDastSiteIdToDastSiteTokens: reverting ====================
-- remove_column(:dast_site_tokens, :dast_site_id)
   -> 0.0024s
== 20211011040203 AddDastSiteIdToDastSiteTokens: reverted (0.0025s) ===========

Screenshots or screen recordings

example

Console

[15] pry(main)> validation = DastSiteValidation.find(222)
  DastSiteValidation Load (0.4ms)  SELECT "dast_site_validations".* FROM "dast_site_validations" WHERE "dast_site_validations"."id" = 222 LIMIT 1 /*application:console,db_config_name:main,line:/lib/gitlab/database/load_balancing/connection_proxy.rb:103:in `block in read_using_load_balancer'*/
=> #<DastSiteValidation:0x00007fe1a7edc368
 id: 222,
 dast_site_token_id: 111,
 created_at: Thu, 14 Oct 2021 02:36:12.781913000 UTC +00:00,
 updated_at: Thu, 14 Oct 2021 02:37:55.200085000 UTC +00:00,
 validation_started_at: Thu, 14 Oct 2021 02:36:33.566351000 UTC +00:00,
 validation_passed_at: nil,
 validation_failed_at: Thu, 14 Oct 2021 02:37:55.198366000 UTC +00:00,
 validation_last_retried_at: Thu, 14 Oct 2021 02:37:54.666518000 UTC +00:00,
 validation_strategy: "text_file",
 url_base: "https://d53425329c4917ed6776b75057001272.com:443",
 url_path: "GitLab-DAST-Site-Validation-2f6cf4db-f8ab-41ea-85f8-87b5bcbd5b18.txt",
 state: "failed">
[16] pry(main)> dast_site_token = validation.dast_site_token
  DastSiteToken Load (0.4ms)  SELECT "dast_site_tokens"."id", "dast_site_tokens"."project_id", "dast_site_tokens"."created_at", "dast_site_tokens"."updated_at", "dast_site_tokens"."expired_at", "dast_site_tokens"."token", "dast_site_tokens"."dast_site_id" FROM "dast_site_tokens" WHERE "dast_site_tokens"."id" = 111 LIMIT 1 /*application:console,db_config_name:main,line:/lib/gitlab/database/load_balancing/connection_proxy.rb:103:in `block in read_using_load_balancer'*/
=> #<DastSiteToken:0x00007fe1b5199058 id: 111, project_id: 9, created_at: Mon, 11 Oct 2021 22:04:00.565819000 UTC +00:00, updated_at: Mon, 11 Oct 2021 22:04:00.565819000 UTC +00:00, expired_at: nil, token: "[FILTERED]", dast_site_id: 201>
[17] pry(main)> dast_site_token.dast_site
  DastSite Load (0.6ms)  SELECT "dast_sites".* FROM "dast_sites" WHERE "dast_sites"."id" = 201 LIMIT 1 /*application:console,db_config_name:main,line:/lib/gitlab/database/load_balancing/connection_proxy.rb:103:in `block in read_using_load_balancer'*/
=> #<DastSite:0x00007fe1a7e91c50 id: 201, project_id: 9, created_at: Mon, 11 Oct 2021 22:04:00.528744000 UTC +00:00, updated_at: Thu, 14 Oct 2021 02:36:12.873871000 UTC +00:00, url: "https://d53425329c4917ed6776b75057001272.com", dast_site_validation_id: 222>

Analysis

Data Migration

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. start your gdk 
gdk start
  1. seed the database
bundle exec rake db:seed_fu FILTER=dast_profiles

  1. navigate to a project in the browser e.g. /root/air-gap

  2. navigate to list of site profiles e.g. /root/air-gap/-/security/configuration/dast_scans#site-profiles

  3. start validation

  4. navigate to pipelines view

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 Philip Cunningham

Merge request reports