Comments (5)
willthames
commented on August 19, 2024
ansible-review doesn't process roles so much as files within those roles. It tends to look at the parent directory of a file to classify them - so a main.yml under tasks would get processed as a task file, whereas a main.yml under defaults would get processed as a variables file.
If you can provide a working test case, that would be helpful
from ansible-review.
willthames
commented on August 19, 2024
If I run ansible-review against https://github.com/willthames/ansible-jenkins-ci ansible subdirectory, it processes my roles fine, using ansible 2.2 or 2.3 (found a bug against ansible latest doing this, so that's useful to know!)
$ git ls-files roles | xargs ansible-review -v
INFO: Using configuration file: /Users/will/Library/Application Support/ansible-review/config.ini
INFO: Reviewing all of Task (roles/ec2_elb/tasks/main.yml)
WARN: Task roles/ec2_elb/tasks/main.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.0
INFO: Task roles/ec2_elb/tasks/main.yml declares standards version 0.0
INFO: Proposed standard "Use become/become_user/become_method rather than sudo/sudo_user" met
INFO: Proposed standard "Commands should be idempotent" met
INFO: Proposed standard "Commands should not be used in place of modules" met
INFO: Proposed standard "Package installs should use present, not latest" met
WARN: Future standard "YAML should be correctly indented" not met:
roles/ec2_elb/tasks/main.yml:23: lines starting with '- ' should have same or less indentation than previous line
INFO: Proposed standard "Shell should only be used when essential" met
WARN: Future standard "Tasks and handlers must be named" not met:
roles/ec2_elb/tasks/main.yml:1: [ANSIBLE0011] All tasks should be named
WARN: Future standard "Tasks and handlers must be named" not met:
roles/ec2_elb/tasks/main.yml:8: [ANSIBLE0011] All tasks should be named
WARN: Future standard "Tasks and handlers must be named" not met:
roles/ec2_elb/tasks/main.yml:15: [ANSIBLE0011] All tasks should be named
INFO: Proposed standard "Tasks and handlers must be uniquely named within a single file" met
INFO: Proposed standard "Environment variables should be passed through the environment key" met
INFO: Proposed standard "Use YAML format for tasks and handlers rather than key=value" met
INFO: Proposed standard "Files should contain useful content" met
INFO: Standard "bare words are deprecated for with_items" met
INFO: Proposed standard "Lineinfile module should not be used as it suggests more than one thing is managing a file" met
INFO: Proposed standard "Variable uses should contain whitespace" met
INFO: Reviewing all of Task (roles/ec2_instance/tasks/main.yml)
WARN: Task roles/ec2_instance/tasks/main.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.0
INFO: Task roles/ec2_instance/tasks/main.yml declares standards version 0.0
INFO: Proposed standard "Use become/become_user/become_method rather than sudo/sudo_user" met
INFO: Proposed standard "Commands should be idempotent" met
INFO: Proposed standard "Commands should not be used in place of modules" met
INFO: Proposed standard "Package installs should use present, not latest" met
INFO: Proposed standard "YAML should be correctly indented" met
INFO: Proposed standard "Shell should only be used when essential" met
INFO: Proposed standard "Tasks and handlers must be named" met
INFO: Proposed standard "Tasks and handlers must be uniquely named within a single file" met
INFO: Proposed standard "Environment variables should be passed through the environment key" met
INFO: Proposed standard "Use YAML format for tasks and handlers rather than key=value" met
INFO: Proposed standard "Files should contain useful content" met
INFO: Standard "bare words are deprecated for with_items" met
INFO: Proposed standard "Lineinfile module should not be used as it suggests more than one thing is managing a file" met
INFO: Proposed standard "Variable uses should contain whitespace" met
INFO: Reviewing all of Task (roles/ec2_vpc/tasks/main.yml)
WARN: Task roles/ec2_vpc/tasks/main.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.0
INFO: Task roles/ec2_vpc/tasks/main.yml declares standards version 0.0
INFO: Proposed standard "Use become/become_user/become_method rather than sudo/sudo_user" met
INFO: Proposed standard "Commands should be idempotent" met
INFO: Proposed standard "Commands should not be used in place of modules" met
INFO: Proposed standard "Package installs should use present, not latest" met
INFO: Proposed standard "YAML should be correctly indented" met
INFO: Proposed standard "Shell should only be used when essential" met
INFO: Proposed standard "Tasks and handlers must be named" met
INFO: Proposed standard "Tasks and handlers must be uniquely named within a single file" met
INFO: Proposed standard "Environment variables should be passed through the environment key" met
INFO: Proposed standard "Use YAML format for tasks and handlers rather than key=value" met
INFO: Proposed standard "Files should contain useful content" met
INFO: Standard "bare words are deprecated for with_items" met
INFO: Proposed standard "Lineinfile module should not be used as it suggests more than one thing is managing a file" met
INFO: Proposed standard "Variable uses should contain whitespace" met
INFO: Reviewing all of Task (roles/jenkins-setup/tasks/main.yml)
WARN: Task roles/jenkins-setup/tasks/main.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.0
INFO: Task roles/jenkins-setup/tasks/main.yml declares standards version 0.0
INFO: Proposed standard "Use become/become_user/become_method rather than sudo/sudo_user" met
INFO: Proposed standard "Commands should be idempotent" met
INFO: Proposed standard "Commands should not be used in place of modules" met
INFO: Proposed standard "Package installs should use present, not latest" met
INFO: Proposed standard "YAML should be correctly indented" met
INFO: Proposed standard "Shell should only be used when essential" met
INFO: Proposed standard "Tasks and handlers must be named" met
INFO: Proposed standard "Tasks and handlers must be uniquely named within a single file" met
INFO: Proposed standard "Environment variables should be passed through the environment key" met
INFO: Proposed standard "Use YAML format for tasks and handlers rather than key=value" met
INFO: Proposed standard "Files should contain useful content" met
INFO: Standard "bare words are deprecated for with_items" met
INFO: Proposed standard "Lineinfile module should not be used as it suggests more than one thing is managing a file" met
INFO: Proposed standard "Variable uses should contain whitespace" met
from ansible-review.
dflock
commented on August 19, 2024
Hmmm.
If I instead of doing this, which outputs nothing:
$ git ls-files ansible | xargs ansible-review -v | grep roleI do this, it processes the roles:
$ git ls-files ansible/roles | xargs ansible-review -v
INFO: Using configuration file: /home/duncan/.config/ansible-review/config.ini
INFO: Reviewing all of Template (roles/role_1/templates/script.sh.j2)
WARN: Template roles/role_1/templates/script.sh.j2 is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.1
INFO: Template roles/role_1/templates/script.sh.j2 declares standards version 0.1
INFO: Best practice "Variable uses should contain whitespace" met
...So given this folder structure:
./provisioning/
└── ansible/
├── group_vars/
├── roles/
│ ├── role_1/
│ │ └── tasks/
│ │ │ └── main.yml
│ │ └── templates/
│ │ │ └── script.sh.j2
│ ├── role_2/
...
├── ansible.cfg
├── ansible.log
├── playbook_1.yml
...
if I'm in the provisioning folder and point ansible-review at the ansible folder, it doesn't process the roles files, but if I point it at just the ansible/roles folder, it does.
So, I tried this on my reduced test case which I added a test role to - and couldn't reproduce it.
I think the actual issue is that when I run this on our main provisioning folder (which is pretty large: 117 roles, 46 playbooks, etc...) it crashes at some point and spits out stuff on stderr. I suspect this is malformed stuff in our code, rather than bugs in ansible-review (although it probably shouldn't just crash. I'll file issues as I figure these out.)
I think, depending on how you pass the list of files into ansible-review, it receives & processes the files in a different order, and gets through a different set of files before finding something that causes it to die. This means that it often doesn't get as far as the roles before it dies. Eg:
$ find ./ansible | xargs ansible-review 1>/dev/null
Traceback (most recent call last):
File "/usr/local/bin/ansible-review", line 11, in <module>
sys.exit(main())
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/main/__init__.py", line 86, in main
candidate = classify(filename)
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/__init__.py", line 197, in classify
return Template(filename)
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/__init__.py", line 83, in __init__
super(RoleFile, self).__init__(filename)
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/__init__.py", line 64, in __init__
self.version = find_version(filename)
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/__init__.py", line 227, in find_version
with codecs.open(filename, mode='rb', encoding='utf-8') as f:
File "/usr/lib/python2.7/codecs.py", line 896, in open
file = __builtin__.open(filename, mode, buffering)
IOError: [Errno 21] Is a directory: './ansible/roles/role_123/templates'
$ git ls-files ./ansible | xargs ansible-review 1>/dev/null
Traceback (most recent call last):
File "/usr/local/bin/ansible-review", line 11, in <module>
sys.exit(main())
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/main/__init__.py", line 95, in main
errors = errors + candidate.review(options, lines)
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/__init__.py", line 72, in review
return utils.review(self, settings, lines)
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/utils/__init__.py", line 120, in review
result = standard.check(candidate, settings)
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/code.py", line 9, in code_passes_flake8
lineno = int(line.split(':')[1])
ValueError: invalid literal for int() with base 10: ''from ansible-review.
dflock
commented on August 19, 2024
However - if I run it like this:
$ ansible-review ./ansible/* 1>/dev/nullit doesn't crash - it runs all the way through - probably because it doesn't process the /roles folder at all:
$ ansible-review ./ansible/* | grep role
WARN: Couldn't classify file ./ansible/rolesThis is reproducible on my reduced test case.
So, I guess if you run it like ansible-review <folder> it decides not to descend into the roles folder and doesn't process anything in there - but if you pass in a file list that already includes those files, then it will attempt to process them (and maybe crash, at some point).
from ansible-review.
willthames
commented on August 19, 2024
Right, ansible-review is not at all recursive. It just reviews files. Which is why I suggest using git ls-files (or even find . -type f) or just running against the output of git diff.
I'm going to close this issue now. If you find any files for which you do get a stack trace in the output, those are bugs, please file a separate issue with reproducible test case. If you think the documentation is lacking, please file a separate issue (or preferably a PR)
from ansible-review.
Related Issues (20)
- Octal Permission representation HOT 1
- Add custom ID to Standard
- Bug using get_action_tasks
- Classification of files as group_vars not working correctly HOT 3
- Python 3 support HOT 3
- AttributeError: 'InventoryManager' object has no attribute 'get_group' HOT 2
- Unable to Load Variables from Directories within defaults/main HOT 1
- Broken PyPI metadata and related problems HOT 3
- Embrace parseable output to stdout HOT 4
- Broken EXTRA0008 check HOT 3
- ansible review with git diff sometimes aborts on non yaml files, non yaml should be skipped
- Warnings vs Errors HOT 4
- Stacktrace when providing custom config file using ansible-review 0.14.0rc1 HOT 1
- Inconsistent output in pre-commit hook compared to other hooks HOT 1
- Support for pre-commit hook `verbose` option
- Inconsistent output from ansible-review for some rules
- release 0.14rc2 produces stack trace when trying to read vaulted file
- ansible-review v0.14.0rc2 produces stacktrace when checking azure_rm.yml inventory plugin HOT 1
- Stack trace/error upon finding vault string in playbook vars HOT 1
- cannot import name 'default_rulesdir' from 'ansiblelint' HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ansible-review.