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: Add '--var-file' flag to iac test for loading external TF variable definition files [CFG-1663] #3116

Merged
merged 3 commits into from
Apr 11, 2022

Conversation

ipapast
Copy link
Contributor

@ipapast ipapast commented Apr 8, 2022

What does this PR do?

Adds a --var-file flag to iac test for loading external Terraform variable definition files. See commit messages below for details.

Where should the reviewer start?

Follow the commits in order:

  1. chore: add 'var-file' flag option to iac test command

This commit adds the 'var-file' flag as an available command under 'iac test'. CFG-1663

  1. chore: Throw errors if 'var-file' flag is misused.

This commit adds checks around the 'var-file' flag usage: - If the user does not have the 'iacTerraformVarSupport' feature flag, we will throw an error. - If the user provides a non-existent path, we will throw an error. CFG-1663

  1. feat: Load TF variable definitions files via --var-file

This commit adds the ability to load an external file by using 'var-file' flag:

  • The user can point to a single filepath, relative to the 'pathToScan'
  • The filepath will be checked for existence, and any vars will be de-referenced into the context of the 'pathToScan' directory only.

How should this be manually tested?

assign yourself the iacTerraformVarSupport flag.

  1. Run: snyk-dev iac test test/fixtures/iac/terraform/var_deref/nested_var_deref and check the issues.
  2. Run snyk-dev iac test test/fixtures/iac/terraform/var_deref/nested_var_deref --var-file=test/fixtures/iac/terraform/vars.tf and check that the sg_open_ssh.tf has one issue more that is found due to the allow_ssh_external_var_file var.

"Bad case" scenarios:

  1. un-assign yourself the iacTerraformVarSupport flag and try to run scan (2) from above
  2. assign yourself the iacTerraformVarSupport flag and point to a non-existing path or directory and see the relevant error
  3. point to an invalid hcl (or other format) file and see the "failed to parse error" for that file.

I also ran checks with other flags, such as detection-depth, sarif, json.

What are the relevant tickets?

CFG-1663

Screenshots

Without the flag:

image

With the use of the flag (and finding an issue extra for the test/fixtures/iac/terraform/var_deref/nested_var_deref/sg_open_ssh.tf, when sharing the variable context of the external definitions file):

image

Ilianna Papastefanou added 2 commits April 8, 2022 11:11
This commit adds the 'var-file' flag as an available command under 'iac test'.

CFG-1663
This commit adds checks around the 'var-file' flag usage:
- If the user does not have the 'iacTerraformVarSupport' feature flag, we will throw an error.
- If the user provides a non-existent path, we will throw an error.

CFG-1663
@ipapast ipapast force-pushed the feat/add-var-file-flag-iac-test-all branch 3 times, most recently from 0233696 to d1ceac9 Compare April 8, 2022 17:27
@ipapast ipapast marked this pull request as ready for review April 8, 2022 17:30
@ipapast ipapast requested review from a team as code owners April 8, 2022 17:30
Copy link
Contributor

@ofekatr ofekatr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spectacular work @ipapast,
There are a few nit comment with suggestions and questions, but none are blocking.
Feel free not to reply to suggestions.
LGTM 💯 :shipit:

This commit adds the ability to load an external file by using 'var-file' flag:
- The user can point to a single filepath, relative to the 'pathToScan'
- The filepath will be checked for existence, and any vars will be de-referenced into the context of the 'pathToScan' directory.

CFG-1663
@ipapast ipapast force-pushed the feat/add-var-file-flag-iac-test-all branch from d1ceac9 to 141b6c0 Compare April 11, 2022 10:10
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