-
-
Notifications
You must be signed in to change notification settings - Fork 437
Enable "Arduino IDE" workflow use by contributors #1115
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
Merged
per1234
merged 2 commits into
arduino:main
from
per1234:enable-workflow-for-contributors
Jun 28, 2022
Merged
Enable "Arduino IDE" workflow use by contributors #1115
per1234
merged 2 commits into
arduino:main
from
per1234:enable-workflow-for-contributors
Jun 28, 2022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlbyIanna
approved these changes
Jun 28, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @per1234 !
Previously, there was some code duplication of the complex code signing certificate handling commands, which made the related code more difficult to understand, maintain, and develop. The cause of this duplication is that there is a separate certificate for each operating system, each of which is stored in separate repository secrets, as well as a different certificate file extension for each OS. Since the secret names and file extensions are associated with the operating system, it is most logical to define them via attributes alongside the operating system definition in the job matrix configuration already used to generate the parallel job runs for native build on each OS. That done, the certificate handling commands are universal and the system can easily expand to additional host targets (e.g., Apple M1) as time goes on.
GitHub Actions workflows may require access to privileged information in order to perform certain operations. GitHub provides the capability for doing this via "repository secrets". For security reasons, repository secrets are only accessible to a GitHub Actions workflow run when it is triggered by an event from within the repository containing the secret. This means that a workflow which requires such secrets would fail when run in a fork (unless the fork owner was able to set up their own secrets with suitable values). In order to make the relevant components of the CI system friendly for use in forks by contributors validating their work in preparation for submitting a PR, when the operations that require access to a secret are supplemental, those operations should be configured to only run from branches of the parent repository. Due to its unfortunate monolithic design, in addition to operations useful to contributors, the "Arduino IDE" workflow contains several such supplemental operations: - Code signing - Publishing release artifacts to Arduino's server Some attempt was previously made to configure the workflow to skip these operations when run in forks, but that configuration was not done correctly. This made the workflow only usable by contributors with a deep enough understanding of GitHub Actions to be able to make the necessary modifications provisionally every time they needed to use the workflow. The average contributor would not be capable or willing to do this, which might result in PRs being submitted in a less validated state, increasing the burden on maintainers. The specific misconfigurations: **`build` job was conditional on the workflow running from `arduino/arduino-ide`** The job itself can run just fine in a fork, so there is no reason to impose this restriction. Since the time this conditional was added, some changes have been made to the GitHub Actions system which makes this sort of configuration unnecessary: - GitHub Actions is globally disabled in forks by default - Workflows which contain a `schedule` trigger (as is the case with this one) are individually disabled by default, requiring the repository owner to enable it specifically even after enabling GitHub Actions in general. This means this workflow will never run unexpectedly in a fork. The fork owner will always have intentionally enabled it. So this conditional can be removed completely. **Code signing was conditional on PR being submitted from a branch of the base repo** This would cause a spurious failure of the signing operation on PRs made within the contributor's fork when the signing secrets were not defined. The more appropriate condition of whether the signing secrets are defined or not is now used. The environment variable name has been updated accordingly. **`release` job was conditional on running from `arduino/arduino-ide`** The GitHub release creation step of this job can run in any repository. It is only the step that uploads to Arduino's AWS server which would only make sense to run from `arduino/arduino-ide`. So the conditional is moved to the AWS upload step, allowing contributors to test the workflow's release operation in their forks to validate related proposals.
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
GitHub Actions workflows may require access to privileged information in order to perform certain operations. GitHub provides the capability for doing this via "repository secrets".
For security reasons, repository secrets are only accessible to a GitHub Actions workflow run when it is triggered by an event from within the repository containing the secret. This means that a workflow which requires such secrets would fail when run in a fork (unless the fork owner has set up their own secrets with suitable values).
In order to make the relevant components of the CI system friendly for use in forks by contributors validating their work in preparation for submitting a PR, when the operations that require access to a secret are supplemental, those operations should be configured to only run from branches of the parent repository.
Due to its unfortunate monolithic design, in addition to operations useful to contributors, the "Arduino IDE" workflow contains several such supplemental operations:
Some attempt was previously made to configure the workflow to skip these operations when run in forks, but that configuration was not done correctly. This made the workflow only usable by contributors with a deep enough understanding of GitHub Actions to be able to make the necessary modifications provisionally every time they needed to use the workflow.
The average contributor would not be capable or willing to do this, which might result in PRs being submitted in a less validated state, increasing the burden on maintainers.
The specific misconfigurations
build
job conditional on the workflow running fromarduino/arduino-ide
The job itself can run just fine in a fork, so there is no reason to impose this restriction.
Since the time this conditional was added, some changes have been made to the GitHub Actions system which makes this sort of configuration unnecessary:
schedule
trigger (as is the case with this one) are individually disabled by default, requiring the repository owner to enable it specifically even after enabling GitHub Actions in general.This means this workflow will never run unexpectedly in a fork. The fork owner will always have intentionally enabled it.
So this conditional can be removed completely.
Code signing conditional on PR being submitted from a branch of the base repo
This would cause a spurious failure of the signing operation on PRs made within the contributor's fork when the signing secrets were not defined.
The more appropriate condition of whether the signing secrets are defined or not is now used. The environment variable name has been updated accordingly.
release
job conditional on running fromarduino/arduino-ide
The GitHub release creation step of this job can run in any repository. It is only the step that uploads to Arduino's AWS server which would only make sense to run from
arduino/arduino-ide
.So the conditional is moved to the AWS upload step, allowing contributors to test the workflow's release operation in their forks to validate related proposals.
Reviewer checklist