Make sure to create user namespaces only for new users

What does this MR do and why?

Context

When a new user is created on a gitlab instance, a personal namespace should also be created for the user. Currently, this is done via a callback

class User < MainClusterwide::ApplicationRecord

  before_save :ensure_namespace_correct

  def ensure_namespace_correct
   if namespace
     namespace.path = username if username_changed?
     namespace.name = name if name_changed?
   else
     # TODO: we should no longer need the `type` parameter once we can make the
     #       the `has_one :namespace` association use the correct class.
     #       issue https://gitlab.com/gitlab-org/gitlab/-/issues/341070
     namespace = build_namespace(path: username, name: name, type: ::Namespaces::UserNamespace.sti_name)
     namespace.build_namespace_settings
   end
  end 
end

This is a before_save callback, so it works on both create and updates. So the method deals with both these scenarios with an if..else

However, the condition of this if..else is a little tricky.

Ideally, we would code this as

if new_record?
  # build associated namespace
else
  # we know that a namespace already exists for this user, so let's just update it
  namespace.path = username if username_changed?
  namespace.name = name if name_changed?
end

However, the code we currently have is a little different from this. It depends on the existence of namespace record to figure out the user is a new_record or not, instead of depending on new_record? itself.

In reality, both approaches are fine, and gives the same result.

Problem

The current approach on depending on namespace? to decide if a new record or not, becomes a problem when building Cells

Why?

Because each cell is a different entity and databases are also physically different in each cell, and hence some tables are also different across cells.

namespaces is one such table. It is a cell-local table, which means that namespaces table in cell 1 will have completely different data compared to namespaces table in cell 2.

However, the routes table is a clusterwide table - which means that all cells point to a single routes table.

And, namespaces and routes are also connected. Each namespace has a route, and even though namespaces are different across cells, all their routes record will be on a single table in the cluster.

This poses a problem to the above callback.

The scenario can be described as:

• User already exists in the cluster, which means they already have a namespace record in cell 1 and a corresponding routes record on the clusterwide routes table.
• This user tries to login to cell 2.
• In cell 2, after login, the before_save callback is executed because some attributes of a User are updated after a login.
• Now, it checks for the if namespace condition. This user already has a personal namespace, but that's in the namespaces table in cell 1. We are in cell 2, so it checks for an associated namespace record in cell 2's namespaces table, it cannot find it, and hence moves on to the else part.
• In the else condition, it builds the user namespace, and then ultimately saves it.
• On saving a new namespace, we also build a route for it and try to save it.
• This save happens on the clusterwide routes table, which already has a route for the personal namespace of this user with the same path.
• On save, conflict happens and it errors out. Boom!

Solution

So, for cells to function properly, user namespaces should only be created for new user records, and this is what this MR does. The check should be based on new_record? rather than the existence of namespace record. However, I am not rewriting the entire callback this way because it is risky, so I have made a minimal change behind a feature flag. Once it is enabled and works ok, we could rewrite the entire thing as :

if new_record?
  # build associated namespace
else
  # we know that a namespace already exists for this user, so let's just update it
  namespace.path = username if username_changed?
  namespace.name = name if name_changed?
end

Current status of records: I ran the query to figure out current status of: how many users do not have a personal namespace? and database-lab says 1:

https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/21744/commands/70582

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before	After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

• I have evaluated the MR acceptance checklist for this MR.

Related to #421597 (closed)