Rails: Enforce not null constraints for workspace personal_access_token_id foreign key

MR: !132026 (merged)

Description

Related issues:

• Rails: DB migrations and model class updates fo... (#421505 - closed)
• Resolve how Workspaces should handle deletion o... (#414384 - closed)

Placeholder summary: We need to decide how to handle foreign keys on workspaces table, when the associated record (personal access token, user, project, etc) have been deleted. We are proposing to create "placeholder" or "ghost" records with a known ID to link to. This is a standard pattern, for example when scrubbing PII data for a GDPR request, but we need to research how this is handled in GitLab and follow any existing rules/precedent.

Context

With the new changes for private repository support in workspaces, every time a user creates a workspaces, a personal access token(PAT) scoped to the user is created for the lifetime of the workspace(maximum 7 days for now). We will be storing a reference to this PAT as a foreign key in the workspaces table to be able to "revoke" in case the user decides to terminate the workspace before the expiry time.

As described in the comments below, we have the following foreign key constraints already on the workspaces table -

Foreign-key constraints:
    "fk_bdb0b31131" FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE
    "fk_dc7c316be1" FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE
    "fk_f78aeddc77" FOREIGN KEY (cluster_agent_id) REFERENCES cluster_agents(id) ON DELETE CASCADE
Access method: heap

workspaces table will now have a personal_access_token_id foreign key. This issue is about how we handle the personal_access_token_id column value for existing workspaces.

Solution

• Add personal_access_token_id column to the workspaces table. Allow NULL values. - Addressed in Add workspace variables table and add PAT to wo... (!129688 - merged)
• For all existing workspaces with a NULL PAT ID, create a PAT for the user who owns the workspace and add the reference in personal_access_token_id column value. These PATs would already be expired. Since we already have a cascade delete for the user_id` column, there will never be a workspace record whose user does not exist. Addressed in Rails: Backfill existing workspaces with PATs (#423006 - closed)
• Add NOT NULL constraints to the personal_access_token_id column. - This issue.
• It is not a security concern as per comments here.
• Pros:
  • Strong integrity constraints(NOT NULL) are enforced at the DB level.
  • Other data integrity checks are also maintained(PAT belongs to the user who owns the workspace).
• Cons:
  • It becomes a more involved migration process. However, we are okay with it because of the benefits it provides. It also reduces the tech debt we will introduce with having some complex conditions around NULL constraints checks at model/DB level. We would like to feel the pain right now rather than delay it and feel it later.

Other solutions considered

Solution 1

• New records will have a value in personal_access_token_id. Existing records will have it as NULL
• Cons:
  • Unable to enforce NOT NULL constraint at the database level. This can be mitigated by introducing a secondary column(config_to_apply_generator_version) whose value for existing records would be 1 and for new records would be 2 and then creating a constraint at DB level saying that for records with config_to_apply_generator_version greater than 1, personal_access_token_id cannot be NULL.

Solution 2

As part of DB migration

• Create a ghost user and a ghost personal access token for the said ghost user.
• For all existing workspaces, associate the ghost user's PAT id for the personal_access_token_id.
• For all new workspaces, create a PAT from the user associated with the workspace and store it in personal_access_token_id.
• Cons:
  • Having a workspace which belongs to one user and an associated personal_access_token_id which belongs to another user is an anomaly and affects the integrity of the data.