From ee388c32dc83734fc3849eafae56fcf205a46313 Mon Sep 17 00:00:00 2001 From: Kevin DeJong Date: Thu, 1 Aug 2024 09:13:15 -0700 Subject: [PATCH] Update E3615 to validate all CloudWatch Alarm periods --- .../aws_namespace_period.json | 29 ----- .../aws_cloudwatch_alarm/period.json | 37 ++++++ ...rmAwsNamespacePeriod.py => AlarmPeriod.py} | 16 ++- .../test_alarm_aws_namespace_period.py | 71 ----------- .../resources/cloudwatch/test_alarm_period.py | 116 ++++++++++++++++++ 5 files changed, 160 insertions(+), 109 deletions(-) delete mode 100644 src/cfnlint/data/schemas/extensions/aws_cloudwatch_alarm/aws_namespace_period.json create mode 100644 src/cfnlint/data/schemas/extensions/aws_cloudwatch_alarm/period.json rename src/cfnlint/rules/resources/cloudwatch/{AlarmAwsNamespacePeriod.py => AlarmPeriod.py} (63%) delete mode 100644 test/unit/rules/resources/cloudwatch/test_alarm_aws_namespace_period.py create mode 100644 test/unit/rules/resources/cloudwatch/test_alarm_period.py diff --git a/src/cfnlint/data/schemas/extensions/aws_cloudwatch_alarm/aws_namespace_period.json b/src/cfnlint/data/schemas/extensions/aws_cloudwatch_alarm/aws_namespace_period.json deleted file mode 100644 index 34963a918d..0000000000 --- a/src/cfnlint/data/schemas/extensions/aws_cloudwatch_alarm/aws_namespace_period.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "additionalProperties": true, - "description": "Period < 60 not supported for namespaces with the following prefix: AWS/", - "if": { - "properties": { - "Namespace": { - "pattern": "^AWS/.*$", - "type": "string" - }, - "Period": { - "type": [ - "number", - "string" - ] - } - }, - "required": [ - "Namespace" - ], - "type": "object" - }, - "then": { - "properties": { - "Period": { - "minimum": 60 - } - } - } -} diff --git a/src/cfnlint/data/schemas/extensions/aws_cloudwatch_alarm/period.json b/src/cfnlint/data/schemas/extensions/aws_cloudwatch_alarm/period.json new file mode 100644 index 0000000000..bc209bcb61 --- /dev/null +++ b/src/cfnlint/data/schemas/extensions/aws_cloudwatch_alarm/period.json @@ -0,0 +1,37 @@ +{ + "additionalProperties": true, + "description": "The period, in seconds, over which the statistic is applied. This is required for an alarm based on a metric. Valid values are 10, 30, 60, and any multiple of 60.", + "if": { + "properties": { + "Period": { + "type": [ + "integer", + "string" + ] + } + }, + "required": [ + "Period" + ], + "type": "object" + }, + "then": { + "properties": { + "Period": { + "else": { + "multipleOf": 60 + }, + "if": { + "maximum": 60 + }, + "then": { + "enum": [ + 10, + 30, + 60 + ] + } + } + } + } +} diff --git a/src/cfnlint/rules/resources/cloudwatch/AlarmAwsNamespacePeriod.py b/src/cfnlint/rules/resources/cloudwatch/AlarmPeriod.py similarity index 63% rename from src/cfnlint/rules/resources/cloudwatch/AlarmAwsNamespacePeriod.py rename to src/cfnlint/rules/resources/cloudwatch/AlarmPeriod.py index d7391531db..a330851934 100644 --- a/src/cfnlint/rules/resources/cloudwatch/AlarmAwsNamespacePeriod.py +++ b/src/cfnlint/rules/resources/cloudwatch/AlarmPeriod.py @@ -8,26 +8,24 @@ from typing import Any import cfnlint.data.schemas.extensions.aws_cloudwatch_alarm -from cfnlint.jsonschema import ValidationError +from cfnlint.jsonschema.exceptions import ValidationError from cfnlint.rules.jsonschema.CfnLintJsonSchema import CfnLintJsonSchema, SchemaDetails -class AlarmAwsNamespacePeriod(CfnLintJsonSchema): +class AlarmPeriod(CfnLintJsonSchema): id = "E3615" - shortdesc = "Validate CloudWatch Alarm using AWS metrics has a correct period" - description = ( - "Period < 60 not supported for namespaces with the following prefix: AWS/" - ) - tags = ["resources"] + shortdesc = "Validate the period is a valid value" + description = "Valid values are 10, 30, 60, and any multiple of 60." + tags = ["resources", "cloudwatch"] def __init__(self) -> None: super().__init__( keywords=["Resources/AWS::CloudWatch::Alarm/Properties"], schema_details=SchemaDetails( module=cfnlint.data.schemas.extensions.aws_cloudwatch_alarm, - filename="aws_namespace_period.json", + filename="period.json", ), ) def message(self, instance: Any, err: ValidationError) -> str: - return err.message + return f"{err.instance!r} is not one of [10, 30, 60] or a multiple of 60" diff --git a/test/unit/rules/resources/cloudwatch/test_alarm_aws_namespace_period.py b/test/unit/rules/resources/cloudwatch/test_alarm_aws_namespace_period.py deleted file mode 100644 index 142fcc4d8d..0000000000 --- a/test/unit/rules/resources/cloudwatch/test_alarm_aws_namespace_period.py +++ /dev/null @@ -1,71 +0,0 @@ -""" -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.cloudwatch.AlarmAwsNamespacePeriod import ( - AlarmAwsNamespacePeriod, -) - - -@pytest.fixture(scope="module") -def rule(): - rule = AlarmAwsNamespacePeriod() - yield rule - - -@pytest.mark.parametrize( - "instance,expected", - [ - ( - { - "Namespace": "AWS/foo", - "Period": 60, - }, - [], - ), - ( - [], # wrong type - [], - ), - ( - { - "Namespace": "AWS/foo", - "Period": {"Ref": "Period"}, # ignore when object - }, - [], - ), - ( - { - "Namespace": {"Ref": "Namespace"}, # ignore when object - "Period": 30, - }, - [], - ), - ( - { - "Namespace": "AWS/foo", - "Period": 30, - }, - [ - ValidationError( - "30 is less than the minimum of 60", - rule=AlarmAwsNamespacePeriod(), - path=deque(["Period"]), - validator="minimum", - schema={"minimum": 60, "type": ["number", "string"]}, - schema_path=deque(["then", "properties", "Period", "minimum"]), - ) - ], - ), - ], -) -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/cloudwatch/test_alarm_period.py b/test/unit/rules/resources/cloudwatch/test_alarm_period.py new file mode 100644 index 0000000000..0d61f67e35 --- /dev/null +++ b/test/unit/rules/resources/cloudwatch/test_alarm_period.py @@ -0,0 +1,116 @@ +""" +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.cloudwatch.AlarmPeriod import AlarmPeriod + + +@pytest.fixture(scope="module") +def rule(): + rule = AlarmPeriod() + yield rule + + +@pytest.mark.parametrize( + "instance,expected", + [ + ( + { + "Period": 60, + }, + [], + ), + ( + [], # wrong type + [], + ), + ( + { + "Period": {"Ref": "Period"}, # ignore when object + }, + [], + ), + ( + { + "Period": 30, + }, + [], + ), + ( + { + "Period": 600, + }, + [], + ), + ( + { + "Period": 45, + }, + [ + ValidationError( + "45 is not one of [10, 30, 60] or a multiple of 60", + rule=AlarmPeriod(), + path=deque(["Period"]), + validator="enum", + schema_path=deque(["then", "properties", "Period", "then", "enum"]), + ) + ], + ), + ( + { + "Period": "45", + }, + [ + ValidationError( + "'45' is not one of [10, 30, 60] or a multiple of 60", + rule=AlarmPeriod(), + path=deque(["Period"]), + validator="enum", + schema_path=deque(["then", "properties", "Period", "then", "enum"]), + ) + ], + ), + ( + { + "Period": 121, + }, + [ + ValidationError( + "121 is not one of [10, 30, 60] or a multiple of 60", + rule=AlarmPeriod(), + path=deque(["Period"]), + validator="multipleOf", + schema_path=deque( + ["then", "properties", "Period", "else", "multipleOf"] + ), + ) + ], + ), + ( + { + "Period": "121", + }, + [ + ValidationError( + "'121' is not one of [10, 30, 60] or a multiple of 60", + rule=AlarmPeriod(), + path=deque(["Period"]), + validator="multipleOf", + schema_path=deque( + ["then", "properties", "Period", "else", "multipleOf"] + ), + ) + ], + ), + ], +) +def test_validate(instance, expected, rule, validator): + errs = list(rule.validate(validator, "", instance, {})) + + assert errs == expected, f"Expected {expected} got {errs}"