From b0d3a242ec97e5ea882e21830b75b9d168d8230e Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Thu, 18 Jul 2024 09:48:30 -0700 Subject: [PATCH] Update condition implies fn --- src/cfnlint/conditions/conditions.py | 20 +++++------ src/cfnlint/template/template.py | 2 +- test/fixtures/templates/good/generic.yaml.dot | 0 .../jsonschema/test_cfn_lint_relationship.py | 33 +++++++++---------- 4 files changed, 24 insertions(+), 31 deletions(-) delete mode 100644 test/fixtures/templates/good/generic.yaml.dot diff --git a/src/cfnlint/conditions/conditions.py b/src/cfnlint/conditions/conditions.py index 6afef36441..7dfe25f138 100644 --- a/src/cfnlint/conditions/conditions.py +++ b/src/cfnlint/conditions/conditions.py @@ -277,20 +277,16 @@ def implies(self, scenarios: dict[str, bool], implies: str) -> bool: and_condition = And(*conditions) - # if the implies condition has to be true already then we don't - # need to imply it - if not scenarios.get(implies): - implies_condition = self._conditions[implies].build_true_cnf( - self._solver_params - ) - cnf.add_prop(Not(Implies(and_condition, implies_condition))) - else: - cnf.add_prop(and_condition) + implies_condition = self._conditions[implies].build_true_cnf( + self._solver_params + ) + cnf.add_prop(Not(Implies(and_condition, implies_condition))) - if satisfiable(cnf): - return True + results = satisfiable(cnf) + if results: + return False - return False + return True except KeyError: # KeyError is because the listed condition doesn't exist because of bad # formatting or just the wrong condition name diff --git a/src/cfnlint/template/template.py b/src/cfnlint/template/template.py index 138a87cddb..3fa1a73433 100644 --- a/src/cfnlint/template/template.py +++ b/src/cfnlint/template/template.py @@ -824,7 +824,7 @@ def is_resource_available(self, path: Path, resource: str) -> list[dict[str, boo return results scenario[condition_name] = list(condition_bool)[0] - if self.conditions.implies(scenario, resource_condition): + if not self.conditions.implies(scenario, resource_condition): return [{**{resource_condition: False}, **scenario}] # if resource condition isn't available then the resource is available diff --git a/test/fixtures/templates/good/generic.yaml.dot b/test/fixtures/templates/good/generic.yaml.dot deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/test/unit/rules/jsonschema/test_cfn_lint_relationship.py b/test/unit/rules/jsonschema/test_cfn_lint_relationship.py index fcbda4cf6d..6606bbf10d 100644 --- a/test/unit/rules/jsonschema/test_cfn_lint_relationship.py +++ b/test/unit/rules/jsonschema/test_cfn_lint_relationship.py @@ -66,11 +66,11 @@ def template(): }, "Three": { "Type": "AWS::EC2::Instance", - "Condition": "IsUsEast1", + "Condition": "IsImageIdSpecified", "Properties": { "ImageId": { "Fn::If": [ - "IsImageIdSpecified", + "IsUsEast1", {"Ref": "ImageId"}, {"Ref": "AWS::NoValue"}, ], @@ -102,7 +102,6 @@ def template(): }, "Five": { "Type": "AWS::EC2::Instance", - "Condition": "IsUsEast1", "Properties": { "ImageId": { "Fn::If": [ @@ -114,17 +113,14 @@ def template(): }, "ParentFive": { "Type": "AWS::EC2::Instance", - "Condition": "IsImageIdSpecified", "Properties": {"ImageId": {"Fn::GetAtt": ["Five", "ImageId"]}}, }, "Six": { "Type": "AWS::EC2::Instance", - "Condition": "IsUsEast1", "Properties": "Foo", }, "ParentSix": { "Type": "AWS::EC2::Instance", - "Condition": "IsImageIdSpecified", "Properties": {"ImageId": {"Fn::GetAtt": ["Six", "ImageId"]}}, }, "Seven": { @@ -168,7 +164,7 @@ def template(): "name,path,status,expected", [ ( - "One", + "Condition for value", deque(["Resources", "ParentOne", "Properties", "ImageId"]), {}, [ @@ -177,48 +173,49 @@ def template(): ], ), ( - "Two", + "Standard", deque(["Resources", "ParentTwo", "Properties", "ImageId"]), {}, [({"Ref": "ImageId"}, {})], ), ( - "Three", + "Lots of conditions along the way", deque(["Resources", "ParentThree", "Properties", "ImageId"]), { "IsImageIdSpecified": True, }, - [({"Ref": "ImageId"}, {"IsUsEast1": True, "IsImageIdSpecified": True})], + [ + ({"Ref": "ImageId"}, {"IsUsEast1": True, "IsImageIdSpecified": True}), + (None, {"IsUsEast1": False, "IsImageIdSpecified": True}), + ], ), ( - "Four", + "Unrelated conditions", deque(["Resources", "ParentFour", "Properties", "ImageId"]), { "IsImageIdSpecified": True, }, - [ - ({"Ref": "ImageId"}, {"IsUsEast1": True, "IsImageIdSpecified": True}), - ], + [], ), ( - "Five", + "Destiniation Fn::If isn't properly formatted", deque(["Resources", "ParentFive", "Properties", "ImageId"]), {}, [], ), ( - "Six", + "Properties isn't an object", deque(["Resources", "ParentSix", "Properties", "ImageId"]), {}, [], ), ( - "Seven", + "Condition's don't align", deque(["Resources", "ParentSeven", "Properties", "ImageId"]), { "IsUsEast1": True, }, - [()], + [], ), ( "Short Path",