Inconsistencies with `keys.type`
Follow-up from !18277 (comment 227990430)
For the keys.type column, we have the following distribution on GitLab.com:
gitlabhq_production=# select type, count(*) from keys group by 1;
type | count
-----------+---------
| 2385078
DeployKey | 256421
(2 rows)
User.regular_keys performs a filter on type = 'Key' or type IS NULL. The OR may become a problem for performance, unless there are other narrow filters applied. It might make sense to migrate NULL values to a proper type value.
- https://gitlab.com/gitlab-org/gitlab/blob/master/app/models/key.rb#L38
- https://gitlab.com/gitlab-org/gitlab/blob/master/ee%2Fapp%2Fmodels%2Fee%2Fkey.rb#L15
Similarly, we have User.find_by_ssh_key which in reality doesn't care about the type of a key. The method should get renamed to avoid confusion.
While we're at this, it may be worth considering to transition to an integer enum column instead of storing strings for type.
Proposal (by @vyaklushin)
The issue description is old. Nowadays, we have DeployKey, LDAPKey and GroupDeployKey types.
- Before doing that we should double check if all NULL type records can be marked as
Key. - If all good, update Rails to set
Keytype for new normal key records - Create a migration to assign
Keytype of all NULL type records. - Make the
typefield NOT NULL - Remove
OR type IS NULLfrom Rails queries (for example here and in other places)
Edited by Vasilii Iakliushin