Race condition that makes it possible to create duplicated labels
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 CASCADEThis 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.