Icinga2 Inventory Plugin - Error handling and inventory name changer (#3906) (#3915)

* Added inventory_attr and filter error handling

* Added inventory_attr and filter error handling

* Added inventory_attr and filter error handling

* Added inventory_attr and filter error handling

* Added changelog

* Added inventory_attr and filter error handling

* Added inventory_attr and filter error handling

* Applying requested changes

* FIxes for tests

* Added inventory_attr and filter error handling

* Error handling

* Error handling

* Error handling

* Modifications to unit tests

* Remove pitfall

(cherry picked from commit 8da2c630d8)

Co-authored-by: Cliff Hults <BongoEADGC6@users.noreply.github.com>

changelogs/fragments/3875-icinga2-inv-fix.yml

@@ -0,0 +1,9 @@
+---
+minor_changes:
+  - icinga2 inventory plugin - inventory object names are changable using ``inventory_attr`` in your config file to the host object name, address, or display_name fields
+    (https://github.com/ansible-collections/community.general/issues/3875, https://github.com/ansible-collections/community.general/pull/3906).
+  - icinga2 inventory plugin - added the ``display_name`` field to variables 
+    (https://github.com/ansible-collections/community.general/issues/3875, https://github.com/ansible-collections/community.general/pull/3906).
+bugfixes:
+  - icinga2 inventory plugin - handle 404 error when filter produces no results 
+    (https://github.com/ansible-collections/community.general/issues/3875, https://github.com/ansible-collections/community.general/pull/3906).

plugins/inventory/icinga2.py

@@ -35,13 +35,23 @@ DOCUMENTATION = '''
         type: string
         required: true
       host_filter:
-        description: An Icinga2 API valid host filter.
+        description:
+          - An Icinga2 API valid host filter. Leave blank for no filtering
         type: string
         required: false
       validate_certs:
         description: Enables or disables SSL certificate verification.
         type: boolean
         default: true
+      inventory_attr:
+        description:
+          - Allows the override of the inventory name based on different attributes.
+          - This allows for changing the way limits are used.
+          - The current default, C(address), is sometimes not unique or present. We recommend to use C(name) instead.
+        type: string
+        default: address
+        choices: ['name', 'display_name', 'address']
+        version_added: 4.2.0
 '''
 
 EXAMPLES = r'''
@@ -52,6 +62,7 @@ user: ansible
 password: secure
 host_filter: \"linux-servers\" in host.groups
 validate_certs: false
+inventory_attr: name
 '''
 
 import json
@@ -59,6 +70,7 @@ import json
 from ansible.errors import AnsibleParserError
 from ansible.plugins.inventory import BaseInventoryPlugin, Constructable
 from ansible.module_utils.urls import open_url
+from ansible.module_utils.six.moves.urllib.error import HTTPError
 
 
 class InventoryModule(BaseInventoryPlugin, Constructable):
@@ -76,6 +88,7 @@ class InventoryModule(BaseInventoryPlugin, Constructable):
         self.icinga2_password = None
         self.ssl_verify = None
         self.host_filter = None
+        self.inventory_attr = None
 
         self.cache_key = None
         self.use_cache = None
@@ -114,9 +127,21 @@ class InventoryModule(BaseInventoryPlugin, Constructable):
         if data is not None:
             request_args['data'] = json.dumps(data)
         self.display.vvv("Request Args: %s" % request_args)
-        response = open_url(request_url, **request_args)
+        try:
+            response = open_url(request_url, **request_args)
+        except HTTPError as e:
+            try:
+                error_body = json.loads(e.read().decode())
+                self.display.vvv("Error returned: {0}".format(error_body))
+            except Exception:
+                error_body = {"status": None}
+            if e.code == 404 and error_body.get('status') == "No objects found.":
+                raise AnsibleParserError("Host filter returned no data. Please confirm your host_filter value is valid")
+            raise AnsibleParserError("Unexpected data returned: {0} -- {1}".format(e, error_body))
+
         response_body = response.read()
         json_data = json.loads(response_body.decode('utf-8'))
+        self.display.vvv("Returned Data: %s" % json.dumps(json_data, indent=4, sort_keys=True))
         if 200 <= response.status <= 299:
             return json_data
         if response.status == 404 and json_data['status'] == "No objects found.":
@@ -155,7 +180,7 @@ class InventoryModule(BaseInventoryPlugin, Constructable):
         """Query for all hosts """
         self.display.vvv("Querying Icinga2 for inventory")
         query_args = {
-            "attrs": ["address", "state_type", "state", "groups"],
+            "attrs": ["address", "display_name", "state_type", "state", "groups"],
         }
         if self.host_filter is not None:
             query_args['host_filter'] = self.host_filter
@@ -177,24 +202,35 @@ class InventoryModule(BaseInventoryPlugin, Constructable):
         """Convert Icinga2 API data to JSON format for Ansible"""
         groups_dict = {"_meta": {"hostvars": {}}}
         for entry in json_data:
-            host_name = entry['name']
             host_attrs = entry['attrs']
+            if self.inventory_attr == "name":
+                host_name = entry.get('name')
+            if self.inventory_attr == "address":
+                # When looking for address for inventory, if missing fallback to object name
+                if host_attrs.get('address', '') != '':
+                    host_name = host_attrs.get('address')
+                else:
+                    host_name = entry.get('name')
+            if self.inventory_attr == "display_name":
+                host_name = host_attrs.get('display_name')
             if host_attrs['state'] == 0:
                 host_attrs['state'] = 'on'
             else:
                 host_attrs['state'] = 'off'
-            host_groups = host_attrs['groups']
+            host_groups = host_attrs.get('groups')
-            host_addr = host_attrs['address']
+            self.inventory.add_host(host_name)
-            self.inventory.add_host(host_addr)
             for group in host_groups:
                 if group not in self.inventory.groups.keys():
                     self.inventory.add_group(group)
-                self.inventory.add_child(group, host_addr)
+                self.inventory.add_child(group, host_name)
-            self.inventory.set_variable(host_addr, 'address', host_addr)
+            # If the address attribute is populated, override ansible_host with the value
-            self.inventory.set_variable(host_addr, 'hostname', host_name)
+            if host_attrs.get('address') != '':
-            self.inventory.set_variable(host_addr, 'state',
+                self.inventory.set_variable(host_name, 'ansible_host', host_attrs.get('address'))
+            self.inventory.set_variable(host_name, 'hostname', entry.get('name'))
+            self.inventory.set_variable(host_name, 'display_name', host_attrs.get('display_name'))
+            self.inventory.set_variable(host_name, 'state',
                                         host_attrs['state'])
-            self.inventory.set_variable(host_addr, 'state_type',
+            self.inventory.set_variable(host_name, 'state_type',
                                         host_attrs['state_type'])
         return groups_dict
 
@@ -211,6 +247,7 @@ class InventoryModule(BaseInventoryPlugin, Constructable):
         self.icinga2_password = self.get_option('password')
         self.ssl_verify = self.get_option('validate_certs')
         self.host_filter = self.get_option('host_filter')
+        self.inventory_attr = self.get_option('inventory_attr')
         # Not currently enabled
         # self.cache_key = self.get_cache_key(path)
         # self.use_cache = cache and self.get_option('cache')

tests/unit/plugins/inventory/test_icinga2.py

@@ -37,6 +37,7 @@ def query_hosts(hosts=None, attrs=None, joins=None, host_filter=None):
             'attrs': {
                 'address': 'test-host1.home.local',
                 'groups': ['home_servers', 'servers_dell'],
+                'display_name': 'Test Host 1',
                 'state': 0.0,
                 'state_type': 1.0
             },
@@ -48,6 +49,7 @@ def query_hosts(hosts=None, attrs=None, joins=None, host_filter=None):
         {
             'attrs': {
                 'address': 'test-host2.home.local',
+                'display_name': 'Test Host 2',
                 'groups': ['home_servers', 'servers_hp'],
                 'state': 1.0,
                 'state_type': 1.0
@@ -56,6 +58,19 @@ def query_hosts(hosts=None, attrs=None, joins=None, host_filter=None):
             'meta': {},
             'name': 'test-host2',
             'type': 'Host'
+        },
+        {
+            'attrs': {
+                'address': '',
+                'display_name': 'Test Host 3',
+                'groups': ['not_home_servers', 'servers_hp'],
+                'state': 1.0,
+                'state_type': 1.0
+            },
+            'joins': {},
+            'meta': {},
+            'name': 'test-host3.example.com',
+            'type': 'Host'
         }
     ]
     return json_host_data
@@ -66,6 +81,7 @@ def test_populate(inventory, mocker):
     inventory.icinga2_user = 'ansible'
     inventory.icinga2_password = 'password'
     inventory.icinga2_url = 'https://localhost:5665' + '/v1'
+    inventory.inventory_attr = "address"
 
     # bypass authentication and API fetch calls
     inventory._check_api = mocker.MagicMock(side_effect=check_api)
@@ -77,6 +93,9 @@ def test_populate(inventory, mocker):
     print(host1_info)
     host2_info = inventory.inventory.get_host('test-host2.home.local')
     print(host2_info)
+    host3_info = inventory.inventory.get_host('test-host3.example.com')
+    assert inventory.inventory.get_host('test-host3.example.com') is not None
+    print(host3_info)
 
     # check if host in the home_servers group
     assert 'home_servers' in inventory.inventory.groups
@@ -87,11 +106,29 @@ def test_populate(inventory, mocker):
     assert group1_data.hosts == group1_test_data
     # Test servers_hp group
     group2_data = inventory.inventory.groups['servers_hp']
-    group2_test_data = [host2_info]
+    group2_test_data = [host2_info, host3_info]
     print(group2_data.hosts)
     print(group2_test_data)
     assert group2_data.hosts == group2_test_data
 
-    # check if host state rules apply properyl
+    # check if host state rules apply properly
     assert host1_info.get_vars()['state'] == 'on'
+    assert host1_info.get_vars()['display_name'] == "Test Host 1"
     assert host2_info.get_vars()['state'] == 'off'
+    assert host3_info.get_vars().get('ansible_host') is None
+
+    # Confirm attribute options switcher
+    inventory.inventory_attr = "name"
+    inventory._populate()
+    assert inventory.inventory.get_host('test-host3.example.com') is not None
+    host2_info = inventory.inventory.get_host('test-host2')
+    assert host2_info is not None
+    assert host2_info.get_vars().get('ansible_host') == 'test-host2.home.local'
+
+    # Confirm attribute options switcher
+    inventory.inventory_attr = "display_name"
+    inventory._populate()
+    assert inventory.inventory.get_host('Test Host 3') is not None
+    host2_info = inventory.inventory.get_host('Test Host 2')
+    assert host2_info is not None
+    assert host2_info.get_vars().get('ansible_host') == 'test-host2.home.local'