Using a package-lock.json for Foreman

Hi, in the last UX interest group meeting @Dewar suggested adding a package-lock.json to Foreman
to speed up installation and also to provide a more stable environment.
we can update the lock file each time there is a new version bump of @theforeman/vendor and after we test that everything still works.

some numbers:

  • without Foreman lock file:
    added 4254 packages from 2661 contributors and audited 4262 packages in 199.281s

  • with Foreman lock file ( the lock file only contains foreman dependencies, not plugins ):
    added 3821 packages in 77.409s

That’s a two-minute diff (on a good machine!)

any reason not to move to package-lock.json?

  • A lot of noise to update it every time in git. Noise is PRs and more commits in git log
  • A lot more PRs and builds means a lot more load on our CI (which is already stressed)
  • Huge increase in our Git repository size. The current package-lock.json is 1.48 MB. If it changes a lot (and with JS it does), that really makes our repository a lot bigger. It is cloned every time in CI as well and we can’t remove anything (because it’s git)

I also wonder how a package-lock.json in git will behave with plugins. In vanilla Foreman itself we will have a different package set than if you have Katello installed as well. Will that mean git diff will by definition give a different set?

So a concrete test:

  • Have a pristine foreman.git checkout
  • bundle install + npm install
  • git add package-lock.json
  • Add a plugin with webpack
  • bundle install + npm install

Now is there a diff? If so, that means we simply can’t include package-lock.json because it will be different for various developers.

All in all I can’t stress how much I am against this.

yes, it will make more PRs, but also a more stable environment.

about the diff, this is what I did:

  • remove the postinstall script (prevent plugins from being install)
  • run npm install
  • remove package-lock.json from .gitignore
  • stage package-lock.json
  • returned the postinstall script
  • run npm ci

results: There was no diff in the package-lock.json.
looks like the posinstall script install the node_modules of plugins.
and I guess that webpack knows how to handle and build each plugin properly with their node_modules. maybe @MariaAga can add more about it?

today we still do a lot of maintenance on version bumps, when tests starting to fail
e.g: last nightly failure in Katello that was caused by Foreman dependency: Fixes #33265 - Katello Nightly Failure by Andrewgdewar · Pull Request #8716 · theforeman/foreman · GitHub</titl

I think there wont be an issue with it as each plugin has it’s own package-lock.json.
But if I remember the Katello team tried to pin a package in the katello package-lock.json and it didn’t work? @Dewar

That’s correct, pinning versions in Katello doesn’t seem to work.

My understanding of things is as follows:

Katello inherits node_modules and their versions from Foreman, regardless of what we do in Katello as far as I’ve seen. The only benefit I can see of our local node modules is to add packages not in foreman or foreman-js.

Commiting the package-lock to foreman will help to stabilize Katello’s node modules and prevent similar test breakages and/or nightly failures in the future. Dependencies and their working versions are stored there, and if someone updates the package-lock with breaking versions, tests will fail preventing those versions from being possible to commit to git.

And to be clear, all tests were failing in CI as well as locally in Katello, so this wasn’t just a local issue.

1 Like

Sorry to hear that so many people are arguing against best practices here

IMHO there are two major things that are missing when not having a package-lock.json are:

  • reproducability
  • speed

About updating the package-lock.json: This should best be done by dependabot (unless when adding a new package) here the frequency of updates (PRs) can be set: Dependabot version updates can now ignore major/minor/patch releases - The GitHub Blog

Even if it might not be possible for foreman or katello it might be possible to recommend/enforce the package-lock.json for all plugins?

Since this thread was created almost 3 years ago maybe in the meantime some of the arguments against having a package-lock.json have changed.

When we all discussed this the best practices were for monolithic applications while we are maintaining a pluggable application. The tooling simply wasn’t a good fit.

I still think the current way we deal with plugins and NPM can be considered a hack. I know @MariaAga is working on updating to webpack 5 and after that look at properly exposing the Foreman code as a real module that can be reused in plugins. Only then can you say that webpack understands the relations. Until then, having a package-lock.json is IMHO just fooling yourself and giving a false sense of sanity.


Thank you for the clarification. In that case we can tackle/revisit this when the refactoring is finished.

Here is a reference implementation for a plugin using package-lock.json together with dependabot

By using package-lock.json build time has dropped significantly (> 50%)