var-spacing: False-positive in name

[sblask]

Summary

I am getting:

var-spacing:Jinja2 variables and filters should have spaces before and after.
ansible/includes/zipped_executable.yml:5 Task/Handler: Add {{ product | title }}

with ansible-lint 6.3

Issue Type

• Bug Report

Ansible and Ansible Lint details

$ ansible --version
ansible [core 2.13.0]
  config file = /Users/sebastian/Clones/dotfiles/ansible.cfg
  configured module search path = ['/Users/sebastian/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/sebastian/.opt/ansible/lib/python3.9/site-packages/ansible
  ansible collection location = /Users/sebastian/.ansible/collections:/usr/share/ansible/collections
  executable location = /Users/sebastian/.bin/ansible
  python version = 3.9.4 (default, Apr 29 2021, 10:26:40) [Clang 12.0.5 (clang-1205.0.22.9)]
  jinja version = 3.0.3
  libyaml = True

$ ansible-lint --version
WARNING: PATH altered to include /Users/sebastian/.opt/ansible/bin
ansible-lint 6.3.0 using ansible 2.13.0

• ansible installation method: pip
• ansible-lint installation method: pip

OS / ENVIRONMENT

MacOS

STEPS TO REPRODUCE

See https://github.com/sblask/dotfiles/runs/6824840729?check_suite_focus=true

You can also just run ansible-lint on https://github.com/sblask/dotfiles/blob/main/ansible/includes/zipped_executable.yml to get the following:

$ ansible-lint ansible/includes/zipped_executable.yml
WARNING: PATH altered to include /Users/sebastian/.opt/ansible/bin
WARNING  Listing 3 violation(s) that are fatal
var-spacing: Jinja2 variables and filters should have spaces before and after.
ansible/includes/zipped_executable.yml:5 Task/Handler: Add {{ product | title  }}

risky-file-permissions: File permissions unset or incorrect.
ansible/includes/zipped_executable.yml:8 Task/Handler: Download {{ product | title }}

var-spacing: Jinja2 variables and filters should have spaces before and after.
ansible/includes/zipped_executable.yml:13 Task/Handler: Unpack {{ product | title }}

You can skip specific rules or tags by adding them to your configuration file:
# .config/ansible-lint.yml
warn_list:  # or 'skip_list' to silence them completely
  - experimental  # all rules tagged as experimental
  - var-spacing  # Jinja2 variables and filters should have spaces before and after.

Finished with 2 failure(s), 1 warning(s) on 1 files.

Desired Behavior

There should be no warning.

Actual Behavior

There is a warning even though the spacing is fine and even though https://github.com/sblask/dotfiles/blob/main/ansible/includes/zipped_executable.yml has the same pattern in the name 7 times, I only get a warning for 2

[mweinelt]

Similiar situation with 6.3.0.

❯ ansible-lint networkd_generate.yml
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: networkd_generate.yml
WARNING  Listing 1 violation(s) that are fatal
var-spacing: Jinja2 variables and filters should have spaces before and after.
networkd_generate.yml:20 Task/Handler: sysctl net.ipv6.conf.*.accept_ra

You can skip specific rules or tags by adding them to your configuration file:
# .config/ansible-lint.yml
warn_list:  # or 'skip_list' to silence them completely
  - var-spacing  # Jinja2 variables and filters should have spaces before and after.

Finished with 1 failure(s), 0 warning(s) on 1 files.

Line 20 is the first line of this task:

    - name: "sysctl net.ipv6.conf.*.accept_ra"
      ansible.builtin.command:
        cmd: "sysctl -n net.ipv6.conf.{{ interface }}.accept_ra"
      loop: "{{ ansible_interfaces | select('match', '^(eth|en)') | list }}"
      loop_control:
        loop_var: interface
      register: "net_ipv6_conf_interface_accept_ra"
      changed_when: false

[LarsKollstedt]

I also stumbled into this:

> ansible-lint --version
ansible-lint 6.3.0 using ansible 2.13.0
> ansible-lint junos.yml
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: ./junos.yml
WARNING  Listing 2 violation(s) that are fatal
var-spacing: Jinja2 variables and filters should have spaces before and after.
roles/junos_version/tasks/main.yml:3 Task/Handler: Retrieve JunOS version
var-spacing: Jinja2 variables and filters should have spaces before and after.
roles/junos_version/tasks/main.yml:13 Task/Handler: Parse JunOS version
You can skip specific rules or tags by adding them to your configuration file:
# .config/ansible-lint.yml
warn_list:  # or 'skip_list' to silence them completely
  - var-spacing  # Jinja2 variables and filters should have spaces before and after.
Finished with 2 failure(s), 0 warning(s) on 2 files.

The relevant parts of the task file are:

- name: "Retrieve JunOS version"
  when: ansible_net_version is not defined
  check_mode: false
  block:
[...]
    - name: "Parse JunOS version"
      ansible.builtin.set_fact:
        ansible_net_version: '{{ sh_version.stdout | regex_search("JUNOS (Base OS boot|Software Release) \[([^]]+)\]", "\2") | first }}'

My current workarround is:

    - name: "Parse JunOS version"
      ansible.builtin.set_fact:
        ansible_net_version: '{{ sh_version.stdout | regex_search("JUNOS (Base OS boot|Software Release) \[([^]]+)\]", "\2") | first }}'  # noqa var-spacing

But I would like to see this newly introduced false positive fixed. ;-)

[Daniel-at-github]

Another example:

var-spacing: Jinja2 variables and filters should have spaces before and after.
vars/cas.yml:5 .cas_shared_instance_config

Referring to:

cas_shared_instance_config: >-
  {{ grp_all_cas_instances_shared_config[cas_instance.split("|").0 | trim] }}

[jantari]

Another example of a false positive where this rule triggers for me (on json_query()):

- name: Save list of all configurable interfaces
  ansible.builtin.set_fact:
    _configurable_interfaces: >
      {{ _available_interfaces.meta.results | json_query("[?!mac_address || mac_address != '00:00:00:00:00:00'].name") | list | sort }}

and

- name: Assert that DHCP interfaces do not have an IP or gateway set
  ansible.builtin.assert:
    that:
      - item.subnet is not defined
      - item.gateway is not defined
    fail_msg: 'Interface is set to use DHCP but also has a static IP or static gateway configured!'
    quiet: true
  loop: >
    {{ interfaces | json_query("[?!mode || mode == 'dhcp']") }}
  loop_control:
    label: '{{ item.name }}'

[ssbarnea]

That bug is valid but it will not be easy to fix because current implementation uses regex, so i quite limited. If someone can make a patch and use jinja2 parsing to check it, it would be awesome.

[leogiraldimg]

I had the same problem here in the following example:

- name: Execute command
  shell: "set -o pipefail && egrep -R 'stringA|stringB' /apps/smartclick/log/* | grep {{ input.user_id }}"
  register: result
  become: true
  changed_when: "result.stdout is defined and \
                result.stdout | length > 0 and \
                'link' in result.stdout"
  failed_when:
    - result.stderr is defined
    - result.stderr | length > 0

As a workaround I inserted the "# noqa var-spacing" comment:

- name: Execute command
  shell: "set -o pipefail && egrep -R 'stringA|stringB' /apps/smartclick/log/* | grep {{ input.user_id }}" # noqa var-spacing
  register: result
  become: true
  changed_when: "result.stdout is defined and \
                result.stdout | length > 0 and \
                'link' in result.stdout"
  failed_when:
    - result.stderr is defined
    - result.stderr | length > 0

[yan12125]

Yet another false positive: (slightly modified to remove sensitive information)

- name: setup nginx for nextcloud
  ansible.builtin.include_role:
    name: nginx
    tasks_from: vhost
  vars:
    server_name: nextcloud.example.com
    proxy_type: http
    vhost_conf: |
      # https://docs.nextcloud.com/server/latest/admin_manual/installation/nginx.html#nextcloud-in-the-webroot-of-nginx

      root /usr/share/webapps/nextcloud;
      index index.php index.html;

      location ~ \.php(?:$|/) {
          fastcgi_split_path_info ^(.+?\.php)(/.*)$;

          include fastcgi.conf;
          fastcgi_param PATH_INFO $fastcgi_path_info;

          # Avoid sending the security headers twice
          # https://github.com/nextcloud/server/blob/v23.0.1/lib/private/legacy/OC_Response.php#L97
          fastcgi_param modHeadersAvailable true;
          fastcgi_pass unix:{{ php.fpm_sockdir }}/nextcloud.sock;
      }

      # Image paths do not contain index.php regardless of `front_controller_active`
      # https://github.com/nextcloud/server/blob/v23.0.1/lib/private/URLGenerator.php#L179
      location / {
          try_files $uri /index.php$request_uri;
      }

@ssbarnea Apparently there are many false positives. How about disabling the var-spacing rule by default (ex: mark as opt-in) until a better implementation comes?

[ssbarnea]

We will not disable this rule but we have plans to fix in the following weeks. For the moment you can either use noqa to disable the rule in your config.

To give some insights why not doing this: the rule is not new and the number of false positive is relatively small to the positive ones.

We already discussed about this issue last week and we identified a way to use jinja2 parser instead of a regular expression.

[yan12125]

Got it. Thanks for clarifying the plan. I'm willing to test it if there is a patch.