Skip to content

Implement private repository limits #12705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 82 commits into from
Closed

Implement private repository limits #12705

wants to merge 82 commits into from

Conversation

mvdkleijn
Copy link

@mvdkleijn mvdkleijn commented Sep 3, 2020

Since @kasbah does not appear to have time to continue his work on PR #6184 I started looking at continuing his work. As there have been many changes to master since, I felt it was easier to start fresh.

Fixes #3653. Feedback very much appreciated.

  • Adjust User model
  • Adjust repo model
  • Check fields when creating repositories
  • Add locale entries
  • Add setting to admin interfaces
  • Adjust TransferOwner counts
  • Adjust DeleteRepo counts
  • Adjust checkers
  • Add tests
  • Check what happens if ownership of a private repositories is transferred to a user at the limit
  • Test in a local install

For clarity: what I'm going for here is that the current num_repos / max_repo_creation stuff is equal to the public number of repos. For private repos, I'm adding new counts and admin screen entries.

@codecov-commenter
Copy link

codecov-commenter commented Sep 4, 2020

Codecov Report

Merging #12705 into master will increase coverage by 0.01%.
The diff coverage is 21.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12705      +/-   ##
==========================================
+ Coverage   43.30%   43.31%   +0.01%     
==========================================
  Files         646      646              
  Lines       71625    71657      +32     
