ansible-review icon indicating copy to clipboard operation
ansible-review copied to clipboard

adding option to allow skipping via tag on yaml_form_rather_than_key_…

Open tonyskidmore opened this issue 4 years ago • 1 comments

In one of our testing roles we using splatting to pass module arguments en bloc. This is very convenient for our particular use case. However, when we run ansible-review against the code we get:

Task arguments appear to be in key value rather than YAML format

An example piece of code is as follows:

- name: Create OpenStack instance
  os_server: "{{ os_server_parameters }}"
  register: os_server_create

With this PR we are able to bypass the warning:

- name: Create OpenStack instance
  os_server: "{{ os_server_parameters }}"
  register: os_server_create
  tags:
  - skip_ansible_lint

tonyskidmore avatar Feb 24 '20 10:02 tonyskidmore

Hi @tonyskidmore , I noticed your PR and was curious about it. So I wrote a simple playbook to test it out called test.yml:

---
- hosts: localhost
  connection: local

  vars:
  - copy_args:
      src: "test.yml"
      dest: "test.yml.bak"

  tasks:
  - copy: "{{ copy_args }}"
    tags:
    - skip_ansible_lint

As you stated, the following warning message is produced from ansible-review:

WARN: Best practice "Use YAML format for tasks and handlers rather than key=value" not met:
test.yml:11: Task arguments appear to be in key value rather than YAML format

Rules within ansible-review seem to be defined using different mechanisms. We can see that in the standards.py file. Some rules defined in there have a rule number associated with it, making it easy to ignore this rule using the comment method # noqa <rule-number>. However, some rules implement some custom code and are not required to provide any rule number. So those rules sadly cannot use the noqa method and require added/duplicate code to allow using the tag or the comment method.

While your code works great, and the PR should be approved easily, my comment is more on the long-term goal which would be to have all rules use the AnsibleLintRule class and that it allows some flexibility. The base class would support all of the methods (comment, tags, etc) to ignore a rule so we don't have to re-write that in different places.

I probably should just move this to a feature request. ;) Sorry for the long comment.

ansiblejunky avatar Feb 24 '20 19:02 ansiblejunky