Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: move node 17 Check from Binary to CLI #19977

Merged
merged 11 commits into from Jan 31, 2022
4 changes: 2 additions & 2 deletions circle.yml
Expand Up @@ -29,7 +29,7 @@ mainBuildFilters: &mainBuildFilters
only:
- develop
- 10.0-release
- test-binary-downstream-windows
- node-17-maybe

# usually we don't build Mac app - it takes a long time
# but sometimes we want to really confirm we are doing the right thing
Expand Down Expand Up @@ -1587,7 +1587,7 @@ jobs:
- run:
name: Check current branch to persist artifacts
command: |
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "test-binary-downstream-windows" ]]; then
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "node-17-maybe" ]]; then
echo "Not uploading artifacts or posting install comment for this branch."
circleci-agent step halt
fi
Expand Down
31 changes: 24 additions & 7 deletions cli/lib/util.js
Expand Up @@ -18,10 +18,12 @@ const executable = require('executable')
const { stripIndent } = require('common-tags')
const supportsColor = require('supports-color')
const isInstalledGlobally = require('is-installed-globally')
const pkg = require(path.join(__dirname, '..', 'package.json'))
const logger = require('./logger')
const debug = require('debug')('cypress:cli')
const fs = require('./fs')
const semver = require('semver')

const pkg = require(path.join(__dirname, '..', 'package.json'))

const issuesUrl = 'https://github.com/cypress-io/cypress/issues'

Expand Down Expand Up @@ -282,18 +284,33 @@ const util = {
.mapValues((value) => { // stringify to 1 or 0
return value ? '1' : '0'
})
.extend(util.getOriginalNodeOptions(options))
.extend(util.getOriginalNodeOptions())
.value()
},

