Skip to content

Commit 5bcac03

Browse files
AkashS0510refeed
andauthored
fix(terraform_plan): When one resource doesn't have an attribute, it should fail (#240)
Previously, when the policy wants to make sure all of the resources have an attribute Tag, but, in the input, when one resource has the tag and the rest are not, Tirith still mark the input as passing. This commit makes it so that when one resource doesn't have a tag, even though the rest has the tag. It will fail the policy. Co-authored-by: Rafid Aslam <[email protected]>
1 parent 8e5982a commit 5bcac03

File tree

6 files changed

+474
-25
lines changed

6 files changed

+474
-25
lines changed

src/tirith/core/core.py

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -56,30 +56,43 @@ def generate_evaluator_result(evaluator_obj, input_data, provider_module):
5656
evaluation_results = []
5757
has_evaluation_passed = True
5858

59-
for evaluator_input in evaluator_inputs:
60-
if isinstance(evaluator_input["value"], ProviderError) and evaluator_input.get("err", None):
61-
severity_value = evaluator_input["value"].severity_value
62-
err_result = dict(message=evaluator_input["err"])
63-
64-
if severity_value > evaluator_error_tolerance:
65-
err_result.update(dict(passed=False))
59+
# If there are no evaluator inputs, it means the provider didn't find any resources
60+
# In this case, the evaluation should fail
61+
if not evaluator_inputs:
62+
has_evaluation_passed = False
63+
evaluation_results = [{"passed": False, "message": "Could not find input value"}]
64+
else:
65+
# Track if we've had at least one valid evaluation (not skipped)
66+
has_valid_evaluation = False
67+
68+
for evaluator_input in evaluator_inputs:
69+
if isinstance(evaluator_input["value"], ProviderError) and evaluator_input.get("err", None):
70+
severity_value = evaluator_input["value"].severity_value
71+
err_result = dict(message=evaluator_input["err"])
72+
73+
if severity_value > evaluator_error_tolerance:
74+
err_result.update(dict(passed=False))
75+
evaluation_results.append(err_result)
76+
has_evaluation_passed = False
77+
continue
78+
# Mark as skipped evaluation
79+
err_result.update(dict(passed=None))
6680
evaluation_results.append(err_result)
67-
has_evaluation_passed = False
81+
has_evaluation_passed = None
6882
continue
69-
# Mark as skipped evaluation
70-
err_result.update(dict(passed=None))
71-
evaluation_results.append(err_result)
83+
84+
evaluation_result = evaluator_instance.evaluate(evaluator_input["value"], evaluator_data)
85+
evaluation_result["meta"] = evaluator_input.get("meta")
86+
evaluation_results.append(evaluation_result)
87+
has_valid_evaluation = True
88+
89+
if not evaluation_result["passed"]:
90+
has_evaluation_passed = False
91+
92+
# If all evaluations were skipped, we need to make sure the overall result is 'None'
93+
if not has_valid_evaluation and has_evaluation_passed is None:
7294
has_evaluation_passed = None
73-
continue
74-
75-
evaluation_result = evaluator_instance.evaluate(evaluator_input["value"], evaluator_data)
76-
evaluation_result["meta"] = evaluator_input.get("meta")
77-
evaluation_results.append(evaluation_result)
78-
if not evaluation_result["passed"]:
79-
has_evaluation_passed = False
80-
if not evaluation_results:
81-
has_evaluation_passed = False
82-
evaluation_results = [{"passed": False, "message": "Could not find input value"}]
95+
8396
result["result"] = evaluation_results
8497
result["passed"] = has_evaluation_passed
8598
return result

src/tirith/providers/terraform_plan/handler.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ def provide(provider_inputs, input_data):
5050
input_resource_change_attrs = {}
5151
input_type = provider_inputs["operation_type"]
5252
resource_changes = input_data.get("resource_changes")
53+
5354
if not resource_changes:
5455
outputs.append(
5556
{
@@ -72,12 +73,17 @@ def provide(provider_inputs, input_data):
7273
if resource_type in (resource_change["type"], "*"):
7374
is_resource_found = True
7475
input_resource_change_attrs = resource_change["change"]["after"]
76+
# [local_is_found_attribute] (local scope)
77+
# Used to decide whether to append a None value for each specific resource that's missing the attribute
7578
if input_resource_change_attrs:
79+
local_is_found_attribute = False
7680
if attribute in input_resource_change_attrs:
7781
is_attribute_found = True
82+
local_is_found_attribute = True
83+
attribute_value = input_resource_change_attrs[attribute]
7884
outputs.append(
7985
{
80-
"value": input_resource_change_attrs[attribute],
86+
"value": attribute_value,
8187
"meta": resource_change,
8288
"err": None,
8389
}
@@ -86,15 +92,21 @@ def provide(provider_inputs, input_data):
8692
evaluated_outputs = _wrapper_get_exp_attribute(attribute, input_resource_change_attrs)
8793
if evaluated_outputs:
8894
is_attribute_found = True
89-
for evaluated_output in evaluated_outputs:
90-
outputs.append({"value": evaluated_output, "meta": resource_change, "err": None})
95+
local_is_found_attribute = True
96+
for evaluated_output in evaluated_outputs:
97+
outputs.append({"value": evaluated_output, "meta": resource_change, "err": None})
98+
99+
# If we didn't find the attribute in this resource, add a None value so it still gets evaluated
100+
if not local_is_found_attribute:
101+
outputs.append({"value": None, "meta": resource_change, "err": None})
91102
else:
92103
outputs.append(
93104
{
94105
"value": ProviderError(severity_value=0),
95106
"err": f"No Terraform changes found for resource type: '{resource_type}'",
96107
}
97108
)
109+
98110
if not outputs:
99111
if not is_resource_found:
100112
outputs.append(

tests/core/test_core.py

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
# Maintain all core related tests here
22
from pytest import mark
3+
import pytest
34

4-
from tirith.core.core import final_evaluator
5+
from tirith.core.core import final_evaluator, generate_evaluator_result
6+
from tirith.providers.common import ProviderError
7+
from unittest.mock import patch, MagicMock
58

69

710
@mark.passing
@@ -38,3 +41,113 @@ def test_final_evaluator_malicious_eval_should_err():
3841
"!skipped_check && passing_check || [].__class__.__base__", dict(skipped_check=None, passing_check=True)
3942
)
4043
assert actual_result == (False, ["The following symbols are not allowed: __class__, __base__"])
44+
45+
46+
class MockEvaluator:
47+
def evaluate(self, input_value, data):
48+
if input_value == "resource1":
49+
return {"passed": True, "message": "First resource passed"}
50+
else:
51+
return {"passed": False, "message": "Second resource failed"}
52+
53+
54+
@mark.passing
55+
def test_generate_evaluator_result_empty_inputs():
56+
"""Test that when a provider returns no inputs, the evaluation should fail."""
57+
# Mock evaluator object
58+
evaluator_obj = {
59+
"id": "test_evaluator",
60+
"provider_args": {"operation_type": "attribute", "key": "value"},
61+
"condition": {"type": "Equals", "value": True},
62+
}
63+
64+
# Mock the provider function to return empty list
65+
with patch("tirith.core.core.get_evaluator_inputs_from_provider_inputs", return_value=[]):
66+
result = generate_evaluator_result(evaluator_obj, {}, "test_provider")
67+
68+
# Verify the result shows a failed evaluation with the correct message
69+
assert result["passed"] is False
70+
assert len(result["result"]) == 1
71+
assert result["result"][0]["passed"] is False
72+
assert result["result"][0]["message"] == "Could not find input value"
73+
74+
75+
@mark.passing
76+
def test_generate_evaluator_result_provider_error_above_tolerance():
77+
"""Test that provider errors with severity higher than tolerance cause the evaluation to fail."""
78+
# Mock evaluator object with error_tolerance = 1
79+
evaluator_obj = {
80+
"id": "test_evaluator",
81+
"provider_args": {"operation_type": "attribute", "key": "value"},
82+
"condition": {"type": "Equals", "value": True, "error_tolerance": 1},
83+
}
84+
85+
# Create a provider error with severity 2 (above tolerance)
86+
provider_error = {"value": ProviderError(severity_value=2), "err": "Resource not found"}
87+
88+
# Mock the provider function to return the error
89+
with patch("tirith.core.core.get_evaluator_inputs_from_provider_inputs", return_value=[provider_error]):
90+
# Create a mapping for EVALUATORS_DICT.get to return a mock evaluator class
91+
mock_evaluator_dict = {"Equals": MockEvaluator}
92+
with patch("tirith.core.core.EVALUATORS_DICT", mock_evaluator_dict):
93+
result = generate_evaluator_result(evaluator_obj, {}, "test_provider")
94+
95+
# Verify the result shows a failed evaluation
96+
assert result["passed"] is False
97+
assert len(result["result"]) == 1
98+
assert result["result"][0]["passed"] is False
99+
assert result["result"][0]["message"] == "Resource not found"
100+
101+
102+
@mark.passing
103+
def test_generate_evaluator_result_provider_error_within_tolerance():
104+
"""Test that provider errors with severity within tolerance are skipped."""
105+
# Mock evaluator object with error_tolerance = 2
106+
evaluator_obj = {
107+
"id": "test_evaluator",
108+
"provider_args": {"operation_type": "attribute", "key": "value"},
109+
"condition": {"type": "Equals", "value": True, "error_tolerance": 2},
110+
}
111+
112+
# Create a provider error with severity 1 (within tolerance)
113+
provider_error = {"value": ProviderError(severity_value=1), "err": "Minor issue"}
114+
115+
# Mock the provider function to return the error
116+
with patch("tirith.core.core.get_evaluator_inputs_from_provider_inputs", return_value=[provider_error]):
117+
# Create a mapping for EVALUATORS_DICT.get to return a mock evaluator class
118+
mock_evaluator_dict = {"Equals": MockEvaluator}
119+
with patch("tirith.core.core.EVALUATORS_DICT", mock_evaluator_dict):
120+
result = generate_evaluator_result(evaluator_obj, {}, "test_provider")
121+
122+
# Verify the result shows a skipped evaluation
123+
assert result["passed"] is None
124+
assert len(result["result"]) == 1
125+
assert result["result"][0]["passed"] is None
126+
assert result["result"][0]["message"] == "Minor issue"
127+
128+
129+
@mark.passing
130+
def test_generate_evaluator_result_multiple_resources_one_failing():
131+
"""Test that when one resource fails, the entire evaluation fails."""
132+
# Mock evaluator object
133+
evaluator_obj = {
134+
"id": "test_evaluator",
135+
"provider_args": {"operation_type": "attribute", "key": "value"},
136+
"condition": {"type": "Equals", "value": "expected_value"},
137+
}
138+
139+
# Mock provider to return two resources
140+
with patch(
141+
"tirith.core.core.get_evaluator_inputs_from_provider_inputs",
142+
return_value=[{"value": "resource1"}, {"value": "resource2"}],
143+
):
144+
# Create a mapping for EVALUATORS_DICT.get to return our mock evaluator class
145+
mock_evaluator_dict = {"Equals": MockEvaluator}
146+
with patch("tirith.core.core.EVALUATORS_DICT", mock_evaluator_dict):
147+
result = generate_evaluator_result(evaluator_obj, {}, "test_provider")
148+
149+
# Verify the result shows a failed evaluation even though one resource passed
150+
assert result["passed"] is False
151+
assert len(result["result"]) == 2
152+
assert result["result"][0]["passed"] is True
153+
assert result["result"][1]["passed"] is False

0 commit comments

Comments
 (0)