From 46be812688c38428cf958f3633b73edd0c561dd9 Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Mon, 10 Feb 2025 10:19:02 -0800 Subject: [PATCH] Add new rules for SQS queue --- .../extensions/aws_sqs_queue/__init__.py | 0 .../extensions/aws_sqs_queue/properties.json | 57 ++++++ .../providers/all/aws_sqs_queue/__init__.py | 0 .../all/aws_sqs_queue/redrivepolicy.json | 19 ++ .../providers/us_east_1/aws-sqs-queue.json | 14 +- src/cfnlint/rules/resources/sqs/QueueDLQ.py | 101 +++++++++ .../rules/resources/sqs/QueueProperties.py | 46 +++++ src/cfnlint/rules/resources/sqs/__init__.py | 0 test/unit/rules/resources/sqs/__init__.py | 0 .../rules/resources/sqs/test_queue_dlq.py | 192 ++++++++++++++++++ .../resources/sqs/test_queue_properties.py | 85 ++++++++ 11 files changed, 510 insertions(+), 4 deletions(-) create mode 100644 src/cfnlint/data/schemas/extensions/aws_sqs_queue/__init__.py create mode 100644 src/cfnlint/data/schemas/extensions/aws_sqs_queue/properties.json create mode 100644 src/cfnlint/data/schemas/patches/providers/all/aws_sqs_queue/__init__.py create mode 100644 src/cfnlint/data/schemas/patches/providers/all/aws_sqs_queue/redrivepolicy.json create mode 100644 src/cfnlint/rules/resources/sqs/QueueDLQ.py create mode 100644 src/cfnlint/rules/resources/sqs/QueueProperties.py create mode 100644 src/cfnlint/rules/resources/sqs/__init__.py create mode 100644 test/unit/rules/resources/sqs/__init__.py create mode 100644 test/unit/rules/resources/sqs/test_queue_dlq.py create mode 100644 test/unit/rules/resources/sqs/test_queue_properties.py diff --git a/src/cfnlint/data/schemas/extensions/aws_sqs_queue/__init__.py b/src/cfnlint/data/schemas/extensions/aws_sqs_queue/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/cfnlint/data/schemas/extensions/aws_sqs_queue/properties.json b/src/cfnlint/data/schemas/extensions/aws_sqs_queue/properties.json new file mode 100644 index 0000000000..b5ac949ed7 --- /dev/null +++ b/src/cfnlint/data/schemas/extensions/aws_sqs_queue/properties.json @@ -0,0 +1,57 @@ +{ + "allOf": [ + { + "if": { + "properties": { + "FifoQueue": { + "enum": [ + true, + "true" + ] + } + }, + "required": [ + "FifoQueue" + ] + }, + "then": { + "if": { + "properties": { + "QueueName": { + "type": "string" + } + }, + "required": [ + "QueueName" + ] + }, + "then": { + "properties": { + "QueueName": { + "pattern": "^.*\\.fifo$" + } + } + } + } + }, + { + "if": { + "properties": { + "FifoQueue": { + "enum": [ + false, + "false" + ] + } + } + }, + "then": { + "properties": { + "ContentBasedDeduplication": false, + "DeduplicationScope": false, + "FifoThroughputLimit": false + } + } + } + ] +} diff --git a/src/cfnlint/data/schemas/patches/providers/all/aws_sqs_queue/__init__.py b/src/cfnlint/data/schemas/patches/providers/all/aws_sqs_queue/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/cfnlint/data/schemas/patches/providers/all/aws_sqs_queue/redrivepolicy.json b/src/cfnlint/data/schemas/patches/providers/all/aws_sqs_queue/redrivepolicy.json new file mode 100644 index 0000000000..09fe9668c0 --- /dev/null +++ b/src/cfnlint/data/schemas/patches/providers/all/aws_sqs_queue/redrivepolicy.json @@ -0,0 +1,19 @@ +[ + { + "op": "add", + "path": "/properties/RedrivePolicy/additionalProperties", + "value": false + }, + { + "op": "add", + "path": "/properties/RedrivePolicy/properties", + "value": { + "deadLetterTargetArn": { + "type": "string" + }, + "maxReceiveCount": { + "type": "integer" + } + } + } +] diff --git a/src/cfnlint/data/schemas/providers/us_east_1/aws-sqs-queue.json b/src/cfnlint/data/schemas/providers/us_east_1/aws-sqs-queue.json index f8c313f73a..03d61ee231 100644 --- a/src/cfnlint/data/schemas/providers/us_east_1/aws-sqs-queue.json +++ b/src/cfnlint/data/schemas/providers/us_east_1/aws-sqs-queue.json @@ -82,10 +82,16 @@ ] }, "RedrivePolicy": { - "type": [ - "object", - "string" - ] + "additionalProperties": false, + "properties": { + "deadLetterTargetArn": { + "type": "string" + }, + "maxReceiveCount": { + "type": "integer" + } + }, + "type": "object" }, "SqsManagedSseEnabled": { "type": "boolean" diff --git a/src/cfnlint/rules/resources/sqs/QueueDLQ.py b/src/cfnlint/rules/resources/sqs/QueueDLQ.py new file mode 100644 index 0000000000..a0f707cd09 --- /dev/null +++ b/src/cfnlint/rules/resources/sqs/QueueDLQ.py @@ -0,0 +1,101 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from collections import deque +from typing import Any, Iterator + +import cfnlint.data.schemas.extensions.aws_sqs_queue +from cfnlint.helpers import bool_compare, is_function +from cfnlint.jsonschema import ValidationError, ValidationResult, Validator +from cfnlint.rules.helpers import get_value_from_path +from cfnlint.rules.jsonschema.CfnLintJsonSchema import CfnLintJsonSchema, SchemaDetails + + +class QueueDLQ(CfnLintJsonSchema): + id = "E3502" + shortdesc = "Validate SQS queue properties are valid" + description = ( + "Depending on if the queue is FIFO or not the properties and " + "allowed values change. This rule validates properties and " + "values based on the queue type" + ) + source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-sqs-queue.html" + tags = ["resources", "sqs"] + + def __init__(self) -> None: + super().__init__( + keywords=[ + "Resources/AWS::SQS::Queue/Properties", + ], + schema_details=SchemaDetails( + module=cfnlint.data.schemas.extensions.aws_sqs_queue, + filename="properties.json", + ), + all_matches=True, + ) + + def _is_fifo_queue( + self, validator: Validator, instance: Any + ) -> Iterator[tuple[str, Validator]]: + standard = "standard" + fifo = "FIFO" + + if "FifoQueue" not in instance: + yield standard, validator + return + + for queue_type, queue_type_validator in get_value_from_path( + validator=validator, instance=instance, path=deque(["FifoQueue"]) + ): + yield ( + fifo if bool_compare(queue_type, True) else standard + ), queue_type_validator + + def validate( + self, validator: Validator, _: Any, instance: Any, schema: dict[str, Any] + ) -> ValidationResult: + + for queue_type, queue_type_validator in self._is_fifo_queue( + validator=validator, + instance=instance, + ): + queue_type_validator = queue_type_validator.evolve( + context=queue_type_validator.context.evolve( + path=validator.context.path.evolve() + ) + ) + + for target, target_validator in get_value_from_path( + queue_type_validator, + instance, + path=deque(["RedrivePolicy", "deadLetterTargetArn"]), + ): + k, v = is_function(target) + if k == "Fn::GetAtt": + if target_validator.is_type(v, "string"): + v = v.split(".") + + if len(v) < 1: + return + + dest_queue = validator.cfn.template.get("Resources", {}).get( + v[0], {} + ) + if dest_queue.get("Type") != "AWS::SQS::Queue": + return + + for dest_queue_type, _ in self._is_fifo_queue( + target_validator, + instance=dest_queue.get("Properties", {}), + ): + if queue_type != dest_queue_type: + yield ValidationError( + ( + f"Source queue type {queue_type!r} does not " + "match destination queue type {dest_queue_type!r}" + ), + rule=self, + path=target_validator.context.path.path, + ) diff --git a/src/cfnlint/rules/resources/sqs/QueueProperties.py b/src/cfnlint/rules/resources/sqs/QueueProperties.py new file mode 100644 index 0000000000..ed6baf2052 --- /dev/null +++ b/src/cfnlint/rules/resources/sqs/QueueProperties.py @@ -0,0 +1,46 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from typing import Any + +import cfnlint.data.schemas.extensions.aws_sqs_queue +from cfnlint.jsonschema import ValidationResult, Validator +from cfnlint.rules.jsonschema.CfnLintJsonSchema import CfnLintJsonSchema, SchemaDetails + + +class QueueProperties(CfnLintJsonSchema): + id = "E3501" + shortdesc = "Validate SQS queue properties are valid" + description = ( + "Depending on if the queue is FIFO or not the " + "properties and allowed values change. " + "This rule validates properties and values based on the queue type" + ) + source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-sqs-queue.html" + tags = ["resources", "sqs"] + + def __init__(self) -> None: + super().__init__( + keywords=[ + "Resources/AWS::SQS::Queue/Properties", + ], + schema_details=SchemaDetails( + module=cfnlint.data.schemas.extensions.aws_sqs_queue, + filename="properties.json", + ), + all_matches=True, + ) + + def validate( + self, validator: Validator, keywords: Any, instance: Any, schema: Any + ) -> ValidationResult: + for err in super().validate(validator, keywords, instance, schema): + if err.schema is False: + err.message = ( + f"Additional properties are not allowed ({err.path[-1]!r} " + "was unexpected)" + ) + + yield err diff --git a/src/cfnlint/rules/resources/sqs/__init__.py b/src/cfnlint/rules/resources/sqs/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/unit/rules/resources/sqs/__init__.py b/test/unit/rules/resources/sqs/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/unit/rules/resources/sqs/test_queue_dlq.py b/test/unit/rules/resources/sqs/test_queue_dlq.py new file mode 100644 index 0000000000..6dc96e6541 --- /dev/null +++ b/test/unit/rules/resources/sqs/test_queue_dlq.py @@ -0,0 +1,192 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from collections import deque + +import pytest + +from cfnlint.jsonschema import ValidationError +from cfnlint.rules.resources.sqs.QueueDLQ import QueueDLQ + + +@pytest.fixture(scope="module") +def rule(): + rule = QueueDLQ() + yield rule + + +@pytest.fixture +def template(): + return { + "Parameters": {"FifoQueue": {"Type": "Boolean"}}, + "Conditions": {"UseFifoQueue": {"Fn::Equals": [{"Ref": "FifoQueue"}, "true"]}}, + "Resources": { + "FifoQueue": { + "Type": "AWS::SQS::Queue", + "Properties": { + "FifoQueue": True, + }, + }, + "StandardQueue1": { + "Type": "AWS::SQS::Queue", + "Properties": {}, + }, + "StandardQueue2": { + "Type": "AWS::SQS::Queue", + "Properties": { + "FifoQueue": "false", + }, + }, + }, + } + + +@pytest.mark.parametrize( + "instance,expected", + [ + ( + { + "FifoQueue": True, + "RedrivePolicy": { + "deadLetterTargetArn": {"Fn::GetAtt": "FifoQueue.Arn"} + }, + }, + [], + ), + ( + { + "RedrivePolicy": { + "deadLetterTargetArn": {"Fn::GetAtt": "StandardQueue1.Arn"} + } + }, + [], + ), + ( + { + "RedrivePolicy": { + "deadLetterTargetArn": {"Fn::GetAtt": "StandardQueue2.Arn"} + } + }, + [], + ), + ( + { + "FifoQueue": { + "Fn::If": [ + "UseFifoQueue", + True, + False, + ] + }, + "RedrivePolicy": { + "deadLetterTargetArn": { + "Fn::If": [ + "UseFifoQueue", + {"Fn::GetAtt": "FifoQueue.Arn"}, + {"Fn::GetAtt": "StandardQueue1.Arn"}, + ] + } + }, + }, + [], + ), + ( + { + "FifoQueue": True, + "RedrivePolicy": { + "deadLetterTargetArn": {"Fn::GetAtt": "StandardQueue1.Arn"} + }, + }, + [ + ValidationError( + ( + "Source queue type 'FIFO' does not match " + "destination queue type 'standard'" + ), + rule=QueueDLQ(), + path=deque(["RedrivePolicy", "deadLetterTargetArn"]), + ) + ], + ), + ( + { + "FifoQueue": True, + "RedrivePolicy": { + "deadLetterTargetArn": {"Fn::GetAtt": "StandardQueue2.Arn"} + }, + }, + [ + ValidationError( + ( + "Source queue type 'FIFO' does not match " + "destination queue type 'standard'" + ), + rule=QueueDLQ(), + path=deque(["RedrivePolicy", "deadLetterTargetArn"]), + ) + ], + ), + ( + { + "FifoQueue": "false", + "RedrivePolicy": { + "deadLetterTargetArn": {"Fn::GetAtt": "FifoQueue.Arn"} + }, + }, + [ + ValidationError( + ( + "Source queue type 'standard' does not " + "match destination queue type 'FIFO'" + ), + rule=QueueDLQ(), + path=deque(["RedrivePolicy", "deadLetterTargetArn"]), + ) + ], + ), + ( + { + "FifoQueue": { + "Fn::If": [ + "UseFifoQueue", + True, + False, + ] + }, + "RedrivePolicy": { + "deadLetterTargetArn": { + "Fn::If": [ + "UseFifoQueue", + {"Fn::GetAtt": "StandardQueue1.Arn"}, + {"Fn::GetAtt": "FifoQueue.Arn"}, + ] + } + }, + }, + [ + ValidationError( + ( + "Source queue type 'FIFO' does not match " + "destination queue type 'standard'" + ), + rule=QueueDLQ(), + path=deque(["RedrivePolicy", "deadLetterTargetArn", "Fn::If", 1]), + ), + ValidationError( + ( + "Source queue type 'standard' does " + "not match destination queue type 'FIFO'" + ), + rule=QueueDLQ(), + path=deque(["RedrivePolicy", "deadLetterTargetArn", "Fn::If", 2]), + ), + ], + ), + ], +) +def test_validate(instance, expected, rule, validator): + errs = list(rule.validate(validator, "", instance, {})) + + assert errs == expected, f"Expected {expected} got {errs}" diff --git a/test/unit/rules/resources/sqs/test_queue_properties.py b/test/unit/rules/resources/sqs/test_queue_properties.py new file mode 100644 index 0000000000..8de3e0d8d1 --- /dev/null +++ b/test/unit/rules/resources/sqs/test_queue_properties.py @@ -0,0 +1,85 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from collections import deque + +import pytest + +from cfnlint.jsonschema import ValidationError +from cfnlint.rules.resources.sqs.QueueProperties import QueueProperties + + +@pytest.fixture(scope="module") +def rule(): + rule = QueueProperties() + yield rule + + +@pytest.mark.parametrize( + "instance,expected", + [ + ( + { + "FifoQueue": True, + }, + [], + ), + ( + { + "FifoQueue": True, + "QueueName": "test.fifo", + }, + [], + ), + ( + { + "FifoQueue": True, + "QueueName": "test", + }, + [ + ValidationError( + "'test' does not match '^.*\\\\.fifo$'", + rule=QueueProperties(), + path=deque(["QueueName"]), + validator="pattern", + schema_path=deque( + [ + "allOf", + 0, + "then", + "then", + "properties", + "QueueName", + "pattern", + ] + ), + ) + ], + ), + ( + { + "ContentBasedDeduplication": True, + }, + [ + ValidationError( + ( + "Additional properties are not allowed " + "('ContentBasedDeduplication' was unexpected)" + ), + rule=QueueProperties(), + path=deque(["ContentBasedDeduplication"]), + validator=None, + schema_path=deque( + ["allOf", 1, "then", "properties", "ContentBasedDeduplication"] + ), + ) + ], + ), + ], +) +def test_validate(instance, expected, rule, validator): + errs = list(rule.validate(validator, "", instance, {})) + + assert errs == expected, f"Expected {expected} got {errs}"