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

feat: create temp filepath for iac engine to write results [CFG-2114] #3640

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

ipapast
Copy link
Contributor

@ipapast ipapast commented Aug 30, 2022

What does this PR do?

This commit creates a temporary filepath and passes it through the -output flag to the snyk-iac-test executable.
The snyk-iac-test will then write all the results to this file, which the CLI will have to read and present to the user.

Notes:

  • the flag is not visible to the user, this will all happen in the background.
  • the file is created with empty content on the CLI side, and then gets overwritten by the snyk-iac-test with the results. This comment explains also reasoning about runnings scans in parallel: https://snyksec.atlassian.net/browse/CFG-2114?focusedCommentId=509517
  • the file is deleted when the results are read on the CLI side.
  • if errors happen during the file creation they will get caught on the CLI side too.
  • if errors happen during the file writing we will handle them on the snyk-iac-test side and return with exit code 1 to the CLI.

Use this in combination with https://github.com/snyk/snyk-iac-test/pull/106 to test.

How should this be manually tested?

  • Go to the branch of snyk-iac-test above and build locally: go build -o snyk-iac-test .
  • Create a bundle in opa-rules repository if you don't have one: make dist
  • Then run a scan as usual to see results:
    SNYK_IAC_POLICY_ENGINE_PATH=~/snyk-iac-test/snyk-iac-test SNYK_IAC_BUNDLE_PATH=~/opa-rules/dist.tar.gz snyk-dev iac test test/fixtures/iac/terraform/sg_open_ssh.tf --experimental


image

@github-actions
Copy link
Contributor

github-actions bot commented Aug 30, 2022

Warnings
⚠️

You've modified files in src/ directory, but haven't updated anything in test folder. Is there something that could be tested?

Generated by 🚫 dangerJS against 12d8e57

@ipapast ipapast force-pushed the feat/improve-big-results-handling-iac branch 2 times, most recently from a06c1e1 to 2d9b687 Compare August 30, 2022 17:02
@ipapast ipapast changed the title feat: create temp filepath for iac engine to write results feat: create temp filepath for iac engine to write results [CFG-2114] Aug 30, 2022
@ipapast ipapast force-pushed the feat/improve-big-results-handling-iac branch from 2d9b687 to b878436 Compare August 30, 2022 17:33
@ipapast ipapast marked this pull request as ready for review August 31, 2022 09:39
@ipapast ipapast requested a review from a team as a code owner August 31, 2022 09:39
@ipapast ipapast requested review from jaspervdj-snyk and francescomari and removed request for jaspervdj-snyk August 31, 2022 09:39
src/lib/iac/test/v2/scan/index.ts Show resolved Hide resolved
src/lib/iac/test/v2/scan/index.ts Outdated Show resolved Hide resolved
@ipapast ipapast force-pushed the feat/improve-big-results-handling-iac branch 2 times, most recently from 9dc5e8e to a65803e Compare September 2, 2022 10:47
@ipapast ipapast force-pushed the feat/improve-big-results-handling-iac branch from a65803e to 58f07f3 Compare September 2, 2022 15:42
This commit creates a temporary filepath and passes it through the -output flag to the snyk-iac-test executable.
The snyk-iac-test will then write all the results to this file, which the CLI will have to read and present to the user.
@ipapast ipapast force-pushed the feat/improve-big-results-handling-iac branch from 58f07f3 to 12d8e57 Compare September 2, 2022 15:52
@ipapast ipapast merged commit 1d55458 into master Sep 2, 2022
@ipapast ipapast deleted the feat/improve-big-results-handling-iac branch September 2, 2022 16:13
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.

2 participants