Skip to content

Add npm dependencies support

David Fernandez requested to merge 10io-add-npm-dependencies-support into master

What does this MR do?

This MR is a MR split from !14263 (closed). A partial review has been conducted there and the relevant feedback was included in this MR.

This MR will add support for package dependencies when npm CLI interacts with the backend.

On package upload, the dependencies are extracted and persisted to Packages::PackageDependency and Packages::PackageDependencyLink.

When npm CLI requests the metadata of a given package, dependencies are properly added in the json response.

In short, this MR adds the models and services to add support for package dependencies. In addition, it updates NpmPackagePresenter to include those dependencies if they exist.

Note that the models used to persist dependencies will be reused for other package managers (eg. NuGet will need such structures). That's why dependency models are not "scoped" for NPM (they don't have the Npm prefix).

related issues: #11867 (closed), #10223 (closed)

Screenshots

Say that I have an npm package @root/bacon on GitLab that has the following dependencies in its package.json:

"dependencies": {
  "strpad": "1.1.0"
}

Now, I'm creating a new project that references @root/bacon as a dependency.

Currently, when running npm install, here is the package-lock.json:

{
  "name": "@root/client",
  "version": "1.0.0",
  "lockfileVersion": 1,
  "requires": true,
  "dependencies": {
    "@root/bacon": {
      "version": "1.0.1",
      "resolved": "http://172.22.22.60:3000/api/v4/projects/9/packages/npm/@root/bacon/-/@root/bacon-1.0.1.tgz",
      "integrity": "sha1-qvGQzw665r5qS6rNKZU/XGfiCs8="
    }
  }
}

-> strpad is not listed as a dependency.

