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

Ansible-lint reports error from foreign roles #1463

Closed
fgierlinger opened this issue Mar 14, 2021 · 6 comments · Fixed by #1479
Closed

Ansible-lint reports error from foreign roles #1463

fgierlinger opened this issue Mar 14, 2021 · 6 comments · Fixed by #1479
Assignees
Labels
Milestone

Comments

@fgierlinger
Copy link

Summary

ansible-lint install requirements into the .cache dir of the currently tested role/playbook. If one of the roles in .cache/roles contains an error, the linting fails for the local role/playbook.

Issue Type
  • Bug Report
Ansible and Ansible Lint details
$ ansible --version
ansible 2.10.6
$ pip freeze | grep ansible
ansible-base==2.10.6
ansible-lint==5.0.3
$ ansible-lint --version
ansible-lint 5.0.3 using ansible 2.10.6
  • ansible installation method: pip
  • ansible-lint installation method: pip
OS / ENVIRONMENT

CentOS8

STEPS TO REPRODUCE
  1. Install ansible-lint
$ pip3 install ansible-lint[core,yamllint]
  1. Create a playbook with a reference to a galaxy role which contains an linting error
---
# playbook.yml
- hosts: all
  roles:
    - role: geerlingguy.docker
  1. Create the corresponding requirements.yml file
---
# requirements.yml
roles:
  - src: geerlingguy.docker
  1. Run ansible-lint and pass the playbook as parameter
$ ansible-lint ./playbook.yml
Desired Behaviour

Ansible-lint exits successfully because the tested playbook contains no errors.

Actual Behaviour

Ansible-lint exits with an error because the included role contains a linting error.

$ pip3 install ansible-lint[core,yamllint]
$ cat > requirements.yml <<EOF
---               
roles:
  - src: geerlingguy.docker
EOF
$ cat > playbook.yml <<EOF
---               
- hosts: localhost
  roles: 
    - role: geerlingguy.docker
EOF
$ ansible-lint ./playbook.yml
Running ansible-galaxy role install --roles-path /home/fgierlinger/.cache/roles -vr requirements.yml
Added ANSIBLE_ROLES_PATH=~/.ansible/roles:/usr/share/ansible/roles:/etc/ansible/roles:/home/fgierlinger/.cache/roles
WARNING  Failed to discover yaml files to lint using git: fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
WARNING  Listing 2 violation(s) that are fatal
yaml: line too long (194 > 160 characters) (line-length)
../../home/fgierlinger/.ansible/roles/geerlingguy.docker/defaults/main.yml:20

risky-shell-pipe: Shells that use pipes should set the pipefail option
../../home/fgierlinger/.ansible/roles/geerlingguy.docker/tasks/setup-Debian.yml:29 Task/Handler: Add Docker apt key (alternative for older systems without SNI).

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
  - risky-shell-pipe  # Shells that use pipes should set the pipefail option
  - yaml  # Violations reported by yamllint
Finished with 2 failure(s), 0 warning(s) on 9 files.
@aminvakil
Copy link
Contributor

aminvakil commented Mar 14, 2021

This is not a bug IMHO.

It's linting playbook and this role should be linted as it's part of this playbook, of course you can exclude it if you don't want linting it:

(venv) [user@hostname tmp]$ ansible-lint ./playbook.yml --exclude ~/.cache/roles
Running ansible-galaxy role install --roles-path /home/user/.cache/roles -vr requirements.yml
Added ANSIBLE_ROLES_PATH=~/.ansible/roles:/usr/share/ansible/roles:/etc/ansible/roles:/home/user/.cache/roles
WARNING  Failed to discover yaml files to lint using git: fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
(venv) [user@hostname tmp]$ echo $?
0

@fgierlinger
Copy link
Author

@aminvakil I thought about this for some time, but I came to the conclusion that the default behavior of ansible-lint should be not to lint roles downloaded to .cache per default. I think I can speak generally, that one is not interested of the linting errors of somebody else's role.

My proposition is to add a parameter to ansible-lint (e.g. --deep or --check-dependencies) which then includes all dependent roles. Those parameters would be false by default and only the local play is linted.

@geerlingguy
Copy link

This also seems to have broken a lot of my own role testing, to the point I'm considering removing ansible-lint from my workflow.

Before 5.0, I was able to lint all my roles without having to also lint all the dependencies and other bits that a test playbook would use. Now, ansible-lint tries running all the playbooks in the molecule directory (and indeed, pretty much anywhere else in my git project), and throws up a ton of different errors.

It's driving me mad, and I don't know if there's anything I can do short of dropping it from my workflow (at least for roles) at this time.

I'm not sure what's going to happen on Galaxy when later versions of ansible-lint are installed, but I guess I'll just have to take the terrible code quality scores on the chin.

@ssbarnea ssbarnea self-assigned this Mar 18, 2021
@ssbarnea ssbarnea added this to the 5.0.5 milestone Mar 18, 2021
ssbarnea added a commit that referenced this issue Mar 18, 2021
To avoid false-positive results from users did not add .cache folder
to their .gitignore file, include it in the implicit exclude_paths.

Fixes: #1463
@ssbarnea
Copy link
Member

I added a patch that should include .cache folder in the implicit exclude list, please try it. Please note that if you define your own exclude_list, you will have to manually add the .cache folder as I do not want to create a case where user cannot ever lint this folder.

Regarding the no-deep proposal is not really needed, we already have the --offline mode and checking for existence of imported roles/modules is not really possible, mainly because ansible itself does load the files. That is why mocking was created, so you can create fake ones that please ansible itself.

If it would have being only about the linter itself, it could have being possible to include errors coming from the "inclusion" points, but that happens to also be done within ansible own code, and we cannot control these.

ssbarnea added a commit that referenced this issue Mar 19, 2021
To avoid false-positive results from users did not add .cache folder
to their .gitignore file, include it in the implicit exclude_paths.

Fixes: #1463
@ghost
Copy link

ghost commented Mar 25, 2021

I'm considering removing ansible-lint from my workflow

It happened :(

@geerlingguy
Copy link

@idolux - For now it's out, but I think I may be able to incorporate it again without too much pain... the annoyance is there are now five or six little 'meta setup' steps I have to take for each role whereas in the past it was usually just "add ansible-lint to my CI, and maybe add a .ansible-lint file if I need to exclude one or two rules".

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