Skip to content

Commit

Permalink
Improve logic of find_children
Browse files Browse the repository at this point in the history
Address few bugs in our implementation of find_children, making it
avoid errors when encountering garbage data.
  • Loading branch information
ssbarnea committed May 15, 2024
1 parent 492d840 commit c6d416d
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ jobs:
env:
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 870
PYTEST_REQPASS: 871
steps:
- uses: actions/checkout@v4
with:
Expand Down
7 changes: 7 additions & 0 deletions examples/playbooks/test_import_playbook_invalid.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
- name: Fixture 3 - not supported
ansible.builtin.import_playbook:
file: community.molecule.validate.yml
- name: Fixture 4 - not supported
ansible.builtin.import_playbook:
other: community.molecule.validate.yml
2 changes: 1 addition & 1 deletion src/ansiblelint/rules/role_name.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def _infer_role_name(meta: Path, default: str) -> str:
if meta_data:
try:
return str(meta_data["galaxy_info"]["role_name"])
except KeyError:
except (KeyError, TypeError):
pass
return default

Expand Down
24 changes: 19 additions & 5 deletions src/ansiblelint/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import logging
import os
import re
from collections.abc import ItemsView, Iterator, Mapping, Sequence
from collections.abc import ItemsView, Iterable, Iterator, Mapping, Sequence
from dataclasses import _MISSING_TYPE, dataclass, field
from functools import cache, lru_cache
from pathlib import Path
Expand Down Expand Up @@ -292,14 +292,21 @@ class HandleChildren:
rules: RulesCollection = field(init=True, repr=False)
app: App

def include_children(
def include_children( # pylint: disable=too-many-return-statements
self,
basedir: str,
k: str,
v: Any,
parent_type: FileType,
) -> list[Lintable]:
"""Include children."""
# import_playbook only accepts a string as argument (no dict syntax)
if k in (
"import_playbook",
"ansible.builtin.import_playbook",
) and not isinstance(v, str):
return []

# handle special case include_tasks: name=filename.yml
if k in INCLUSION_ACTION_NAMES and isinstance(v, dict) and "file" in v:
v = v["file"]
Expand All @@ -322,12 +329,19 @@ def include_children(
# pylint: disable=unused-variable
(command, args, kwargs) = tokenize(f"{k}: {v}")

result = path_dwim(basedir, args[0])
if args:
file = args[0]
elif "file" in kwargs:
file = kwargs["file"]

Check warning on line 335 in src/ansiblelint/utils.py

View check run for this annotation

Codecov / codecov/patch

src/ansiblelint/utils.py#L334-L335

Added lines #L334 - L335 were not covered by tests
else:
return []

Check warning on line 337 in src/ansiblelint/utils.py

View check run for this annotation

Codecov / codecov/patch

src/ansiblelint/utils.py#L337

Added line #L337 was not covered by tests

result = path_dwim(basedir, file)
while basedir not in ["", "/"]:
if os.path.exists(result):
break
basedir = os.path.dirname(basedir)
result = path_dwim(basedir, args[0])
result = path_dwim(basedir, file)

return [Lintable(result, kind=parent_type)]

Expand Down Expand Up @@ -435,7 +449,7 @@ def roles_children(
"""Roles children."""
# pylint: disable=unused-argument # parent_type)
results: list[Lintable] = []
if not v:
if not v or not isinstance(v, Iterable):
# typing does not prevent junk from being passed in
return results
for role in v:
Expand Down
14 changes: 14 additions & 0 deletions test/test_import_playbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,17 @@ def test_import_playbook_from_collection(

assert len(runner.lintables) == 1
assert len(results) == 0


def test_import_playbook_invalid(
default_rules_collection: RulesCollection,
) -> None:
"""Assures import_playbook from collection."""
playbook_path = "examples/playbooks/test_import_playbook_invalid.yml"
runner = Runner(playbook_path, rules=default_rules_collection)
results = runner.run()

assert len(runner.lintables) == 1
assert len(results) == 1
assert results[0].tag == "syntax-check[specific]"
assert results[0].lineno == 2

0 comments on commit c6d416d

Please sign in to comment.