semaphor-dk/dansabel

Pre-flight linter for Jinja2/Ansible repositories with Git pre-commit hooks

dansabel's Issues

Warn when register: is indented incorrectly

For example:

- name: copy template
  template:
    src: /foo
    dest: /bar/
    register: did_copy

instead of

- name: copy template
  template:
    src: /foo
    dest: /bar/
  register: did_copy

false positive warning on playbook: "potentially conflicting modules: {'connection', 'tasks'}"

This is on 5964a9f

(py_env-python3.10) [me@example pwd]$ 
(py_env-python3.10) [me@example pwd]$ jinjalint.py lint-example.yml 
WARNING: potentially conflicting modules: {'connection', 'tasks'} at lint-example.yml:0 lines 3-13
(py_env-python3.10) [me@example pwd]$ 

---
# lint-example.yml

- name: show case for linting
  hosts: localhost
  gather_facts: no
  connection: local

  tasks:
    - name: example task
      debug:
        msg: A placeholder.

...

become_*: without become:

As documented become_user: foo or become_flags: ..... without become: has no effect and we should raise it as an issue when encountered.

with_* vs *:

- name: make some directories
  file:
    type: directory
    dest: "/tmp/directory_{{ item }}"
  items:
    - a
    - b

will throw an error:

ERROR! conflicting action statements: file, items                                                                             

because it should be with_items:. It seems like a plausible typo and should be easy to detect that there's more than one key under a name: when the second key isn't one of the finite set of additional allowed toplevel keys {with_items, register, when, etc}.

ansible_distribution == "debian"

It should be "Debian" in uppercase, so either comparison should be case-insensitive, or it should not be compared to "debian".

An easy measure to spot the most common mistakes might be to have a list of various ansible_distribution and creating an annotation when the other comparison operand "looks like" one that we know.

support pre-commit.ci

Hello :)

I wanted to use dansabel on pre-commit.ci but it seems to be importing "too much":

pre-commit-ci/issues#167

Can this be fixed or is it by design? Thanks! 👍

Handle YAML references

what is YAML references / anchors

---
foo: &anchor
  K1: "One"
  K2: "Two"
  <<: &anchor4
    K4: "Four"

bar:
  <<: *anchor
  K2: "I Changed"
  K3: "Three"

kilroy: *anchor4

{
    "foo": {
        "K1": "One",
        "K2": "Two",
        "K4": "Four"
    },
    "bar": {
        "K1": "One",
        "K2": "I Changed",
        "K3": "Three",
        "K4": "Four",
    },
    "kilroy": {
        "K4": "Four"
    }
}

We should be careful to handle cyclic references sensibly.

Issues with parsing a yml template with jinja2 template

HI
I noticed this git hook, parses/lexers fine if the file extension is .j2 or without file extension for jinja2 code, but misses out on validating the YAML code.
If I change the file extension to .yml, it validates the YML but complains about jinja2 with the error below.

while scanning for the next token
found character '%' that cannot start any token
  in "ansible/roles/createCloudformationStack/templates/cloudformation/cfn-route53.yml", line 15, column 2
YAML parser/lexer exit before end of document.

How can I validate both jinja2 and yml when they're present in the same file?

"not" is not recognised as a builtin test

Basically I'm doing variable is not defined and dansabel complains about the not not being builtin?

Should display filename when given multiple source files

Below, a screenshot from a large repository (line 67 in which file ?)

block: not supported in handlers

This bit me today, and should be simple enough to detect too:

https://medium.com/opsops/using-block-for-handlers-in-ansible-a55f45b62a96

copy / file inconsistency

The copy module takes a path: argument, and the file module takes a dest: argument.

This probably ties into a larger issue of mandatory arguments, but for now catching typos in these two as a special case would also be nice.

block: and loop: are mutually exclusive

Below an example of invalid syntax; should be easy enough to add linting of this to the code that generates the "more than one module" warnings in jinjalint.py

- name: do lots of things
  loop: "{{ my_collection }}"
  block:
    - name: foo
      debug: msg=foomsg
    - name: bar
      debug: msg=barmsg

Eric Anderson has a neat workaround that we could suggest to the user: https://ericsysmin.com/2019/06/20/how-to-loop-blocks-of-code-in-ansible/

systemd state: started vs started: yes

- name: start something
  systemd:
    name: myservice
    started: True

is wrong, should be state: started.
I've made this typo a number of times because the other directives are boolean, and it would be nice to catch this.

split()

If there is no abc | split() filter (on older Ansible versions) it may make sense to suggest (abc).split() instead.

Exception when block nesting is incorrect

Trigger in https://github.com/tykling/ansible-roles , thanks to @tykling for the trigger.

