Support parsing multiple files #331

kddejong · 2018-09-07T16:47:13Z

Issue #, if available:

Description of changes:

Supports linting multiple files

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

kddejong · 2018-09-15T12:07:30Z

A few things may need to get done before merging this one.

Make sure there is no adverse affect to the people using cfn-lint as a module
Double check that the IDE plugins still work

cmmeyer · 2018-09-17T19:40:21Z

I noticed you've removed the WIP tag from this PR -- is it ready for review and merge?

kddejong · 2018-09-19T11:02:41Z

yea I think this is about ready. In the past there has a been a few argparse things that trip me up. I'm hoping I can get a few people to put their eyes on it to make sure I didn't miss anything.

cmmeyer · 2018-09-19T15:06:50Z

@alexjurkiewicz and @adamchainz -- would either of you be willing to do a quick review/test of this PR to see if it helps with your multi-file use cases?

cmmeyer

I can only get this to work if I run it as:
cfn-lint new-temp.yaml with-private-subnets.json

I can't get it to parse multiple files using -template/-t. I tried:

cfn-lint -template new-temp.yaml -template with-private-subnets.json
cfn-lint -t new-temp.yaml -t with-private-subnets.json
cfn-lint -t new-temp.yaml,with-private-subnets.json

In all cases it only parses one file or fails outright. Does this only work with bare filenames?

kddejong · 2018-09-19T15:47:08Z

It should work for both... I'll check in on this.

cmmeyer

Ok, almost there. Now these all work:

cfn-lint -template new-temp.yaml with-private-subnets.json
cfn-lint -t new-temp.yaml,with-private-subnets.json
cfn-lint -t new-temp.yaml -t with-private-subnets.json
cfn-lint new-temp.yaml with-private-subnets.json

...but these fail:

cfn-lint -template new-temp.yaml -template with-private-subnets.json
cfn-lint new-temp.yaml,with-private-subnets.json


ps -- I have not tested with any other arguments (rules, whitelisting, etc.)

adamchainz · 2018-09-20T10:32:28Z

@cmmeyer comma separated lists shouldn't really pass since commas are allowed in filenames, and there's already a way of passing multiple with space separation

adamchainz · 2018-09-20T10:35:51Z

Takes my lint of 73 templates from 53 seconds to 4 seconds!

