Resolve how Workspaces should handle deletion of associated Project/Agent/User/Token
MR: Pending
Description
Currently, whenever the project, agent, or user record associated with a workspace is deleted from the database, the workspace record is also deleted, because the foreign keys are set up to :cascade
delete.
We could change this to :nullify
or :restrict
, but this would require changes in UX behavior and logic.
This decision depends upon whether the associations are hard-deleted or soft-deleted. Project is hard-deleted (see linked comment below), have not investigated the others. For background on hard vs. soft delete, see: https://stackoverflow.com/questions/378331/physical-vs-logical-hard-vs-soft-delete-of-database-record
See this associated comment for more details: #412342 (comment 1415335858)
Here's a copy of that comment for convenience:
Discussion of cascading delete of workspace records
Can't see all the workspace shown in this comment or comment
🤔 Did someone delete them?
👀
Yes, I confirmed this in prod. There's ranges of workspace IDs missing.This is expected, and it is happening because we intentionally have cascade delete defined on all the foreign keys of workspaces table.So, whenever the workspaces agent, user, or project is deleted, the workspace is deleted as well.
- https://gitlab.com/gitlab-org/gitlab/-/blob/77283cc6a05c4f8535442da6dce108128a52fe9f/db/migrate/20221225010102_create_workspaces_user_foreign_key.rb#L10
- https://gitlab.com/gitlab-org/gitlab/-/blob/77283cc6a05c4f8535442da6dce108128a52fe9f/db/migrate/20221225010103_create_workspaces_project_foreign_key.rb#L10
- https://gitlab.com/gitlab-org/gitlab/-/blob/77283cc6a05c4f8535442da6dce108128a52fe9f/db/migrate/20221225010104_create_workspaces_cluster_agent_foreign_key.rb#L10
These keys all have the following comment:
# NOTE: All workspace foreign key references are currently `on_delete: :cascade`, because we have no support or
# testing around null values. However, in the future we may want to switch these to nullify, especially
# once we start introducing logging, metrics, billing, etc. around workspaces.
add_concurrent_foreign_key :workspaces, :users, column: :user_id, on_delete: :cascade
The other options are :nullify
and :restrict
: https://api.rubyonrails.org/v4.2/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-add_foreign_key
As the comment explains, in the initial release, I did not want to define them as this though, because that could cause unexpected behavior, and at a minimum would have required us to add additional logic in various places to handle these cases of "orphaned" workspaces.For GA/Viable, we should definitely revisit this, because the current behavior of losing all traces of the workspace record in the database is not optimal from an audit trail perspective.
But we would need input from Design and Product about what exactly needs to happen in these scenarios - cc @jmiocene @ericschurter
These questions are also closely related to the security questions of what to do with "orphaned" workspaces, as they have some of the same UI/UX implications:
https://gitlab.com/gitlab-org/gitlab/-/issues/412283#note_1411771173
EDIT: For example, if the project is deleted, and we set the workspace.project_id
foreign key to null
, that means we can no longer look up and show the project name in the UI based on the project_id
, as we are currently doing. There would be many other edge cases like this to consider if we want to allow these fields to be nullable.
Also, this does actually make that security issue a non-concern for now, because if the devfile project is deleted, it deletes the all workspaces associated with it! Which probably isn't optimal.We should definitely revisit this soon.
UPDATE: THere's also the same sort of questions around the new association to personal_access_tokens
. See discussion here: &10882 (comment 1446535448)
Association | Action | Details |
---|---|---|
Project | nullify |
Anywhere in the UI where the formerly associated project name is displayed will have to be updated to Deleted
|
Agent | cascade |
We need to let the user (likely an admin) know that deleting the agent will immediately terminate all running workspaces, potentially resulting in data loss. A simple acknowledgment of this is fine. |
User | cascade |
Same as agent |
PAT | cascade |
I'm not sure this is actually needed if the tokens are being created transparently and revoked when the workspace is terminated. |
Acceptance Criteria
TODO: Fill out (required)
-
Deleting a project that has workspaces associated with it does not immediately terminate the workspaces. -
Workspaces that display a project association do not show the project name for projects that have been deleted -
Deleting an Agent or User immediately cascades that action to the workspace and we have a job that terminates any active workspace associated with the agent or user immediately. -
A user is warned before deleting an agent or user that any active workspaces will be immediately terminated which may result in data loss.
Technical Requirements
TODO: Fill out or delete [If applicable, please list out any technical requirements for this feature/enhancement.]
Design Requirements
TODO: Fill out or delete [If applicable, please provide a link to the design specifications for this feature/enhancement.]
Impact Assessment
TODO: Fill out or delete [Please describe the impact this feature/enhancement will have on the user experience and/or the product as a whole.]
User Story
TODO: Fill out or delete [Provide a user story to illustrate the use case for this feature/enhancement. Include examples to help communicate the intended functionality.]