From de82d5ac71819877b4e31bd0115bdaade028bb78 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 13:52:43 +0200 Subject: [PATCH 01/23] Remove duplicates from the list of hostgroups --- modules/device.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/device.py b/modules/device.py index e61cede..ee8b666 100644 --- a/modules/device.py +++ b/modules/device.py @@ -135,6 +135,9 @@ class PhysicalDevice: self.hostgroups = [hg.generate(f) for f in hg_format] else: self.hostgroups.append(hg.generate(hg_format)) + self.hostgroups = list(set(self.hostgroups)) + self.logger.debug(f"Host {self.name}: Should be member " + f"of groups: {self.hostgroups}") def set_template(self, prefer_config_context, overrule_custom): """Set Template""" From 753633e7d2673683e8b54df2674c30f98d3f0302 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 15:01:45 +0200 Subject: [PATCH 02/23] Added checks for empty list of hostgroups, improved some logging --- modules/device.py | 27 +++++++++++++++------------ modules/hostgroups.py | 14 ++++++++------ netbox_zabbix_sync.py | 2 ++ 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/modules/device.py b/modules/device.py index ee8b666..26bac0e 100644 --- a/modules/device.py +++ b/modules/device.py @@ -135,9 +135,14 @@ class PhysicalDevice: self.hostgroups = [hg.generate(f) for f in hg_format] else: self.hostgroups.append(hg.generate(hg_format)) - self.hostgroups = list(set(self.hostgroups)) - self.logger.debug(f"Host {self.name}: Should be member " - f"of groups: {self.hostgroups}") + # Remove dyuplicates and None values + self.hostgroups = list(filter(None, list(set(self.hostgroups)))) + if self.hostgroups: + self.logger.debug(f"Host {self.name}: Should be member " + f"of groups: {self.hostgroups}") + return True + return False + def set_template(self, prefer_config_context, overrule_custom): """Set Template""" @@ -180,8 +185,6 @@ class PhysicalDevice: self.logger.warning(e) raise TemplateError(e) - - def get_templates_context(self): """Get Zabbix templates from the device context""" if "zabbix" not in self.config_context: @@ -301,7 +304,8 @@ class PhysicalDevice: "name": zbx_template["name"], } ) - e = f"Host {self.name}: found template {zbx_template['name']}" + e = (f"Host {self.name}: Found template '{zbx_template['name']}' " + f"(ID:{zbx_template['templateid']})") self.logger.debug(e) # Return error should the template not be found in Zabbix if not template_match: @@ -324,7 +328,7 @@ class PhysicalDevice: if group["name"] == hg: self.group_ids.append({"groupid": group["groupid"]}) e = ( - f"Host {self.name}: matched group " + f"Host {self.name}: Matched group " f"\"{group['name']}\" (ID:{group['groupid']})" ) self.logger.debug(e) @@ -506,7 +510,6 @@ class PhysicalDevice: templateids.append({"templateid": template["templateid"]}) # Set interface, group and template configuration interfaces = self.setInterfaceDetails() - groups = self.group_ids # Set Zabbix proxy if defined self.setProxy(proxies) # Set basic data for host creation @@ -515,7 +518,7 @@ class PhysicalDevice: "name": self.visible_name, "status": self.zabbix_state, "interfaces": interfaces, - "groups": groups, + "groups": self.group_ids, "templates": templateids, "description": description, "inventory_mode": self.inventory_mode, @@ -544,7 +547,7 @@ class PhysicalDevice: # Set NetBox custom field to hostID value. self.nb.custom_fields[config["device_cf"]] = int(self.zabbix_id) self.nb.save() - msg = f"Host {self.name}: Created host in Zabbix." + msg = f"Host {self.name}: Created host in Zabbix. (ID:{self.zabbix_id})" self.logger.info(msg) self.create_journal_entry("success", msg) else: @@ -944,8 +947,8 @@ class PhysicalDevice: tmpls_from_zabbix.pop(pos) succesfull_templates.append(nb_tmpl) self.logger.debug( - f"Host {self.name}: template " - f"{nb_tmpl['name']} is present in Zabbix." + f"Host {self.name}: Template " + f"'{nb_tmpl['name']}' is present in Zabbix." ) break if ( diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 8bbec86..cad31e1 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -93,6 +93,7 @@ class Hostgroup: format_options["cluster"] = self.nb.cluster.name format_options["cluster_type"] = self.nb.cluster.type.name self.format_options = format_options + self.logger.debug(f"Host {self.name}: Resolved properties for use in hostgroups: {self.format_options}") def set_nesting( self, nested_sitegroup_flag, nested_region_flag, nb_groups, nb_regions @@ -135,17 +136,18 @@ class Hostgroup: hostgroup_value = self.format_options[hg_item] if hostgroup_value: hg_output.append(hostgroup_value) + else: + self.logger.info(f"Host {self.name}: Used field '{hg_item}' has no value.") # Check if the hostgroup is populated with at least one item. if bool(hg_output): return "/".join(hg_output) msg = ( - f"Unable to generate hostgroup for host {self.name}." - " Not enough valid items. This is most likely" - " due to the use of custom fields that are empty" - " or an invalid hostgroup format." + f"Host {self.name}: Generating hostgroup name for '{hg_format}' failed. " + f"This is most likely due to fields that have no value." ) - self.logger.error(msg) - raise HostgroupError(msg) + self.logger.warning(msg) + return None + #raise HostgroupError(msg) def list_formatoptions(self): """ diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index d9ff71b..1515041 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -212,6 +212,8 @@ def main(arguments): config["hostgroup_format"], netbox_site_groups, netbox_regions) # Check if a valid hostgroup has been found for this VM. if not device.hostgroups: + logger.warning(f"Host {device.name}: Host has no valid " + f"hostgroups, Skipping this host...") continue device.set_inventory(nb_device) device.set_usermacros() From 2a3d5863029431ac70355d899c704ea5ac2b5d8f Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 15:06:52 +0200 Subject: [PATCH 03/23] corrected typo --- modules/device.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/device.py b/modules/device.py index 26bac0e..472384c 100644 --- a/modules/device.py +++ b/modules/device.py @@ -135,7 +135,7 @@ class PhysicalDevice: self.hostgroups = [hg.generate(f) for f in hg_format] else: self.hostgroups.append(hg.generate(hg_format)) - # Remove dyuplicates and None values + # Remove duplicates and None values self.hostgroups = list(filter(None, list(set(self.hostgroups)))) if self.hostgroups: self.logger.debug(f"Host {self.name}: Should be member " From 906c7198630e727065eb66da22e6c30e9e218a37 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 15:16:39 +0200 Subject: [PATCH 04/23] corrected linting errors --- modules/device.py | 2 +- modules/hostgroups.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/modules/device.py b/modules/device.py index 472384c..524be55 100644 --- a/modules/device.py +++ b/modules/device.py @@ -138,7 +138,7 @@ class PhysicalDevice: # Remove duplicates and None values self.hostgroups = list(filter(None, list(set(self.hostgroups)))) if self.hostgroups: - self.logger.debug(f"Host {self.name}: Should be member " + self.logger.debug(f"Host {self.name}: Should be member " f"of groups: {self.hostgroups}") return True return False diff --git a/modules/hostgroups.py b/modules/hostgroups.py index cad31e1..f98c09b 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -11,6 +11,7 @@ class Hostgroup: Takes type (vm or dev) and NB object""" # pylint: disable=too-many-arguments, disable=too-many-positional-arguments + # pylint: disable=logging-fstring-interpolation def __init__( self, obj_type, @@ -93,7 +94,8 @@ class Hostgroup: format_options["cluster"] = self.nb.cluster.name format_options["cluster_type"] = self.nb.cluster.type.name self.format_options = format_options - self.logger.debug(f"Host {self.name}: Resolved properties for use in hostgroups: {self.format_options}") + self.logger.debug(f"Host {self.name}: Resolved properties for use " + f"in hostgroups: {self.format_options}") def set_nesting( self, nested_sitegroup_flag, nested_region_flag, nb_groups, nb_regions @@ -142,7 +144,7 @@ class Hostgroup: if bool(hg_output): return "/".join(hg_output) msg = ( - f"Host {self.name}: Generating hostgroup name for '{hg_format}' failed. " + f"Host {self.name}: Generating hostgroup name for '{hg_format}' failed. " f"This is most likely due to fields that have no value." ) self.logger.warning(msg) From 435fd1fa7822b995a227e0bdace41bd54a49d193 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 17:09:23 +0200 Subject: [PATCH 05/23] Fixed issues with tag mapping --- modules/device.py | 8 ++++---- modules/tags.py | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/modules/device.py b/modules/device.py index 524be55..bdde4a1 100644 --- a/modules/device.py +++ b/modules/device.py @@ -428,16 +428,16 @@ class PhysicalDevice: tags = ZabbixTags( self.nb, self._tag_map(), - config['tag_sync'], - config['tag_lower'], + tag_sync=config['tag_sync'], + tag_lower=config['tag_lower'], tag_name=config['tag_name'], tag_value=config['tag_value'], logger=self.logger, host=self.name, ) - if tags.sync is False: + if config['tag_sync'] is False: self.tags = [] - + return False self.tags = tags.generate() return True diff --git a/modules/tags.py b/modules/tags.py index 441ebe2..659966c 100644 --- a/modules/tags.py +++ b/modules/tags.py @@ -15,7 +15,7 @@ class ZabbixTags: self, nb, tag_map, - tag_sync, + tag_sync=False, tag_lower=True, tag_name=None, tag_value=None, @@ -130,4 +130,6 @@ class ZabbixTags: if t: tags.append(t) - return remove_duplicates(tags, sortkey="tag") + tags = remove_duplicates(tags, sortkey="tag") + self.logger.debug(f"Host {self.name}: Resolved tags: {tags}") + return tags From 9933c97e949d0731ff668394626a7bb89fb59d84 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 17:28:57 +0200 Subject: [PATCH 06/23] improved debug logging --- modules/device.py | 1 + modules/tools.py | 5 +++-- modules/usermacros.py | 5 ++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/modules/device.py b/modules/device.py index bdde4a1..aa323fb 100644 --- a/modules/device.py +++ b/modules/device.py @@ -229,6 +229,7 @@ class PhysicalDevice: self.inventory = field_mapper( self.name, self._inventory_map(), nbdevice, self.logger ) + self.logger.debug(f"Host {self.name}: Resolved inventory: {self.inventory}") return True def isCluster(self): diff --git a/modules/tools.py b/modules/tools.py index 823410e..ed327a7 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -159,7 +159,7 @@ def sanatize_log_output(data): """ Used for the update function to Zabbix which shows the data that its using to update the host. - Removes and sensitive data from the input. + Removes any sensitive data from the input. """ if not isinstance(data, dict): return data @@ -168,7 +168,8 @@ def sanatize_log_output(data): if "macros" in data: for macro in sanitized_data["macros"]: # Check if macro is secret type - if not macro["type"] == str(1): + if not (macro["type"] == str(1) or + macro["type"] == 1): continue macro["value"] = "********" # Check for interface data diff --git a/modules/usermacros.py b/modules/usermacros.py index 6d396c8..1e23213 100644 --- a/modules/usermacros.py +++ b/modules/usermacros.py @@ -6,7 +6,7 @@ All of the Zabbix Usermacro related configuration from logging import getLogger from re import match -from modules.tools import field_mapper +from modules.tools import field_mapper, sanatize_log_output class ZabbixUsermacros: @@ -98,6 +98,7 @@ class ZabbixUsermacros: Generate full set of Usermacros """ macros = [] + data={} # Parse the field mapper for usermacros if self.usermacro_map: self.logger.debug(f"Host {self.nb.name}: Starting usermacro mapper") @@ -119,4 +120,6 @@ class ZabbixUsermacros: m = self.render_macro(macro, properties) if m: macros.append(m) + data={'macros': macros} + self.logger.debug(f"Host {self.name}: Resolved macros: {sanatize_log_output(data)}") return macros From c2b25e0cd2747b8a5d04bd0c8ba5ee38ad873ece Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 17:35:10 +0200 Subject: [PATCH 07/23] fixed linting --- modules/device.py | 2 +- modules/tools.py | 2 +- modules/usermacros.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/device.py b/modules/device.py index aa323fb..28e78b4 100644 --- a/modules/device.py +++ b/modules/device.py @@ -229,7 +229,7 @@ class PhysicalDevice: self.inventory = field_mapper( self.name, self._inventory_map(), nbdevice, self.logger ) - self.logger.debug(f"Host {self.name}: Resolved inventory: {self.inventory}") + self.logger.debug(f"Host {self.name}: Resolved inventory: {self.inventory}") return True def isCluster(self): diff --git a/modules/tools.py b/modules/tools.py index ed327a7..f3d27cc 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -168,7 +168,7 @@ def sanatize_log_output(data): if "macros" in data: for macro in sanitized_data["macros"]: # Check if macro is secret type - if not (macro["type"] == str(1) or + if not (macro["type"] == str(1) or macro["type"] == 1): continue macro["value"] = "********" diff --git a/modules/usermacros.py b/modules/usermacros.py index 1e23213..fd95eab 100644 --- a/modules/usermacros.py +++ b/modules/usermacros.py @@ -121,5 +121,5 @@ class ZabbixUsermacros: if m: macros.append(m) data={'macros': macros} - self.logger.debug(f"Host {self.name}: Resolved macros: {sanatize_log_output(data)}") + self.logger.debug(f"Host {self.name}: Resolved macros: {sanatize_log_output(data)}") return macros From 1de0b0781bcf71d5ea2edacc59c0a1f329f3770b Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 20:44:59 +0200 Subject: [PATCH 08/23] Removed default for hostgroups and fixed bug for hostgroup attributes which do not exist --- modules/device.py | 3 +++ modules/hostgroups.py | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/modules/device.py b/modules/device.py index 28e78b4..c3baa17 100644 --- a/modules/device.py +++ b/modules/device.py @@ -564,6 +564,9 @@ class PhysicalDevice: final_data = [] # Check if the hostgroup is in a nested format and check each parent for hostgroup in self.hostgroups: + # Check if hostgroup is string. If Nonetype skip hostgroup + if not isinstance(hostgroup, str): + continue for pos in range(len(hostgroup.split("/"))): zabbix_hg = hostgroup.rsplit("/", pos)[0] if self.lookupZabbixHostgroup(hostgroups, zabbix_hg): diff --git a/modules/hostgroups.py b/modules/hostgroups.py index f98c09b..d5306a3 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -108,11 +108,6 @@ class Hostgroup: def generate(self, hg_format=None): """Generate hostgroup based on a provided format""" - # Set format to default in case its not specified - if not hg_format: - hg_format = ( - "site/manufacturer/role" if self.type == "dev" else "cluster/role" - ) # Split all given names hg_output = [] hg_items = hg_format.split("/") From a522c989298429a3dd165943f15b1e6f706c9f15 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 20:50:04 +0200 Subject: [PATCH 09/23] Removed default None for hg_format making a hostgroup format input required. --- modules/hostgroups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/hostgroups.py b/modules/hostgroups.py index d5306a3..38c44c0 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -106,7 +106,7 @@ class Hostgroup: "region": {"flag": nested_region_flag, "data": nb_regions}, } - def generate(self, hg_format=None): + def generate(self, hg_format): """Generate hostgroup based on a provided format""" # Split all given names hg_output = [] From 6d4f1ac0a54ebed6883f8f882bb27ffc5c0cef20 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:28:13 +0200 Subject: [PATCH 10/23] Added hostgroup tests --- tests/test_hostgroups.py | 80 +++++++++++----- tests/test_list_hostgroup_formats.py | 137 +++++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 24 deletions(-) create mode 100644 tests/test_list_hostgroup_formats.py diff --git a/tests/test_hostgroups.py b/tests/test_hostgroups.py index 1e652ec..995d26c 100644 --- a/tests/test_hostgroups.py +++ b/tests/test_hostgroups.py @@ -163,10 +163,6 @@ class TestHostgroups(unittest.TestCase): """Test different hostgroup formats for devices.""" hostgroup = Hostgroup("dev", self.mock_device, "4.0", self.mock_logger) - # Default format: site/manufacturer/role - default_result = hostgroup.generate() - self.assertEqual(default_result, "TestSite/TestManufacturer/TestRole") - # Custom format: site/region custom_result = hostgroup.generate("site/region") self.assertEqual(custom_result, "TestSite/TestRegion") @@ -180,7 +176,7 @@ class TestHostgroups(unittest.TestCase): hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) # Default format: cluster/role - default_result = hostgroup.generate() + default_result = hostgroup.generate("cluster/role") self.assertEqual(default_result, "TestCluster/TestRole") # Custom format: site/tenant @@ -251,7 +247,7 @@ class TestHostgroups(unittest.TestCase): hostgroup = Hostgroup("dev", minimal_device, "4.0", self.mock_logger) # Generate with default format - result = hostgroup.generate() + result = hostgroup.generate("site/manufacturer/role") # Site is missing, so only manufacturer and role should be included self.assertEqual(result, "MinimalManufacturer/MinimalRole") @@ -259,24 +255,6 @@ class TestHostgroups(unittest.TestCase): with self.assertRaises(HostgroupError): hostgroup.generate("site/nonexistent/role") - def test_hostgroup_missing_required_attributes(self): - """Test handling when no valid hostgroup can be generated.""" - # Create a VM with minimal attributes that won't satisfy any format - minimal_vm = MagicMock() - minimal_vm.name = "minimal-vm" - minimal_vm.site = None - minimal_vm.tenant = None - minimal_vm.platform = None - minimal_vm.role = None - minimal_vm.cluster = None - minimal_vm.custom_fields = {} - - hostgroup = Hostgroup("vm", minimal_vm, "4.0", self.mock_logger) - - # With default format of cluster/role, both are None, so should raise an error - with self.assertRaises(HostgroupError): - hostgroup.generate() - def test_nested_region_hostgroups(self): """Test hostgroup generation with nested regions.""" # Mock the build_path function to return a predictable result @@ -334,6 +312,60 @@ class TestHostgroups(unittest.TestCase): calls = [call.write(f"The following options are available for host test-device"), call.write('\n')] mock_stdout.assert_has_calls(calls, any_order=True) + + def test_vm_list_based_hostgroup_format(self): + """Test VM hostgroup generation with a list-based format.""" + hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) + + # Test with a list of format strings + format_list = ["platform", "role", "cluster_type/cluster"] + + # Generate hostgroups for each format in the list + hostgroups = [] + for fmt in format_list: + result = hostgroup.generate(fmt) + if result: # Only add non-None results + hostgroups.append(result) + + # Verify each expected hostgroup is generated + self.assertEqual(len(hostgroups), 3) # Should have 3 hostgroups + self.assertIn("TestPlatform", hostgroups) + self.assertIn("TestRole", hostgroups) + self.assertIn("TestClusterType/TestCluster", hostgroups) + + def test_nested_format_splitting(self): + """Test that formats with slashes correctly split and resolve each component.""" + hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) + + # Test a format with slashes that should be split + complex_format = "cluster_type/cluster" + result = hostgroup.generate(complex_format) + + # Verify the format is correctly split and each component resolved + self.assertEqual(result, "TestClusterType/TestCluster") + + def test_multiple_hostgroup_formats_device(self): + """Test device hostgroup generation with multiple formats.""" + hostgroup = Hostgroup("dev", self.mock_device, "4.0", self.mock_logger) + + # Test with various formats that would be in a list + formats = [ + "site", + "manufacturer/role", + "platform/location", + "tenant_group/tenant" + ] + + # Generate and check each format + results = {} + for fmt in formats: + results[fmt] = hostgroup.generate(fmt) + + # Verify results + self.assertEqual(results["site"], "TestSite") + self.assertEqual(results["manufacturer/role"], "TestManufacturer/TestRole") + self.assertEqual(results["platform/location"], "TestPlatform/TestLocation") + self.assertEqual(results["tenant_group/tenant"], "TestTenantGroup/TestTenant") if __name__ == "__main__": diff --git a/tests/test_list_hostgroup_formats.py b/tests/test_list_hostgroup_formats.py new file mode 100644 index 0000000..9b8cc21 --- /dev/null +++ b/tests/test_list_hostgroup_formats.py @@ -0,0 +1,137 @@ +"""Tests for list-based hostgroup formats in configuration.""" +import unittest +from unittest.mock import MagicMock, patch +from modules.hostgroups import Hostgroup +from modules.exceptions import HostgroupError +from modules.tools import verify_hg_format + + +class TestListHostgroupFormats(unittest.TestCase): + """Test class for list-based hostgroup format functionality.""" + + def setUp(self): + """Set up test fixtures.""" + # Create mock logger + self.mock_logger = MagicMock() + + # Create mock device + self.mock_device = MagicMock() + self.mock_device.name = "test-device" + + # Set up site information + site = MagicMock() + site.name = "TestSite" + + # Set up region information + region = MagicMock() + region.name = "TestRegion" + region.__str__.return_value = "TestRegion" + site.region = region + + # Set device site + self.mock_device.site = site + + # Set up role information + self.mock_device_role = MagicMock() + self.mock_device_role.name = "TestRole" + self.mock_device_role.__str__.return_value = "TestRole" + self.mock_device.role = self.mock_device_role + + # Set up rack information + rack = MagicMock() + rack.name = "TestRack" + self.mock_device.rack = rack + + # Set up platform information + platform = MagicMock() + platform.name = "TestPlatform" + self.mock_device.platform = platform + + # Device-specific properties + device_type = MagicMock() + manufacturer = MagicMock() + manufacturer.name = "TestManufacturer" + device_type.manufacturer = manufacturer + self.mock_device.device_type = device_type + + # Create mock VM + self.mock_vm = MagicMock() + self.mock_vm.name = "test-vm" + + # Reuse site from device + self.mock_vm.site = site + + # Set up role for VM + self.mock_vm.role = self.mock_device_role + + # Set up platform for VM + self.mock_vm.platform = platform + + # VM-specific properties + cluster = MagicMock() + cluster.name = "TestCluster" + cluster_type = MagicMock() + cluster_type.name = "TestClusterType" + cluster.type = cluster_type + self.mock_vm.cluster = cluster + + def test_verify_list_based_hostgroup_format(self): + """Test verification of list-based hostgroup formats.""" + # List format with valid items + valid_format = ["region", "site", "rack"] + + # List format with nested path + valid_nested_format = ["region", "site/rack"] + + # List format with invalid item + invalid_format = ["region", "invalid_item", "rack"] + + # Should not raise exception for valid formats + verify_hg_format(valid_format, hg_type="dev", logger=self.mock_logger) + verify_hg_format(valid_nested_format, hg_type="dev", logger=self.mock_logger) + + # Should raise exception for invalid format + with self.assertRaises(HostgroupError): + verify_hg_format(invalid_format, hg_type="dev", logger=self.mock_logger) + + def test_simulate_hostgroup_generation_from_config(self): + """Simulate how the main script would generate hostgroups from list-based config.""" + # Mock configuration with list-based hostgroup format + config_format = ["region", "site", "rack"] + hostgroup = Hostgroup("dev", self.mock_device, "4.0", self.mock_logger) + + # Simulate the main script's hostgroup generation process + hostgroups = [] + for fmt in config_format: + result = hostgroup.generate(fmt) + if result: + hostgroups.append(result) + + # Check results + self.assertEqual(len(hostgroups), 3) + self.assertIn("TestRegion", hostgroups) + self.assertIn("TestSite", hostgroups) + self.assertIn("TestRack", hostgroups) + + def test_vm_hostgroup_format_from_config(self): + """Test VM hostgroup generation with list-based format.""" + # Mock VM configuration with mixed format + config_format = ["platform", "role", "cluster_type/cluster"] + hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) + + # Simulate the main script's hostgroup generation process + hostgroups = [] + for fmt in config_format: + result = hostgroup.generate(fmt) + if result: + hostgroups.append(result) + + # Check results + self.assertEqual(len(hostgroups), 3) + self.assertIn("TestPlatform", hostgroups) + self.assertIn("TestRole", hostgroups) + self.assertIn("TestClusterType/TestCluster", hostgroups) + + +if __name__ == "__main__": + unittest.main() From 4a53b53789d9af793e18fdae9b911d87e61d7375 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:28:32 +0200 Subject: [PATCH 11/23] Removed previous patch for Nonetype hostgroups and made a proper fix by refactoring the set_hostgroup() function and removing it from virtual_machines.py --- modules/device.py | 6 ++---- modules/hostgroups.py | 1 - modules/virtual_machine.py | 20 +------------------- 3 files changed, 3 insertions(+), 24 deletions(-) diff --git a/modules/device.py b/modules/device.py index c3baa17..03ca191 100644 --- a/modules/device.py +++ b/modules/device.py @@ -48,6 +48,7 @@ class PhysicalDevice: self.zbx_template_names = [] self.zbx_templates = [] self.hostgroups = [] + self.hostgroup_type = "dev" self.tenant = nb.tenant self.config_context = nb.config_context self.zbxproxy = None @@ -121,7 +122,7 @@ class PhysicalDevice: """Set the hostgroup for this device""" # Create new Hostgroup instance hg = Hostgroup( - "dev", + self.hostgroup_type, self.nb, self.nb_api_version, logger=self.logger, @@ -564,9 +565,6 @@ class PhysicalDevice: final_data = [] # Check if the hostgroup is in a nested format and check each parent for hostgroup in self.hostgroups: - # Check if hostgroup is string. If Nonetype skip hostgroup - if not isinstance(hostgroup, str): - continue for pos in range(len(hostgroup.split("/"))): zabbix_hg = hostgroup.rsplit("/", pos)[0] if self.lookupZabbixHostgroup(hostgroups, zabbix_hg): diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 38c44c0..213b4cf 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -144,7 +144,6 @@ class Hostgroup: ) self.logger.warning(msg) return None - #raise HostgroupError(msg) def list_formatoptions(self): """ diff --git a/modules/virtual_machine.py b/modules/virtual_machine.py index e0f7abb..847375d 100644 --- a/modules/virtual_machine.py +++ b/modules/virtual_machine.py @@ -16,6 +16,7 @@ class VirtualMachine(PhysicalDevice): super().__init__(*args, **kwargs) self.hostgroup = None self.zbx_template_names = None + self.hostgroup_type = "vm" def _inventory_map(self): """use VM inventory maps""" @@ -29,25 +30,6 @@ class VirtualMachine(PhysicalDevice): """use VM tag maps""" return config["vm_tag_map"] - def set_hostgroup(self, hg_format, nb_site_groups, nb_regions): - """Set the hostgroup for this device""" - # Create new Hostgroup instance - hg = Hostgroup( - "vm", - self.nb, - self.nb_api_version, - logger=self.logger, - nested_sitegroup_flag=config["traverse_site_groups"], - nested_region_flag=config["traverse_regions"], - nb_groups=nb_site_groups, - nb_regions=nb_regions, - ) - # Generate hostgroup based on hostgroup format - if isinstance(hg_format, list): - self.hostgroups = [hg.generate(f) for f in hg_format] - else: - self.hostgroups.append(hg.generate(hg_format)) - def set_vm_template(self): """Set Template for VMs. Overwrites default class to skip a lookup of custom fields.""" From 29a54e5a86e6fca5b1858cac5f6c5b71fd512628 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:29:36 +0200 Subject: [PATCH 12/23] Removed unused hostgroup import since the hostgroup generate function function has been moved to devices.py --- modules/virtual_machine.py | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/virtual_machine.py b/modules/virtual_machine.py index 847375d..8c52033 100644 --- a/modules/virtual_machine.py +++ b/modules/virtual_machine.py @@ -2,7 +2,6 @@ """Module that hosts all functions for virtual machine processing""" from modules.device import PhysicalDevice from modules.exceptions import InterfaceConfigError, SyncInventoryError, TemplateError -from modules.hostgroups import Hostgroup from modules.interface import ZabbixInterface from modules.config import load_config # Load config From 5923682d48b8836d339ae2c05f826adf80af65cd Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:42:46 +0200 Subject: [PATCH 13/23] Fixes workflows to be executed 2 times. --- .github/workflows/quality.yml | 1 - .github/workflows/run_tests.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 8dfbe3f..81cad97 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -2,7 +2,6 @@ name: Pylint Quality control on: - push: pull_request: workflow_call: diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 6a16a64..589fc47 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -2,7 +2,6 @@ name: Pytest code testing on: - push: pull_request: workflow_call: From e4a1a17ded7a911a52b360a7bf61cd6142f11434 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 10:43:47 +0200 Subject: [PATCH 14/23] Logging improvements --- modules/device.py | 80 +++++++++++++++++++++---------------------- modules/tags.py | 8 ++--- modules/tools.py | 10 +++--- modules/usermacros.py | 8 ++--- netbox_zabbix_sync.py | 14 ++++---- 5 files changed, 59 insertions(+), 61 deletions(-) diff --git a/modules/device.py b/modules/device.py index 03ca191..61a11c6 100644 --- a/modules/device.py +++ b/modules/device.py @@ -98,7 +98,7 @@ class PhysicalDevice: self.zabbix_id = self.nb.custom_fields[config["device_cf"]] else: e = f'Host {self.name}: Custom field {config["device_cf"]} not present' - self.logger.warning(e) + self.logger.error(e) raise SyncInventoryError(e) # Validate hostname format. @@ -226,7 +226,7 @@ class PhysicalDevice: return False self.inventory = {} if config["inventory_sync"] and self.inventory_mode in [0, 1]: - self.logger.debug(f"Host {self.name}: Starting inventory mapper") + self.logger.debug(f"Host {self.name}: Starting inventory mapper.") self.inventory = field_mapper( self.name, self._inventory_map(), nbdevice, self.logger ) @@ -248,14 +248,14 @@ class PhysicalDevice: f"Unable to proces {self.name} for cluster calculation: " f"not part of a cluster." ) - self.logger.warning(e) + self.logger.info(e) raise SyncInventoryError(e) if not self.nb.virtual_chassis.master: e = ( f"{self.name} is part of a NetBox virtual chassis which does " "not have a master configured. Skipping for this reason." ) - self.logger.error(e) + self.logger.warning(e) raise SyncInventoryError(e) return self.nb.virtual_chassis.master.id @@ -267,14 +267,14 @@ class PhysicalDevice: """ masterid = self.getClusterMaster() if masterid == self.id: - self.logger.debug( + self.logger.info( f"Host {self.name} is primary cluster member. " f"Modifying hostname from {self.name} to " + f"{self.nb.virtual_chassis.name}." ) self.name = self.nb.virtual_chassis.name return True - self.logger.debug(f"Host {self.name} is non-primary cluster member.") + self.logger.info(f"Host {self.name} is non-primary cluster member.") return False def zbxTemplatePrepper(self, templates): @@ -286,7 +286,7 @@ class PhysicalDevice: # Check if there are templates defined if not self.zbx_template_names: e = f"Host {self.name}: No templates found" - self.logger.info(e) + self.logger.warning(e) raise SyncInventoryError() # Set variable to empty list self.zbx_templates = [] @@ -477,7 +477,7 @@ class PhysicalDevice: # If the proxy name matches if proxy["name"] == proxy_name: self.logger.debug( - f"Host {self.name}: using {proxy['type']}" f" {proxy_name}" + f"Host {self.name}: using {proxy['type']}" f" '{proxy_name}'" ) self.zbxproxy = proxy return True @@ -640,8 +640,6 @@ class PhysicalDevice: ) self.logger.warning(e) raise SyncInventoryError(e) - #if self.group_ids: - # self.group_ids.append(self.pri_group_id) # Prepare templates and proxy config self.zbxTemplatePrepper(templates) @@ -674,10 +672,10 @@ class PhysicalDevice: raise SyncInventoryError(e) host = host[0] if host["host"] == self.name: - self.logger.debug(f"Host {self.name}: hostname in-sync.") + self.logger.debug(f"Host {self.name}: Hostname in-sync.") else: - self.logger.warning( - f"Host {self.name}: hostname OUT of sync. " + self.logger.info( + f"Host {self.name}: Hostname OUT of sync. " f"Received value: {host['host']}" ) self.updateZabbixHost(host=self.name) @@ -685,17 +683,17 @@ class PhysicalDevice: # Execute check depending on wether the name is special or not if self.use_visible_name: if host["name"] == self.visible_name: - self.logger.debug(f"Host {self.name}: visible name in-sync.") + self.logger.debug(f"Host {self.name}: Visible name in-sync.") else: - self.logger.warning( - f"Host {self.name}: visible name OUT of sync." + self.logger.info( + f"Host {self.name}: Visible name OUT of sync." f" Received value: {host['name']}" ) self.updateZabbixHost(name=self.visible_name) # Check if the templates are in-sync if not self.zbx_template_comparer(host["parentTemplates"]): - self.logger.warning(f"Host {self.name}: template(s) OUT of sync.") + self.logger.info(f"Host {self.name}: Template(s) OUT of sync.") # Prepare Templates for API parsing templateids = [] for template in self.zbx_templates: @@ -705,7 +703,7 @@ class PhysicalDevice: templates_clear=host["parentTemplates"], templates=templateids ) else: - self.logger.debug(f"Host {self.name}: template(s) in-sync.") + self.logger.debug(f"Host {self.name}: Template(s) in-sync.") # Check if Zabbix version is 6 or higher. Issue #93 group_dictname = "hostgroups" @@ -714,15 +712,15 @@ class PhysicalDevice: # Check if hostgroups match if (sorted(host[group_dictname], key=itemgetter('groupid')) == sorted(self.group_ids, key=itemgetter('groupid'))): - self.logger.debug(f"Host {self.name}: hostgroups in-sync.") + self.logger.debug(f"Host {self.name}: Hostgroups in-sync.") else: - self.logger.warning(f"Host {self.name}: hostgroups OUT of sync.") + self.logger.info(f"Host {self.name}: Hostgroups OUT of sync.") self.updateZabbixHost(groups=self.group_ids) if int(host["status"]) == self.zabbix_state: - self.logger.debug(f"Host {self.name}: status in-sync.") + self.logger.debug(f"Host {self.name}: Status in-sync.") else: - self.logger.warning(f"Host {self.name}: status OUT of sync.") + self.logger.info(f"Host {self.name}: Status OUT of sync.") self.updateZabbixHost(status=str(self.zabbix_state)) # Check if a proxy has been defined @@ -730,13 +728,13 @@ class PhysicalDevice: # Check if proxy or proxy group is defined if (self.zbxproxy["idtype"] in host and host[self.zbxproxy["idtype"]] == self.zbxproxy["id"]): - self.logger.debug(f"Host {self.name}: proxy in-sync.") + self.logger.debug(f"Host {self.name}: Proxy in-sync.") # Backwards compatibility for Zabbix <= 6 elif "proxy_hostid" in host and host["proxy_hostid"] == self.zbxproxy["id"]: - self.logger.debug(f"Host {self.name}: proxy in-sync.") + self.logger.debug(f"Host {self.name}: Proxy in-sync.") # Proxy does not match, update Zabbix else: - self.logger.warning(f"Host {self.name}: proxy OUT of sync.") + self.logger.info(f"Host {self.name}: Proxy OUT of sync.") # Zabbix <= 6 patch if not str(self.zabbix.version).startswith("7"): self.updateZabbixHost(proxy_hostid=self.zbxproxy["id"]) @@ -759,7 +757,7 @@ class PhysicalDevice: if proxy_power and proxy_set: # Zabbix <= 6 fix self.logger.warning( - f"Host {self.name}: no proxy is configured in NetBox " + f"Host {self.name}: No proxy is configured in NetBox " "but is configured in Zabbix. Removing proxy config in Zabbix" ) if "proxy_hostid" in host and bool(host["proxy_hostid"]): @@ -773,26 +771,26 @@ class PhysicalDevice: # Checks if a proxy has been defined in Zabbix and if proxy_power config has been set if proxy_set and not proxy_power: # Display error message - self.logger.error( - f"Host {self.name} is configured " + self.logger.warning( + f"Host {self.name}: Is configured " f"with proxy in Zabbix but not in NetBox. The" " -p flag was ommited: no " "changes have been made." ) if not proxy_set: - self.logger.debug(f"Host {self.name}: proxy in-sync.") + self.logger.debug(f"Host {self.name}: Proxy in-sync.") # Check host inventory mode if str(host["inventory_mode"]) == str(self.inventory_mode): self.logger.debug(f"Host {self.name}: inventory_mode in-sync.") else: - self.logger.warning(f"Host {self.name}: inventory_mode OUT of sync.") + self.logger.info(f"Host {self.name}: inventory_mode OUT of sync.") self.updateZabbixHost(inventory_mode=str(self.inventory_mode)) if config["inventory_sync"] and self.inventory_mode in [0, 1]: # Check host inventory mapping if host["inventory"] == self.inventory: - self.logger.debug(f"Host {self.name}: inventory in-sync.") + self.logger.debug(f"Host {self.name}: Inventory in-sync.") else: - self.logger.warning(f"Host {self.name}: inventory OUT of sync.") + self.logger.info(f"Host {self.name}: Inventory OUT of sync.") self.updateZabbixHost(inventory=self.inventory) # Check host usermacros @@ -815,18 +813,18 @@ class PhysicalDevice: netbox_macros.sort(key=filter_with_macros) # Check if both lists are the same if host["macros"] == netbox_macros: - self.logger.debug(f"Host {self.name}: usermacros in-sync.") + self.logger.debug(f"Host {self.name}: Usermacros in-sync.") else: - self.logger.warning(f"Host {self.name}: usermacros OUT of sync.") + self.logger.info(f"Host {self.name}: Usermacros OUT of sync.") # Update Zabbix with NetBox usermacros self.updateZabbixHost(macros=self.usermacros) # Check host tags if config['tag_sync']: if remove_duplicates(host["tags"], sortkey="tag") == self.tags: - self.logger.debug(f"Host {self.name}: tags in-sync.") + self.logger.debug(f"Host {self.name}: Tags in-sync.") else: - self.logger.warning(f"Host {self.name}: tags OUT of sync.") + self.logger.info(f"Host {self.name}: Tags OUT of sync.") self.updateZabbixHost(tags=self.tags) # If only 1 interface has been found @@ -864,11 +862,11 @@ class PhysicalDevice: updates[key] = item if updates: # If interface updates have been found: push to Zabbix - self.logger.warning(f"Host {self.name}: Interface OUT of sync.") + self.logger.info(f"Host {self.name}: Interface OUT of sync.") if "type" in updates: # Changing interface type not supported. Raise exception. e = ( - f"Host {self.name}: changing interface type to " + f"Host {self.name}: Changing interface type to " f"{str(updates['type'])} is not supported." ) self.logger.error(e) @@ -878,7 +876,7 @@ class PhysicalDevice: try: # API call to Zabbix self.zabbix.hostinterface.update(updates) - e = (f"Host {self.name}: updated interface " + e = (f"Host {self.name}: Updated interface " f"with data {sanatize_log_output(updates)}.") self.logger.info(e) self.create_journal_entry("info", e) @@ -888,11 +886,11 @@ class PhysicalDevice: raise SyncExternalError(msg) from e else: # If no updates are found, Zabbix interface is in-sync - e = f"Host {self.name}: interface in-sync." + e = f"Host {self.name}: Interface in-sync." self.logger.debug(e) else: e = ( - f"Host {self.name} has unsupported interface configuration." + f"Host {self.name}: Has unsupported interface configuration." f" Host has total of {len(host['interfaces'])} interfaces. " "Manual intervention required." ) diff --git a/modules/tags.py b/modules/tags.py index 659966c..f341abd 100644 --- a/modules/tags.py +++ b/modules/tags.py @@ -76,7 +76,7 @@ class ZabbixTags: else: tag["tag"] = tag_name else: - self.logger.warning(f"Tag {tag_name} is not a valid tag name, skipping.") + self.logger.warning(f"Tag '{tag_name}' is not a valid tag name, skipping.") return False if self.validate_value(tag_value): @@ -85,8 +85,8 @@ class ZabbixTags: else: tag["value"] = tag_value else: - self.logger.warning( - f"Tag {tag_name} has an invalid value: '{tag_value}', skipping." + self.logger.info( + f"Tag '{tag_name}' has an invalid value: '{tag_value}', skipping." ) return False return tag @@ -99,7 +99,7 @@ class ZabbixTags: tags = [] # Parse the field mapper for tags if self.tag_map: - self.logger.debug(f"Host {self.nb.name}: Starting tag mapper") + self.logger.debug(f"Host {self.nb.name}: Starting tag mapper.") field_tags = field_mapper(self.nb.name, self.tag_map, self.nb, self.logger) for tag, value in field_tags.items(): t = self.render_tag(tag, value) diff --git a/modules/tools.py b/modules/tools.py index f3d27cc..c49f5dd 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -71,20 +71,20 @@ def field_mapper(host, mapper, nbdevice, logger): data[zbx_field] = str(value) elif not value: # empty value should just be an empty string for API compatibility - logger.debug( + logger.info( f"Host {host}: NetBox lookup for " - f"'{nb_field}' returned an empty value" + f"'{nb_field}' returned an empty value." ) data[zbx_field] = "" else: # Value is not a string or numeral, probably not what the user expected. - logger.error( + logger.info( f"Host {host}: Lookup for '{nb_field}'" " returned an unexpected type: it will be skipped." ) logger.debug( f"Host {host}: Field mapping complete. " - f"Mapped {len(list(filter(None, data.values())))} field(s)" + f"Mapped {len(list(filter(None, data.values())))} field(s)." ) return data @@ -151,7 +151,7 @@ def verify_hg_format(hg_format, device_cfs=None, vm_cfs=None, hg_type="dev", log f"Hostgroup item {hg_object} is not valid. Make sure you" " use valid items and separate them with '/'." ) - logger.error(e) + logger.warning(e) raise HostgroupError(e) diff --git a/modules/usermacros.py b/modules/usermacros.py index fd95eab..cfa8082 100644 --- a/modules/usermacros.py +++ b/modules/usermacros.py @@ -57,7 +57,7 @@ class ZabbixUsermacros: macro["macro"] = str(macro_name) if isinstance(macro_properties, dict): if not "value" in macro_properties: - self.logger.warning(f"Host {self.name}: Usermacro {macro_name} has " + self.logger.info(f"Host {self.name}: Usermacro {macro_name} has " "no value in Netbox, skipping.") return False macro["value"] = macro_properties["value"] @@ -83,11 +83,11 @@ class ZabbixUsermacros: macro["description"] = "" else: - self.logger.warning(f"Host {self.name}: Usermacro {macro_name} " + self.logger.info(f"Host {self.name}: Usermacro {macro_name} " "has no value, skipping.") return False else: - self.logger.error( + self.logger.warning( f"Host {self.name}: Usermacro {macro_name} is not a valid usermacro name, skipping." ) return False @@ -101,7 +101,7 @@ class ZabbixUsermacros: data={} # Parse the field mapper for usermacros if self.usermacro_map: - self.logger.debug(f"Host {self.nb.name}: Starting usermacro mapper") + self.logger.debug(f"Host {self.nb.name}: Starting usermacro mapper.") field_macros = field_mapper( self.nb.name, self.usermacro_map, self.nb, self.logger ) diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index 1515041..3e783e8 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -143,7 +143,7 @@ def main(arguments): try: vm = VirtualMachine(nb_vm, zabbix, netbox_journals, nb_version, config["create_journal"], logger) - logger.debug(f"Host {vm.name}: started operations on VM.") + logger.debug(f"Host {vm.name}: Started operations on VM.") vm.set_vm_template() # Check if a valid template has been found for this VM. if not vm.zbx_template_names: @@ -162,12 +162,12 @@ def main(arguments): # Delete device from Zabbix # and remove hostID from NetBox. vm.cleanup() - logger.debug(f"VM {vm.name}: cleanup complete") + logger.info(f"VM {vm.name}: cleanup complete") continue # Device has been added to NetBox # but is not in Activate state logger.info( - f"VM {vm.name}: skipping since this VM is " + f"VM {vm.name}: Skipping since this VM is " f"not in the active state." ) continue @@ -202,7 +202,7 @@ def main(arguments): # Set device instance set data such as hostgroup and template information. device = PhysicalDevice(nb_device, zabbix, netbox_journals, nb_version, config["create_journal"], logger) - logger.debug(f"Host {device.name}: started operations on device.") + logger.debug(f"Host {device.name}: Started operations on device.") device.set_template(config["templates_config_context"], config["templates_config_context_overrule"]) # Check if a valid template has been found for this VM. @@ -229,7 +229,7 @@ def main(arguments): # Device is secondary in cluster. # Don't continue with this device. e = ( - f"Device {device.name}: is part of cluster " + f"Device {device.name}: Is part of cluster " f"but not primary. Skipping this host..." ) logger.info(e) @@ -245,7 +245,7 @@ def main(arguments): # Device has been added to NetBox # but is not in Activate state logger.info( - f"Device {device.name}: skipping since this device is " + f"Device {device.name}: Skipping since this device is " f"not in the active state." ) continue @@ -282,7 +282,7 @@ if __name__ == "__main__": description="A script to sync Zabbix with NetBox device data." ) parser.add_argument( - "-v", "--verbose", help="Turn on debugging.", action="store_true" + "-v", "--verbose", help="Turn on verbose logging.", action="store_true" ) parser.add_argument( "-vv", "--debug", help="Turn on debugging.", action="store_true" From e0ec3c063275d4f4fa2ba8e8ab86e0f05478c037 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 10:54:39 +0200 Subject: [PATCH 15/23] updated usermacro test for new loglevels --- tests/test_usermacros.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_usermacros.py b/tests/test_usermacros.py index 28305af..5c2b6a4 100644 --- a/tests/test_usermacros.py +++ b/tests/test_usermacros.py @@ -88,7 +88,7 @@ class TestZabbixUsermacros(unittest.TestCase): macros = ZabbixUsermacros(self.nb, {}, False, logger=self.logger) result = macros.render_macro("{$FOO}", {"type": "text"}) self.assertFalse(result) - self.logger.warning.assert_called() + self.logger.info.assert_called() def test_render_macro_str(self): macros = ZabbixUsermacros(self.nb, {}, False, logger=self.logger) @@ -102,7 +102,7 @@ class TestZabbixUsermacros(unittest.TestCase): macros = ZabbixUsermacros(self.nb, {}, False, logger=self.logger) result = macros.render_macro("FOO", "bar") self.assertFalse(result) - self.logger.error.assert_called() + self.logger.warning.assert_called() def test_generate_from_map(self): nb = DummyNB(memory="bar", role="baz") From 57c7f83e6a5fb7c9a37c03a2fe79cf5d0830c6da Mon Sep 17 00:00:00 2001 From: Wouter de Bruijn Date: Wed, 25 Jun 2025 13:56:41 +0200 Subject: [PATCH 16/23] =?UTF-8?q?=F0=9F=94=8A=20Removed=20f-strings=20usag?= =?UTF-8?q?e=20from=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/device.py | 191 +++++++++++++++++++++++------------------- modules/hostgroups.py | 11 ++- modules/tags.py | 9 +- modules/tools.py | 89 +++++++++++--------- modules/usermacros.py | 29 +++++-- netbox_zabbix_sync.py | 93 ++++++++++++-------- 6 files changed, 246 insertions(+), 176 deletions(-) diff --git a/modules/device.py b/modules/device.py index 61a11c6..dcae69c 100644 --- a/modules/device.py +++ b/modules/device.py @@ -26,6 +26,7 @@ from modules.config import load_config config = load_config() + class PhysicalDevice: # pylint: disable=too-many-instance-attributes, too-many-arguments, too-many-positional-arguments """ @@ -97,7 +98,7 @@ class PhysicalDevice: if config["device_cf"] in self.nb.custom_fields: self.zabbix_id = self.nb.custom_fields[config["device_cf"]] else: - e = f'Host {self.name}: Custom field {config["device_cf"]} not present' + e = f"Host {self.name}: Custom field {config['device_cf']} not present" self.logger.error(e) raise SyncInventoryError(e) @@ -111,9 +112,10 @@ class PhysicalDevice: self.visible_name = self.nb.name self.use_visible_name = True self.logger.info( - f"Host {self.visible_name} contains special characters. " - f"Using {self.name} as name for the NetBox object " - f"and using {self.visible_name} as visible name in Zabbix." + "Host %s contains special characters. Using %s as name for the NetBox object and using %s as visible name in Zabbix.", + self.visible_name, + self.name, + self.visible_name, ) else: pass @@ -126,8 +128,8 @@ class PhysicalDevice: self.nb, self.nb_api_version, logger=self.logger, - nested_sitegroup_flag=config['traverse_site_groups'], - nested_region_flag=config['traverse_regions'], + nested_sitegroup_flag=config["traverse_site_groups"], + nested_region_flag=config["traverse_regions"], nb_groups=nb_site_groups, nb_regions=nb_regions, ) @@ -139,12 +141,12 @@ class PhysicalDevice: # Remove duplicates and None values self.hostgroups = list(filter(None, list(set(self.hostgroups)))) if self.hostgroups: - self.logger.debug(f"Host {self.name}: Should be member " - f"of groups: {self.hostgroups}") + self.logger.debug( + "Host %s: Should be member of groups: %s", self.name, self.hostgroups + ) return True return False - def set_template(self, prefer_config_context, overrule_custom): """Set Template""" self.zbx_template_names = None @@ -210,9 +212,10 @@ class PhysicalDevice: # Set inventory mode. Default is disabled (see class init function). if config["inventory_mode"] == "disabled": if config["inventory_sync"]: - self.logger.error(f"Host {self.name}: Unable to map NetBox inventory to Zabbix. " - "Inventory sync is enabled in " - "config but inventory mode is disabled.") + self.logger.error( + "Host %s: Unable to map NetBox inventory to Zabbix. Inventory sync is enabled in config but inventory mode is disabled", + self.name, + ) return True if config["inventory_mode"] == "manual": self.inventory_mode = 0 @@ -220,17 +223,20 @@ class PhysicalDevice: self.inventory_mode = 1 else: self.logger.error( - f"Host {self.name}: Specified value for inventory mode in" - f" config is not valid. Got value {config['inventory_mode']}" + "Host %s: Specified value for inventory mode in config is not valid. Got value %s", + self.name, + config["inventory_mode"], ) return False self.inventory = {} if config["inventory_sync"] and self.inventory_mode in [0, 1]: - self.logger.debug(f"Host {self.name}: Starting inventory mapper.") + self.logger.debug("Host %s: Starting inventory mapper.", self.name) self.inventory = field_mapper( self.name, self._inventory_map(), nbdevice, self.logger ) - self.logger.debug(f"Host {self.name}: Resolved inventory: {self.inventory}") + self.logger.debug( + "Host %s: Resolved inventory: %s", self.name, self.inventory + ) return True def isCluster(self): @@ -268,13 +274,14 @@ class PhysicalDevice: masterid = self.getClusterMaster() if masterid == self.id: self.logger.info( - f"Host {self.name} is primary cluster member. " - f"Modifying hostname from {self.name} to " - + f"{self.nb.virtual_chassis.name}." + "Host %s is primary cluster member. Modifying hostname from %s to %s.", + self.name, + self.name, + self.nb.virtual_chassis.name, ) self.name = self.nb.virtual_chassis.name return True - self.logger.info(f"Host {self.name} is non-primary cluster member.") + self.logger.info("Host %s is non-primary cluster member.", self.name) return False def zbxTemplatePrepper(self, templates): @@ -306,8 +313,10 @@ class PhysicalDevice: "name": zbx_template["name"], } ) - e = (f"Host {self.name}: Found template '{zbx_template['name']}' " - f"(ID:{zbx_template['templateid']})") + e = ( + f"Host {self.name}: Found template '{zbx_template['name']}' " + f"(ID:{zbx_template['templateid']})" + ) self.logger.debug(e) # Return error should the template not be found in Zabbix if not template_match: @@ -331,7 +340,7 @@ class PhysicalDevice: self.group_ids.append({"groupid": group["groupid"]}) e = ( f"Host {self.name}: Matched group " - f"\"{group['name']}\" (ID:{group['groupid']})" + f'"{group["name"]}" (ID:{group["groupid"]})' ) self.logger.debug(e) if len(self.group_ids) == len(self.hostgroups): @@ -412,7 +421,7 @@ class PhysicalDevice: macros = ZabbixUsermacros( self.nb, self._usermacro_map(), - config['usermacro_sync'], + config["usermacro_sync"], logger=self.logger, host=self.name, ) @@ -430,14 +439,14 @@ class PhysicalDevice: tags = ZabbixTags( self.nb, self._tag_map(), - tag_sync=config['tag_sync'], - tag_lower=config['tag_lower'], - tag_name=config['tag_name'], - tag_value=config['tag_value'], + tag_sync=config["tag_sync"], + tag_lower=config["tag_lower"], + tag_name=config["tag_name"], + tag_value=config["tag_value"], logger=self.logger, host=self.name, ) - if config['tag_sync'] is False: + if config["tag_sync"] is False: self.tags = [] return False self.tags = tags.generate() @@ -477,12 +486,12 @@ class PhysicalDevice: # If the proxy name matches if proxy["name"] == proxy_name: self.logger.debug( - f"Host {self.name}: using {proxy['type']}" f" '{proxy_name}'" + "Host %s: using {proxy['type']} '%s'", self.name, proxy_name ) self.zbxproxy = proxy return True self.logger.warning( - f"Host {self.name}: unable to find proxy {proxy_name}" + "Host %s: unable to find proxy %s", self.name, proxy_name ) return False @@ -554,7 +563,7 @@ class PhysicalDevice: self.create_journal_entry("success", msg) else: self.logger.error( - f"Host {self.name}: Unable to add to Zabbix. Host already present." + "Host %s: Unable to add to Zabbix. Host already present.", self.name ) def createZabbixHostgroup(self, hostgroups): @@ -612,7 +621,9 @@ class PhysicalDevice: ) self.logger.error(e) raise SyncExternalError(e) from None - self.logger.info(f"Host {self.name}: updated with data {sanatize_log_output(kwargs)}.") + self.logger.info( + "Host %s: updated with data %s.", self.name, sanatize_log_output(kwargs) + ) self.create_journal_entry("info", "Updated host in Zabbix with latest NB data.") def ConsistencyCheck( @@ -623,7 +634,7 @@ class PhysicalDevice: Checks if Zabbix object is still valid with NetBox parameters. """ # If group is found or if the hostgroup is nested - if not self.setZabbixGroupID(groups): # or len(self.hostgroups.split("/")) > 1: + if not self.setZabbixGroupID(groups): # or len(self.hostgroups.split("/")) > 1: if create_hostgroups: # Script is allowed to create a new hostgroup new_groups = self.createZabbixHostgroup(groups) @@ -672,28 +683,30 @@ class PhysicalDevice: raise SyncInventoryError(e) host = host[0] if host["host"] == self.name: - self.logger.debug(f"Host {self.name}: Hostname in-sync.") + self.logger.debug("Host %s: Hostname in-sync.", self.name) else: self.logger.info( - f"Host {self.name}: Hostname OUT of sync. " - f"Received value: {host['host']}" + "Host %s: Hostname OUT of sync. Received value: %s", + self.name, + host["host"], ) self.updateZabbixHost(host=self.name) # Execute check depending on wether the name is special or not if self.use_visible_name: if host["name"] == self.visible_name: - self.logger.debug(f"Host {self.name}: Visible name in-sync.") + self.logger.debug("Host %s: Visible name in-sync.", self.name) else: self.logger.info( - f"Host {self.name}: Visible name OUT of sync." - f" Received value: {host['name']}" + "Host %s: Visible name OUT of sync. Received value: %s", + self.name, + host["name"], ) self.updateZabbixHost(name=self.visible_name) # Check if the templates are in-sync if not self.zbx_template_comparer(host["parentTemplates"]): - self.logger.info(f"Host {self.name}: Template(s) OUT of sync.") + self.logger.info("Host %s: Template(s) OUT of sync.", self.name) # Prepare Templates for API parsing templateids = [] for template in self.zbx_templates: @@ -703,38 +716,41 @@ class PhysicalDevice: templates_clear=host["parentTemplates"], templates=templateids ) else: - self.logger.debug(f"Host {self.name}: Template(s) in-sync.") + self.logger.debug("Host %s: Template(s) in-sync.", self.name) # Check if Zabbix version is 6 or higher. Issue #93 group_dictname = "hostgroups" if str(self.zabbix.version).startswith(("6", "5")): group_dictname = "groups" # Check if hostgroups match - if (sorted(host[group_dictname], key=itemgetter('groupid')) == - sorted(self.group_ids, key=itemgetter('groupid'))): - self.logger.debug(f"Host {self.name}: Hostgroups in-sync.") + if sorted(host[group_dictname], key=itemgetter("groupid")) == sorted( + self.group_ids, key=itemgetter("groupid") + ): + self.logger.debug("Host %s: Hostgroups in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Hostgroups OUT of sync.") + self.logger.info("Host %s: Hostgroups OUT of sync.", self.name) self.updateZabbixHost(groups=self.group_ids) if int(host["status"]) == self.zabbix_state: - self.logger.debug(f"Host {self.name}: Status in-sync.") + self.logger.debug("Host %s: Status in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Status OUT of sync.") + self.logger.info("Host %s: Status OUT of sync.", self.name) self.updateZabbixHost(status=str(self.zabbix_state)) # Check if a proxy has been defined if self.zbxproxy: # Check if proxy or proxy group is defined - if (self.zbxproxy["idtype"] in host and - host[self.zbxproxy["idtype"]] == self.zbxproxy["id"]): - self.logger.debug(f"Host {self.name}: Proxy in-sync.") + if ( + self.zbxproxy["idtype"] in host + and host[self.zbxproxy["idtype"]] == self.zbxproxy["id"] + ): + self.logger.debug("Host %s: Proxy in-sync.", self.name) # Backwards compatibility for Zabbix <= 6 elif "proxy_hostid" in host and host["proxy_hostid"] == self.zbxproxy["id"]: - self.logger.debug(f"Host {self.name}: Proxy in-sync.") + self.logger.debug("Host %s: Proxy in-sync.", self.name) # Proxy does not match, update Zabbix else: - self.logger.info(f"Host {self.name}: Proxy OUT of sync.") + self.logger.info("Host %s: Proxy OUT of sync.", self.name) # Zabbix <= 6 patch if not str(self.zabbix.version).startswith("7"): self.updateZabbixHost(proxy_hostid=self.zbxproxy["id"]) @@ -757,8 +773,8 @@ class PhysicalDevice: if proxy_power and proxy_set: # Zabbix <= 6 fix self.logger.warning( - f"Host {self.name}: No proxy is configured in NetBox " - "but is configured in Zabbix. Removing proxy config in Zabbix" + "Host %s: No proxy is configured in NetBox but is configured in Zabbix. Removing proxy config in Zabbix", + self.name, ) if "proxy_hostid" in host and bool(host["proxy_hostid"]): self.updateZabbixHost(proxy_hostid=0) @@ -772,59 +788,59 @@ class PhysicalDevice: if proxy_set and not proxy_power: # Display error message self.logger.warning( - f"Host {self.name}: Is configured " - f"with proxy in Zabbix but not in NetBox. The" - " -p flag was ommited: no " - "changes have been made." + "Host %s: Is configured with proxy in Zabbix but not in NetBox. The -p flag was ommited: no changes have been made.", + self.name, ) if not proxy_set: - self.logger.debug(f"Host {self.name}: Proxy in-sync.") + self.logger.debug("Host %s: Proxy in-sync.", self.name) # Check host inventory mode if str(host["inventory_mode"]) == str(self.inventory_mode): - self.logger.debug(f"Host {self.name}: inventory_mode in-sync.") + self.logger.debug("Host %s: inventory_mode in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: inventory_mode OUT of sync.") + self.logger.info("Host %s: inventory_mode OUT of sync.", self.name) self.updateZabbixHost(inventory_mode=str(self.inventory_mode)) if config["inventory_sync"] and self.inventory_mode in [0, 1]: # Check host inventory mapping if host["inventory"] == self.inventory: - self.logger.debug(f"Host {self.name}: Inventory in-sync.") + self.logger.debug("Host %s: Inventory in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Inventory OUT of sync.") + self.logger.info("Host %s: Inventory OUT of sync.", self.name) self.updateZabbixHost(inventory=self.inventory) # Check host usermacros - if config['usermacro_sync']: + if config["usermacro_sync"]: # Make a full copy synce we dont want to lose the original value # of secret type macros from Netbox netbox_macros = deepcopy(self.usermacros) # Set the sync bit - full_sync_bit = bool(str(config['usermacro_sync']).lower() == "full") + full_sync_bit = bool(str(config["usermacro_sync"]).lower() == "full") for macro in netbox_macros: # If the Macro is a secret and full sync is NOT activated if macro["type"] == str(1) and not full_sync_bit: # Remove the value as the Zabbix api does not return the value key # This is required when you want to do a diff between both lists macro.pop("value") + # Sort all lists def filter_with_macros(macro): return macro["macro"] + host["macros"].sort(key=filter_with_macros) netbox_macros.sort(key=filter_with_macros) # Check if both lists are the same if host["macros"] == netbox_macros: - self.logger.debug(f"Host {self.name}: Usermacros in-sync.") + self.logger.debug("Host %s: Usermacros in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Usermacros OUT of sync.") + self.logger.info("Host %s: Usermacros OUT of sync.", self.name) # Update Zabbix with NetBox usermacros self.updateZabbixHost(macros=self.usermacros) # Check host tags - if config['tag_sync']: + if config["tag_sync"]: if remove_duplicates(host["tags"], sortkey="tag") == self.tags: - self.logger.debug(f"Host {self.name}: Tags in-sync.") + self.logger.debug("Host %s: Tags in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Tags OUT of sync.") + self.logger.info("Host %s: Tags OUT of sync.", self.name) self.updateZabbixHost(tags=self.tags) # If only 1 interface has been found @@ -862,7 +878,7 @@ class PhysicalDevice: updates[key] = item if updates: # If interface updates have been found: push to Zabbix - self.logger.info(f"Host {self.name}: Interface OUT of sync.") + self.logger.info("Host %s: Interface OUT of sync.", self.name) if "type" in updates: # Changing interface type not supported. Raise exception. e = ( @@ -876,26 +892,27 @@ class PhysicalDevice: try: # API call to Zabbix self.zabbix.hostinterface.update(updates) - e = (f"Host {self.name}: Updated interface " - f"with data {sanatize_log_output(updates)}.") - self.logger.info(e) - self.create_journal_entry("info", e) + err_msg = ( + f"Host {self.name}: Updated interface " + f"with data {sanatize_log_output(updates)}." + ) + self.logger.info(err_msg) + self.create_journal_entry("info", err_msg) except APIRequestError as e: msg = f"Zabbix returned the following error: {str(e)}." self.logger.error(msg) raise SyncExternalError(msg) from e else: # If no updates are found, Zabbix interface is in-sync - e = f"Host {self.name}: Interface in-sync." - self.logger.debug(e) + self.logger.debug("Host %s: Interface in-sync.", self.name) else: - e = ( + err_msg = ( f"Host {self.name}: Has unsupported interface configuration." f" Host has total of {len(host['interfaces'])} interfaces. " "Manual intervention required." ) - self.logger.error(e) - raise SyncInventoryError(e) + self.logger.error(err_msg) + raise SyncInventoryError(err_msg) def create_journal_entry(self, severity, message): """ @@ -906,7 +923,7 @@ class PhysicalDevice: # Check if the severity is valid if severity not in ["info", "success", "warning", "danger"]: self.logger.warning( - f"Value {severity} not valid for NB journal entries." + "Value %s not valid for NB journal entries.", severity ) return False journal = { @@ -917,12 +934,11 @@ class PhysicalDevice: } try: self.nb_journals.create(journal) - self.logger.debug(f"Host {self.name}: Created journal entry in NetBox") + self.logger.debug("Host %s: Created journal entry in NetBox", self.name) return True except NetboxRequestError as e: self.logger.warning( - "Unable to create journal entry for " - f"{self.name}: NB returned {e}" + "Unable to create journal entry for %s: NB returned {e}", self.name ) return False return False @@ -947,8 +963,9 @@ class PhysicalDevice: tmpls_from_zabbix.pop(pos) succesfull_templates.append(nb_tmpl) self.logger.debug( - f"Host {self.name}: Template " - f"'{nb_tmpl['name']}' is present in Zabbix." + "Host %s: Template '%s' is present in Zabbix.", + self.name, + nb_tmpl["name"], ) break if ( diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 213b4cf..58bb057 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -94,8 +94,11 @@ class Hostgroup: format_options["cluster"] = self.nb.cluster.name format_options["cluster_type"] = self.nb.cluster.type.name self.format_options = format_options - self.logger.debug(f"Host {self.name}: Resolved properties for use " - f"in hostgroups: {self.format_options}") + self.logger.debug( + "Host %s: Resolved properties for use in hostgroups: %s", + self.name, + self.format_options, + ) def set_nesting( self, nested_sitegroup_flag, nested_region_flag, nb_groups, nb_regions @@ -134,7 +137,9 @@ class Hostgroup: if hostgroup_value: hg_output.append(hostgroup_value) else: - self.logger.info(f"Host {self.name}: Used field '{hg_item}' has no value.") + self.logger.info( + "Host %s: Used field '%s' has no value.", self.name, hg_item + ) # Check if the hostgroup is populated with at least one item. if bool(hg_output): return "/".join(hg_output) diff --git a/modules/tags.py b/modules/tags.py index f341abd..835490c 100644 --- a/modules/tags.py +++ b/modules/tags.py @@ -3,6 +3,7 @@ """ All of the Zabbix Usermacro related configuration """ + from logging import getLogger from modules.tools import field_mapper, remove_duplicates @@ -76,7 +77,7 @@ class ZabbixTags: else: tag["tag"] = tag_name else: - self.logger.warning(f"Tag '{tag_name}' is not a valid tag name, skipping.") + self.logger.warning("Tag '%s' is not a valid tag name, skipping.", tag_name) return False if self.validate_value(tag_value): @@ -86,7 +87,7 @@ class ZabbixTags: tag["value"] = tag_value else: self.logger.info( - f"Tag '{tag_name}' has an invalid value: '{tag_value}', skipping." + "Tag '%s' has an invalid value: '%s', skipping.", tag_name, tag_value ) return False return tag @@ -99,7 +100,7 @@ class ZabbixTags: tags = [] # Parse the field mapper for tags if self.tag_map: - self.logger.debug(f"Host {self.nb.name}: Starting tag mapper.") + self.logger.debug("Host %s: Starting tag mapper.", self.nb.name) field_tags = field_mapper(self.nb.name, self.tag_map, self.nb, self.logger) for tag, value in field_tags.items(): t = self.render_tag(tag, value) @@ -131,5 +132,5 @@ class ZabbixTags: tags.append(t) tags = remove_duplicates(tags, sortkey="tag") - self.logger.debug(f"Host {self.name}: Resolved tags: {tags}") + self.logger.debug("Host %s: Resolved tags: %s", self.name, tags) return tags diff --git a/modules/tools.py b/modules/tools.py index c49f5dd..ae7a12b 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -1,6 +1,8 @@ """A collection of tools used by several classes""" + from modules.exceptions import HostgroupError + def convert_recordset(recordset): """Converts netbox RedcordSet to list of dicts.""" recordlist = [] @@ -72,19 +74,22 @@ def field_mapper(host, mapper, nbdevice, logger): elif not value: # empty value should just be an empty string for API compatibility logger.info( - f"Host {host}: NetBox lookup for " - f"'{nb_field}' returned an empty value." + "Host %s: NetBox lookup for '%s' returned an empty value.", + host, + nb_field, ) data[zbx_field] = "" else: # Value is not a string or numeral, probably not what the user expected. logger.info( - f"Host {host}: Lookup for '{nb_field}'" - " returned an unexpected type: it will be skipped." + "Host %s: Lookup for '%s' returned an unexpected type: it will be skipped.", + host, + nb_field, ) logger.debug( - f"Host {host}: Field mapping complete. " - f"Mapped {len(list(filter(None, data.values())))} field(s)." + "Host %s: Field mapping complete. Mapped %s field(s).", + host, + len(list(filter(None, data.values()))), ) return data @@ -101,7 +106,9 @@ def remove_duplicates(input_list, sortkey=None): return output_list -def verify_hg_format(hg_format, device_cfs=None, vm_cfs=None, hg_type="dev", logger=None): +def verify_hg_format( + hg_format, device_cfs=None, vm_cfs=None, hg_type="dev", logger=None +): """ Verifies hostgroup field format """ @@ -109,44 +116,51 @@ def verify_hg_format(hg_format, device_cfs=None, vm_cfs=None, hg_type="dev", log device_cfs = [] if not vm_cfs: vm_cfs = [] - allowed_objects = {"dev": ["location", - "rack", - "role", - "manufacturer", - "region", - "site", - "site_group", - "tenant", - "tenant_group", - "platform", - "cluster"] - ,"vm": ["cluster_type", - "role", - "manufacturer", - "region", - "site", - "site_group", - "tenant", - "tenant_group", - "cluster", - "device", - "platform"] - ,"cfs": {"dev": [], "vm": []} - } + allowed_objects = { + "dev": [ + "location", + "rack", + "role", + "manufacturer", + "region", + "site", + "site_group", + "tenant", + "tenant_group", + "platform", + "cluster", + ], + "vm": [ + "cluster_type", + "role", + "manufacturer", + "region", + "site", + "site_group", + "tenant", + "tenant_group", + "cluster", + "device", + "platform", + ], + "cfs": {"dev": [], "vm": []}, + } for cf in device_cfs: - allowed_objects['cfs']['dev'].append(cf.name) + allowed_objects["cfs"]["dev"].append(cf.name) for cf in vm_cfs: - allowed_objects['cfs']['vm'].append(cf.name) + allowed_objects["cfs"]["vm"].append(cf.name) hg_objects = [] - if isinstance(hg_format,list): + if isinstance(hg_format, list): for f in hg_format: hg_objects = hg_objects + f.split("/") else: hg_objects = hg_format.split("/") hg_objects = sorted(set(hg_objects)) for hg_object in hg_objects: - if (hg_object not in allowed_objects[hg_type] and - hg_object not in allowed_objects['cfs'][hg_type]): + if ( + hg_object not in allowed_objects[hg_type] + and hg_object not in allowed_objects["cfs"][hg_type] + ): e = ( f"Hostgroup item {hg_object} is not valid. Make sure you" " use valid items and separate them with '/'." @@ -168,8 +182,7 @@ def sanatize_log_output(data): if "macros" in data: for macro in sanitized_data["macros"]: # Check if macro is secret type - if not (macro["type"] == str(1) or - macro["type"] == 1): + if not (macro["type"] == str(1) or macro["type"] == 1): continue macro["value"] = "********" # Check for interface data diff --git a/modules/usermacros.py b/modules/usermacros.py index cfa8082..acf8725 100644 --- a/modules/usermacros.py +++ b/modules/usermacros.py @@ -3,6 +3,7 @@ """ All of the Zabbix Usermacro related configuration """ + from logging import getLogger from re import match @@ -57,8 +58,11 @@ class ZabbixUsermacros: macro["macro"] = str(macro_name) if isinstance(macro_properties, dict): if not "value" in macro_properties: - self.logger.info(f"Host {self.name}: Usermacro {macro_name} has " - "no value in Netbox, skipping.") + self.logger.info( + "Host %s: Usermacro %s has no value in Netbox, skipping.", + self.name, + macro_name, + ) return False macro["value"] = macro_properties["value"] @@ -83,12 +87,17 @@ class ZabbixUsermacros: macro["description"] = "" else: - self.logger.info(f"Host {self.name}: Usermacro {macro_name} " - "has no value, skipping.") + self.logger.info( + "Host %s: Usermacro %s has no value, skipping.", + self.name, + macro_name, + ) return False else: self.logger.warning( - f"Host {self.name}: Usermacro {macro_name} is not a valid usermacro name, skipping." + "Host %s: Usermacro %s is not a valid usermacro name, skipping.", + self.name, + macro_name, ) return False return macro @@ -98,10 +107,10 @@ class ZabbixUsermacros: Generate full set of Usermacros """ macros = [] - data={} + data = {} # Parse the field mapper for usermacros if self.usermacro_map: - self.logger.debug(f"Host {self.nb.name}: Starting usermacro mapper.") + self.logger.debug("Host %s: Starting usermacro mapper.", self.nb.name) field_macros = field_mapper( self.nb.name, self.usermacro_map, self.nb, self.logger ) @@ -120,6 +129,8 @@ class ZabbixUsermacros: m = self.render_macro(macro, properties) if m: macros.append(m) - data={'macros': macros} - self.logger.debug(f"Host {self.name}: Resolved macros: {sanatize_log_output(data)}") + data = {"macros": macros} + self.logger.debug( + "Host %s: Resolved macros: %s", self.name, sanatize_log_output(data) + ) return macros diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index 3e783e8..79fa27e 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -2,6 +2,7 @@ # pylint: disable=invalid-name, logging-not-lazy, too-many-locals, logging-fstring-interpolation """NetBox to Zabbix sync script.""" + import argparse import logging import ssl @@ -67,15 +68,15 @@ def main(arguments): try: # Get NetBox version nb_version = netbox.version - logger.debug(f"NetBox version is {nb_version}.") + logger.debug("NetBox version is %s.", nb_version) except RequestsConnectionError: logger.error( - f"Unable to connect to NetBox with URL {netbox_host}." - " Please check the URL and status of NetBox." + "Unable to connect to NetBox with URL %s. Please check the URL and status of NetBox.", + netbox_host, ) sys.exit(1) except NBRequestError as e: - logger.error(f"NetBox error: {e}") + logger.error("NetBox error: %s", e) sys.exit(1) # Check if the provided Hostgroup layout is valid device_cfs = [] @@ -83,14 +84,18 @@ def main(arguments): device_cfs = list( netbox.extras.custom_fields.filter(type="text", content_types="dcim.device") ) - verify_hg_format(config["hostgroup_format"], - device_cfs=device_cfs, hg_type="dev", logger=logger) + verify_hg_format( + config["hostgroup_format"], device_cfs=device_cfs, hg_type="dev", logger=logger + ) if config["sync_vms"]: vm_cfs = list( - netbox.extras.custom_fields.filter(type="text", - content_types="virtualization.virtualmachine") + netbox.extras.custom_fields.filter( + type="text", content_types="virtualization.virtualmachine" + ) + ) + verify_hg_format( + config["vm_hostgroup_format"], vm_cfs=vm_cfs, hg_type="vm", logger=logger ) - verify_hg_format(config["vm_hostgroup_format"], vm_cfs=vm_cfs, hg_type="vm", logger=logger) # Set Zabbix API try: ssl_ctx = ssl.create_default_context() @@ -120,7 +125,8 @@ def main(arguments): netbox_vms = [] if config["sync_vms"]: netbox_vms = list( - netbox.virtualization.virtual_machines.filter(**config["nb_vm_filter"])) + netbox.virtualization.virtual_machines.filter(**config["nb_vm_filter"]) + ) netbox_site_groups = convert_recordset((netbox.dcim.site_groups.all())) netbox_regions = convert_recordset(netbox.dcim.regions.all()) netbox_journals = netbox.extras.journal_entries @@ -141,15 +147,22 @@ def main(arguments): # Go through all NetBox devices for nb_vm in netbox_vms: try: - vm = VirtualMachine(nb_vm, zabbix, netbox_journals, nb_version, - config["create_journal"], logger) - logger.debug(f"Host {vm.name}: Started operations on VM.") + vm = VirtualMachine( + nb_vm, + zabbix, + netbox_journals, + nb_version, + config["create_journal"], + logger, + ) + logger.debug("Host %s: Started operations on VM.", vm.name) vm.set_vm_template() # Check if a valid template has been found for this VM. if not vm.zbx_template_names: continue - vm.set_hostgroup(config["vm_hostgroup_format"], - netbox_site_groups, netbox_regions) + vm.set_hostgroup( + config["vm_hostgroup_format"], netbox_site_groups, netbox_regions + ) # Check if a valid hostgroup has been found for this VM. if not vm.hostgroups: continue @@ -162,13 +175,12 @@ def main(arguments): # Delete device from Zabbix # and remove hostID from NetBox. vm.cleanup() - logger.info(f"VM {vm.name}: cleanup complete") + logger.info("VM %s: cleanup complete", vm.name) continue # Device has been added to NetBox # but is not in Activate state logger.info( - f"VM {vm.name}: Skipping since this VM is " - f"not in the active state." + "VM %s: Skipping since this VM is not in the active state.", vm.name ) continue # Check if the VM is in the disabled state @@ -200,20 +212,31 @@ def main(arguments): for nb_device in netbox_devices: try: # Set device instance set data such as hostgroup and template information. - device = PhysicalDevice(nb_device, zabbix, netbox_journals, nb_version, - config["create_journal"], logger) - logger.debug(f"Host {device.name}: Started operations on device.") - device.set_template(config["templates_config_context"], - config["templates_config_context_overrule"]) + device = PhysicalDevice( + nb_device, + zabbix, + netbox_journals, + nb_version, + config["create_journal"], + logger, + ) + logger.debug("Host %s: Started operations on device.", device.name) + device.set_template( + config["templates_config_context"], + config["templates_config_context_overrule"], + ) # Check if a valid template has been found for this VM. if not device.zbx_template_names: continue device.set_hostgroup( - config["hostgroup_format"], netbox_site_groups, netbox_regions) + config["hostgroup_format"], netbox_site_groups, netbox_regions + ) # Check if a valid hostgroup has been found for this VM. if not device.hostgroups: - logger.warning(f"Host {device.name}: Host has no valid " - f"hostgroups, Skipping this host...") + logger.warning( + "Host %s: Host has no valid hostgroups, Skipping this host...", + device.name, + ) continue device.set_inventory(nb_device) device.set_usermacros() @@ -223,16 +246,16 @@ def main(arguments): if device.isCluster() and config["clustering"]: # Check if device is primary or secondary if device.promoteMasterDevice(): - e = f"Device {device.name}: is " f"part of cluster and primary." - logger.info(e) + logger.info( + "Device %s: is part of cluster and primary.", device.name + ) else: # Device is secondary in cluster. # Don't continue with this device. - e = ( - f"Device {device.name}: Is part of cluster " - f"but not primary. Skipping this host..." + logger.info( + "Device %s: Is part of cluster but not primary. Skipping this host...", + device.name, ) - logger.info(e) continue # Checks if device is in cleanup state if device.status in config["zabbix_device_removal"]: @@ -240,13 +263,13 @@ def main(arguments): # Delete device from Zabbix # and remove hostID from NetBox. device.cleanup() - logger.info(f"Device {device.name}: cleanup complete") + logger.info("Device %s: cleanup complete", device.name) continue # Device has been added to NetBox # but is not in Activate state logger.info( - f"Device {device.name}: Skipping since this device is " - f"not in the active state." + "Device %s: Skipping since this device is not in the active state.", + device.name, ) continue # Check if the device is in the disabled state From e71856068985e39b237cbac17f5808cf2cfdc1ed Mon Sep 17 00:00:00 2001 From: Wouter de Bruijn Date: Wed, 25 Jun 2025 16:37:44 +0200 Subject: [PATCH 17/23] =?UTF-8?q?=F0=9F=9A=A8=20Line=20length=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/device.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/device.py b/modules/device.py index dcae69c..cf0e2a2 100644 --- a/modules/device.py +++ b/modules/device.py @@ -112,7 +112,8 @@ class PhysicalDevice: self.visible_name = self.nb.name self.use_visible_name = True self.logger.info( - "Host %s contains special characters. Using %s as name for the NetBox object and using %s as visible name in Zabbix.", + "Host %s contains special characters." + "Using %s as name for the NetBox object and using %s as visible name in Zabbix.", self.visible_name, self.name, self.visible_name, @@ -213,7 +214,8 @@ class PhysicalDevice: if config["inventory_mode"] == "disabled": if config["inventory_sync"]: self.logger.error( - "Host %s: Unable to map NetBox inventory to Zabbix. Inventory sync is enabled in config but inventory mode is disabled", + "Host %s: Unable to map NetBox inventory to Zabbix." + "Inventory sync is enabled in config but inventory mode is disabled", self.name, ) return True @@ -773,7 +775,8 @@ class PhysicalDevice: if proxy_power and proxy_set: # Zabbix <= 6 fix self.logger.warning( - "Host %s: No proxy is configured in NetBox but is configured in Zabbix. Removing proxy config in Zabbix", + "Host %s: No proxy is configured in NetBox but is configured in Zabbix." + "Removing proxy config in Zabbix", self.name, ) if "proxy_hostid" in host and bool(host["proxy_hostid"]): @@ -788,7 +791,8 @@ class PhysicalDevice: if proxy_set and not proxy_power: # Display error message self.logger.warning( - "Host %s: Is configured with proxy in Zabbix but not in NetBox. The -p flag was ommited: no changes have been made.", + "Host %s: Is configured with proxy in Zabbix but not in NetBox." + "The -p flag was ommited: no changes have been made.", self.name, ) if not proxy_set: @@ -938,7 +942,9 @@ class PhysicalDevice: return True except NetboxRequestError as e: self.logger.warning( - "Unable to create journal entry for %s: NB returned {e}", self.name + "Unable to create journal entry for %s: NB returned %s", + self.name, + e, ) return False return False From 98c13919c570a63564240186a4cac5094231e035 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 16:50:17 +0200 Subject: [PATCH 18/23] Added support for hardcoded strings in hostgroups --- modules/hostgroups.py | 31 ++++++++++++++++++------------- modules/tools.py | 1 + 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 58bb057..6da099a 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -117,19 +117,24 @@ class Hostgroup: for hg_item in hg_items: # Check if requested data is available as option for this host if hg_item not in self.format_options: - # Check if a custom field exists with this name - cf_data = self.custom_field_lookup(hg_item) - # CF does not exist - if not cf_data["result"]: - msg = ( - f"Unable to generate hostgroup for host {self.name}. " - f"Item type {hg_item} not supported." - ) - self.logger.error(msg) - raise HostgroupError(msg) - # CF data is populated - if cf_data["cf"]: - hg_output.append(cf_data["cf"]) + if hg_item.startswith(("'", '"')) and hg_item.endswith(("'", '"')): + hg_item = hg_item.strip("\'") + hg_item = hg_item.strip('\"') + hg_output.append(hg_item) + else: + # Check if a custom field exists with this name + cf_data = self.custom_field_lookup(hg_item) + # CF does not exist + if not cf_data["result"]: + msg = ( + f"Unable to generate hostgroup for host {self.name}. " + f"Item type {hg_item} not supported." + ) + self.logger.error(msg) + raise HostgroupError(msg) + # CF data is populated + if cf_data["cf"]: + hg_output.append(cf_data["cf"]) continue # Check if there is a value associated to the variable. # For instance, if a device has no location, do not use it with hostgroup calculation diff --git a/modules/tools.py b/modules/tools.py index ae7a12b..dacab20 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -160,6 +160,7 @@ def verify_hg_format( if ( hg_object not in allowed_objects[hg_type] and hg_object not in allowed_objects["cfs"][hg_type] + and not hg_object.startswith(('"',"'")) ): e = ( f"Hostgroup item {hg_object} is not valid. Make sure you" From 3910e0de2d04a779d79a0aeee9c355657319b4d4 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 16:54:12 +0200 Subject: [PATCH 19/23] Updated docs --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index ef280cc..169aca0 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,17 @@ in `config.py` the script will render a full region path of all parent regions for the hostgroup name. `traverse_site_groups` controls the same behaviour for site_groups. +**Hardcoded text** + +You can add hardcoded text in the hostgroup format by using quotes, this will +insert the literal text: + +```python +hostgroup_format = "'MyDevices'/location/role" +``` + +In this case, the prefix MyDevices will be used for all generated groups. + **Custom fields** You can use the value of custom fields for hostgroup generation. This allows From e82c098e26603d07fc67aff9ed6ccd9c9e101c8c Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 17:00:04 +0200 Subject: [PATCH 20/23] corrected linting error --- modules/hostgroups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 6da099a..954c218 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -120,7 +120,7 @@ class Hostgroup: if hg_item.startswith(("'", '"')) and hg_item.endswith(("'", '"')): hg_item = hg_item.strip("\'") hg_item = hg_item.strip('\"') - hg_output.append(hg_item) + hg_output.append(hg_item) else: # Check if a custom field exists with this name cf_data = self.custom_field_lookup(hg_item) From 161b310ba379afdd91be7240549823205db85e41 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 17:07:46 +0200 Subject: [PATCH 21/23] corrected linting error --- modules/hostgroups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 954c218..5916c0a 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -120,7 +120,7 @@ class Hostgroup: if hg_item.startswith(("'", '"')) and hg_item.endswith(("'", '"')): hg_item = hg_item.strip("\'") hg_item = hg_item.strip('\"') - hg_output.append(hg_item) + hg_output.append(hg_item) else: # Check if a custom field exists with this name cf_data = self.custom_field_lookup(hg_item) From c58a3e8dd50b10d994b0e35eaf30cd6ee03334f4 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Thu, 26 Jun 2025 09:48:25 +0200 Subject: [PATCH 22/23] Update README.md Replaced dependency pyzabbix with zabbix-utils as this was changed a few months ago. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 169aca0..e6cabfa 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ pip. ```sh # Packages: pynetbox -pyzabbix +zabbix-utils # Install them through requirements.txt from a venv: virtualenv .venv From b81d4abfcd0d70cc0df927601346533fcf280dc4 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 23 Sep 2025 12:47:05 +0200 Subject: [PATCH 23/23] Add support for Zabbix 7.4 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 295b59f..1a3470f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,2 @@ pynetbox==7.4.1 -zabbix-utils==2.0.2 +zabbix-utils==2.0.3