[go: up one dir, main page]

Skip to content

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.

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.

  1. Before doing that we should double check if all NULL type records can be marked as Key.
  2. If all good, update Rails to set Key type for new normal key records
  3. Create a migration to assign Key type of all NULL type records.
  4. Make the type field NOT NULL
  5. Remove OR type IS NULL from Rails queries (for example here and in other places)
Edited by Vasilii Iakliushin