tools: add non-default OpenSSL versions to the test-shared workflow#62862
tools: add non-default OpenSSL versions to the test-shared workflow#62862panva wants to merge 8 commits intonodejs:mainfrom
Conversation
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
|
Review requested:
|
nodejs-github-bot
added
meta
panva
added
commit-queue-squash
|
Adding extra GHA workflows comes at the expense of spending more minutes to prepare security releases. We can skip the test-shared workflow on the private repo, but idk if it's worth it given that there are some path that we only run on that workflow. |
| # 3. Fetch OpenSSL release versions from endoflife.date, keep entries that | ||
| # are either not past EOL or still under extended support, then pick the | ||
| # first nix attr whose `.version` starts with the release version | ||
| # followed by `.` / letter / end-of-string (so "3.6" matches "3.6.1", | ||
| # "1.1.1" matches "1.1.1w", and "1.1" does NOT swallow "1.1.1"). | ||
| # Releases without a matching nix attr and the one covered by default in | ||
| # `build` are dropped. | ||
| curl -sf https://endoflife.date/api/openssl.json \ |
There was a problem hiding this comment.
Wouldn't it be better to have a manually curated list (by manually, I mean: the bot would open PRs to update the list every Sunday)? The logic is quite hard to grasp, and it's a lot of repetitive work that does not scale well if we wanted to add e.g. non-OpenSSL variants.
There was a problem hiding this comment.
uff, there's an idea, nixpgs actually has BoringSSL too. I'd do this in a follow-up.
There was a problem hiding this comment.
And it'd have to be the same job that does nixpkgs-unstable updates, else we'd run the risk of the two being open at the same time and not being in sync between their merges.
| --arg ccache "${NIX_SCCACHE:-null}" \ | ||
| --arg devTools '[]' \ | ||
| --arg benchmarkTools '[]' \ | ||
| ${{ endsWith(inputs.system, '-darwin') && '--arg withAmaro false --arg withLief false --arg withSQLite false --arg withFFI false --arg extraConfigFlags ''["--without-inspector" "--without-node-options"]'' \' || '\' }} |
There was a problem hiding this comment.
I think this would need to be moved to inputs.extra-nix-args
There was a problem hiding this comment.
I figured only extras (not shared between all uses of the composite action) would go through those inputs.
The OpenSSL versions are a big gap. Since we don't/can't keep up with the versions in CI. |
| build-openssl: | ||
| needs: | ||
| - build-tarball | ||
| - collect-openssl-versions |
There was a problem hiding this comment.
We might want to wait for the build job to finish, otherwise we'd have to run 5 times the same V8 version
There was a problem hiding this comment.
though it increases the total time waiting for the workflow since we even have to wait for the slow build ones. wdyt?
3 participants
Adds an additional OpenSSL shared-libraries matrix to
test-shared.ymlso PRs run against the supported OpenSSL releases we don't already build against by default. No more waiting for a full CI to find out a crypto/TLS change is broken on another version 🙏.collect-openssl-versionsdynamically builds the matrix from the pinned nixpkgs and https://endoflife.date, dropping EOL versions and the version the existingbuildjob already covers while keeping ones that have extended support - 1.1.1 and soon 3.0.SUPPORTED_OPENSSL_VERSIONenv run withcontinue-on-error: trueso that OpenSSL versions we haven't adjusted for yet don't fail GHA.First commit for simple review, second is refactoring to a composite action, rest is fixes/applying feedback/minor refactors.