aws-cloudformation / cfn-lint

CloudFormation Linter
MIT No Attribution
2.43k stars 588 forks source link

New Lines in Custom Rules file causes failures #2756

Closed shahamy closed 1 year ago

shahamy commented 1 year ago

CloudFormation Lint Version

cfn-lint 0.77.6

What operating system are you using?

Mac

Describe the bug

Similar to the issue reported in https://github.com/aws-cloudformation/cfn-lint/issues/2069, adding a new blank line in the custom rules file causes cfn-lint to fail

My rules.txt file is the following. Note that 2 is an empty line (mostly just for readability)

  1 AWS::S3::Bucket AccessControl EQUALS Public ERROR
  2
  3 AWS::S3::Bucket WebsiteConfiguration.IndexDocument EQUALS Retains WARN

When i run the following command: cfn-lint -t cft.yml -z rules.txt --format pretty --debug - I get

2023-06-02 17:20:25,429 - cfnlint - DEBUG - Begin linting of file: cft.yml
2023-06-02 17:20:25,799 - cfnlint.rules - DEBUG - Processing Custom Rule Line 1
2023-06-02 17:20:25,800 - cfnlint.rules - DEBUG - Processing Custom Rule Line 2
Traceback (most recent call last):
  File "/usr/local/bin/cfn-lint", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/local/lib/python3.11/site-packages/cfnlint/__main__.py", line 39, in main
    matches = list(cfnlint.core.get_matches(filenames, args))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/cfnlint/core.py", line 169, in get_matches
    (template, rules, errors) = get_template_rules(filename, args)
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/cfnlint/core.py", line 310, in get_template_rules
    _build_rule_cache(args)
  File "/usr/local/lib/python3.11/site-packages/cfnlint/core.py", line 270, in _build_rule_cache
    __CACHED_RULES = cfnlint.core.get_rules(
                     ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/cfnlint/core.py", line 158, in get_rules
    rules.create_from_custom_rules_file(custom_rules)
  File "/usr/local/lib/python3.11/site-packages/cfnlint/rules/__init__.py", line 603, in create_from_custom_rules_file
    custom_rule = cfnlint.rules.custom.make_rule(line, line_number)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/cfnlint/rules/custom/__init__.py", line 113, in make_rule
    "E" + str(rule_id), operator
                        ^^^^^^^^
UnboundLocalError: cannot access local variable 'operator' where it is not associated with a value

Expected behavior

Expected for this to just skip the new line and not parse it and fail.

I assume these if statement needs to be altered to check for line == ""

https://github.com/aws-cloudformation/cfn-lint/blob/431cf4fb9fefdfec25cb4cb69005f4dd064421ba/src/cfnlint/rules/custom/__init__.py#L44-L48

I believe we can also fix #2069 a similar way by checking if the line starts with #

Reproduction template

{
  2   "AWSTemplateFormatVersion" : "2010-09-09",
  3
  4   "Description" : "AWS CloudFormation Sample Template S3_Website_Bucket_With_Retain_On_Delete: Sample template showing how to create a publicly accessible S3 bucket config    ured for website access with a deletion policy of retain on delete. **WARNING** This template creates an S3 bucket that will NOT be deleted when the stack is deleted. You     will be billed for the AWS resources used if you create a stack from this template.",
  5
  6   "Resources" : {
  7     "S3Bucket" : {
  8       "Type" : "AWS::S3::Bucket",
  9       "Properties" : {
 10         "AccessControl" : "PublicRead",
 11         "WebsiteConfiguration" : {
 12           "IndexDocument" : "index.html",
 13           "ErrorDocument" : "error.html"
 14          }
 15       },
 16       "DeletionPolicy" : "Retain"
 17     }
 18   },
 19
 20   "Outputs" : {
 21     "WebsiteURL" : {
 22       "Value" : { "Fn::GetAtt" : [ "S3Bucket", "WebsiteURL" ] },
 23       "Description" : "URL for website hosted on S3"
 24     },
 25     "S3BucketSecureURL" : {
 26       "Value" : { "Fn::Join" : [ "", [ "https://", { "Fn::GetAtt" : [ "S3Bucket", "DomainName" ] } ] ] },
 27       "Description" : "Name of S3 bucket to hold website content"
 28     }
 29   }
shahamy commented 1 year ago

I would like to contribute this fix as my team uses cfn-lint and I would like to add new lines and comments to help explain some of the custom rules i am writing

johntaormina commented 1 year ago

+1 On this issue