From dac3bf5b71d98cf31da9410ee7318a70f1722c98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20L=C3=B3pez?= Date: Mon, 16 Dec 2019 06:25:05 +0100 Subject: [PATCH] zabbix_host: interface.port parameter is a string (#64986) interface.port should be a string to be able to use macros in that value. This fixes the case when interface.port is a macro (eg.: "{$MACRO}" and force=false. Because, until now, setting the interface.port to an integer was the correct way to work with force=false, a type validation has been added to that parameter. Previously, if a string was used for interface.port, it was converted to an integer, the comparison didn't work (if interface not in interfaces) and the module tried to register the same interface twice, returning an error. Zabbix API manual specifies that only 'main, 'type', 'useip' and 'bulk' are integers. https://www.zabbix.com/documentation/current/manual/api/reference/hostinterface/object Tests are changed to use always str in the interface.port. Two new tests are added. The first one is to show that now registering a host with force=false and a macro in interface.port works. The other one tests that interfaces defined using string for port are compared correctly when force=false is used. Previously it was a comparison between int and str, interfaces were seen as different and an error was thrown because we were trying to create twice a main interface. * Try to kindly convert interface port to the string As suggested, the previous behaviour could break current configurations. This solution accepts integers and strings. Co-Authored-By: Dusan Matejka --- .../modules/monitoring/zabbix/zabbix_host.py | 11 +- .../zabbix_host/tasks/zabbix_host_doc.yml | 6 +- .../zabbix_host/tasks/zabbix_host_tests.yml | 142 ++++++++++++++++-- 3 files changed, 144 insertions(+), 15 deletions(-) diff --git a/lib/ansible/modules/monitoring/zabbix/zabbix_host.py b/lib/ansible/modules/monitoring/zabbix/zabbix_host.py index 572dd15d5ce..69f669d10fa 100644 --- a/lib/ansible/modules/monitoring/zabbix/zabbix_host.py +++ b/lib/ansible/modules/monitoring/zabbix/zabbix_host.py @@ -252,13 +252,13 @@ EXAMPLES = ''' useip: 1 ip: 10.xx.xx.xx dns: "" - port: 10050 + port: "10050" - type: 4 main: 1 useip: 1 ip: 10.xx.xx.xx dns: "" - port: 12345 + port: "12345" proxy: a.zabbix.proxy - name: Update an existing host's TLS settings local_action: @@ -780,6 +780,11 @@ def main(): interface['ip'] = '' if 'main' not in interface: interface['main'] = 0 + if 'port' in interface and not isinstance(interface['port'], str): + try: + interface['port'] = str(interface['port']) + except ValueError: + module.fail_json(msg="port should be convertable to string on interface '%s'." % interface) if 'port' not in interface: if interface['type'] == 1: interface['port'] = "10050" @@ -840,7 +845,7 @@ def main(): interface.pop(key, None) for index in interface.keys(): - if index in ['useip', 'main', 'type', 'port']: + if index in ['useip', 'main', 'type']: interface[index] = int(interface[index]) if interface not in interfaces: diff --git a/test/integration/targets/zabbix_host/tasks/zabbix_host_doc.yml b/test/integration/targets/zabbix_host/tasks/zabbix_host_doc.yml index 41690947ca4..dfdd2001c61 100644 --- a/test/integration/targets/zabbix_host/tasks/zabbix_host_doc.yml +++ b/test/integration/targets/zabbix_host/tasks/zabbix_host_doc.yml @@ -36,13 +36,13 @@ useip: 1 ip: 10.1.1.1 dns: "" - port: 10050 + port: "10050" - type: 4 main: 1 useip: 1 ip: 10.1.1.1 dns: "" - port: 12345 + port: "12345" register: zabbix_host1 - name: Update an existing host's tls settings @@ -58,7 +58,7 @@ useip: 1 ip: 10.1.1.2 dns: "" - port: 10050 + port: "10050" host_groups: - Linux servers tls_psk_identity: test diff --git a/test/integration/targets/zabbix_host/tasks/zabbix_host_tests.yml b/test/integration/targets/zabbix_host/tasks/zabbix_host_tests.yml index af952519082..1f51677bea3 100644 --- a/test/integration/targets/zabbix_host/tasks/zabbix_host_tests.yml +++ b/test/integration/targets/zabbix_host/tasks/zabbix_host_tests.yml @@ -31,13 +31,19 @@ useip: 1 ip: 10.1.1.1 dns: "" - port: 10050 + port: "10050" + - type: 1 + main: 0 + useip: 1 + ip: 10.1.1.1 + dns: "" + port: "{$MACRO}" - type: 4 main: 1 useip: 1 ip: 10.1.1.1 dns: "" - port: 12345 + port: "12345" proxy: ExampleProxy tls_psk_identity: test tls_connect: 2 @@ -80,13 +86,19 @@ useip: 1 ip: 10.1.1.1 dns: "" - port: 10050 + port: "10050" + - type: 1 + main: 0 + useip: 1 + ip: 10.1.1.1 + dns: "" + port: "{$MACRO}" - type: 4 main: 1 useip: 1 ip: 10.1.1.1 dns: "" - port: 12345 + port: "12345" proxy: ExampleProxy tls_psk_identity: test tls_connect: 2 @@ -98,6 +110,118 @@ that: - "not zabbix_host1 is changed" +- name: "test: try to create the same host with the same settings and force false" + zabbix_host: + force: false + server_url: "{{ zabbix_server_url }}" + login_user: "{{ zabbix_login_user }}" + login_password: "{{ zabbix_login_password }}" + host_name: ExampleHost + visible_name: ExampleName + description: My ExampleHost Description + host_groups: + - Linux servers + - Zabbix servers + link_templates: + - Template App IMAP Service + - Template App NTP Service + status: enabled + state: present + inventory_mode: manual + inventory_zabbix: + tag: test-tag + alias: test-alias + notes: "Special Informations: test-info" + location: test-location + site_rack: test-rack + os: test-os + hardware: test-hw + interfaces: + - type: 1 + main: 1 + useip: 1 + ip: 10.1.1.1 + dns: "" + port: "10050" + - type: 1 + main: 0 + useip: 1 + ip: 10.1.1.1 + dns: "" + port: "{$MACRO}" + - type: 4 + main: 1 + useip: 1 + ip: 10.1.1.1 + dns: "" + port: "12345" + proxy: ExampleProxy + tls_psk_identity: test + tls_connect: 2 + tls_psk: 123456789abcdef123456789abcdef12 + register: zabbix_host1 + +- name: updating with same values and force false should be idempotent + assert: + that: + - "not zabbix_host1 is changed" + +- name: "test: try to create the same host changing one parameter in the inventory with force false" + zabbix_host: + force: false + server_url: "{{ zabbix_server_url }}" + login_user: "{{ zabbix_login_user }}" + login_password: "{{ zabbix_login_password }}" + host_name: ExampleHost + visible_name: ExampleName + description: My ExampleHost Description + host_groups: + - Linux servers + - Zabbix servers + link_templates: + - Template App IMAP Service + - Template App NTP Service + status: enabled + state: present + inventory_mode: manual + inventory_zabbix: + tag: test-tag + alias: test-alias + notes: "Special Informations: test-info" + location: test-location + site_rack: test-rack + os: test-os + hardware: test-hw-modified + interfaces: + - type: 1 + main: 1 + useip: 1 + ip: 10.1.1.1 + dns: "" + port: "10050" + - type: 1 + main: 0 + useip: 1 + ip: 10.1.1.1 + dns: "" + port: "{$MACRO}" + - type: 4 + main: 1 + useip: 1 + ip: 10.1.1.1 + dns: "" + port: "12345" + proxy: ExampleProxy + tls_psk_identity: test + tls_connect: 2 + tls_psk: 123456789abcdef123456789abcdef12 + register: zabbix_host1 + +- name: changing the value of an already defined inventory should work and mark task as changed + assert: + that: + - "zabbix_host1 is changed" + - name: "test: change visible_name" zabbix_host: server_url: "{{ zabbix_server_url }}" @@ -524,7 +648,7 @@ useip: 1 ip: 10.1.1.1 dns: "" - port: 10050 + port: "10050" register: zabbix_host1 - name: expect to succeed and that things have changed @@ -544,7 +668,7 @@ useip: 1 ip: 10.1.1.1 dns: "" - port: 10050 + port: "10050" register: zabbix_host1 - name: expect to succeed and that things have not changed @@ -564,7 +688,7 @@ useip: 1 ip: 10.1.1.1 dns: "" - port: 12345 + port: "12345" force: no register: zabbix_host1 @@ -585,13 +709,13 @@ useip: 1 ip: 10.1.1.1 dns: "" - port: 10050 + port: "10050" - type: 4 main: 1 useip: 1 ip: 10.1.1.1 dns: "" - port: 12345 + port: "12345" register: zabbix_host1 - name: expect to succeed and that things have not changed