Add prefix checking for facts and variables

[ssbarnea]

Summary

As one of our clients suggested, they would like to enforce the presence of a particular prefix for all facts they use, something like:

• All facts within a role should match {role_name_*} pattern
• All inventory variables should match inv_* pattern
• All credentials from tower/app should match credential_* pattern
• All survey variables should match survey_* pattern

It should be noted that this rule is closely related to one that we already have for loop_var prefix, where we require users to use unique variables, especially within roles.

Notes

• facts seems relatively easy to implement, by checking set_fact. Still we need to add some context information, so a task would know if is part of a role (and its name) or a playbook.
• inventory could be implemented at source but likely only for static YAML inventory files, as we do not have plans to start linting ini files. Also, it will likely not work for dynamic inventories, at least initially. Still, we could use ansible-inventory output and check only this, making it work for any kind of inventory, but we will not be able to tell exactly where the non-conformance comes from. Still, would help with enforcement.
• No idea about how to check for credential and survey. We need to investigate that.
• We could check that any used variable is matching one of these patterns, making much harder for a user to accidentally introduce non-conforming names.
• What should we do about locally used vars? (contained in one task), like below:

- name: Print something
  vars:
    foo: "some text"
  ansible.builtin.debug:
    msg: "{{ foo }}"

In my view the example above is legit as this variable can be considered "local", so there is no real need to enforce prefixes on it.

Still, if that vars: would be at playbook level or even a block level, I could see more reasons to be careful with its naming.

It is interesting to mention that I did not see any proposal from the customer for adding prefixes to role vars/ and defaults/. I will wait for feedback and update the ticket.

As usual, any feedback is welcomed.

[MarkusTeufelberger]

Another case to consider is register: foo in tasks where other subsequent tasks then use the output of this action.

[ssbarnea]

@MarkusTeufelberger You are right and I was already thinking about it. For example, I would not want to discourage uses of register: result if the result is used right away in the next task. Still, if that variable is used later, it would clearly need a better name.

That would an interesting rule to write, to scan the task tree for bad usages while ignoring basic ones. Still, it would clearly help some people improve their playbook authoring skills.

[MarkusTeufelberger]

Especially problematic with register: result is that this might be used all over the place in different roles and if a host gets skipped on the second result-registering task but still has a "result" from a different task left over, this might lead to very strange errors. So even the basic " register: result + use that result in the next task" pattern can be an issue once a when is involved anywhere else.

[djdanielsson]

I would love to have a rule that requires registered vars to follow some sort of pattern name, I kinda try to do this in my own code most of the time but a rule would be great.

[alice-rc]

So this is an expansion to the COP standards. For the first one, All facts within a role should match {role_name_*} pattern, as long as you set this to be an opt_in rule I don't see an issue:
Just be careful that you aren't breaking a COP rule by implementing this though
COP RULE 3.1.4: Moreover, internal variables (those that are not expected to be set by users) are to be prefixed by two underscores: __foo_variable
This COP rule actually covers the register and task specific variables in the above examples.

For the second one, 'All inventory variables should match inv_* pattern' I don't think this is a good idea. Currently we have multiple dynamic inventory sources and the variables set by those use the plugin name not inv_ as the prefix. I think this would fall onto the latter category (see below).

For the last 2, how would you know which variables fall into the categories:
All survey variables should match survey_* pattern
All credentials from tower/app should match credential_* pattern

These really look not like standards but company conventions that would be covered in a code review process and not in ansible-lint.