diff --git a/CHANGELOG.md b/CHANGELOG.md index a869ad9..d32f90f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/pycfmodel/cloudformation_actions.py b/pycfmodel/cloudformation_actions.py index 3fd018d..65da99e 100644 --- a/pycfmodel/cloudformation_actions.py +++ b/pycfmodel/cloudformation_actions.py @@ -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", @@ -4851,6 +4861,7 @@ "ec2:DisableEbsEncryptionByDefault", "ec2:DisableFastLaunch", "ec2:DisableFastSnapshotRestores", + "ec2:DisableImage", "ec2:DisableImageBlockPublicAccess", "ec2:DisableImageDeprecation", "ec2:DisableIpamOrganizationAdminAccount", @@ -4879,6 +4890,7 @@ "ec2:EnableEbsEncryptionByDefault", "ec2:EnableFastLaunch", "ec2:EnableFastSnapshotRestores", + "ec2:EnableImage", "ec2:EnableImageBlockPublicAccess", "ec2:EnableImageDeprecation", "ec2:EnableIpamOrganizationAdminAccount", diff --git a/pycfmodel/model/cf_model.py b/pycfmodel/model/cf_model.py index 2cc80ef..19c5951 100644 --- a/pycfmodel/model/cf_model.py +++ b/pycfmodel/model/cf_model.py @@ -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 = { diff --git a/setup.py b/setup.py index 4fc6fdf..2db59ba 100644 --- a/setup.py +++ b/setup.py @@ -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 = [ diff --git a/tests/test_resolver.py b/tests/test_resolver.py index 05b66bd..34f4be2 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -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 = {} diff --git a/tests/test_types.py b/tests/test_types.py index a99c6d0..4f07102 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -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")), ], ) @@ -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")), ], ) @@ -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 @@ -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):