From 6c6064128185040c67d79a7f3340cddd0b7c6825 Mon Sep 17 00:00:00 2001
From: Kevin DeJong <kddejong@amazon.com>
Date: Thu, 18 Jan 2024 12:49:58 -0800
Subject: [PATCH] Validate rate periods in rule E3027 (#3017)

---
 .../events/RuleScheduleExpression.py          | 58 ++++++++++++-------
 .../events/rule_schedule_expression.yaml      | 12 ++++
 .../events/test_rule_schedule_expression.py   |  2 +-
 3 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/src/cfnlint/rules/resources/events/RuleScheduleExpression.py b/src/cfnlint/rules/resources/events/RuleScheduleExpression.py
index 4deef8839b..2f7a06f304 100644
--- a/src/cfnlint/rules/resources/events/RuleScheduleExpression.py
+++ b/src/cfnlint/rules/resources/events/RuleScheduleExpression.py
@@ -25,29 +25,43 @@ def check_rate(self, value, path):
         rate_expression = value[value.find("(") + 1 : value.find(")")]
 
         if not rate_expression:
-            matches.append(
-                RuleMatch(path, "Rate value of ScheduleExpression cannot be empty")
-            )
-        else:
-            # Rate format: rate(Value Unit)
-            items = rate_expression.split(" ")
-
-            if len(items) != 2:
-                message = "Rate expression must contain 2 elements (Value Unit), rate contains {} elements"
-                matches.append(RuleMatch(path, message.format(len(items))))
-            else:
-                # Check the Value
-                if not items[0].isdigit():
-                    message = "Rate Value ({}) should be of type Integer."
-                    extra_args = {
-                        "actual_type": type(items[0]).__name__,
-                        "expected_type": int.__name__,
-                    }
-                    matches.append(
-                        RuleMatch(path, message.format(items[0]), **extra_args)
-                    )
+            return [RuleMatch(path, "Rate value of ScheduleExpression cannot be empty")]
+
+        # Rate format: rate(Value Unit)
+        items = rate_expression.split(" ")
+
+        if len(items) != 2:
+            message = "Rate expression must contain 2 elements (Value Unit), rate contains {} elements"
+            matches.append(RuleMatch(path, message.format(len(items))))
+            return [RuleMatch(path, message.format(len(items)))]
+
+        # Check the Value
+        if not items[0].isdigit():
+            message = "Rate Value ({}) should be of type Integer."
+            extra_args = {
+                "actual_type": type(items[0]).__name__,
+                "expected_type": int.__name__,
+            }
+            return [RuleMatch(path, message.format(items[0]), **extra_args)]
+
+        if float(items[0]) <= 0:
+            return [
+                RuleMatch(path, f"Rate Value {items[0]!r} should be greater than 0.")
+            ]
+
+        if float(items[0]) <= 1:
+            valid_periods = ["minute", "hour", "day"]
+        elif float(items[0]) > 1:
+            valid_periods = ["minutes", "hours", "days"]
+        # Check the Unit
+        if items[1] not in valid_periods:
+            return [
+                RuleMatch(
+                    path, f"Rate Unit {items[1]!r} should be one of {valid_periods!r}."
+                )
+            ]
 
-        return matches
+        return []
 
     def check_cron(self, value, path):
         """Check Cron configuration"""
diff --git a/test/fixtures/templates/bad/resources/events/rule_schedule_expression.yaml b/test/fixtures/templates/bad/resources/events/rule_schedule_expression.yaml
index 13d2d2fb08..ac66f0498a 100644
--- a/test/fixtures/templates/bad/resources/events/rule_schedule_expression.yaml
+++ b/test/fixtures/templates/bad/resources/events/rule_schedule_expression.yaml
@@ -33,3 +33,15 @@ Resources:
     Type: AWS::Events::Rule
     Properties:
       ScheduleExpression: "cron(* 1 * * * *)" # specify the Day-of-month and Day-of-week fields in the same cron expression
+  MyScheduledRule9:
+    Type: AWS::Events::Rule
+    Properties:
+      ScheduleExpression: "rate(1 minutes)" # Value of 1 should be singular. 'minute' not 'minutes'
+  MyScheduledRule10:
+    Type: AWS::Events::Rule
+    Properties:
+      ScheduleExpression: "rate(2 hour)" # Value of 2 should be plural. 'hours' not `hour`
+  MyScheduledRule11:
+    Type: AWS::Events::Rule
+    Properties:
+      ScheduleExpression: "rate(0 hour)" # Value has to be greater than 0
diff --git a/test/unit/rules/resources/events/test_rule_schedule_expression.py b/test/unit/rules/resources/events/test_rule_schedule_expression.py
index a4fe7b6002..f8a55f794c 100644
--- a/test/unit/rules/resources/events/test_rule_schedule_expression.py
+++ b/test/unit/rules/resources/events/test_rule_schedule_expression.py
@@ -28,5 +28,5 @@ def test_file_negative_alias(self):
         """Test failure"""
         self.helper_file_negative(
             "test/fixtures/templates/bad/resources/events/rule_schedule_expression.yaml",
-            8,
+            11,
         )