Draft: Protected packages: Adding finder for PackageProtectionRule [Follow-up]
requested to merge gitlab-community/gitlab:416997--protected-packages-introducing-ff-and-basics-for-protecting-packages-follow-up-finder into master
What does this MR do and why?
- This MR is a refactoring follow-up for the MR #416382 .
- This MR intends to refactor the
PackagePolicy
because the logic for matching packages was concentrated in the classPackagePolicy
. However, this class should be kept light-weight. - We moved this logic to a separate finder
PackageProtectionRuleMatchingFinder
for better reuse by other components.
Screenshots or screen recordings
- Introduced a new service class
PackageProtectionRuleMatchingFinder
that can be used to match the existing package protection rules for a given package. - This finder is integrated in the package model and can be used in other classes, e.g.
Packages::Npm::CreatePackageService
How to set up and validate locally
This MR is just a refactoring follow-up for the MR !124776 (merged) . For local validation, please see !124776 (merged) .
- Ensure you have an existing project and an existing project developer user
- Create a package protection rule to protect package from project developers
Packages::PackageProtectionRule.create!(
project: project,
package_name_pattern: "flightjs/test_npm_package*",
package_type: :npm,
push_protected_up_to_access_level: Gitlab::Access::DEVELOPER
)
- Use the
Packages::PackageProtectionRulePushFinder
to find an appropriate package protection rule
package = project.packages.build(package_type: :npm, name: "flightjs/test_npm_package")
package.push_protected_from?(user_project_developer) # => this will use `Packages::PackageProtectionRulePushFinder`
# => Should be true because the package protection rule matches
package.push_protected_from?(user_project_owner) # => this will use `Packages::PackageProtectionRulePushFinder`
# => Should be false because the package protection rule only applies to developers
- Try out the extended
Packages::PackagePolicy
that checks if a specific user is allowed to push to a protected package.
user = User.find_by(email: "admin@example.com")
project = Project.find_by_full_path("flightjs/Flight")
Packages::Protection::Rule.matching_package_name("flightjs/protected_npm_package", PackageProtectionRule.all).any?
# => should be true
Packages::Protection::Rule.matching_package_name("flightjs/protected_npm_package_any_wildcard", PackageProtectionRule.all).any?
# => should be true
Packages::Protection::Rule.matching_package_name("flightjs/npm_package", PackageProtectionRule.all).any?
# => should be false because no matching package protection rule exists
TODO
-
Rebase from !124776 (merged) -
Add tests with admin mode enabled to ensure that admins are able to push to protected packages
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 -
Wait for MR Protected packages: Add model and migration for... (!124776 - merged)
-
Related to #416997
Edited by Gerardo Navarro