389-ds-base

#50409 Issue 50052 - Add package-lock.json and use "npm ci"
Closed by spichugi. Opened by spichugi.
spichugi/389-ds-base npm-lock  into  master

Download 50409.patch

Bug description: All software changes incur some risk,
and it's critical to be able to manage this risk.
We can use a common way of dealing with it - npm-shrinkwrap.

Fix description: The suggested approach - npm-shrinkwrap - is an "overkill"
for our case. We don't need to publish the package on NPM.
It will be sufficient enough to use existing NPM functionality added in 5.7 version.

Replace npm install with npm ci which uses package-lock.json
and throws an error if any inconsistencies with pachage.json are found.
Add package-lock.json to the repo.
When we change the package.json content, a new pachage-lock.json should be
generated (using npm install) and the change should be commited.

https://pagure.io/389-ds-base/issue/50052

Reviewed by: ?

LGTM

I'd like to take this a bit further and incorporate npm audit-ci in our build process.
Currently we have known vulnerabilities in our dependency tree:

added 1082 packages from 741 contributors and audited 9442 packages in 22.219s
found 37 vulnerabilities (36 moderate, 1 high)
  run `npm audit fix` to fix them, or `npm audit` for details

audit-ci is similar to npm ci, but will stop the build if it finds vulnerabilities above configured level.

1 new commit added

  • Add audit-ci tool, fix audit issues, add "repository" field

I'd like to take this a bit further and incorporate npm audit-ci in our build process.
Currently we have known vulnerabilities in our dependency tree:
added 1082 packages from 741 contributors and audited 9442 packages in 22.219s
found 37 vulnerabilities (36 moderate, 1 high)
run npm audit fix to fix them, or npm audit for details

audit-ci is similar to npm ci, but will stop the build if it finds vulnerabilities above configured level.

Nice catch! I've added it.
I just hope it won't get in a way of shipping RPMs on time (some small issue will pop up in the audit and it'll block the process).
For now, I've set the moderate vulnerability level but I think it's possible we will have to move it to high in the future.
What do you think?

2 new commits added

  • Add audit-ci tool, fix audit issues, add "repository" field
  • Issue 50052 - Add package-lock.json and use "npm ci"

@spichugi Good thinking. However, I think the "hope" not being fulfilled might cost a lot in the future. Additionally this seems to me as going against reproducible build idea. Could we make the audit only a part of e.g. make check? Running that in nightly CI would result in much smoother experience, I believe.

Disregard my previous comment. Seems like we run this before e.g. creating upstream tarball (thanks Simon for clarification), which is ok as we don't want to release known vulnerable code.

Once more, LGTM.

LGTM too, thanks!

rebased onto 255faf9377afe9a4552b17aacfda961e66c6fafe

Pull-Request has been merged by spichugi

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3467

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.