Add routes to allow git actions on snippet repositories

@stanhu do you mind to take a quick look at this MR? It's still WIP so I'd love to know your thoughts about the approaches taken here. There is no need for it to be a real full review.

assigned to @stanhu

added 1 commit

bef56063 - Fixed weird bug with repository cache

Compare with previous version

added 1 commit

d346de8d - More fixes about the same spec

Compare with previous version

added 1 commit

b6045633 - Trying to fix 'again' the weird cache spec

Compare with previous version

mentioned in issue #121589 (closed)

added 1 commit

92176acd - Added some specs

Compare with previous version

added 1 commit

de59e246 - Fixing specs

Compare with previous version

added 278 commits

de59e246...5a8ca1cb - 263 commits from branch master
3fffa037 - Create project snippet repository
da4a4947 - Fixed bug
89e3ae36 - Add support for ssh clone/push/pull
38a2d9d6 - Joining the controllers
b5e75195 - Removing unnecessary files
79c4c88c - Fixed specs
97fd3638 - Fixed some more specs
dd352c27 - Small refactor
b27dac1a - Removed snippets specific controllers
78568105 - Refactoring some more code
6442c93c - Fixed weird bug with repository cache
0dc23840 - More fixes about the same spec
1849a354 - Trying to fix 'again' the weird cache spec
5a766a1b - Added some specs
716419cc - Fixing specs

Compare with previous version

added 1 commit

f9e4cafd - Small refactor

Compare with previous version

mentioned in merge request !22266 (merged)

marked the checklist item Code review guidelines as completed

marked the checklist item Merge request performance guidelines as completed

marked the checklist item Style guides as completed

marked the checklist item Separation of EE specific content as completed

changed the description

added 1 commit

eec6687b - Add changelog

Compare with previous version

added 1 commit

322f2aa9 - Create snippet repository models

Compare with previous version

mentioned in merge request !22275 (merged)

@engwan do you mind to perform the first review? I've created !22266 (merged) and !22275 (merged) to shrink a little bit this MR. I hope once you come back from your OOO they will be merged. Anyway, I'd like to have early comments in case you see any red flag.

assigned to @engwan and unassigned @stanhu and @fjsanpedro

added 1 commit

6ed7375a - Create snippet repository models

Compare with previous version

added 1 commit

6214fb9e - Create snippet repository models

Compare with previous version

added 1 commit

ebf07a5f - Create snippet repository models

Compare with previous version

added 1 commit

e1f9bdd5 - Create snippet repository models

Compare with previous version

added 1 commit

85b59651 - Create snippet repository models

Compare with previous version

added 37 commits

85b59651...0d01198a - 36 commits from branch master
801f867a - Create snippet repository models

Compare with previous version

added 1 commit

edbae390 - Create snippet repository models

Compare with previous version

added 1 commit

f2dc4db7 - Create snippet repository models

Compare with previous version

added 1 commit

5983719d - Create snippet repository models

Compare with previous version

added 164 commits

5983719d...40b9bf8f - 163 commits from branch master
7cac9007 - Create snippet repository models

Compare with previous version

added 1 commit

4e17f057 - Create snippet repository models

Compare with previous version

added 1 commit

42d61030 - Create snippet repository models

Compare with previous version

added 1 commit

7ecbc3ef - Create snippet repository models

Compare with previous version

added 1 commit

8c556d96 - Create snippet repository models

Compare with previous version

added 32 commits

8c556d96...00086828 - 31 commits from branch master
7a17164f - Create snippet repository models

Compare with previous version

added 1 commit

20cc8b71 - Create snippet repository models

Compare with previous version

added 136 commits

20cc8b71...94974e2c - 135 commits from branch master
da14130d - Create snippet repository models

Compare with previous version

added 36 commits

da14130d...491a939f - 35 commits from branch master
c1ff7ce0 - Create snippet repository models

Compare with previous version

removed database databasereview pending labels

marked the checklist item Changelog entry as completed

unmarked as a Work In Progress

@fjsanpedro Great work here! I don't see major issues with the approach here. Though I don't really have the best domain-knowledge on the repo-side of things. I think another look from someone from devopscreate would be good

Sorry it took a while to review. I was catching up from OOO.

assigned to @fjsanpedro and unassigned @engwan

added 1 commit

aaa9ccbb - Code review comments applied