==========================================
+ Hits        31016    31039      +23     
- Misses      35591    35594       +3     
- Partials     5018     5024       +6     
Impacted Files Coverage Δ
models/error.go 35.02% <0.00%> (+0.21%) ⬆️
routers/repo/repo.go 32.20% <0.00%> (-0.51%) ⬇️
models/repo.go 55.55% <20.00%> (-0.14%) ⬇️
models/user.go 53.26% <33.33%> (-0.30%) ⬇️
modules/setting/repository.go 58.49% <100.00%> (+0.79%) ⬆️
modules/indexer/stats/db.go 43.47% <0.00%> (-17.40%) ⬇️
modules/indexer/stats/queue.go 64.70% <0.00%> (-11.77%) ⬇️
services/pull/pull.go 41.57% <0.00%> (ø)
modules/log/event.go 57.54% <0.00%> (+0.94%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bda9e51...7ba71c8. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 4, 2020
@lafriks lafriks added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Sep 4, 2020
@lafriks lafriks added this to the 1.14.0 milestone Sep 4, 2020
Comment on lines +154 to +152
if ctx.Data["private"] == false && !ctx.User.CanCreateRepo() {
ctx.RenderWithErr(ctx.Tr("repo.form.reach_limit_of_creation", ctx.User.MaxCreationLimit()), tplCreate, nil)
} else if ctx.Data["private"] == true && !ctx.User.CanCreatePrivateRepo() {
ctx.RenderWithErr(ctx.Tr("repo.form.reach_limit_of_private_creation", ctx.User.MaxPrivateCreationLimit()), tplCreate, nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah so you intend to split the repo creation limits so that MaxCreationLimit only applies to public repositories and MaxPrivateCreationLimit only applies to private repositories.

This will essentially double users repo creation limits. You would be better off having a third limit MaxPublicCreationLimit.

You also need to handle what happens when a public repo becomes private and vice versa - see models/repo.go:1417:func updateRepository(...).

There is also the issue of ForkedRepos when their base repos have their visibility changed to private then the fork has to become private. How does that affect this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comments, I'm still working through the source code to see what's arranged where... and it was 02:30 when I was committing this. 🤣

My initial thought was to split them. You want to have a limit for private, public and total? That would mean a user could have 2 private, 2 public and 3 total for example... leading to faulty settings and yet another error case needing to be checked. Or is that not what you meant?

As for the ForkedRepos.... When a user forks a repo, doesn't the fork become a mostly separate copy in its own right? I haven't tested it, but I'd be surprised if, on GitHub, a forked repo suddenly becomes private if the original repo does so. I don't really see the point of forcing forks to become private since, instead of forking, someone could just clone the original when it was still public and then push it as a new public repo with the same content as the original. Effectively bypassing the "forked" status.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to have a limit for private, public and total? That would mean a user could have 2 private, 2 public and 3 total for example... leading to faulty settings and yet another error case needing to be checked.

I would read that is: A user would be allowed 3 repositories in total: 2 private, 1 public or 1 private, 2 public - up to the user how that's split - it would mean that you could make things private or public as necessary.

As for the ForkedRepos....

On Gitea if the parent repo becomes private - your fork will become private. You can break the fork relationship, and then make it public if you like.

This is particularly important if you have forked an organizations repos - a fork of a private organisation's repos is private even if the repo is a "public" repo of that organization. So if you make an organization private then all repos forked from its repos will become private.

@6543
Copy link
Member

6543 commented Sep 6, 2020

just idears: different limits for: forks, public and private repos
-> let's say a user is allowd to have 1 private, 10 public and kan fork as he like ( to contribute to other projects ... )

note: forks can be conferted to normal repos, before this hapen a check has to happen again too ...

@mvdkleijn
Copy link
Author

just idears: different limits for: forks, public and private repos
-> let's say a user is allowd to have 1 private, 10 public and kan fork as he like ( to contribute to other projects ... )

note: forks can be conferted to normal repos, before this hapen a check has to happen again too ...

For the purposes of this PR, I'm going to limit it to public and private repo counts. A forked repo is also either public or private so it'll count against either the public or the private count.

mvdkleijn and others added 20 commits September 9, 2020 21:16
* deleteIssuesByRepoID: delete related CommentTypeRemoveDependency & CommentTypeAddDependency comments too

* Ignore ErrIssueNotExist on comment.LoadDepIssueDetails()

* Add migration

* Ignore 'dependent_issue_id = 0' case

* exchange as per @lunny

Co-authored-by: techknowlogick <[email protected]>
* Changelog for 1.12.4 release (#12687)


Co-authored-by: zeripath <[email protected]>

* update gitea version in docs

Co-authored-by: techknowlogick <[email protected]>
Co-authored-by: zeripath <[email protected]>
Co-authored-by: zeripath <[email protected]>
…12700)

* Add github api token option to generate-license & generate-gitignore

Without api toke, Will face rate limit sometimes.

Signed-off-by: a1012112796 <[email protected]>

* Use Basic authentication with tokens

Signed-off-by: Andrew Thornton <[email protected]>

Co-authored-by: a1012112796 <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
* API: Expose its limitation settings

* TESTs

Co-authored-by: zeripath <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
* Bump min required golang to 1.13

* Update config.yaml

* Update Makefile

* per silverwind feedback

* per silverwind

Co-authored-by: zeripath <[email protected]>
Signed-off-by: Andrew Thornton <[email protected]>

Co-authored-by: techknowlogick <[email protected]>
GiteaBot and others added 24 commits September 9, 2020 21:17
Provides new command: `gitea doctor recreate-table` which will recreate
db tables and copy the old data in to the new table.

This function can be used to remove the old warning of struct defaults being
out of date.

Fix #8868
Fix #3265
Fix #8894

Signed-off-by: Andrew Thornton <[email protected]>
* Fix bug on migration 111

* Upgrade bleve to 1.0.10

Co-authored-by: zeripath <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
…es (#12752)

* Set setting.AppURL as GITEA_ROOT_URL environment variable during pushes

Fix #11738

Signed-off-by: Andrew Thornton <[email protected]>
* Add queue for code indexer

* Fix lint

* Fix test

* Fix lint

* Fix bug

* Fix bug

* Fix lint

* Add noqueue

* Fix tests

* Rename noqueue to immediate
* gitea dump: include version

* Check InstallLock (close #12759)

* fix test

* fix lint
Right now we only compare the hostname from a submodule with the prefixURL it is viewed from to check if the submodule is hosted on the same Gitea instance. This adds an additional check to compare it against SSH_DOMAIN as well since the same Gitea instance might have a different hostname for SSH and if the submodule uses that hostname we should also detect that and link to the proper DOMAIN value.

Fixes #12747, #9756
We now have a helm chart that users should use instead
* LFS support to be stored on minio

* Fix test

* Fix lint

* Fix lint

* Fix check

* Fix test

* Update documents and add migration for LFS

* Fix some bugs
* Add field with isIssueWriter to front end

* Make branch field editable

* Switch frontend to form and POST from javascript

* Add /issue/id/ref endpoint to routes

* Use UpdateIssueTitle model to change ref in backend

* Removed crossreference check and adding comments on branch change

* Use ref returned from POST to update the field

* Prevent calling loadRepo from models/

* Branch/tag refreshed without page reload

* Remove filter for empty branch name

* Add clear option to tag list as well

* Delete button translation and coloring

* Fix for not showing selected branch name in new issue

* Check that branch is not being changed on a PR

* Change logic

* Notification when changing issue ref

* Fix for renamed permission parameter

* Fix for failing build

* Apply suggestions from code review

Co-authored-by: zeripath <[email protected]>

Co-authored-by: Gitea <[email protected]>
Co-authored-by: zeripath <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
- replace two instances of fontawesome with octicons
- add new "class" optional argument to "svg" helper
- add many new CSS helpers and move their import to the end for
  increaseed precedence

Co-authored-by: zeripath <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
* Implement pwn

Signed-off-by: jolheiser <[email protected]>

* Update module

Signed-off-by: jolheiser <[email protected]>

* Apply suggestions mrsdizzie

Co-authored-by: mrsdizzie <[email protected]>

* Add link to HIBP

Signed-off-by: jolheiser <[email protected]>

* Add more details to admin command

Signed-off-by: jolheiser <[email protected]>

* Add context to pwn

Signed-off-by: jolheiser <[email protected]>

* Consistency and making some noise ;)

Signed-off-by: jolheiser <[email protected]>

Co-authored-by: mrsdizzie <[email protected]>
Co-authored-by: zeripath <[email protected]>
Following further testing it has become apparent that the diff line
cannot be used to determine filenames for diffs with any sort of predictability
the answer therefore is to use the other lines that are provided with a diff

Fix #12768

Signed-off-by: Andrew Thornton <[email protected]>
* Add MySQL FAQ section

Signed-off-by: Andrew Thornton <[email protected]>

* Update docs/content/doc/help/faq.en-us.md

* Update docs/content/doc/help/faq.en-us.md

Co-authored-by: mrsdizzie <[email protected]>

Co-authored-by: mrsdizzie <[email protected]>
* Add a migrat service type switch page

* Improve translations

* remove images

* Fix images

* remove extra create repo button on dashboard

* Follow reviewers' opinions

* Fix frontend lint

* Remove wrong submit file

* Fix tests

* Adjust the size of image

* Apply suggestions from code review

Co-authored-by: 赵智超 <[email protected]>

* Remove username and password from migration of github/gitlab

* Improve docs

* Improve interface docs

Co-authored-by: 赵智超 <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
@mvdkleijn
Copy link
Author

Right! F-ed this up good. A combination of laptop performance issue, a fault between keyboard and chair as well as a four year old screaming in my ears. 🤣

I'll close this PR and re-create a new one that's not f-ed up. Sorry about this! 😄

@mvdkleijn mvdkleijn closed this Sep 9, 2020
@mvdkleijn mvdkleijn deleted the limit-private-repos branch September 9, 2020 19:32
@mvdkleijn mvdkleijn mentioned this pull request Sep 10, 2020
@lunny lunny removed this from the 1.14.0 milestone Sep 10, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit maximum count of private or public repositories for individual users