Add lock file version check to CI

[mtrezza]

New Pull Request Checklist

• I am not disclosing a vulnerability.
• I am creating this PR in reference to an issue.

Issue Description

Ci does not check NPM lock file version; from NPM 7 onwards the version changed.

Related issue: closes #7326

Approach

Adds lock file version check to CI.

TODOs before merging

none

[mtrezza]

Check confirmed: fails if package-lock file version is 2, passes if version is 1.

[codecov]

Codecov Report

Merging #7333 (1d63680) into master (bf732b9) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7333      +/-   ##
==========================================
+ Coverage   93.88%   93.90%   +0.02%     
==========================================
  Files         181      181              
  Lines       13194    13194              
==========================================
+ Hits        12387    12390       +3     
+ Misses        807      804       -3     

Impacted Files	Coverage Δ
src/RestWrite.js	93.76% <0.00%> (ø)
src/Adapters/Files/GridFSBucketAdapter.js	80.32% <0.00%> (+0.81%)	⬆️
src/ParseServerRESTController.js	98.50% <0.00%> (+1.49%)	⬆️
src/batch.js	93.10% <0.00%> (+1.72%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf732b9...1d63680. Read the comment docs.

[dplewis]

We should upload a v2 package-lock file and check for that. If a v1 is uploaded for some reason this check should fail.

v2 is backwards compatible with npm 6 no?

[mtrezza]

v2 is compatible with npm >=7
v1 is used with npm <7

I checked uploading a v2 and check fails, as expected.

v2 is backwards compatible with npm 6 no?

Don't think so, see #7079 (comment); I think npm 7 can interpret v1 but npm 6 cannot interpret v2. npm 7 is bundled with node 15; as long as we have any test with node <15, we have to stick with v1 I believe.

[dplewis]

https://github.blog/2021-02-02-npm-7-is-now-generally-available/#changes-to-the-lockfile

One change to take note of is the new lockfile format, which is backwards compatible with npm 6 users.

mansona/npm-lockfile-version@v1

I think there is a reason why it default to npm >= 7

[mtrezza]

So locally generated v2 lock file with npm 7 should be interpretable by node 6?

I think we can only try this out and see if we run into the same issue as #7079 (comment) or that was another hiccup.

[dplewis]

I think @Moumouls was worried about the rewriting not so much as the compatibility

ensure that Parse Server lock is not changed/rewrited

[mtrezza]

If a local npm i replaces the v1 lock file with v2, but v2 is backwards compatible with v1, it shouldn't matter. We wouldn't need any check for this because also for the Node 15 test we don't have a yarn.lock file.

Edit: I think I got it now, are you referring to the lock file diffs that mess up the lock file becoming a mix of v1 and v2? If so, I think you are right, we should make v2 the default, but maybe not so soon.

Once the majority of contributors run npm 7 and produce v2 files, the default should be v2. Once we do it, every contributor would have to use npm 7 to submit a valid lock file. Until then we have to enforce v1 I believe. I don't know whether a check is necessary or the CI will fail anyway, but a check has the benefits of giving a clear error message at least that the contributor can easily resolve on their own.

[dplewis]

We wouldn't need any check for this

Thats was my conclusion

[mtrezza]

@dplewis I edited my comment

[Moumouls]

@dplewis @mtrezza the issue came from some change on how npm seems to handle v2, some packages of parse-server seems to not work under lock v2 in CI env, that's the issue i encountered on my Auth PR.

Also i see in the CI log that some package raise some support warning during npm ci due to the v2 lock version.

Finally since NPM 7 was published in October 13th, 2020, it's too fresh i think and can break some developers installs.

[mtrezza]

@Moumouls Did you delete and recreate the lock file or just merge the diffs?

I don't see how the v2 lock would require any dependency compatibility. This should solely concern the host environment.

[Moumouls]

yes @mtrezza , full re generate (no merge operation). But on my side it works, here the CI fails because the Node version of the CI is 14, and node 14 is shipped with NPM 6, so the install fail. Also i don't know what really happen if you npm i parse-server that contain lock v2 under a project that use Node 14/Npm 6.

Also Node 15 is not LTS, but node 14/npm 6 is a LTS

[mtrezza]

here the CI fails because the Node version of the CI is 14, and node 14 is shipped with NPM 6

Shouldn't matter as v2 is supposed to be backwards compatible with npm 6 🤔

Maybe a bug, I will give it a shot with npm 7 and see if I can reproduce this.

[Moumouls]

Yes may be a bug, you are right i checked npm docs:

Note that the file format changed significantly in npm v7 to track information that would have otherwise required looking in node_modules or the npm registry. Lockfiles generated by npm v7 will contain lockfileVersion: 2.

No version provided: an "ancient" shrinkwrap file from a version of npm prior to npm v5.
1: The lockfile version used by npm v5 and v6.
2: The lockfile version used by npm v7, which is backwards compatible to v1 lockfiles.
3: The lockfile version used by npm v7, without backwards compatibility affordances. This is used for the hidden lockfile at node_modules/.package-lock.json, and will likely be used in a future version of npm, once support for npm v6 is no longer relevant.

https://docs.npmjs.com/cli/v7/configuring-npm/package-lock-json

[Moumouls]

Also @mtrezza these last months we encountered many issue with mongodb-runner package (here responsible of the issue on the lock v2), on my company we stopped to use it, and we just replaced it with a tiny package based on mongodb-memory-server that i can open source.

May be we can create a new tiny repo under parse community ie mongodb-testing

here the main file example:

process.env.MONGOMS_VERSION = "4.4.2";
const http = require("http");
const { MongoMemoryReplSet } = require("mongodb-memory-server");

const hostname = "127.0.0.1";
const port = 27272;

const run = async () => {
  const replSet = new MongoMemoryReplSet({
    instanceOpts: [
      {
        port: 27017, // port number for the instance
      },
    ],
    replSet: { storageEngine: "wiredTiger", dbName: "parse" },
  });
  await replSet.waitUntilRunning();
  console.info("MongDB Running");
  const server = http.createServer(async (req, res) => {
    res.statusCode = 200;
    res.setHeader("Content-Type", "text/plain");
    res.end("ok");
  });
  server.listen(port, hostname, async () => {
    await replSet.waitUntilRunning();
  });
};

run();

package.json

{
  "name": "mongodb-launcher",
  "version": "1.0.0",
  "main": "index.js",
  "author": "Company",
  "scripts": {
    "start": "node index.js"
  },
  "dependencies": {
    "mongodb-memory-server": "^6.9.3"
  }
}

Simple 🙂

[mtrezza]

The mongodb-memory-server seems to be well maintained. I guess we'd have to look at compatibility, especially with Docker (alpine), and what the resource / performance differences are. I suggest we can explore that in a separate issue if you want to pursue this.

[dplewis]

@Moumouls I'm all down for using mongodb-memory-server. I think only parse and a few other projects use mongodb-runner and it isn't that well maintained.

[Moumouls]

yeah, @dplewis if you are ok, i do not have rights to create a new repo under parse community. Then i can push the beginning of this tiny package or open an issue on it to draft main objectives.

[mtrezza]

How about we add this as a PR to Parse Server first before creating a separate repo? This will likely take some refinement to work in the CI, may be easier in a PR. And if we face an obstacle that prevents us from using it, we don't end up with an unused draft repo.

You can add it as a sub-package dev-dependency in spec/dependencies, so it will be simply to extract it later on.

[Moumouls]

No problem @mtrezza the 2 options seems good to me 👍

[mtrezza]

I cannot even build with npm 7 locally, after deleting package-lock and node_modules:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/graphql
npm ERR!   graphql@"15.5.0" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer graphql@"^0.5.0 || ^0.6.0 || ^0.7.0 || ^0.8.0-b || ^0.9.0 || ^0.10.0 || ^0.11.0 || ^0.12.0 || ^0.13.0 || ^14.0.0" from [email protected]
npm ERR! node_modules/graphql-relay
npm ERR!   graphql-relay@"0.6.0" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

Updated all to latest version and still have this issue. Install only works with --legacy-peer-deps, but that is effectively imitating npm 6 dependency resolving behavior. We actually have the same issue reported for some time already in Parse Dashboard: parse-community/parse-dashboard#1633.

@dplewis I think until we (or graphql) resolve this, we can set the required package-lock to v1?

[dplewis]

we can set the requires package-lock to v1

You will still get errors if somebody deletes the package-lock file.

[mtrezza]

You mean someone deletes the lock file without rebuilding it?

[dplewis]

I think we should stick to v1. I personally delete and regenerate the package-lock files when I close the Snyx PRs sometimes.

[mtrezza]

Uhm, that's what it currently does, it passes only for v1 (npm 6). So we can merge this?

[parseplatformorg]

🎉 This change has been released in version 5.0.0-beta.1

[parseplatformorg]

🎉 This change has been released in version 5.0.0