Skip to content

Commit

Permalink
Revert "Accommodate specified inventory files (#4393)" (#4450)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea authored Dec 12, 2024
1 parent c629b23 commit 4ce8e49
Show file tree
Hide file tree
Showing 10 changed files with 4 additions and 180 deletions.
11 changes: 0 additions & 11 deletions examples/playbooks/test_using_inventory.yml

This file was deleted.

7 changes: 0 additions & 7 deletions inventories/bad_inventory

This file was deleted.

4 changes: 0 additions & 4 deletions inventories/bar

This file was deleted.

4 changes: 0 additions & 4 deletions inventories/baz

This file was deleted.

4 changes: 0 additions & 4 deletions inventories/foo

This file was deleted.

8 changes: 0 additions & 8 deletions src/ansiblelint/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,14 +457,6 @@ 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",
Expand Down
1 change: 0 additions & 1 deletion src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ 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
Expand Down
75 changes: 3 additions & 72 deletions src/ansiblelint/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,8 @@
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
Expand Down Expand Up @@ -246,7 +242,6 @@ def _run(self) -> list[MatchError]:
def worker(lintable: Lintable) -> list[MatchError]:
return self._get_ansible_syntax_check_matches(
lintable=lintable,
inventory_opts=inventory_opts,
app=self.app,
)

Expand All @@ -258,15 +253,6 @@ def worker(lintable: Lintable) -> list[MatchError]:
continue
files.append(lintable)

inventory_opts = [
inventory_opt
for inventory_opts in [
("-i", inventory_file)
for inventory_file in self._get_inventory_files(self.app)
]
for inventory_opt in inventory_opts
]

# avoid resource leak warning, https://github.com/python/cpython/issues/90549
# pylint: disable=unused-variable
global_resource = multiprocessing.Semaphore() # noqa: F841
Expand Down Expand Up @@ -314,7 +300,6 @@ def worker(lintable: Lintable) -> list[MatchError]:
def _get_ansible_syntax_check_matches(
self,
lintable: Lintable,
inventory_opts: list[str],
app: App,
) -> list[MatchError]:
"""Run ansible syntax check and return a list of MatchError(s)."""
Expand Down Expand Up @@ -355,9 +340,11 @@ 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",
*inventory_opts,
"--syntax-check",
playbook_path,
]
Expand Down Expand Up @@ -462,62 +449,6 @@ 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
Expand Down
68 changes: 0 additions & 68 deletions test/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

from pathlib import Path

import pytest

from ansiblelint.constants import RC
from ansiblelint.file_utils import Lintable
from ansiblelint.testing import run_ansible_lint
Expand Down Expand Up @@ -33,72 +31,6 @@ def test_app_no_matches(tmp_path: Path) -> None:
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


def test_with_inventory_concurrent_syntax_checks(tmp_path: Path) -> None:
"""Validate using inventory file with concurrent syntax checks aren't faulty."""
(tmp_path / "ansible.cfg").write_text("[defaults]\ninventory = foo\n")
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 906
PYTEST_REQPASS = 900
FORCE_COLOR = 1
pre: PIP_PRE = 1
allowlist_externals =
Expand Down

0 comments on commit 4ce8e49

Please sign in to comment.