willthames / ansible-review Goto Github PK
View Code? Open in Web Editor NEWLicense: MIT License
License: MIT License
There are three situations when a check fails against a playbook (unversioned playbooks should be treated as playbooks on very latest version)
Currently it isn't possible to provide one or more directories and to run through the whole directory tree to test every Ansible's file.
It would be a nice addition to be able to run it by providing directories and recursively browse them.
Filed the issue here as ansible-review seemed to be the most fitted for it, but maybe this could also be in ansible-lint.
If you point asible-review
at this YAML file (or presumably any broken/invalid YAML file):
test: [one, two]
three, four]
you get the error message form pyaml
and ansible-review
quits:
INFO: Playbook ./ansible/broken_yaml/broken_yaml_1.yml declares standards version 0.1
Failed to parse YAML in ./ansible/broken_yaml/broken_yaml_1.yml: while parsing a block mapping
in "<unicode string>", line 1, column 1:
test: [one, two]
^
expected <block end>, but found '<scalar>'
in "<unicode string>", line 2, column 3:
three, four]
^
Ansible-review should trap this error and emit an ERROR:...
message, and continue.
There seems to be a bug in classification of files under group_vars if group_vars contains subdirectories e. g.
group_vars
|___ group_1
|_______ main.yml
|_______ vaulted.yml
|___ group_2
...
Regards
I am trying the new tagged v0.14.0rc1 and even though it seems to fix the original issue with vaulted strings, I hit an issue with providing a custom config file.
> cat .ansible-review
[rules]
lint = ./.ansible-linting/lint-rules/
standards = ./.ansible-linting
And running the tool produces...
> ansible-review -c .ansible-review vars/main.yml
Traceback (most recent call last):
File "/usr/local/bin/ansible-review", line 10, in <module>
sys.exit(main())
File "/tmp/ansible-review/lib/ansiblereview/__main__.py", line 56, in main
settings = read_config(options.configfile)
File "/tmp/ansible-review/lib/ansiblereview/utils/__init__.py", line 163, in read_config
indent_list_items=config.getboolean('rules', 'indent_list_items')
File "/usr/local/Cellar/python@2/2.7.16/Frameworks/Python.framework/Versions/2.7/lib/python2.7/ConfigParser.py", line 369, in getboolean
if v.lower() not in self._boolean_states:
AttributeError: 'bool' object has no attribute 'lower'
I created a little script to wrap ansible-review
and post-process the output. This was harder than it might have been, for two reasons:
ansible-review
splits the output for some things over two lines, like this:WARN: Best practice "Playbooks should not contain logic (vars, tasks, handlers)" not met:
ansible/playbook.yml:10: [EXTRA0008] tasks should not be required in a play
.. but not others. Most parsing tools/code/logic etc... is simpler line-by-line, so this makes it more awkward. I ended up simplifying my life by joining them back together using sed
:
$ git ls-files ansible/. | xargs ansible-review 2>/dev/null | sed -n '/met:/ {N; s/[\r\n]/ /g; p}'
This has a (mostly happy) side effect of removing several "non-standard" (see below) messages, like this:
WARN: Couldn't classify file ansible/ansible.cfg
regex = r'^WARN: (.*) not met: (.*?):(.*?): (.*)$'
However, the message output in #21 doesn't:
WARN: Best practice "Same variable defined in siblings" not met:
ansible/group_vars/test.yml:Inventory is broken:
Attempted to read "/home/duncan/dev/tools/provisioning/ansible/ansible.log" as ini file:
/home/duncan/dev/tools/provisioning/ansible/ansible.log:1:
Expected key=value host variable assignment, got: 16:47:41,708
it goes off the rails after the filename, putting Inventory is broken
where the line number would go - i.e. matching the regex, but putting the "wrong" values in.
I could use some extra code to detect these different messages, but in the case of couldn't classify
& the #21 messages, they're basically problems with ansible-review
(from my point of view), so I just filtered them out instead.
Here's the script, if you're interested:
"""
Post process the output from Ansible Review.
This script runs Ansible Review, captures the output, then
post-processes it into a Github formated markdown Checklist, which
it then outputs to stdout.
It uses the blessed module for output, so it has nice formatting
in the console, but is pipe aware, so you don't get control chars
in piped output.
"""
import os
import re
import subprocess
from blessed import Terminal
from operator import itemgetter
# Get the output from ansible-review, and collapse the two-line output into one line:
# when a line ends in `met:`, the next line is a continuation, so collapse into one line
# Skip group_vars - this outputs a non-standard msg about inventory file being broken
# see here: https://github.com/willthames/ansible-review/issues/21
cmd = r"git ls-files ansible/. | grep -v group_vars | xargs ansible-review 2>/dev/null | sed -n '/met:/ {N; s/[\r\n]/ /g; p}'"
try:
output = subprocess.check_output(
cmd,
stderr=subprocess.STDOUT,
shell=True
).split('\n')
except subprocess.CalledProcessError as e:
output = e.output.split('\n')
t = Terminal()
output_lines = []
class WarningLine:
regex = r'^WARN: (.*) not met: (.*?):(.*?): (.*)$'
text, filename, line, rule = range(1, 5)
for line in output:
# Convert the string line into an output_item dict, then add
# it to the output_lines list.
if line.startswith('WARN: '):
# It's a warning item
match = re.search(WarningLine.regex, line)
if match and len(match.groups()) == 4:
output_lines.append({
'log_level': t.yellow('WARN'),
'file': match.group(WarningLine.filename),
'line': int(match.group(WarningLine.line)),
'text': match.group(WarningLine.text),
'rule': match.group(WarningLine.rule),
})
# The output from ansible-review is sorted by filename, but it's nicer
# if we sort it by filename & line_no
sorted_output = sorted(output_lines, key=itemgetter('file', 'line'))
#
# Print the output_lines list, to the console, in markdown format
#
print(t.dim('# ') + t.bold('Ansible Review Checklist'))
curr_file = ''
for line in sorted_output:
if line['file'] != curr_file:
curr_file = line['file']
print(t.dim('\n## ') + t.bold(curr_file))
print t.dim('- []') + ' {l[log_level]}: Line: {l[line]:>3}: {l[text]}, rule: {l[rule]}'.format(l=line)
and here's an example of the console output:
which can be piped to a file, then opened in a markdown editor that understands github formatted markdown (like Abricotine) and used as a checklist:
I did attempt to get ansible-review
to run from source, and add this as a feature (where it would probably be simpler to implement) - but couldn't figure out how to do that: #20
Is there a way to define rules as just warnings
instead of an error
? Meaning, the rule will be listed when running ansible-review
but it won't produce a return code of 1, which prevents the git commit. I see this feature on yamllint
tool and would love to be able to do that here too. Thanks!
I can't get ansible-review
to process my roles. It'll process my playbooks in the root, but won't look at the roles (where the bulk of the work is).
If I run this:
$ git ls-files ansible/. | grep role
ansible/roles/role_1/templates/template.sh.j2
ansible/roles/role_1/tasks/main.yml
...
I see roles as part of the output from git ls-files
, as expected. However, if I then run this:
$ git ls-files ansible/. | xargs ansible-review -v | grep roles
I get nothing. However, if I run this it'll tell me it can't classify the roles:
$ ansible-review -v ./ansible/* | grep role
WARN: Couldn't classify file ./ansible/roles
$ ansible-review ./ansible/roles/*
WARN: Couldn't classify file ./ansible/roles/role_1
WARN: Couldn't classify file ./ansible/roles/role_2
...
My folder structure looks like this:
./provisioning/
└── ansible/
├── group_vars/
│ ├── all.yml
│ ├── group_1.yml
│ └── group_2.yml
├── hosts/
│ └── host1.yml
├── roles/
│ ├── role_1/
│ ├── role_2/
│ ...
├── ansible.cfg
├── ansible.log
├── playbook_1.yml
├── playbook_2.yml
│ ...
I'm in the provisioning
folder at the top when I'm running ansible-review
.
Firstly, many thanks for this great tool.
I'm running ansible-review using the supplied example/standards.py against a role which contains some stub yml files which contain no tasks (yet).
When ansible-review hits one of these files using the supplied example/standards.py, it errors out when applying the 'playbooks_should_not_contain_logic' standard, as follows:
Traceback (most recent call last):
File "/usr/local/bin/ansible-review", line 87, in <module>
sys.exit(main(sys.argv[1:]))
File "/usr/local/bin/ansible-review", line 80, in main
errors = errors + candidate.review(options, lines)
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/__init__.py", line 68, in review
return utils.review(self, settings, lines)
File "/usr/local/lib/python2.7/dist-packages/ansiblereview/utils/__init__.py", line 107, in review
result = standard.check(candidate, settings)
File "/home/anth/dev/ac/anthcourtney.cis-amazon-linux/tests/ansible-review/standards.py", line 104, in playbook_contains_logic
for play in plays:
TypeError: 'NoneType' object is not iterable
I can work around this by adding a `if plays is not None:`` conditional to the function, however would be interested in your perspective on how to best address this.
I encountered the following issue when running ansible-review:
$ ansible-review
/apps/tools/pip/lib/python2.6/site-packages/cryptography/__init__.py:26: DeprecationWarning: Python 2.6 is no longer supported by the Python core team, please upgrade your Python. A future version of cryptography will drop support for Python 2.6
DeprecationWarning
Traceback (most recent call last):
File "/apps/tools/pip/bin/ansible-review", line 8, in <module>
from ansiblereview.version import __version__
File "/apps/tools/pip/lib/python2.6/site-packages/ansiblereview/__init__.py", line 6, in <module>
import utils
File "/apps/tools/pip/lib/python2.6/site-packages/ansiblereview/utils/__init__.py", line 12, in <module>
import importlib
ImportError: No module named importlib
As you can guess this is on python2.6.6.
It seems that installing ansible-review via pip doesn't install importlib. Should this dependency exist?
Installing import lib via pip seems to have resolved the issue.
If a review fails. It exits with a return code 0. We want it to return non-zero to stop Jenkins from continuing with the build. Is it possible?
This:
- name: Template Mcrouter config
template:
src: mcrouter-config.json.j2
dest: /etc/mcrouter/mcrouter-config.json
owner: mcrouter
group: mcrouter
mode: 0755
tags:
- mcrouter-config
becomes:
{'delegate_to': None, 'name': 'Template Mcrouter config', 'tags': ['mcrouter-config'], '__file__': 'mcrouter/tasks/mcrouter.yml', '__line__': 44, '__ansible_action_type__': 'task', 'action': {'src': 'mcrouter-config.json.j2', '__ansible_arguments__': [], 'dest': '/etc/mcrouter/mcrouter-config.json', '__ansible_module__': 'template', '__file__': 'mcrouter/tasks/mcrouter.yml', '__line__': 45, 'mode': 493, 'owner': 'mcrouter', 'group': 'mcrouter'}}
I was looking to create a lint for octal permissions but their representation in ansible-review seems off.
We have a task that needs to use the template module to output a python file to the target node. This tasks folder looks like this:
$ tree ./ansible/roles/test_role
./ansible/roles/test_role
├── files
│ └── other.py
├── tasks
│ ├── main.yml
├── templates
│ ├── test.py
The test.py
jinja template looks like this:
DEPLOYMENT = dict(
env='{{ test.env }}'
)
ansible-review
is trying to flake8
this file and failing, leading to a crash, same as #26:
$ ansible-review -v ./ansible/roles/test/templates/test.py
INFO: Using configuration file: /home/duncan/.config/ansible-review/config.ini
INFO: Reviewing all of Code (./ansible/roles/test/templates/test.py)
INFO: Code ./ansible/roles/test/templates/test.py declares standards version 0.1
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: ''
The instructions in the README for running from source are:
# Install dependency https://github.com/willthames/ansible-lint
git clone https://github.com/willthames/ansible-review
export PYTHONPATH=$PYTHONPATH:`pwd`/ansible-review/lib
export PATH=$PATH:`pwd`/ansible-review/bin
There's no /ansible-review/bin
sub-folder, so these instructions don't work?
Hi!
In my playbook, I use role spring-boot
, which depends from role maven-download
, which provides module maven_download
.
Ansible-review fails with error:
Couldn't parse task at roles/spring-boot/tasks/extract_artifact.yml:17 (no action detected in task. This often indicates a misspelled module name, or incorrect module path.)
{ 'maven_download': 'group_id={{ item.group_id }} artifact_id={{ item.artifact_id }} version={{ item.artifact_version }} classifier=apidoc dest={{ doc_dest }}/{{ item.artifact_id }}.zip repository_url={{ maven_repo }} username={{ maven_repo_username }} password={{ maven_repo_password }}',
'name': 'download {{ item.artifact_id }} java package',
'register': 'download_result'}
This is a new feature with Ansible 2.6.
More information regarding it can be found here:
Currently, an IOError
is raised.
Traceback (most recent call last):
File "/usr/local/bin/ansible-review", line 11, in <module>
sys.exit(main())
File "/usr/local/lib/python2.7/site-packages/ansiblereview/__main__.py", line 86, in main
candidate = classify(filename)
File "/usr/local/lib/python2.7/site-packages/ansiblereview/__init__.py", line 184, in classify
return RoleVars(filename)
File "/usr/local/lib/python2.7/site-packages/ansiblereview/__init__.py", line 87, in __init__
super(RoleFile, self).__init__(filename)
File "/usr/local/lib/python2.7/site-packages/ansiblereview/__init__.py", line 68, in __init__
self.version = find_version(filename)
File "/usr/local/lib/python2.7/site-packages/ansiblereview/__init__.py", line 231, in find_version
with codecs.open(filename, mode='rb', encoding='utf-8') as f:
File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/codecs.py", line 898, in open
file = __builtin__.open(filename, mode, buffering)
IOError: [Errno 21] Is a directory: './roles/foreman-server/defaults/main'
If you have any non-python file into your ./library
folder, ansible-review
will crash:
$ cat ./ansible/library/test.txt
test string
$ ansible-review -v ./ansible/library/test.txt
INFO: Reviewing all of Code (./ansible/library/test.txt)
INFO: Code ./ansible/library/test.txt declares standards version 0.1
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: ''
Presumably this just runs flake8
on the file, assuming it's python, doesn't trap and crashes. It's valid to write Ansible modules in languages other than python; I came across this because I have a custom module that's written as a shell script.
Since #15 remains open, I'd like to disable the files_should_be_indented
standard in my codebase. How can I do this?
Hi, I'm running into the following error:
$ git ls-files | xargs ansible-review
Traceback (most recent call last):
File "/usr/local/bin/ansible-review", line 11, in <module>
sys.exit(main())
File "/usr/local/lib/python3.5/dist-packages/ansiblereview/__main__.py", line 57, in main
for key, value in settings.__dict__.iteritems():
AttributeError: 'dict' object has no attribute 'iteritems'
This seems to be the culprit:
for key, value in settings.__dict__.iteritems():
https://github.com/willthames/ansible-review/blob/master/lib/ansiblereview/__main__.py#L57
The all-knowing Stack Overflow seems to indicate this is related to Python 3:
As you are in python3 , use
dict.items()
instead ofdict.iteritems()
iteritems()
was removed in python3, so you can't use this method anymore.
https://stackoverflow.com/a/30418498/399105
Does ansible-review support Python 3?
In case it helps, this is how I installed ansible-review:
$ sudo pip3 install ansible-review
And some other relevant info:
$ ansible-review --version
ansible-review 0.13.4
$ python --version
Python 3.5.2
Thanks!
Regarding "WARN: Couldn't classify file", quick look at
candidate = classify(filename)
and classify
function make's me understand that if a file is not classified in the classify it's determined as None , as result ansible-review will always return a WARN for such files.
Should it have a mechanism to provide FILE we can ignore. Path ignore mechanism already exists for ansible-lint, i think ansible-review should use similar approach or use the existing config.
I think that the relationship between ansible-review and ansible-lint should be explained in the main README, making clear when you may/should want one use or another.
There is a bug using get_action_tasks. Therefor i think all rules which uses get_action_tasks from ansiblelint will not work:
The function try to get file['type'] but the corresponding candidate object from ansiblereview has no attribute named like that. There is only an attribute 'filetype'. So to get get_action_tasks working the simplest workaround is to set candidate.type = candidate.filetype
before calling get_action_tasks()
Regards
When using the -q
flag, warning messages are still printed. Is this the intended behavior? I would expect that only errors are shown. I'm glad to open a PR to address this but wanted to get feedback first.
According to both YAML documentation and Ansible YAML syntax (http://docs.ansible.com/ansible/YAMLSyntax.html), the indentation that gets checked with the function files_should_be_indented is incorrect.
A warning is thrown if I have indentation at the - with a list of tags for example.
- name: Create kill link
file:
state: link
src: /opt/uptime-agent/bin/upt_daemon.sh
dest: /etc/rc.d/rc3.d/K1uptm_daemon
tags:
- uptimeconfig
WARN: Best practice "YAML should be correctly indented" not met:
roles/configure.uptimeagent/tasks/main.yml:11: lines starting with '- ' should have same or less indentation than previous line
The warning disappears when I remove the two spaces in front of the dash at the tag, as such.
- name: Create kill link
file:
state: link
src: /opt/uptime-agent/bin/upt_daemon.sh
dest: /etc/rc.d/rc3.d/K1uptm_daemon
tags:
- uptimeconfig
This doesn't follow Ansible YAML syntax as per the documentation, though it works apparently without any issues.
Can you advice on this, if it's intentional or not? Thanks in advance.
Kind regards,
Eric V.
It is common to print smth parseable for the sake of pipe chaining among POSIX utils.
However, ansible-review doesn't seem to care about this:
✦ ➜ ansible-review rebuild-world.yml -q 1>/dev/null
Couldn't parse task at rebuild-world.yml:8 (no action detected in task. This often indicates a misspelled module name, or incorrect module path.
The error appears to have been in '<unicode string>': line 8, column 5, but may
be elsewhere in the file depending on the exact syntax problem.
(could not open file to display line))
{ 'eix': {'__file__': 'rebuild-world.yml', '__line__': 9, 'action': 'sync'},
'name': 'Sync portage/layman trees and index ebuilds'}
I'm building a GitHub App integration demo, which would really benefit from not having to use heuristics and manual conditional parsing to retrieve structured result data.
Using ansible-review
version v0.14.0rc2
.
The output of ansible-review when you run the command in the pre-commit hook is different from yamllint
for example. I am not sure if there even is a standard output format for hooks, but I think there should be. This allows users of hooks to filter and do some grep work if they want.
In the least, I think the output in the pre-commit hook for ansible-review is really too "wordy" and hard to read. The yamllint format (seen below in my example) is easier to read and determine what file failed, what line in the file, and what problem it was. It's clearly broken into nice columns.
Additionally, the pre-commit output does not need to tell me if the vars/main.yml
does not declare a standards version. This seems like info I don't need to really know as a developer. I want to know the errors specifically.
Here is the example with my pre-commit hook using ansible-review
and yamllint
:
> git commit -m "Added some files to test linting"
ansible-review...........................................................Failed
hookid: ansible-review
WARN: RoleVars vars/main.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.1
WARN: Best practice "YAML should be correctly indented" not met:
vars/main.yml:4: indentation should increase by 2 chars
WARN: Best practice "Vars should only occur once per file" not met:
vars/main.yml:3: Variable password occurs more than once
WARN: Best practice "Vars should only occur once per file" not met:
vars/main.yml:10: Variable password occurs more than once
WARN: Task tasks/main.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.1
WARN: Best practice "Commands should be idempotent" not met:
tasks/main.yml:27: [301] Commands should not change things if nothing needs doing
WARN: Best practice "YAML should be correctly indented" not met:
tasks/main.yml:5: lines starting with '- ' should have same or less indentation than previous line (indent_list_items is False)
WARN: RoleVars vars/redhat.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.1
WARN: Playbook .pre-commit-config.yaml does not present standards version. Using latest standards version 0.1
WARN: Best practice "YAML should be correctly indented" not met:
.pre-commit-config.yaml:4: lines starting with '- ' should have same or less indentation than previous line (indent_list_items is False)
WARN: Best practice "YAML should be correctly indented" not met:
.pre-commit-config.yaml:7: lines starting with '- ' should have same or less indentation than previous line (indent_list_items is False)
WARN: Best practice "YAML should be correctly indented" not met:
.pre-commit-config.yaml:15: lines starting with '- ' should have same or less indentation than previous line (indent_list_items is False)
WARN: Best practice "YAML should be correctly indented" not met:
.pre-commit-config.yaml:21: lines starting with '- ' should have same or less indentation than previous line (indent_list_items is False)
WARN: RoleVars vars/windows.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.1
ERROR: Standard "Shell should only be used when essential" not met:
tasks/main.yml:27: [305] Use shell only when shell functionality is required
ERROR: Standard "Tasks and handlers must be named" not met:
tasks/main.yml:24: [502] All tasks should be named
ERROR: Standard "Tasks and handlers must be named" not met:
tasks/main.yml:27: [502] All tasks should be named
ERROR: Standard "Variable names should only use lowercase letters and underscore" not met:
tasks/main.yml:24: [CUSTOM0001] Variables must be named using only lowercase and underscores
yamllint.................................................................Failed
hookid: yamllint
vars/main.yml
10:1 error duplication of key "password" in mapping (key-duplicates)
12:1 error too many blank lines (1 > 0) (empty-lines)
tasks/main.yml
28:1 error too many blank lines (1 > 0) (empty-lines)
vars/windows.yml
2:81 error line too long (85 > 80 characters) (line-length)
Looking at the some of the rules e.g. https://github.com/willthames/ansible-review/blob/master/lib/ansiblereview/examples/lint-rules/MetaMainHasInfoRule.py it would appear you might be able to limit the scope of rules to run to certain tags. Is this correct and if so how do you do it?
We currently check for duplicate inventory variables at the file level.
For a group_vars or host_vars or inventory change, would be good to check the whole inventory hierarchy to see if any new duplicate variables appear as a result of diamond inheritance hierarchies.
https://github.com/willthames/ansible-review/blob/master/examples/standards.py does not exist, but it is referred to from the documentation
Python 2.7.12
pip freeze (fragment):
ansible==2.2.1.0
ansible-lint==3.4.13
ansible-review==0.13.0
source yaml:
- name: This code will fail because 'port' is not wrapped in double quotes
wait_for:
host: "{{ somevar }}"
port: {{ i_have_missing_double_quotes }}
timeout: "{{ i_am_ok }}"
error trace:
censored/roles/censored/tasks/main.yml:29: Task arguments appear to be in key value rather than YAML format
Traceback (most recent call last):
File "censored/.venv/bin/ansible-review", line 11, in <module>
sys.exit(main())
File "censored/.venv/local/lib/python2.7/site-packages/ansiblereview/main/__init__.py", line 95, in main
errors = errors + candidate.review(options, lines)
File "censored/.venv/local/lib/python2.7/site-packages/ansiblereview/__init__.py", line 72, in review
return utils.review(self, settings, lines)
File "censored/.venv/local/lib/python2.7/site-packages/ansiblereview/utils/__init__.py", line 120, in review
result = standard.check(candidate, settings)
File "censored/.venv/local/lib/python2.7/site-packages/ansiblereview/__init__.py", line 219, in ansiblelint
matches = rules.run(fileinfo, rulename.split(','))
File "censored/.venv/local/lib/python2.7/site-packages/ansiblelint/__init__.py", line 139, in run
matches.extend(rule.matchtasks(playbookfile, text))
File "censored/.venv/local/lib/python2.7/site-packages/ansiblelint/__init__.py", line 66, in matchtasks
yaml = ansiblelint.utils.parse_yaml_linenumbers(text, file['path'])
File "censored/.venv/local/lib/python2.7/site-packages/ansiblelint/utils.py", line 524, in parse_yaml_linenumbers
data = loader.get_single_data()
File "censored/.venv/local/lib/python2.7/site-packages/yaml/constructor.py", line 39, in get_single_data
return self.construct_document(node)
File "censored/.venv/local/lib/python2.7/site-packages/yaml/constructor.py", line 48, in construct_document
for dummy in generator:
File "censored/.venv/local/lib/python2.7/site-packages/yaml/constructor.py", line 398, in construct_yaml_map
value = self.construct_mapping(node)
File "censored/.venv/local/lib/python2.7/site-packages/ansiblelint/utils.py", line 515, in construct_mapping
mapping = Constructor.construct_mapping(loader, node, deep=deep)
File "censored/.venv/local/lib/python2.7/site-packages/yaml/constructor.py", line 208, in construct_mapping
return BaseConstructor.construct_mapping(self, node, deep=deep)
File "censored/.venv/local/lib/python2.7/site-packages/yaml/constructor.py", line 132, in construct_mapping
"found unacceptable key (%s)" % exc, key_node.start_mark)
yaml.constructor.ConstructorError: while constructing a mapping
in "<unicode string>", line 54, column 13:
port: {{ i_have_missing_double_quotes }}
^
found unacceptable key (unhashable type: 'dict')
in "<unicode string>", line 54, column 14:
port: {{ i_have_missing_double_quotes }}
^
Expected behaviour:
no traceback but meaningful error explanation.
It errors out if there's tasks:
in playbook, but using roles:
is currently discouraged. One should use tasks:
+ import_role:
instead. This check should be improved.
In running a review, the following error is reported which is not correct.
ERROR: Standard "Tasks and handlers must be named" not met:
database/tasks/postgresql.yml:61: [ANSIBLE0011] All tasks should be named
The line concerned is:
There may need to be special handling enabled to know this task type.
Could you provide default standards?
We use following to do ansible review only on changes :
git diff origin/develop | ansible-review -c tests/ansible-review/config.ini
But if one of the changed files is not a yaml file the program aborts.
example file content that causes problems :
{
"timestamp": {{ report_timestamp | to_nice_json }},
"version": {{ version_framework | to_nice_json }},
"report_version": {{ version_reporting | to_nice_json }},
"id": {{ asset_id | to_nice_json }},
"info": {
"lab": {{ asset_lab | to_nice_json }},
{% if "hostname" in asset_info %}
"hostname": {{ asset_info["hostname"] | to_nice_json }},
{% endif %}
{% if "interfaces" in asset_info %}
"interfaces": {{ asset_info["interfaces"] | to_nice_json }},
{% endif %}
{% if "os" in asset_info %}
"os": {{ asset_info["os"] | to_nice_json }},
{% endif %}
{% if "wsus_id" in asset_info %}
"wsus_id": {% if asset_info["wsus_id"]=="" %}null{% else %}{{ asset_info["wsus_id"] | to_nice_json }}{% endif %},
{% endif %}
{% if (asset_errors is defined) and (asset_errors | length > 0) %}
"errors": {{ asset_errors | to_nice_json }},
{% endif %}
"status": {{ asset_info["status"] | to_nice_json }}
}
}
the output is :
Failed to parse YAML in roles/report/templates/asset_info.json.j2: while scanning for the next token
found character '%' that cannot start any token
in "", line 8, column 4:
{% if "hostname" in asset_info %}
^
ansible-review
should be able to detect configuration rules by looking inside current code directory, just like any other build and linting tools pep8, flake8, tox.
Having a user specific config file that is not kept as the same place as the code does not work well for codebases shared across multiple users.
I think that the use of setup.cfg
should be a good idea as it avoids spamming the repository root with tons of config files for each tool. Most tools already support it.
Since these can be used in playbooks/roles alongside other tasks I think it makes sense to treat them as such for linting. If I add a name to an import task and run the linter TaskShouldHaveName this isn't picked up. Any thoughts on this?
We are using the latest RC (0.14rc2) and had a vaulted file vars/pass.yml
and ansible-review crashed with stacktrace when it hit that file. ansible-review is being triggered through pre-commit in this case.
WARN: RoleVars vars/pass.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.1
Traceback (most recent call last):
File "/home/user/.cache/pre-commit/repolIoGop/py_env-python2.7/bin/ansible-review", line 10, in <module>
sys.exit(main())
File "/home/user/.cache/pre-commit/repolIoGop/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/__main__.py", line 102, in main
errors = errors + candidate.review(options, lines)
File "/home/user/.cache/pre-commit/repolIoGop/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/__init__.py", line 76, in review
return utils.review(self, settings, lines)
File "/home/user/.cache/pre-commit/repolIoGop/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/utils/__init__.py", line 121, in review
result = standard.check(candidate, settings)
File "/home/user/.cache/pre-commit/repolIoGop/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/vars.py", line 45, in repeated_vars
for err_key in errors for err_line in errors[err_key]])
TypeError: string indices must be integers, not str
The workaround was to replace the vaulted file with a file containing encrypt_string vaulted strings.
If I let ansible-review
process my group_vars
files, I get messages like this in the output:
WARN: Best practice "Same variable defined in siblings" not met:
ansible/group_vars/test.yml:Inventory is broken:
Attempted to read "/home/duncan/dev/tools/provisioning/ansible/ansible.log" as ini file:
/home/duncan/dev/tools/provisioning/ansible/ansible.log:1:
Expected key=value host variable assignment, got: 16:47:41,708
I'm running ansible-review
like this: git ls-files ansible/. | xargs ansible-review
.
Two things puzzle me about this:
ansible.log
- which is .gitignored
and thus not in the output from git ls-files ansible/.
- so I guess ansible-review
isn't using that list as a list of files to process, but reading the filesystem itself? Is there a way to make ansible-review
ignore some files?group_vars
, or with the inventory, for that matter? Is it assuming ansible.log
is my inventory file?If I exclude the group_vars
folder, this problem goes away: git ls-files ansible/. | grep -v group_vars | xargs ansible-review
- which indicates that maybe ansible-review
does use the list of files?
The stacktrace happens because ansible-review attempts to access/load the inventory plugin and the plugin needs to import msrest
, but this module is not inside the pre-commit environment. Perhaps this is an issue with pre-commit, but I am not exactly sure yet.
Here is the stacktrace:
ansible-review...........................................................Failed
hookid: ansible-review
WARN: Playbook Build.yml does not present standards version. Using latest standards version 0.1
WARN: Best practice "YAML should be correctly indented" not met:
Build.yml:8: lines starting with '- ' should have same or less indentation than previous line (indent_list_items is False)
Traceback (most recent call last):
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/bin/ansible-review", line 10, in <module>
sys.exit(main())
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/__main__.py", line 102, in main
errors = errors + candidate.review(options, lines)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/__init__.py", line 76, in review
return utils.review(self, settings, lines)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/utils/__init__.py", line 121, in review
result = standard.check(candidate, settings)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/groupvars.py", line 45, in same_variable_defined_in_competing_groups
inv = _inv or InventoryManager(invfile).inventory
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/inventory.py", line 86, in __init__
vault_password_files, vault_ids)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansiblereview/inventory.py", line 23, in __init__
sources=inventory)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansible/inventory/manager.py", line 162, in __init__
self.parse_sources(cache=True)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansible/inventory/manager.py", line 220, in parse_sources
parse = self.parse_source(source, cache=cache)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansible/inventory/manager.py", line 254, in parse_source
parsed_this_one = self.parse_source(fullpath, cache=cache)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansible/inventory/manager.py", line 266, in parse_source
for plugin in self._fetch_inventory_plugins():
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansible/inventory/manager.py", line 199, in _fetch_inventory_plugins
plugin = inventory_loader.get(name)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansible/plugins/loader.py", line 552, in get
self._module_cache[path] = self._load_module_source(name, path)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansible/plugins/loader.py", line 530, in _load_module_source
module = imp.load_source(to_native(full_name), to_native(path), module_file)
File "/home/user/.cache/pre-commit/repoN7_lSN/py_env-python2.7/lib/python2.7/site-packages/ansible/plugins/inventory/azure_rm.py", line 186, in <module>
from msrest import ServiceClient, Serializer, Deserializer
ImportError: No module named msrest
Success.
example main.yaml:
- when: ansible_distribution_release == 'xenial'
block:
output - no error.
ansible shows somethig way different:
ERROR! A malformed block was encountered.
The error appears to have been in 'roles/something/tasks/main.yml': line ...., column 3, but may
be elsewhere in the file depending on the exact syntax problem.
The offending line appears to be:
- when: ansible_distribution_release == 'xenial'
^ here
Expected behavior: detect and report malformed blocks.
Look at https://pypi.org/project/ansible-review/#description, it shows:
Project description
The author of this package has not provided a project description
This probably means that you use outdated setuptools when building the dist.
There's also a few other problems I see:
setup.cfg
refers to README.md
, but RST is default for warehouse. To make it understand makrdown you need to provide extra setting with mime type specified correctly (and use up-to-date setuptools)project_urls
metadata optionsdist
, while per current recommendations you should also publish a universal wheel as well. This would also save end-users from having to have modern setuptools in place.setuptools-scm
, which helps to handle this smoothly.So if you're open to such changes let me know as I've gotten a very good experience with all I mentioned above and I can send a PR or/and consult you on those if you have any questions/conserns.
Seeing this when trying to run ansible-review on a diff. Anyone see it before?
⇒ git diff origin branch | ansible-review -c .ansible-review.ini
Traceback (most recent call last):
File "/usr/bin/ansible-review", line 11, in <module>
sys.exit(main())
File "/usr/lib/python2.7/site-packages/ansiblereview/__main__.py", line 76, in main
candidates = get_candidates_from_diff(sys.stdin)
File "/usr/lib/python2.7/site-packages/ansiblereview/__main__.py", line 20, in get_candidates_from_diff
patch = unidiff.PatchSet(sys.stdin)
File "/usr/lib/python2.7/site-packages/unidiff/patch.py", line 353, in __init__
self._parse(data, encoding=encoding)
File "/usr/lib/python2.7/site-packages/unidiff/patch.py", line 399, in _parse
current_file._parse_hunk(line, diff, encoding)
File "/usr/lib/python2.7/site-packages/unidiff/patch.py", line 267, in _parse_hunk
hunk.append(original_line)
File "/usr/lib/python2.7/site-packages/unidiff/patch.py", line 165, in append
s = str(line)
File "/usr/lib/python2.7/site-packages/unidiff/patch.py", line 58, in <lambda>
cls.__str__ = lambda x: x.__unicode__().encode(DEFAULT_ENCODING)
File "/usr/lib/python2.7/site-packages/unidiff/patch.py", line 86, in __str__
return "%s%s" % (self.line_type, self.value)
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 21: ordinal not in range(128)
Looks like ansible-review or ansible-lint is triggering warning if var is detected among jinja2 curly braces but actual code is commented out.
This happens in .yml file, not in the template.
Example:
---
# if below variable is empty then ansible will try to download "{{some_var}}.md5"
i_like_turtles: true
Output:
WARN: Best practice "Variable uses should contain whitespace" not met:
.../defaults/main.yml:35: [EXTRA0001] Variables should have spaces after {{ and before }}
Expected output:
Should not generate WARN at all, because code is commented out.
We have a script that calls review()
, and this crashes in same_variable_defined_in_competing_groups()
, because we call get_group()
on InventoryManager
, and this method does not exist.
$ python $(git rev-parse --show-toplevel)/test/run-review.py
Traceback (most recent call last):
File "/home/kdreyer/dev/plm-playbooks/test/run-review.py", line 121, in <module>
errors = errors + arg.review(settings, lines)
File "/usr/lib/python2.7/site-packages/ansiblereview/__init__.py", line 76, in review
return utils.review(self, settings, lines)
File "/usr/lib/python2.7/site-packages/ansiblereview/utils/__init__.py", line 120, in review
result = standard.check(candidate, settings)
File "/usr/lib/python2.7/site-packages/ansiblereview/groupvars.py", line 76, in same_variable_defined_in_competing_groups
group = inv.get_group(os.path.basename(candidate.path))
AttributeError: 'InventoryManager' object has no attribute 'get_group'
I tried downgrading ansible a few times, and it's the same result for each of these:
ansible-2.6.4-1.fc28
ansible-2.5.5-2.fc28
ansible-2.4.3.0-2.fc28
When this rule runs on an inventory file, it fails with the following traceback:
Traceback (most recent call last):
File "/home/pmacarth/.local/bin/ansible-review", line 11, in <module>
load_entry_point('ansible-review==0.13.4', 'console_scripts', 'ansible-review')()
File "/home/pmacarth/.local/lib/python2.7/site-packages/ansiblereview/__main__.py", line 95, in main
errors = errors + candidate.review(options, lines)
File "/home/pmacarth/.local/lib/python2.7/site-packages/ansiblereview/__init__.py", line 76, in review
return utils.review(self, settings, lines)
File "/home/pmacarth/.local/lib/python2.7/site-packages/ansiblereview/utils/__init__.py", line 120, in review
result = standard.check(candidate, settings)
File "/home/pmacarth/.local/lib/python2.7/site-packages/ansiblereview/groupvars.py", line 56, in same_variable_defined_in_competing_groups
var_manager = ansible.vars.VariableManager()
AttributeError: 'module' object has no attribute 'VariableManager'
Fixing this to import from ansible.vars.manager
only makes it fail with another traceback:
Traceback (most recent call last):
File "/home/pmacarth/.local/bin/ansible-review", line 11, in <module>
load_entry_point('ansible-review==0.13.4', 'console_scripts', 'ansible-review')()
File "/home/pmacarth/.local/lib/python2.7/site-packages/ansiblereview/__main__.py", line 95, in main
errors = errors + candidate.review(options, lines)
File "/home/pmacarth/.local/lib/python2.7/site-packages/ansiblereview/__init__.py", line 76, in review
return utils.review(self, settings, lines)
File "/home/pmacarth/.local/lib/python2.7/site-packages/ansiblereview/utils/__init__.py", line 120, in review
result = standard.check(candidate, settings)
File "/home/pmacarth/.local/lib/python2.7/site-packages/ansiblereview/groupvars.py", line 61, in same_variable_defined_in_competing_groups
inv = _inv or ansible.inventory.Inventory(loader=loader,
AttributeError: 'module' object has no attribute 'Inventory'
This would appear to be due to the overhaul in ansible/ansible#23001, so the corresponding code here should be adjusted to allow it to work with Ansible 2.4.
For now, LICENCE Files are not classified by ansible-review. Also dotfiles (in my case .drone.yml) would be handled as a playbook so i think it would be a better solution to exclude (optionally) dotfiles from review. DO you accept pull requests?
By the way, whats the status of the project? There are Pull Requests not merged since April 2017..
Right now, trying to run ansible-review with the default standards when a group_vars
or host_vars
file is encrypted using ansible-vault
results in the error:
ERROR: Standard "Inventory must be parseable" not met:
inventory/hosts:Inventory is broken: Decryption failed on inventory/group_vars/all/private.yml
The vault password file directive is used in the ansible.cfg file in the playbook directory.
ansible-review should either (1) add an option to supply a vault password file or (2) pull it from the ansible config via the ansible API (not sure how difficult this is).
Hi,
would it be possible that everybody can add an custom ID to a Standard definition in standards.py? I like the idea to define IDs and use these in the review output and also in external documentations or something like this.
Regards
Currently when we use the pre-commit hook with ansible-review
and set the verbose
option on the hook definition in the .pre-commit-config.yaml
file the ansible-review
tool does not react to this option at all - the output of the tool is the same.
I would imagine with verbose: true
, ansible-review will produce the WARN and ERROR messages. With verbose: false
, ansible-review will only show the ERROR messages.
---
repos:
- repo: https://github.com/willthames/ansible-review
rev: v0.14.0rc2
hooks:
- id: ansible-review
name: ansible-review
entry: ansible-review -c .ansible-review
verbose: true
The output from ansible-review
shows some inconsistent formatting. For example, some internal WARN
messages are shown without a rule number reference. I believe all WARN
and ERROR
messages should produce the same formatted output and contain:
Most of the lint rules follow this concept. Here are some internal rules that do not.
From init.py. Two warnings are raised in this code but again they should follow the same output requirements as described above:
if not candidate.version:
candidate.version = standards_latest(standards.standards)
if candidate.expected_version:
if isinstance(candidate, ansiblereview.RoleFile):
warn("%s %s is in a role that contains a meta/main.yml without a declared "
"standards version. "
"Using latest standards version %s" %
(type(candidate).__name__, candidate.path, candidate.version),
settings)
else:
warn("%s %s does not present standards version. "
"Using latest standards version %s" %
(type(candidate).__name__, candidate.path, candidate.version),
settings)
They produce messages like this:
WARN: RoleVars vars/main.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.1
WARN: Task tasks/main.yml is in a role that contains a meta/main.yml without a declared standards version. Using latest standards version 0.1
Also, another example is the following rule that checks YAML indentation. Here is the rule output:
WARN: Best practice "YAML should be correctly indented" not met:
vars/main.yml:4: indentation should increase by 2 chars
However, there is no rule number in the output.
Recommended changes:
warn()
and error()
functions so that you can provide all of this info when these are raised internally.A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.