Skip to content

Commit 6c60641

Browse files
authored
Validate rate periods in rule E3027 (#3017)
1 parent 2660e6c commit 6c60641

File tree

3 files changed

+49
-23
lines changed

3 files changed

+49
-23
lines changed

Diff for: src/cfnlint/rules/resources/events/RuleScheduleExpression.py

+36-22
Original file line numberDiff line numberDiff line change
@@ -25,29 +25,43 @@ def check_rate(self, value, path):
2525
rate_expression = value[value.find("(") + 1 : value.find(")")]
2626

2727
if not rate_expression:
28-
matches.append(
29-
RuleMatch(path, "Rate value of ScheduleExpression cannot be empty")
30-
)
31-
else:
32-
# Rate format: rate(Value Unit)
33-
items = rate_expression.split(" ")
34-
35-
if len(items) != 2:
36-
message = "Rate expression must contain 2 elements (Value Unit), rate contains {} elements"
37-
matches.append(RuleMatch(path, message.format(len(items))))
38-
else:
39-
# Check the Value
40-
if not items[0].isdigit():
41-
message = "Rate Value ({}) should be of type Integer."
42-
extra_args = {
43-
"actual_type": type(items[0]).__name__,
44-
"expected_type": int.__name__,
45-
}
46-
matches.append(
47-
RuleMatch(path, message.format(items[0]), **extra_args)
48-
)
28+
return [RuleMatch(path, "Rate value of ScheduleExpression cannot be empty")]
29+
30+
# Rate format: rate(Value Unit)
31+
items = rate_expression.split(" ")
32+
33+
if len(items) != 2:
34+
message = "Rate expression must contain 2 elements (Value Unit), rate contains {} elements"
35+
matches.append(RuleMatch(path, message.format(len(items))))
36+
return [RuleMatch(path, message.format(len(items)))]
37+
38+
# Check the Value
39+
if not items[0].isdigit():
40+
message = "Rate Value ({}) should be of type Integer."
41+
extra_args = {
42+
"actual_type": type(items[0]).__name__,
43+
"expected_type": int.__name__,
44+
}
45+
return [RuleMatch(path, message.format(items[0]), **extra_args)]
46+
47+
if float(items[0]) <= 0:
48+
return [
49+
RuleMatch(path, f"Rate Value {items[0]!r} should be greater than 0.")
50+
]
51+
52+
if float(items[0]) <= 1:
53+
valid_periods = ["minute", "hour", "day"]
54+
elif float(items[0]) > 1:
55+
valid_periods = ["minutes", "hours", "days"]
56+
# Check the Unit
57+
if items[1] not in valid_periods:
58+
return [
59+
RuleMatch(
60+
path, f"Rate Unit {items[1]!r} should be one of {valid_periods!r}."
61+
)
62+
]
4963

50-
return matches
64+
return []
5165

5266
def check_cron(self, value, path):
5367
"""Check Cron configuration"""

Diff for: test/fixtures/templates/bad/resources/events/rule_schedule_expression.yaml

+12
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,15 @@ Resources:
3333
Type: AWS::Events::Rule
3434
Properties:
3535
ScheduleExpression: "cron(* 1 * * * *)" # specify the Day-of-month and Day-of-week fields in the same cron expression
36+
MyScheduledRule9:
37+
Type: AWS::Events::Rule
38+
Properties:
39+
ScheduleExpression: "rate(1 minutes)" # Value of 1 should be singular. 'minute' not 'minutes'
40+
MyScheduledRule10:
41+
Type: AWS::Events::Rule
42+
Properties:
43+
ScheduleExpression: "rate(2 hour)" # Value of 2 should be plural. 'hours' not `hour`
44+
MyScheduledRule11:
45+
Type: AWS::Events::Rule
46+
Properties:
47+
ScheduleExpression: "rate(0 hour)" # Value has to be greater than 0

Diff for: test/unit/rules/resources/events/test_rule_schedule_expression.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,5 @@ def test_file_negative_alias(self):
2828
"""Test failure"""
2929
self.helper_file_negative(
3030
"test/fixtures/templates/bad/resources/events/rule_schedule_expression.yaml",
31-
8,
31+
11,
3232
)

0 commit comments

Comments
 (0)