Skip to content

wip fix(terraform): evaluate inline checks for looped modules#6793

Closed
Alex-Waring wants to merge 4 commits intobridgecrewio:mainfrom
Alex-Waring:AWar/inline_checks/fix_bug
Closed

wip fix(terraform): evaluate inline checks for looped modules#6793
Alex-Waring wants to merge 4 commits intobridgecrewio:mainfrom
Alex-Waring:AWar/inline_checks/fix_bug

Conversation

@Alex-Waring
Copy link
Copy Markdown
Contributor

@Alex-Waring Alex-Waring commented Oct 24, 2024

User description

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Description

The inline comment checker for enriched plan checking, currently works by looking up exceptions using the name of the module, in form module.module_name, even when there is a count or for_each set on the resource.

When we then loop through the resources in the plan to look up their enrichments, this lookup fails as we're using module.module_name[index]. This PR proves that this is the case by introducing a new test that fails, and then fixes that test.

Fixes #6113

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my feature, policy, or fix is effective and works
  • New and existing tests pass locally with my changes

Generated description

Dear maintainer, below is a concise technical summary of the changes proposed in this PR:

Fix the inline comment checker for enriched plan checking by addressing the issue with module name lookups when using count or for_each in Terraform modules. The Report class in checkov/common/output/report.py is updated to correctly handle module paths by ignoring indices in module names. New tests are added in test_runner_registry_plan_enrichment.py to validate the fix, ensuring that checks are correctly skipped for looped modules. The test setup includes Terraform configuration files and a plan JSON file to simulate the scenario.

TopicDetails
Inline Checker Fix Fix the inline comment checker for enriched plan checking by addressing the issue with module name lookups when using count or for_each in Terraform modules.
Modified files (1)
  • checkov/common/output/report.py
Latest Contributors(2)
EmailCommitDate
146818014+sourava01@us...feat-github-add-summar...May 05, 2024
AdamVarsan@gmail.comfix-graph-remove-SCA-r...February 11, 2024
Test Enhancements Add tests to validate the fix for module name lookups in looped modules, ensuring checks are correctly skipped.
Modified files (4)
  • tests/common/runner_registry/test_runner_registry_plan_enrichment.py
  • tests/common/runner_registry/plan_with_looped_module/tf/tfplan.json
  • tests/common/runner_registry/plan_with_looped_module/tf/main.tf
  • tests/common/runner_registry/plan_with_looped_module/mod_ref/main.tf
Latest Contributors(2)
EmailCommitDate
35402131+bo156@users.n...feat-terraform-Remove-...August 16, 2023
63583491+arielkru@user...feat-terraform-Impleme...December 01, 2022
This pull request is reviewed by Baz. Join @Alex-Waring and the rest of your team on (Baz).

@Alex-Waring Alex-Waring changed the title fix(terraform): evaluate inline checks for looped modules wip fix(terraform): evaluate inline checks for looped modules Oct 25, 2024
@cristian-rincon
Copy link
Copy Markdown

cristian-rincon commented Nov 13, 2024

Hi 👋 @Alex-Waring , do you have any workaround while we are waiting for your PR to be merged?

@Alex-Waring
Copy link
Copy Markdown
Contributor Author

Hi @cristian-rincon, no there's no workaround. This is only WIP because it's just a partial fix, I can always come back if this gets merged.

If you have any way of getting this looked at feel free, it passes all tests so can be merged

@adovy
Copy link
Copy Markdown

adovy commented Apr 15, 2025

can this be merged? it is really a big blocker

@MaryArmaly
Copy link
Copy Markdown
Contributor

Hey @Alex-Waring,
Could you please merge the main branch into your branch and fix the failing tests? Thanks for contributing!

@Alex-Waring
Copy link
Copy Markdown
Contributor Author

Hi @MaryArmaly , this PR needs a fair amount of work to fix and I do not have the capacity to do so at the moment. If someone from PaloAlto wants to pick this up they are welcome to.

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.

terraform_plan - modules - for_each / count issues

7 participants