feat: iac experimental terraform support #1654

rontalx · 2021-02-22T14:07:36Z

What does this PR do?

Adds Single Terraform file scan support to the: snyk iac test --experimental command.

Running Locally

In order to run locally, checkout the branch, build and run the local built version.
make sure to download the .iac-data from here and place it in your CWD root folder where you're running the CLI from.
e.g:

snyk iac test ./terraform/some_file.tf --experimental

TODO

Add relevant .wasm file for terraform regression tests.
Decide how to proceed with hcl-to-json lib and HCL2 syntax validations.
Double check with new wasm supporting multiple CSP from this PR: https://github.com/snyk/cloud-config-opa-policies/pull/21

github-actions · 2021-02-22T14:09:30Z

	Warnings
⚠️	Looks like you added a new Tap test. Consider making it a Jest test instead. See files like `test/*.spec.ts` for examples. Files found: `test/smoke/.iac-data/tf_data.json` `test/smoke/.iac-data/tf_policy.wasm`

	Messages
📖	You are modifying something in test/smoke directory, yet you are not on the branch starting with smoke/. You can prefix your branch with smoke/ and Smoke tests will trigger for this PR.

Generated by 🚫 dangerJS against b455497

aron

Looking great so far. Are you planning to add unit tests for this PR? I think we're at the point now where we'll want to consider them so it doesn't become a big task to add them later. Nor do we accidentally write the code in ways that make it harder to test in the future.

src/cli/commands/test/iac-local-execution/local-cache.ts

src/cli/commands/test/iac-local-execution/index.ts

aron · 2021-02-22T15:32:54Z

src/cli/commands/test/iac-local-execution/parsers.ts

+        docId,
+      };
+    } else {
+      throw new Error('Invalid K8s File!');


Would be really nice to include the missing fields here.

Opened a task for it:
https://snyksec.atlassian.net/browse/CC-700
Will include it as part of the beta-overhaul with the other tasks we opened.

aron · 2021-02-22T15:34:29Z

src/cli/commands/test/iac-local-execution/parsers.ts

+        return tryParsingKubernetesFile(fileContent, fileMetadata);
+      } catch (err) {
+        // TODO: if it's also not TF file then we should error
+        return [tryParsingTerraformFile(fileContent, fileMetadata)];


I think your comment covers this but this will mask the above error and look like a failed Terraform file right? Perhaps we should do this a) only if fileType === "json" and b) rethrow the original error if tryParsingTerraformFile also errors.

src/cli/commands/test/iac-local-execution/policy-engine.ts

src/cli/commands/test/iac-local-execution/types.ts

rontalx · 2021-02-22T17:11:31Z

@aron

re: Are you planning to add unit tests for this PR

I'm planning to add the unit-tests together with the dir support story (next one), as there are still lots changes until we reach that core-feature parity state in the experimental version.
I am taking that into account.

JackuB · 2021-02-23T10:13:00Z

package.json

@@ -77,6 +77,7 @@
    "configstore": "^5.0.1",
    "debug": "^4.1.1",
    "diff": "^4.0.1",
+    "hcl-to-json": "^0.1.1",


Decide how to proceed with hcl-to-json lib and HCL2 syntax validations.

https://github.com/gokmen/hcl-to-json
Seeing it's an archived and unmaintained project, is there an alternative?

you are correct, it was an informed decision to start with it as a temporary "unblocker" solution.
as generally there is no good HCL-2-JSON parser npm package out there.

we are aware of it's limitations and it was an informed decision to start with it taking into account it's a "good-enough" approach for the dog-fooding internal releases.

For the public beta and long-term solution, we will probably do one of the following:

generate a Golang HCL to JSON parser into WASM and use it.

OR

include HCL to JSON parsing inside our Terraform scanning wasm file, so you can just provide it with HCL.

JackuB · 2021-02-23T10:29:17Z

test/smoke/spec/iac/snyk_test_local_exec_spec.sh

@@ -48,4 +48,48 @@ Describe "Snyk iac test --experimental command"
      The result of function check_valid_json should be success
    End
  End
+
+  Describe "terraform single file scan"


Would you run this only in the Regression test and not in the smoke test? I don't think we need hourly checks of this.

With this after the Describe like above:

Skip if "execute only in regression test" check_if_regression_test

yes will add it as a regression 👍

aron

👍 Looks good, nice work. Looks like we need to add the new WebAssembly fixtures for the tests to be happy.

