ansible / ansible-lint

ansible-lint checks playbooks for practices and behavior that could potentially be improved and can fix some of the most common ones for you
https://ansible.readthedocs.io/projects/lint/
GNU General Public License v3.0
3.49k stars 663 forks source link

`var-naming` is enforcing personal preference #2870

Closed The-Judge closed 1 year ago

The-Judge commented 1 year ago
Summary

var-naming seems to enforce personal preferences on variable naming, that are neither required nor recommended by Ansible, nor is a rationale provided in the docs; not even in the doc referenced in that RTD-Page (creating valid variable names). Also, the check seems not to be configurable to change this behavior.

What makes absolute sense is that it enforces stuff that is plain wrong in Ansible, like using - instead of _ or using special characters. But no Ansible Doc I know (or is referenced by Ansible-lint) declares Uppercase variable names wrong. The only indicator that this may be considered best practice is that the examples in creating valid variable names do it like that. But: Examples != Rules.

CamelCase may not look very nice to some people, but nowhere it is stated that you may not use that in Ansible variable names. That's why I state this is personal preference being enforced here.

Other Ansible-lint rules offer sub-rules that can be enabled/disabled individually; such as name, which has name[casing], name[prefix], name[template], ...

Issue Type
OS / ENVIRONMENT
# ansible-lint --version
6.10.2
STEPS TO REPRODUCE
  1. Activate var-naming rule in ansible-lint
  2. Create an Ansible file that uses uppercase letters in a variable name, like this:
---
- name: demo
  set_fact:
    thisIsSparta: yes
...
  1. Check the file using ansible-lint.
Desired Behavior

The module should do one of the following things:

  1. Check only for things that are officially (by Ansible upstream) considered bad practice. In this case, this reference should be in the ansible-lint docs as a rationale.
  2. Offer the casing as a sub-rule that users can either activate or deactivate (like var-naming[casing]).
  3. Provide an objective rationale at var-naming why this is a must-have.

My favorited solution by far would be option 2.

Actual Behavior

See STEPS TO REPRODUCE

briantist commented 1 year ago

In some sense, I would say all linting is subjective (otherwise it's a syntax check).

But I think a casing sub-rule, and maybe others that cover various aspects of var-naming, could be a good idea.

konstruktoid commented 1 year ago

Have a look at https://redhat-cop.github.io/automation-good-practices/#_naming_things, it is mentioned in a few PRs and tickets

cidrblock commented 1 year ago

The regex used for this rule is user-configurable, but not documented. We'll fix that.

WRT to subrules, I personally like the idea, I'm sure a PR for it would be welcomed.