From 8fe1ea145fb8090a56c786e6dda6a5f9ab3b0d66 Mon Sep 17 00:00:00 2001 From: Conner Crosby Date: Thu, 24 Oct 2024 08:28:48 -0400 Subject: [PATCH] Handle FQCN when using import_playbook (#4369) --- .../local/testcollection/playbooks/foo.yml | 9 +++ examples/playbooks/import_playbook_fqcn.yml | 3 + src/ansiblelint/runner.py | 4 +- src/ansiblelint/utils.py | 81 ++++++++++++++----- test/test_utils.py | 16 ++++ tox.ini | 2 +- 6 files changed, 92 insertions(+), 23 deletions(-) create mode 100644 collections/ansible_collections/local/testcollection/playbooks/foo.yml create mode 100644 examples/playbooks/import_playbook_fqcn.yml diff --git a/collections/ansible_collections/local/testcollection/playbooks/foo.yml b/collections/ansible_collections/local/testcollection/playbooks/foo.yml new file mode 100644 index 0000000000..a465be95e6 --- /dev/null +++ b/collections/ansible_collections/local/testcollection/playbooks/foo.yml @@ -0,0 +1,9 @@ +--- +- name: Fixture + hosts: localhost + connection: local + gather_facts: false + tasks: + - name: Another task + ansible.builtin.debug: + msg: debug message diff --git a/examples/playbooks/import_playbook_fqcn.yml b/examples/playbooks/import_playbook_fqcn.yml new file mode 100644 index 0000000000..398f324125 --- /dev/null +++ b/examples/playbooks/import_playbook_fqcn.yml @@ -0,0 +1,3 @@ +--- +- name: Import a playbook + ansible.builtin.import_playbook: local.testcollection.foo diff --git a/src/ansiblelint/runner.py b/src/ansiblelint/runner.py index a7a122fb20..5ea833efea 100644 --- a/src/ansiblelint/runner.py +++ b/src/ansiblelint/runner.py @@ -553,8 +553,8 @@ def play_children( "block": handlers.taskshandlers_children, "include": handlers.include_children, "ansible.builtin.include": handlers.include_children, - "import_playbook": handlers.include_children, - "ansible.builtin.import_playbook": handlers.include_children, + "import_playbook": handlers.import_playbook_children, + "ansible.builtin.import_playbook": handlers.import_playbook_children, "roles": handlers.roles_children, "dependencies": handlers.roles_children, "handlers": handlers.taskshandlers_children, diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index 3f56f8d9fe..629e5edfee 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -299,12 +299,6 @@ def include_children( ) -> list[Lintable]: """Include children.""" basedir = str(lintable.path.parent) - # 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: @@ -314,17 +308,6 @@ def include_children( if not v or "{{" in v: return [] - if k in ("import_playbook", "ansible.builtin.import_playbook"): - included = Path(basedir) / v - if not included.exists(): - msg = f"Failed to find {v} playbook." - elif not self.app.runtime.has_playbook(v, basedir=Path(basedir)): - msg = f"Failed to load {v} playbook due to failing syntax check." - else: - return [Lintable(included, kind=parent_type)] - logging.error(msg) - return [] - # handle include: filename.yml tags=blah (args, kwargs) = tokenize(v) @@ -457,6 +440,61 @@ def roles_children( results.extend(_look_for_role_files(basedir, role)) return results + def import_playbook_children( + self, + lintable: Lintable, + k: str, # pylint: disable=unused-argument + v: Any, + parent_type: FileType, + ) -> list[Lintable]: + """Include import_playbook children.""" + + def append_playbook_path(loc: str, playbook_name: str) -> None: + possible_paths.append( + Path( + path_dwim( + os.path.expanduser(loc), + os.path.join( + "ansible_collections", + namespace_name, + collection_name, + "playbooks", + playbook_name, + ), + ), + ), + ) + + # import_playbook only accepts a string as argument (no dict syntax) + if not isinstance(v, str): + return [] + + possible_paths = [] + namespace_name, collection_name, playbook_name = parse_fqcn(v) + if namespace_name and collection_name: + for loc in get_app(cached=True).runtime.config.collections_paths: + append_playbook_path(loc, f"{playbook_name}.yml") + append_playbook_path(loc, f"{playbook_name}.yaml") + else: + possible_paths.append(lintable.path.parent / v) + + for possible_path in possible_paths: + if not possible_path.exists(): + msg = f"Failed to find {v} playbook." + elif not self.app.runtime.has_playbook( + str(possible_path), + ): + msg = f"Failed to load {v} playbook due to failing syntax check." + break + elif namespace_name and collection_name: + # don't lint foreign playbook + return [] + else: + return [Lintable(possible_path, kind=parent_type)] + + logging.error(msg) + return [] + def _get_task_handler_children_for_tasks_or_playbooks( task_handler: dict[str, Any], @@ -505,9 +543,7 @@ def _get_task_handler_children_for_tasks_or_playbooks( def _rolepath(basedir: str, role: str) -> str | None: role_path = None - namespace_name, collection_name, role_name = ( - role.split(".") if is_fqcn(role) else ("", "", role) - ) + namespace_name, collection_name, role_name = parse_fqcn(role) possible_paths = [ # if included from a playbook @@ -1135,3 +1171,8 @@ def load_plugin(name: str) -> PluginLoadContext: check_aliases=True, ) return loaded_module + + +def parse_fqcn(name: str) -> tuple[str, ...]: + """Parse name parameter into FQCN segments.""" + return tuple(name.split(".")) if is_fqcn(name) else ("", "", name) diff --git a/test/test_utils.py b/test/test_utils.py index 09f623466a..3d221462ab 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -503,3 +503,19 @@ def test_include_children_load_playbook_failed_syntax_check() -> None: "Failed to load syntax-error.yml playbook due to failing syntax check." in result.stderr ) + + +def test_import_playbook_children() -> None: + """Verify import_playbook_children().""" + result = run_ansible_lint( + Path("playbooks/import_playbook_fqcn.yml"), + cwd=Path(__file__).resolve().parent.parent / "examples", + env={ + "ANSIBLE_COLLECTIONS_PATH": "../collections", + }, + ) + assert "Failed to find local.testcollection.foo playbook." not in result.stderr + assert ( + "Failed to load local.testcollection.foo playbook due to failing syntax check." + not in result.stderr + ) diff --git a/tox.ini b/tox.ini index 881f8b6064..d250cf7699 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 = 894 + PYTEST_REQPASS = 895 FORCE_COLOR = 1 pre: PIP_PRE = 1 allowlist_externals =