Skip to content

Commit

Permalink
Fix detection of play roles vars missing prefix (#3765)
Browse files Browse the repository at this point in the history
Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>
  • Loading branch information
cavcrosby and ssbarnea committed Sep 28, 2023
1 parent 0920931 commit 9558c92
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 13 deletions.
12 changes: 0 additions & 12 deletions .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,10 @@ jobs:
# proof that we failed to catch a bug by not running it. Using
# distribution should be preferred instead of custom builds.
env:
# vars safe to be passed to wsl:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 828
steps:
- name: Activate WSL1
if: "contains(matrix.shell, 'wsl')"
uses: Vampire/setup-wsl@v2

- name: MacOS workaround for https://github.com/actions/virtual-environments/issues/1187
if: ${{ matrix.os == 'macOS-latest' }}
run: |
sudo sysctl -w net.link.generic.system.hwcksum_tx=0
sudo sysctl -w net.link.generic.system.hwcksum_rx=0
- uses: actions/checkout@v4
with:
fetch-depth: 0 # needed by setuptools-scm
Expand Down
51 changes: 51 additions & 0 deletions examples/playbooks/role_vars_prefix_detection.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
- name: Test role-prefix
hosts: localhost
connection: local
roles:
- role_vars_prefix_detection

- role: role_vars_prefix_detection
var1: val1

- role: role_vars_prefix_detection
var1: val1
become: true
vars:
var2: val2

- role: role_vars_prefix_detection
become: true
environment:
FOO: /bar/barr
role_vars_prefix_detection_var1: val1

- role: role_vars_prefix_detection
vars:
var1: val1

- role: role_vars_prefix_detection
become: true
environment:
BAR: /baz
vars:
var1: val1

- role: role_vars_prefix_detection
become: true
environment:
BAR: /baz
vars:
role_vars_prefix_detection_var1: val1
tasks:
- name: Include1
ansible.builtin.include_role:
name: role_vars_prefix_detection
vars:
var1: val1

- name: Include2
ansible.builtin.include_role:
name: role_vars_prefix_detection
vars:
role_vars_prefix_detection_var2: val2
30 changes: 30 additions & 0 deletions src/ansiblelint/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,36 @@ def main():
"pre_tasks",
"post_tasks",
]
PLAYBOOK_ROLE_KEYWORDS = [
"any_errors_fatal",
"become",
"become_exe",
"become_flags",
"become_method",
"become_user",
"check_mode",
"collections",
"connection",
"debugger",
"delegate_facts",
"delegate_to",
"diff",
"environment",
"ignore_errors",
"ignore_unreachable",
"module_defaults",
"name",
"role",
"no_log",
"port",
"remote_user",
"run_once",
"tags",
"throttle",
"timeout",
"vars",
"when",
]
NESTED_TASK_KEYS = [
"block",
"always",
Expand Down
60 changes: 59 additions & 1 deletion src/ansiblelint/rules/var_naming.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
from ansible.vars.reserved import get_reserved_names

from ansiblelint.config import options
from ansiblelint.constants import ANNOTATION_KEYS, LINE_NUMBER_KEY, RC
from ansiblelint.constants import (
ANNOTATION_KEYS,
LINE_NUMBER_KEY,
PLAYBOOK_ROLE_KEYWORDS,
RC,
)
from ansiblelint.errors import MatchError
from ansiblelint.file_utils import Lintable
from ansiblelint.rules import AnsibleLintRule, RulesCollection
Expand Down Expand Up @@ -193,6 +198,37 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
else our_vars[LINE_NUMBER_KEY]
)
raw_results.append(match_error)
roles = data.get("roles", [])
for role in roles:
if isinstance(role, AnsibleUnicode):
continue
role_fqcn = role.get("role", role.get("name"))
prefix = role_fqcn.split("/" if "/" in role_fqcn else ".")[-1]
for key in list(role.keys()):
if key not in PLAYBOOK_ROLE_KEYWORDS:
match_error = self.get_var_naming_matcherror(key, prefix=prefix)
if match_error:
match_error.filename = str(file.path)
match_error.message += f" (vars: {key})"
match_error.lineno = (
key.ansible_pos[1]
if isinstance(key, AnsibleUnicode)
else role[LINE_NUMBER_KEY]
)
raw_results.append(match_error)

our_vars = role.get("vars", {})
for key in our_vars:
match_error = self.get_var_naming_matcherror(key, prefix=prefix)
if match_error:
match_error.filename = str(file.path)
match_error.message += f" (vars: {key})"
match_error.lineno = (
key.ansible_pos[1]
if isinstance(key, AnsibleUnicode)
else our_vars[LINE_NUMBER_KEY]
)
raw_results.append(match_error)
if raw_results:
lines = file.content.splitlines()
for match in raw_results:
Expand Down Expand Up @@ -360,6 +396,28 @@ def test_var_naming_with_role_prefix(
for result in results:
assert result.tag == "var-naming[no-role-prefix]"

def test_var_naming_with_role_prefix_plays(
default_rules_collection: RulesCollection,
) -> None:
"""Test rule matches."""
results = Runner(
Lintable("examples/playbooks/role_vars_prefix_detection.yml"),
rules=default_rules_collection,
exclude_paths=["examples/roles/role_vars_prefix_detection"],
).run()
expected_errors = (
("var-naming[no-role-prefix]", 9),
("var-naming[no-role-prefix]", 12),
("var-naming[no-role-prefix]", 15),
("var-naming[no-role-prefix]", 25),
("var-naming[no-role-prefix]", 32),
("var-naming[no-role-prefix]", 45),
)
assert len(results) == len(expected_errors)
for idx, result in enumerate(results):
assert result.tag == expected_errors[idx][0]
assert result.lineno == expected_errors[idx][1]

def test_var_naming_with_pattern() -> None:
"""Test rule matches."""
role_path = "examples/roles/var_naming_pattern/tasks/main.yml"
Expand Down

0 comments on commit 9558c92

Please sign in to comment.