Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail gracefully if no templates match wildcard #3603

Merged

Conversation

thecodingsysadmin
Copy link
Contributor

@thecodingsysadmin thecodingsysadmin commented Aug 16, 2024

Description of changes:

cfn-lint currently fails if there are no Cloudformation templates that
match the pattern specified either via CLI arguments or via config file.

$ cfn-lint "cfn/**/*.y*ml"
2024-08-16 13:41:38,381 - cfnlint.decode.decode - ERROR - Template file not found: cfn/**/*.y*ml
E0000 Template file not found: cfn/**/*.y*ml
cfn/**/*.y*ml:1:1

$ echo $?
2

It appears that when the glob pattern matching does not find any match,
the actual string is appended as a template file.

This PR improves the handling of wildcard templates by ensuring only
matched templates are added to be linted by cfn-lint and would
gracefully exit without an output message.

If run with debug switch, it lists the Cloudformation templates found by the glob.

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

src/cfnlint/runner.py Show resolved Hide resolved
# If templates were specified, but didn't create any file matches, print out message and exit
template = self.config.get_input_templates
if template:
print(f"\nNo matching files found for template arguments: {template}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually wonder if this should just be a raised exception like InvalidRegionException is configured (InvalidTemplateException ?). or a LOGGER.debug statement?

A random print here may cause issues with people parsing the output as json or some of other formats.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in hindsight, I can the print to the _glob_filenames function inside config.py and change it to a warn message?

Currently if no templates are found, it'll output the usage - What would you like the behaviour of cfn-lint to be in this case?

    def _glob_filenames(self, filenames: Sequence[str]) -> list[str]:
        # handle different shells and Config files
        # some shells don't expand * and configparser won't expand wildcards
        all_filenames = []

        for filename in filenames:
            add_filenames = glob.glob(filename, recursive=True)

            if isinstance(add_filenames, list):
                all_filenames.extend(add_filenames)
            else:
                all_filenames.append(filename)
+      if filenames and not all_filenames:
+          LOGGER.warn(f"No matching files found for {filenames}")
        return sorted(list(map(str, map(Path, all_filenames))))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, good question. Thinking through this. Always like some outside feedback here to @benbridts ?

I'm not sure we should print the usage as they technically used it correctly they just provided a valid path doesn't have any files (or a bad path).

This will also depend on a little by the shell a bad path will get caught by ZSH but not in bash (or at least my configuration of both shells).

This is what I'm seeing now. I don't think the E0000 makes sense here either. I'm thinking we should send the logging message and then continue gracefully. If the list is empty the list is empty and we just exit.

2024-08-19 10:28:55,415 - cfnlint.decode.decode - ERROR - Template file not found: ~/Downloads/templates1/**/*.yaml
E0000 Template file not found: ~/Downloads/templates1/**/*.yaml
~/Downloads/templates1/**/*.yaml:1:1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the latest changes, I've added a debug statement:

$ cfn-lint -D "cfn/**/*.y*ml"
2024-08-21 10:52:56,047 - cfnlint - DEBUG - List of Cloudformation Templates to lint: [] from ['cfn/**/*.y*ml']

@thecodingsysadmin thecodingsysadmin force-pushed the fix_no_matching_templates branch from 0f6ada9 to 0467c14 Compare August 21, 2024 00:41
@thecodingsysadmin
Copy link
Contributor Author

thecodingsysadmin commented Aug 21, 2024

@kddejong I've refactored the PR a bit and tested the different scenarios.

Config file:

$ cfn-lint
$ echo $?
0
$ cfn-lint -D
2024-08-21 11:02:07,399 - cfnlint - DEBUG - List of Cloudformation Templates to lint: [] from ['cfn/**/*.y*ml']

Piped template:

$ cat dummy.yaml | cfn-lint
$ echo $?
0

CLI args

$ cfn-lint "cfn/**/*.y*ml"
$ echo $?
0

CLI args via -t:

$ cfn-lint -t "cfn/**/*.y*ml"
$ echo $?
0

Tox tests:

  py38: SKIP (0.22 seconds)
  py39: OK (381.87=setup[0.82]+cmd[36.56,332.57,11.92] seconds)
  py310: OK (341.07=setup[0.10]+cmd[37.33,297.78,5.86] seconds)
  py311: OK (285.37=setup[0.09]+cmd[36.09,244.84,4.36] seconds)
  py312: OK (258.85=setup[0.05]+cmd[23.21,227.20,8.38] seconds)
  style: OK (352.48=setup[23.35]+cmd[85.65,239.23,4.25] seconds)
  congratulations :) (1621.68 seconds)

@thecodingsysadmin thecodingsysadmin force-pushed the fix_no_matching_templates branch 2 times, most recently from 3bbb7e8 to cd3b999 Compare September 1, 2024 23:43
*Description of changes:*

`cfn-lint` currently fails if there are no Cloudformation templates that
match the pattern specified either via CLI arguments or via config file.

```
$ cfn-lint "cfn/**/*.y*ml"
2024-08-16 13:41:38,381 - cfnlint.decode.decode - ERROR - Template file not found: cfn/**/*.y*ml
E0000 Template file not found: cfn/**/*.y*ml
cfn/**/*.y*ml:1:1

$ echo $?
2
```

It appears that when the glob pattern matching does not find any match,
the actual string is appended as a template file.

This PR improves the handling of wildcard templates by ensuring only
matched templates are added to be linted by `cfn-lint` and would
gracefully exit without an output message.

If run with debug switch, it lists the Cloudformation templates found by the glob.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
@thecodingsysadmin thecodingsysadmin force-pushed the fix_no_matching_templates branch from cd3b999 to 66a59a8 Compare September 5, 2024 02:35
@thecodingsysadmin
Copy link
Contributor Author

Rebased to latest release: v1.12.3.

Branch tests are passing: https://github.com/thecodingsysadmin/cfn-lint/actions/runs/10712856489

Copy link

codecov bot commented Sep 9, 2024

Codecov Report

Attention: Patch coverage is 87.80488% with 5 lines in your changes missing coverage. Please review.

Project coverage is 93.65%. Comparing base (75bfb28) to head (c94fa49).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/cfnlint/config.py 83.33% 2 Missing and 2 partials ⚠️
src/cfnlint/runner.py 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3603      +/-   ##
==========================================
+ Coverage   93.63%   93.65%   +0.01%     
==========================================
  Files         358      358              
  Lines       12206    12223      +17     
  Branches     2616     2619       +3     
==========================================
+ Hits        11429    11447      +18     
+ Misses        431      429       -2     
- Partials      346      347       +1     
Flag Coverage Δ
unittests 93.59% <82.92%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kddejong kddejong merged commit e2a6c83 into aws-cloudformation:main Sep 9, 2024
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants