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

no-tabs also triggers on managed content, not just ansible syntax #1334

Closed
MarkusTeufelberger opened this issue Feb 10, 2021 · 2 comments · Fixed by #1373
Closed

no-tabs also triggers on managed content, not just ansible syntax #1334

MarkusTeufelberger opened this issue Feb 10, 2021 · 2 comments · Fixed by #1373
Labels

Comments

@MarkusTeufelberger
Copy link
Contributor

Summary

When writing a line containing a tab character to a file, ansible-lint complains that the tasks file containing that line contains a tab character.

Issue Type
  • Bug Report
Ansible and Ansible Lint details
ansible-base==2.10.5

ansible-lint==5.0.0
  • ansible installation method: pre-commit
  • ansible-lint installation method: pre-commit
OS / ENVIRONMENT

Linux (Ubuntu 18.04), though I doubt that's really relevant.

STEPS TO REPRODUCE

Lint the following playbook:

---
- hosts: localhost
  gather_facts: false
  tasks:
    - name: Write a line containing tab
      lineinfile:
        dest: /tmp/some_file
        regexp: '^\s*target_line'
        insertafter: '^\s*target_line'
        line: "\tsome_content"
        state: present
Desired Behaviour

No error, since the tab character is escaped(!) as well as part of a "payload". It is not the job of ansible-lint to decide how config files that are managed by Ansible should be formatted. Tab characters in templates or the files folder are not affected as far as I see.

I would really like to keep this error and not disable it for the whole repository (it might be a bit more thorough than yamllint in some places I guess), but if a character is already escaped and part of a managed file (where I can't always choose the syntax or "just use spaces") that's too far reaching in my opinion. At least only trigger on unescaped tabulators.

Actual Behaviour
$ ansible-lint --offline foo.yml 
Added ANSIBLE_ROLES_PATH=roles
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: foo.yml
INFO     Discovering files to lint: git ls-files *.yaml *.yml
INFO     Executing syntax check on foo.yml (1.45s)
WARNING  Listing 1 violation(s) that are fatal
no-tabs: Most files should not contain tabs
foo.yml:5 Task/Handler: Write line containing tab

You can skip specific rules or tags by adding them to your configuration file:
# .ansible-lint
warn_list:  # or 'skip_list' to silence them completely
  - no-tabs  # Most files should not contain tabs
Finished with 1 failure(s), 0 warning(s) on 1 files.

@ssbarnea
Copy link
Member

Yeah, this is known and I am afraid there is no way to avoid it other than adding a # noqa: no-tabs to the problematic line.

This is because the tab presence is identified inside the already loaded YAMl file, so we cannot distinguish between an plain binary tab or an escaped tab \t inside a field, both of them endup as a real tab in the loaded string.

Use of tabs inside yaml files is quite uncommon and I do think that adding specific noqa comments is a small price to pay for preventing accidental introduction of tabs in other places. Obviously that anyone can skip the entire tule.

Maybe someone find a way to address this? ... or at least find a way to reduce the false-positives.

@MarkusTeufelberger
Copy link
Contributor Author

MarkusTeufelberger commented Feb 10, 2021

I would have expected that these formatting things are part of yamllint rather than ansible-lint?

It might be possible to ignore tabs in certain parameters of certain modules at least (e.g. line of lineinfile) and I think with a handful of exceptions there would be already a very high coverage of false positives. Might still cause a few false negatives though?

ssbarnea added a commit that referenced this issue Feb 18, 2021
As we identified some particular cases where use of tabs is perfectly
justified, we add exceptions for these.

Fixes: #1334
ssbarnea added a commit that referenced this issue Feb 18, 2021
As we identified some particular cases where use of tabs is perfectly
justified, we add exceptions for these.

Fixes: #1334
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants