firewall icon indicating copy to clipboard operation
firewall copied to clipboard

Make the role variable ansible-lint compliance

Open deamen opened this issue 1 year ago • 2 comments

What would you like to be added: The firewall variable is not ansible-lint compliant:

- name: Test
  hosts: localhost

  tasks:

    - name: Configure Firewall for Web Console
      ansible.builtin.include_role:
        name: linux_system_roles.firewall
      vars:
        firewall:
          service: cockpit
          state: enabled

~/.local/bin/ansible-lint test.yml WARNING Listing 1 violation(s) that are fatal var-naming[no-role-prefix]: Variables names from within roles should use firewall_ as a prefix. (vars: firewall) test.yml:10 Task/Handler: Configure Firewall for Web Console

Read documentation for instructions on how to ignore specific rule violations.

              Rule Violation Summary                     

count tag profile rule associated tags 1 var-naming[no-role-prefix] basic idiom

Failed: 1 failure(s), 0 warning(s) on 1 files. Last profile that met the validation criteria was 'min'.

~/.local/bin/ansible-lint --version
ansible-lint 24.2.0 using ansible-core:2.16.2 ansible-compat:4.1.11 ruamel-yaml:0.18.6 ruamel-yaml-clib:0.2.8

Why is this needed: Other roles in the collection seems to be compliant with ansible-lint, this makes the behaviour consistent. And it makes user's life easier.

deamen avatar Mar 08 '24 09:03 deamen

Where do you face this problem exactly? Would ignoring this rule work for you? Having the main variable to have the same way as the role makes sense to me.

spetrosi avatar Mar 08 '24 12:03 spetrosi

It would be quite difficult to fix this. The problem is the role parameter firewall is the primary role API. If we just unilaterally change this to e.g. firewall_config, it will break every user of this role. Not good.

richm avatar Mar 08 '24 13:03 richm

Sigh, forgot to update my conclusion, just exclude the check in .ansible-lint

skip_list:
  - "var-naming[no-role-prefix]"  # Allow variables without role prefix, a lot of roles don't comply with this.

I think this check should not be enabled on the consumer side, should be enabled on the role developer side. As the consumer does not have control to it and the developers would have a lot of reasons not to honour this rule.

An end user can specifically skip the check on a single role, but since many roles do not comply with this, I think this check is useless on the user side.

deamen avatar Jul 23 '24 22:07 deamen