getOriginalNodeOptions (options) {
getOriginalNodeOptions () {
const opts = {}

if (process.env.NODE_OPTIONS) {
return {
ORIGINAL_NODE_OPTIONS: process.env.NODE_OPTIONS,
}
opts.ORIGINAL_NODE_OPTIONS = process.env.NODE_OPTIONS
}

// https://github.com/cypress-io/cypress/issues/18914
// Node 17+ ships with OpenSSL 3 by default, so we may need the option
// --openssl-legacy-provider so that webpack@4 can use the legacy MD4 hash
// function. This option doesn't exist on Node <17 or when it is built
// against OpenSSL 1, so we have to detect Node's major version and check
// which version of OpenSSL it was built against before spawning the plugins
// process.

// To be removed when the Cypress binary pulls in the @cypress/webpack-batteries-included-preprocessor
// version that has been updated to webpack >= 5.61, which no longer relies on
// Node's builtin crypto.hash function.
if (process.versions && semver.satisfies(process.versions.node, '>=17.0.0') && process.versions.openssl.startsWith('3.')) {
opts.ORIGINAL_NODE_OPTIONS = `${opts.ORIGINAL_NODE_OPTIONS || ''} --openssl-legacy-provider`
}

return {}
return opts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this return value get used for? Previously it always returned an empty object, but now we actually return stuff.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previously in the if process.env.NODE_OPTIONS check it'd return fast with

{ ORIGINAL_NODE_OPTIONS: ... }

This removed the fast-return to got through both checks before returning. If neither check are hit, it'll continue to return an empty object.

},

getForceTty () {
Expand Down
1 change: 1 addition & 0 deletions cli/package.json
Expand Up @@ -58,6 +58,7 @@
"pretty-bytes": "^5.6.0",
"proxy-from-env": "1.0.0",
"request-progress": "^3.0.0",
"semver": "^7.3.2",
"supports-color": "^8.1.1",
"tmp": "~0.2.1",
"untildify": "^4.0.0",
Expand Down
66 changes: 66 additions & 0 deletions cli/test/lib/util_spec.js
Expand Up @@ -257,6 +257,7 @@ describe('util', () => {

context('.getOriginalNodeOptions', () => {
let restoreEnv
const sandbox = sinon.createSandbox()

afterEach(() => {
if (restoreEnv) {
Expand All @@ -266,6 +267,9 @@ describe('util', () => {
})

it('copy NODE_OPTIONS to ORIGINAL_NODE_OPTIONS', () => {
sandbox.stub(process.versions, 'node').value('v16.5.0')
sandbox.stub(process.versions, 'openssl').value('1.0.0')

restoreEnv = mockedEnv({
NODE_OPTIONS: '--require foo.js',
})
Expand All @@ -274,6 +278,68 @@ describe('util', () => {
ORIGINAL_NODE_OPTIONS: '--require foo.js',
})
})

// https://github.com/cypress-io/cypress/issues/18914
it('includes --openssl-legacy-provider in Node 17+ w/ OpenSSL 3', () => {
sandbox.stub(process.versions, 'node').value('v17.1.0')
sandbox.stub(process.versions, 'openssl').value('3.0.0-quic')

restoreEnv = mockedEnv({
NODE_OPTIONS: '--require foo.js',
})

let childOptions = util.getOriginalNodeOptions()

expect(childOptions.ORIGINAL_NODE_OPTIONS).to.eq('--require foo.js --openssl-legacy-provider')

restoreEnv()
restoreEnv = mockedEnv({})
childOptions = util.getOriginalNodeOptions()

expect(childOptions.ORIGINAL_NODE_OPTIONS).to.eq(' --openssl-legacy-provider')
})

// https://github.com/cypress-io/cypress/issues/19320
it('does not include --openssl-legacy-provider in Node 17+ w/ OpenSSL 1', () => {
sandbox.stub(process.versions, 'node').value('v17.1.0')
sandbox.stub(process.versions, 'openssl').value('1.0.0')

restoreEnv = mockedEnv({
NODE_OPTIONS: '--require foo.js',
})

let childOptions = util.getOriginalNodeOptions()

expect(childOptions.ORIGINAL_NODE_OPTIONS).to.eq('--require foo.js')
expect(childOptions.ORIGINAL_NODE_OPTIONS).not.to.contain('--openssl-legacy-provider')

restoreEnv()
restoreEnv = mockedEnv({})
childOptions = util.getOriginalNodeOptions()

expect(childOptions.ORIGINAL_NODE_OPTIONS).to.be.undefined
})

// https://github.com/cypress-io/cypress/issues/18914
it('does not include --openssl-legacy-provider in Node <=16', () => {
sandbox.stub(process.versions, 'node').value('v16.5.0')
sandbox.stub(process.versions, 'openssl').value('1.0.0')

restoreEnv = mockedEnv({})

let childOptions = util.getOriginalNodeOptions()

expect(childOptions.ORIGINAL_NODE_OPTIONS).to.be.undefined

restoreEnv = mockedEnv({
NODE_OPTIONS: '--require foo.js',
})

childOptions = util.getOriginalNodeOptions()

expect(childOptions.ORIGINAL_NODE_OPTIONS).to.eq('--require foo.js')
expect(childOptions.ORIGINAL_NODE_OPTIONS).not.to.contain('--openssl-legacy-provider')
})
})

context('.exit', () => {
Expand Down
16 changes: 0 additions & 16 deletions packages/server/lib/plugins/index.js
Expand Up @@ -8,7 +8,6 @@ const inspector = require('inspector')
const errors = require('../errors')
const util = require('./util')
const pkg = require('@packages/root')
const semver = require('semver')

let pluginsProcess = null
let registeredEvents = {}
Expand Down Expand Up @@ -52,21 +51,6 @@ const getChildOptions = (config) => {
childOptions.execPath = config.resolvedNodePath
}

// https://github.com/cypress-io/cypress/issues/18914
// Node 17+ ships with OpenSSL 3 by default, so we may need the option
// --openssl-legacy-provider so that webpack@4 can use the legacy MD4 hash
// function. This option doesn't exist on Node <17 or when it is built
// against OpenSSL 1, so we have to detect Node's major version and check
// which version of OpenSSL it was built against before spawning the plugins
// process.

// To be removed on update to webpack >= 5.61, which no longer relies on
// Node's builtin crypto.hash function.
if (semver.satisfies(config.resolvedNodeVersion, '>=17.0.0') &&
!process.versions.openssl.startsWith('1.')) {
childOptions.env.NODE_OPTIONS += ' --openssl-legacy-provider'
}

if (inspector.url()) {
childOptions.execArgv = _.chain(process.execArgv.slice(0))
.remove('--inspect-brk')
Expand Down
39 changes: 0 additions & 39 deletions packages/server/test/unit/plugins/index_spec.js
Expand Up @@ -64,45 +64,6 @@ describe('lib/plugins/index', () => {

expect(childOptions.execPath).to.eq(undefined)
})

// https://github.com/cypress-io/cypress/issues/18914
it('includes --openssl-legacy-provider in Node 17+ w/ OpenSSL 3', () => {
const sandbox = sinon.createSandbox()

sandbox.stub(process.versions, 'openssl').value('3.0.0-quic')

const childOptions = plugins.getChildOptions({
resolvedNodeVersion: 'v17.1.0',
})

expect(childOptions.env.NODE_OPTIONS).to.contain('--openssl-legacy-provider')

sandbox.restore()
})

// https://github.com/cypress-io/cypress/issues/19320
it('does not include --openssl-legacy-provider in Node 17+ w/ OpenSSL 1', () => {
const sandbox = sinon.createSandbox()

sandbox.stub(process.versions, 'openssl').value('1.1.1m')

const childOptions = plugins.getChildOptions({
resolvedNodeVersion: 'v17.3.0',
})

expect(childOptions.env.NODE_OPTIONS).not.to.contain('--openssl-legacy-provider')

sandbox.restore()
})

// https://github.com/cypress-io/cypress/issues/18914
it('does not include --openssl-legacy-provider in Node <=16', () => {
const childOptions = plugins.getChildOptions({
resolvedNodeVersion: 'v16.31.0',
})

expect(childOptions.env.NODE_OPTIONS).not.to.contain('--openssl-legacy-provider')
})
})

context('#init', () => {
Expand Down
10 changes: 1 addition & 9 deletions scripts/binary/bump.js
Expand Up @@ -268,15 +268,7 @@ Testing new Cypress version ${version}
}

// first try to commit to branch for next upcoming version
const specificBranchOptions = {
owner,
repo,
token: creds.githubToken,
message,
branch: version,
}

return makeEmptyGithubCommit(specificBranchOptions)
return makeEmptyGithubCommit({ ...defaultOptions, branch: version })
.catch(() => {
// maybe there is no branch for next version
// try default branch
Expand Down