(The problem is that we encounter endif twice and close two scopes, one of them begin the for scope. When we then try to pop the endfor scope, we get this exception.

In this case Dansabel should add an annotation and either stop parsing or try to guess what the user meant and continue.

EDIT: It looks like we should also check that what we pop is actually endfor :-)

Namespaced filter names seems not supported (continuation of #19)

I've used main as rev:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━                                                          
13   {{ hash_files|zip(query("file",*hash_files))|community.general.dict }}                                                       
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                          
                                
  13:18 ┏━━━━ variable_begin: {{
  13:20 ┣━━━━━━━━ whitespace:  
  13:21 ┣━━━━━━━━━━━━━━ name: hash_files
  13:31 ┣━━━━━━━━━━ operator: |
  13:32 ┣━━━━━━━━━━━━━━ name: zip
  13:35 ┃ ┏━━━━━━━━━━━━━ operator: (
  13:36 ┃ ┣━━━━━━━━━━━━━━━━━ name: query
  13:41 ┃ ┃ ┏━━━━━━━━━━━━━━━━ operator: (
  13:42 ┃ ┃ ┣━━━━━━━━━━━━━━━━━━ string: "file"
  13:48 ┃ ┃ ┣━━━━━━━━━━━━━━━━ operator: ,
  13:49 ┃ ┃ ┣━━━━━━━━━━━━━━━━ operator: *
  13:50 ┃ ┃ ┣━━━━━━━━━━━━━━━━━━━━ name: hash_files
  13:60 ┃ ┃ ┗━━━━━━━━━━━━━━━━ operator: )
  13:61 ┃ ┗━━━━━━━━━━━━━ operator: )
  13:62 ┣━━━━━━━━━━ operator: |
  13:63 ┣━━━━━━━━━━━━━━ name: community
                ⚞ Not a builtin filter? Maybe: comment, count⚟
  13:72 ┣━━━━━━━━━━ operator: .
  13:73 ┣━━━━━━━━━━━━━━ name: general
  13:80 ┣━━━━━━━━━━ operator: .
  13:81 ┣━━━━━━━━━━━━━━ name: dict
  13:85 ┣━━━━━━━━ whitespace:  
  13:86 ┗━━━━━━ variable_end: }}

There are a lot of namespaces except ansible:
https://docs.ansible.com/ansible/latest/collections/index_filter.html
Also you can create your own non-public collections.

enumerate tags:

in addition to --external it would be great to have --tags

Namespaced filter names seems not supported

63   {{ composes|ansible.utils.keep_keys(target=["name", "short_id", "service", "image", "status"])|ansible.utils.to_paths }}     
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                          
                                                                                                                                  
  63:23 ┏━━━━ variable_begin: {{                                                                                                  
  63:25 ┣━━━━━━━━ whitespace:                                                                                                     
  63:26 ┣━━━━━━━━━━━━━━ name: composes                                                                                            
  63:34 ┣━━━━━━━━━━ operator: |                                                                                                   
  63:35 ┣━━━━━━━━━━━━━━ name: ansible                                                                                             
                ⚞ Not a builtin filter? Maybe: title, slice⚟                                                                      
  63:42 ┣━━━━━━━━━━ operator: .                                                                                                   
  63:43 ┣━━━━━━━━━━━━━━ name: utils                                                                                               
  63:48 ┣━━━━━━━━━━ operator: .                                                                                                   
  63:49 ┣━━━━━━━━━━━━━━ name: keep_keys                                                                                           
  63:58 ┃ ┏━━━━━━━━━━━━━ operator: (                                                                                              
  63:59 ┃ ┣━━━━━━━━━━━━━━━━━ name: target
  63:65 ┃ ┣━━━━━━━━━━━━━ operator: =

PS: Adding ansible to additional_dependencies doesn't helps.

Add support for `{% raw %} ... {% endraw %}`

The linter fails when I use {% raw %} ... {% endraw %} in my templated yml files

Basic static type inference

While full blown type inference may not be feasible, it should be quite doable to catch at least some basic things, like:

selectattr | length

Where Ansible will complain with something like:

The conditional check 'dbms_postgres_reconfig.results | selectattr('changed'
) | selectattr('restart_required') | length' failed.

The error was: Unexpected templating type error occurred on ({% if dbms_postgres_reconfig.results | selectattr('changed') | selectattr('restart_required') | length %} True {% else %} False {% endif %}
):
object of type 'generator' has no len()

The error appears to be in '/home/vagrant/ansible/roles/dbms/tasks/main.yml': line 124, column
 3, but may\nbe elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:
- name: restart postgres if our changes necessitate it 
  ^ here"

Basically we can infer that the selectattr filter is a generator, and that length will not work on a generator, but needs a list.