Skip to content

Add FK for deployments.environment_id to environments table

Shinya Maeda requested to merge add-fk-to-deployments-environment-id into master

What does this MR do?

There is a data integrity issue on deployments table that deployments.environment_id doesn't have a foreign key, so there are some orphaned deployments reported.

This MR adds the foreign key to stop creating orphaned deployments. This is long overdue.

FYI, the environment_id is indexed and has NOT NULL constraint already.

Related #26229 (closed)

Migration test on db-lab

exec ALTER TABLE deployments
ADD CONSTRAINT fk_009fd21147
FOREIGN KEY (environment_id)
REFERENCES environments (id)
ON DELETE CASCADE
NOT VALID;

The query has been executed. Duration: 31.715 ms (edited)

https://gitlab.slack.com/archives/CLJMDRD8C/p1624515683213100

The following query - the validation on all rows, won't be performed due to the orphaned rows. See !64585 (comment 609380270) for more info.

exec ALTER TABLE deployments VALIDATE CONSTRAINT fk_009fd21147;

Migration test on local environment

shinya@shinya-B550-VISION-D:~/workspace/thin-gdk/services/rails/src$ tre bin/rails db:migrate:redo VERSION=20210622135221
INFO: This script is a predefined script in devkitkat.
== 20210622135221 AddForeignKeyForEnvironmentIdToEnvironments: reverting ======
-- foreign_keys(:deployments)
   -> 0.0022s
-- remove_foreign_key(:deployments, :environments)
   -> 0.0033s
== 20210622135221 AddForeignKeyForEnvironmentIdToEnvironments: reverted (0.0094s) 

== 20210622135221 AddForeignKeyForEnvironmentIdToEnvironments: migrating ======
-- transaction_open?()
   -> 0.0000s
-- foreign_keys(:deployments)
   -> 0.0022s
-- execute("ALTER TABLE deployments\nADD CONSTRAINT fk_009fd21147\nFOREIGN KEY (environment_id)\nREFERENCES environments (id)\nON DELETE CASCADE\nNOT VALID;\n")
   -> 0.0039s
== 20210622135221 AddForeignKeyForEnvironmentIdToEnvironments: migrated (0.0094s) 

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • 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
Edited by Shinya Maeda

Merge request reports