📝 Note
Setting up GitHub Actions is entirely optional for this series. I do not link back to anything in this post in any future post, so you don't need to set this up if you don't want to.
Introduction
Before we continue implementing our CLI, let's take time to set up some GitHub Actions to build and test our commits. We'll use the actions defined in the actions-rs GitHub Organization.
Table of Contents
Build and test
The workflow I normally follow when coding is to create a short-lived branch on which all commits are built and tested then create a Pull Request (PR) into the master branch. Let's set this up. Check out a new branch, let's call it actions. Then, create a file called build.yml
under .github/workflows/
.
➜ mkdir -p .github/workflows
➜ touch .github/workflows/build.yml
We'll create a simple action that builds and tests our CLI. Add the following to build.yml
.
on: [push]
name: build
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
- name: build
uses: actions-rs/cargo@v1
with:
command: build
- name: test
uses: actions-rs/cargo@v1
with:
command: test
on
defines the event(s)1 which should trigger this workflow. We set it to run on a push
to any branch. The workflow has a single job, called build
. This job runs on the ubuntu-latest
Virtual Environment. It checks out our code, installs the stable Rust toolchain and runs a cargo build
then runs a cargo test
.
Commit this change and push to your repo. GitHub Actions will then run this workflow. Once complete, when you click the Actions
tab on your repo, you should see something like this:
I had two builds, as I rebased a commit, you will likely only see one. Click into the build, then click the build
job on the left with the checkmark and it will show you the steps in the workflow.
This is a good start. All commits will be built and tested. We can add a check to PR's so that they will block merges if this workflow fails. But first, let's add some improvements. The code up to this point can be seen here.
Clippy lint
One thing we did not do in the last post was to run a linter. Rust has an awesome linter called Clippy which can highlight quite a lot of issues. Let's run it in our project now.
➜ cargo clippy
Finished dev [unoptimized + debuginfo] target(s) in 0.01s
No issues, great! Like any test, let's see it fail. Or in this case, make Clippy give out to us. Let's add something Clippy does not like to main.rs
.
fn main() -> Result<(), String> {
assert!(false); // clippy won't like this
let args = std::env::args().skip(1).collect();
let validator = CertValidator;
execute(validator, args)
}
Assertions on a constant are optimized out by the compiler2, so Clippy will warn about this.
➜ cargo clippy
warning: `assert!(false)` should probably be replaced
--> src/main.rs:35:5
|
35 | assert!(false);
| ^^^^^^^^^^^^^^^
|
= note: `#[warn(clippy::assertions_on_constants)]` on by default
= help: use `panic!()` or `unreachable!()`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
= note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
warning: 1 warning emitted
Finished dev [unoptimized + debuginfo] target(s) in 0.01s
📝 Note
Sometimes on a subsequent run, Clippy will just say it's successful and not run correctly. See this issue for more information. If you think this is happening you can force a full re-run by modifying a file, e.g.
touch src/main.rs; cargo clippy
. It seems this should be fixed, but might only be fixed in nightly Rust, not stable.
Great! Just what we want. Let's leave this assertion here and update our existing workflow to add a lint step which runs Clippy. The clippy-check action will allow us to easily do this.
on: [push]
name: build
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
toolchain: table
- name: build
uses: actions-rs/cargo@v1
with:
command: build
- name: test
uses: actions-rs/cargo@v1
with:
command: test
- name: lint
uses: actions-rs/clippy-check@v1
with:
token: ${{ secrets.GITHUB_TOKEN }}
Commit both the assertion and the lint
addition to the workflow and push. Once the workflow run finishes, click into the Clippy step and you should see something like this:
The warning is now attached to a commit and links to a specific file. Click the text "Check warning on line 35 in src/main.rs" in the annotation and it should bring you to exactly where this issue occurs.
This is nice! Create a PR and you will see this annotation exactly as it is above in the Files Changed
tab. I find this useful, even if I am the sole developer. Sometimes I forget to run Clippy and I always do a quick review of the code in a PR. Create a PR into the master branch for this and you will also see the checks have all passed. Don't merge it yet!
Let's make one last change. The code up this point can be found here.
Block PR's on failing checks
If we somehow manage to check something in which does not compile, has failing tests, or has Clippy lint issues which it is set to deny, we should block the PR, i.e. not allow it to be merged until these issues are fixed. This may seem like overkill if you are the only dev on a repo, but I do this all the time because I make these mistakes! No matter how careful I try to be.
Before we configure our repo to block PRs, a quick note on Clippy lints.
Clippy deny lints
The lint we have in our PR above is just a warning. Clippy also has several lints that will cause it to fail if detected. These are lints at the Deny
level in the list of Clippy lints. For example, the lint approx_constant checks for constants which are defined under std::f32::consts or std::f64::consts. If it finds matching constants in your code it will error out.
To see this in action, update main
with an unused variable which approximates pi. Before you do that, make sure you have committed your changes up to here, so we can easily reset. I've commented out the assert!(false)
we added earlier so we don't also get a warning for that from Clippy.
fn main() -> Result<(), String> {
// assert!(false);
let _ = 3.14;
let args = std::env::args().skip(1).collect();
let validator = CertValidator;
execute(validator, args)
}
Run Clippy and it will give an error, and return a non-zero exit code.
➜ cargo clippy
Checking cert-decode v0.1.0 (/home/chaospie/repos/blog-cert-decode/cert-decode)
error: approximate value of `f{32, 64}::consts::PI` found. Consider using it directly
--> src/main.rs:36:13
|
36 | let _ = 3.14;
| ^^^^
|
= note: `#[deny(clippy::approx_constant)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
error: aborting due to previous error
error: could not compile `cert-decode`.
To learn more, run the command again with --verbose.
➜ echo $?
101
echo $?
will print the return code of the last command executed in the shell. Don't forget to reset these changes, either manually or by running git checkout src/main.rs
.
Protected branch rule
The configuration for blocking PRs is defined with a protected branch3 rule. Go to Settings -> Branches -> Branch protection rules -> Add rule in you repo. Set the Branch name pattern to master
and select the status checks to verify in PRs as follows.
📝 Note
I have an extra status check here called
build_and_test
because I had initially defined the job in our workflow YAMLbuild_and_test
. It is now calledbuild
.
Now if we raise a PR with any failure in our build
job - the job in our workflow YAML - or the Clippy step specifically, the PR will be blocked from merging. This is what it would look like:
And the details of the lint error:
Note that because I am the only dev, I am an admin on this repo so I can still merge. But it is glaringly obvious something is wrong. This has saved me a few times.
Clean up and merge
The PR you raised, before our Clippy deny lints digression, has a warning because of the assert!(false)
call in main
. Clean that up, commit, push, and merge it. main
should look as follows again:
fn main() -> Result<(), String> {
let args = std::env::args().skip(1).collect();
let validator = CertValidator;
execute(validator, args)
}
The only change in your PR should be the build.yml
file. Once merged the workflow will run again on master
and be successful. There is one more workflow I usually add to Rust repositories. So let's add that also. See here for the code up to this point.
Security audit
GitHub tracks vulnerabilities from several supported language package managers. It will alert you if your repository contains a vulnerable dependency. It doesn't support cargo
yet, I'm not sure if they have plans to support it either. However, cargo
has the cargo-audit plugin and we can use this in a workflow. actions-rs has an action just for this, audit-check.
Let's create a new branch called audit-workflow
and add a new workflow at .github/workflows/security-audit.yml
.
name: Security audit
on:
schedule:
- cron: "0 8 * * *"
push:
paths:
- "**/Cargo.*"
pull_request:
branches:
- master
jobs:
security_audit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions-rs/audit-check@v1
with:
token: ${{ secrets.GITHUB_TOKEN }}
This will run the audit check at 8 am UTC every day, whenever we commit and push a change to any branch which updates Cargo.toml
or Cargo.lock
and whenever we create a PR into master.
Why all these cases? Well, we do it daily at 8 am as a vulnerability can be found in dependencies at any time, so we should check daily that nothing new was found. We do it on changes to Cargo.*
files so we can immediately detect if a change in dependencies has introduced and vulnerability. Finally, we do it on a PR into master
so we can be sure, at least at the time of the PR, that we are not pushing known vulnerabilities to our master
branch.
Commit and push this workflow change to our branch. Let's test that it works. To do so add a dependency to Cargo.toml
.
[package]
name = "cert-decode"
version = "0.1.0"
authors = ["Stephen OBrien <wayofthepie@users.noreply.github.com>"]
edition = "2018"
[dependencies]
x509-parser = "0.7.0"
Run a build with cargo build
so Cargo.lock
gets updated. Because this is just a test, we can edit the last commit and then later remove the dependency and edit again. Don't ever do this on a shared branch! This is how to do it:
$ git add Cargo.*
$ git commit --amend --no-edit
$ git push origin +HEAD
The +HEAD
in the git push
command says to force push to the branch HEAD
4 points to. A force push rewrites history, this is why you should not do it on a shared branch. HEAD
is the current branch. As we have changed the Cargo.*
files the audit workflow should run. It will hopefully be successful.
To go back to the previous state you can remove the dependency from Cargo.toml
:
[package]
name = "cert-decode"
version = "0.1.0"
authors = ["Stephen OBrien <wayofthepie@users.noreply.github.com>"]
edition = "2018"
[dependencies]
Build with cargo build
, and edit the last commit again.
$ git add Cargo.*
$ git commit --amend --no-edit
$ git push origin +HEAD
Before creating another PR for this, you should update the branch protection settings and add security_audit
as a status check.
Now you can create a PR into master
and this should run again. If it's green, merge it. The code up to the end of this post can be found here.
📝 Note
This audit workflow can take some time to run as it first needs to install
cargo-audit
. This takes about 5 minutes looking at the logs for a run. We can improve this with caching, but I'll leave that for another post.
Conclusion
We now have:
- A build and test workflow for all our commits.
- Security auditing on our dependencies.
- Checks which block PR's if we have broken builds, lint issues, or vulnerabilities in dependencies
This is something I always set up at the start of a project. We'll go back to implementing the CLI in the next post.
-
See the assertions_on_constants lint. ↩
Top comments (0)