Skip to content

Commit

Permalink
var-naming now prevents use of Ansible reserved names (#3460)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea committed May 18, 2023
1 parent 02804cd commit 50bdeca
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 7 deletions.
3 changes: 2 additions & 1 deletion examples/playbooks/vars/rule_var_naming_fail.yml
Expand Up @@ -6,5 +6,6 @@ this_is_valid: # valid because content is a dict, not a variable
ALL_CAPS_ARE_BAD_TOO: ... # invalid
"{{ 'test_' }}var": "value" # noqa: schema
CamelCaseButErrorIgnored: true # noqa: var-naming
assert: true # invalid due to reserved keyword
assert: true # invalid due to being Python reserved keyword
é: true # invalid due to non-ascii character
hosts: true # invalid as being Ansible reserved name
3 changes: 3 additions & 0 deletions src/ansiblelint/rules/var_naming.md
Expand Up @@ -18,6 +18,7 @@ Possible errors messages:
- `var-naming[pattern]`: Variables names should match ... regex.
- `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.

## Settings

Expand All @@ -38,6 +39,7 @@ var_naming_pattern: "^[a-z_][a-z0-9_]*$"
CamelCase: true # <- Contains a mix of lowercase and uppercase characters.
ALL_CAPS: bar # <- Contains only uppercase characters.
v@r!able: baz # <- Contains special characters.
hosts: [] # <- hosts is an Ansible reserved name
```

## Correct Code
Expand All @@ -50,6 +52,7 @@ var_naming_pattern: "^[a-z_][a-z0-9_]*$"
lowercase: true # <- Contains only lowercase characters.
no_caps: bar # <- Does not contains uppercase characters.
variable: baz # <- Does not contain special characters.
my_hosts: [] # <- Does not use a reserved names.
```

[cop]: https://redhat-cop.github.io/automation-good-practices/#_naming_things
Expand Down
19 changes: 14 additions & 5 deletions src/ansiblelint/rules/var_naming.py
Expand Up @@ -7,6 +7,7 @@
from typing import TYPE_CHECKING, Any

from ansible.parsing.yaml.objects import AnsibleUnicode
from ansible.vars.reserved import get_reserved_names

from ansiblelint.config import options
from ansiblelint.constants import LINE_NUMBER_KEY, RC
Expand All @@ -31,8 +32,9 @@ class VariableNamingRule(AnsibleLintRule):
needs_raw_task = True
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()

# pylint: disable=too-many-return-statements)
# pylint: disable=too-many-return-statements
def get_var_naming_matcherror(
self,
ident: str,
Expand All @@ -52,14 +54,21 @@ def get_var_naming_matcherror(
except UnicodeEncodeError:
return MatchError(
tag="var-naming[non-ascii]",
message="Variables names must be ASCII.",
message=f"Variables names must be ASCII. ({ident})",
rule=self,
)

if keyword.iskeyword(ident):
return MatchError(
tag="var-naming[no-keyword]",
message="Variables names must not be Python keywords.",
message=f"Variables names must not be Python keywords. ({ident})",
rule=self,
)

if ident in self.reserved_names:
return MatchError(
tag="var-naming[no-reserved]",
message=f"Variables names must not be Ansible reserved names. ({ident})",
rule=self,
)

Expand All @@ -74,7 +83,7 @@ def get_var_naming_matcherror(
if not bool(self.re_pattern.match(ident)):
return MatchError(
tag="var-naming[pattern]",
message=f"Variables names should match {self.re_pattern_str} regex.",
message=f"Variables names should match {self.re_pattern_str} regex. ({ident})",
rule=self,
)

Expand Down Expand Up @@ -140,7 +149,6 @@ def matchtask(
results.append(match_error)

# If the task uses the 'set_fact' module
# breakpoint()
ansible_module = task["action"]["__ansible_module__"]
if ansible_module == "set_fact":
for key in filter(
Expand Down Expand Up @@ -239,6 +247,7 @@ def test_invalid_var_name_varsfile(
("var-naming[no-jinja]", 7),
("var-naming[no-keyword]", 9),
("var-naming[non-ascii]", 10),
("var-naming[no-reserved]", 11),
)
assert len(results) == len(expected_errors)
for idx, result in enumerate(results):
Expand Down
1 change: 0 additions & 1 deletion src/ansiblelint/runner.py
Expand Up @@ -147,7 +147,6 @@ def run(self) -> list[MatchError]:
)
else:
filename = warn.source
# breakpoint()
match = MatchError(
message=warn.message
if isinstance(warn.message, str)
Expand Down

0 comments on commit 50bdeca

Please sign in to comment.