aws-cloudformation / cfn-lint

CloudFormation Linter
MIT No Attribution
2.46k stars 597 forks source link

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

Open DeflateAwning opened 1 week ago

DeflateAwning commented 1 week ago

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 commented 1 week ago

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

kddejong commented 1 week ago

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 commented 1 week ago

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

For example:

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 commented 1 week ago

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 commented 1 week ago

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 commented 8 hours ago

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.