-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(graph): fix attributes checks logic #7264
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
Conversation
return False | ||
return None if has_unrendered_attribute else True | ||
if not found: | ||
return "not found" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be extracted to a const
return reduce(and_, args) | ||
|
||
def get_operation(self, vertex: Dict[str, Any]) -> Optional[bool]: | ||
def get_operation(self, vertex: Dict[str, Any]) -> Union[Optional[bool], str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that the return type changed like that, specifically if we want to return not found
we should use Literal["not found"]
to be more specific.
In addition - is it really needed? Can't we just return the value which will not change the check?
For example in and-solver
we can return True
and in or-solver
return False
, to make sure this is simpler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was my last resort, it's essential because otherwise it affects checks results, which is not what we want.
Updated the logic of the "and" and "or" condition solvers to correctly handle cases where required resource_types are not present in the evaluated tf file.
Previously, if a policy condition referenced a resource type that did not exist in the code under test, the evaluation would incorrectly result in a failed check. This behavior has now been fixed:
Conditions that reference missing resource types now return a special "not found" value.
The solvers treat "not found" as non-applicable, and the overall check ignores that return value.