From 8571b3e68dd0293e0397202337f4e552583933b8 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Mon, 25 Oct 2021 13:19:17 +0200
Subject: [PATCH] feat: binarySource=docker avoid unstable tags (#12309)

---
 lib/manager/npm/post-update/lerna.ts          |  5 +--
 .../npm/post-update/node-version.spec.ts      | 40 -------------------
 lib/manager/npm/post-update/node-version.ts   | 39 ++----------------
 lib/manager/npm/post-update/npm.ts            |  2 +-
 lib/manager/npm/post-update/pnpm.ts           |  2 +-
 lib/manager/npm/post-update/yarn.ts           |  2 +-
 lib/util/exec/docker/index.spec.ts            | 10 +++++
 lib/util/exec/docker/index.ts                 |  5 +++
 8 files changed, 24 insertions(+), 81 deletions(-)

diff --git a/lib/manager/npm/post-update/lerna.ts b/lib/manager/npm/post-update/lerna.ts
index 2cd440498d..a7b3329cce 100644
--- a/lib/manager/npm/post-update/lerna.ts
+++ b/lib/manager/npm/post-update/lerna.ts
@@ -72,8 +72,7 @@ export async function generateLockFiles(
       lernaCommand = lernaCommand.replace('--ignore-scripts ', '');
     }
     lernaCommand += cmdOptions;
-    const allowUnstable = true; // lerna will pick the default installed npm@6 unless we use node@>=15
-    const tagConstraint = await getNodeConstraint(config, allowUnstable);
+    const tagConstraint = await getNodeConstraint(config);
     const execOptions: ExecOptions = {
       cwd,
       extraEnv: {
@@ -82,7 +81,7 @@ export async function generateLockFiles(
       },
       docker: {
         image: 'node',
-        tagScheme: 'npm',
+        tagScheme: 'node',
         tagConstraint,
         preCommands,
       },
diff --git a/lib/manager/npm/post-update/node-version.spec.ts b/lib/manager/npm/post-update/node-version.spec.ts
index 617ce7d988..c32f94d271 100644
--- a/lib/manager/npm/post-update/node-version.spec.ts
+++ b/lib/manager/npm/post-update/node-version.spec.ts
@@ -1,5 +1,4 @@
 import { fs } from '../../../../test/util';
-import { isStable } from '../../../versioning/node';
 import { getNodeConstraint } from './node-version';
 
 jest.mock('../../../util/fs');
@@ -16,45 +15,6 @@ describe('manager/npm/post-update/node-version', () => {
     const res = await getNodeConstraint(config);
     expect(res).toEqual('^12.16.0');
   });
-  it('augments to avoid node 15', async () => {
-    fs.readLocalFile = jest.fn();
-    fs.readLocalFile.mockResolvedValueOnce(null);
-    fs.readLocalFile.mockResolvedValueOnce(null);
-    const res = await getNodeConstraint({
-      ...config,
-      constraints: { node: '>= 12.16.0' },
-    });
-    const isAugmentedRange = res === '>= 12.16.0 <15';
-    const node16IsStable = isStable('16.100.0');
-    expect(isAugmentedRange || node16IsStable).toBe(true);
-  });
-  it('forces node 15 if v2 lockfile detected and constraint allows', async () => {
-    fs.readLocalFile.mockResolvedValueOnce(null);
-    fs.readLocalFile.mockResolvedValueOnce(null);
-    fs.readLocalFile.mockResolvedValueOnce('{"lockfileVersion":2}');
-    const res = await getNodeConstraint({
-      ...config,
-      constraints: { node: '>= 12.16.0' },
-    });
-    const isAugmentedRange = res === '>=15 <17';
-    const node16IsStable = isStable('16.100.0');
-    expect(isAugmentedRange || node16IsStable).toBe(true);
-  });
-  it('forces node 15 if v2 lockfile detected and no constraint', async () => {
-    fs.readLocalFile.mockResolvedValueOnce(null);
-    fs.readLocalFile.mockResolvedValueOnce(null);
-    fs.readLocalFile.mockResolvedValueOnce('{"lockfileVersion":2}');
-    const res = await getNodeConstraint(
-      {
-        ...config,
-        constraints: {},
-      },
-      true
-    );
-    const isAugmentedRange = res === '>=15';
-    const node16IsStable = isStable('16.100.0');
-    expect(isAugmentedRange || node16IsStable).toBe(true);
-  });
   it('returns .node-version value', async () => {
     fs.readLocalFile = jest.fn();
     fs.readLocalFile.mockResolvedValueOnce(null);
diff --git a/lib/manager/npm/post-update/node-version.ts b/lib/manager/npm/post-update/node-version.ts
index 30d11b1385..597cb4ce94 100644
--- a/lib/manager/npm/post-update/node-version.ts
+++ b/lib/manager/npm/post-update/node-version.ts
@@ -1,8 +1,7 @@
-import { satisfies, validRange } from 'semver';
+import { validRange } from 'semver';
 import { logger } from '../../../logger';
 import { getSiblingFileName, readLocalFile } from '../../../util/fs';
 import { regEx } from '../../../util/regex';
-import { isStable } from '../../../versioning/node';
 import type { PostUpdateConfig } from '../../types';
 
 async function getNodeFile(filename: string): Promise<string> | null {
@@ -30,44 +29,14 @@ function getPackageJsonConstraint(config: PostUpdateConfig): string | null {
 }
 
 export async function getNodeConstraint(
-  config: PostUpdateConfig,
-  allowUnstable = false
+  config: PostUpdateConfig
 ): Promise<string> | null {
   const { packageFile } = config;
-  let constraint =
+  const constraint =
     (await getNodeFile(getSiblingFileName(packageFile, '.nvmrc'))) ||
     (await getNodeFile(getSiblingFileName(packageFile, '.node-version'))) ||
     getPackageJsonConstraint(config);
-  let lockfileVersion = 1;
-  try {
-    const lockFileName = getSiblingFileName(packageFile, 'package-lock.json');
-    lockfileVersion = JSON.parse(
-      await readLocalFile(lockFileName, 'utf8')
-    ).lockfileVersion;
-  } catch (err) {
-    // do nothing
-  }
-  // Avoid using node 15 if node 14 also satisfies the same constraint
-  // Remove this once node 16 is LTS
-  if (constraint) {
-    if (
-      validRange(constraint) &&
-      satisfies('14.100.0', constraint) &&
-      satisfies('15.100.0', constraint) &&
-      !isStable('16.100.0')
-    ) {
-      if (lockfileVersion === 2) {
-        logger.debug('Forcing node 15+ to ensure lockfileVersion=2 is used');
-        constraint = '>=15 <17';
-      } else if (validRange(`${constraint} <15`)) {
-        logger.debug('Augmenting constraint to avoid node 15');
-        constraint = `${constraint} <15`;
-      }
-    }
-  } else if (allowUnstable && lockfileVersion === 2) {
-    logger.debug('Using node >=15 for lockfileVersion=2');
-    constraint = '>=15';
-  } else {
+  if (!constraint) {
     logger.debug('No node constraint found - using latest');
   }
   return constraint;
diff --git a/lib/manager/npm/post-update/npm.ts b/lib/manager/npm/post-update/npm.ts
index b94e424015..c31770c815 100644
--- a/lib/manager/npm/post-update/npm.ts
+++ b/lib/manager/npm/post-update/npm.ts
@@ -65,7 +65,7 @@ export async function generateLockFile(
       },
       docker: {
         image: 'node',
-        tagScheme: 'npm',
+        tagScheme: 'node',
         tagConstraint,
         preCommands,
       },
diff --git a/lib/manager/npm/post-update/pnpm.ts b/lib/manager/npm/post-update/pnpm.ts
index 2317d55c6a..d2fe217858 100644
--- a/lib/manager/npm/post-update/pnpm.ts
+++ b/lib/manager/npm/post-update/pnpm.ts
@@ -38,7 +38,7 @@ export async function generateLockFile(
       },
       docker: {
         image: 'node',
-        tagScheme: 'npm',
+        tagScheme: 'node',
         tagConstraint,
         preCommands,
       },
diff --git a/lib/manager/npm/post-update/yarn.ts b/lib/manager/npm/post-update/yarn.ts
index 1ca1c95233..d6a517b1a3 100644
--- a/lib/manager/npm/post-update/yarn.ts
+++ b/lib/manager/npm/post-update/yarn.ts
@@ -129,7 +129,7 @@ export async function generateLockFile(
       extraEnv,
       docker: {
         image: 'node',
-        tagScheme: 'npm',
+        tagScheme: 'node',
         tagConstraint,
         preCommands,
       },
diff --git a/lib/util/exec/docker/index.spec.ts b/lib/util/exec/docker/index.spec.ts
index 5c8f35b1f0..4c8d4dea7d 100644
--- a/lib/util/exec/docker/index.spec.ts
+++ b/lib/util/exec/docker/index.spec.ts
@@ -95,6 +95,16 @@ describe('util/exec/docker/index', () => {
       getPkgReleases.mockResolvedValueOnce({ releases } as never);
       expect(await getDockerTag('foo', '^1.2.3', 'npm')).toBe('1.9.9');
     });
+    it('filters out node unstable', async () => {
+      const releases = [
+        { version: '12.0.0' },
+        { version: '13.0.1' },
+        { version: '14.0.2' },
+        { version: '15.0.2' },
+      ];
+      getPkgReleases.mockResolvedValueOnce({ releases } as never);
+      expect(await getDockerTag('foo', '>=12', 'node')).toBe('14.0.2');
+    });
   });
 
   describe('removeDockerContainer', () => {
diff --git a/lib/util/exec/docker/index.ts b/lib/util/exec/docker/index.ts
index 0cecae5905..a6a160653d 100644
--- a/lib/util/exec/docker/index.ts
+++ b/lib/util/exec/docker/index.ts
@@ -93,6 +93,11 @@ export async function getDockerTag(
     versions = versions.filter(
       (version) => ver.isVersion(version) && ver.matches(version, constraint)
     );
+    // Prefer stable versions over unstable, even if the range satisfies both types
+    if (!versions.every((version) => ver.isStable(version))) {
+      logger.debug('Filtering out unstable versions');
+      versions = versions.filter((version) => ver.isStable(version));
+    }
     versions = versions.sort(ver.sortVersions.bind(ver));
     if (versions.length) {
       const version = versions.pop();
-- 
GitLab