fix: Fix include_role with loop

Makefile

@@ -1,5 +1,5 @@
 ANSIBLE_VERSION=2.10.7
-VIRTUALENV_DIR=venv
+VIRTUALENV_DIR=venv-test-install
 PACKAGE := dist/$(shell ls dist 2> /dev/null)
 SRC=$(wildcard ansibleplaybookgrapher/*.py setup.py ansibleplaybookgrapher/data/*)
 

ansibleplaybookgrapher/parser.py

@@ -238,27 +238,32 @@ def _include_tasks_in_blocks(self, current_play: Play, parent_nodes: List[Compos
                     # Here we have an 'include_role'. The class IncludeRole is a subclass of TaskInclude.
                     # We do this because the management of an 'include_role' is different.
                     # See :func:`~ansible.playbook.included_file.IncludedFile.process_include_results` from line 155
-                    self.display.v(
-                        f"An 'include_role' found. Including tasks from the role '{task_or_block.args['name']}'")
+                    self.display.v(f"An 'include_role' found. Including tasks from '{task_or_block.get_name()}'")
 
-                    role_node = RoleNode(task_or_block.args['name'], raw_object=task_or_block)
+                    role_node = RoleNode(task_or_block.get_name(), raw_object=task_or_block)
                     parent_nodes[-1].add_node(f"{node_type}s", EdgeNode(parent_nodes[-1], role_node,
                                                                         convert_when_to_str(task_or_block.when)))
 
-                    if self.include_role_tasks:
-                        # If we have an include_role and we want to include role tasks, the parent node now becomes
-                        # the role.
-                        parent_nodes.append(role_node)
-
-                    block_list, _ = task_or_block.get_block_list(play=current_play, loader=self.data_loader,
-                                                                 variable_manager=self.variable_manager)
+                    if task_or_block.loop:  # Looping on include_role is not supported
+                        self.display.warning(
+                            "Including role with loop is not supported for the moment. The include will not be "
+                            "evaluated.")
+                        block_list = []
+                    else:
+                        if self.include_role_tasks:
+                            # If we have an include_role, and we want to include its tasks, the parent node now becomes
+                            # the role.
+                            parent_nodes.append(role_node)
+
+                        block_list, _ = task_or_block.get_block_list(play=current_play, loader=self.data_loader,
+                                                                     variable_manager=self.variable_manager)
                 else:
                     self.display.v(f"An 'include_tasks' found. Including tasks from '{task_or_block.get_name()}'")
 
                     templar = Templar(loader=self.data_loader, variables=task_vars)
                     try:
-                        include_file = handle_include_path(original_task=task_or_block, loader=self.data_loader,
-                                                           templar=templar)
+                        included_file_path = handle_include_path(original_task=task_or_block, loader=self.data_loader,
+                                                                 templar=templar)
                     except AnsibleUndefinedVariable as e:
                         # TODO: mark this task with some special shape or color
                         self.display.warning(
@@ -268,9 +273,9 @@ def _include_tasks_in_blocks(self, current_play: Play, parent_nodes: List[Compos
                                        parent_node=parent_nodes[-1])
                         continue
 
-                    data = self.data_loader.load_from_file(include_file)
+                    data = self.data_loader.load_from_file(included_file_path)
                     if data is None:
-                        self.display.warning(f"The file '{include_file}' is empty and has no tasks to include")
+                        self.display.warning(f"The file '{included_file_path}' is empty and has no tasks to include")
                         continue
                     elif not isinstance(data, list):
                         raise AnsibleParserError("Included task files must contain a list of tasks", obj=data)
@@ -283,6 +288,9 @@ def _include_tasks_in_blocks(self, current_play: Play, parent_nodes: List[Compos
                 for b in block_list:  # loop through the blocks inside the included tasks or role
                     self._include_tasks_in_blocks(current_play=current_play, parent_nodes=parent_nodes, block=b,
                                                   play_vars=task_vars, node_type=node_type)
+                if self.include_role_tasks and isinstance(task_or_block, IncludeRole):
+                    # We remove the parent node we have added if we included some tasks from a role
+                    parent_nodes.pop()
             else:
                 if (len(parent_nodes) > 1 and  # 1
                         not has_role_parent(task_or_block) and  # 2

ansibleplaybookgrapher/utils.py

@@ -2,14 +2,19 @@
 import uuid
 from typing import Tuple, List
 
+from ansible.errors import AnsibleError
+from ansible.module_utils._text import to_text
 from ansible.parsing.dataloader import DataLoader
 from ansible.playbook import Play
 from ansible.playbook.role_include import IncludeRole
 from ansible.playbook.task import Task
 from ansible.playbook.task_include import TaskInclude
 from ansible.template import Templar
+from ansible.utils.display import Display
 from colour import Color
 
+display = Display()
+
 
 def convert_when_to_str(when: List) -> str:
     """
@@ -75,7 +80,7 @@ def has_role_parent(task_block: Task) -> bool:
 
 def handle_include_path(original_task: TaskInclude, loader: DataLoader, templar: Templar) -> str:
     """
-    Handle include path. We may have some nested includes with relative paths to handle.
+    handle relative includes by walking up the list of parent include tasks
 
     This function is widely inspired by the static method ansible uses when executing the playbook.
     See :func:`~ansible.playbook.included_file.IncludedFile.process_include_results`
@@ -98,7 +103,15 @@ def handle_include_path(original_task: TaskInclude, loader: DataLoader, templar:
         if isinstance(parent_include, IncludeRole):
             parent_include_dir = parent_include._role_path
         else:
-            parent_include_dir = os.path.dirname(templar.template(parent_include.args.get('_raw_params')))
+            try:
+                parent_include_dir = os.path.dirname(templar.template(parent_include.args.get('_raw_params')))
+            except AnsibleError as e:
+                parent_include_dir = ''
+                display.warning(
+                    'Templating the path of the parent %s failed. The path to the '
+                    'included file may not be found. '
+                    'The error was: %s.' % (original_task.action, to_text(e))
+                )
 
         if cumulative_path is not None and not os.path.isabs(cumulative_path):
             cumulative_path = os.path.join(parent_include_dir, cumulative_path)

tests/fixtures/include_role.yml

@@ -7,13 +7,23 @@
       debug:
         msg: "Debug 1"
       when: ansible_os == "ubuntu"
-    - name: (2) Include role
+
+    - name: (2) First Include role
       include_role:
         name: fake_role
+
     - name: (3) Debug 2
       debug:
         msg: "Debug 2"
-    - name: (4) Include role 2
+
+    - name: (4) Second Include role
       when: x is not defined
       include_role:
         name: display_some_facts
+
+    - name: (5) Third Include role (with loop)
+      include_role:
+        name: '{{ item }}'
+      loop:
+        - fake_role
+        - display_some_facts

tests/test_parser.py

@@ -35,7 +35,7 @@ def test_include_role_parsing(grapher_cli: PlaybookGrapherCLI, display: Display)
     assert len(playbook_node.plays) == 1
     play_node = playbook_node.plays[0].destination
     tasks = play_node.tasks
-    assert len(tasks) == 4
+    assert len(tasks) == 5
 
     # first task
     assert tasks[0].destination.name == "(1) Debug"
@@ -55,6 +55,11 @@ def test_include_role_parsing(grapher_cli: PlaybookGrapherCLI, display: Display)
     assert isinstance(include_role_2, RoleNode)
     assert len(include_role_2.tasks) == 3
 
+    # third include_role
+    include_role_3 = tasks[4].destination
+    assert isinstance(include_role_3, RoleNode)
+    assert len(include_role_3.tasks) == 0, "We don't support adding tasks from include_role with loop"
+
 
 @pytest.mark.parametrize('grapher_cli', [["with_block.yml"]], indirect=True)
 def test_block_parsing(grapher_cli: PlaybookGrapherCLI, display: Display):
@@ -75,7 +80,8 @@ def test_block_parsing(grapher_cli: PlaybookGrapherCLI, display: Display):
     total_pre_tasks = get_all_tasks(pre_tasks)
     total_tasks = get_all_tasks(tasks)
     total_post_tasks = get_all_tasks(post_tasks)
-    assert len(total_pre_tasks) == 4, f"The play should contain 4 pre tasks but we found {len(total_pre_tasks)} pre task(s)"
+    assert len(
+        total_pre_tasks) == 4, f"The play should contain 4 pre tasks but we found {len(total_pre_tasks)} pre task(s)"
     assert len(total_tasks) == 7, f"The play should contain 3 tasks but we found {len(total_tasks)} task(s)"
     assert len(
         total_post_tasks) == 2, f"The play should contain 2 post tasks but we found {len(total_post_tasks)} post task(s)"

tests/test_playbook_grapher.py

@@ -158,7 +158,7 @@ def test_include_role(request, include_role_tasks_option, expected_tasks_number)
                                           additional_args=[include_role_tasks_option])
 
     _common_tests(svg_path=svg_path, playbook_path=playbook_path, plays_number=1, tasks_number=expected_tasks_number,
-                  roles_number=2)
+                  roles_number=3)
 
 
 def test_with_block(request):