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

Add basic CircleCI config #34

Merged
merged 2 commits into from
Feb 10, 2021
Merged

Add basic CircleCI config #34

merged 2 commits into from
Feb 10, 2021

Conversation

eloquence
Copy link
Member

@eloquence eloquence commented Feb 9, 2021 •
edited
Loading

Checks that we have a ruleset matching the latest timestamp and, if so, verifies its signature using openssl

Towards #21 but does not yet add any content validation.

Test plan

  • Visual review
  • Run make verify locally
  • Observe exit code zero (echo $?) and "Verified OK" output
  • Replace signature file temporarily with an invalid one
  • Run make verify
  • Observe nonzero exit code and verification error
  • Temporarily remove defaults.rulesets file for latest timestamp
  • Run make verify
  • Observe nonzero exit code and "file not found" error

Status

Ready for initial review.

@eloquence
Copy link
Member Author

I'm not seeing CI fire yet; I'm guessing this is because the Webhooks are not set up for this repo? Marked as draft for this reason.

eloquence
eloquence commented Feb 9, 2021
View reviewed changes
.circleci/config.yml Outdated
jobs:
verify:
docker:
- image: circleci:python/3.9
Copy link
Member Author

Choose a reason for hiding this comment

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

We're not using Python here yet, but given some of the more complex logic described in #21, I'm guessing we may want to start doing so soon. If not, cimg/base:stable could be a reasonable alternative (currently maps to Ubuntu 20.04 and will track LTS).

Copy link
Contributor

@emkll emkll Feb 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Let's use cimg/* images here, since they are circleci's next gen images [1,2]. We have been using cimg:python in some areas of core (freedomofpress/securedrop#5720) and they have proven to be reliable so far.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, done.

@eloquence eloquence force-pushed the add-basic-ci branch 2 times, most recently from 88099f9 to 220b478 Compare February 9, 2021 17:42
@eloquence
Copy link
Member Author

FYI, I've tested the CI job locally via the CircleCI CLI.

@conorsch
Copy link
Contributor

conorsch commented Feb 9, 2021

I've enabled CircleCI on this repo, you can see a failing build here: https://app.circleci.com/pipelines/github/freedomofpress/securedrop-https-everywhere-ruleset/1/workflows/de11bb16-d1f2-4acb-b100-f01b32bd6d84/jobs/1 That's on main, where there is no config yet. A force-push to this branch should wake it up, @eloquence

@eloquence
Add basic CircleCI config
2e12bd4 
Checks that we have a ruleset matching the latest timestamp and,
if so, verifies its signature using openssl
@eloquence eloquence marked this pull request as ready for review February 9, 2021 17:48
Lints new CI scripts
4bcfdb8 
Implementing whatever shellcheck & yamllint complained about, as a
rule of thumb.
@conorsch conorsch self-requested a review February 9, 2021 17:56
conorsch
conorsch approved these changes Feb 9, 2021
View reviewed changes
Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

Appended a lint-only commit. Test plan checks out, looks great. Thanks, @eloquence!

emkll
emkll approved these changes Feb 10, 2021
View reviewed changes
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

thanks @eloquence

@emkll emkll merged commit c33ca40 into main Feb 10, 2021
@emkll emkll deleted the add-basic-ci branch February 10, 2021 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emkll emkll emkll approved these changes

@conorsch conorsch conorsch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eloquence @conorsch @emkll