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.

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 "file-output-argument" parameter to the snyk test command. #112

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jlam-bills
Copy link

Add "file-output-argument" parameter to the snyk test command.

Issue trying to solve:
Adding "--json-file-output=..." option to "additional-arguments" parameter will cause the snyk monitor to fail.

Error: "The following option combination is not currently supported: monitor + json-file-output"

Issue trying to solve: Adding the option
"--json-file-oupt=..." to "additional-arguments" parameter will cause the snyk
monitor to fail.

Error: "The following option combination is not currently supported:
monitor + json-file-output"
@jlam-bills jlam-bills requested a review from a team as a code owner September 9, 2023 00:12
@CLAassistant
Copy link

CLAassistant commented Sep 9, 2023

CLA assistant check
All committers have signed the CLA.

@bastiandoetsch
Copy link
Contributor

Hey there, thanks for the PR. Why would this help your case? Snyk monitor just does not support outputting to a file, so I'm unclear, why we should allow that parameter in the general config.

@slyszkowski
Copy link

Hello @bastiandoetsch it appears that additional-arguments var is being passed to both:

  • snyk monitor
  • snyk test

If passing --json-file-output to this var it appears it breaks the pipeline due to:

The following option combination is not currently supported: monitor + json-file-output

It should be possible to pass arguments related to snyk test only. Such option is already supported for snyk monitor -> additional-monitor-arguments

@bastiandoetsch
Copy link
Contributor

bastiandoetsch commented Dec 14, 2023

@slyszkowski , I don't think it makes sense to pass that argument, as the backend does not support it. But yeah, I understand now that you just want it for test, and not pass it to monitor.

@slyszkowski
Copy link

@bastiandoetsch what do you mean by backend here? Is there any way to make possible to make use of https://docs.snyk.io/snyk-cli/commands/test#json-file-output-less-than-output_file_path-greater-than within snyk orb? Basically what we need is having this being fixed ASAP. Without it we cannot integrate snyk with other tool we use.

Please note that this issue was as well raised via support portal: https://support.snyk.io/hc/en-us/requests/67831

@bastiandoetsch
Copy link
Contributor

@slyszkowski I'm aware of the support ticket. The command line parameter is not supported for snyk monitor though. This would need to be implemented not only in the CLI, but also the backend first and is a feature request, not a bug. For integrating, I would advise to use snyk test with json output and maybe run snyk monitor afterwards or in parallel.

@slyszkowski
Copy link

@bastiandoetsch we don't expect snyk monitor to generate json report. We need snyk test json report only. Since our pipeline is using snyk orb, not snyk CLI directly how can we separate snyk test & snyk monitor runs? Based on orb examples I see that only snyk/scan is possible which triggers both: snyk test & snyk monitor.

How then can we generate json report for snyk test using snyk orb?

@bastiandoetsch
Copy link
Contributor

You can just deactivate monitor with setting monitor-on-build to false. Then you can have a standalone snyk test run and add a second circleci step that runs without that parameter and monitors, if needed. You can find an example here: https://github.com/snyk/snyk-orb/blob/master/src/examples/with-params-app-scanning.yml on how to add the monitor-on-build parameter.

@jlam-bills
Copy link
Author

Hey there, thanks for the PR. Why would this help your case? Snyk monitor just does not support outputting to a file, so I'm unclear, why we should allow that parameter in the general config.

Hi @bastiandoetsch,
It has been a few months since I created the PR and I was trying to remember our use case. In our case, we don't set
monitor-on-build to true in the early stages of the dev cycle. Snyk has the "snyk-to-html" helper, https://github.com/snyk/snyk-to-html, which generates a html report that we can attach as artifact to the build. The generated html has a lot more details than reading the console output.

You can just deactivate monitor with setting monitor-on-build to false. Then you can have a standalone snyk test run and add a second circleci step that runs without that parameter and monitors, if needed. You can find an example here: https://github.com/snyk/snyk-orb/blob/master/src/examples/with-params-app-scanning.yml on how to add the monitor-on-build parameter.

I think snyk test will get executed twice and also add a second or more to the pipeline execute time.

@bastiandoetsch
Copy link
Contributor

@jlam-bills, if you would like to continue with this PR it would need tests to verify that it works.

@slyszkowski
Copy link

@bastiandoetsch same from our end, if we want to make use of snyk test json report then we will need to duplicate our pipeline execution which seem to be overhead. Would be wonderful to proceed with this PR @jlam-bills

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants