Repository: ansible/ansible
Pull request: zabbix_template - fixing errors linked templates and dump/imports
##### SUMMARY
fixes #66417

Line 409 fixes KeyError if existing template has no linked templates - as Zabbix API isn't returning an empty array either.
Line 417 is needed to mark as changed if new/modified template has no linked but existing one has.
Line 442 is needed to actually update even if link_templates was None.

##### ISSUE TYPE
- Bugfix Pull Request

##### COMPONENT NAME
zabbix_template

##### ADDITIONAL INFORMATION
Tested against Zabbix 4.4.4 with Ansible 2.9.2 and python 3.6.9


Problem statement:
zabbix_template check_template_changed error
##### SUMMARY
KeyError: 'templates' on second run

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
zabbix_template

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
ansible 2.9.2
  config file = /mnt/d/Code/gitlab/zabbix-servers/ansible/ansible.cfg
  configured module search path = ['/home/fism/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /mnt/d/Code/gitlab/zabbix-servers/venv/lib/python3.6/site-packages/ansible
  executable location = /mnt/d/Code/gitlab/zabbix-servers/venv/bin/ansible
  python version = 3.6.9 (default, Nov  7 2019, 10:44:02) [GCC 8.3.0]
```

##### STEPS TO REPRODUCE
On first run the new template is created
```yaml
- name: "create test template"
      zabbix_template:
        server_url: "{{ zabbix_apiserver_url }}"
        login_user: "{{ zabbix_apiuser_rw }}"
        login_password: "{{ zabbix_apipassword_rw }}"
        state: present
        template_name: test1
        template_groups:
          - "Templates"
```
If you run it a second time errors are thrown.

##### EXPECTED RESULTS
The second run should succeed without changes.

##### ACTUAL RESULTS
```shell
The full traceback is:
Traceback (most recent call last):
  File "<stdin>", line 102, in <module>
  File "<stdin>", line 94, in _ansiballz_main
  File "<stdin>", line 40, in invoke_module
  File "/usr/lib/python3.6/runpy.py", line 205, in run_module
    return _run_module_code(code, init_globals, run_name, mod_spec)
  File "/usr/lib/python3.6/runpy.py", line 96, in _run_module_code
    mod_name, mod_spec, pkg_name, script_name)
  File "/usr/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/tmp/ansible_zabbix_template_payload_9t34ekm5/ansible_zabbix_template_payload.zip/ansible/modules/zabbix_template.py", line 743, in <module>
  File "/tmp/ansible_zabbix_template_payload_9t34ekm5/ansible_zabbix_template_payload.zip/ansible/modules/zabbix_template.py", line 727, in main
  File "/tmp/ansible_zabbix_template_payload_9t34ekm5/ansible_zabbix_template_payload.zip/ansible/modules/zabbix_template.py", line 398, in check_template_changed
KeyError: 'templates'

