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

"noqa" comment is ignored when using auto-detection mode #1368

Closed
imomaliev opened this issue Feb 17, 2021 · 9 comments · Fixed by #1474
Closed

"noqa" comment is ignored when using auto-detection mode #1368

imomaliev opened this issue Feb 17, 2021 · 9 comments · Fixed by #1474
Labels

Comments

@imomaliev
Copy link

Summary

"noqa" comment is not working properly when using auto-detection mode

Issue Type
  • Bug Report
Ansible and Ansible Lint details
$ ansible --version
ansible 2.10.5
  config file = None
  configured module search path = ['/Users/batiskaf/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/batiskaf/.local/share/pipx/venvs/ansible/lib/python3.8/site-packages/ansible
  executable location = /Users/batiskaf/.local/bin/ansible
  python version = 3.8.2 (default, Sep 24 2020, 19:37:08) [Clang 12.0.0 (clang-1200.0.32.21)]
$ ansible-lint --version
ansible-lint 5.0.0
  • ansible installation method: pip via pipx
  • ansible-lint installation method: pip via pipx
OS / ENVIRONMENT

macOS 10.15.7

$ uname -v
Darwin Kernel Version 19.6.0: Mon Aug 31 22:12:52 PDT 2020; root:xnu-6153.141.2~1/RELEASE_X86_64
STEPS TO REPRODUCE
$ echo '---
- name: Setup
  hosts: host
  tasks:
      # package-latest: Package installs should not use latest
      - name: Upgrade all yum packages  # noqa package-latest
        yum:
            name: "*"
            state: latest
' > site.yml
ansible-lint -v
Desired Behaviour

Warning package-latest should be ignored

$ ansible-lint -v
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: site.yml
INFO     Discovering files to lint: git ls-files *.yaml *.yml
INFO     Executing syntax check on site.yml (2.51s)
Actual Behaviour

Getting ansible-lint warning

$ ansible-lint -v
INFO     Discovering files to lint: git ls-files *.yaml *.yml
INFO     Discovering files to lint: git ls-files *.yaml *.yml
WARNING  Listing 1 violation(s) that are fatal
package-latest: Package installs should not use latest
site.yml:6 Task/Handler: Upgrade all yum packages

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
  - package-latest  # Package installs should not use latest
Finished with 1 failure(s), 0 warning(s) on 1 files.
@imomaliev imomaliev added the bug label Feb 17, 2021
@ssbarnea
Copy link
Member

@imomaliev are you 100% sure that the noqa comment is on exact line number on which the error is reported from? Double check, try to move it to next and let me know.

In line noqa in YAML is really a PITA as the noqa comes from one parser and the linting errors come from another one.

@imomaliev
Copy link
Author

imomaliev commented Feb 17, 2021

@ssbarnea Hi, ansible-lint itself highlighted me this line.

package-latest: Package installs should not use latest
site.yml:6 Task/Handler: Upgrade all yum package

If I check on previous version and use on same line noqa 403 this check passes

@jpoliv
Copy link

jpoliv commented Feb 21, 2021

Hi,

I'm also seeing this regression/issue - honoring inline noqa - with ansible-lint 5.0.0/1 and I can add some more information; I'm able to trigger the issue by using directories paths instead of full path names of playbooks:

  • triggers the issue: ansible-lint playbooks/subdir/
  • does NOT trigger the issue (i.e. honors the inline noqa) ansible-lint playbooks/subdir/*.yaml

Increasing the verbosity level (-vv) shows that in the first case (using directories paths) the YAML files are not promoted to playbooks and are checked as files of type yaml. In the second case, the YAML files are promoted and checked as files of type playbook. Log messages:

Directories

$ ansible-lint playbooks/subdir/ -vv
DEBUG    Logging initialized to level 10
DEBUG    Options: Namespace(...)
DEBUG    Loading rules from /usr/local/lib/python3.9/site-packages/ansiblelint/rules
INFO     Discovering files to lint: git ls-files -z *.yaml *.yml
DEBUG    Examining playbooks/subdir/play1.yaml of type yaml
DEBUG    Examining playbooks/subdir/play2.yaml of type yaml
DEBUG    Examining playbooks/subdir of type role
WARNING  Listing 1 violation(s) that are fatal
command-instead-of-module: rpm used in place of yum or rpm_key module
...

Playbooks pathnames

$ ansible-lint playbooks/subdir/*.yaml -vv
DEBUG    Logging initialized to level 10
DEBUG    Options: Namespace(...)
DEBUG    Loading rules from /usr/local/lib/python3.9/site-packages/ansiblelint/rules
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: playbooks/subdir/play1.yaml
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: playbooks/subdir/play2.yaml
INFO     Discovering files to lint: git ls-files -z *.yaml *.yml
INFO     Executing syntax check on playbooks/subdir/play1.yaml (1.35s)
INFO     Executing syntax check on playbooks/subdir/play2.yaml (1.35s)
DEBUG    Examining playbooks/subdir/play1.yaml of type playbook
DEBUG    Examining playbooks/subdir/play2.yaml of type playbook
WARNING  Replaced deprecated tag '303' with 'command-instead-of-module' but it will become an error in the future.

Regards,
/jpo

@jpoliv
Copy link

jpoliv commented Feb 21, 2021

Additional information:

  • the commands of the above scenario were executed on the root directory of a git repository
  • renaming the tracked directory prevents playbook files from being discovered/checked (without committing the changes)
$ mv playbooks/subdir/ playbooks/newsubdir/
$ ansible-lint playbooks/newsubdir/ -vv
...
DEBUG    Loading rules from /usr/local/lib/python3.9/site-packages/ansiblelint/rules
INFO     Discovering files to lint: git ls-files -z *.yaml *.yml
DEBUG    Examining playbooks/newpackages of type role
  • no issues running ansible-lint playbooks/newsubdir/*.yaml -vv

@ssbarnea
Copy link
Member

When running in auto-detection mode, the linter looks only at tracked git-files. Anything that is not tracked by git will be ignored.

This means that if you add a new file on disk and you run the linter without arguments, it will ignore the new file.

Dealing with this case can be tricky so I am considering failing to run the linter if git reports dirty status may be seen as an improvement, as users will be less likely to get confused by the results.

@yvespp
Copy link

yvespp commented Mar 9, 2021

I have a similar case:

Directly specifying the file works, it discovers the file correctly as play.

dlw@8be1fabc0b79://rancher$ ansible-lint -vv xzy/test.yml
DEBUG    Logging initialized to level 10
DEBUG    Options: Namespace(colored=True, config_file=None, cwd=PosixPath('/rancher'), display_relative_path=True, exclude_paths=[], extra_vars=None, format='rich', kinds=[{'requirements': '**/meta/requirements.yml'}, {'reno': '**/releasenotes/*/*.{yaml,yml}'}, {'playbook': '**/playbooks/*.{yml,yaml}'}, {'playbook': '**/*playbook*.{yml,yaml}'}, {'role': '**/roles/*/'}, {'tasks': '**/tasks/**/*.{yaml,yml}'}, {'handlers': '**/handlers/*.{yaml,yml}'}, {'vars': '**/{host_vars,group_vars,vars,defaults}/**/*.{yaml,yml}'}, {'meta': '**/meta/main.{yaml,yml}'}, {'yaml': '.config/molecule/config.{yaml,yml}'}, {'requirements': '**/molecule/*/{collections,requirements}.{yaml,yml}'}, {'yaml': '**/molecule/*/{base,molecule}.{yaml,yml}'}, {'requirements': '**/requirements.yml'}, {'playbook': '**/molecule/*/*.{yaml,yml}'}, {'yaml': '**/*.{yaml,yml}'}, {'yaml': '**/.*.{yaml,yml}'}], lintables=['xzy/test.yml'], listrules=False, listtags=False, loop_var_prefix=None, mock_modules=[], mock_roles=[], offline=None, parseable=False, parseable_severity=False, progressive=False, project_dir='.', quiet=False, rulesdir=[], rulesdirs=['/usr/local/lib/python3.7/dist-packages/ansiblelint/rules'], skip_action_validation=True, skip_list=[], tags=[], use_default_rules=False, verbosity=2, version=False, warn_list=['experimental'])
DEBUG    Loading rules from /usr/local/lib/python3.7/dist-packages/ansiblelint/rules
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: xzy/test.yml
INFO     Discovering files to lint: git ls-files -z *.yaml *.yml
INFO     Executing syntax check on xzy/test.yml (0.61s)
DEBUG    Examining xzy/test.yml of type playbook
WARNING  Replaced deprecated tag '301' with 'no-changed-when' but it will become an error in the future.

But only specifying the directory doesn't, it discovers xzy/test.yml as yaml and xzy as role and the error can't be supressed:

dlw@8be1fabc0b79://rancher$ ansible-lint -vv xzy
DEBUG    Logging initialized to level 10
DEBUG    Options: Namespace(colored=True, config_file=None, cwd=PosixPath('/rancher'), display_relative_path=True, exclude_paths=[], extra_vars=None, format='rich', kinds=[{'requirements': '**/meta/requirements.yml'}, {'reno': '**/releasenotes/*/*.{yaml,yml}'}, {'playbook': '**/playbooks/*.{yml,yaml}'}, {'playbook': '**/*playbook*.{yml,yaml}'}, {'role': '**/roles/*/'}, {'tasks': '**/tasks/**/*.{yaml,yml}'}, {'handlers': '**/handlers/*.{yaml,yml}'}, {'vars': '**/{host_vars,group_vars,vars,defaults}/**/*.{yaml,yml}'}, {'meta': '**/meta/main.{yaml,yml}'}, {'yaml': '.config/molecule/config.{yaml,yml}'}, {'requirements': '**/molecule/*/{collections,requirements}.{yaml,yml}'}, {'yaml': '**/molecule/*/{base,molecule}.{yaml,yml}'}, {'requirements': '**/requirements.yml'}, {'playbook': '**/molecule/*/*.{yaml,yml}'}, {'yaml': '**/*.{yaml,yml}'}, {'yaml': '**/.*.{yaml,yml}'}], lintables=['xzy'], listrules=False, listtags=False, loop_var_prefix=None, mock_modules=[], mock_roles=[], offline=None, parseable=False, parseable_severity=False, progressive=False, project_dir='.', quiet=False, rulesdir=[], rulesdirs=['/usr/local/lib/python3.7/dist-packages/ansiblelint/rules'], skip_action_validation=True, skip_list=[], tags=[], use_default_rules=False, verbosity=2, version=False, warn_list=['experimental'])
DEBUG    Loading rules from /usr/local/lib/python3.7/dist-packages/ansiblelint/rules
INFO     Discovering files to lint: git ls-files -z *.yaml *.yml
DEBUG    Examining xzy/test.yml of type yaml
DEBUG    Examining xzy of type role
WARNING  Listing 1 violation(s) that are fatal
no-changed-when: Commands should not change things if nothing needs doing
xzy/test.yml:5 Task/Handler: Get vm info

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-changed-when  # Commands should not change things if nothing needs doing
Finished with 1 failure(s), 0 warning(s) on 2 files.

Content of xzy/test.yml:

- hosts: all
  force_handlers: false
  connection: local
  tasks:
    - name: Get vm info # noqa 301
      command: 'ls' # noqa 301
      register: output # noqa 301

The directory xzy is empty and I did a git add xzy beforehand (which is very weird that I have to do that tbh).

Version:

dlw@8be1fabc0b79://rancher$ ansible-lint --version
ansible-lint 5.0.3rc1 using ansible 2.10.6

@ssbarnea
Copy link
Member

I am closing this because it there were 4 more releases since, and at least some of the mentioned issues were addressed. Please use discussions if you still have issues with latest release and I will open a bug once I get confirmation is not the expected behavior.

@imomaliev
Copy link
Author

Issue is still present. Used same steps from initial post and also created git repository and commit

Ansible and Ansible Lint details
$ ansible --version
ansible 2.10.5
  config file = None
  configured module search path = ['/Users/batiskaf/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/batiskaf/.local/share/pipx/venvs/ansible/lib/python3.8/site-packages/ansible
  executable location = /Users/batiskaf/.local/bin/ansible
  python version = 3.8.2 (default, Sep 24 2020, 19:37:08) [Clang 12.0.0 (clang-1200.0.32.21)]
$ ansible-lint --version
ansible-lint 5.0.4 using ansible 2.10.5
  • ansible installation method: pip via pipx
  • ansible-lint installation method: pip via pipx
OS / ENVIRONMENT

macOS 10.15.7

$ uname -v
Darwin Kernel Version 19.6.0: Mon Aug 31 22:12:52 PDT 2020; root:xnu-6153.141.2~1/RELEASE_X86_64
STEPS TO REPRODUCE
$ mkdir noqa-test && cd noqa-test
$ echo '---
- name: Setup
  hosts: host
  tasks:
      # package-latest: Package installs should not use latest
      - name: Upgrade all yum packages  # noqa package-latest
        yum:
            name: "*"
            state: latest
' > site.yml
$ git init
$ git add site.yml
$ git commit -m 'add site.yml'
$ ansible-lint -v
Desired Behaviour

Warning package-latest should be ignored

$ ansible-lint -v
INFO     Discovering files to lint: git ls-files -z
INFO     Discovering files to lint: git ls-files -z
Actual Behaviour

Getting ansible-lint warning

$ ansible-lint -v
INFO     Discovering files to lint: git ls-files -z
INFO     Discovering files to lint: git ls-files -z
WARNING  Listing 1 violation(s) that are fatal
package-latest: Package installs should not use latest
site.yml:6 Task/Handler: Upgrade all yum packages

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
  - package-latest  # Package installs should not use latest
Finished with 1 failure(s), 0 warning(s) on 1 files.

@ssbarnea
Copy link
Member

Reopening, I confirm it.

@ssbarnea ssbarnea reopened this Mar 18, 2021
ssbarnea added a commit that referenced this issue Mar 18, 2021
Fixes bug where matchtasks() was called for file kinds that may
not be containers for task.

Fixes: #1368
ssbarnea added a commit that referenced this issue Mar 18, 2021
Fixes bug where matchtasks() was called for file kinds that may
not be containers for task.

Fixes: #1368
JimMadge added a commit to alan-turing-institute/data-safe-haven that referenced this issue Mar 23, 2021
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.

4 participants