Fix - Update routes for lost-and-found group and projects
What does this MR do?
Related to #219658 (closed)
This MR fixes the issue encountered when !33653 (merged) was deployed to GitLab.com production:
PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_routes_on_path"
DETAIL: Key (path)=(lost-and-found) already exists.!33653 (merged) has been reverted with !34163 (merged)
We have identified the root cause for this incident:
!33653 (merged) was working on top of !31675 (merged), which was generating a unique namespace name for the lost-and-found group, but it was not checking whether the path was also unique (you can have different names that result to the same path).
That causes no issues while generating a namespace, but results on an error when a route is created if there is another route with the same path.
Description of the patch
The fix required is pretty simple: we have to make sure that the lost-and-found group has been assigned a unique path.
We ensure that by checking the routes table and making sure that a unique path is assigned to the lost-and-found group before generating the route for it and its projects by calling ensure_route!
The spec for the data migration has been updated accordingly to test against a data set, where an existing group already owns the lost-and-found path and the lost-and-found group is set with the wrong path.
By ensuring that the lost-and-found group's path is unique, we are also sure that all the routes generated for the orphaned projects that assigned to it will be also unique (as we already assign a unique sub-path to each project in !31675 (merged))
Proof of concept
As an incident was caused by the original MR, an additional sanity check was done in the local dev environment.
Starting with the data set generated by rake dev:setup, we dropped the foreign key from projects to namespaces and set one project to reference an invalid namespace.
We then rerun all the migrations from !31675 (merged) to add the FK, a Ghost user and the lost-and-found group and assign the orphaned project to it.
We then reset the Ghost User to a normal user and did everything all over again with a new Ghost user and 3 orphaned projects.
We finally reset the Ghost User once more, set 4 final projects to reference an invalid namespace and did the migrations one last time.
The result is similar to the current dataset in production with one Ghost user and a conflicting lost-and-found namespace with the same path as the one generated for the real Ghost user:
SELECT id, username, name, user_type FROM users WHERE id >= 45;
id | username | name | user_type
----+----------+------------+-----------
45 | ghost | Ghost User | -- Normal user
46 | ghost1 | Ghost User | -- Normal user
47 | ghost2 | Ghost User | 5 -- The real Ghost User
SELECT id, owner_id, name, path FROM namespaces WHERE id > 51;
id | owner_id | name | path
----+----------+------------------------+-----------------
52 | 45 | Ghost User | ghost
53 | | Lost and Found | lost-and-found
54 | 46 | Ghost User | ghost1
55 | | Another Lost and Found | lost-and-found1
56 | 47 | Ghost User | ghost2 -- Correct path for the Ghost User's namespace
57 | | lost-and-found | lost-and-found -- Conflicting path with existing namespace.
-- That's what caused the incident
-- The 4 orphaned projects have been assigned to Ghost User's lost-and-found group (with unique sub-paths and names)
SELECT id, name, path, namespace_id FROM projects WHERE namespace_id = 57;
id | name | path | namespace_id
----+-----------+-----------+--------------
3 | Wget2_3 | wget2_3 | 57
12 | Flight_12 | flight_12 | 57
13 | Flight_13 | flight_13 | 57
15 | Flight_15 | flight_15 | 57
-- But the routes for them have not been updated
-- and there is no route for the lost-and-found group
-- That's the reason the original MR was created in the first place
SELECT id, source_id, source_type, path, name
FROM routes
WHERE (source_type = 'Namespace' AND source_id > 51)
OR (source_id in (1, 2, 3, 10, 11, 12, 13, 15) AND source_type = 'Project');
id | source_id | source_type | path | name
----+-----------+-------------+------------------------------------+-----------------------------------------
70 | 52 | Namespace | ghost | Ghost User
71 | 53 | Namespace | lost-and-found | Lost and Found
23 | 1 | Project | lost-and-found/gitlab-test_1 | Lost and Found / Gitlab Test_1
72 | 54 | Namespace | ghost1 | Ghost User
73 | 55 | Namespace | lost-and-found1 | Another Lost and Found
24 | 2 | Project | lost-and-found1/gitlab-shell_2 | Another Lost and Found / Gitlab Shell_2
61 | 10 | Project | lost-and-found1/underscore_10 | Another Lost and Found / Underscore_10
62 | 11 | Project | lost-and-found1/gitlab-test_11 | Another Lost and Found / Gitlab Test_11
26 | 3 | Project | gnuwget/wget2 | Gnuwget / Wget2
63 | 12 | Project | reported_user_1/flight | Minda Roob / Flight
64 | 13 | Project | reported_user_12/flight | Chun Hoppe / Flight
66 | 15 | Project | reported_user_11/flight | Earline Walter / FlightAfter the migration in this MR is run, everything is fixed:
- The
lost-and-foundgroup's path has been updated to a unique one:lost-and-found2 - A route has been added for it
- The routes for the orphaned projects have been properly updated
SELECT id, owner_id, name, path FROM namespaces WHERE id > 51;
id | owner_id | name | path
----+----------+------------------------+-----------------
52 | 45 | Ghost User | ghost
53 | | Lost and Found | lost-and-found
54 | 46 | Ghost User | ghost1
55 | | Another Lost and Found | lost-and-found1
56 | 47 | Ghost User | ghost2
57 | | lost-and-found | lost-and-found2
SELECT id, source_id, source_type, path, name
FROM routes
WHERE (source_type = 'Namespace' AND source_id > 51)
OR (source_id in (1, 2, 3, 10, 11, 12, 13, 15) AND source_type = 'Project');
id | source_id | source_type | path | name
----+-----------+-------------+---------------------------------+--------------------------------------
70 | 52 | Namespace | ghost | Ghost User
71 | 53 | Namespace | lost-and-found | Lost and Found
23 | 1 | Project | lost-and-found/gitlab-test_1 | Lost and Found / Gitlab Test_1
72 | 54 | Namespace | ghost1 | Ghost User
73 | 55 | Namespace | lost-and-found1 | Another Lost and Found
24 | 2 | Project | lost-and-found1/gitlab-shell_2 | Another Lost and Found / Gitlab Shell_2
61 | 10 | Project | lost-and-found1/flight_10 | Another Lost and Found / Flight_10
62 | 11 | Project | lost-and-found1/typeahead-js_11 | Another Lost and Found / Typeahead.Js_11
74 | 56 | Namespace | ghost2 | Ghost User
75 | 57 | Namespace | lost-and-found2 | lost-and-found
26 | 3 | Project | lost-and-found2/wget2_3 | lost-and-found / Wget2_3
63 | 12 | Project | lost-and-found2/flight_12 | lost-and-found / Flight_12
64 | 13 | Project | lost-and-found2/flight_13 | lost-and-found / Flight_13
66 | 15 | Project | lost-and-found2/flight_15 | lost-and-found / Flight_15As with the original MR, the results of this have been also checked through GitLab's web UI and everything is accessible.
You can find additional sanity checks that have been performed in the following snippet
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry - [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides - [-] Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers - [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
- [-] Label as security and @ mention
@gitlab-com/gl-security/appsec - [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
- [-] Security reports checked/validated by a reviewer from the AppSec team