JackuB · 2021-02-24T18:42:44Z

src/cli/commands/test/iac-local-execution/parsers.ts

+    case 'tf':
+      return [tryParsingTerraformFile(fileContent, fileMetadata)];
+    default:
+      throw new Error('Invalid IaC file');


Could have a more descriptive message e.g.

Suggested change

throw new Error('Invalid IaC file');

throw new Error(`Unsupported filetype ${fileMetadata.fileType }`);

going to do a bit of an overhaul to the error handling in the next pr for Dir-Support, will take care of it over there.
tnx 👍

github-actions · 2021-02-28T09:00:57Z

Expected release notes (by @rontalx)

features:
iac experimental tf support (b455497)

others (will not be included in Semantic-Release notes):
remove tests previously migrated to jest (02c99b8)
set timeout in beforeAll (e203fd1)
add longer timeoutsfor flaky tests (bc9c0f2)

I hereby acknowledge these release notes are 🥙 AWESOME 🥙

rontalx force-pushed the feat/iac-experimental-terraform-support branch from 37989df to c2133f5 Compare February 22, 2021 14:08

rontalx force-pushed the feat/iac-experimental-terraform-support branch 4 times, most recently from 18c419d to 5c7c74e Compare February 22, 2021 15:29

aron reviewed Feb 22, 2021

View reviewed changes

rontalx force-pushed the feat/iac-experimental-terraform-support branch 4 times, most recently from b3502c4 to f1cc1a9 Compare February 22, 2021 18:14

rontalx marked this pull request as ready for review February 22, 2021 18:14

rontalx requested a review from a team as a code owner February 22, 2021 18:14

rontalx requested a review from a team February 22, 2021 18:14

rontalx requested a review from a team as a code owner February 22, 2021 18:14

dtrunley-snyk approved these changes Feb 22, 2021

View reviewed changes

rontalx changed the title ~~feat: iac experimental tf support~~ feat: iac experimental terraform support Feb 23, 2021

JackuB reviewed Feb 23, 2021

View reviewed changes

aron approved these changes Feb 23, 2021

View reviewed changes

rontalx force-pushed the feat/iac-experimental-terraform-support branch 6 times, most recently from b3c455c to 5a92c16 Compare February 24, 2021 18:28

JackuB approved these changes Feb 24, 2021

View reviewed changes

rontalx force-pushed the feat/iac-experimental-terraform-support branch 2 times, most recently from f4da83d to d9abb7f Compare February 25, 2021 14:46

rontalx force-pushed the feat/iac-experimental-terraform-support branch from d9abb7f to ae474a2 Compare February 25, 2021 14:47

feat: iac experimental tf support

b455497

rontalx force-pushed the feat/iac-experimental-terraform-support branch from ae474a2 to b455497 Compare February 28, 2021 09:00

rontalx merged commit a988600 into master Feb 28, 2021

rontalx deleted the feat/iac-experimental-terraform-support branch February 28, 2021 09:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: iac experimental terraform support #1654

feat: iac experimental terraform support #1654

rontalx commented Feb 22, 2021 •

edited

github-actions bot commented Feb 22, 2021 •

edited

aron left a comment

aron Feb 22, 2021

rontalx Feb 22, 2021

aron Feb 22, 2021

rontalx commented Feb 22, 2021

JackuB Feb 23, 2021

rontalx Feb 23, 2021 •

edited

JackuB Feb 23, 2021

rontalx Feb 23, 2021

aron left a comment •

edited

JackuB Feb 24, 2021

rontalx Feb 24, 2021

github-actions bot commented Feb 28, 2021 •

edited by rontalx

	throw new Error('Invalid IaC file');
	throw new Error(`Unsupported filetype ${fileMetadata.fileType }`);

feat: iac experimental terraform support #1654

feat: iac experimental terraform support #1654

Conversation

rontalx commented Feb 22, 2021 • edited

What does this PR do?

Running Locally

github-actions bot commented Feb 22, 2021 • edited

aron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rontalx commented Feb 22, 2021

Choose a reason for hiding this comment

rontalx Feb 23, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aron left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 28, 2021 • edited by rontalx

rontalx commented Feb 22, 2021 •

edited

github-actions bot commented Feb 22, 2021 •

edited

rontalx Feb 23, 2021 •

edited

aron left a comment •

edited

github-actions bot commented Feb 28, 2021 •

edited by rontalx