Fix group path update validations with NPM packages
🍬 Context
In the first iterations, the npm Registry required packages to follow the naming convention. In short words, given a project full path root/subgroup/project
, the NPM package name was required to be @root/<package_name>
.
Because of this requirement, these operations:
- Project transfer.
- Group transfer.
- Group path update.
would check for NPM packages. If there were NPM packages, the operation would be rejected.
We lifted that requirement in Lift the NPM package naming convention for the ... (#33685 - closed).
So users could have packages not following the naming convention. Examples, assuming the previous full path: @test/<package_name>
or even <package_name>
(no scope).
These packages don't present an issue for the above operations. Guess what happened? Yep, we forgot to update the validations in those operations
NPM packages not following the naming conventio... (#404764 - closed) fixed operations (1.) and (2.).
(3.) was left behind.
I assume that (3.) is not a very common operation since we didn't notice this typebug until today. That's issue NPM packages not following the naming conventio... (#419289 - closed).
This MR aims to fix (3.)
To be extra precise here, we need to block group path updates when:
- A root group is the target of the update and
- namespaced NPM packages (packages where the
@
is the group path).
Note that subgroup path updates should be ok.
🤔 What does this MR do and why?
- Update the NPM packages validation in the group update service.
- Update the related specs.
- These changes are behind a feature flag. Rollout issue: #420160 (closed)
📺 Screenshots or screen recordings
Previously, updating a subgroup path:
With this MR:
⚗ How to set up and validate locally
Have
- a group
group
- a subgroup
subgroup
- a project
project
So that we have group/subgroup/project
.
Now, upload NPM packages to project
(you can use https://gitlab.com/10io/gl_pru for this):
@group/test
test
@this_is_a_scope/test
Now, update the path in the group settings page (General Settings
-> Advanced
-> Change Group URL
):
- Update the
subgroup
path. This update will go through✅ - This is a subgroup -> the NPM packages are not considered here.
- Update the
group
path. This update will not go through⛔ .- We have a namespaced package: this update in the root group is not allowed.
- Delete the package
@group/test
(Package Registry -> delete button next to the package). Try again to update thegroup
path.- This time around, it goes through.
✅
- This time around, it goes through.
🏁 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.
💾 Database review
- Existence check: !127164 (comment 1488794854)
⤴ Migration up
$ rails db:migrate
main: == [advisory_lock_connection] object_id: 222200, pg_backend_pid: 18362
main: == 20230726072442 AddNpmScopeAndProjectIndexToPackages: migrating =============
main: -- transaction_open?()
main: -> 0.0001s
main: -- view_exists?(:postgres_partitions)
main: -> 0.0464s
main: -- index_exists?(:packages_packages, "split_part(name, '/', 1), project_id", {:where=>"package_type = 2 AND position('/' in name) > 0 AND status IN (0, 3) AND version IS NOT NULL", :name=>"idx_packages_packages_on_npm_scope_and_project_id", :algorithm=>:concurrently})
main: -> 0.0070s
main: -- execute("SET statement_timeout TO 0")
main: -> 0.0002s
main: -- add_index(:packages_packages, "split_part(name, '/', 1), project_id", {:where=>"package_type = 2 AND position('/' in name) > 0 AND status IN (0, 3) AND version IS NOT NULL", :name=>"idx_packages_packages_on_npm_scope_and_project_id", :algorithm=>:concurrently})
main: -> 0.0015s
main: -- execute("RESET statement_timeout")
main: -> 0.0003s
main: == 20230726072442 AddNpmScopeAndProjectIndexToPackages: migrated (0.0657s) ====
main: == [advisory_lock_connection] object_id: 222200, pg_backend_pid: 18362
⤵ Migration down
$ rails db:rollback
main: == [advisory_lock_connection] object_id: 222020, pg_backend_pid: 18077
main: == 20230726072442 AddNpmScopeAndProjectIndexToPackages: reverting =============
main: -- transaction_open?()
main: -> 0.0000s
main: -- view_exists?(:postgres_partitions)
main: -> 0.0497s
main: -- indexes(:packages_packages)
main: -> 0.0087s
main: -- execute("SET statement_timeout TO 0")
main: -> 0.0003s
main: -- remove_index(:packages_packages, {:algorithm=>:concurrently, :name=>"idx_packages_packages_on_npm_scope_and_project_id"})
main: -> 0.0012s
main: -- execute("RESET statement_timeout")
main: -> 0.0002s
main: == 20230726072442 AddNpmScopeAndProjectIndexToPackages: reverted (0.0709s) ====