From 8ecdf35afd7fb136b2b0991c0d03e4be84ab7ab7 Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Wed, 31 Jul 2024 08:59:24 -0700 Subject: [PATCH] Improve testing --- .../resources/ecs/TaskDefinitionAwsVpc.py | 2 +- .../resources/ecs/test_service_fargate.py | 116 ++++++++++++++++++ .../ecs/test_service_network_configuration.py | 99 +++++++++++++++ .../ecs/test_task_definition_aws_vpc.py | 55 +++++++++ 4 files changed, 271 insertions(+), 1 deletion(-) diff --git a/src/cfnlint/rules/resources/ecs/TaskDefinitionAwsVpc.py b/src/cfnlint/rules/resources/ecs/TaskDefinitionAwsVpc.py index c120bccd27..0788449bfa 100644 --- a/src/cfnlint/rules/resources/ecs/TaskDefinitionAwsVpc.py +++ b/src/cfnlint/rules/resources/ecs/TaskDefinitionAwsVpc.py @@ -49,7 +49,7 @@ def _get_port_mappings( container_definition, path=deque(["ContainerPort"]), ): - if not isinstance(host_port, (str, int)): + if not isinstance(container_port, (str, int)): continue if str(host_port) != str(container_port): yield host_port, container_port, host_port_validator diff --git a/test/unit/rules/resources/ecs/test_service_fargate.py b/test/unit/rules/resources/ecs/test_service_fargate.py index 60cc6fff47..5ea578ccde 100644 --- a/test/unit/rules/resources/ecs/test_service_fargate.py +++ b/test/unit/rules/resources/ecs/test_service_fargate.py @@ -50,6 +50,122 @@ def rule(): deque(["Resources", "Service", "Properties"]), [], ), + ( + { + "Resources": { + "TaskDefinition": jsonpatch.apply_patch( + dict(_task_definition), + [ + { + "op": "remove", + "path": "/Properties/RequiresCompatibilities", + }, + ], + ), + "Service": jsonpatch.apply_patch( + dict(_service), + [ + { + "op": "replace", + "path": "/Properties/TaskDefinition", + "value": {"Fn::Sub": "${TaskDefinition.Arn}"}, + }, + ], + ), + } + }, + deque(["Resources", "Service", "Properties"]), + [], + ), + ( + { + "Resources": { + "TaskDefinition": jsonpatch.apply_patch( + dict(_task_definition), + [ + { + "op": "remove", + "path": "/Properties/RequiresCompatibilities", + }, + ], + ), + "Service": jsonpatch.apply_patch( + dict(_service), + [ + { + "op": "replace", + "path": "/Properties/TaskDefinition", + "value": {"Fn::GetAtt": "TaskDefinition.Arn"}, + }, + ], + ), + } + }, + deque(["Resources", "Service", "Properties"]), + [ + ValidationError( + ("'RequiresCompatibilities' is a required property"), + validator="required", + rule=ServiceFargate(), + path_override=deque(["Resources", "TaskDefinition", "Properties"]), + ) + ], + ), + ( + { + "Parameters": {"MyTask": {"Type": "String"}}, + "Resources": { + "TaskDefinition": jsonpatch.apply_patch( + dict(_task_definition), + [ + { + "op": "remove", + "path": "/Properties/RequiresCompatibilities", + }, + ], + ), + "Service": jsonpatch.apply_patch( + dict(_service), + [ + { + "op": "replace", + "path": "/Properties/TaskDefinition", + "value": {"Ref": "MyTask"}, + }, + ], + ), + }, + }, + deque(["Resources", "Service", "Properties"]), + [], + ), + ( + { + "Resources": { + "TaskDefinition": jsonpatch.apply_patch( + dict(_task_definition), + [ + { + "op": "remove", + "path": "/Properties/RequiresCompatibilities", + }, + ], + ), + "Service": jsonpatch.apply_patch( + dict(_service), + [ + { + "op": "replace", + "path": "/Properties/TaskDefinition", + "value": "MyTask", + }, + ], + ), + } + }, + deque(["Resources", "Service", "Properties"]), + [], + ), ( { "Resources": { diff --git a/test/unit/rules/resources/ecs/test_service_network_configuration.py b/test/unit/rules/resources/ecs/test_service_network_configuration.py index dfd08490d0..08215e8bda 100644 --- a/test/unit/rules/resources/ecs/test_service_network_configuration.py +++ b/test/unit/rules/resources/ecs/test_service_network_configuration.py @@ -158,6 +158,105 @@ def rule(): ) ], ), + ( + { + "Resources": { + "TaskDefinition": dict(_task_definition), + "Service": jsonpatch.apply_patch( + dict(_service), + [ + { + "op": "remove", + "path": "/Properties/NetworkConfiguration", + }, + { + "op": "replace", + "path": "/Properties/TaskDefinition", + "value": {"Fn::GetAtt": "TaskDefinition.Arn"}, + }, + ], + ), + } + }, + deque(["Resources", "Service", "Properties"]), + [ + ValidationError( + ("'NetworkConfiguration' is a required property"), + validator="required", + rule=ServiceNetworkConfiguration(), + ) + ], + ), + ( + { + "Parameters": {"MyTargetGroup": {"Type": "String"}}, + "Resources": { + "TaskDefinition": dict(_task_definition), + "Service": jsonpatch.apply_patch( + dict(_service), + [ + { + "op": "remove", + "path": "/Properties/NetworkConfiguration", + }, + { + "op": "replace", + "path": "/Properties/TaskDefinition", + "value": {"Ref": "MyTargetGroup"}, + }, + ], + ), + }, + }, + deque(["Resources", "Service", "Properties"]), + [], + ), + ( + { + "Resources": { + "TaskDefinition": dict(_task_definition), + "Service": jsonpatch.apply_patch( + dict(_service), + [ + { + "op": "remove", + "path": "/Properties/NetworkConfiguration", + }, + { + "op": "replace", + "path": "/Properties/TaskDefinition", + "value": {"Foo": "Bar"}, + }, + ], + ), + }, + }, + deque(["Resources", "Service", "Properties"]), + [], + ), + ( + { + "Resources": { + "TaskDefinition": dict(_task_definition), + "Service": jsonpatch.apply_patch( + dict(_service), + [ + { + "op": "remove", + "path": "/Properties/NetworkConfiguration", + }, + { + "op": "replace", + "path": "/Properties/TaskDefinition", + "value": {"Fn::Sub": "${TaskDefinition.Arn}"}, + }, + ], + ), + }, + }, + deque(["Resources", "Service", "Properties"]), + [], + ), ], indirect=["template"], ) diff --git a/test/unit/rules/resources/ecs/test_task_definition_aws_vpc.py b/test/unit/rules/resources/ecs/test_task_definition_aws_vpc.py index 371d58ef87..3db1ac38f4 100644 --- a/test/unit/rules/resources/ecs/test_task_definition_aws_vpc.py +++ b/test/unit/rules/resources/ecs/test_task_definition_aws_vpc.py @@ -73,6 +73,35 @@ def rule(): deque(["Resources", "TaskDefinition", "Properties"]), [], ), + ( + { + "Resources": { + "TaskDefinition": jsonpatch.apply_patch( + dict(_task_definition), + [ + { + "op": "add", + "path": ( + "/Properties/ContainerDefinitions/" + "0/PortMappings/0/HostPort" + ), + "value": "8080", + }, + { + "op": "replace", + "path": ( + "/Properties/ContainerDefinitions/" + "0/PortMappings/0/ContainerPort" + ), + "value": {"Fn::Sub": "8080"}, + }, + ], + ), + } + }, + deque(["Resources", "TaskDefinition", "Properties"]), + [], + ), ( { "Parameters": { @@ -106,6 +135,32 @@ def rule(): deque(["Resources", "TaskDefinition", "Properties"]), [], ), + ( + { + "Resources": { + "TaskDefinition": jsonpatch.apply_patch( + dict(_task_definition), + [ + { + "op": "add", + "path": ( + "/Properties/ContainerDefinitions/0" + "/PortMappings/0/HostPort" + ), + "value": "80", + }, + { + "op": "replace", + "path": ("/Properties/NetworkMode"), + "value": "bridge", + }, + ], + ), + } + }, + deque(["Resources", "TaskDefinition", "Properties"]), + [], + ), ( { "Resources": {