Running npm install again fixes the issue (npm will use the local @root/bacon's package.json file instead of query the API):

{
  "name": "@root/client",
  "version": "1.0.0",
  "lockfileVersion": 1,
  "requires": true,
  "dependencies": {
    "@fav/text.pad": {
      "version": "1.0.3",
      "resolved": "https://registry.npmjs.org/@fav/text.pad/-/text.pad-1.0.3.tgz",
      "integrity": "sha512-Khnfzn9fy8+IMFGRKOkdAmT0LINvLdTi2LeTcd17OSFzWFDcwexkXw9MAJn/HNaWhzVKyiEDrt/n/6pS6gK/4A==",
      "requires": {
        "@fav/text.repeat": "^1.0.1"
      }
    },
    "@fav/text.repeat": {
      "version": "1.0.4",
      "resolved": "https://registry.npmjs.org/@fav/text.repeat/-/text.repeat-1.0.4.tgz",
      "integrity": "sha512-ztFSCghMPnFllWGcCMNh/cq6uA0iKWLPNjMYEknxfmEsUB1JsohP9z5tw3LVOKt06hlHRhPk35+NgVQ8rWb6hA=="
    },
    "@root/bacon": {
      "version": "1.0.1",
      "resolved": "http://172.22.22.60:3000/api/v4/projects/9/packages/npm/@root/bacon/-/@root/bacon-1.0.1.tgz",
      "integrity": "sha1-qvGQzw665r5qS6rNKZU/XGfiCs8=",
      "requires": {
        "strpad": "1.1.0"
      }
    },
    "strpad": {
      "version": "1.1.0",
      "resolved": "https://registry.npmjs.org/strpad/-/strpad-1.1.0.tgz",
      "integrity": "sha512-PXZkRgABWbET43QEonbtSIktJ0+nZAtr3uAA+PtqfhP+Dkjh+Drg7pkQGxYApfvt2Jh4x9A3Hh4tQN/dIuWREA==",
      "requires": {
        "@fav/text.pad": "^1.0.3"
      }
    }
  }
}

-> strpad and its dependencies are properly declared.

With this MR, the first npm install gives:

{
  "name": "@root/client",
  "version": "1.0.0",
  "lockfileVersion": 1,
  "requires": true,
  "dependencies": {
    "@fav/text.pad": {
      "version": "1.0.3",
      "resolved": "https://registry.npmjs.org/@fav/text.pad/-/text.pad-1.0.3.tgz",
      "integrity": "sha512-Khnfzn9fy8+IMFGRKOkdAmT0LINvLdTi2LeTcd17OSFzWFDcwexkXw9MAJn/HNaWhzVKyiEDrt/n/6pS6gK/4A==",
      "requires": {
        "@fav/text.repeat": "^1.0.1"
      }
    },
    "@fav/text.repeat": {
      "version": "1.0.4",
      "resolved": "https://registry.npmjs.org/@fav/text.repeat/-/text.repeat-1.0.4.tgz",
      "integrity": "sha512-ztFSCghMPnFllWGcCMNh/cq6uA0iKWLPNjMYEknxfmEsUB1JsohP9z5tw3LVOKt06hlHRhPk35+NgVQ8rWb6hA=="
    },
    "@root/bacon": {
      "version": "1.0.1",
      "resolved": "http://172.22.22.60:3000/api/v4/projects/9/packages/npm/@root/bacon/-/@root/bacon-1.0.1.tgz",
      "integrity": "sha1-qvGQzw665r5qS6rNKZU/XGfiCs8=",
      "requires": {
        "strpad": "1.1.0"
      }
    },
    "strpad": {
      "version": "1.1.0",
      "resolved": "https://registry.npmjs.org/strpad/-/strpad-1.1.0.tgz",
      "integrity": "sha512-PXZkRgABWbET43QEonbtSIktJ0+nZAtr3uAA+PtqfhP+Dkjh+Drg7pkQGxYApfvt2Jh4x9A3Hh4tQN/dIuWREA==",
      "requires": {
        "@fav/text.pad": "^1.0.3"
      }
    }
  }
}

-> strpad and its dependencies are properly declared on the first execution of npm install.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Database checklist

When adding migrations:

  • Updated db/schema.rb
  • Added a down method so the migration can be reverted
  • Added the output of the migration(s) to the MR body
$ r db:migrate
**************************************************
⛔️ WARNING: Sidekiq testing API enabled, but this is not the test environment.  Your jobs will not go to Redis.
**************************************************
== 20191121111621 CreatePackagesDependencies: migrating =======================
-- create_table(:packages_dependencies)
   -> 0.0058s
-- add_index(:packages_dependencies, [:name, :version_pattern], {:unique=>true})
   -> 0.0027s
== 20191121111621 CreatePackagesDependencies: migrated (0.0086s) ==============

== 20191121121947 CreatePackagesDependencyLinks: migrating ====================
-- create_table(:packages_dependency_links)
   -> 0.0045s
-- add_index(:packages_dependency_links, [:package_id, :dependency_id, :dependency_type], {:unique=>true, :name=>"idx_pkgs_dep_links_on_pkg_id_dependency_id_dependency_type"})
   -> 0.0018s
== 20191121121947 CreatePackagesDependencyLinks: migrated (0.0063s) ===========

== 20191121122856 DropPackagesPackageMetadataTable: migrating =================
-- drop_table(:packages_package_metadata)
   -> 0.0011s
== 20191121122856 DropPackagesPackageMetadataTable: migrated (0.0011s) ========

== 20191121161018 AddProjectIdNameVersionPackageTypeIndexToPackagesPackages: migrating 
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:packages_packages, [:project_id, :name, :version, :package_type], {:name=>"idx_packages_packages_on_project_id_name_version_package_type", :algorithm=>:concurrently})
   -> 0.0023s
-- execute("SET statement_timeout TO 0")
   -> 0.0003s
-- add_index(:packages_packages, [:project_id, :name, :version, :package_type], {:name=>"idx_packages_packages_on_project_id_name_version_package_type", :algorithm=>:concurrently})
   -> 0.0025s
-- execute("RESET ALL")
   -> 0.0003s
== 20191121161018 AddProjectIdNameVersionPackageTypeIndexToPackagesPackages: migrated (0.0055s)
  • Added tests for the migration in spec/migrations if necessary (e.g. when migrating data)
  • Added rollback procedure. Include either a rollback procedure or description how to rollback changes

When adding or modifying queries to improve performance:

  • Included data that shows the performance improvement, preferably in the form of a benchmark
  • Included the output of EXPLAIN (ANALYZE, BUFFERS) of the relevant queries

When adding foreign keys to existing tables:

  • Included a migration to remove orphaned rows in the source table before adding the foreign key
  • Removed any instances of dependent: ... that may no longer be necessary

When adding tables:

  • Ordered columns based on the Ordering Table Columns guidelines
  • Added foreign keys to any columns pointing to data in other tables
  • Added indexes for fields that are used in statements such as WHERE, ORDER BY, GROUP BY, and JOINs

When removing columns, tables, indexes or other structures:

  • Removed these in a post-deployment migration
  • Made sure the application no longer uses (or ignores) these structures
Edited by David Fernandez

Merge request reports