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

improve CI actions #301

Merged
merged 16 commits into from
Apr 2, 2024
Merged

improve CI actions #301

merged 16 commits into from
Apr 2, 2024

Conversation

michaelmera
Copy link
Contributor

@michaelmera michaelmera commented Mar 4, 2024 •
edited

WARNING: The default github actions permissions for the entire tlspuffin organization have been set to "restricted" mode. If you came here because a previously working workflow now fails because of insufficient permissions, set the required permissions explicitly in this particular workflow. See the github documentation for the list of default permissions granted in "restricted" mode.

This PR aims to provide several improvements over the current github actions:

  • add justfile recipe to run github actions locally
  • fix calls to Swatinem/rust-cache
  • update external actions relying on deprecated Node 16
  • add ability to control the CI workflow based on PR labels
  • add smoke test before running the complete build matrix
  • fix CI/CD permissions

See also:

PENDING

@michaelmera michaelmera self-assigned this Mar 4, 2024
@michaelmera michaelmera mentioned this pull request Mar 6, 2024
Merged
@michaelmera michaelmera force-pushed the improve-ci branch 6 times, most recently from ba229ef to 62d706c Compare March 12, 2024 09:37
@michaelmera michaelmera added the ci:full Run full CI checks on labeled PR label Mar 12, 2024
@michaelmera michaelmera marked this pull request as ready for review March 12, 2024 09:47
@michaelmera michaelmera force-pushed the improve-ci branch 3 times, most recently from df1f50c to e93471b Compare March 25, 2024 08:08
LCBH
LCBH requested changes Mar 27, 2024
View reviewed changes
Copy link
Contributor

@LCBH LCBH left a comment

Choose a reason for hiding this comment

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

Seems like CI is now disabled on PRs forked from a branch different from main, despite using the ci:full flag. Would be nice to keep the control on when CI is enabled with the flags.

.github/workflows/ci.yml Show resolved Hide resolved
maxammann
maxammann reviewed Mar 27, 2024
View reviewed changes
Copy link
Contributor

@maxammann maxammann left a comment

Choose a reason for hiding this comment

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

Feels like some changes from other PRs leaked into this PR.

puffin/src/cli.rs Outdated Show resolved Hide resolved
tlspuffin/src/openssl/mod.rs Outdated Show resolved Hide resolved
test_tlspuffin.sh Outdated Show resolved Hide resolved
justfile Show resolved Hide resolved
justfile Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.github/workflows/run-checks.yml Show resolved Hide resolved
.github/act/events/pr_to_main.json Show resolved Hide resolved
.github/workflows/on_main_push.yml Outdated Show resolved Hide resolved
@michaelmera
Copy link
Contributor Author

michaelmera commented Mar 28, 2024 •
edited

Seems like CI is now disabled on PRs forked from a branch different from main, despite using the ci:full flag. Would be nice to keep the control on when CI is enabled with the flags.

@LCBH I'm not sure what you mean. This PR makes CI run a reduced set of checks on every PR, regardless of the source branch and source repository. The label ci:full activates the full set of CI checks on the annotated PR. The full set of checks always run when pushing to the branches main and trailofbits. I didn't change the behavior in that respect. Did I miss something?

edit: After some more investigation, the problem noticed in other PRs is not linked to the changes proposed here. However, it also appeared that the current behavior is more restrictive than what we suggest in this PR: currently, CI only runs on PRs with source branch main and trailofbits. We have agreed in an offline discussion that the new behavior proposed here is better suited for our development process.

@LCBH
Copy link
Contributor

LCBH commented Mar 29, 2024

Seems like CI is now disabled on PRs forked from a branch different from main, despite using the ci:full flag. Would be nice to keep the control on when CI is enabled with the flags.

Will be resolved after the merge.

@michaelmera
ci: add recipe to run github actions locally
02ceef1
@michaelmera
ci: fix wrong input in call to workflow Swatinem/rust-cache@v2
84a7dad
@michaelmera
ci: update external actions with Node 16 deprecation warnings
cb53dac 
Github-hosted runners have taken a first step to transition away from Node 16,
as it is reaching end of life.  As a consequence, a large number of actions
our CI depends on have been updated to run with Node 20 instead.

This commit updates to these new versions.

- see: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/
@michaelmera
ci: ensure save-cache flag is correctly propagated
dcf4bbc
@michaelmera
ci: allow setting CI checks level through PR labels
e974f87
@michaelmera
ci: merge workflows for pull request and push
cf0c17f
@michaelmera
ci: print CI/CD configuration at the end of the configure job
e39fb8b
@michaelmera
ci: add recipe to run super-linter locally
ae9525e
@michaelmera
ci: add smoke test before expensive CI/CD stages
21bd924
@michaelmera michaelmera merged commit afb3755 into tlspuffin:main Apr 2, 2024
33 checks passed
@michaelmera michaelmera deleted the improve-ci branch April 2, 2024 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxammann maxammann maxammann left review comments

@LCBH LCBH LCBH approved these changes

Assignees

@michaelmera michaelmera

Labels
ci:full Run full CI checks on labeled PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@michaelmera @LCBH @maxammann