-
Notifications
You must be signed in to change notification settings - Fork 150
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
Added trigger for Google Cloud Build #36
base: master
Are you sure you want to change the base?
Conversation
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.
A few comments inline, but the major ones are:
- please install & use
snyk-to-html
for all supported products (we just added code) and get 4 separate artifacts, soresults-code.html
,results-open-source.html
,results-container.html
,results-iac.html
- please rename the file to
GoogleCloudBuild-docker-generic.yaml
(the naming scheme will be detailed in a future README file, but the idea is roughly to explain within the filename where the file should go to, the type of installation for the snyk tool and the project type)
GoogleCloudBuild/config.yaml
Outdated
args: | ||
- '-c' | ||
- |- | ||
snyk config set api=${_SNYK_TOKEN} |
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.
does google cloud build support "before" directives, so that this command is only written once in this file?
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.
A YAML anchor would also help to make this DRYer: https://support.atlassian.com/bitbucket-cloud/docs/yaml-anchors/
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.
Great to see this being improved, thank you!
- Can you remove all other GCB files for simplicity?
- Please add screenshots & description to the README.md file, similar to other CI/CD scripts recently updated
- '-c' | ||
- |- | ||
snyk config set api=${_SNYK_TOKEN} | ||
snyk test --json-file-output=results-open-source.html || true |
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.
On the file output:
this line (and L16, L24, L32) do not produce the expected output.
--json-file-output
will create a JSON file, not a HTML file as the name would suggest.
Please add a snyk-to-html
step in between. You can refer to the example in
snyk-cicd-integration-examples/AzurePipelines/AzurePipelines-npm-generic-html.yml
Line 42 in c21e80b
snyk test --all-projects --json-file-output=results.json |
On the return code:
As shown in that file, we capture the return code and exit with it.
Unfortunately GCB does not support something like allow_failure
, yet, so the exit
line should be something like:
exit 0 # exit $RETURN_CODE
Which shows the developer our approach for exiting with 0 by default, but showing them how to enable the security gate again.
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.
I've checked in the wrong codes previously hence the confusion. Changes made.
@@ -0,0 +1,42 @@ | |||
# You can find the Snyk Official CLI container images here: https://github.com/snyk/cli#snyk-cli-in-a-docker-image |
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.
Please add a line or two of comments on the top of the file similar to other recently updated file to explain what is being shown in the file.
entrypoint: bash | ||
artifacts: | ||
objects: | ||
location: 'gs://<STORE_NAME>/scan_output' |
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.
I'm not familiar with GCB here, but is scan_output
a local folder or is that a output folder?
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.
output folder
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.
It's ok, I've removed this output folder on Google Cloud Storage. I will just store all in the root directory of the storage. All good.
entrypoint: bash | ||
artifacts: | ||
objects: | ||
location: 'gs://<STORE_NAME>/scan_output' |
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.
STORE_NAME seems to be a required variable. Please explain the requirement on the top of file as well.
- '-c' | ||
- |- | ||
snyk config set api=${_SNYK_TOKEN} | ||
snyk container test <CONTAINER_IMAGE> --json-file-output=results-container.html || true |
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.
please add the --file=Dockerfile
parameter as well so it's easier to copy/paste
No description provided.