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

fix: tf plan full scan #1891

Merged
merged 1 commit into from May 6, 2021
Merged

fix: tf plan full scan #1891

merged 1 commit into from May 6, 2021

Conversation

rontalx
Copy link
Contributor

@rontalx rontalx commented May 6, 2021

What does this PR do?

This PR fixes a bug we had with scanning the full state in a given terraform plan output.
It does it by simplifying the logic and remove extraction logic we don't need any more.
The new mechanism iterates on resource_changes for both full & delta scan, but the only difference is that for full scans it also scans resource which had no-op action on them, resulting in a full scan of the infrastructure state.
This PR also adds a more comprehensive regression test-suite to all various use-cases of scan modes and different terraform plan use-cases (create, delete, update, no-op)

Where should the reviewer start?

See the changes to the terraform parser.
Then see the new regression tests added to cover major use-cases.
https://snyksec.atlassian.net/browse/CC-840

@rontalx rontalx marked this pull request as ready for review May 6, 2021 09:36
@rontalx rontalx requested a review from a team May 6, 2021 09:36
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2021

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/fixtures/iac/terraform-plan/expected-parser-results/delta-scan/tf-plan-create.resources.json
  • test/fixtures/iac/terraform-plan/expected-parser-results/delta-scan/tf-plan-destroy.resources.json
  • test/fixtures/iac/terraform-plan/expected-parser-results/delta-scan/tf-plan-no-op.resources.json
  • test/fixtures/iac/terraform-plan/expected-parser-results/delta-scan/tf-plan-update.resources.json
  • test/fixtures/iac/terraform-plan/expected-parser-results/full-scan/tf-plan-create.resources.json
  • test/fixtures/iac/terraform-plan/expected-parser-results/full-scan/tf-plan-destroy.resources.json
  • test/fixtures/iac/terraform-plan/expected-parser-results/full-scan/tf-plan-no-op.resources.json
  • test/fixtures/iac/terraform-plan/expected-parser-results/full-scan/tf-plan-update.resources.json
  • test/fixtures/iac/terraform-plan/tf-plan-create.json
  • test/fixtures/iac/terraform-plan/tf-plan-destroy.json
  • test/fixtures/iac/terraform-plan/tf-plan-no-op.json
  • test/fixtures/iac/terraform-plan/tf-plan-update.json
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 ac6208d

@rontalx rontalx force-pushed the fix/terraform-plan-full-scan branch from bc28500 to 36b1450 Compare May 6, 2021 09:46
@rontalx rontalx requested a review from aron May 6, 2021 11:13
@rontalx rontalx force-pushed the fix/terraform-plan-full-scan branch from 36b1450 to ac6208d Compare May 6, 2021 12:01
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2021

Expected release notes (by @rontalx)

fixes:
tf plan full scan (ac6208d)

others (will not be included in Semantic-Release notes):
iac documentation url in outputs (be34e5c)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

@rontalx rontalx merged commit 1d205b7 into master May 6, 2021
@rontalx rontalx deleted the fix/terraform-plan-full-scan branch May 6, 2021 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants