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

tools: added support for notarytool for osx notarization #48701

Closed
wants to merge 15 commits into from

Conversation

UlisesGascon
Copy link
Member

@UlisesGascon UlisesGascon commented Jul 8, 2023

Main Changes

  • Encapsulated gon to be used only when notarytool is not available (Xcode version < 13.0)
  • Added notarytool pkg submission by using user, pass, teamId combination.

How to test this code?

There is a cloned job in the ci-release named testing-new-osx-notarization-ojs+release where you can build with parameters as always. Please always use disttype: test.

As an example:

The Build 14 is passing. see. The pkg files are working fine

Screenshot 2023-09-15 at 20 37 15

The output from the Jenkins process is the expected:

Screenshot 2023-09-15 at 20 36 26

Notes

In order to properly work is required to include a new environmental variable NOTARIZATION_TEAM_ID in the Jenkins job, this data is not sensitive information.

I am a bit rusty in bash, so feel free to improve the script. Only the linter steps are relevant in the CI validation for this PR.

This is working with the existing credentials (NOTARIZATION_PASSWORD and NOTARIZATION_ID) so there is no breaking changes in the credentials for gon

Context

TL;DR:

By November 1, 2023, we should be able to use notarytool to notarize Node.js. Currently, we use gon, but there is no support for notarytool yet. Additionally, we want to replace gon as notarytool currently provides the option to wait for the notarization to be completed with the argument --wait.

@nodejs-github-bot nodejs-github-bot added macos Issues and PRs related to the macOS platform / OSX. tools Issues and PRs related to the tools directory. labels Jul 8, 2023
tools/osx-notarize.sh Outdated Show resolved Hide resolved
@richardlau richardlau mentioned this pull request Sep 13, 2023
3 tasks
@UlisesGascon
Copy link
Member Author

UlisesGascon commented Sep 13, 2023

We have a new job in the release CI to test this script: https://ci-release.nodejs.org/job/testing-new-osx-notarization-ojs+release/.

Next steps:

  • Consolidate the script
  • Check the binaries generated
  • remove the temporary job and merge this PR (after review)
  • Try the standard job with the new script and validate the binaries in a canary build

Thanks for the great pairing session @mhdawson!

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM, @UlisesGascon thanks for putting this together

@mhdawson
Copy link
Member

To clarify for readers in terms of

By November 1, 2023, we should be able to use notarytool to notarize Node.js.

It is really By November 1, 2023 we must use notarytool so we need this PR landed and backported to all release lines before then.

@UlisesGascon
Copy link
Member Author

UlisesGascon commented Sep 15, 2023

To clarify for readers in terms of

By November 1, 2023, we should be able to use notarytool to notarize Node.js.

It is really By November 1, 2023 we must use notarytool so we need this PR landed and backported to all release lines before then.

I think that this will make imposible to notarize releases with macos10.15 since Nov, so we can repurpose/upgrade the hardware (release-nearform-macos10.15-x64-1 and release-orka-macos10.15-x64-1) and reconfigure the Jenkins jobs in separate PRs.

Screenshot 2023-09-15 at 21 05 03

Based on xcodereleases

Important: Node.js should/can be tested in macos 10.15, this only affect the release process

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 16, 2023
mhdawson pushed a commit that referenced this pull request Sep 28, 2023
Signed-off-by: Ulises Gascon <UlisesGascon@users.noreply.github.com>

Refs: nodejs/build#3385
PR-URL: #48701
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@mhdawson
Copy link
Member

landed in e97d256

@mhdawson mhdawson closed this Sep 28, 2023
@richardlau
Copy link
Member

Maybe someone with a mac can verify if the https://nodejs.org/download/nightly/v21.0.0-nightly202309295570c29780/node-v21.0.0-nightly202309295570c29780.pkg macOS build was notarized? We didn't get a "Your Mac software was successfully notarized." email from Apple for v21.0.0-nightly202309295570c29780 but maybe that's a change with the use of the new tool?

FWIW https://ci-release.nodejs.org/job/iojs+release/9667/nodes=osx11-release-pkg/console succeeded but the parsed operation_id was empty:

