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

Fix collision between macOS workflow artifacts in release workflows #2732

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Fix collision between macOS workflow artifacts in release workflows #2732

merged 1 commit into from
Oct 22, 2024

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Oct 14, 2024

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • [N/A] Tests for the changes have been added (for bug fixes / features)
  • [N/A] Docs have been added / updated (for bug fixes / features)
  • [N/A] UPGRADING.md has been updated with a migration guide (for breaking changes)
  • [N/A] configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

GitHub Workflows are used to automatically generate and publish production and nightly releases of the project. This is done for a range of host architectures, including macOS. The macOS builds are then put through a notarization process in a dedicated workflow job.

GitHub Actions workflow artifacts are used to transfer the generated files between sequential jobs in the workflow. The "actions/upload-artifact" and "actions/download-artifact" actions are used for this purpose.

The workflow artifact handling had to be reworked recently (#2679) in order to handle a breaking change in the 4.0.0 release of the "actions/upload-artifact". Previously, a single artifact was used for the transfer of the builds for all hosts. However, support for uploading multiple times to a single artifact was dropped in version 4.0.0 of the "actions/upload-artifact" action. So it is now necessary to use a dedicated artifact for each of the builds. These are downloaded in aggregate in a subsequent job by using the artifact name globbing and merging features which were introduced in version 4.1.0 of the "actions/download-artifact" action.

A regression was introduced at that time. The chosen approach was to use a separate set of artifacts for the non-notarized and notarized files. An overview of the sequence (the prefixes are the workflow job names):

  1. create-release-artifacts/create-nightly-artifacts: Generate builds.
  2. create-release-artifacts/create-nightly-artifacts: Upload builds to workflow artifacts
  3. notarize-macos: Download workflow artifacts.
  4. notarize-macos: Notarize macOS build from downloaded artifact.
  5. notarize-macos: Upload notarized build to workflow artifact with a different name than the source artifact.
  6. create-release/publish-nightly: Download workflow artifacts.
  7. create-release/publish-nightly: Publish builds.

The problem with this is that the artifacts for the non-notarized (uploaded by the create-release-artifacts/create-nightly-artifacts job) and notarized (created by the notarize-macos job) files are then downloaded and merged by the create-release/publish-nightly job. Since each artifact contains a file with the same path in the merged output, the contents of the last downloaded artifact overwrite the contents of the first. It happens that the non-notarized artifact is downloaded after the notarized artifact, so this file path collision results in non-notarized macOS builds being published instead of the notarized builds as intended, and as done by the workflow prior to the regression:

% wget https://downloads.arduino.cc/arduino-cli/nightly/arduino-cli_nightly-latest_macOS_ARM64.tar.gz

[...]

% tar -xf arduino-cli_nightly-latest_macOS_ARM64.tar.gz

% spctl -a -vvv -t install arduino-cli
arduino-cli: rejected
% wget https://downloads.arduino.cc/arduino-cli/arduino-cli_latest_macOS_ARM64.tar.gz

[..]

% tar -xf arduino-cli_latest_macOS_ARM64.tar.gz

% spctl -a -vvv -t install arduino-cli
arduino-cli: rejected

What is the new behavior?

The chosen solution is to delete the non-notarized artifacts after downloading each in the notarize-macos jobs. An overview of the new sequence (the prefixes are the workflow job names):

  1. create-release-artifacts/create-nightly-artifacts: Generate builds.
  2. create-release-artifacts/create-nightly-artifacts: Upload builds to workflow artifacts
  3. notarize-macos: Download macOS x86 or Apple Silicon workflow artifact.
  4. notarize-macos: Delete macOS x86 or Apple Silicon workflow artifact.
  5. notarize-macos: Notarize macOS build from downloaded artifact.
  6. notarize-macos: Upload notarized build to workflow artifact.
  7. create-release/publish-nightly: Download workflow artifacts.
  8. create-release/publish-nightly: Publish builds.

The result is that there is no file path collision when the create-release/publish-nightly job downloads and merges the artifacts.

Does this PR introduce a breaking change, and is titled accordingly?

No breaking change.

Other information

In order to facilitate the review, I made a demonstration release in my own fork, notarized with my own Apple Developer program certificate:

https://github.com/per1234/arduino-cli/actions/runs/11331487511

https://github.com/per1234/arduino-cli/releases/tag/v0.0.0-rc.2

% wget https://github.com/per1234/arduino-cli/releases/download/v0.0.0-rc.2/arduino-cli_0.0.0-rc.2_macOS_ARM64.tar.gz

[...]

% tar -xf arduino-cli_0.0.0-rc.2_macOS_ARM64.tar.gz

% spctl -a -vvv -t install arduino-cli

arduino-cli: accepted
source=Notarized Developer ID
origin=Developer ID Application: Per Tillisch (9M5NQMNWBJ)
% wget https://github.com/per1234/arduino-cli/releases/download/v0.0.0-rc.2/arduino-cli_0.0.0-rc.2_macOS_64bit.tar.gz

[...]

% tar -xf arduino-cli_0.0.0-rc.2_macOS_64bit.tar.gz
% spctl -a -vvv -t install arduino-cli
arduino-cli: accepted
source=Notarized Developer ID
origin=Developer ID Application: Per Tillisch (9M5NQMNWBJ)

I also triggered a run of the "Publish Nightly Build" in my fork:

https://github.com/per1234/arduino-cli/actions/runs/11331526910

The generated builds are available for download from the "Artifacts" section of the page I linked above.

GitHub Workflows are used to automatically generate and publish production and nightly releases of the project. This is
done for a range of host architectures, including macOS. The macOS builds are then put through a notarization process in
a dedicated workflow job.

GitHub Actions workflow artifacts are used to transfer the generated files between sequential jobs in the workflow. The
"actions/upload-artifact" and "actions/download-artifact" actions are used for this purpose.

The workflow artifact handling had to be reworked recently in order to handle a breaking change in the 4.0.0 release of
the "actions/upload-artifact". Previously, a single artifact was used for the transfer of the builds for all hosts.
However, support for uploading multiple times to a single artifact was dropped in version 4.0.0 of the
"actions/upload-artifact" action. So it is now necessary to use a dedicated artifact for each of the builds. These are
downloaded in aggregate in a subsequent job by using the artifact name globbing and merging features which were
introduced in version 4.1.0 of the "actions/download-artifact" action.

A regression was introduced at that time. The chosen approach was to use a separate set of artifacts for the
non-notarized and notarized files. An overview of the sequence (the prefixes are the workflow job names):

1. create-release-artifacts/create-nightly-artifacts: Generate builds.
2. create-release-artifacts/create-nightly-artifacts: Upload builds to workflow artifacts
3. notarize-macos: Download workflow artifacts.
4. notarize-macos: Notarize macOS build from downloaded artifact.
5. notarize-macos: Upload notarized build to workflow artifact with a different name than the source artifact.
6. create-release/publish-nightly: Download workflow artifacts.
7. create-release/publish-nightly: Publish builds.

The problem with this is that the artifacts for the non-notarized (uploaded by the
create-release-artifacts/create-nightly-artifacts job) and notarized (created by the notarize-macos job) files are then
downloaded and merged by the create-release/publish-nightly job. Since each artifact contains a file with the same path
in the merged output, the contents of the last downloaded artifact overwrite the contents of the first. It happens that
the non-notarized artifact is downloaded after the notarized artifact, so this file path collision results in
non-notarized macOS builds being published instead of the notarized builds as intended, and as done by the workflow
prior to the regression:

```
% wget https://downloads.arduino.cc/arduino-cli/nightly/arduino-cli_nightly-latest_macOS_ARM64.tar.gz

[...]

% tar -xf arduino-cli_nightly-latest_macOS_ARM64.tar.gz

% spctl -a -vvv -t install arduino-cli
arduino-cli: rejected
```

```
% wget https://downloads.arduino.cc/arduino-cli/arduino-cli_latest_macOS_ARM64.tar.gz

[..]

% tar -xf arduino-cli_latest_macOS_ARM64.tar.gz

% spctl -a -vvv -t install arduino-cli
arduino-cli: rejected
```

The chosen solution is to delete the non-notarized artifacts after downloading each in the notarize-macos jobs. An
overview of the new sequence (the prefixes are the workflow job names):

1. create-release-artifacts/create-nightly-artifacts: Generate builds.
2. create-release-artifacts/create-nightly-artifacts: Upload builds to workflow artifacts
3. notarize-macos: Download macOS x86 or Apple Silicon workflow artifact.
4. notarize-macos: Delete macOS x86 or Apple Silicon workflow artifact.
5. notarize-macos: Notarize macOS build from downloaded artifact.
6. notarize-macos: Upload notarized build to workflow artifact.
7. create-release/publish-nightly: Download workflow artifacts.
8. create-release/publish-nightly: Publish builds.

The result is that there is no file path collision when the create-release/publish-nightly job downloads and merges the
artifacts.
@per1234 per1234 added os: macos Specific to macOS operating system topic: infrastructure Related to project infrastructure type: imperfection Perceived defect in any part of project labels Oct 14, 2024
@per1234 per1234 requested a review from cmaglie October 14, 2024 17:00
@cmaglie cmaglie self-assigned this Oct 22, 2024
@cmaglie cmaglie merged commit 26b0b55 into arduino:master Oct 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
os: macos Specific to macOS operating system topic: infrastructure Related to project infrastructure type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants