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

var-naming now prevents use of Ansible reserved names #3460

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion examples/playbooks/vars/rule_var_naming_fail.yml
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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