08:08:23 sh tools/osx-notarize.sh v21.0.0-nightly202309295570c29780
08:08:23 objc[10383]: Class AMSupportURLConnectionDelegate is implemented in both /usr/lib/libauthinstall.dylib (0x202f4f490) and /System/Library/PrivateFrameworks/MobileDevice.framework/Versions/A/MobileDevice (0x10f3942b8). One of the two will be used. Which one is undefined.
08:08:23 objc[10383]: Class AMSupportURLSession is implemented in both /usr/lib/libauthinstall.dylib (0x202f4f4e0) and /System/Library/PrivateFrameworks/MobileDevice.framework/Versions/A/MobileDevice (0x10f394308). One of the two will be used. Which one is undefined.
08:08:23 Notarization process is done with Notarytool.
08:10:23 Notarization submitted. Operation ID: 
08:10:23 ssh node-www "mkdir -p nodejs/nightly/v21.0.0-nightly202309295570c29780"

notarization_output=$(
xcrun notarytool submit \
--apple-id "$NOTARIZATION_ID" \
--password "$NOTARIZATION_PASSWORD" \
--team-id "$NOTARIZATION_TEAM_ID" \
--wait \
"node-$pkgid.pkg" 2>&1
)
if [ $? -eq 0 ]; then
# Extract the operation ID from the output
operation_id=$(echo "$notarization_output" | awk '/RequestUUID/ {print $NF}')
echo "Notarization submitted. Operation ID: $operation_id"
exit 0
else

@mhdawson
Copy link
Member

I was able to install the sep 29th on my mac without any messages/complaints so looks to me like it worked. Would be good to get other people to install/test as well to be double sure.

@BethGriggs
Copy link
Member

Looks good to me:

$ /usr/local/bin/node --version                
v21.0.0-nightly202309295570c29780
$ spctl -a -vvv -t install /usr/local/bin/node
/usr/local/bin/node: accepted
source=Notarized Developer ID
origin=Developer ID Application: Node.js Foundation (HX7739G8FX)

@mhdawson mhdawson removed dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v21.x labels Oct 2, 2023
@mhdawson
Copy link
Member

mhdawson commented Oct 2, 2023

Looks good in terms of nightlies, removed the don't land tags

@richardlau richardlau added the lts-watch-v18.x PRs that may need to be released in v18.x. label Oct 2, 2023
@richardlau richardlau added the lts-watch-v20.x PRs that may need to be released in v20.x label Oct 19, 2023
@targos targos added backported-to-v18.x PRs backported to the v18.x-staging branch. and removed lts-watch-v18.x PRs that may need to be released in v18.x. labels Oct 28, 2023
targos pushed a commit that referenced this pull request Oct 28, 2023
Signed-off-by: Ulises Gascon <UlisesGascon@users.noreply.github.com>

Refs: nodejs/build#3385
PR-URL: #48701
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
Signed-off-by: Ulises Gascon <UlisesGascon@users.noreply.github.com>

Refs: nodejs/build#3385
PR-URL: nodejs#48701
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
richardlau pushed a commit that referenced this pull request Nov 7, 2023
Signed-off-by: Ulises Gascon <UlisesGascon@users.noreply.github.com>

Refs: nodejs/build#3385
PR-URL: #48701
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@targos targos added backported-to-v20.x PRs backported to the v20.x-staging branch. and removed lts-watch-v20.x PRs that may need to be released in v20.x labels Nov 12, 2023
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
Signed-off-by: Ulises Gascon <UlisesGascon@users.noreply.github.com>

Refs: nodejs/build#3385
PR-URL: nodejs#48701
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Signed-off-by: Ulises Gascon <UlisesGascon@users.noreply.github.com>

Refs: nodejs/build#3385
PR-URL: nodejs/node#48701
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Signed-off-by: Ulises Gascon <UlisesGascon@users.noreply.github.com>

Refs: nodejs/build#3385
PR-URL: nodejs/node#48701
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported-to-v18.x PRs backported to the v18.x-staging branch. backported-to-v20.x PRs backported to the v20.x-staging branch. macos Issues and PRs related to the macOS platform / OSX. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Action required by Apple: Transition to the notarytool command-line utility
10 participants