Update condition implies fn

aws-cloudformation · Jul 18, 2024 · b0d3a24 · b0d3a24
1 parent bfad237
commit b0d3a24
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 31 deletions.
diff --git 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
@@ -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
diff --git 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",