From 12c1678111a5e9f79afec048f4d4162d44a25e24 Mon Sep 17 00:00:00 2001 From: Conner Crosby Date: Sat, 9 Nov 2024 18:35:09 -0500 Subject: [PATCH] Accommodate specified inventory files The ansible_cfg_inventory conditional in the _get_inventory_files method is used because otherwise, when not passing in an inventory file via CLI, inventory_files would set to the /etc/ansible/hosts (the default value for the DEFAULT_HOST_LIST config). In most cases, I'd presume that wouldn't be desired. --- examples/playbooks/test_using_inventory.yml | 11 ++++ inventories/bad_inventory | 7 ++ inventories/bar | 4 ++ inventories/baz | 4 ++ inventories/foo | 4 ++ src/ansiblelint/cli.py | 8 +++ src/ansiblelint/config.py | 1 + src/ansiblelint/runner.py | 73 +++++++++++++++++++-- test/test_app.py | 68 +++++++++++++++++++ tox.ini | 2 +- 10 files changed, 176 insertions(+), 6 deletions(-) create mode 100644 examples/playbooks/test_using_inventory.yml create mode 100644 inventories/bad_inventory create mode 100644 inventories/bar create mode 100644 inventories/baz create mode 100644 inventories/foo diff --git a/examples/playbooks/test_using_inventory.yml b/examples/playbooks/test_using_inventory.yml new file mode 100644 index 0000000000..acc2743343 --- /dev/null +++ b/examples/playbooks/test_using_inventory.yml @@ -0,0 +1,11 @@ +--- +- name: Test + hosts: + - group_name + serial: "{{ batch | default(groups['group_name'] | length) }}" + gather_facts: false + tasks: + - name: Debug + delegate_to: localhost + ansible.builtin.debug: + msg: "{{ batch | default(groups['group_name'] | length) }}" diff --git a/inventories/bad_inventory b/inventories/bad_inventory new file mode 100644 index 0000000000..5eb9dd7229 --- /dev/null +++ b/inventories/bad_inventory @@ -0,0 +1,7 @@ +[group] += = I += = am += = not += = a += = valid += = inventory diff --git a/inventories/bar b/inventories/bar new file mode 100644 index 0000000000..2aa15ade4e --- /dev/null +++ b/inventories/bar @@ -0,0 +1,4 @@ +not_the_group_name: + hosts: + host1: + host2: diff --git a/inventories/baz b/inventories/baz new file mode 100644 index 0000000000..ff76b63cdf --- /dev/null +++ b/inventories/baz @@ -0,0 +1,4 @@ +group_name: + hosts: + host1: + host2: diff --git a/inventories/foo b/inventories/foo new file mode 100644 index 0000000000..ff76b63cdf --- /dev/null +++ b/inventories/foo @@ -0,0 +1,4 @@ +group_name: + hosts: + host1: + host2: diff --git a/src/ansiblelint/cli.py b/src/ansiblelint/cli.py index 6d96e5b795..4054d0990c 100644 --- a/src/ansiblelint/cli.py +++ b/src/ansiblelint/cli.py @@ -456,6 +456,14 @@ def get_cli_parser() -> argparse.ArgumentParser: default=None, help=f"Specify ignore file to use. By default it will look for '{IGNORE_FILE.default}' or '{IGNORE_FILE.alternative}'", ) + parser.add_argument( + "-I", + "--inventory", + dest="inventory", + action="append", + type=str, + help="Specify inventory host path or comma separated host list", + ) parser.add_argument( "--offline", dest="offline", diff --git a/src/ansiblelint/config.py b/src/ansiblelint/config.py index e472cc7e49..99e5e7b853 100644 --- a/src/ansiblelint/config.py +++ b/src/ansiblelint/config.py @@ -173,6 +173,7 @@ class Options: # pylint: disable=too-many-instance-attributes version: bool = False # display version command list_profiles: bool = False # display profiles command ignore_file: Path | None = None + inventory: list[str] | None = None max_tasks: int = 100 max_block_depth: int = 20 # Refer to https://docs.ansible.com/ansible/latest/reference_appendices/release_and_maintenance.html#ansible-core-support-matrix diff --git a/src/ansiblelint/runner.py b/src/ansiblelint/runner.py index 75767883ca..7f9e6d2cdf 100644 --- a/src/ansiblelint/runner.py +++ b/src/ansiblelint/runner.py @@ -18,8 +18,12 @@ from pathlib import Path from tempfile import NamedTemporaryFile from typing import TYPE_CHECKING, Any +from unittest import mock +import ansible.inventory.manager +from ansible.config.manager import ConfigManager from ansible.errors import AnsibleError +from ansible.parsing.dataloader import DataLoader from ansible.parsing.splitter import split_args from ansible.parsing.yaml.constructor import AnsibleMapping from ansible.plugins.loader import add_all_plugin_dirs @@ -340,13 +344,16 @@ def _get_ansible_syntax_check_matches( playbook_path = fh.name else: playbook_path = str(lintable.path.expanduser()) - # To avoid noisy warnings we pass localhost as current inventory: - # [WARNING]: No inventory was parsed, only implicit localhost is available - # [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all' cmd = [ "ansible-playbook", - "-i", - "localhost,", + *[ + inventory_opt + for inventory_opts in [ + ("-i", inventory_file) + for inventory_file in self._get_inventory_files(app) + ] + for inventory_opt in inventory_opts + ], "--syntax-check", playbook_path, ] @@ -451,6 +458,62 @@ def _get_ansible_syntax_check_matches( fh.close() return results + def _get_inventory_files(self, app: App) -> list[str]: + config_mgr = ConfigManager() + ansible_cfg_inventory = config_mgr.get_config_value( + "DEFAULT_HOST_LIST", + ) + if app.options.inventory or ansible_cfg_inventory != [ + config_mgr.get_configuration_definitions()["DEFAULT_HOST_LIST"].get( + "default", + ), + ]: + inventory_files = [ + inventory_file + for inventory_list in [ + # creates nested inventory list + (inventory.split(",") if "," in inventory else [inventory]) + for inventory in ( + app.options.inventory + if app.options.inventory + else ansible_cfg_inventory + ) + ] + for inventory_file in inventory_list + ] + + # silence noise when using parse_source + with mock.patch.object( + ansible.inventory.manager, + "display", + mock.Mock(), + ): + for inventory_file in inventory_files: + if not Path(inventory_file).exists(): + _logger.warning( + "Unable to use %s as an inventory source: no such file or directory", + inventory_file, + ) + elif os.access( + inventory_file, + os.R_OK, + ) and not ansible.inventory.manager.InventoryManager( + DataLoader(), + ).parse_source( + inventory_file, + ): + _logger.warning( + "Unable to parse %s as an inventory source", + inventory_file, + ) + else: + # To avoid noisy warnings we pass localhost as current inventory: + # [WARNING]: No inventory was parsed, only implicit localhost is available + # [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all' + inventory_files = ["localhost"] + + return inventory_files + def _filter_excluded_matches(self, matches: list[MatchError]) -> list[MatchError]: return [ match diff --git a/test/test_app.py b/test/test_app.py index cbeae3d5fe..eee8e5cb1c 100644 --- a/test/test_app.py +++ b/test/test_app.py @@ -2,6 +2,8 @@ from pathlib import Path +import pytest + from ansiblelint.constants import RC from ansiblelint.file_utils import Lintable from ansiblelint.testing import run_ansible_lint @@ -29,3 +31,69 @@ def test_app_no_matches(tmp_path: Path) -> None: """Validate that linter returns special exit code if no files are analyzed.""" result = run_ansible_lint(cwd=tmp_path) assert result.returncode == RC.NO_FILES_MATCHED + + +@pytest.mark.parametrize( + "inventory_opts", + ( + pytest.param(["-I", "inventories/foo"], id="1"), + pytest.param( + [ + "-I", + "inventories/bar", + "-I", + "inventories/baz", + ], + id="2", + ), + pytest.param( + [ + "-I", + "inventories/foo,inventories/bar", + "-I", + "inventories/baz", + ], + id="3", + ), + ), +) +def test_with_inventory(inventory_opts: list[str]) -> None: + """Validate using --inventory remedies syntax-check[specific] violation.""" + lintable = Lintable("examples/playbooks/test_using_inventory.yml") + result = run_ansible_lint(lintable.filename, *inventory_opts) + assert result.returncode == RC.SUCCESS + + +@pytest.mark.parametrize( + ("inventory_opts", "error_msg"), + ( + pytest.param( + ["-I", "inventories/i_dont_exist"], + "Unable to use inventories/i_dont_exist as an inventory source: no such file or directory", + id="1", + ), + pytest.param( + ["-I", "inventories/bad_inventory"], + "Unable to parse inventories/bad_inventory as an inventory source", + id="2", + ), + ), +) +def test_with_inventory_emit_warning(inventory_opts: list[str], error_msg: str) -> None: + """Validate using --inventory can emit useful warnings about inventory files.""" + lintable = Lintable("examples/playbooks/test_using_inventory.yml") + result = run_ansible_lint(lintable.filename, *inventory_opts) + assert error_msg in result.stderr + + +def test_with_inventory_via_ansible_cfg(tmp_path: Path) -> None: + """Validate using inventory file from ansible.cfg remedies syntax-check[specific] violation.""" + (tmp_path / "ansible.cfg").write_text("[defaults]\ninventory = foo\n") + (tmp_path / "foo").write_text("[group_name]\nhost1\nhost2\n") + lintable = Lintable(tmp_path / "playbook.yml") + lintable.content = "---\n- name: Test\n hosts:\n - group_name\n serial: \"{{ batch | default(groups['group_name'] | length) }}\"\n" + lintable.kind = "playbook" + lintable.write(force=True) + + result = run_ansible_lint(lintable.filename, cwd=tmp_path) + assert result.returncode == RC.SUCCESS diff --git a/tox.ini b/tox.ini index fc2245183d..713936100d 100644 --- a/tox.ini +++ b/tox.ini @@ -75,7 +75,7 @@ setenv = PRE_COMMIT_COLOR = always # Number of expected test passes, safety measure for accidental skip of # tests. Update value if you add/remove tests. (tox-extra) - PYTEST_REQPASS = 895 + PYTEST_REQPASS = 901 FORCE_COLOR = 1 pre: PIP_PRE = 1 allowlist_externals =