Skip to content

Commit

Permalink
Update condition implies fn
Browse files Browse the repository at this point in the history
  • Loading branch information
kddejong committed Jul 18, 2024
1 parent ad60e9a commit 1c2e463
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 85 deletions.
20 changes: 8 additions & 12 deletions src/cfnlint/conditions/conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 5 additions & 9 deletions src/cfnlint/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,8 @@

from cfnlint.rules._rule import CloudFormationLintRule, Match, RuleMatch
from cfnlint.rules._rules import Rules, RulesCollection

# type: ignore
from cfnlint.rules.jsonschema import (
CfnLintJsonSchema,
CfnLintJsonSchemaRegional,
CfnLintKeyword,
CfnLintRelationship,
SchemaDetails,
)
from cfnlint.rules.jsonschema import CfnLintJsonSchema # type: ignore
from cfnlint.rules.jsonschema import CfnLintJsonSchemaRegional # type: ignore
from cfnlint.rules.jsonschema import CfnLintKeyword # type: ignore
from cfnlint.rules.jsonschema import CfnLintRelationship # type: ignore
from cfnlint.rules.jsonschema import SchemaDetails
105 changes: 60 additions & 45 deletions src/cfnlint/rules/jsonschema/CfnLintRelationship.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,42 +22,59 @@ def __init__(
self.relationship = relationship or ""
self.parent_rules = ["E1101"]

def _get_relationship(
self, validator: Validator, instance: Any, path: deque[str | int]
def _get_relationship_fn_if(
self, validator: Validator, key: Any, value: Any, path: deque[str | int]
) -> Iterator[tuple[Any, Validator]]:
fn_k, fn_v = is_function(instance)
if fn_k == "Fn::If":
if not isinstance(fn_v, list) or len(fn_v) != 3:
return
condition = fn_v[0]
# True path
status = {
k: set([v]) for k, v in validator.context.conditions.status.items()
}
if condition not in status:
status[condition] = set([True, False])
for scenario in validator.cfn.conditions.build_scenarios(status):
if_validator = validator.evolve(
context=validator.context.evolve(
conditions=validator.context.conditions.evolve(
status=scenario,
)
if not isinstance(value, list) or len(value) != 3:
return
condition = value[0]
# True path
status = {k: set([v]) for k, v in validator.context.conditions.status.items()}
if condition not in status:
status[condition] = set([True, False])
for scenario in validator.cfn.conditions.build_scenarios(status):
if_validator = validator.evolve(
context=validator.context.evolve(
conditions=validator.context.conditions.evolve(
status=scenario,
)
)
)

i = 1 if scenario[condition] else 2
for r, v in self._get_relationship(
validator.evolve(
context=if_validator.context.evolve(
path=if_validator.context.path.descend(path=key).descend(path=i)
)
),
value[i],
path.copy(),
):
yield r, v

i = 1 if scenario[condition] else 2
def _get_relationship_list(
self, validator: Validator, key: Any, instance: Any, path: deque[str | int]
) -> Iterator[tuple[Any, Validator]]:
if key == "*":
for i, v in enumerate(instance):
for r, v in self._get_relationship(
validator.evolve(
context=if_validator.context.evolve(
path=if_validator.context.path.descend(path=fn_k).descend(
path=i
)
)
context=validator.context.evolve(
path=validator.context.path.descend(path=i)
),
),
fn_v[i],
v,
path.copy(),
):
yield r, v

def _get_relationship(
self, validator: Validator, instance: Any, path: deque[str | int]
) -> Iterator[tuple[Any, Validator]]:
fn_k, fn_v = is_function(instance)
if fn_k == "Fn::If":
yield from self._get_relationship_fn_if(validator, fn_k, fn_v, path)
return

if fn_k == "Ref" and fn_v == "AWS::NoValue":
Expand All @@ -74,18 +91,7 @@ def _get_relationship(

key = path.popleft()
if isinstance(instance, list):
if key == "*":
for i, v in enumerate(instance):
for r, v in self._get_relationship(
validator.evolve(
context=validator.context.evolve(
path=validator.context.path.descend(path=i)
),
),
v,
path.copy(),
):
yield r, v
yield from self._get_relationship_list(validator, key, instance, path)
return

if not isinstance(instance, dict):
Expand All @@ -102,22 +108,31 @@ def _get_relationship(
):
yield r, v

def get_relationship(self, validator: Validator) -> Iterator[tuple[Any, Validator]]:
def _validate_get_relationship_parameters(self, validator: Validator) -> bool:
if len(validator.context.path.path) < 2:
return
return False

if "Resources" != validator.context.path.path[0]:
return False

if not validator.cfn.graph:
return False

Check warning on line 119 in src/cfnlint/rules/jsonschema/CfnLintRelationship.py

View check run for this annotation

Codecov / codecov/patch

src/cfnlint/rules/jsonschema/CfnLintRelationship.py#L119

Added line #L119 was not covered by tests

if len(self.relationship.split("/")) < 2:
return False

return True

def get_relationship(self, validator: Validator) -> Iterator[tuple[Any, Validator]]:
if not self._validate_get_relationship_parameters(validator):
return

resource_name = validator.context.path.path[1]
path = self.relationship.split("/")
if len(path) < 2:
return

# get related resources by ref/getatt to the source
# and relationship types
if not validator.cfn.graph:
return
for edge in validator.cfn.graph.graph.out_edges(resource_name):
for edge in validator.cfn.graph.graph.out_edges(resource_name): # type: ignore
other_resource = validator.cfn.template.get(path[0], {}).get(edge[1], {})

if other_resource.get("Type") != path[1]:
Expand Down
2 changes: 1 addition & 1 deletion src/cfnlint/template/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Empty file.
33 changes: 15 additions & 18 deletions test/unit/rules/jsonschema/test_cfn_lint_relationship.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
],
Expand Down Expand Up @@ -102,7 +102,6 @@ def template():
},
"Five": {
"Type": "AWS::EC2::Instance",
"Condition": "IsUsEast1",
"Properties": {
"ImageId": {
"Fn::If": [
Expand All @@ -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": {
Expand Down Expand Up @@ -168,7 +164,7 @@ def template():
"name,path,status,expected",
[
(
"One",
"Condition for value",
deque(["Resources", "ParentOne", "Properties", "ImageId"]),
{},
[
Expand All @@ -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",
Expand Down

0 comments on commit 1c2e463

Please sign in to comment.