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

Running cfn-lint file_that_does_not_exist.yml has exit code 0 and no error message #3827

Closed
DeflateAwning opened this issue Nov 15, 2024 · 8 comments · Fixed by #3862
Closed

Comments

@DeflateAwning
Copy link

CloudFormation Lint Version

cfn-lint 1.19.0

What operating system are you using?

Ubuntu

Describe the bug

Running cfn-lint file_that_does_not_exist.yml should return exit_code > 0, and should ideally give a stderr message that the file doesn't exist.

Currently, the program exists the exact same way as if nothing happened.

Expected behavior

Running cfn-lint file_that_does_not_exist.yml should return exit_code > 0, and should ideally give a stderr message that the file doesn't exist.

Reproduction template

N/A

@kddejong
Copy link
Contributor

I think this is related to #3603. I agree on this. With an individual file this should fail. Looking into it.

@kddejong
Copy link
Contributor

kddejong commented Nov 15, 2024

Since glob returns an empty list if the path doesn't exist or the results are empty we aren't detecting if an individual file is valid. I think the original issue falls into an issue of caveats with how shells work. Certain shells (and configurations) send the glob pattern to as an argument if no files are found. Some shells error before calling cfn-lint (my default shell of zsh does this)

I don't believe there is an easy option for determining if a path is a glob pattern or not (crazy regex patterns may have to be created). The end result seems to be to bring back the functionality prior to #3603. Additionally we can add a parameter to not error if there are no files to lint. At least that customer group would have an option at that point.

This is additionally issue if I mistype the path. For instance dne\**\*.yaml and the question should be is this an error as well? It can be misleading to not know no files were scanned because I mistyped the pattern. Which is why I think the default should be to error.

Understandably the original issue was for a valid path glob pattern just not having any matching files. Which I would also agree shouldn't error but we may have to rely on a new parameter to make that determination since we can't easily determine if the path is valid or not using the glob functionality.

@thecodingsysadmin do you have any thoughts on this? Would you be okay with a new parameter that would not error if the file list is empty?

@DeflateAwning
Copy link
Author

Seems the Posix standard (or at least Ubuntu's chosen implementation of it) returns >0 when globs fail.

For example:

  • cat this_file_does_not_exist.txt -> Return 1.
  • cat this_file*does_not_exist.txt -> Return 1.

Consider this scenario: Say you're relying on this in a deployment pipeline. The file gets renamed by someone. The check will continue to pass, no matter what's in the file. It seems crazy to think anyone would want this behaviour.

@randybasrs
Copy link

Came to see if there were any open issues relating to this behavior change. In our use case, we can check for the existence of the file before linting. We had relied on a non-zero exit code for lint when a file didn't exist but this is not a huge change if the default functionality is intended to stay this way.

I don't know that a single solution will fit all cases because you could easily justify both an error and a graceful exit with 0 linted results. On one hand in my organization we never call lint in a place where we would expect it to return 0 linted files even with a wildcard, so we would expect a failure. You could easily have an org with the reverse though where they mix cloudformation templates with other code in various folders and only some of them might contain templates that need linting.

One thing PowerShell does that I'm not sure I see in linux commands is that there's a general option for cmdlets called ErrorAction that allows you to configure whether the cmdlet will fail silently or throw an exception if it fails. The linux alternative would be that the command always fails and you would pipe the errors to null or something, I suppose.

Glad the issue already existed, thanks for looking at it. I'll keep an eye on this issue before committing any changes to address this behavior change.

@DeflateAwning
Copy link
Author

Regarding PowerShell's ErrorAction, bash has set -e which enables "fail on error". By default, bash scripts "steamroll" (continue on errors, just with a >0 return code).

@randybasrs
Copy link

Any update on a course of action for this issue? We will likely need to modify our pipelines to check for the existence of the file explicitly regardless, but it would be good to put this to rest.

@kddejong
Copy link
Contributor

kddejong commented Dec 9, 2024

I've been working on this. This will probably get done this week. I agree we should error on this since this was how it worked prior to #3603 . I want to offer some compatibility for the people relying on that PR. They will be switched to a parameter

@thecodingsysadmin
Copy link
Contributor

thecodingsysadmin commented Dec 12, 2024

hi apologies for this.

I should've kept an eye on my @ s a bit more. I'll have a look at the changes made in #3862 and see if we can satisfy both use cases.

Perhaps with an additional option if @kddejong is OK with that?

EDIT: thanks for the making the changes there, having the extra parameter would suit both use cases.

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 a pull request may close this issue.

4 participants