Bugfix: Allow comments and new lines in custom rules file (#2757)

* 🐛 Add a check for comments and new lines in custom rules

In the documentation, comments are allowed to start with `#`.
Currently, cfn-lint throws an error of `not in supported operators` which means the comment is being parsed as a custom error.
This is now fixed to allow any amount of comments anywhere in the custom rules file.

New lines are now allowed just for readability.
Currently, cfn-lint throws a traceback error of `UnboundLocalError: cannot access local variable 'operator' where it is not associated with a value` which is not an intuitive error message just for a blank new line.
This is now fixed to allow new lines anywhere in the custom rules file.

* ✅ Add unit tests for comments and whitespace in custom rules.

* ✏️ Update docs for custom rules to allow new lines and comments.

docs/custom_rules.md

@@ -13,7 +13,8 @@ The template used for each custom rules have been included below. Angle brackets
 
 * The ruleID is auto-generated based on the line number. The E9XXX and W9XXX blocks are allocated towards custom rules.
     * As an example, custom rule on line 4 of the rules file which has error level “ERROR” would become E9004
-* Comments are supported through the use of the # symbol at the beginning of a line (e.g `#This is a comment`)
+* Comments are supported through the use of the # symbol at the beginning of a line (e.g `# This is a comment`)
+* New lines are supported throughout the rules file
 * The syntax is "quote-flexible" and will support all permutations shown below
 ```
 AWS::EC2::Instance Property EQUALS "Cloud Formation"
@@ -69,6 +70,7 @@ Pre-Requisites: The Custom Error Message requires an error level to be specified
 A custom error message can be used to override the existing fallback messages. (e.g `Show me this custom message`)
 
 ## Example
+
 This following example shows how a you can create a custom rule.
 
 This rule validates all EC2 instances in a template aren’t using the instance type “p3.2xlarge”.
@@ -84,5 +86,3 @@ AWS::Lambda::Function Environment.Variables.NODE_ENV IS DEFINED
 ```
 
 To include this rules, include your custom rules text file using the `-z custom_rules.txt` argument when running cfn-lint.
-
-

src/cfnlint/rules/custom/__init__.py

@@ -42,73 +42,79 @@ def process_sets(raw_value):
         return raw_value
 
     line = line.rstrip()
-    rule_id = lineNumber + 9000
-    line = line.split(" ", 3)
-    error_level = "E"
-    if len(line) == 4:
-        resourceType = line[0]
-        prop = line[1]
-        operator = line[2]
-        value = None
-        error_message = None
-        if "WARN" in line[3]:
-            error_level = "W"
-            value, error_message = set_arguments(line[3], "WARN")
-        elif "ERROR" in line[3]:
-            error_level = "E"
-            value, error_message = set_arguments(line[3], "ERROR")
-        else:
-            value = process_sets(line[3])
-            value = get_value(line[3])
+    # check line is not a comment or empty line
+    if not line.startswith("#") and line != "":
+        rule_id = lineNumber + 9000
+        line = line.split(" ", 3)
+        error_level = "E"
+        if len(line) == 4:
+            resourceType = line[0]
+            prop = line[1]
+            operator = line[2]
+            value = None
+            error_message = None
+            if "WARN" in line[3]:
+                error_level = "W"
+                value, error_message = set_arguments(line[3], "WARN")
+            elif "ERROR" in line[3]:
+                error_level = "E"
+                value, error_message = set_arguments(line[3], "ERROR")
+            else:
+                value = process_sets(line[3])
+                value = get_value(line[3])
 
-        if isinstance(value, str):
-            value = value.strip().strip('"')
+            if isinstance(value, str):
+                value = value.strip().strip('"')
 
-        if operator in ["EQUALS", "=="]:
-            return cfnlint.rules.custom.Operators.CreateEqualsRule(
-                error_level + str(rule_id), resourceType, prop, value, error_message
-            )
-        if operator in ["NOT_EQUALS", "!="]:
-            return cfnlint.rules.custom.Operators.CreateNotEqualsRule(
-                error_level + str(rule_id), resourceType, prop, value, error_message
-            )
-        if operator == "REGEX_MATCH":
-            return cfnlint.rules.custom.Operators.CreateRegexMatchRule(
-                error_level + str(rule_id), resourceType, prop, value, error_message
-            )
-        if operator == "IN":
-            return cfnlint.rules.custom.Operators.CreateInSetRule(
-                error_level + str(rule_id), resourceType, prop, value, error_message
-            )
-        if operator == "NOT_IN":
-            return cfnlint.rules.custom.Operators.CreateNotInSetRule(
-                error_level + str(rule_id), resourceType, prop, value, error_message
-            )
-        if operator == ">":
-            return cfnlint.rules.custom.Operators.CreateGreaterRule(
-                error_level + str(rule_id), resourceType, prop, value, error_message
-            )
-        if operator == ">=":
-            return cfnlint.rules.custom.Operators.CreateGreaterEqualRule(
-                error_level + str(rule_id), resourceType, prop, value, error_message
-            )
-        if operator == "<":
-            return cfnlint.rules.custom.Operators.CreateLesserRule(
-                error_level + str(rule_id), resourceType, prop, value, error_message
-            )
-        if operator == "<=":
-            return cfnlint.rules.custom.Operators.CreateLesserEqualRule(
-                error_level + str(rule_id), resourceType, prop, value, error_message
-            )
-        if operator == "IS":
-            if value in ["DEFINED", "NOT_DEFINED"]:
-                return cfnlint.rules.custom.Operators.CreateCustomIsDefinedRule(
+            if operator in ["EQUALS", "=="]:
+                return cfnlint.rules.custom.Operators.CreateEqualsRule(
+                    error_level + str(rule_id), resourceType, prop, value, error_message
+                )
+            if operator in ["NOT_EQUALS", "!="]:
+                return cfnlint.rules.custom.Operators.CreateNotEqualsRule(
+                    error_level + str(rule_id), resourceType, prop, value, error_message
+                )
+            if operator == "REGEX_MATCH":
+                return cfnlint.rules.custom.Operators.CreateRegexMatchRule(
+                    error_level + str(rule_id), resourceType, prop, value, error_message
+                )
+            if operator == "IN":
+                return cfnlint.rules.custom.Operators.CreateInSetRule(
+                    error_level + str(rule_id), resourceType, prop, value, error_message
+                )
+            if operator == "NOT_IN":
+                return cfnlint.rules.custom.Operators.CreateNotInSetRule(
                     error_level + str(rule_id), resourceType, prop, value, error_message
                 )
-            return cfnlint.rules.custom.Operators.CreateInvalidRule(
-                "E" + str(rule_id), f"{operator} {value}"
-            )
+            if operator == ">":
+                return cfnlint.rules.custom.Operators.CreateGreaterRule(
+                    error_level + str(rule_id), resourceType, prop, value, error_message
+                )
+            if operator == ">=":
+                return cfnlint.rules.custom.Operators.CreateGreaterEqualRule(
+                    error_level + str(rule_id), resourceType, prop, value, error_message
+                )
+            if operator == "<":
+                return cfnlint.rules.custom.Operators.CreateLesserRule(
+                    error_level + str(rule_id), resourceType, prop, value, error_message
+                )
+            if operator == "<=":
+                return cfnlint.rules.custom.Operators.CreateLesserEqualRule(
+                    error_level + str(rule_id), resourceType, prop, value, error_message
+                )
+            if operator == "IS":
+                if value in ["DEFINED", "NOT_DEFINED"]:
+                    return cfnlint.rules.custom.Operators.CreateCustomIsDefinedRule(
+                        error_level + str(rule_id),
+                        resourceType,
+                        prop,
+                        value,
+                        error_message,
+                    )
+                return cfnlint.rules.custom.Operators.CreateInvalidRule(
+                    "E" + str(rule_id), f"{operator} {value}"
+                )
 
-    return cfnlint.rules.custom.Operators.CreateInvalidRule(
-        "E" + str(rule_id), operator
-    )
+        return cfnlint.rules.custom.Operators.CreateInvalidRule(
+            "E" + str(rule_id), operator
+        )

test/fixtures/custom_rules/good/custom_rule_perfect.txt

@@ -1,3 +1,4 @@
+#This is a comment to ensure comments in custom rules work
 AWS::IAM::Role AssumeRolePolicyDocument.Version EQUALS "2012-10-17"
 AWS::IAM::Role AssumeRolePolicyDocument.Version IN [2012-10-16,2012-10-17,2012-10-18]
 AWS::IAM::Role AssumeRolePolicyDocument.Version NOT_EQUALS "2012-10-15"
@@ -6,10 +7,12 @@ AWS::IAM::Policy PolicyName EQUALS "root" WARN ABC
 AWS::IAM::Policy PolicyName IN [2012-10-16,root,2012-10-18] ERROR ABC
 AWS::IAM::Policy PolicyName NOT_EQUALS "user" WARN ABC
 AWS::IAM::Policy PolicyName NOT_IN [2012-10-16,2012-11-20,2012-10-18] ERROR ABC
+# Adding a comment in the middle of custom rules and new line for readability
+
 AWS::EC2::Instance BlockDeviceMappings.Ebs.VolumeSize >= 20 WARN
 AWS::EC2::Instance BlockDeviceMappings.Ebs.VolumeSize > 10 ERROR ABC
 AWS::EC2::Instance BlockDeviceMappings.Ebs.VolumeSize <= 50 ERROR DEF
 AWS::EC2::Instance BlockDeviceMappings.Ebs.VolumeSize < 40 WARN ABC
 AWS::CloudFormation::Stack TemplateURL REGEX_MATCH "^https.*$" WARN ABC
 AWS::Lambda::Function Environment.Variables.NODE_ENV IS DEFINED
-AWS::Lambda::Function Environment.Variables.PRIVATE_KEY IS NOT_DEFINED
+AWS::Lambda::Function Environment.Variables.PRIVATE_KEY IS NOT_DEFINED