Skip to content

WIP: Unique index for type and project_id on services

Andy Schoenen requested to merge 207385-allow-only-one-service-of-each-type-constraint into master

What does this MR do?

On the application level we already prevent multiple services of the same type in one project. There is no ActiveRecord validation but from a UI point of view it is not possible to have more than one service of each type on a project

This MR adds a AR validation and a unique index to prevent multiple services of the same type in one project

Related to #207385 (closed)

Deleting inconsistent data

It is possible that the database holds data that would violate the unique index. Only one service of each type can be used in a project and will show up in the UI. The others are ghost services and can be deleted. I couldn't find a reliable way to find those services though. I run some tests locally and it looks like service with the highest ID is selected. We select the service here. A query that deletes all duplicated services, excluding the ones with highest IDs can look like this:

DELETE FROM services
WHERE project_id IS NOT NULL
AND id NOT IN (
  SELECT Max(id)
  FROM services
  WHERE project_id IS NOT NULL
  GROUP BY type, project_id
);

However I think we can not relay on the service with the highest ID being the selected one. We don't specify an order for the services to be returned and according to the postgres docs that means we can not relay on the order.

If sorting is not chosen, the rows will be returned in an unspecified order. The actual order in that case will depend on the scan and join plan types and the order on disk, but it must not be relied on.

find_service executes the following queries:

  Service Load (1.9ms)  SELECT "services".* FROM "services" WHERE "services"."project_id" = 10
  ↳ app/models/project.rb:1252:in `find_service'
  Service Load (0.4ms)  SELECT "services".* FROM "services" WHERE "services"."template" = TRUE
  ↳ app/models/project.rb:1252:in `find_service'

I tried this query in database-lab(internal only) but I’m not sure if the data is correct because it shows 1789821 invalid services that we have to delete. Can someone confirm the query is correct to find duplicates?

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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
Edited by 🤖 GitLab Bot 🤖

Merge request reports