OpenSSL update instructions - feedback wanted
mhdawson opened this issue · 8 comments
To help me get up to speed on the instructions to update OpenSSL in
maintaining-openssl.md
I ran through the instructions and built some docker files that would automate almost the full process.
These are in https://github.com/mhdawson/node-openssl-utils (one branch for main and one for 16.x so far)
What I like about them is that I can edit the version of openssl to pull in, start the process with bash buildit.sh and come back an hour later(on a big machine, likely longer on something more typical) to a docker container with the changes applied, including our standardize commit comments.
It does not push a PR, as I'm not sure it would be successful enough for that to make sense but it keeps the results of running the tests and you can confirm that all modified files were included in the commits. If there are any additional changes required (like the changes o the OpenSSL tests in the last security release) you can cherry pick those over and re-run the tests easily/quickly It is also handy for doing a quick sniff check of a PR by confirming that the same number of files have been modified as expected.
I wondering if we'd want to do one of the following
- Integrate using docker to do the full build into the main instructions (it is already partially used for the non-linux instructions).
- transfer the node-openssl-utils into the node.js repo as a set of examples/utilities that can be referenced in the instructions to help people if they want to use docker.
Automating in an action would not quite be the same in my mind as you don't get the container which you can use to carry out the last few steps or cherry pick commits if necessary. If we think the complete process might work frequently enough then an action running similar steps could make sense.
They are not quite complete as I can see that I missed some of the externalization of the git email and user name but I'd do that if 1) or 2) above make sense.
@richardlau, @hassaanp since I know you have done some of the recently OpenSSL updates.
I'm broadly in favour of adding more dockerisation of the process as an option -- I actually run the existing container (the non-Linux instructions) on Linux when I've done OpenSSL updates.
From my POV the main complexity of the OpenSSL update process is that it is slightly different for the older release lines using upstream OpenSSL vs the more recent release lines that use the quic fork.
@richardlau in terms of
From my POV the main complexity of the OpenSSL update process is that it is slightly different for the older release lines using upstream OpenSSL vs the more recent release lines that use the quic fork.
A branch for those older versions which has the changes needed for the older versions is what I had in mind.
@mhdawson I'm wondering if there is a chance to run those commands in a self-hosted CI, which may create the PR automatically.
I don't know how much time would it take in a normal CI (I assume a lot), but, we can use dedicated machines for those work. Is it feasible?
@RafaelGSS I had wondered about setting up CI to run/generate the PR. What I'm not sure of is how many times it will just work or need some sort of tweaks. If it will just work 80% of the time then the CI/generate PR makes a lot of sense to be and that would be a good thing for us to add to be able to respond to OpenSSL updates quickly.
This was my comment above:
It does not push a PR, as I'm not sure it would be successful enough for that to make sense but it keeps the results of running the tests and you can confirm that all modified files were included in the commits. If there are any additional changes required (like the changes o the OpenSSL tests in the last security release) you can cherry pick those over and re-run the tests easily/quickly.
@richardlau what's your take on how often it should just work based on your past experience?
@mhdawson In the past it has generally just worked but the last two updates did not and caused test failures.
- 3.0.2: d37dceb.
- 3.0.1/1.1.1m: #41177 (comment).
If most of the past record is that it just works then trying to automate it makes sense to me.
Sorry for the late response here.
My 2 cents: I have contributed to the ssl updates a few times across 2 years and I would say that apart from a single time (there was a change in the file structure), the steps were pretty much consistent and therefore can be automated.
Having a CI that creates a PR with the openssl update sounds neat. But I think we should also talk about the workflow in case the CI fails. I think in the event of failure, the pipeline should open a new issue making sure the relevant stakeholders are tagged to ensure necessary steps can be taken.
FYI I'll be working on that action in the next coming days