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

[backport] PR #7920,#8313 to 4.6 - Upgrade to Node 6.4.0 #8213

Merged
merged 2 commits into from
Oct 11, 2016

Conversation

tylersmalley
Copy link
Contributor

@tylersmalley tylersmalley commented Sep 10, 2016

Backport PR #7920

Upgrade to Node 6.4.0

  • Updated some dependencies to include graceful-fs ~4.0
  • Replaced deprecated grunt-s3 package with suggested grunt-aws-s3
  • Update licenses task to better support npm 3

Backport PR #8313

Commit 1:
Upgrades Hapi to 14.2.0

As of Node 6, crypto.pbkdf emits a deprecation warning when the digest isn't explicily set. Under certain conditions we are seeing this warning from Hapi's dependency Iron. Iron resolved this issue as of 4.0.4, which was introduced into Hapi as of 14.0.0.

Node deprecation: nodejs/node@8e8959d
Iron's resolution: hapijs/iron@9e0a1ef

As of Hapi v9, they have removed three build-in plugins from the core which we rely on inert (files and directories), vision (view templates), and h2o2 (proxy). hapijs/hapi#2682

Signed-off-by: Tyler Smalley tyler.smalley@elastic.co

@epixa epixa self-assigned this Sep 12, 2016
@tylersmalley
Copy link
Contributor Author

Discovered a deprecation warning when used with Shield: (node:23629) DeprecationWarning: crypto.pbkdf2 without specifying a digest is deprecated. Please specify a digest. Investigating.

@epixa
Copy link
Contributor

epixa commented Sep 12, 2016

I just saw this a few minutes ago. It's happening in master as well.

@tylersmalley
Copy link
Contributor Author

#8313 resolves the deprecation warning. Once that is merged, I will backport to 4.6 then continue with this PR.

* Updated dependencies to include graceful-fs ~4.0
* Replaced deprecated grunt-s3 package with suggested grunt-aws-s3
* Update licenses task to better support npm 3

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
As of Node 6, crypto.pbkdf emits a deprecation warning when the digest isn't explicily set. Under certain conditions we are seeing this warning from Hapi's dependency Iron. Iron resolved this issue as of 4.0.4, which was introduced into Hapi as of 14.0.0.

Node deprecation: nodejs/node@8e8959d
Iron's resolution: hapijs/iron@9e0a1ef

As of Hapi v9, they have removed three build-in plugins from the core which we rely on inert (files and directories), vision (view templates), and h2o2 (proxy). hapijs/hapi#2682

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley tylersmalley changed the title [backport] PR #7920 to 4.6 - Upgrade to Node 6.4.0 [backport] PR #7920,#8313 to 4.6 - Upgrade to Node 6.4.0 Oct 3, 2016
@tylersmalley
Copy link
Contributor Author

This PR now includes the backport of #8313 as both needed manual intervention and should happen at the same time anyways.

@epixa
Copy link
Contributor

epixa commented Oct 3, 2016

We should definitely do thorough review/testing on this before we click merge. I wouldn't want any important bug fixes not to be able to go out while we verify that this is good to go.

@thomasneirynck thomasneirynck self-assigned this Oct 4, 2016
@thomasneirynck
Copy link
Contributor

LGTM

I tested this on Kibana and with the Security plugin, no issues as far as I can tell.

Copy link
Contributor

@stacey-gammon stacey-gammon left a comment

Choose a reason for hiding this comment

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

After some guidance for setting this up properly, everything appears to be working as expected. I do see many "Error: read ECONNRESET
at exports._errnoException (util.js:1026:11)
at TLSWrap.onread (net.js:563:26)
"
errors, but as discussed offline, that will be filed as a separate issue. Doesn't seem to affect usage.

@tylersmalley
Copy link
Contributor Author

In my long-running testing I ran into an error which ultimately stalled the server:

server    log   [18:20:49.720] [error][client][connection] Error: Parse Error
    at Error (native)

Holding off on merging for now.

This was referenced Oct 10, 2016
@tylersmalley
Copy link
Contributor Author

The error ended up being caused by performing a request to http when https was enabled, something which is also present in 4.6 and not introduced in this PR. The server was not stalled, but rather the request did not immediately fail. Discussed with @spalger to confirm it's not a show-stopper for this PR.

@tylersmalley
Copy link
Contributor Author

Spoke with @epixa and we are merging this to begin testing on the snapshot. There is still discussion on if this warrants a 4.7 release or not.

@tylersmalley tylersmalley merged commit 332abe8 into elastic:4.6 Oct 11, 2016
@tylersmalley tylersmalley deleted the 4.6-node6 branch October 11, 2016 19:01
@epixa epixa added v4.6.3 and removed v4.6.2 labels Oct 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants