Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Accommodate specified inventory files (#4393)" #4450

Merged
merged 1 commit into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading