Skip to content

Commit

Permalink
Support recursive condition (#114)
Browse files Browse the repository at this point in the history 
* remove unnecessary parenthesis in tests

* resolve conditions recursively

* add tests

* update CHANGELOG.md

* merge with master and update format

* update CHANGELOG.md

* update cloudformation actions

* update version

* update version and CHANGELOG.md

* add test for deadlock conditions

---------

Co-authored-by: Ramon <ramon.guimera@skyscanner.net>
  • Loading branch information
@w0rmr1d3r
w0rmr1d3r and Ramon authored Oct 30, 2023
1 parent cb1dbf0 commit 32262eb
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 15 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
All notable changes to this project will be documented in this file.

## 0.21.0
### Updates
### Fixes
- Fix resolving conditions recursively [#114](https://github.com/Skyscanner/pycfmodel/pull/114)
### Enhancements
- Compatible with Python3.11 [#110](https://github.com/Skyscanner/pycfmodel/pull/110)
- Compatible with Python3.12 [#115](https://github.com/Skyscanner/pycfmodel/pull/115)
### Updates
Expand Down
12 changes: 12 additions & 0 deletions pycfmodel/cloudformation_actions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,16 @@
"application-cost-profiler:ListReportDefinitions",
"application-cost-profiler:PutReportDefinition",
"application-cost-profiler:UpdateReportDefinition",
"application-transformation:GetGroupingAssessment",
"application-transformation:GetPortingCompatibilityAssessment",
"application-transformation:GetPortingRecommendationAssessment",
"application-transformation:GetRuntimeAssessment",
"application-transformation:PutLogData",
"application-transformation:PutMetricData",
"application-transformation:StartGroupingAssessment",
"application-transformation:StartPortingCompatibilityAssessment",
"application-transformation:StartPortingRecommendationAssessment",
"application-transformation:StartRuntimeAssessment",
"applicationinsights:AddWorkload",
"applicationinsights:CreateApplication",
"applicationinsights:CreateComponent",
Expand Down Expand Up @@ -4851,6 +4861,7 @@
"ec2:DisableEbsEncryptionByDefault",
"ec2:DisableFastLaunch",
"ec2:DisableFastSnapshotRestores",
"ec2:DisableImage",
"ec2:DisableImageBlockPublicAccess",
"ec2:DisableImageDeprecation",
"ec2:DisableIpamOrganizationAdminAccount",
Expand Down Expand Up @@ -4879,6 +4890,7 @@
"ec2:EnableEbsEncryptionByDefault",
"ec2:EnableFastLaunch",
"ec2:EnableFastSnapshotRestores",
"ec2:EnableImage",
"ec2:EnableImageBlockPublicAccess",
"ec2:EnableImageDeprecation",
"ec2:EnableIpamOrganizationAdminAccount",
Expand Down
9 changes: 5 additions & 4 deletions pycfmodel/model/cf_model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,11 @@ def resolve(self, extra_params=None) -> "CFModel":
dict_value = self.dict()

conditions = dict_value.pop("Conditions", {})
resolved_conditions = {
key: _extended_bool(resolve(value, extended_parameters, self.Mappings, {}))
for key, value in conditions.items()
}
resolved_conditions = {}
for key, value in conditions.items():
resolved_conditions.update(
{key: _extended_bool(resolve(value, extended_parameters, self.Mappings, resolved_conditions))}
)

resources = dict_value.pop("Resources")
resolved_resources = {
Expand Down
1 change: 1 addition & 0 deletions setup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"pip-tools>=2.0.2",
"pytest>=6.0.1",
"pytest-cov>=2.10.1",
"pytest-repeat==0.9.3",
]

docs_requires = [
Expand Down
143 changes: 143 additions & 0 deletions tests/test_resolver.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,149 @@ def test_condition(function, expected_output):
assert resolve(function, parameters, mappings, conditions) == expected_output


# We will repeat the test 10 times, in order to check conditions don't have a different order
# and break the resolving of the model when they are depending of other conditions
@pytest.mark.repeat(10)
@pytest.mark.parametrize(
"num_custom_tags, expected",
[
(0, []),
(1, ["HasAtLeast1Tags"]),
(2, ["HasAtLeast1Tags", "HasAtLeast2Tags"]),
(3, ["HasAtLeast1Tags", "HasAtLeast2Tags", "HasAtLeast3Tags"]),
(4, ["HasAtLeast1Tags", "HasAtLeast2Tags", "HasAtLeast3Tags", "HasAtLeast4Tags"]),
(5, ["HasAtLeast1Tags", "HasAtLeast2Tags", "HasAtLeast3Tags", "HasAtLeast4Tags", "HasAtLeast5Tags"]),
(
6,
[
"HasAtLeast1Tags",
"HasAtLeast2Tags",
"HasAtLeast3Tags",
"HasAtLeast4Tags",
"HasAtLeast5Tags",
"HasAtLeast6Tags",
],
),
(
7,
[
"HasAtLeast1Tags",
"HasAtLeast2Tags",
"HasAtLeast3Tags",
"HasAtLeast4Tags",
"HasAtLeast5Tags",
"HasAtLeast6Tags",
"HasAtLeast7Tags",
],
),
(
8,
[
"HasAtLeast1Tags",
"HasAtLeast2Tags",
"HasAtLeast3Tags",
"HasAtLeast4Tags",
"HasAtLeast5Tags",
"HasAtLeast6Tags",
"HasAtLeast7Tags",
"HasAtLeast8Tags",
],
),
(
9,
[
"HasAtLeast1Tags",
"HasAtLeast2Tags",
"HasAtLeast3Tags",
"HasAtLeast4Tags",
"HasAtLeast5Tags",
"HasAtLeast6Tags",
"HasAtLeast7Tags",
"HasAtLeast8Tags",
"HasAtLeast9Tags",
],
),
(
10,
[
"HasAtLeast1Tags",
"HasAtLeast2Tags",
"HasAtLeast3Tags",
"HasAtLeast4Tags",
"HasAtLeast5Tags",
"HasAtLeast6Tags",
"HasAtLeast7Tags",
"HasAtLeast8Tags",
"HasAtLeast9Tags",
"HasAtLeast10Tags",
],
),
(11, []),
],
)
def test_resolve_recursive_conditions(num_custom_tags, expected):
template = {
"Parameters": {
"NumCustomTags": {"Type": "Number", "Default": 0},
},
"Conditions": {
"HasAtLeast10Tags": {"Fn::Equals": [{"Ref": "NumCustomTags"}, 10]}, # this is the condition stopper
"HasAtLeast9Tags": {
"Fn::Or": [{"Fn::Equals": [{"Ref": "NumCustomTags"}, 9]}, {"Condition": "HasAtLeast10Tags"}]
},
"HasAtLeast8Tags": {
"Fn::Or": [{"Fn::Equals": [{"Ref": "NumCustomTags"}, 8]}, {"Condition": "HasAtLeast9Tags"}]
},
"HasAtLeast7Tags": {
"Fn::Or": [{"Fn::Equals": [{"Ref": "NumCustomTags"}, 7]}, {"Condition": "HasAtLeast8Tags"}]
},
"HasAtLeast6Tags": {
"Fn::Or": [{"Fn::Equals": [{"Ref": "NumCustomTags"}, 6]}, {"Condition": "HasAtLeast7Tags"}]
},
"HasAtLeast5Tags": {
"Fn::Or": [{"Fn::Equals": [{"Ref": "NumCustomTags"}, 5]}, {"Condition": "HasAtLeast6Tags"}]
},
"HasAtLeast4Tags": {
"Fn::Or": [{"Fn::Equals": [{"Ref": "NumCustomTags"}, 4]}, {"Condition": "HasAtLeast5Tags"}]
},
"HasAtLeast3Tags": {
"Fn::Or": [{"Fn::Equals": [{"Ref": "NumCustomTags"}, 3]}, {"Condition": "HasAtLeast4Tags"}]
},
"HasAtLeast2Tags": {
"Fn::Or": [{"Fn::Equals": [{"Ref": "NumCustomTags"}, 2]}, {"Condition": "HasAtLeast3Tags"}]
},
"HasAtLeast1Tags": {
"Fn::Or": [{"Fn::Equals": [{"Ref": "NumCustomTags"}, 1]}, {"Condition": "HasAtLeast2Tags"}]
},
},
"Resources": {},
}

model = parse(template).resolve(extra_params={"NumCustomTags": num_custom_tags})

# retrieve positive conditions in the model
positive_conditions = [
condition_name for condition_name, condition_value in model.Conditions.items() if condition_value
]
assert expected.sort() == positive_conditions.sort()


def test_resolve_infinite_loop_or_deadlock_conditions_will_resolve_to_false():
template = {
"Parameters": {},
"Conditions": {
"ConditionA": {"Condition": "ConditionB"},
"ConditionB": {"Condition": "ConditionA"},
},
"Resources": {},
}

model = parse(template).resolve()

assert model.Conditions.get("ConditionA") is False
assert model.Conditions.get("ConditionB") is False


def test_select_and_ref():
parameters = {"DbSubnetIpBlocks": ["10.0.48.0/24", "10.0.112.0/24", "10.0.176.0/24"]}
mappings = {}
Expand Down
20 changes: 10 additions & 10 deletions tests/test_types.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ class Model(BaseModel):
@pytest.mark.parametrize(
"value",
[
("192.168.0.0/24"),
("192.168.128.0/30"),
"192.168.0.0/24",
"192.168.128.0/30",
(2**32 - 1), # no mask equals to mask /32
(b"\xff\xff\xff\xff"), # /32
(("192.168.0.0", 24)),
b"\xff\xff\xff\xff", # /32
("192.168.0.0", 24),
(IPv4Network("192.168.0.0/24")),
],
)
Expand All @@ -53,10 +53,10 @@ class Model(BaseModel):
@pytest.mark.parametrize(
"value",
[
("2001:db00::0/120"),
(20_282_409_603_651_670_423_947_251_286_015), # /128
(b"\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"),
(("2001:db00::0", 120)),
"2001:db00::0/120",
20_282_409_603_651_670_423_947_251_286_015, # /128
b"\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff",
("2001:db00::0", 120),
(IPv6Network("2001:db00::0/120")),
],
)
Expand All @@ -67,7 +67,7 @@ class Model(BaseModel):
assert Model(ip=value).ip == IPv6Network(value)


@pytest.mark.parametrize("value", [("213.174.214.100/27"), ("192.168.56.101/16"), ("192.0.2.1/24")])
@pytest.mark.parametrize("value", ["213.174.214.100/27", "192.168.56.101/16", "192.0.2.1/24"])
def test_loose_ip_v4_is_not_strict(value):
class Model(BaseModel):
ip: LooseIPv4Network = None
Expand All @@ -79,7 +79,7 @@ class Model(BaseModel):

@pytest.mark.parametrize(
"value",
[("2012::1234:abcd:ffff:c0a8:101/64"), ("2022::1234:abcd:ffff:c0a8:101/64"), ("2032::1234:abcd:ffff:c0a8:101/64")],
["2012::1234:abcd:ffff:c0a8:101/64", "2022::1234:abcd:ffff:c0a8:101/64", "2032::1234:abcd:ffff:c0a8:101/64"],
)
def test_loose_ip_v6_is_not_strict(value):
class Model(BaseModel):
Expand Down

0 comments on commit 32262eb

Please sign in to comment.