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.
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
Add snyk monitor (closes #52) #53
Conversation
This needs updating slightly as I've noticed it doesn't pick up the project name automatically, but uses the folder name. You can fix this however:
Additionally, Dilyan noticed some issues with sbt 1.3 in his testing. Not sure if you had the same issue when testing this locally? |
- name: Run Snyk to check for vulnerabilities | ||
uses: snyk/actions/scala@master | ||
with: | ||
command: monitor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
command: monitor | |
command: monitor | |
args: --project-name=stream-collector --org=data-cap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to specify --org=data-cap if the service account token is generated against the Data Cap organisation in Snyk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, to know! Thanks @paulboocock!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But somehow you do need to specify --project-name
if I'm not mistaken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll probably need args: --project-name=stream-collector
yes. It tries to find the project name from config, or it defaults to the containing folder name. Unfortunately Github actions maps this into a workspace
folder in the docker image that's running behind the scenes. So specifying --project-name
just guarentees we get nice names in the Snyk UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benjben I think this is explained, could you change PR status, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
As Paul mentioned there might be problems with SBT 1.3, but it worked when I added latest RC of sbt-dependency-graph to project plugins.
- name: Run Snyk to check for vulnerabilities | ||
uses: snyk/actions/scala@master | ||
with: | ||
command: monitor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mine works without monitor
. Do we need it? https://github.com/snowplow/enrich/blob/release/1.3.0/.github/workflows/snyk.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it defaults to test
which doesn't upload the report to the Snyk UI. Currently, we're looking at running monitor
so we have full visibility of our projects in the Snyk UI.
test
is useful if you want your build to fail if there are any vulnerabilities, which we will likely add to our build processes in the future once we're more comfortable with where we are at with Snyk.
- name: Run Snyk to check for vulnerabilities | ||
uses: snyk/actions/scala@master | ||
with: | ||
command: monitor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But somehow you do need to specify --project-name
if I'm not mistaken.
yeah, but I added the global plugin and it worked. Now I checked the version, I use the same as Dilyan mentioned. |
Thanks Anton! I already had that :) |
c97b26a
to
5694d09
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This adds snyk monitor as gh action, just because it's a lot easier than configure the Travis at the moment and it should happen only on push to master I believe, so it's not needed to install snyk via npm in travis (and waste time for it on every build).