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

Inconsistent/incorrect behaviour between playbooks and roles #488

Closed
g1ps opened this issue Mar 7, 2019 · 8 comments
Closed

Inconsistent/incorrect behaviour between playbooks and roles #488

g1ps opened this issue Mar 7, 2019 · 8 comments

Comments

@g1ps
Copy link

g1ps commented Mar 7, 2019

Issue Type

  • Bug report

Ansible and Ansible Lint details

ansible --version 2.6.3
ansible-lint --version 4.1.0
  • ansible installation method: pip
  • ansible-lint installation method: pip

Desired Behaviour

Playbooks and roles should be linted consistently. Rule exclusion should work consistently.

Actual Behaviour (Bug report only)

Structure:
playbook.yml
roles
-myrole
--tasks
---main.yaml
---[other task files]

playbook.yml includes myrole.

ansible-lint -c ./tests/ansibleLint/.ansible-lint ./playbook.yml -v
If I run ansible-lint against the playbook, the role is included, all files are parsed as shown in the output and lint errors displayed.

ansible-lint -c ./tests/ansibleLint/.ansible-lint ./roles/myrole -v
If I run ansible-lint against the role, all role files are parsed as shown in the output and lint errors displayed.

ansible-lint -c ./tests/ansibleLint/.ansible-lint ./roles/myrole/tasks -v
If I run ansible-lint against the role's tasks directory, no task files are parsed as shown in the output and no errors displayed.

ansible-lint -c ./tests/ansibleLint/.ansible-lint ./roles/myrole/tasks/main.yml -v
If I run ansible-lint against the role's tasks/main.yml, all task files are parsed as shown in the output and no lint errors displayed.

ansible-lint -c ./tests/ansibleLint/.ansible-lint ./roles/myrole/tasks/otherfile.yml -v
If I run ansible-lint against any other file in the the role's tasks directory`, the file is parsed as shown in the output and no lint errors displayed.

That just seems wrong and nothing is output to expain it. If you don't have verbose output on you wouldn't even know, you'd just think all files has passed the tests.

Further, using E301 as an example. I have the following example task:

- name: Check if correct version is installed
  command: "<application> --version"
  register: current_version

which raises [E301] Commands should not change things if nothing needs doing

along with others that raise the same error.

I am unable to selectively disable this warning in the role. warn=no does not work. noqa does not work anywhere in the task.

  args:
    warn: False

does not work.

  tags:
  - skip_ansible_lint

does work and is the only option that does.

All of these options work as expected when used in a playbook.

I have a .ansible-lint file which is similar to:

exclude_paths:
  - ./development/
  - ./inventories/
  - ./library/
  - ./module_utils/
  - ./tests/
parseable: true
quiet: false
rulesdir:
  - ./tests/ansibleLint/
skip_list:
  - ANSIBLE0002     # Override - TrailingWhitespaceRule
  - 'BIO252'             # Broken   - Block should have rescue
  - '204'                   # Override - Line too long
  - '402'                   # Disable  - Mercurial checkouts must contain explicit revision
  - '601'                   # Temp     - Don't compare to literal True/False
  - '602'                   # Temp     - Don't compare to empty string
#tags:
#  - run_this_tag
use_default_rules: true
verbosity: 0
@g1ps
Copy link
Author

g1ps commented Mar 15, 2019

Is anyone looking at this? It seems pretty important.

@awcrosby
Copy link
Contributor

Per the docs ansible-lint can be passed either a role directory or a playbook file. If you pass a .yml file that is part of a role it will assume it is a playbook (discussed in #280). If folks think exiting with an error code when calling ansible-lint *.yml and the .yml is not in the playbook structure, please weigh in.

For the skipping piece… I successfully skipped the given example role task with # noqa 301. I did however see that warn: False does not skip 301 even though it skips other command rules like 303. I recommend using noqa as it is preferred over warn: False. If you still see the issue with # noqa 301, please provide the output and I’ll look at that more.

@g1ps
Copy link
Author

g1ps commented Mar 19, 2019

Hi,

I understand that the doc says that. My point is that the output is very badly misleading and the opportunity for users to believe that the code has been linted and passed should be obvious. If the application is passed something it can't work with, it should say so.

I spent several more hours testing. I believe I have found the issue. I created a new role to test. Using # noqa in meta/main.yml and tasks/main.yml worked fine and this caused much consternation as I repeatedly tested this against our roles trying to understand what was different. I isolated parts of our config, disabled parts systematically, avoided it completely, ignored our custom rules, etc. warn=no and warn: false also worked. Even with our config and custom rules it worked fine in the test role until I created a second file, other.yml in tasks. This file has exactly the same content as tasks/main.yml. noqa in that fails to work at all. warn=no and warn: false don't work either. The files are parsed, verified by using -vvv. I have no idea why a neighbouring file to main.yml wouldn't work. All of our roles are large and they all use multiple files to separate concerns, as you'd expect. This seems to be the cause of my problem.

The complete role is attached for your testing.
[OK, it isn't. There doesn't seem to be a way to attach files. Please advise how to get it to you.]

@g1ps g1ps closed this as completed Mar 19, 2019
@g1ps g1ps reopened this Mar 19, 2019
@awcrosby
Copy link
Contributor

That is a good point, I will make a separate issue to discuss exiting with error code if we cannot lint a .yml (i.e. it's not a playbook)

Thank you for the detailed testing! You should be able to drag and drop a file to attach it, I will take a look.

@g1ps
Copy link
Author

g1ps commented Mar 19, 2019

test-role.zip

@g1ps
Copy link
Author

g1ps commented Mar 19, 2019

Super, thanks.

@ssbarnea
Copy link
Member

Closing as very old and most of the code changed since. If you still find it misbehaving please build a minimal reproducing case. Keep in mind that at this moment the linter allows you to pass only playbooks and roles, not other file types.

@g1ps
Copy link
Author

g1ps commented Feb 12, 2021

Please re-open this. All you have to do is test against the test role provided to see that there are 300-series warnings which arent suppressed.

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

No branches or pull requests

3 participants