Moves Protected paths throttling from Omnibus to GitLab Rails
What does this MR do?
Moves protected paths to GitLab-Rails:
- Adds 4 columns to
application_settings
- 3 to mimic the configuration of existing throttles
- 1 to store the protected paths on database
- Set default protected paths (taken from Omnibus)
- Add new section on admin panel to personalize protected paths configuration
- This new throttle is active by default.
- Includes additional protected paths throttles
- Rack Attack file was renamed to
rack_attack_new.rb
, otherwise the Omnibus file will overwrite this file. - If the Omnibus settings are present, the application settings are ignored.
Related to #29952 (closed)
Documentation MR - !16540 (merged)
Screenshots
With Omnibus throttle present | Without Omnibus throttle |
---|---|
![]() |
![]() |
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry for user-facing changes, or community contribution. Check the link for other scenarios. -
Documentation created/updated or follow-up review issue created - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/32773 -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Performance and testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. - [-] Tested in all supported browsers
To do - backend
-
Move Protected Paths to config/gitlab.yml
-
Handle the throttle on RackAttackGlobal
-
Add specs -
Log error on auth.log
-
Add new application_settings to UI -
Refactor -
Configure protected paths on UI
Merge request reports
Activity
changed milestone to %12.3
added backend backstage [DEPRECATED] gitlab.com + 1 deleted label
added database databasereview pending labels
1 Message This merge request adds or changes files that require a review from the Database team. This merge request requires a database review. To make sure these changes are reviewed, take the following steps:
- Ensure the merge request has database and databasereview pending labels. If the merge request modifies database files, Danger will do this for you.
- Use the Database changes checklist template or add the appropriate items to the MR description.
- Assign and mention the database reviewer suggested by Reviewer Roulette.
The following files require a review from the Database team:
db/migrate/20190801142441_add_throttle_protected_path_columns.rb
db/schema.rb
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.
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 James Fargher ( @proglottis
)Thong Kuah ( @tkuah
)frontend Coung Ngo ( @cngo
)Paul Slaughter ( @pslaughter
)database Adam Hegyi ( @ahegyi
)Andreas Brandl ( @abrandl
)Generated by
DangerEdited by 🤖 GitLab Bot 🤖added 169 commits
-
85c2a3fc...212cfbc4 - 168 commits from branch
master
- f75f461c - Moves protected path to gitlab-rails
-
85c2a3fc...212cfbc4 - 168 commits from branch
added 16 commits
-
b1c8a531...7ba91a0f - 15 commits from branch
master
- 6836fecf - Moves protected path to gitlab-rails
-
b1c8a531...7ba91a0f - 15 commits from branch
added databaseapproved label and removed databasereview pending label
mentioned in issue #29952 (closed)
added 21 commits
-
6836fecf...ce5554c3 - 20 commits from branch
master
- 7d1992c5 - Moves protected path to gitlab-rails
-
6836fecf...ce5554c3 - 20 commits from branch
@stanhu could you please take another look? I've addressed all the discussions made on the CE version https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31246.
FYI: Documentation (https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/16540/) and Omnibus deprecation (omnibus-gitlab!3597 (merged), omnibus-gitlab#4688 (closed)) are being handled on different issues / MR's
assigned to @stanhu
- Resolved by Stan Hu
- Resolved by Stan Hu
Looks good to me! Perhaps we should build an Omnibus package to ensure that our understanding of how these settings play together is correct?
added 329 commits
-
489e4ff8...186010cf - 328 commits from branch
master
- adf5f8c3 - Moves protected path to gitlab-rails
-
489e4ff8...186010cf - 328 commits from branch
- Resolved by Stan Hu
unassigned @stanhu
changed milestone to %12.4
added 625 commits
-
49093164...ed4d4353 - 624 commits from branch
master
- f5ab3d70 - Moves protected path to gitlab-rails
-
49093164...ed4d4353 - 624 commits from branch
- Resolved by Stan Hu
thanks for all your help @stanhu
! Could you please take another look?assigned to @stanhu
- Resolved by Stan Hu
unassigned @mayra-cabrera
assigned to @mayra-cabrera and unassigned @stanhu
added 567 commits
-
bafbb05b...73d7f5fc - 566 commits from branch
master
- bb7bbf2c - Moves protected path to gitlab-rails
-
bafbb05b...73d7f5fc - 566 commits from branch
@stanhu could you please take another look? I've responded to your inquiry on !16463 (comment 220195993)
assigned to @stanhu