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

CI job for OpenSSL 3.0 #2584

Closed
danbev opened this issue Mar 22, 2021 · 10 comments
Closed

CI job for OpenSSL 3.0 #2584

danbev opened this issue Mar 22, 2021 · 10 comments
Assignees

Comments

@danbev
Copy link
Contributor

danbev commented Mar 22, 2021

I would like to request a CI job to be created for dynamically linking to OpenSSL 3.0, similar to what is done today for OpenSSL 1.1.1

OpenSSL 3.0 has not been released yet but there are alpha releases and it would be great if we could have a CI job for this that is run on each PR to be able to detect breaking crypto changes early.

I'm not sure if this information is useful but I'm adding it just in case. The following are the configure/build commands that I've been using to build OpenSSL 3.x and Node.js:

$ ./config -Werror --strict-warnings --debug --prefix=/path/openssl_build_master linux-x86_64
$ make -j8 install_sw
$ ./configure --shared-openssl --shared-openssl-libpath=/path/openssl_build_master/lib --shared-openssl-includes=/path/openssl_build_master/include --shared-openssl-libname=crypto,ssl
$ make -j8 test
@danbev danbev changed the title Build for OpenSSL 3.0 CI job for OpenSSL 3.0 Mar 22, 2021
@rvagg
Copy link
Member

rvagg commented Mar 22, 2021

Does it pass the test suite already? Introducing a partially broken job would be pretty annoying.

@danbev
Copy link
Contributor Author

danbev commented Mar 22, 2021

@rvagg It passed when nodejs/node#37669 was landed. I'll update and run though the tests locally today and double check that this is still true.

@rvagg
Copy link
Member

rvagg commented Mar 22, 2021

And would this only be for >=16.x? (i.e. master now and forward, but no earlier).

@danbev
Copy link
Contributor Author

danbev commented Mar 24, 2021

nodejs/node#37860 has been merged so there should not be any test failure specifically related to OpenSSL 3.0.
There is a flaky test which I've seen quite frequently when linking with OpenSSL 3.0 though be it also happens when statically linking with OpenSSL 1.1.1. We might be able to mark it as flaky if it becomes an issue.

And yes please, this would only be for >=16. Thanks!

@richardlau
Copy link
Member

Opened a PR for Dockerfile and VersionSelectorScript changes: #2601
Actually changing the job has run into a snag being tracked in #2602.

richardlau added a commit that referenced this issue Apr 1, 2021
Add OpenSSL 3.0.0 alpha13 to the sharedlibs container. Add an entry to
the VersionSelectorScript to build against it with Node.js 15 onwards.

Update OpenSSL 1.1.1g to 1.1.1k.

Refs: #2584
@richardlau
Copy link
Member

I managed to get an editable copy of node-test-commit-linux-containered (#2602 (comment)) and was able to test the changes from #2601.

I'll work on updating node-test-commit-linux-containered based on the copied job. We'll need to land nodejs/node#38027 to get a clean test run before we enable testing against OpenSSL 3.

@richardlau richardlau self-assigned this Apr 1, 2021
@richardlau
Copy link
Member

I've edited node-test-commit-linux-containered to add the logic for build/testing against the OpenSSL 3 in the sharedlibs container, but have not yet added the ubuntu1804_sharedlibs_openssl300_x64 label to the job nodes as otherwise the job would start failing until nodejs/node#38027 is merged.

@danbev
Copy link
Contributor Author

danbev commented Apr 2, 2021

@richardlau Thanks for doing this!

@richardlau
Copy link
Member

Added the ubuntu1804_sharedlibs_openssl300_x64 label to node-test-commit-linux-containered: https://github.com/nodejs/jenkins-config-test/commit/ce4a541517aa0f85467d0cfbc40c863a811dfe7b

Keeping an eye on the job runs.

@richardlau
Copy link
Member

Job appears to be working. There's a new test failure introduced by a recently changed test -- have opened nodejs/node#38136 to address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants