warpnet / salt-lint

A command-line utility that checks for best practices in SaltStack.
https://salt-lint.readthedocs.io/en/latest/
MIT License
150 stars 39 forks source link

salt-lint lints a phantom temporary file when run by another program #302

Open matthewsht opened 1 year ago

matthewsht commented 1 year ago

Describe the bug I have a python program that runs multiple linters on our salt stack tree. When running salt-lint, salt-lint sees that it has no tty and assumes that stdin contains the file to be linted - it then opens a "phantom" temporary file and adds this to the filenames actually provided on the command line.

To Reproduce Run a "wrapper" program around salt-lint (v0.8.0) and provide 1 or more salt (.sls) files on the command line. salt-lint will lint a phantom temporary file assuming that stdin contains a file.

The following is the output of my wrapper: DEBUG: ** DEBUG: Running a linter: salt-lint DEBUG: ** DEBUG: salt-lint -v -R -r linters /Users/matthewsht/projects/saltstack/main/formulas/foobar/proxy.sls /Users/matthewsht/projects/saltstack/main/formulas/foobar/server.sls ['/opt/homebrew/bin/salt-lint', '-v', '-R', '-r', 'linters', '/Users/matthewsht/projects/saltstack/main/formulas/foobar/proxy.sls', '/Users/matthewsht/projects/saltstack/main/formulas/foobar/server.sls'] Examining /Users/matthewsht/projects/saltstack/main/formulas/foobar/server.sls of type sls Examining /Users/matthewsht/projects/saltstack/main/formulas/foobar/proxy.sls of type sls Examining /var/folders/kg/ylmhpwmn3hb2bdlwl84t6c989ztfk2/T/tmpk_jpw919.sls of type sls

Note that it provided two files to salt-lint: server.sls and proxy.sls: however salt-lint reports that it lints those two files AND a third temp files (tmpk_ipw919.sls)

CODE: This is from: cli.py, line 30 testing for lack of a tty. I don't think thats a sufficient test. It should be lack of a tty AND no files submitted on the command line.

Adding some "hunter" debugging lines to cli.py, I get this output in a run: hunter: Namespace(files=['/Users/matthewsht/projects/saltstack/main/formulas/foobar/proxy.sls', '/Users/matthewsht/projects/saltstack/main/formulas/foobar/server.sls'], listrules=False, rulesdir=['linters'], use_default_rules=True, tags=[], listtags=False, verbosity=1, skip_list=[], colored=False, exclude_paths=[], json=False, severity=False, c=None) hunter: im not a tty hunter: /var/folders/kg/ylmhpwmn3hb2bdlwl84t6c989ztfk2/T/tmparoytg2h.sls Examining /Users/matthewsht/projects/saltstack/main/formulas/foobar/server.sls of type sls Examining /var/folders/kg/ylmhpwmn3hb2bdlwl84t6c989ztfk2/T/tmparoytg2h.sls of type sls Examining /Users/matthewsht/projects/saltstack/main/formulas/foobar/proxy.sls of type sls

we print the options and see the two provided sls files (proxy.sls and server.sls) we then see we're not a tty and invent a phantom sls file for the stdin.

The modified code that generated this output is: options = parser.parse_args(args if args is not None else sys.argv[1:]) print('hunter:', options)

stdin_filename = None
file_names = set(options.files)
checked_files = set()

# Read input from STDIN
if not sys.stdin.isatty():
    print('hunter: im not a tty')
    with tempfile.NamedTemporaryFile('w', suffix='.sls', delete=False) as stdin_tmpfile:
        stdin_tmpfile.write(sys.stdin.read())
        stdin_filename = stdin_tmpfile.name
        print('hunter:', stdin_filename)
        file_names.add(stdin_filename)

Expected behavior salt-lint should lint ONLY lint files (and dirs) provided on the command line when any files/dirs are provided on the command line.

Additional context Add any other context about the problem here.

jbouter commented 1 year ago

Hi @matthewsht! Thanks for your bug report.

I'm fairly certain that v0.9.1 hasn't magically fixed this issue. But just to certain, could you please verify the issue also occurs on the latest release of salt-lint?

Thanks in advance!

eliezerlp commented 1 year ago

I am also experiencing this using the latest version (0.9.2) leveraging the salt-lint Docker image.

Relevant files:

$ find . -type f \( -iname "*.jinja" -or -iname "*.sls" \)
./.ci/.gitlab-ci.template.yml.jinja
./top.sls
./empty.sls

When run from an interactive terminal:

$ echo "Linting all sls/jinja files using $(salt-lint --version)..."
Linting all sls/jinja files using salt-lint 0.9.2...
$ find . -type f \( -iname "*.jinja" -or -iname "*.sls" \) -exec sh -c ' filepath=$(realpath {} | sed "s#$(pwd)/##"); echo Linting: $filepath; salt-lint -c .salt-lint.yml $filepath' \;
Linting: .ci/.gitlab-ci.template.yml.jinja
Examining .ci/.gitlab-ci.template.yml.jinja of type jinja
Linting: top.sls
Examining top.sls of type sls
Linting: empty.sls
Examining empty.sls of type sls

Here is what I get when run within the GitLab CI Docker Runner:

$ echo "Linting all sls/jinja files using $(salt-lint --version)..."
Linting all sls/jinja files using salt-lint 0.9.2...
$ find . -type f \( -iname "*.jinja" -or -iname "*.sls" \) -exec sh -c ' filepath=$(realpath {} | sed "s#$(pwd)/##"); echo Linting: $filepath; salt-lint -c .salt-lint.yml $filepath' \;
Linting: .ci/.gitlab-ci.template.yml.jinja
Examining .ci/.gitlab-ci.template.yml.jinja of type jinja
Examining /tmp/tmpuv0088du.sls of type sls
Linting: empty.sls
Examining empty.sls of type sls
Examining /tmp/tmpq332_k4f.sls of type sls
Linting: top.sls
Examining top.sls of type sls
Examining /tmp/tmp8hwtvrxj.sls of type sls

My initial thought was to add a command line arg that suppresses this behavior for use in CI and other non-tty situations.

Note that I am using a find command since I'd like to lint *.jinja files in addition to the *.sls files (which is what gets picked up by default.