Skip to content

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:

  1. Project transfer.
  2. Group transfer.
  3. 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:

Screenshot_2023-07-20_at_21.26.47

With this MR:

Screenshot_2023-07-20_at_21.28.27

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 the group path.
    • 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.

💾 Database review

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) ====
Edited by David Fernandez

Merge request reports