fatal: [localhost]: FAILED! => {
    "changed": false,
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 102, in <module>\n  File \"<stdin>\", line 94, in _ansiballz_main\n  File \"<stdin>\", line 40, in invoke_module\n  F
ile \"/usr/lib/python3.6/runpy.py\", line 205, in run_module\n    return _run_module_code(code, init_globals, run_name, mod_spec)\n  File \"/usr/lib/python3.6/runpy.py\", line 96, in _run_module_code\
n    mod_name, mod_spec, pkg_name, script_name)\n  File \"/usr/lib/python3.6/runpy.py\", line 85, in _run_code\n    exec(code, run_globals)\n  File \"/tmp/ansible_zabbix_template_payload_9t34ekm5/ansi
ble_zabbix_template_payload.zip/ansible/modules/zabbix_template.py\", line 743, in <module>\n  File \"/tmp/ansible_zabbix_template_payload_9t34ekm5/ansible_zabbix_template_payload.zip/ansible/modules/
zabbix_template.py\", line 727, in main\n  File \"/tmp/ansible_zabbix_template_payload_9t34ekm5/ansible_zabbix_template_payload.zip/ansible/modules/zabbix_template.py\", line 398, in check_template_ch
anged\nKeyError: 'templates'\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1
}
```

This is what the existing_template variable looks like
```json
{
    'zabbix_export':
    {
        'version': '4.4',
        'date': '2020-01-13T14: 47: 28Z',
        'groups': [
            {
                'name': 'Templates'
            }
        ],
        'templates': [
            {
                'template': 'fismc',
                'name': 'fismc',
                'groups': [
                    {
                        'name': 'Templates'
                    }
                ],
                'items': [
                    {
                        'name': 'asdf',
                        'key': 'asdf'
                    }
                ]
            }
        ]
    }
}
```

So it seams that it fails if there aren't other templates linked.

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/modules/monitoring/zabbix/zabbix_template.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/monitoring/zabbix/zabbix_template.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @D3DeFi @logan2211 @rubentsirunyan @sky-joker @sookido
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
If you add a link in the frontend the play succeeds without a change.
If you add link_templates the play changes the zabbix template correctly.
If you remove items from link_templates (leaving at least one item) the template is also changed correctly.

So just the unlinking if no link_templates option is giving doesn't work.

Tried `link_template: []` but that doesn't work.

Commit message: Create 66463-zabbix_template-fix-error-linktemplate-and-importdump.yml

Patch to review:
diff --git a/changelogs/fragments/66463-zabbix_template-fix-error-linktemplate-and-importdump.yml b/changelogs/fragments/66463-zabbix_template-fix-error-linktemplate-and-importdump.yml
new file mode 100644
index 00000000000000..d73d46e672653f
--- /dev/null
+++ b/changelogs/fragments/66463-zabbix_template-fix-error-linktemplate-and-importdump.yml
@@ -0,0 +1,6 @@
+---
+
+bugfixes:
+  - zabbix_template fix for error on linked templates and their removal
+  - zabbix_template fix for error on dump/import of templates
+  
diff --git a/lib/ansible/modules/monitoring/zabbix/zabbix_template.py b/lib/ansible/modules/monitoring/zabbix/zabbix_template.py
index ae60cfa82f4192..59446455076cd5 100644
--- a/lib/ansible/modules/monitoring/zabbix/zabbix_template.py
+++ b/lib/ansible/modules/monitoring/zabbix/zabbix_template.py
@@ -406,11 +406,17 @@ def check_template_changed(self, template_ids, template_groups, link_templates,
             if set(template_groups) != set(existing_groups):
                 changed = True
 
+        if 'templates' not in existing_template['zabbix_export']['templates'][0]:
+            existing_template['zabbix_export']['templates'][0]['templates'] = []
+
         # Check if any new templates would be linked or any existing would be unlinked
         exist_child_templates = [t['name'] for t in existing_template['zabbix_export']['templates'][0]['templates']]
         if link_templates is not None:
             if set(link_templates) != set(exist_child_templates):
                 changed = True
+        else:
+            if set([]) != set(exist_child_templates):
+                changed = True
 
         # Mark that there will be changes when at least one existing template will be unlinked
         if clear_templates is not None:
@@ -433,6 +439,8 @@ def update_template(self, template_ids, group_ids, link_template_ids, clear_temp
 
         if link_template_ids is not None:
             template_changes.update({'templates': link_template_ids})
+        else:
+            template_changes.update({'templates': []})
 
         if clear_template_ids is not None:
             template_changes.update({'templates_clear': clear_template_ids})
@@ -522,7 +530,7 @@ def filter_xml_template(self, template_xml):
                     template.remove(element)
 
         # Filter new lines and indentation
-        xml_root_text = list(line.strip() for line in ET.tostring(parsed_xml_root).split('\n'))
+        xml_root_text = list(line.strip() for line in ET.tostring(parsed_xml_root, encoding='utf8', method='xml').decode().split('\n'))
         return ''.join(xml_root_text)
 
     def load_json_template(self, template_json):


Code review comments:
diff_hunk: @@ -0,0 +1,6 @@
+---
+
+bugfixes:
+  - zabbix_template fix for error on linked templates and their removal
path: changelogs/fragments/66463-zabbix_template-fix-error-linktemplate-and-importdump.yml
text: @user1:
```suggestion
  - zabbix_template - fixed error when providing empty ``link_templates`` to the module (see https://github.com/ansible/ansible/issues/66417)
```

diff_hunk: @@ -0,0 +1,6 @@
+---
+
+bugfixes:
+  - zabbix_template fix for error on linked templates and their removal
+  - zabbix_template fix for error on dump/import of templates
path: changelogs/fragments/66463-zabbix_template-fix-error-linktemplate-and-importdump.yml
text: @user1:
```suggestion
  - zabbix_template - fixed invalid (non-importable) output provided by exporting XML (see https://github.com/ansible/ansible/issues/66466)
```