Geo: make repositories location immutable

We have some challenges for https://gitlab.com/gitlab-org/gitlab-ee/issues/2828 and https://gitlab.com/gitlab-org/gitlab-ee/issues/2827. Based on earlies discussions around new Geo replication architecture, making this type of change reliable, would require a global lock mechanism, which can be slow, creates a single point of failure, makes restoring from a backup harder as we have to synchronize state between database and the filesystem, etc.

Moving/renaming is problematic because operations that update the repository points to the full path, which if changed will result in catastrophic events like unintended data breaches (exposing a secret repository, or commmits intended to repository A going to B, because of lack of synchronization, etc), data loss, etc.

The same can be said to creating/removing repositories. A lack of synchronization of the events will make the replication behave incorrectly.

But there is another way.

If we assume the repository path will never change, we will always be replicating the events to the correct location. Renaming and Moving should be a "virtual" change on the database, and should not be reflected on the disk.

This is already partially done by the introduction of the multiple storages, which maps repositories to different places on the local disk, based on which "storage" it is.

Removing a repository will always remove the "correct" one, no mater when this event happens, also creating a new repository in the same fullpath will never ever rewrite existing ones, as this will be stored in a different place (remember, it's based on the ID, not on the name or the namespace).

Proposal (simple)

Repository UUID will be built as: MD5("project-#{project.id}")

We can store on the disk as: "#{uuid[0..1]}/#{uuid}.git" and "#{uuid[0..1]}/#{uuid}.wiki.git" Or we can even use more levels to help with filesystem evenly allocation of repositories on folders:

"#{uuid[0]}/#{uuid[0..1]}/#{uuid}.git" or more levels if we think this is necessary. By using a predictable hash function we can even make operations without using the database to figure out where the file is on disk, and a simple script can implement the same algorithm.

Building on top of https://gitlab.com/gitlab-org/gitlab-ce/issues/28283 proposal, if security is a concern, we can add a "salt" to the hash function, which can be stored in the secrets file, so UUID would became unique for every gitlab install, like: MD5("#{secret-salt}/project-#{project.id}").

MD5 is used here just for simplicity, but we can consider alternatives like SHA256, and/or use the database to prevent a collision from allowing the repository to be created, etc.

Initial deployment

The "concern" where to store/read the repository from the disk should use a Bridge pattern and became switchable. So we can keep existing instances reading from where it normally does, and in a clean secondary Geo node, we can bootstrap it with the new format.

This will allow a fast bootstraping of the solution as we don't need to implement the migrate existing data and we can use this to prototype/test the solution before going into the gitlab.com scale test.

This also provides a path to migrate existing data in the future (to be described below)

Migrating from old to new format

As we will have the two implementations, we can use symlinks or hardlinks on disk and start to backfill the required structure to the format. We can then do a slow Rollout, machine by machine, just changing the local configuration to use the new implementation, and because of the symlinks/hardlinks both will still be able to read from the disk.

Ex:

1d/1d90357640b85550b35989198e93fee9.git -> gitlab/gitlab_ce.git 

Old format will read from gitlab/gitlab_ce.git, new will read from 1d/1d90357640b85550b35989198e93fee9.git

After all machines are Rolledout, we can mv gitlab/gitlab_ce.git 1d/1d90357640b85550b35989198e93fee9.git and have the repository moved to the new path.

cc @stanhu @dbalexandre @rspeicher @DouweM @pcarranza @smcgivern @lbot

related: https://gitlab.com/gitlab-org/gitlab-ce/issues/28112

I'm doing some exploratory refactoring to identify what will be the challenges. At first sight, we are exposing implementation details from where the a repository is located on the disk all over the codebase.

There are many places calling project.repository_storage_path and project.path_with_namespace to concatenate boths with .git and access the repository.

The solution is to deprecate path_with_namespace and have the: full_path (which path_with_namespace alias to) and disk_path. The first is concerned to URLs and the other concerned to on disk operations.

That way we can have different implementations for disk_path and make the switch easier. disk_path should give a full path including the project.repository_storage_path part.

@brodock

if security is a concern, we can add a "salt" to the hash function

I would just do the simple thing and use UUIDs as originally proposed in https://gitlab.com/gitlab-org/gitlab-ce/issues/28283

Using a hashing function and adding salt seems like over-engineering to me.

cc/ @ilyaf @briann for the security side.

There are many places calling project.repository_storage_path and project.path_with_namespace to concatenate boths with .git and access the repository.

The solution is to deprecate path_with_namespace and have the: full_path (which path_with_namespace alias to) and disk_path. The first is concerned to URLs and the other concerned to on disk operations.

That way we can have different implementations for disk_path and make the switch easier. disk_path should give a full path including the project.repository_storage_path part.

I agree

As we will have the two implementations, we can use symlinks or hardlinks on disk and start to backfill the required structure to the format. We can then do a slow Rollout, machine by machine, just changing the local configuration to use the new implementation, and because of the symlinks/hardlinks both will still be able to read from the disk.

We can't use hard links on NFS https://gitlab.com/gitlab-org/gitlab-ce/issues/28283#note_23631290

We now have three issues for this

@smcgivern I know they are very similar but have different "concerns" in mind. We need "something" for Geo and we can't base the repository on the namespace, because this is what we want to prevent: when moving a repository to a different namespace or changing the name, having to move the file on disk.

Also the one from the infrastructure team needs to take gitlab.com migration in mind, we may be able to ask customers to just redeploy a new secondary node, so we don't need to tackle the migration part here (but we want to build a path to do that in a +1 release). So they are complementary, and I think we can use Geo to test the solution.

@pcarranza would work for you if we add an UUID for the Project and use that to generate the path? do you see benefits of "uuid[0]/uuid[0..1]/uuid.git" or should we just have "uuid[0..1]/uuid.git" ? (or more levels)?

If we agree in a direction, what I want to do very early is isolate the "Storage" concern and make the full_path, disk_path changes in the codebase and release this soon so we can start tinkering with a different Storage format for Geo which can be backported to CE later.

Our idea is that we would be able to use this new Storage format for a secondary node, even if the primary is not using it yet. We need the immutability on the secondary so we don't need to sync database <> disk changes.

assigned to @brodock

@brodock sure, I agree with starting out by decoupling them in the code, even if the end result is the same.

@brodock

do you see benefits of "uuid[0]/uuid[0..1]/uuid.git" or should we just have "uuid[0..1]/uuid.git" ? (or more levels)?

Not really, I think that the security concern is gone once we can't reach a repo by namespace/name, after that, I don't think we should be having too much indirection.

Consider that the grouping of a namespace is meaningless the moment when we have multiple shards because we have multiple projects of the same namespace in multiple drives.

So I would consider aligning more with the git style of storing things:

[first 2 chars]/[rest of the uuid]

And would not even add a .git or .wiki termination at all.

@pcarranza I would stick with .git and .wiki.git for simplicity, as we don't have a way to differentiate project from wiki in within the database right now. What are the downsides of keeping it in your opinion?

I think the main downside is that we have a number of hacks in our code that look for projects that end with .wiki.git. But perhaps for the first iteration we keep this.

I'm still a bit confused why we need a separate issue from https://gitlab.com/gitlab-org/gitlab-ce/issues/28283. I think @brodock mentioned that in this proposal, the hash function would just be known, and we wouldn't store it in the database. I think the advantages of storing the value in the database outweigh the disadvantages (e.g. tied to a hash function, etc.).

As @pcarranza has said, I don't think adding a salt would provide any security benefit. I like the overall idea, so long as we can preserve the existing routes.

I think https://gitlab.com/gitlab-org/gitlab-ce/issues/28283 is a better first step security-wise but if this is needed for Geo anyways then it makes sense.

Let's close this issue in favor of https://gitlab.com/gitlab-org/gitlab-ce/issues/28283.

closed

mentioned in merge request !2477 (merged)