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

setup continuous integration #17

Merged
merged 2 commits into from
Jan 26, 2021
Merged

setup continuous integration #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd35109
use cargo deny to check dependency licenses
webern Jan 20, 2021
27c1c22
setup continuous integration
webern Jan 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/cache-bust
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# this file provides a manual way to clear out github actions caches. any change
# to this file will cause all github action caches to miss. increment the number
# below by 1 if you need to clear the caches.
1
55 changes: 55 additions & 0 deletions .github/workflows/updater-ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
name: Updater CI
on:
pull_request:
paths-ignore:
- '**.md'
branches: ['*']
push:
paths-ignore:
- '**.md'
branches: [develop]
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2

- name: Cargo Home Cache
uses: actions/cache@v2
env:
cache-name: cargo-home
with:
path: /usr/share/rust/.cargo
key: ${{ hashFiles('.github/cache-bust', '/usr/share/rust/.cargo/bin/cargo') }}-${{ hashFiles('updater/Cargo.lock', 'integ/Cargo.lock') }}
restore-keys: |
${{ hashFiles('.github/cache-bust', '/usr/share/rust/.cargo/bin/cargo') }}-${{ hashFiles('updater/Cargo.lock', 'integ/Cargo.lock') }}
${{ hashFiles('.github/cache-bust', '/usr/share/rust/.cargo/bin/cargo') }}-

- name: Updater Build Cache
uses: actions/cache@v2
env:
cache-name: updater-target
with:
path: updater/target
key: ${{ hashFiles('.github/cache-bust') }}-${{ hashFiles('updater/Cargo.lock') }}
restore-keys: |
${{ hashFiles('.github/cache-bust') }}-
Copy link
Member

Choose a reason for hiding this comment

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

Is this matched as a prefix? Or should this say:

Suggested change
${{ hashFiles('.github/cache-bust') }}-
${{ hashFiles('.github/cache-bust') }}-${{ hashFiles('updater/Cargo.lock') }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The syntax is correct. What it's doing is providing a fallback to a cache where the first part matches but the second part doesn't. So when the Cargo.lock changes, a cache miss will happen on the two-part key, but a cache hit will happen on the one-part-only key. Then since a cache miss happened, a new cache will be created on the full two part key at the end of the build.


- name: Integ Build Cache
uses: actions/cache@v2
env:
cache-name: integ-target
with:
path: integ/target
key: ${{ hashFiles('.github/cache-bust') }}-${{ hashFiles('integ/Cargo.lock') }}
restore-keys: |
${{ hashFiles('.github/cache-bust') }}-
Copy link
Member

Choose a reason for hiding this comment

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

Same question here (is this a prefix?).


- run: rustup update stable && cargo install cargo-deny
- run: make ci

image:
webern marked this conversation as resolved.
Show resolved Hide resolved
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- run: make image
25 changes: 25 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,28 @@ image: fetch-sdk
.PHONY: fetch-sdk
fetch-sdk: # fetches and loads the image we use to build the updater docker image
scripts/load-bottlerocket-sdk.sh --site ${BOTTLEROCKET_SDK_SITE} --image ${BUILDER_IMAGE}

.PHONY: check-licenses
check-licenses:
cd updater && cargo deny check licenses
cd integ && cargo deny check licenses

.PHONY: unit-tests
unit-tests:
cd updater && cargo test --locked
cd integ && cargo test --locked

.PHONY: build
build:
cd updater && cargo build --locked
cd integ && cargo build --locked

.PHONY: lint
lint:
cd updater && cargo fmt -- --check
cd updater && cargo clippy --locked -- -D warnings
cd integ && cargo fmt -- --check
cd integ && cargo clippy --locked -- -D warnings
Comment on lines +42 to +45
Copy link
Member

Choose a reason for hiding this comment

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

This seems to point to a Cargo workspace being a reasonable choice to manage the crates driven by this Makefile. Is that something that was considered and dismissed? Can you see if a workspace simplifies the developer's experience (and the Makefile's verbosity)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tjkirch and I rejected a workspace because:

  • workspaces can be kind of a pain sometimes.
  • the updater project should be self-contained since that is the actual product, i.e. if you want to build and deploy this, you shouldn't have to build all the dependencies that only the integ crate needs.

It's not dissimilar to having tools and sources in Bottlerocket.


.PHONY: ci # these are all of the checks (except for image) that we run for ci
ci: check-licenses lint build unit-tests
Copy link
Member

Choose a reason for hiding this comment

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

I'd have called this all, though the conventional names use it for a tighter scoped use case. Either that or have the CI builds run this (with some minor renaming):

# Run easy lints on projects
# Run build for projects
# Run checks for the projects
make lint build check

Copy link
Contributor Author

@webern webern Jan 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

What I'm going for is an easy, single thing to remember that I can type locally to see if my ci run will pass before pushing.

make ci or make all would both be fine, but make lint build check would suck because I would have to open the CI yaml definition to remember what to do.

I used make ci for the same purpose in tough, so keeping consistent with that pattern would make me happy, but if we like all better, I can change it.

38 changes: 38 additions & 0 deletions integ/deny.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
[licenses]
unlicensed = "deny"

# Deny licenses unless they are specifically listed here
copyleft = "deny"
allow-osi-fsf-free = "neither"
default = "deny"

# We want really high confidence when inferring licenses from text
confidence-threshold = 0.93

# Licenses that are allowed but unused are commented out to silence warnings
allow = [
"Apache-2.0",
#"BSD-2-Clause",
"BSD-3-Clause",
"BSL-1.0",
#"CC0-1.0",
#"ISC",
"MIT",
# OpenSSL",
"Unlicense",
"Zlib"
]

[[licenses.clarify]]
name = "ring"
expression = "MIT AND ISC AND OpenSSL"
license-files = [
{ path = "LICENSE", hash = 0xbd0eed23 },
]

[[licenses.clarify]]
name = "webpki"
expression = "ISC"
license-files = [
{ path = "LICENSE", hash = 0x001c7e6c },
]
38 changes: 38 additions & 0 deletions updater/deny.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
[licenses]
unlicensed = "deny"

# Deny licenses unless they are specifically listed here
copyleft = "deny"
allow-osi-fsf-free = "neither"
default = "deny"

# We want really high confidence when inferring licenses from text
confidence-threshold = 0.93

# Licenses that are allowed but unused are commented out to silence warnings
allow = [
"Apache-2.0",
#"BSD-2-Clause",
"BSD-3-Clause",
"BSL-1.0",
#"CC0-1.0",
"ISC",
"MIT",
"OpenSSL",
"Unlicense",
"Zlib"
]

[[licenses.clarify]]
name = "ring"
expression = "MIT AND ISC AND OpenSSL"
license-files = [
{ path = "LICENSE", hash = 0xbd0eed23 },
]

[[licenses.clarify]]
name = "webpki"
expression = "ISC"
license-files = [
{ path = "LICENSE", hash = 0x001c7e6c },
]