But one thing that might be worth documenting is using the -- separator, since e.g. --regions consumes multiple arguments, I have to do cfn-lint --regions us-east-1 -- cloudformation/*.yml to make it work. The error message is unclear that there is no template passed without the --.

cmmeyer · 2018-09-20T15:35:31Z

It would be nice to print each filename in debug mode -- I got an exception when I pointed the linter at a random directory and it was hard to figure out which file caused the exception.

(For what it's worth, the exception was when we hit a binary file(.zip). I'll create a separate issue for that)

Traceback (most recent call last):
  File "/usr/local/bin/cfn-lint", line 11, in <module>
    load_entry_point('cfn-lint==0.7.2', 'console_scripts', 'cfn-lint')()
  File "/usr/local/lib/python3.6/site-packages/cfn_lint-0.7.2-py3.6.egg/cfnlint/__main__.py", line 30, in main
    (template, rules, template_matches) = cfnlint.core.get_template_rules(filename, args)
  File "/usr/local/lib/python3.6/site-packages/cfn_lint-0.7.2-py3.6.egg/cfnlint/core.py", line 245, in get_template_rules
    (template, matches) = cfnlint.decode.decode(filename, args.ignore_bad_template)
  File "/usr/local/lib/python3.6/site-packages/cfn_lint-0.7.2-py3.6.egg/cfnlint/decode/__init__.py", line 40, in decode
    template = cfnlint.decode.cfn_yaml.load(filename)
  File "/usr/local/lib/python3.6/site-packages/cfn_lint-0.7.2-py3.6.egg/cfnlint/decode/cfn_yaml.py", line 206, in load
    return loads(fp.read(), filename)
  File "/usr/local/lib/python3.6/site-packages/cfn_lint-0.7.2-py3.6.egg/cfnlint/decode/cfn_yaml.py", line 191, in loads
    loader = MarkedLoader(yaml_string, fname)
  File "/usr/local/lib/python3.6/site-packages/cfn_lint-0.7.2-py3.6.egg/cfnlint/decode/cfn_yaml.py", line 140, in __init__
    Reader.__init__(self, stream)
  File "/usr/local/lib/python3.6/site-packages/yaml/reader.py", line 74, in __init__
    self.check_printable(stream)
  File "/usr/local/lib/python3.6/site-packages/yaml/reader.py", line 144, in check_printable
    'unicode', "special characters are not allowed")
yaml.reader.ReaderError: unacceptable character #x0000: special characters are not allowed
  in "<unicode string>", position 0

kddejong · 2018-09-21T01:39:27Z

Thanks for the feedback. Added logging for the filename information and did some extra items around the comma/space parsing. Also added some more documentation.

alexjurkiewicz

Great work mate! This is very exciting :)

I have tested locally and no extra bugs from what's above. A few review comments you can accept or ignore

alexjurkiewicz · 2018-09-21T01:56:59Z

README.md

-`cfn-lint --template template.yaml`
+- `cfn-lint template.yaml`
+- `cfn-lint --template template.yaml`
+- `cfn-lint -t template.yaml`


Is it worth documenting all these different ways to do the same thing so high up in the README? It will take a lot of space. If you want to show both positional and argument -t and --template are wholly redundant

To expand on this, for quick start people probably just need to see cfn-lint template.yaml. Other forms can be left for the --help output.

Removed --template and moved up the usage section in the README so it isn't so far down. I will look into the adding information into the help. I think there was a request to make sure we showed the -- in the README so we didn't cause confusion when using the default parameters with other parameters. The -t examples can be moved though.

src/cfnlint/core.py

alexjurkiewicz · 2018-09-21T03:50:59Z

src/cfnlint/decode/__init__.py

+def create_match_file_error(filename, msg):
+    """Create a Match for a parser error"""
+    return cfnlint.Match(
+        1, 1, 1, 2, filename, cfnlint.ParseError(), message=msg)


this function call is a bit cryptic. Should it used named parameters?

Updated. Thanks!

alexjurkiewicz · 2018-09-23T23:31:39Z

src/cfnlint/core.py

@@ -104,10 +104,11 @@ def create_parser():

    # Alllow the template to be passes as an optional or a positional argument
    standard.add_argument(
-        'template', nargs='?', help='The CloudFormation template to be linted')
+        'template', nargs='*', help='The CloudFormation template to be linted')


This is a real nitpick, but should probably call this variable 'templates' (with a metavar of TEMPLATE).

alexjurkiewicz · 2018-09-27T21:02:17Z

Top stuff guys!!

…

On Thu., 27 Sep. 2018, 03:53 Chuck Meyer, ***@***.***> wrote: Merged #331 <#331> into master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#331 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXKdfpdXIHU9y_LkKhGrNdCWR-7Vgwhks5ue78pgaJpZM4WfJk3> .

adamchainz · 2018-09-28T08:02:25Z

🐇

Saichovsky · 2018-10-02T15:27:16Z

pip install git+git://github.com/awslabs/cfn-python-lint.git works, but when I run pre-commit-validate-config .pre-commit-config.yaml, it fails with the following Traceback:

Traceback (most recent call last):
  File "/usr/bin/pre-commit-validate-config", line 11, in <module>
    sys.exit(validate_config_main())
  File "/usr/lib/python2.7/site-packages/pre_commit/clientlib.py", line 190, in validate_config_main
    load_config(filename)
  File "/usr/lib/python2.7/site-packages/cfgv.py", line 347, in load_from_filename
    contents = f.read()
  File "/usr/lib64/python2.7/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 386: ordinal not in range(128)

I'm installing from Github because the version that is on pypi does not seem to support multiple files. I feel that it is not up-to-date.

EDIT: There was a funny character on the last line of my config file which became visible once I viewed the file in vi. However, I think we have an uncovered scenario here, resulting in an uncaught exception being thrown.

adamchainz · 2018-10-02T17:02:00Z

@Saichovsky that doesn't seem related to this at all, none of the stack trace frames are in cfn-python-lint. From the stacktrace I guess you have non-unicode characters in your configuration file.

Saichovsky · 2018-11-18T15:39:04Z

@adamchainz Indeed. Reported on the wrong repo. Apologies.

kddejong added the WIP work in progress -- DO NOT MERGE label Sep 7, 2018

kddejong force-pushed the Feature/LintMultipleFiles branch from 0ff6e0b to e26201d Compare September 15, 2018 12:02

kddejong removed the WIP work in progress -- DO NOT MERGE label Sep 15, 2018

kddejong mentioned this pull request Sep 16, 2018

Multiple filenames accepted but not linted #323

Closed

cmmeyer suggested changes Sep 19, 2018

View reviewed changes

kddejong force-pushed the Feature/LintMultipleFiles branch 3 times, most recently from 6f50ce4 to 5203584 Compare September 19, 2018 19:14

cmmeyer suggested changes Sep 19, 2018

View reviewed changes

kddejong force-pushed the Feature/LintMultipleFiles branch from 25618b8 to 57a1c1e Compare September 21, 2018 01:38

alexjurkiewicz approved these changes Sep 21, 2018

View reviewed changes

kddejong force-pushed the Feature/LintMultipleFiles branch from 57a1c1e to f9643e5 Compare September 21, 2018 19:45

alexjurkiewicz reviewed Sep 23, 2018

View reviewed changes

kddejong force-pushed the Feature/LintMultipleFiles branch from f9643e5 to 31e9825 Compare September 26, 2018 07:01

Support parsing multiple files

9794c62

kddejong force-pushed the Feature/LintMultipleFiles branch from 31e9825 to 9794c62 Compare September 26, 2018 07:09

cmmeyer approved these changes Sep 26, 2018

View reviewed changes

cmmeyer merged commit c343cd7 into aws-cloudformation:master Sep 26, 2018

cmmeyer mentioned this pull request Sep 27, 2018

Support checking multiple files at once #149

Closed

kddejong deleted the Feature/LintMultipleFiles branch November 18, 2018 15:47

PatMyron mentioned this pull request Jun 14, 2020

Feature request - multiple files martysweet/cfn-lint#218

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support parsing multiple files #331

Support parsing multiple files #331

kddejong commented Sep 7, 2018

kddejong commented Sep 15, 2018

cmmeyer commented Sep 17, 2018

kddejong commented Sep 19, 2018

cmmeyer commented Sep 19, 2018

cmmeyer left a comment

kddejong commented Sep 19, 2018

cmmeyer left a comment

adamchainz commented Sep 20, 2018

adamchainz commented Sep 20, 2018

cmmeyer commented Sep 20, 2018

kddejong commented Sep 21, 2018

alexjurkiewicz left a comment

alexjurkiewicz Sep 21, 2018

alexjurkiewicz Sep 23, 2018

kddejong Sep 26, 2018

alexjurkiewicz Sep 21, 2018

kddejong Sep 21, 2018

alexjurkiewicz Sep 23, 2018

alexjurkiewicz commented Sep 27, 2018 via email

adamchainz commented Sep 28, 2018

Saichovsky commented Oct 2, 2018 •

edited

adamchainz commented Oct 2, 2018

Saichovsky commented Nov 18, 2018

Support parsing multiple files #331

Support parsing multiple files #331

Conversation

kddejong commented Sep 7, 2018

kddejong commented Sep 15, 2018

cmmeyer commented Sep 17, 2018

kddejong commented Sep 19, 2018

cmmeyer commented Sep 19, 2018

cmmeyer left a comment

Choose a reason for hiding this comment

kddejong commented Sep 19, 2018

cmmeyer left a comment

Choose a reason for hiding this comment

adamchainz commented Sep 20, 2018

adamchainz commented Sep 20, 2018

cmmeyer commented Sep 20, 2018

kddejong commented Sep 21, 2018

alexjurkiewicz left a comment

Choose a reason for hiding this comment

alexjurkiewicz Sep 21, 2018

Choose a reason for hiding this comment

alexjurkiewicz Sep 23, 2018

Choose a reason for hiding this comment

kddejong Sep 26, 2018

Choose a reason for hiding this comment

alexjurkiewicz Sep 21, 2018

Choose a reason for hiding this comment

kddejong Sep 21, 2018

Choose a reason for hiding this comment

alexjurkiewicz Sep 23, 2018

Choose a reason for hiding this comment

alexjurkiewicz commented Sep 27, 2018 via email

adamchainz commented Sep 28, 2018

Saichovsky commented Oct 2, 2018 • edited

adamchainz commented Oct 2, 2018

Saichovsky commented Nov 18, 2018

Saichovsky commented Oct 2, 2018 •

edited