From 8d68ccda50cacab675c1d2dd49eee6ceabffbe11 Mon Sep 17 00:00:00 2001 From: Jiati Le Date: Wed, 18 Jan 2023 13:26:55 -0500 Subject: [PATCH 1/3] Validate scheduling strategy for Fargate services --- docs/rules.md | 1 + .../FargateDeploymentSchedulingStrategy.py | 28 +++++++++++++++++ .../ecs/TaskDefinitionEssentialContainer.py | 6 ++-- .../ecs/test_fargate_scheduling_strategy.yaml | 6 ++++ .../ecs/test_fargate_scheduling_strategy.yaml | 16 ++++++++++ ...est_fargate_service_scheduling_strategy.py | 31 +++++++++++++++++++ 6 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 src/cfnlint/rules/resources/ecs/FargateDeploymentSchedulingStrategy.py create mode 100644 test/fixtures/templates/bad/resources/ecs/test_fargate_scheduling_strategy.yaml create mode 100644 test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml create mode 100644 test/unit/rules/resources/ecs/test_fargate_service_scheduling_strategy.py diff --git a/docs/rules.md b/docs/rules.md index 9b6a65684b..71888b87c9 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -132,6 +132,7 @@ The following **151** rules are applied by this linter: | [E3041](../src/cfnlint/rules/resources/route53/RecordSetName.py) | RecordSet HostedZoneName is a superdomain of Name | In a RecordSet, the HostedZoneName must be a superdomain of the Name being validated | | [Source](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-route53-recordset.html#cfn-route53-recordset-name) | `resource`,`properties`,`route53` | | [E3042](../src/cfnlint/rules/resources/ecs/TaskDefinitionEssentialContainer.py) | Check at least one essential container is specified | Check that every TaskDefinition specifies at least one essential container | | [Source](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ecs-taskdefinition-containerdefinitions.html#cfn-ecs-taskdefinition-containerdefinition-essential) | `properties`,`ecs`,`task`,`container`,`fargate` | | [E3043](../src/cfnlint/rules/resources/cloudformation/NestedStackParameters.py) | Validate parameters for in a nested stack | Evalute if parameters for a nested stack are specified and if parameters are specified for a nested stack that aren't required. | | [Source](https://github.com/awslabs/cfn-python-lint) | `resources`,`cloudformation` | +| [E3044](../src/cfnlint/rules/resources/ecs/FargateDeploymentSchedulingStrategy.py) | Validate scheduling strategy for Fargate services | Fargate services currently only support REPLICA scheduling strategy | | [Source](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecs-service.html#cfn-ecs-service-schedulingstrategy) | `resources`,`container`, `fargate`, `service` | | [E3050](../src/cfnlint/rules/resources/iam/RefWithPath.py) | Check if REFing to a IAM resource with path set | Some resources don't support looking up the IAM resource by name. This check validates when a REF is being used and the Path is not '/' | | [Source](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements.html) | `properties`,`iam` | | [E3502](../src/cfnlint/rules/resources/properties/JsonSize.py) | Check if a JSON Object is within size limits | Validate properties that are JSON values so that their length is within the limits | | [Source](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cloudformation-limits.html) | `resources`,`limits`,`json` | | [E3503](../src/cfnlint/rules/resources/certificatemanager/DomainValidationOptions.py) | ValidationDomain is superdomain of DomainName | In ValidationDomainOptions, the ValidationDomain must be a superdomain of the DomainName being validated | | [Source](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-certificatemanager-certificate-domainvalidationoption.html#cfn-certificatemanager-certificate-domainvalidationoption-validationdomain) | `certificate`,`certificatemanager`,`domainvalidationoptions`,`validationdomain` | diff --git a/src/cfnlint/rules/resources/ecs/FargateDeploymentSchedulingStrategy.py b/src/cfnlint/rules/resources/ecs/FargateDeploymentSchedulingStrategy.py new file mode 100644 index 0000000000..9d9b991278 --- /dev/null +++ b/src/cfnlint/rules/resources/ecs/FargateDeploymentSchedulingStrategy.py @@ -0,0 +1,28 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +from cfnlint.rules import CloudFormationLintRule, RuleMatch + + +class FargateDeploymentSchedulingStrategy(CloudFormationLintRule): + id = "E3044" + shortdesc = "Check Fargate service scheduling strategy" + description = "Check that Fargate service scheduling strategy is REPLICA" + source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecs-service.html#cfn-ecs-service-schedulingstrategy" + tags = ["properties", "ecs", "service", "container", "fargate"] + + def match(self, cfn): + matches = [] + ecs_services = cfn.get_resource_properties(["AWS::ECS::Service"]) + + for ecs_service in ecs_services: + path = ecs_service["Path"] + properties = ecs_service["Value"] + if isinstance(properties, dict): + if properties.get("LaunchType", None) != "Fargate": + continue + if properties.get("SchedulingStrategy", None) != "REPLICA": + error_message = f"Fargate service only support REPLICA as scheduling strategy at {'/'.join(map(str, path))}" + matches.append(RuleMatch(path, error_message)) + return matches diff --git a/src/cfnlint/rules/resources/ecs/TaskDefinitionEssentialContainer.py b/src/cfnlint/rules/resources/ecs/TaskDefinitionEssentialContainer.py index 598037178e..2ffb0b6285 100644 --- a/src/cfnlint/rules/resources/ecs/TaskDefinitionEssentialContainer.py +++ b/src/cfnlint/rules/resources/ecs/TaskDefinitionEssentialContainer.py @@ -41,8 +41,10 @@ def match(self, cfn): has_essential_container = True if not has_essential_container: - message = "No essential containers defined for {0}" - rule_match = RuleMatch(path, message.format("/".join(map(str, path)))) + error_message = ( + f"No essential containers defined for {'/'.join(map(str, path))}" + ) + rule_match = RuleMatch(path, error_message) matches.append(rule_match) return matches diff --git a/test/fixtures/templates/bad/resources/ecs/test_fargate_scheduling_strategy.yaml b/test/fixtures/templates/bad/resources/ecs/test_fargate_scheduling_strategy.yaml new file mode 100644 index 0000000000..f5246cccd7 --- /dev/null +++ b/test/fixtures/templates/bad/resources/ecs/test_fargate_scheduling_strategy.yaml @@ -0,0 +1,6 @@ +Resources: + Service1: + Type: AWS::ECS::Service + Properties: + LaunchType: Fargate + SchedulingStrategy: DAEMON diff --git a/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml b/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml new file mode 100644 index 0000000000..2f333349f2 --- /dev/null +++ b/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml @@ -0,0 +1,16 @@ +Resources: + Service1: + Type: AWS::ECS::Service, + Properties: + LaunchType: Fargate, + SchedulingStrategy: REPLICA + Service2: + Type: AWS::ECS::Service + Properties: + LaunchType: EC2 + SchedulingStrategy: DAEMON + Service3: + Type: AWS::ECS::Service + Properties: + LaunchType: EXTERNAL + SchedulingStrategy: DAEMON diff --git a/test/unit/rules/resources/ecs/test_fargate_service_scheduling_strategy.py b/test/unit/rules/resources/ecs/test_fargate_service_scheduling_strategy.py new file mode 100644 index 0000000000..06adc30a4d --- /dev/null +++ b/test/unit/rules/resources/ecs/test_fargate_service_scheduling_strategy.py @@ -0,0 +1,31 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" +from test.unit.rules import BaseRuleTestCase + +from cfnlint.rules.resources.ecs.FargateDeploymentSchedulingStrategy import ( + FargateDeploymentSchedulingStrategy, +) + + +class TestFargateServiceSchedulingStrategy(BaseRuleTestCase): + """ + Test that Fargate service has correct scheduling strategy (REPLICA) + """ + + def setUp(self): + super(TestFargateServiceSchedulingStrategy, self).setUp() + self.collection.register(FargateDeploymentSchedulingStrategy()) + self.success_templates = [ + "test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml" + ] + + def test_file_positive(self): + self.helper_file_positive() + + def test_file_negative(self): + self.helper_file_negative( + "test/fixtures/templates/bad/resources/ecs/test_fargate_scheduling_strategy.yaml", + 1, + ) From cf80bfe245b5aa74ef554df1a6c95254b419af9c Mon Sep 17 00:00:00 2001 From: Jiati Le <6442124+lejiati@users.noreply.github.com> Date: Wed, 25 Jan 2023 20:27:37 -0500 Subject: [PATCH 2/3] Update test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml Co-authored-by: Pat Myron --- .../good/resources/ecs/test_fargate_scheduling_strategy.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml b/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml index 2f333349f2..5614da29cb 100644 --- a/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml +++ b/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml @@ -1,8 +1,8 @@ Resources: Service1: - Type: AWS::ECS::Service, + Type: AWS::ECS::Service Properties: - LaunchType: Fargate, + LaunchType: Fargate SchedulingStrategy: REPLICA Service2: Type: AWS::ECS::Service From 7a0730a3e95d1769d1e940e83cde30eae5b2acd3 Mon Sep 17 00:00:00 2001 From: Jiati Le Date: Tue, 21 Feb 2023 13:44:36 -0500 Subject: [PATCH 3/3] Fix Fargate strategy rule to handle non string properties Change: Intrinsic functions can be used for resource properties and previously this rule checker doesn't handle such scenario. --- .../ecs/FargateDeploymentSchedulingStrategy.py | 14 +++++++++----- .../ecs/test_fargate_scheduling_strategy.yaml | 5 +++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/cfnlint/rules/resources/ecs/FargateDeploymentSchedulingStrategy.py b/src/cfnlint/rules/resources/ecs/FargateDeploymentSchedulingStrategy.py index 9d9b991278..1e00011294 100644 --- a/src/cfnlint/rules/resources/ecs/FargateDeploymentSchedulingStrategy.py +++ b/src/cfnlint/rules/resources/ecs/FargateDeploymentSchedulingStrategy.py @@ -20,9 +20,13 @@ def match(self, cfn): path = ecs_service["Path"] properties = ecs_service["Value"] if isinstance(properties, dict): - if properties.get("LaunchType", None) != "Fargate": - continue - if properties.get("SchedulingStrategy", None) != "REPLICA": - error_message = f"Fargate service only support REPLICA as scheduling strategy at {'/'.join(map(str, path))}" - matches.append(RuleMatch(path, error_message)) + launch_type = properties.get("LaunchType", None) + if isinstance(launch_type, str) and launch_type == "Fargate": + scheduling_strategy = properties.get("SchedulingStrategy", None) + if ( + isinstance(scheduling_strategy, str) + and scheduling_strategy != "REPLICA" + ): + error_message = f"Fargate service only support REPLICA as scheduling strategy at {'/'.join(map(str, path))}" + matches.append(RuleMatch(path, error_message)) return matches diff --git a/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml b/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml index 5614da29cb..0331566ce8 100644 --- a/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml +++ b/test/fixtures/templates/good/resources/ecs/test_fargate_scheduling_strategy.yaml @@ -14,3 +14,8 @@ Resources: Properties: LaunchType: EXTERNAL SchedulingStrategy: DAEMON + Service4: + Type: AWS::ECS::Service + Properties: + LaunchType: !Join ["", ["FAR", "GATE"]] + SchedulingStrategy: DAEMON