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

chore(local-stack): improve make targets and avoid node17 issues for docker #2955

Merged
merged 1 commit into from
Nov 28, 2021

Conversation

jbarbin
Copy link
Contributor

@jbarbin jbarbin commented Nov 21, 2021

These changes skip database creation if exist for make targets and avoid node 17 openssl3 issues with webpack with docker.

Changelog

  • Fix: Fix node image version for docker to avoid node17 openssl3 issues with webpack (nodejs 17: digital envelope routines::unsupported)
  • Chore: Skip database creation if already exist into make targets
  • Chore: Start automatically webpack encore dev for watching files into node docker container

@jbarbin jbarbin changed the title chore(local-stack): improve make targets and avoir node17 issues for docker chore(local-stack): improve make targets and avoid node17 issues for docker Nov 21, 2021
Copy link
Member

@I-Valchev I-Valchev left a comment

Choose a reason for hiding this comment

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

👋 Hey @jbarbin , thanks for the PR! Looking good so far, though I have to admit I'm not too aware of the docker/node issues. Can you explain it to me? Also, what'd be the consequence of locking the docker install to that specific version of node? Many thanks!

@jbarbin
Copy link
Contributor Author

jbarbin commented Nov 22, 2021

👋 Hey @jbarbin , thanks for the PR! Looking good so far, though I have to admit I'm not too aware of the docker/node issues. Can you explain it to me? Also, what'd be the consequence of locking the docker install to that specific version of node? Many thanks!

Hello @I-Valchev,
In fact, the hashing algorithm is not compatible with the node version 17.
The error message on asset building with webpack :

[webpack-cli] Error: error:0308010C:digital envelope routines::unsupported
    at new Hash (node:internal/crypto/hash:67:19)
    at Object.createHash (node:crypto:130:10)
    at BulkUpdateDecorator.hashFactory (/opt/src/node_modules/webpack/lib/util/createHash.js:155:18)
    at BulkUpdateDecorator.digest (/opt/src/node_modules/webpack/lib/util/createHash.js:80:21)
    at /opt/src/node_modules/webpack/lib/DefinePlugin.js:595:38
    at Hook.eval [as call] (eval at create (/opt/src/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:100:1)
    at Hook.CALL_DELEGATE [as _call] (/opt/src/node_modules/tapable/lib/Hook.js:14:14)
    at Compiler.newCompilation (/opt/src/node_modules/webpack/lib/Compiler.js:1053:26)
    at /opt/src/node_modules/webpack/lib/Compiler.js:1097:29
    at Hook.eval [as callAsync] (eval at create (/opt/src/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1) {
  opensslErrorStack: [ 'error:03000086:digital envelope routines::initialization error' ],
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'
}

That will be solve with webpack 6 🤞. While waiting for this version we have 3 options to solve that (to my knowledge) :

  • Lock node container version until webpack 6 is available (What I do, but I can add a TODO)
  • Add the node option "--openssl-legacy-provider" until webpack 6 is available (not tested)
  • Change the hash function in webpack configuration until webpack 6 is available (not tested)

Would you prefere i check other options like change the hash function ?
These solutions remain temporary until version 6 of webpack.

Copy link
Member

@I-Valchev I-Valchev left a comment

Choose a reason for hiding this comment

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

Hey @jbarbin , thanks a lot for helping me understand this 🙌

I think your current solution works fine 👍 Let's roll with it

@jbarbin
Copy link
Contributor Author

jbarbin commented Nov 24, 2021

@I-Valchev , no problem ;)
My cypress tests fail but i don't know why 🤔
I see this PR to fix cypress tests, i hope it fix my problem.

@I-Valchev
Copy link
Member

Hi @jbarbin, yes you're right! Cypress is fixed by @Joossensei + I don't think the changes here will have affected the tests. I'll get somebody to provide a second review and merge this in swiftly :-) Thanks a lot!

Copy link
Member

@bobdenotter bobdenotter left a comment

Choose a reason for hiding this comment

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

Thanks, @jbarbin 👍

@bobdenotter bobdenotter merged commit 9ca8374 into bolt:master Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants