var-naming can't be skipped

[jrb-github]

Summary

Individual violations of the var-naming rule can't be skipped.

Issue Type

• Bug Report

Ansible and Ansible Lint details

ansible [core 2.11.3]
  config file = None
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.8/dist-packages/ansible
  ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.8.10 (default, Jun  2 2021, 10:49:15) [GCC 9.4.0]
  jinja version = 3.0.1
  libyaml = True

ansible-lint 5.1.3 using ansible 2.11.3

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

OS / ENVIRONMENT

Ubuntu 20.04.3 LTS (Focal Fossa)

STEPS TO REPRODUCE

ansible-lint -p in a directory with a file containing this:

---
  - name: the_name
    vars:
      THE_VAR: "value"

produces this output, as I believe is expected:

WARNING  Listing 1 violation(s) that are fatal
test.yml:4: var-naming Play defines variable 'THE_VAR' within 'vars' section that violates variable naming standards

However, adding # noqa var-naming to the end of any or every line in that file has no effect on the output. Changing the case of THE_VAR to the_var does satisfy the rule.

Note - If you pass the yaml filename to ansible-lint on the command line explicitly as a lintable, it fails syntax-check instead, which I haven't quite figured out, and may be a separate bug.

Desired Behaviour

Adding # noqa var-naming to the end of line 4 results in that violation being skipped.

Actual Behaviour

Adding # noqa var-naming to the end of any line results in that violation still being returned, see steps to reproduce.

[relrod]

It looks like in skip_utils.py, we don't process rule skips for yaml unless the kind is "meta" or we know we're in a playbook/task list/handler list.

I don't know the history here or if this is intentional, but here is a proposed fix that lets noqa apply to yaml/vars/etc as well. It'll need some discussion/review by @ssbarnea and/or @webknjaz, but if it's the right direction, I can add a test and make it into a PR. It seems to pass existing tests.

diff --git a/src/ansiblelint/skip_utils.py b/src/ansiblelint/skip_utils.py
index e8efbd6..60b93c0 100644
--- a/src/ansiblelint/skip_utils.py
+++ b/src/ansiblelint/skip_utils.py
@@ -97,7 +97,7 @@ def _append_skipped_rules(
     # parse file text using 2nd parser library
     ruamel_data = load_data(lintable.content)
 
-    if lintable.kind == 'meta':
+    if lintable.kind in ['yaml', 'requirements', 'vars', 'meta', 'reno']:
         pyyaml_data[0]['skipped_rules'] = _get_rule_skips_from_yaml(ruamel_data)
         return pyyaml_data
 
@@ -114,8 +114,6 @@ def _append_skipped_rules(
             # assume it is a playbook, check needs to be added higher in the
             # call stack, and can remove this except
             return pyyaml_data
-    elif lintable.kind in ['yaml', 'requirements', 'vars', 'meta', 'reno']:
-        return pyyaml_data
     else:
         # For unsupported file types, we return empty skip lists
         return None

[ssbarnea]

I do not see a real issue for not allowing noqa notes on any yaml file type so please raise a PR to fix it. You will find very soon if that is breaking some tests, for good or bad reasons.

[jrb-github]

@relrod I really appreciate your investigation and contribution on this, but I was curious if you ran into any issues, or just haven't gotten to this yet. Thanks!

[SirUli]

I'm running ansible-lint 5.3.2 using ansible 2.12.1 and in this version that seems to be fixed. I guess that could be closed then?

[jrb-github]

@SirUli - Could you provide more detail about how you attempted to reproduce this? Upon seeing your note, I tried myself and I still see this issue.

Specifically, we have a container build that specifies:

ansible==4.4.0
ansible-core==2.11.3
ansible-lint==5.1.2

I confirmed I still see the problem there, and after updating to:

ansible==5.2.0
ansible-core==2.12.1
ansible-lint==5.3.2

and rebuilding the container, this problem still occurs. FYI @ssbarnea.

[SirUli]

Sure. I have in my playbook:

---
- name: Runs foo variable setting
  hosts: "{{ target|default('all') }}"
  gather_facts: false
  tasks:
    - name: "Set the Cluster Node name"
      set_fact:
        CLUSTER_NODE_NAME: "foobar"

The execution looks as follows:

$ ansible-lint
WARNING  Loading custom .yamllint config file, this extends our internal yamllint config.
WARNING  Listing 3 violation(s) that are fatal
var-naming: Task uses 'set_fact' to define variables that violates variable naming standards
playbooks/test.yml:6 Task/Handler: Set the Cluster Node name

If i then change this playbook to:

---
- name: Runs foo variable setting
  hosts: "{{ target|default('all') }}"
  gather_facts: false
  tasks:
    - name: "Set the Cluster Node name"
      set_fact:
        CLUSTER_NODE_NAME: "foobar"  # noqa var-naming

The warning is gone. Now i went in detail through your report ans saw that it was related to the vars section (sorry didn't think this makes a difference but apparent it does because the following playbook:

---
- name: Runs foo variable setting
  hosts: "{{ target|default('all') }}"
  gather_facts: false
  vars:
    SOMEFOO: Bar  # noqa var-naming
  tasks:
    - name: "Set the Cluster Node name"
      set_fact:
        CLUSTER_NODE_NAME: "foobar"  # noqa var-naming

raises:

var-naming: Play defines variable 'SOMEFOO' within 'vars' section that violates variable naming standards
playbooks/test.yml:6

but doesn't on the other one. I'm very sorry - @ssbarnea this is apparently not fixed yet and @relrod already went down to the root cause. Sorry for that! This has to be reopened.

[SirUli]

To make up for my wrong determination i added a PR with a test based on the work of @relrod

[jrb-github]

Just to circle back, I updated to:

ansible==5.3.0
ansible-core==2.12.2
ansible-lint==5.4.0

and this now works, thanks again!