Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable var-name rule to detect read-only variables #3462

Merged
merged 1 commit into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
10 changes: 10 additions & 0 deletions src/ansiblelint/rules/var_naming.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ 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].

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. While Ansible will
just ignore any attempt to set them, the linter will notify the user, so they
would not be confused about a line that does not effectively do anything.

Possible errors messages:

- `var-naming[non-string]`: Variables names must be strings.
Expand All @@ -19,6 +24,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 +46,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 +60,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