bug_fix (terraform_plan): enhance evaluation logic and add new test fi…

[AkashS0510]

…xtures

Pull Request Template

Description

With these changes, when your policy checks for tag 'a':

The bucket with the tag returns a value, which passes the evaluation
The bucket without the tag returns None, which fails the evaluation
Since one resource fails, the overall policy correctly fails
This ensures that all resources must meet the policy requirements for the policy to pass, which is the expected behavior for policy enforcement.

• What changes are being made?

  • Provider Implementation Issue: The Terraform plan provider was only returning resources that had the requested attribute (tag 'a'), and ignoring resources that didn't have that attribute. This meant the evaluation only saw the S3 bucket with the tag, not the one without it.

• Evaluation Logic Issue: The core evaluation logic was considering a policy as passed if any of the resources passed, instead of requiring all resources to pass

• Why are these changes necessary?

  • Updated the Provider Logic: Modified the Terraform plan provider to return all resources of the specified type, even if they don't have the requested attribute. Resources without the requested attribute now return None as the value, which ensures they get evaluated.

• Fixed the Evaluation Logic: Enhanced the core evaluation logic to properly handle multiple resources. Now, the policy will only pass if all resources pass the evaluation, and will fail if any resource fails.

• Which issues or tickets does this PR close or relate to?

  • Related issue: #

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Other (please specify):

Checklist

• Code has been linted.
• Documentation has been updated (if needed).
• Tests have been added or updated (if needed).
• Any breaking changes have been communicated to the team.

Screenshots or Recordings (if applicable)

Additional Information

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines	Coverage Δ
src/tirith/core/core.py	83.14% <100.00%> (+6.13%)	⬆️
src/tirith/providers/terraform_plan/handler.py	60.08% <100.00%> (+1.43%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.

Files not reviewed (2)

• tests/providers/terraform_plan/fixtures/input_multiple_rescource_tag_check.json: Language not supported
• tests/providers/terraform_plan/fixtures/policy_multiple_rescource_tag_check.json: Language not supported

Comments suppressed due to low confidence (1)

src/tirith/core/core.py:93

• The logic for handling skipped evaluations appears ambiguous. If no valid evaluations are processed, ensure that setting 'has_evaluation_passed' to None accurately reflects the desired overall evaluation result.

if not has_valid_evaluation and has_evaluation_passed is None:

[refeed]

Looks good to me, btw how much time do you need to cover the missing tests?

[AkashS0510]

Looks good to me, btw how much time do you need to cover the missing tests?

I think we can do it in a separate PR. The tests for the current changes have been added. We need to add coverage tests only for our existing core implentation

[refeed]

I think we can do it in a separate PR. The tests for the current changes have been added. We need to add coverage tests only for our existing core implentation

I think we can do it in this PR unless it takes a lot of time. Only for what commented by @codecov, the code that we touch in this PR, not all of the existing core implementation.

[refeed]

LGTM, thanks!

[Copilot]

Pull Request Overview

This PR fixes a bug in the Terraform plan provider and updates the core evaluation logic so that policies correctly fail when any resource fails the tag check. It also adds new tests to verify both failing and passing scenarios for multiple resource evaluations.

• Updated the provider to return a None value when a resource is missing the requested tag.
• Enhanced the evaluator to require all resources to pass for overall success.
• Added tests for multiple resource evaluations and for provider error scenarios.

Reviewed Changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated no comments.

File	Description
tests/providers/terraform_plan/test_dot_star_attr.py	Added tests to validate tag-check behavior across multiple resources; note one test function is not descriptively named.
tests/core/test_core.py	Introduced new tests for generating evaluator results, including handling provider errors and multiple resource results.
src/tirith/providers/terraform_plan/handler.py	Updated logic to include resources missing the attribute by appending a None value for evaluation.
src/tirith/core/core.py	Refactored generate_evaluator_result to fail evaluation when no inputs are found and to track valid evaluations.

Files not reviewed (2)

• tests/providers/terraform_plan/fixtures/input_multiple_resource_tag_check.json: Language not supported
• tests/providers/terraform_plan/fixtures/policy_multiple_resource_tag_check.json: Language not supported

Comments suppressed due to low confidence (1)

tests/providers/terraform_plan/test_dot_star_attr.py:59

• The test function name 'test_' is not descriptive of its purpose; consider renaming it to reflect what is being validated.

def test_(split_expressions, input_data, expected_result):

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud