Protected packages: Add model and migration for package protection rules
requested to merge gitlab-community/gitlab:416382-protected-packages-introducing-ff-and-basics-for-protecting-packages into master
What does this MR do and why?
- This MR is related to issue Protected packages: Add basic model and migrati... (#416382).
- This MR introduces the basic backend components for protecting packages, e.g. database migration, model, etc.
- The scope of the basic functionality was discussed in the epic: &5574
Screenshots or screen recordings
No visuals to show. This MR contains mostly backend-related code, e.g.:
Packages::Protection::Rule.create!(
project: Project.find_by_full_path("flightjs/Flight"),
package_name_pattern: "flightjs/test_npm_package*",
package_type: :npm,
push_protected_up_to_access_level: Gitlab::Access::OWNER
)
How to set up and validate locally
rails db:migrate
- Open the rails console (
rails c
) and start playing around with the new model
Packages::Protection::Rule.create!(
project: Project.find_by_full_path("flightjs/Flight"),
package_name_pattern: "flightjs/protected_npm_package*",
package_type: :npm,
push_protected_up_to_access_level: Gitlab::Access::OWNER
)
TODO
-
Add basic tests for model -
Implement wildcard pattern matching to compare packages with package protection rules (incl. tests) -
Implement policy for package protection and respective test cases (incl. tests) -
Initiate discussion about pattern matching in sql how it is done? Are there other preferred ways to do the pattern matching? ANSWER: !124776 (comment 1460445274) -
Do we support packages with whitesapces? Protected branches allows this ... -
Create issue for limiting number of package protection rules, see #418000 -
Implement PackageProtectionRulesFinder
to extract matching logic, see !126209 (closed) -
Check if user role admin has to be included in push_protected_up_to_...
, see !124776 (comment 1481017665) => ANSWER: As stated in the docs, GitLab administrators have all permissions. This means that push protection should be disregarded for GitLab admins. See https://docs.gitlab.com/ee/user/permissions.html -
Check if deploy_token and job_tokens are considered, see !124776 (comment 1481017651) => ANSWER: Yes, deploy tokens are important and should be handled. We decided to work on deploy tokens as part of another issue. For now, we just disregard deploy tokens and ensure the policy does not break when a deploy token is passed / given. -
Use before
instead ofbefore_all
when stubbing a feature flag in a test -
Add validation to limit package_name_pattern
-
Remove the changes related to Packages::PackagePolicy
, see !124776 (comment 1498621743) -
Update the migration to be up to date
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. -
Changelog entry added, if necessary -
Documentation created/updated via this MR -
Documentation reviewed by technical writer or follow-up review issue created -
Tests added for this feature/bug -
Tested in all supported browsers -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides -
Conforms to the javascript style guides -
Conforms to the database guides
-
Related to #416382
Edited by Gerardo Navarro