From a782eafed14b76c4e6a9ab3cccc069705e290fe7 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Thu, 18 May 2023 17:38:32 +0100 Subject: [PATCH] Enable var-name rule to detect read-only variables (#3462) --- .../playbooks/vars/rule_var_naming_fail.yml | 3 + src/ansiblelint/rules/var_naming.md | 10 +++ src/ansiblelint/rules/var_naming.py | 68 ++++++++++++++++++- 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/examples/playbooks/vars/rule_var_naming_fail.yml b/examples/playbooks/vars/rule_var_naming_fail.yml index 24cd490614..3870c263df 100644 --- a/examples/playbooks/vars/rule_var_naming_fail.yml +++ b/examples/playbooks/vars/rule_var_naming_fail.yml @@ -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 diff --git a/src/ansiblelint/rules/var_naming.md b/src/ansiblelint/rules/var_naming.md index 48b77be562..88db2d8b05 100644 --- a/src/ansiblelint/rules/var_naming.md +++ b/src/ansiblelint/rules/var_naming.md @@ -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. @@ -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 @@ -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 @@ -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 diff --git a/src/ansiblelint/rules/var_naming.py b/src/ansiblelint/rules/var_naming.py index 9917a63fcf..de2aefbc4c 100644 --- a/src/ansiblelint/rules/var_naming.py +++ b/src/ansiblelint/rules/var_naming.py @@ -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( @@ -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: @@ -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( @@ -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):