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.

Sign up for GitHub

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

Jump to bottom

[v16.x] tools: automate update openssl v16 #48500

Closed
wants to merge 2 commits into from
Closed

[v16.x] tools: automate update openssl v16 #48500

wants to merge 2 commits into from

Conversation

marco-ippolito
Copy link
Member

@marco-ippolito marco-ippolito commented Jun 19, 2023 •
edited
Loading

PR-URL: #48377
Reviewed-By: Rafael Gonzaga [email protected]
Reviewed-By: Antoine du Hamel [email protected]
Reviewed-By: Paolo Insogna [email protected]

@targos
test: allow SIGBUS in signal-handler abort test
9bbd991 
FreeBSD uses SIGBUS after update to v12.4.

Refs: nodejs/build#3134
PR-URL: nodejs#47851
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added tools Issues and PRs related to the tools directory. v16.x labels Jun 19, 2023
@marco-ippolito marco-ippolito added the wip Issues and PRs that are still a work in progress. label Jun 19, 2023
@marco-ippolito marco-ippolito force-pushed the backport/openssl-update-backport branch from 84fd21b to aa60c8c Compare June 19, 2023 14:12
@marco-ippolito
tools: automate update openssl v16
dd2a467 
PR-URL: nodejs#48377
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
@marco-ippolito marco-ippolito force-pushed the backport/openssl-update-backport branch from aa60c8c to dd2a467 Compare June 19, 2023 14:13
@marco-ippolito marco-ippolito removed the wip Issues and PRs that are still a work in progress. label Jun 19, 2023
RafaelGSS
RafaelGSS reviewed Jun 20, 2023
View reviewed changes
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

I think we need another action for 1.1.1. IIRC scheduled workflows only run in the main branch. So I believe we need to have this automate update v16 on main.

cc: @nodejs/actions

@marco-ippolito
Copy link
Member Author

I think we need another action for 1.1.1. IIRC scheduled workflows only run in the main branch. So I believe we need to have this automate update v16 on main.

cc: @nodejs/actions

The action update-openssl.yml checks out on v16.x-staging and executes the update-openssl.sh and opens a pr. I think it will work

@RafaelGSS
Copy link
Member

RafaelGSS commented Jun 21, 2023 •
edited
Loading

I think we need another action for 1.1.1. IIRC scheduled workflows only run in the main branch. So I believe we need to have this automate update v16 on main.
cc: @nodejs/actions

The action update-openssl.yml checks out on v16.x-staging and executes the update-openssl.sh and opens a pr. I think it will work

but it won't run in the v16.x-staging branch. It will run only on main. @richardlau I remember you've mentioned it somewhere, right?

@richardlau
Copy link
Member

GitHub Actions cron scheduler only runs workflow files from the default branch (e.g. main for this repository), which means it would be running https://github.com/nodejs/node/blob/main/.github/workflows/update-openssl.yml. However #48377 added a subjob that checks out the v16.x-staging branch

node/.github/workflows/update-openssl.yml

Lines 61 to 68 in 198affc

openssl-v1-update:
if: github.repository == 'nodejs/node'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.2
with:
persist-credentials: false
ref: v16.x-staging
so I think #48500 (review) should already be taken care of once this PR lands to update the scripts that the workflow in main references for openssl-v1-update.

RafaelGSS
RafaelGSS approved these changes Jun 21, 2023
View reviewed changes
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM. Sorry, I missed the openssl-v1-update.

@marco-ippolito marco-ippolito added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 26, 2023
@richardlau
Copy link
Member

Given that Node.js 16 reaches End-of-Life in a few weeks (11 September 2023), I'm going to close this as won't land on v16.x.

@richardlau richardlau closed this Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafaelGSS RafaelGSS RafaelGSS approved these changes

Assignees
No one assigned
Labels
commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@marco-ippolito @nodejs-github-bot @RafaelGSS @richardlau @targos