Ignore var-naming[pattern] to foreign role vars (#4095)

The changes to the utils.py's _rolepath were to enable the function to
handle fully qualified collection names (FQCNs). Otherwise, the function
would be unable to parse the role name from the FQCN returning basedir
as the role_path. This can result in additional files with violations
being returned by the _look_for_role_files function.

Author: cavcrosby

.github/workflows/tox.yml

@@ -72,7 +72,7 @@ jobs:
     env:
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 884
+      PYTEST_REQPASS: 886
     steps:
       - uses: actions/checkout@v4
         with:

collections/ansible_collections/local/testcollection/roles/bug4095/tasks/main.yml

@@ -0,0 +1,10 @@
+---
+- name: Test task
+  hosts: localhost
+  connection: local
+  tasks:
+    - name: Reproduce bug related bug 4095
+      ansible.builtin.include_role:
+        name: local.testcollection.bug4095
+      vars:
+        var1: val1

examples/playbooks/bug-4095.yml

@@ -5,7 +5,7 @@
 import keyword
 import re
 import sys
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Any, NamedTuple
 
 from ansible.parsing.yaml.objects import AnsibleUnicode
 from ansible.vars.reserved import get_reserved_names
@@ -22,13 +22,20 @@
 from ansiblelint.rules import AnsibleLintRule, RulesCollection
 from ansiblelint.runner import Runner
 from ansiblelint.skip_utils import get_rule_skips_from_line
-from ansiblelint.text import has_jinja, is_fqcn_or_name
+from ansiblelint.text import has_jinja, is_fqcn, is_fqcn_or_name
 from ansiblelint.utils import parse_yaml_from_file
 
 if TYPE_CHECKING:
     from ansiblelint.utils import Task
 
 
+class Prefix(NamedTuple):
+    """Prefix."""
+
+    value: str = ""
+    from_fqcn: bool = False
+
+
 class VariableNamingRule(AnsibleLintRule):
     """All variables should be named using only lowercase and underscores."""
 
@@ -109,7 +116,7 @@ def get_var_naming_matcherror(
         self,
         ident: str,
         *,
-        prefix: str = "",
+        prefix: Prefix | None = None,
     ) -> MatchError | None:
         """Return a MatchError if the variable name is not valid, otherwise None."""
         if not isinstance(ident, str):  # pragma: no cover
@@ -160,7 +167,9 @@ def get_var_naming_matcherror(
                 rule=self,
             )
 
-        if not bool(self.re_pattern.match(ident)):
+        if not bool(self.re_pattern.match(ident)) and (
+            not prefix or not prefix.from_fqcn
+        ):
             return MatchError(
                 tag="var-naming[pattern]",
                 message=f"Variables names should match {self.re_pattern_str} regex. ({ident})",
@@ -169,13 +178,13 @@ def get_var_naming_matcherror(
 
         if (
             prefix
-            and not ident.lstrip("_").startswith(f"{prefix}_")
-            and not has_jinja(prefix)
-            and is_fqcn_or_name(prefix)
+            and not ident.lstrip("_").startswith(f"{prefix.value}_")
+            and not has_jinja(prefix.value)
+            and is_fqcn_or_name(prefix.value)
         ):
             return MatchError(
                 tag="var-naming[no-role-prefix]",
-                message=f"Variables names from within roles should use {prefix}_ as a prefix.",
+                message=f"Variables names from within roles should use {prefix.value}_ as a prefix.",
                 rule=self,
             )
         return None
@@ -204,10 +213,13 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
             if isinstance(role, AnsibleUnicode):
                 continue
             role_fqcn = role.get("role", role.get("name"))
-            prefix = role_fqcn.split("/" if "/" in role_fqcn else ".")[-1]
+            prefix = self._parse_prefix(role_fqcn)
             for key in list(role.keys()):
                 if key not in PLAYBOOK_ROLE_KEYWORDS:
-                    match_error = self.get_var_naming_matcherror(key, prefix=prefix)
+                    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})"
@@ -220,7 +232,10 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
 
             our_vars = role.get("vars", {})
             for key in our_vars:
-                match_error = self.get_var_naming_matcherror(key, prefix=prefix)
+                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})"
@@ -250,22 +265,25 @@ def matchtask(
     ) -> list[MatchError]:
         """Return matches for task based variables."""
         results = []
-        prefix = ""
+        prefix = Prefix()
         filename = "" if file is None else str(file.path)
         if file and file.parent and file.parent.kind == "role":
-            prefix = file.parent.path.name
+            prefix = Prefix(file.parent.path.name)
         ansible_module = task["action"]["__ansible_module__"]
         # If the task uses the 'vars' section to set variables
         our_vars = task.get("vars", {})
         if ansible_module in ("include_role", "import_role"):
             action = task["action"]
             if isinstance(action, dict):
                 role_fqcn = action.get("name", "")
-                prefix = role_fqcn.split("/" if "/" in role_fqcn else ".")[-1]
+                prefix = self._parse_prefix(role_fqcn)
         else:
-            prefix = ""
+            prefix = Prefix()
         for key in our_vars:
-            match_error = self.get_var_naming_matcherror(key, prefix=prefix)
+            match_error = self.get_var_naming_matcherror(
+                key,
+                prefix=prefix,
+            )
             if match_error:
                 match_error.filename = filename
                 match_error.lineno = our_vars[LINE_NUMBER_KEY]
@@ -290,7 +308,10 @@ def matchtask(
         # If the task registers a variable
         registered_var = task.get("register", None)
         if registered_var:
-            match_error = self.get_var_naming_matcherror(registered_var, prefix=prefix)
+            match_error = self.get_var_naming_matcherror(
+                registered_var,
+                prefix=prefix,
+            )
             if match_error:
                 match_error.message += f" (register: {registered_var})"
                 match_error.filename = filename
@@ -309,7 +330,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
         if str(file.kind) == "vars" and file.data:
             meta_data = parse_yaml_from_file(str(file.path))
             for key in meta_data:
-                prefix = file.role if file.role else ""
+                prefix = Prefix(file.role) if file.role else Prefix()
                 match_error = self.get_var_naming_matcherror(key, prefix=prefix)
                 if match_error:
                     match_error.filename = filename
@@ -330,6 +351,9 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
             results.extend(super().matchyaml(file))
         return results
 
+    def _parse_prefix(self, fqcn: str) -> Prefix:
+        return Prefix("" if "." in fqcn else fqcn.split("/")[-1], is_fqcn(fqcn))
+
 
 # testing code to be loaded only with pytest or when executed the rule file
 if "pytest" in sys.modules:
@@ -434,6 +458,17 @@ def test_var_naming_with_pattern() -> None:
         assert result.returncode == RC.SUCCESS
         assert "var-naming" not in result.stdout
 
+    def test_var_naming_with_pattern_foreign_role() -> None:
+        """Test rule matches."""
+        role_path = "examples/playbooks/bug-4095.yml"
+        conf_path = "examples/roles/var_naming_pattern/.ansible-lint"
+        result = run_ansible_lint(
+            f"--config-file={conf_path}",
+            role_path,
+        )
+        assert result.returncode == RC.SUCCESS
+        assert "var-naming" not in result.stdout
+
     def test_var_naming_with_include_tasks_and_vars() -> None:
         """Test with include tasks and vars."""
         role_path = "examples/roles/var_naming_pattern/tasks/include_task_with_vars.yml"

src/ansiblelint/rules/var_naming.py

@@ -55,3 +55,10 @@ def has_glob(value: str) -> bool:
 def is_fqcn_or_name(value: str) -> bool:
     """Return true if a string seems to be a module/filter old name or a fully qualified one."""
     return bool(isinstance(value, str) and RE_IS_FQCN_OR_NAME.search(value))
+
+
+@cache
+def is_fqcn(value: str) -> bool:
+    """Return true if a string seems to be a fully qualified collection name."""
+    match = RE_IS_FQCN_OR_NAME.search(value)
+    return bool(isinstance(value, str) and match and match.group(1))

src/ansiblelint/text.py

@@ -76,7 +76,7 @@
 from ansiblelint.errors import MatchError
 from ansiblelint.file_utils import Lintable, discover_lintables
 from ansiblelint.skip_utils import is_nested_task
-from ansiblelint.text import has_jinja, removeprefix
+from ansiblelint.text import has_jinja, is_fqcn, removeprefix
 
 if TYPE_CHECKING:
     from ansiblelint.rules import RulesCollection
@@ -505,21 +505,40 @@ def _get_task_handler_children_for_tasks_or_playbooks(
 
 def _rolepath(basedir: str, role: str) -> str | None:
     role_path = None
+    namespace_name, collection_name, role_name = (
+        role.split(".") if is_fqcn(role) else ("", "", role)
+    )
 
     possible_paths = [
         # if included from a playbook
-        path_dwim(basedir, os.path.join("roles", role)),
-        path_dwim(basedir, role),
+        path_dwim(basedir, os.path.join("roles", role_name)),
+        path_dwim(basedir, role_name),
         # if included from roles/[role]/meta/main.yml
-        path_dwim(basedir, os.path.join("..", "..", "..", "roles", role)),
-        path_dwim(basedir, os.path.join("..", "..", role)),
+        path_dwim(basedir, os.path.join("..", "..", "..", "roles", role_name)),
+        path_dwim(basedir, os.path.join("..", "..", role_name)),
         # if checking a role in the current directory
-        path_dwim(basedir, os.path.join("..", role)),
+        path_dwim(basedir, os.path.join("..", role_name)),
     ]
 
     for loc in get_app(cached=True).runtime.config.default_roles_path:
         loc = os.path.expanduser(loc)
-        possible_paths.append(path_dwim(loc, role))
+        possible_paths.append(path_dwim(loc, role_name))
+
+    if namespace_name and collection_name:
+        for loc in get_app(cached=True).runtime.config.collections_paths:
+            loc = os.path.expanduser(loc)
+            possible_paths.append(
+                path_dwim(
+                    loc,
+                    os.path.join(
+                        "ansible_collections",
+                        namespace_name,
+                        collection_name,
+                        "roles",
+                        role_name,
+                    ),
+                ),
+            )
 
     possible_paths.append(path_dwim(basedir, ""))
 

src/ansiblelint/utils.py

@@ -480,3 +480,13 @@ def test_find_children_in_module(default_rules_collection: RulesCollection) -> N
     # Child correctly looks like a YAML file
     assert child.base_kind == "text/yaml"
     assert child.content.startswith("---")
+
+
+def test_find_children_in_playbook(default_rules_collection: RulesCollection) -> None:
+    """Verify correct function of find_children() in playbooks."""
+    lintable = Lintable("examples/playbooks/bug-4095.yml")
+    children = Runner(
+        rules=default_rules_collection,
+    ).find_children(lintable)
+    assert len(children) == 1
+    assert children[0].role == "bug4095"