chore: run snyk when package.json has changed or base is master #4285
Conversation
This file contains 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
spencern
changed the title
impactmass
approved these changes
Jun 1, 2018
There was a problem hiding this comment.
LGTM
@ticean, I'd like your thoughts on this before merging
ticean
reviewed
Jun 5, 2018
Dockerfile
Outdated
| # We use it to determine the base branch of a PR so we can run diffs to conditionally run circle workflows | ||
| RUN curl -L "https://github.com/stedolan/jq/releases/download/jq-1.5/jq-linux64" -o /home/node/jq | ||
| RUN chmod +x /home/node/jq | ||
|
|
There was a problem hiding this comment.
What do you think about removing the jq addition in project Dockerfile? Doesn't look like it's required. The CI script is running in the CircleCI build environment - there's no docker run ... in the command.
Unless you found otherwise, the script will run using the circleci/node:8-stretch image, as inherited from the default config. It already contains jq. You can verify that with
docker pull circleci/node:8-stretch
docker run --rm circleci/node:8-stretch jq -hThere was a problem hiding this comment.
I did not check to see if jq was in circleci/node:8-stretch if that's the case, then we shouldn't need to install jq here. I'll remove it.
ticean
approved these changes
Jun 5, 2018
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Snyk flags packages that have security vulnerabilities. Our process for resolving these dependencies involves a number of potential resolution steps: updating dependency versions, creating PRs to resolve security issues within dependencies, creating issues to notify package owners of potential security issues, and sometimes ignoring the issue if nothing can be done to resolve the issue at the time.
When we ignore an issue, we set a expiration date on the ignore that will require us to address the issue again when the ignore has expired.
Once an ignore expires, every PR in that repository will fail until the snyk issue has been resolved. This is undesirable because we would rather deal with snyk failures where they are introduced in the case of a PR adding a new dependency with a vulnerability, or in a separate PR to a release branch when the snyk failure is caused by the expiration of an ignore.
This PR adjusts the CircleCI config file to run snyk tests conditionally.
masterbranch, snyk should run.Docker
This PR installs a new binary jq into the
meteor-devstage of our Dockerfile.jqmakes it simple to work with JSON formatted data within a Unix shell environment. I'm using it to determine the base branch of a PR so that we can accurately determine whether a given PR has made changes topackage.jsonTesting
I've made a few commits in this PR that should demonstrate that this circle config is correctly determining the base/target branch of a PR and that the bash script to conditionally run snyk is working correctly. You could also copy this code into your own branch and push it to github to trigger a CI process and see if it's running snyk in the proper circumstances. I've also
sshed into a running CI instance to ensure thatjqis correctly installed and set up and that the script correctly identifies the PR number and base/target branch for a PR.Other testing is welcomed, though I'm unsure how else to best test a PR like this.