GitLab Next

Summary

While testing automatic labelling on incident issues I discovered that users can create duplicate labels on a project.

In this case we utilize Labels::FindOrCreateService to find or create a label for each incoming Prometheus alert. If the alerts come in simultaneously duplicate labels are created. Using these duplicate labels in issue fail due to validation constraints (Label is invalid).

Refs https://gitlab.com/gitlab-org/gitlab-ce/issues/34953

Steps to reproduce

On your local GDK run Rails console:

user = User.first # mostly root
project = Project.last
project.group # => #<Group id:99 @gitlab-qa-sandbox-group/qa-test-2019-07-14-12-49-15-42529f648751dc6d>


# Simulate concurrency 
threads = 5.times.map { Thread.new { Labels::FindOrCreateService.new(user, project, title: "not uniq").execute } }
threads.map(&:join)

label = Label.where(project_id: project, title: "not uniq").first # Does really matter which one

issue = Issues::CreateService.new(project, user, title: "fail", label_ids: [label.id]).execute
issue.persisted? # => false
issue.errors
# => #<ActiveModel::Errors:0x000055a336637200
# @base=#<Issue id: manual-prometheus/autodevops-deploy#192>,
# @details={:labels=>[{:error=>:invalid}]},
# @messages={:labels=>["is invalid"]}>

What is the current bug behavior?

Users can create duplicate labels.

What is the expected correct behavior?

Labels should be unique.

Relevant logs and/or screenshots

Possible fixes

1. Database index does not work properly

Not sure as we already have an UNIQUE index on labels table:

   ...
    "index_labels_on_group_id_and_project_id_and_title" UNIQUE, btree (group_id, project_id, title)
   ...

gitlabhq_development=# \d labels
                                            Table "public.labels"
         Column          |            Type             |                      Modifiers
-------------------------+-----------------------------+-----------------------------------------------------
 id                      | integer                     | not null default nextval('labels_id_seq'::regclass)
 title                   | character varying           |
 color                   | character varying           |
 project_id              | integer                     |
 created_at              | timestamp without time zone |
 updated_at              | timestamp without time zone |
 template                | boolean                     | default false
 description             | character varying           |
 description_html        | text                        |
 type                    | character varying           |
 group_id                | integer                     |
 cached_markdown_version | integer                     |
Indexes:
    "labels_pkey" PRIMARY KEY, btree (id)
    "index_labels_on_group_id_and_project_id_and_title" UNIQUE, btree (group_id, project_id, title)
    "index_labels_on_project_id" btree (project_id)
    "index_labels_on_template" btree (template) WHERE template
    "index_labels_on_title" btree (title)
    "index_labels_on_type_and_project_id" btree (type, project_id)
Foreign-key constraints:
    "fk_7de4989a69" FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE
    "fk_rails_c1ac5161d8" FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE
Referenced by:
    TABLE "lists" CONSTRAINT "fk_7a5553d60f" FOREIGN KEY (label_id) REFERENCES labels(id) ON DELETE CASCADE
    TABLE "label_links" CONSTRAINT "fk_d97dd08678" FOREIGN KEY (label_id) REFERENCES labels(id) ON DELETE CASCADE
    TABLE "board_labels" CONSTRAINT "fk_rails_362b0600a3" FOREIGN KEY (label_id) REFERENCES labels(id) ON DELETE CA
SCADE
    TABLE "resource_label_events" CONSTRAINT "fk_rails_b126799f57" FOREIGN KEY (label_id) REFERENCES labels(id) ON 
DELETE SET NULL
    TABLE "label_priorities" CONSTRAINT "fk_rails_e161058b0f" FOREIGN KEY (label_id) REFERENCES labels(id) ON DELET
E CASCADE

This index does not work properly:

gitlabhq_development=# select project_id, title, group_id from labels where project_id = 61;
 project_id |   title   | group_id
------------+-----------+----------
         61 | not uniq  |
         61 | not uniq  |
         61 | not uniq  |
         61 | not uniq  |
         61 | not uniq  |

2. Handle duplicate label in service

Labels::FindOrCreateService should handle the case where the label was not persisted after creation.