Compare with previous version

added 1 commit

6d07d763 - Create snippet repository models

Compare with previous version

added 182 commits

6d07d763...f70542e8 - 181 commits from branch master
0d75e057 - Create snippet repository models

Compare with previous version

added 1 commit

a3d9f927 - Create snippet repository models

Compare with previous version

added 1 commit

d0359232 - Create snippet repository models

Compare with previous version

Thanks very much @engwan for the review!!. I know it's not an easy MR, but in order to have a quick way to revert it in case something goes south, I have to keep most of the new changes together.

Do you mind another review?

assigned to @engwan and unassigned @fjsanpedro

marked as a Work In Progress

added 107 commits

d0359232...dddefcf5 - 106 commits from branch master
6da863ed - Create snippet repository models

Compare with previous version

I've set this MR again to WIP in order to avoid merging it into the current milestone. Since we're close to the release freeze date, we won't have the chance to test this MR in production before cutting the release. I'd prefer to wait until 12.8 to test it in production instead of releasing patches and hotfixes to the release in case something comes up.

added 193 commits

6da863ed...df64d5d2 - 192 commits from branch master
abd6b4e4 - Create snippet repository models

Compare with previous version

added 1 commit

7beb15df - Create snippet repository models

Compare with previous version

@nick.thomas would you mind to take a look at this MR? it needs some groupsource code love (sorry in advance for the size but it's better if we have to revert it).

assigned to @nick.thomas

added 174 commits

7beb15df...6bc60b5a - 173 commits from branch master
610749b7 - Create snippet repository models

Compare with previous version

@nick.thomas I'm unassigning you for now and assigning @brodock to polish some things regarding not using old legacy storage.

@brodock do you mind to take a look?

assigned to @brodock and unassigned @nick.thomas

added 204 commits

610749b7...e74f59af - 203 commits from branch master
9734358d - Create snippet repository models

Compare with previous version

Because snippets is not supposed to be/work as a project, are we considering (smaller) storage limitations for it?

added 52 commits

9734358d...337e1fe2 - 51 commits from branch master
7d907672 - Create snippet repository models

Compare with previous version

added 1 commit

12a9afb2 - Create snippet repository models

Compare with previous version

Another thing I thought about is the moving of repositories to other nodes. We would probably need to update the service to also move the project snippet repositories when the project is moved to another gitaly node?

And for the personal snippets, I guess we just don't support moving of these for now?

@engwan project snippets aren't located inside the project hashed storage. They will be stored under @hashed/snippets/.... If a project is moved to a different gitaly node, do we really need to move the snippet from the existing location?.

Anyway, we can work on that in a different MR, this one already too big. WDYT?

added 1 commit

7ef76c1d - Renamed subject to repositorable

Compare with previous version

added 300 commits

7ef76c1d...fa883510 - 298 commits from branch master
84c3e880 - Create snippet repository models
b322d321 - Renamed subject to repositorable

Compare with previous version

added 1 commit

6c5a0fb8 - Fixing rubocop offenses

Compare with previous version

added 78 commits

6c5a0fb8...76ff8a57 - 75 commits from branch master
1c783dea - Create snippet repository models
4a76765d - Renamed subject to repositorable
f234a157 - Fixing rubocop offenses

Compare with previous version

added 388 commits

f234a157...9d154542 - 385 commits from branch master
c3fab0ef - Create snippet repository models
e9831719 - Renamed subject to repositorable
4cbd8f49 - Fixing rubocop offenses

Compare with previous version

changed milestone to %12.8

mentioned in issue #39514 (closed)

mentioned in merge request !22960 (merged)

added 137 commits

4cbd8f49...fd49ef88 - 134 commits from branch master
52cc9bc0 - Create snippet repository models
6beb5030 - Renamed subject to repositorable
23fe6e82 - Fixing rubocop offenses

Compare with previous version

added 1 commit

4574d14a - Create snippet repository models

Compare with previous version

added 1 commit

6b7b29d9 - Refactored repository#project

Compare with previous version

added 2 commits

ef2c5948 - Small refactor
06119f68 - Trying to fix bug with Geo::DeletedProject

Compare with previous version

mentioned in merge request !23494 (merged)

added 137 commits

06119f68...7da363cc - 136 commits from branch master
a63aff96 - Create snippet repository models

Compare with previous version

added 1 commit

c400530e - Code review comments applied

Compare with previous version

added 260 commits

c400530e...5e1fdec0 - 259 commits from branch master
c0d090c6 - Create snippet repository models

Compare with previous version

Thanks @fjsanpedro, this is looking much better now

unassigned @engwan

added 1 commit

189047eb - Create snippet repository models

Compare with previous version

mentioned in merge request !23678 (merged)

added 156 commits

189047eb...f5d0c7c5 - 154 commits from branch master
d0bffc6c - Create snippet repository models
eff881ef - Change copy

Compare with previous version

added 1 commit

583d5644 - Create snippet repository models

Compare with previous version

added 1 commit

0e8f7c24 - Create snippet repository models

Compare with previous version

added 61 commits

0e8f7c24...5fb30b28 - 60 commits from branch master
e79b2553 - Create snippet repository models

Compare with previous version

added 1 commit

3d99b226 - Create snippet repository models

Compare with previous version

@tmslvnkc hi , for this issue I'd like to several QA tests. Specifically:

clone a personal snippet through HTTP (ie. git clone http://127.0.0.1:3000/snippets/1.git)
clone a personal snippet through SSH (ie. git clone ssh://fran@localhost:2221/snippets/1.git)
clone a project snippet through HTTP (ie. git clone http://127.0.0.1:3000/h5bp/html5-boilerplate/snippets/53.git)
clone a project snippet through SSH (ie. git clone ssh://fran@localhost:2221/h5bp/html5-boilerplate/snippets/53.git)
clone a project snippet through HTTP, using a project moved to a different namespace and using the old project url.
clone a project snippet through SSH, using a project moved to a different namespace and using the old project url.

I'm not acquainted with QA and its structure. Could you help me with these tests?

added 1 commit

8b4c0692 - Create snippet repository models

Compare with previous version

added 66 commits

8b4c0692...ab78d4b1 - 65 commits from branch master
97d5005c - Create snippet repository models

Compare with previous version

added 1 commit

247eee82 - Create snippet repository models

Compare with previous version

assigned to @fjsanpedro and unassigned @brodock

added 104 commits

247eee82...0ee3c032 - 103 commits from branch master
f86171bd - Create snippet repository models

Compare with previous version

added 85 commits

f86171bd...47e09db7 - 84 commits from branch master
7b7a2388 - Create snippet repository models

Compare with previous version

added 75 commits

7b7a2388...c01d0411 - 74 commits from branch master
dd796e58 - Create snippet repository models

Compare with previous version

added 1 commit

a736e307 - Create snippet repository models

Compare with previous version

added 72 commits

a736e307...a033ad83 - 71 commits from branch master
288e456b - Create snippet repository models

Compare with previous version

added 765 commits

288e456b...ae5d83de - 764 commits from branch master
65ca3467 - Create snippet repository models

Compare with previous version

unmarked as a Work In Progress

changed title from WIP: Version Control for Snippets: Create repository model for snippets to Add routes to allow git actions on snippet repositories

changed the description

mentioned in issue #201886 (closed)

added workflowblocked label

added 1286 commits

65ca3467...1ea6cbbb - 1285 commits from branch master
7be6a91c - Create snippet repository models

Compare with previous version

added 1 commit

72e523c1 - Removing unnecessary changes

Compare with previous version

added 2 commits

76a8b45b - Fix specs
bbaf13e1 - Fix bug

Compare with previous version

changed the description

@nick.thomas do you mind to take a look and give me your thoughts? It's not a review .

I opted for still returning the project (as well as the container) in the controllers, because there are others, like LFS ones, that still need the reference. Besides, there is some logic inside the controllers that still need that reference as well.

assigned to @nick.thomas

changed the description

Thanks @fjsanpedro , a few initial spots. I should definitely give this some manual testing!

WDYT to adding a couple of QA tests that will clone and push to personal and project snippets at this point?

unassigned @nick.thomas

@Anastasia / @tmslvnkc , please let us know if you have any additional QA recommendations.

added 442 commits

bbaf13e1...22f6af08 - 438 commits from branch master
016c41ea - Create snippet repository models
7cb275bf - Removing unnecessary changes
5462773c - Fix specs
14f5bd82 - Fix bug

Compare with previous version

added 1 commit

83a915a4 - Changes after rebase

Compare with previous version

added 1 commit

8db99e8e - Fix post receive for snippets

Compare with previous version

added 1 commit

f054d1b8 - Create snippet repository models

Compare with previous version

mentioned in issue #196050 (closed)

added 1 commit

d2ceab12 - Fix rubocop offense

Compare with previous version

added 1 commit

3ea91b74 - Code review comments applied

Compare with previous version

changed milestone to %12.9

added 1 commit

f32cb9fe - Create project from push only when repo type is project

Compare with previous version

mentioned in issue #207110 (closed)

assigned to @nick.thomas and unassigned @fjsanpedro

added 208 commits

f32cb9fe...c626c6a5 - 204 commits from branch master
85b8b8e9 - Create snippet repository models
20520e4e - Fix rubocop offense
831b5cb0 - Code review comments applied
4245a211 - Create project from push only when repo type is project

Compare with previous version

added 1 commit

4952c037 - Add snippet git routes

Compare with previous version

added 1 commit

e998aa39 - Rubocop offense fixed

Compare with previous version

mentioned in merge request !23492 (merged)

assigned to @fjsanpedro and unassigned @nick.thomas

added 1 commit

b873270c - Code review comments applied

Compare with previous version

@nick.thomas I address almost all your comments. The only thing left is the policy thing !21739 (comment 288413467) and the problem @lulalala found in !23492 (comment 291461844).

added 1 commit

4b65f269 - Fix specs

Compare with previous version

added 1 commit

5dc7f355 - Fix spec

Compare with previous version

added 373 commits

5dc7f355...b840d09f - 368 commits from branch master
4c8d70cf - Add snippet git routes
c9a79e98 - Rubocop offense fixed
16d900c5 - Code review comments applied
6b928332 - Fix specs
bc9984a5 - Fix spec

Compare with previous version

added 1 commit

406dc2f0 - Fix spec after rebase

Compare with previous version

@nick.thomas comments answered

assigned to @nick.thomas and unassigned @fjsanpedro

assigned to @fjsanpedro and unassigned @nick.thomas

added 184 commits

406dc2f0...8de9414e - 178 commits from branch master
e6217bc8 - Add snippet git routes
76538d93 - Rubocop offense fixed
b2c8dffb - Code review comments applied
9386a878 - Fix specs
6793f037 - Fix spec
a0a2b46d - Fix spec after rebase

Compare with previous version

added 1 commit

ec9c027b - Add snippet git routes

Compare with previous version

added workflowin review label and removed workflowblocked label

Okay, everything addressed and tested @nick.thomas. I changed the http_download_allowed? to remove the changes we made to the policies until we address what Mark found, but include the access check for snippets and guest users. Another round?

assigned to @nick.thomas and unassigned @fjsanpedro

added 1 commit

e5276ccd - Removing guest access check in http controller

Compare with previous version

added 361 commits

e5276ccd...885e4d44 - 359 commits from branch master
0bdf3213 - Add snippet git routes
a7f1d873 - Removing guest access check in http controller

Compare with previous version

mentioned in issue #208155 (closed)

assigned to @fjsanpedro and unassigned @nick.thomas

mentioned in issue #207865 (closed)

added 182 commits

a7f1d873...681b47de - 180 commits from branch master
3cc9c946 - Add snippet git routes
f5cbe638 - Removing guest access check in http controller

Compare with previous version

added 43 commits

f5cbe638...c7b7c67f - 41 commits from branch master
cd3f825b - Add snippet git routes
b6466123 - Removing guest access check in http controller

Compare with previous version

added database databasereview pending labels

mentioned in issue #207869 (closed)

mentioned in issue #207870 (closed)

mentioned in merge request !24435 (closed)

added 319 commits

b6466123...b7fa9dc4 - 317 commits from branch master
9d3ea557 - Add snippet git routes
3a2c7acf - Removing guest access check in http controller

Compare with previous version

added 1 commit

787054f1 - Restoring change

Compare with previous version

added 205 commits

787054f1...5b12631c - 202 commits from branch master
e6b4c828 - Add snippet git routes
6fdfdf1e - Removing guest access check in http controller
92bf5fd2 - Restoring change

Compare with previous version

added 1 commit

471f15c5 - Add snippet git routes

Compare with previous version

added 1 commit

58ab4dc2 - Add snippet git routes

Compare with previous version

added 102 commits

58ab4dc2...36029973 - 101 commits from branch master
4e85c504 - Add snippet git routes

Compare with previous version

added 1 commit

88304db5 - Add guest read ability to repo_type

Compare with previous version

@nick.thomas Since !25929 (closed) seems to generate more problems than benefits, I opted here for the other variant we have, updating RepoType and include there the read ability (not 100% sure of the naming). This way we can use it without checking the repo type.

Besides, it will help fix the problem with wikis whose project repository is disabled. We would just need to add the ability download_wiki_code instead of the default download_code.

assigned to @nick.thomas and unassigned @fjsanpedro

If we don't like the info in the RepoType we can create a method in GitHttpClientController like:

def guest_read_ability
  if repo_type.snippet?
    :read_snippet
  elsif repo_type.wiki?
    :download_wiki_code
  else
    :download_code
  end
end

added 129 commits

88304db5...8da519a8 - 127 commits from branch master
b22ebfad - Add snippet git routes
f1fa215a - Add guest read ability to repo_type

Compare with previous version

mentioned in merge request !24150 (merged)

resolved all threads

The package-and-qa job from pipeline https://gitlab.com/gitlab-org/gitlab/pipelines/123132872 triggered https://gitlab.com/gitlab-org/build/omnibus-gitlab-mirror/pipelines/123167020 downstream.

The Trigger:qa-test job from pipeline https://gitlab.com/gitlab-org/build/omnibus-gitlab-mirror/pipelines/123167020 triggered https://gitlab.com/gitlab-org/gitlab-qa/pipelines/123194127 downstream.

The gitlab-qa downstream pipeline failed! .

The errors don't seem related. Most of them are related to adding the deploy key.

OK, this LGTM - I don't mind the repo_type approach tot he guest ability. Thanks @fjsanpedro!

I've got one question about the feature flag, and I've set off QA to see the results of that, but I think we'll be able to merge this more or less as-is.

approved this merge request

removed database databasereview pending labels

resolved all threads

OK, the QA pipeline failed a set of deploy key things - but these seem to be already known, and not to do with the changes in this MR. I'm happy to merge it as-is!

	2 Warnings
	This merge request is quite big (more than 1173 lines changed), please consider splitting it into multiple merge requests.
	: The merge request title length is acceptable, but please try to reduce it to 50 characters.

Commit message standards

The merge request title that will be used in the squash commit does not meet our Git commit message standards.

For more information on how to write a good commit message, take a look at How to Write a Git Commit Message.

Here is an example of a good commit message:

Reject ruby interpolation in externalized strings

When using ruby interpolation in externalized strings, they can't be
detected. Which means they will never be presented to be translated.

To mix variables into translations we need to use `sprintf`
instead.

Instead of:

    _("Hello #{subject}")

Use:

    _("Hello %{subject}") % { subject: 'world' }

This is an example of a bad commit message:

updated README.md

This commit message is bad because although it tells us that README.md is updated, it doesn't tell us why or how it was updated.

Reviewer roulette

Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.

To spread load more evenly across eligible reviewers, Danger has randomly picked a candidate for each review slot. Feel free to override this selection if you think someone else would be better-suited, or the chosen person is unavailable.

To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines.

Once you've decided who will review this merge request, mention them as you normally would! Danger does not (yet?) automatically notify them for you.

Category	Reviewer	Maintainer
backend	Josianne Hyson (`@jhyson`)	Grzegorz Bizon (`@grzesiek`)

Generated by Danger

enabled an automatic merge when the pipeline for 0d21022b succeeds

merged

mentioned in commit e94f6229

added workflowverification label and removed workflowin review label

Thanks very much @nick.thomas!! Thanks to your insightful review we could catch several edge cases and improve the MR a lot.

added workflowstaging label and removed workflowverification label

added workflowcanary label and removed workflowstaging label

added workflowproduction label and removed workflowcanary label

changed the description

added Category:Source Code Management snippets labels and removed 1 deleted label

Add routes to allow git actions on snippet repositories

What does this MR do?

Does this MR meet the acceptance criteria?

Conformity

Activity

Commit message standards

Reviewer roulette

Add routes to allow git actions on snippet repositories

What does this MR do?

Does this MR meet the acceptance criteria?

Conformity

Merge request reports

Activity

Commit message standards

Reviewer roulette