Skip to content

Commit

Permalink
Enable var-name rule to detect read-only variables
Browse files Browse the repository at this point in the history
Fixes: #3456
  • Loading branch information
ssbarnea committed May 18, 2023
1 parent 0786920 commit 939dd87
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 2 deletions.
3 changes: 3 additions & 0 deletions examples/playbooks/vars/rule_var_naming_fail.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ CamelCaseButErrorIgnored: true # noqa: var-naming
assert: true # invalid due to being Python reserved keyword
é: true # invalid due to non-ascii character
hosts: true # invalid as being Ansible reserved name
role_name: boo # invalid as being Ansible special magic variable
ansible_facts: {} # special variable that we allow to be written
ansible_python_interpreter: python3 # special variable that we allow to be written
9 changes: 8 additions & 1 deletion src/ansiblelint/rules/var_naming.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ underscore `_` character. Variable names must also start with either an
alphabetic or underscore `_` character.

For more information see the [creating valid variable names][var-names] topic in
Ansible documentation and [Naming things (Good Practices for Ansible)][cop].
Ansible documentation and [Naming things (Good Practices for Ansible)][cop]. You
should also be fully aware of [special variables][magic-vars], also known as
magic variables, especially as most of them can only be read.

Possible errors messages:

Expand All @@ -19,6 +21,7 @@ Possible errors messages:
- `var-naming[no-role-prefix]`: Variables names from within roles should use
`role_name_` as a prefix.
- `var-naming[no-reserved]`: Variables names must not be Ansible reserved names.
- `var-naming[read-only]`: This special variable is read-only.

## Settings

Expand All @@ -40,6 +43,7 @@ var_naming_pattern: "^[a-z_][a-z0-9_]*$"
ALL_CAPS: bar # <- Contains only uppercase characters.
v@r!able: baz # <- Contains special characters.
hosts: [] # <- hosts is an Ansible reserved name
role_name: boo # <-- invalid as being Ansible special magic variable
```

## Correct Code
Expand All @@ -53,8 +57,11 @@ var_naming_pattern: "^[a-z_][a-z0-9_]*$"
no_caps: bar # <- Does not contains uppercase characters.
variable: baz # <- Does not contain special characters.
my_hosts: [] # <- Does not use a reserved names.
my_role_name: boo
```

[cop]: https://redhat-cop.github.io/automation-good-practices/#_naming_things
[var-names]:
https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html#creating-valid-variable-names
[magic-vars]:
https://docs.ansible.com/ansible/latest/reference_appendices/special_variables.html
68 changes: 67 additions & 1 deletion src/ansiblelint/rules/var_naming.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,64 @@ class VariableNamingRule(AnsibleLintRule):
re_pattern_str = options.var_naming_pattern or "^[a-z_][a-z0-9_]*$"
re_pattern = re.compile(re_pattern_str)
reserved_names = get_reserved_names()
# List of special variables that should be treated as read-only. This list
# does not include connection variables, which we expect users to tune in
# specific cases.
# https://docs.ansible.com/ansible/latest/reference_appendices/special_variables.html
read_only_names = {
"ansible_check_mode",
"ansible_collection_name",
"ansible_config_file",
"ansible_dependent_role_names",
"ansible_diff_mode",
"ansible_forks",
"ansible_index_var",
"ansible_inventory_sources",
"ansible_limit",
"ansible_local", # special fact
"ansible_loop",
"ansible_loop_var",
"ansible_parent_role_names",
"ansible_parent_role_paths",
"ansible_play_batch",
"ansible_play_hosts",
"ansible_play_hosts_all",
"ansible_play_name",
"ansible_play_role_names",
"ansible_playbook_python",
"ansible_role_name",
"ansible_role_names",
"ansible_run_tags",
"ansible_search_path",
"ansible_skip_tags",
"ansible_verbosity",
"ansible_version",
"group_names",
"groups",
"hostvars",
"inventory_dir",
"inventory_file",
"inventory_hostname",
"inventory_hostname_short",
"omit",
"play_hosts",
"playbook_dir",
"role_name",
"role_names",
"role_path",
}

# These special variables are used by Ansible but we allow users to set
# them as they might need it in certain cases.
allowed_special_names = {
"ansible_facts",
"ansible_become_user",
"ansible_connection",
"ansible_host",
"ansible_python_interpreter",
"ansible_user",
"ansible_remote_tmp", # no included in docs
}

# pylint: disable=too-many-return-statements
def get_var_naming_matcherror(
Expand All @@ -49,7 +107,7 @@ def get_var_naming_matcherror(
rule=self,
)

if ident in ANNOTATION_KEYS:
if ident in ANNOTATION_KEYS or ident in self.allowed_special_names:
return None

try:
Expand All @@ -75,6 +133,13 @@ def get_var_naming_matcherror(
rule=self,
)

if ident in self.read_only_names:
return MatchError(
tag="var-naming[read-only]",
message=f"This special variable is read-only. ({ident})",
rule=self,
)

# We want to allow use of jinja2 templating for variable names
if "{{" in ident:
return MatchError(
Expand Down Expand Up @@ -251,6 +316,7 @@ def test_invalid_var_name_varsfile(
("var-naming[no-keyword]", 9),
("var-naming[non-ascii]", 10),
("var-naming[no-reserved]", 11),
("var-naming[read-only]", 12),
)
assert len(results) == len(expected_errors)
for idx, result in enumerate(results):
Expand Down

0 comments on commit 939dd87

Please sign in to comment.