fix: ansible checker run on repos with dependencies"

polymath_code_standard/checkers/ansible.py

@@ -1,21 +1,186 @@
 # SPDX-FileCopyrightText: 2026 Polymath Robotics, Inc.
 # SPDX-License-Identifier: Apache-2.0
 import argparse
+import hashlib
+import json
+from pathlib import Path
+
+import yaml
 
 from polymath_code_standard.checker import CONFIG_DIR, CheckerGroup, Result, check_group
 
+# Where a repo declares the collections and roles its playbooks import.
+REQUIREMENTS = Path('ansible/requirements.yml')
+
+# Where we put them: a cache this hook owns, in the consuming repo but out of the
+# way. It carries its own '*' .gitignore so no downstream repo has to add one.
+#
+# The path is ours to choose because ansible-lint runs `ansible-playbook
+# --syntax-check` as a child process that receives exactly the paths exported
+# below (plus site-packages). Note ansible-lint logs a `<repo>/.ansible/collections`
+# entry belonging to its own isolated runtime which it never passes on -- content
+# installed there is invisible to syntax-check, which then reports every role as
+# "not found".
+CACHE_DIR = Path('.polymath-ansible')
+COLLECTIONS_DIR = CACHE_DIR / 'collections'
+ROLES_DIR = CACHE_DIR / 'roles'
+
+# Installing means downloading, and cloning from git for our own collection: far
+# too slow for every commit. Stamp the requirements digest beside the installed
+# tree and re-install only when the file changes.
+STAMP = CACHE_DIR / 'requirements-sha256'
+
 
 @check_group
 class AnsibleGroup(CheckerGroup):
     name = 'ansible'
 
     def run(self, args: argparse.Namespace) -> list[Result]:
-        return [
+        results = []
+
+        for result in self._install_requirements():
+            results.append(result)
+            if not result.passed:
+                # Linting now would bury the real error under a pile of
+                # syntax-check "role not found" noise.
+                return results
+
+        results.append(
             self._check(
                 'python3',
-                ['-m', 'ansiblelint', '-v', '--force-color', '-c', CONFIG_DIR / 'ansible-lint.yml'],
+                [
+                    '-m',
+                    'ansiblelint',
+                    '-v',
+                    '--force-color',
+                    # Without this the project root is wherever --config lives (inside
+                    # this package), which misplaces requirements lookup and excludes.
+                    '--project-dir',
+                    '.',
+                    '-c',
+                    CONFIG_DIR / 'ansible-lint.yml',
+                ],
                 args.files,
                 name='ansible-lint',
-                env={'ANSIBLE_COLLECTIONS_PATH': 'ansible/collections'},
+                env={
+                    'ANSIBLE_COLLECTIONS_PATH': str(COLLECTIONS_DIR),
+                    'ANSIBLE_ROLES_PATH': str(ROLES_DIR),
+                },
+            )
+        )
+        return results
+
+    @classmethod
+    def _install_requirements(cls) -> list[Result]:
+        """Install declared content, or return nothing when there is nothing to do.
+
+        ansible-lint does install requirements itself, but only from the locations
+        ansible_compat hardcodes (requirements.yml, roles/requirements.yml,
+        collections/requirements.yml, tests/...), resolved against its notion of the
+        project root -- the directory of the --config file we pass, i.e. a path
+        inside this package. Neither matches a Polymath repo, so we install here.
+        """
+        if not REQUIREMENTS.is_file():
+            return []
+
+        raw = REQUIREMENTS.read_bytes()
+        digest = hashlib.sha256(raw).hexdigest()
+        if STAMP.is_file() and STAMP.read_text().strip() == digest:
+            return []
+
+        try:
+            declared = yaml.safe_load(raw) or {}
+        except yaml.YAMLError as exc:
+            return [Result(name='ansible-galaxy', passed=False, output=f'{REQUIREMENTS}: {exc}')]
+
+        cls._make_cache_dir()
+
+        # `collection install` silently skips a roles-only file ("Skipping install,
+        # no requirements found", exit 0) and vice versa, so dispatch on what is
+        # actually declared rather than always running both.
+        installs = [
+            ('collection', COLLECTIONS_DIR, declared.get('collections')),
+            ('role', ROLES_DIR, declared.get('roles')),
+        ]
+
+        results = []
+        for kind, dest, entries in installs:
+            if not entries:
+                continue
+            result = cls._check(
+                'ansible-galaxy',
+                [kind, 'install', '-r', str(REQUIREMENTS), '-p', str(dest)],
+                None,
+                name='ansible-galaxy',
+                # `-p` says where to install but not where to look: galaxy decides
+                # "already installed" from the configured search paths, so anything
+                # present in the developer's ~/.ansible ends with "Nothing to do"
+                # and an empty cache that the linter then cannot resolve.
+                env={'ANSIBLE_COLLECTIONS_PATH': str(COLLECTIONS_DIR), 'ANSIBLE_ROLES_PATH': str(ROLES_DIR)},
+            )
+            results.append(result)
+            if not result.passed:
+                return results
+
+        git_deps = cls._install_git_dependencies(declared.get('collections') or [])
+        results.extend(git_deps)
+        if any(not r.passed for r in git_deps):
+            return results
+
+        STAMP.write_text(f'{digest}\n')
+        return results
+
+    @classmethod
+    def _install_git_dependencies(cls, collections: list) -> list[Result]:
+        """Install the dependencies of git-sourced collections.
+
+        ansible-galaxy resolves dependencies for collections it pulls from a galaxy
+        server, but not for ones installed from git: those arrive with their
+        dependencies unmet, and playbooks then fail on modules like
+        community.general.modprobe. Requiring every consumer to restate the list is
+        how it silently rots, so read it from the collection we just installed.
+
+        Only one pass is needed: whatever we install here comes from a galaxy server,
+        so galaxy resolves its dependencies for us.
+        """
+        if not any(cls._is_git_source(entry) for entry in collections):
+            return []
+
+        installed = {}
+        for manifest in sorted(COLLECTIONS_DIR.glob('ansible_collections/*/*/MANIFEST.json')):
+            try:
+                info = json.loads(manifest.read_text())['collection_info']
+            except (json.JSONDecodeError, KeyError, OSError):
+                continue
+            installed[f'{info["namespace"]}.{info["name"]}'] = info.get('dependencies') or {}
+
+        missing = {name: spec for deps in installed.values() for name, spec in deps.items() if name not in installed}
+        if not missing:
+            return []
+
+        # A '*' requirement is expressed by passing the bare name.
+        targets = [f'{name}:{spec}' if spec and spec != '*' else name for name, spec in sorted(missing.items())]
+        return [
+            cls._check(
+                'ansible-galaxy',
+                ['collection', 'install', *targets, '-p', str(COLLECTIONS_DIR)],
+                None,
+                name='ansible-galaxy',
+                env={'ANSIBLE_COLLECTIONS_PATH': str(COLLECTIONS_DIR), 'ANSIBLE_ROLES_PATH': str(ROLES_DIR)},
             )
         ]
+
+    @staticmethod
+    def _is_git_source(entry: object) -> bool:
+        if not isinstance(entry, dict):
+            return False
+        name = str(entry.get('name', ''))
+        return entry.get('type') == 'git' or name.startswith('git@') or name.endswith('.git')
+
+    @staticmethod
+    def _make_cache_dir() -> None:
+        """Create the cache and make it ignore itself, including on a failed install."""
+        CACHE_DIR.mkdir(parents=True, exist_ok=True)
+        gitignore = CACHE_DIR / '.gitignore'
+        if not gitignore.is_file():
+            gitignore.write_text('*\n')

polymath_code_standard/config/ansible-lint.yml

@@ -1,2 +1,9 @@
 ---
 profile: production
+
+exclude_paths:
+  # The hook's own cache of collections and roles installed from
+  # ansible/requirements.yml. Third-party (or another repo's) content that a
+  # consumer cannot fix, and linting it here would fail commits over violations
+  # in someone else's tree. Keep in sync with CACHE_DIR in checkers/ansible.py.
+  - .polymath-ansible/

pyproject.toml

@@ -37,6 +37,12 @@ dev = [
     "pytest>=9.0.3",
 ]
 
+[tool.pytest.ini_options]
+markers = [
+    # Reaches Ansible Galaxy to install real requirements. Deselect with -m 'not network'.
+    "network: test requires network access",
+]
+
 [project.scripts]
 polymath_code_standard = "polymath_code_standard.runner:main"
 polymath_copyright_header = "polymath_code_standard.insert_license:main"

test_files/ansible/inventory.yml

@@ -0,0 +1,5 @@
+---
+
+all:
+  vars:
+    foo: well hello there

test_files/ansible/playbook.yml

@@ -0,0 +1,20 @@
+---
+
+- name: Configure some computer
+  hosts: all
+  pre_tasks:
+    - name: Ensure host is reachable
+      ansible.builtin.ping:
+
+    # From the ansible.posix collection: syntax-check can only resolve this if the
+    # hook installed requirements.yml somewhere the child process can see.
+    - name: Enable IP forwarding
+      ansible.posix.sysctl:
+        name: net.ipv4.ip_forward
+        value: '1'
+        sysctl_set: true
+        state: present
+        reload: true
+
+  roles:
+    - role: geerlingguy.docker

test_files/ansible/requirements.yml

@@ -0,0 +1,7 @@
+---
+# Both kinds are declared on purpose: `ansible-galaxy collection install` silently
+# ignores a roles-only file and vice versa, so the hook has to dispatch on each key.
+collections:
+  - name: ansible.posix
+roles:
+  - name: geerlingguy.docker

tests/test_ansible.py

@@ -0,0 +1,110 @@
+# SPDX-FileCopyrightText: 2026 Polymath Robotics, Inc.
+# SPDX-License-Identifier: Apache-2.0
+"""Tests for the ansible checker group.
+
+Beyond running ansible-lint, this group installs what a repo declares in
+ansible/requirements.yml, because ansible-lint will not: it only auto-installs
+from paths ansible_compat hardcodes, resolved against its notion of the project
+root -- which is the directory of the --config file we pass, i.e. a path inside
+this package.
+"""
+
+import hashlib
+import json
+import shutil
+from pathlib import Path
+
+import pytest
+
+from polymath_code_standard import runner
+from polymath_code_standard.checker import Result
+from polymath_code_standard.checkers import ansible as ansible_checker
+
+_PROJECT_ROOT = Path(__file__).parent.parent
+
+
+def _write_manifest(root: Path, fqcn: str, dependencies: dict) -> None:
+    """Fake a collection installed by ansible-galaxy, which records deps in MANIFEST.json."""
+    namespace, name = fqcn.split('.')
+    path = root / 'ansible_collections' / namespace / name
+    path.mkdir(parents=True)
+    (path / 'MANIFEST.json').write_text(
+        json.dumps({'collection_info': {'namespace': namespace, 'name': name, 'dependencies': dependencies}})
+    )
+
+
+@pytest.mark.network
+def test_ansible_installs_requirements(tmp_path, monkeypatch):
+    """A downstream repo declaring requirements.yml gets them installed, then linted.
+
+    The fixture playbook imports a role and a module from a collection, neither of
+    which ansible-lint resolves on its own.
+    """
+    shutil.copytree(_PROJECT_ROOT / 'test_files' / 'ansible', tmp_path / 'ansible')
+    monkeypatch.chdir(tmp_path)
+
+    assert runner.main(['ansible', 'ansible/playbook.yml']) == 0
+
+    # A passing lint already proves the syntax-check child resolved both, but assert
+    # the layout too, so moving these paths fails loudly instead of silently
+    # depending on some other collections path that happens to be populated.
+    assert (tmp_path / ansible_checker.COLLECTIONS_DIR / 'ansible_collections' / 'ansible' / 'posix').is_dir()
+    assert (tmp_path / ansible_checker.ROLES_DIR / 'geerlingguy.docker').is_dir()
+
+    # The cache hides itself, so consuming repos need no .gitignore edit.
+    assert (tmp_path / ansible_checker.CACHE_DIR / '.gitignore').read_text().strip() == '*'
+
+    # Second run installs nothing: re-cloning on every commit would be far too slow.
+    assert ansible_checker.AnsibleGroup._install_requirements() == []
+
+
+def test_ansible_stamp_tracks_requirements(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+    (tmp_path / 'ansible').mkdir()
+    requirements = tmp_path / ansible_checker.REQUIREMENTS
+    requirements.write_text('---\ncollections:\n  - name: ansible.posix\n')
+
+    stamp = tmp_path / ansible_checker.STAMP
+    stamp.parent.mkdir(parents=True)
+    stamp.write_text(hashlib.sha256(requirements.read_bytes()).hexdigest())
+    assert ansible_checker.AnsibleGroup._install_requirements() == []
+
+    # Editing requirements.yml invalidates the stamp so the install runs again.
+    requirements.write_text('---\nroles:\n  - name: geerlingguy.docker\n')
+    assert ansible_checker.AnsibleGroup._install_requirements() != []
+
+
+def test_ansible_without_requirements_installs_nothing(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+    assert ansible_checker.AnsibleGroup._install_requirements() == []
+    assert not (tmp_path / ansible_checker.CACHE_DIR).exists()
+
+
+def test_ansible_git_dependencies_detected_from_manifest(tmp_path, monkeypatch):
+    """Unmet dependencies of an installed collection are found, met ones ignored."""
+    monkeypatch.chdir(tmp_path)
+    cache = tmp_path / ansible_checker.COLLECTIONS_DIR
+    _write_manifest(cache, 'polymath.core', {'community.general': '>=10.4', 'amazon.aws': '*'})
+    _write_manifest(cache, 'amazon.aws', {})
+
+    calls = []
+    monkeypatch.setattr(
+        ansible_checker.AnsibleGroup,
+        '_check',
+        classmethod(lambda cls, tool, args, files, **kw: calls.append(args) or Result(name=tool, passed=True)),
+    )
+
+    git_entry = [{'name': 'git@github.com:polymathrobotics/polymath_core.git', 'type': 'git'}]
+    assert ansible_checker.AnsibleGroup._install_git_dependencies(git_entry) == [
+        Result(name='ansible-galaxy', passed=True)
+    ]
+
+    # amazon.aws is already installed; a '*' spec is passed as a bare name.
+    assert calls == [['collection', 'install', 'community.general:>=10.4', '-p', str(ansible_checker.COLLECTIONS_DIR)]]
+
+
+def test_ansible_registry_only_requirements_skip_dependency_pass(tmp_path, monkeypatch):
+    """galaxy resolves dependencies itself for non-git sources, so we stay out of it."""
+    monkeypatch.chdir(tmp_path)
+    _write_manifest(tmp_path / ansible_checker.COLLECTIONS_DIR, 'polymath.core', {'community.general': '>=10.4'})
+    assert ansible_checker.AnsibleGroup._install_git_dependencies([{'name': 'ansible.posix'}]) == []